Skip to content

gate: the file-size baseline is a shared cell — two correct PRs compose into a red main (3rd occurrence) #2004

Description

@macanderson

Problem

scripts/file-size-baseline.txt is a single shared cell that every growing PR
must write
, and two PRs that each write it correctly can still compose into a
red main. The guard asks a whole-tree question — "is this tree consistent with
this baseline snapshot?" — when the thing it wants to prevent is a per-change
question: "did this change grow a god file past its ceiling?"

Concretely, on main at 6c345532:

$ bash scripts/check-file-size.sh
check-file-size: FAILED
  crates/stella-core/src/driver.rs grew to 2572 lines, over its baseline ceiling of 2571 (+1)
  crates/stella-pipeline/src/pipeline/tests.rs grew to 2537 lines, over its baseline ceiling of 2536 (+1)

Neither contributing PR is at fault. #1979 added three lines to driver.rs;
#1962 added a line to pipeline/tests.rs. Each regenerated the baseline against
a snapshot of main that did not yet carry the other's growth, each was green on
its own merge commit, and the composition is red. main then stays red — see
#1986 — so every subsequent PR inherits a failure it did not cause. PR #1992 sat
MERGEABLE/BLOCKED on exactly this with a tree byte-identical to main for both
the baseline and both failing files.

This is the third occurrence of the same shape: #1761 (deck.rs 1517 vs a
1510 ceiling), #1782 (#1776 grew driver.rs +16 and registry.rs +35), and now
#2003. All three were fixed by regenerating and closed, leaving the mechanism
intact.

AGENTS.md already names this exact failure mode for a different file, and rejects
the shared-cell design there:

Deliberately no total: the guard checks each step by name, so two PRs adding
different guards merge cleanly, while a spelled-out count is one shared cell
both must write — and two of them collided on it twice in a day, each time
leaving main red for everyone (#1883).

The baseline has the identical shape and has not had the identical treatment.

Files involved

  • scripts/check-file-size.sh — the guard; the awk pass at ~L125–165 judges the
    working tree against the baseline map
  • scripts/file-size-baseline.txt — the shared cell (generated; 30 entries)
  • scripts/test-file-size.sh — the guard's own hermetic tests, where a new case
    belongs
  • Makefilefile-size and file-size-update targets
  • .github/workflows/ci.yml — runs the guard in the required job
  • AGENTS.md § "God files — plan around them, never into them" — documents the
    ratchet's contract; any semantic change must be reflected here

Reproduce

The skew is reconstructible from history without waiting for a new collision:

git checkout -b skew-repro e0fbbe02          # main, one commit before #1979's growth landed
bash scripts/check-file-size.sh              # OK
git cherry-pick ac4a13fd                     # #1979: driver.rs +3, baseline regenerated
bash scripts/check-file-size.sh              # OK
git checkout -b skew-other e0fbbe02          # a sibling PR branched from the same point
# ...regenerate the baseline here for an unrelated god file, merge both...

The already-landed instance is simpler to observe:

git checkout -b probe origin/main
bash scripts/check-file-size.sh              # FAILED, +1 on two files, with no local change

Proposed fix

Make the ratchet judge the change, not the tree. When a file exceeds its
recorded ceiling, the guard should distinguish two cases:

  1. The diff under test grew it → fail, as today. This is the bloat the
    ratchet exists to block, and the message should stay exactly as it is.
  2. The file already exceeded its ceiling at the merge base → the growth
    arrived from another PR that was itself reviewed. Do not fail the innocent PR.
    Report it as drift the baseline owes an update, and let the branch-level
    check (or a scheduled job on main) be what demands the regeneration.

A merge-base comparison is enough to separate them; scripts/impacted-crates.sh
already establishes the precedent for a guard reading the pushed diff, and the
pre-push hook already computes a range.

Alternatives worth weighing in the fix, not pre-decided here:

  • Require branches up to date before merging (branch protection). Solves it
    without touching the guard, but slows every merge and does nothing for the
    hook's local run. Interacts with CI: ci.yml does not run on a push to main, so main goes red and stays red (twice by the same boot.rs doc link) #1986.
  • Regenerate the baseline in CI and commit it — removes the shared cell by
    removing the human write, but a generated file that mutates on main outside a
    PR is its own hazard, and it would defeat the "visible in review" property
    AGENTS.md deliberately relies on.
  • Per-file baseline fragments (one file per entry, à la the GATE_STEPS
    by-name fix) — cheapest conceptually, and two PRs touching different god files
    then never collide. Does not help two PRs touching the same god file.

Constraints

  • The baseline is generated; make file-size-update must remain the only
    supported way to write it, and hand-editing must stay unsupported.
  • The baseline may only ever shrink in entry count — check-file-size.sh
    L27–31 enforces that an entry dropping to ≤ LIMIT becomes obsolete and must
    be retired. Any redesign must preserve that.
  • Only the ceiling direction is enforced, on purpose (L34–35): a baseline file
    shrinking must not fail. Preserve that.
  • check-god-files.sh cross-checks the baseline against AGENTS.md and every
    crate README; the baseline is the tiebreaker (gate: AGENTS.md's god-file table and crate README god-file lists hand-mirror file-size-baseline.txt with no drift guard #1435). A format change touches
    that guard too.
  • Global guards are never narrowed by CARGO_SCOPE — "a 1500-line file is a fact
    about the repository, not about a crate" — so a merge-base-aware guard must
    still behave correctly when the range is the whole workspace or absent.

Definition of done

  • Two PRs that each grow a different god file, each regenerating the baseline
    against a main lacking the other's growth, compose into a green main.
  • A PR that genuinely grows a god file past its ceiling still fails, with the
    current message unchanged.
  • A case in scripts/test-file-size.sh covering the skew, constructed the way
    the repro above does — hermetic, no network, no reliance on real repo history.
  • AGENTS.md § "God files" updated to state what the ratchet now judges.
  • make gate green; check-god-files.sh still green.

Related: #1986 (main goes red and stays red), #1883 (the shared-cell lesson,
applied to GATE_STEPS), #1435 (the three-copy cross-check), #1761 and #1782
(prior occurrences), #2003 (this occurrence's unbreak).

Metadata

Metadata

Assignees

No one assigned

    Labels

    triageUntyped request — convert by adding bug / feature / epic

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions