refactor(architecture): extract connection routing into routing.mjs - #266
Open
AzScep wants to merge 2 commits into
Open
refactor(architecture): extract connection routing into routing.mjs#266AzScep wants to merge 2 commits into
AzScep wants to merge 2 commits into
Conversation
Moves the connection-routing block out of render-architecture.mjs into a `createRouter(components, connections)` factory. Function bodies are unchanged; the only edit is that `components` and `connections` arrive as arguments instead of module scope, and each router owns its own path cache and port spread. Why: render-architecture.mjs is a top-level script, so the router is reachable only by running a whole render pass. Anything that needs to ask "what would this route look like?" for a scene it is still deciding - tooling, a test, a future placement pass - currently cannot, and the alternative is reimplementing routeVia and letting the copy drift. The region was already self-contained: 309 lines with exactly two references to outer scope (`components`, and `arch.connections` for the port spread). Ten geometry imports it solely owned move with it, and no symbol it defines is used elsewhere in the file. No behavior change. test/golden.mjs byte-compares fresh renders of every checked-in example against the committed HTML and passes unchanged, which is the property this refactor is asserting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The renderer payload gained renderers/architecture/routing.mjs, so the committed archive no longer reproduces from tracked inputs. Rebuilt with the canonical Node 22 toolchain via scripts/build-zip.sh. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
renderers/architecture/render-architecture.mjsis a top-level script: it readsprocess.argv, awaits at module scope, and callsprocess.exit. The connection router lives inside it and closes over module-scopecomponents, so the only way to route anything is to run a whole render pass over a document on disk.That makes the router unreachable for any caller that wants to ask "what would this route look like?" about a scene it is still deciding — tooling, a targeted test, or a placement pass. The alternative is reimplementing
routeVia, and a second copy of six candidate families plus the near-parallel-port special cases will drift from this one silently.No linked issue: this changes no schema, contract, validation rule, or output, so it did not seem to warrant a planning issue. Happy to open one if you would rather discuss the seam first.
Change
Moves the routing block into
renderers/architecture/routing.mjs, exporting:Each router owns its own
pathCacheand its ownautomaticPortSpreadcall. Function bodies are unchanged — the only edit is thatcomponentsandconnectionsarrive as arguments rather than module scope.The region was already self-contained: 309 lines with exactly two references to outer scope (
components, andarch.connectionsfor the port spread). No symbol it defines is used elsewhere in the file, and the tengeometry.mjsimports it solely owned move with it.Moved verbatim:
routeClearsComponents,routeClearsEndpointComponents,OUTWARD_SIDE_VECTOR,outwardStub,collinearBacktrack,sideAwareBridgeCandidates,AUTOMATIC_PORT_CORNER_GUTTER,AUTOMATIC_PORT_ALIGNMENT_DELTA,portHasCornerClearance,alignFacingPorts,routeVia,connectionSides,connectionEndpointSide,pathFor.Compatibility
No behavior change, and the give-up branch's diagnostic contract is untouched — the router still returns a knowingly-bad route so validation reports the real obstacle rather than second-guessing what it was told.
Evidence
npm test— 1021 tests, 994 pass, 0 fail, 27 skipped, on this branch from currentmain.node test/golden.mjs— passes. This is the load-bearing check: it byte-compares fresh renders of every checked-in example against the committed HTML, so an identical result is exactly the property a pure move should have.scripts/generate-validators.mjs --checkandgenerate-brand-marks.mjs --check— in sync, untouched.Regenerated files
archify.zip— the renderer payload gained a file, so the committed archive no longer reproduced from tracked inputs. Rebuilt withscripts/build-zip.shon Node 22. Nothing else was regenerated.Disclosure
I built an automatic-layout mode for
architectureon a fork, which is what surfaced this: a solver has to score candidate placements against routed geometry, and an approximate router approves layouts the real one then rejects. I am not proposing that feature here —ROADMAP.mdis clear that it is out of scope, and this extraction is useful on its own or not at all. Judge it as a refactor.