perf(contracts): granularize entry surfaces so hub importers stop evaluating the facade clump - #1969
Merged
Merged
Conversation
…luating the facade clump `@agent-device/contracts/platform` unions 32 vocabulary modules and `/interaction` another 18. A file that value-imports either evaluates the whole union to reach one function, and because permanent hubs sat behind them — `command-descriptor/registry.ts`, `core/capabilities.ts`, `interactors/register-builtins.ts`, `command-descriptor/platform-execution-entry.ts` — that union rode into roughly half the unit suite's test graphs. Give every vocabulary module its own entry subpath and move all value-importers onto the module that owns the symbol. Type-only importers are left alone: `import type` is erased, so it already evaluated nothing. Measured with the #1950 eager-import-closure walker over all 974 unit-core test files, against base e5bfde3: aggregate eager module evaluations 143,248 -> 129,738 (-9.4%) facades/platform.ts carried by 466 -> 1 test graphs facades/interaction.ts carried by 451 -> 0 test graphs registry.ts 105 -> 66 capabilities.ts 113 -> 76 register-builtins.ts 111 -> 73 platform-execution-entry.ts 43 -> 3 dispatch.ts 134 -> 100 Three gate adjustments the split forces: - R11's pinned contracts subpath list grows to the new entries, and the resolver test's "must not resolve" example moves to `./clipboard`, since `./gesture-plan` is now a real entry. - R16 anchored the record-runtime join on the literal `contracts/platform` specifier. It now accepts any contracts entry — the assertion's provenance is what the rule pins, not which subpath carried it. - `gesture-plan.ts` became an entry target, and the no-bare-star rule rejects the `export * from './gesture-plan-types.ts'` it carried. Its one internal consumer now imports the owning module directly. Both facades keep their type re-exports for the ~490 type-only importers, so every symbol on them now reads as value-unused; one fallow entry records that and names retiring them as the follow-up. Closes #1959
Size Report
npm unpacked components
Startup median (7 runs, lower is better):
Top changed chunks:
Top changed packed files
|
The repo-wide scan parsed all ~3000 sources, which the coverage lane's instrumentation pushed past both the 5s test timeout and the 2.5s slow-test budget. A file that never names the specifier cannot import it, so filter on the text first and parse only the ~490 candidates. Non-vacuity moves with it: instead of counting narrow imports across every file, require that the surviving type-only importers were seen and classified as erased — which an empty scan cannot satisfy.
Member
Author
|
Reviewed exact head |
thymikee
added
the
ready-for-human
|
thymikee
added a commit
that referenced
this pull request
Aug 22, 2026
… reseed tight Review findings on #1965: 1. Discovery scanned only `packages/*/src/facades/*.ts`, which omits every package that publishes its entry surface straight from the manifest — including all six `packages/platform-*/src/index.ts` façades, the exact subject of ADR-0019's implementation-laziness rule. Discovery now derives from `readWorkspacePackages(...).exportTargets` and then adds `/src/facades/` files, reusing the R11 helper rather than reimplementing it so the two gates cannot disagree about what an entry surface is. The table grows from 14 façades + 2 hubs to 95 entry surfaces + 8 hubs. 2. Budgets carried a few files of slack each. They are now exact ratchets with no headroom, matching how the repo pins R9/R10 and test-file size: growth is allowed, it just has to be a visible number change in the diff of the PR that causes it. Every budget is reseeded from post-#1969 measurement. 3. Violations printed a flat sorted set, which named the offender but not the route. `eagerClosureGraphOf` records each file's discoverer, so failures now print the transitive chain entry -> ... -> offender. `eagerClosureOf` keeps its contract and is expressed in terms of the new walk; per-file edges are memoized, which also cuts the existing pins' runtime (cli closure test 2336ms -> ~550ms). The platform-package façades evaluate exactly one module each — themselves — so their budget of 1 is the tightest statement of "metadata-eager, implementation-lazy" the walker can make.
thymikee
added a commit
that referenced
this pull request
Aug 22, 2026
…equest-bound-migration-803b60 * origin/main: perf(contracts): granularize entry surfaces so hub importers stop evaluating the facade clump (#1969) fix(ios): preserve regular snapshot depth through structural wrappers (#1947) diagnose(1874): instrument the synthesized commit wait and add a dispatchable stall loop (#1941) refactor(tests): replace the test-utils barrel with direct module imports (#1956) fix(ios): enforce regular snapshot clip invariant (#1946) refactor(daemon): lazy-load platform cleanup helpers in session teardown (#1950) perf(typecheck): make the root tsc project incremental (#1957) docs: drop stale apps.ts over-budget warning from AGENTS.md (#1954) refactor: dedupe containsPoint and rectArea into @agent-device/kernel (#1953) # Conflicts: # packages/platform-android/src/runtime.ts # packages/platform-harmonyos/src/runtime.ts # packages/platform-linux/src/runtime.ts # packages/platform-vega/src/runtime.ts # packages/platform-web/src/runtime.ts # packages/provider-limrun/src/app-log-runtime.test.ts # packages/provider-limrun/src/interaction-operations.ts # packages/provider-webdriver/src/platform-runtime.ts # src/core/command-descriptor/registry.ts # src/core/dispatch.ts # src/platforms/apple/plugin.ts
thymikee
added a commit
that referenced
this pull request
Aug 22, 2026
…/tv-remote/keyboard Following main's #1969 (facade granularization), give each of this branch's five new contract modules their own package.json entry subpath and move every value-importer (owner runtime packages, the daemon binders, and their tests) off the wide @agent-device/contracts/platform facade onto the specific module that owns the symbol — the same convention #1969 established for the rest of the vocabulary. Keeps this migration's files out of the contracts-entry-closure gate and out of the eager-evaluation cost #1969 measured for the daemon's permanent hubs (registry.ts, dispatch.ts).
This was referenced Aug 22, 2026
thymikee
added a commit
that referenced
this pull request
Aug 22, 2026
Stryker runs the suite from a sandbox copy under `.tmp/stryker/`, so a test that asserts about the repository checkout itself — its files on disk, or its git history — reads a repository that does not exist, and no answer it gives there means anything. `src/__tests__/test-file-size-ratchet.test.ts` is such a gate, and it fails in the sandbox for two independent reasons. Stryker's `disableTypeChecks` default prepends `// @ts-nocheck` to every copied file, so all 26 pinned files read one line longer than they are; and the sandbox has no `origin/main`, so the gate's history-backed half cannot resolve its merge-base. Fixing either leaves the other — turning off the type-check rewrite was tried first, and CI then failed on the missing ref. The gate's own `.tmp` skip entry cannot save it either: that is matched relative to `REPO_ROOT`, which inside the sandbox *is* the sandbox. Excluding it hides nothing, for the reason this module's header already gives: a mutant only an excluded test could kill shows up as a survivor. A repo-shape gate cannot kill a kernel mutant at all — it asserts about file lengths and history, never about behavior under mutation. Derived rather than listed, matching the neighbouring thread-hostile exclusion, so a future sibling gate is caught the same way. Nothing was wrong with the ratchet or with #1964; before #1969 rewired the module graph, nothing pulled this gate into a lane's scope and the trap stayed hidden. Verified with `pnpm mutation:run --modules kernel-errors`: scope 804 -> 803 test files (exactly this one), dry run clean, lane `pass` in 560s, score 75.2% (188 killed / 62 survived / 250). `pnpm mutation:test` 39/39. `stryker.config.json` is deliberately untouched, so the config content hash the report and lane envelope carry is unchanged and scores stay comparable. Unblocks #1964, whose two mutation checks fail on main's tip without its code.
thymikee
added a commit
that referenced
this pull request
Aug 22, 2026
Stryker runs the suite from a sandbox copy under `.tmp/stryker/`, so a test that asserts about the repository checkout itself — its files on disk, or its git history — reads a repository that does not exist, and no answer it gives there means anything. `src/__tests__/test-file-size-ratchet.test.ts` is such a gate, and it fails in the sandbox for two independent reasons. Stryker's `disableTypeChecks` default prepends `// @ts-nocheck` to every copied file, so all 26 pinned files read one line longer than they are; and the sandbox has no `origin/main`, so the gate's history-backed half cannot resolve its merge-base. Fixing either leaves the other — turning off the type-check rewrite was tried first, and CI then failed on the missing ref. The gate's own `.tmp` skip entry cannot save it either: that is matched relative to `REPO_ROOT`, which inside the sandbox *is* the sandbox. Excluding it hides nothing, for the reason this module's header already gives: a mutant only an excluded test could kill shows up as a survivor. A repo-shape gate cannot kill a kernel mutant at all — it asserts about file lengths and history, never about behavior under mutation. Derived rather than listed, matching the neighbouring thread-hostile exclusion, so a future sibling gate is caught the same way. Nothing was wrong with the ratchet or with #1964; before #1969 rewired the module graph, nothing pulled this gate into a lane's scope and the trap stayed hidden. Verified with `pnpm mutation:run --modules kernel-errors`: scope 804 -> 803 test files (exactly this one), dry run clean, lane `pass` in 560s, score 75.2% (188 killed / 62 survived / 250). `pnpm mutation:test` 39/39. `stryker.config.json` is deliberately untouched, so the config content hash the report and lane envelope carry is unchanged and scores stay comparable. Unblocks #1964, whose two mutation checks fail on main's tip without its code.
thymikee
added a commit
that referenced
this pull request
Aug 22, 2026
… reseed tight Review findings on #1965: 1. Discovery scanned only `packages/*/src/facades/*.ts`, which omits every package that publishes its entry surface straight from the manifest — including all six `packages/platform-*/src/index.ts` façades, the exact subject of ADR-0019's implementation-laziness rule. Discovery now derives from `readWorkspacePackages(...).exportTargets` and then adds `/src/facades/` files, reusing the R11 helper rather than reimplementing it so the two gates cannot disagree about what an entry surface is. The table grows from 14 façades + 2 hubs to 95 entry surfaces + 8 hubs. 2. Budgets carried a few files of slack each. They are now exact ratchets with no headroom, matching how the repo pins R9/R10 and test-file size: growth is allowed, it just has to be a visible number change in the diff of the PR that causes it. Every budget is reseeded from post-#1969 measurement. 3. Violations printed a flat sorted set, which named the offender but not the route. `eagerClosureGraphOf` records each file's discoverer, so failures now print the transitive chain entry -> ... -> offender. `eagerClosureOf` keeps its contract and is expressed in terms of the new walk; per-file edges are memoized, which also cuts the existing pins' runtime (cli closure test 2336ms -> ~550ms). The platform-package façades evaluate exactly one module each — themselves — so their budget of 1 is the tightest statement of "metadata-eager, implementation-lazy" the walker can make.
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.
Closes #1959.
Problem
@agent-device/contracts/platformvalue-re-exports 32 vocabulary modules;/interactionanother 18. A file that value-imports either evaluates the whole union to reach one function — and permanent hubs sat behind them (command-descriptor/registry.ts,core/capabilities.ts,interactors/register-builtins.ts,command-descriptor/platform-execution-entry.ts), so that union rode into roughly half of the unit suite's test graphs.Change
Every vocabulary module gets its own entry subpath, and all value-importers move onto the module that owns the symbol. Type-only importers are left alone exactly as the issue scoped it —
import typeis erased, so it already evaluated nothing.The rewrite was mechanical: symbol → owning-module resolved from each façade's own re-export table, so no import was hand-picked. Every symbol resolved; nothing was skipped.
Measured
Eager-import-closure walker from #1950, over all 974 unit-core test files, against base
e5bfde3d1:facades/platform.tscarried byfacades/interaction.tscarried byPer hub:
command-descriptor/registry.tscore/capabilities.tsinteractors/register-builtins.tscommand-descriptor/platform-execution-entry.tscore/dispatch.tsThe issue projected ~10–14%. The gap is the part the leaf-simulation could not model: consumers still evaluate the modules they genuinely use, so the recovered work is only what they didn't.
Two hubs (
capabilities.ts,register-builtins.ts) did not move on the first pass —platforms/apple/plugin.tspulled the clump back in behind them. That is why the change covers every value-importer rather than the four files the issue named.Regression pin
src/__tests__/contracts-entry-closure.test.ts, built on the #1950 walker, from both directions:A non-vacuity assertion requires the scan still sees the narrow imports that replaced these, so a broken walker fails loudly instead of passing empty.
Planted red (reverted before commit): restoring
capabilities.tstocontracts/platformfailed both tests, each naming the offending edge —Gate adjustments the split forces
./clipboard, since./gesture-planis now a real entry.contracts/platformspecifier; it now accepts any contracts entry. The assertion's provenance is what the rule pins, not which subpath carried it.gesture-plan.tsbecame an entry target, and the rule rejects theexport * from './gesture-plan-types.ts'it carried. Its one internal consumer (client-gesture.ts) now imports the owning module directly.Known follow-up
Both façades keep their type re-exports for the ~490 type-only importers, so every symbol on them now reads as value-unused. One
.fallowrc.jsonentry records that with the reasoning. Netcheck:production-exportsfindings go 37 → 29 (below base, because the entry also absorbs pre-existing façade findings).Retiring the two façades outright — moving the type-only importers too and deleting the files — is the change that makes that entry unnecessary. It is ~490 further one-line edits with no additional perf gain (type imports are already free), so it is deliberately not bundled here.
Worth tightening after #1965 lands: its budget table seeds façade budgets from pre-#1959 closures, which this PR makes much smaller.
Verification
pnpm typecheck,pnpm lint,pnpm format,pnpm check:layering(181/181),pnpm check:fallow(no issues in 258 changed files),pnpm gate production-exports, andpnpm test:unit— 1002 files / 7473 tests, all green.One earlier run showed a single
runner-client.test.tstimeout; it passes in isolation and is the known under-contention signature (timeouts, never assertion failures) from four agents sharing this host. Clean on the full re-run.