Skip to content

gates: let the eager-closure budget tolerate a module split without admitting new eager work #2469

Description

@thymikee

Purpose

Decide how the eager-closure gate (scripts/__tests__/eager-closure-budgets.ts) should treat a module split, so that extracting a large module inside a gated closure stops being impossible without weakening the property the gate protects.

Current behavior

Every entry that exists at the merge-base is held to NO GROWTH of its eager import closure, counted in modules. There is no approval path for growth of an existing entry (APPROVED_OVER_CEILING covers first-introduced entries only). A module that is in a gated closure therefore cannot be split: one module becoming N modules is +N−1 for every entry that evaluates it.

Measured on PR #2423 at b21efb0: extracting packages/platform-apple/src/runner/runner-cache-metadata.ts (621 lines) into runner-phase-budget.ts + runner-toolchain-probe.ts grows seven entries by two each: app-lifecycle-facade 120→122, app-resolution-facade 61→63, doctor-facade 101→103, perf-facade 60→62, physical-device-facade 47→49, runner-operations-facade 100→102, runner/index.ts 13→15. The review on #2423 asked for exactly that extraction for ownership clarity, and it had to be deferred.

What the gate protects

The ADR-0019 loading shape: a façade must not evaluate implementation it does not need at import time. The module count is a proxy for eager work. A split does not add eager work when the new modules together evaluate what the old one did, but it does add modules, so the proxy reads it as growth.

Design question

Choose a rule that admits healthy extraction without admitting new eager work. Candidates, none proven:

  1. Count something closer to eager work than module count: total source bytes, or statements at module top level, of the closure. A pure split keeps it flat; a new static edge to real implementation raises it. Costs: a heavier walker; renames and formatting churn move the number.
  2. Recognize a split structurally: the new modules are all new at the head, and each is imported only by modules that were in the old module's importer set at the merge-base. As the reviewer of fix(ios): budget cold toolchain probes for the first-exec signature stall #2423 noted, this does not by itself prove no new eager work (a "new" module can carry a new heavy edge).
  3. An explicit, reviewed approval row for growth of an existing entry (APPROVED_GROWTH), naming the issue, the reason, and the expected delta, retired automatically once the merge-base carries the new shape. Same mechanism as the existing ceiling rows; costs a hand-edited row per split.
  4. Combine 2 and 1: accept a structural split only when the closure's byte or statement total does not grow.

Deliverable

A short design note choosing one, with the walker change and the test that plants (a) a pure split and (b) a split that smuggles a new heavy edge, showing the first passes and the second fails. Then re-run the #2423 extraction as the first consumer.

Non-goals

Loosening no-growth for anything other than splits; changing category ceilings.

Activity

  1. thymikee commented on Oct 7, 2026

    @thymikee
    MemberAuthor

    This deserves priority in the architecture-quality umbrella #3276. Two current developments strengthen the case:

    • ADR 0027 records a byte-faithful command-registry split blocked by the same no-growth rule, while repository policy requires oversized files to split.
    • Merged perf(daemon): lazy-load Maestro in request binding; budget the daemon entry #3284 adds src/daemon.ts to the hub budget, extending the scope where a healthy extraction can fail. Its Maestro optimization is useful; the blanket module-count constraint is the separate design question.

    Please retain the existing split-versus-new-heavy-edge acceptance pair and use it to choose the smallest solution. In particular, distinguish the hard ADR-0019 loading constraints from module count as a proxy. Existing checks that prevent premature platform implementation loading should remain hard checks. Avoid solving this with a general source-equivalence/split-inference framework or forcing artificial import() seams merely to keep counts flat.

    For the design comparison, include the maintenance cost of ordinary extraction, not just gate runtime: how many source, configuration, and approval edits does the next split require? Bytes/statements are also proxies and need adversarial evidence; equal total size alone does not prove equal eager behavior.

    Resolve ADR 0027 as part of the chosen policy so the file-size and eager-loading instructions stop giving contradictory directions. This is a request to prioritize the existing issue, not to bypass the current guard or add an unreviewed allowance.

  2. thymikee commented on Oct 7, 2026

    @thymikee
    MemberAuthor

    Resolved in #3298 (implementation + planted tests + first consumer in one stack).

    Rule chosen: candidate 4 (structure ∧ weight), minimal slice. NO-GROWTH tolerates module-count growth for exactly one shape, computed from the two closure graphs — nothing hand-entered:

    1. Containment: the head closure still evaluates every merge-base module (renames canonicalized), so growth is purely additive.
    2. Flat weight: the closure's total top-level-statement count — wiring (import, export … from) excluded — does not grow.

    Each half alone is refutable and both directions are planted in tests: containment without weight accepts a split smuggling a module-scope call into a new heavy edge; weight without containment accepts a flat-size swap that drops a merge-base module. Together they admit the #2423 extraction and refuse both. Statements over bytes: formatting/comment churn moves bytes; statements track module-scope evaluation work, the thing being preserved. Candidate 3 (approval rows) stays refuted — an allowlist on the one invariant deliberately given no approval path, plus a hand-edited row per split. Under the chosen rule an ordinary extraction costs zero configuration edits.

    Per the umbrella constraint: no inference framework — one comparison of two numbers plus a subset fact in a 98-line module (scripts/__tests__/closure-growth-rule.ts). The hard ADR-0019 checks (façade exactness, no platform implementation before binding) are untouched; the tolerance relaxes the count proxy only. A tolerated split must keep containment, so it cannot smuggle an eager edge that drops anything.

    First consumer: the #2423 extraction itself — runner-cache-metadata.ts splits into runner-phase-budget.ts + runner-toolchain-probe.ts. All seven entries this issue names pass with containment at flat weight (e.g. app-lifecycle-facade 117→119 modules, weight 1952→1952; runner/index.ts 13→15, 399→399), with zero approval rows. docs/adr/0027 is resolved to Accepted in the same PR, which also unblocks the byte-faithful descriptor-root split recorded there (+11, statement-neutral).

    Gate evidence (pnpm check:affected --run, head 5e96a8785): all runnable checks passed; 470 files / 4359 tests, layering, fallow clean, xctest-selection 0 unreachable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions