fix(layering): unblock main — raise the daemon-server R9 ceiling for #1779's snapshot interactor seam - #1835
Closed
thymikee wants to merge 1 commit into
Closed
fix(layering): unblock main — raise the daemon-server R9 ceiling for #1779's snapshot interactor seam#1835thymikee wants to merge 1 commit into
thymikee wants to merge 1 commit into
Conversation
…teractor seam `pnpm check:layering` is red on main: R9 reports 47 files (baseline 46) and R10 reports 17 daemon-server files (baseline 16). Two PRs that were each green against their own base: - #1825 (ef6ec29) made the R9 shrink mandatory and pinned LARGEST_TYPE_CYCLE_ZONE_CEILINGS to the then-measured daemon-server 16 / total 46. - #1779 (d76e0f9) added `src/daemon/handlers/snapshot-interactor-capture.ts`, a seam wedged between two existing cycle members: `handlers/snapshot-capture.ts` -> `handlers/snapshot-interactor-capture.ts` -> `core/interactors.ts` (getInteractor). A choke point between two SCC members always joins the SCC, so it is exactly +1 node. Member-list diff at a853734 vs d76e0f9 confirms `snapshot-interactor-capture.ts` is the only file that joined; `daemon-command-registry.ts` named in the R10 message was already a member (the message reports the alphabetically first zone member, not the file that grew). Zero-growth hosting would mean deleting the seam and moving the call back into `snapshot-capture.ts` or `core/interactors.ts`; the seam is the mock point for 43 test files, so that is a real refactor and not a main-unblocking change. Raising the ceiling explicitly instead, so the growth is a number a reviewer saw move.
Size Report
Startup median (7 runs, lower is better):
Top changed chunks: no changes in the largest emitted chunks. |
Member
Author
|
Superseded by maintainer fix. |
|
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
1 participant
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.
main is red on the Layering Guard job. This unblocks it. Not a draft.
Cause: two green PRs, one interaction
Neither PR is wrong; they were green against their own bases and never met until merge.
ef6ec2995, "make R9 shrink mandatory") pinnedLARGEST_TYPE_CYCLE_ZONE_CEILINGSto the then-measured values:daemon-server: 16, total46. The shrink is now mandatory, so the ceiling is an equality pin, not headroom.d76e0f94e, "migrate snapshot to device runtime") merged after it and added one new file that joins the largest type-level cycle.pnpm check:layeringon9a0d6dead:The joining edge
The +1 node is
src/daemon/handlers/snapshot-interactor-capture.ts(new ind76e0f94e), a choke point wedged between two files that were already cycle members:A new file that value-imports one SCC member and is value-imported by another always joins the SCC — +1 node, no new edges.
Attribution is a measured member-list diff of
largestTypeCycleMembers, taken ata853734f0(#1779's parent, 46 members) vsd76e0f94e(47 members). The only differing line is+ src/daemon/handlers/snapshot-interactor-capture.ts.Note on the R10 message: it names
src/daemon/daemon-command-registry.ts, which is not the file that grew — that file was already a member ata853734f0.checkTypeCycleBaselinereportsmembers.find(m => targetDagZone(m) === zone), i.e. the alphabetically first member of the over-budget zone. Worth a follow-up; deliberately not touched here.The fix: explicit ceiling raise (option 3 of the brief)
daemon-server: 16 -> 17inscripts/layering/daemon-modularity.ts, with a one-line comment namingd76e0f94eand the joining edge.TYPE_CYCLE_BASELINEis derived from the zone ceilings, so the total follows to 47 automatically.daemon-modularity.test.tspins both numbers as literals, so they move with it.I did not break the edge, and the maintainer should decide whether that is acceptable. The zero-growth shape (per the #1633 precedent: host the seam in an existing cycle node) means deleting
snapshot-interactor-capture.tsand moving thegetInteractorcall back intosnapshot-capture.tsor up intocore/interactors.ts. That module is the deliberatevi.mockseam for 43 test files (grep -rl snapshot-interactor-capture src→ 44 including itself); relocating it changes every one of those mock paths to a module whose other exports those tests still need unmocked. That is a real refactor, not a red-main fix. Happy to follow up with it as a separate PR if preferred — this PR only restores the gate to matching reality.check:layering after the fix
Test plan
pnpm check:layering— green (47 total / 17 daemon-server), 178/178 layering unit tests passpnpm typecheck— greenpnpm lint— greenpnpm format:check— greenscripts/layering/daemon-modularity.tsand its test.