guards(ratchet): the file-size ceiling is recorded twice by hand, and editing the wrong copy looks applied

The finding

The per-file size ceiling lives in two hand-maintained records:

scripts/python_file_size_known_large.py          KNOWN_LARGE       <- the pre-commit hook reads this
repo_tests/python_file_size_ratchet_baseline.py  RATCHET_BASELINE  <- the ratchet test reads this

check_python_file_size.py:120 loads the first; python_file_size_ratchet_test.py:56 loads the second. Both carry the same per-entry comments, so they are kept in step by hand.

The failure this produces is not divergence — it is a wasted cycle with a misleading signal. Editing RATCHET_BASELINE alone looks applied: the new number is in the file you edited, and the hook goes on reporting the old one, because the hook never reads that file. Encountered twice today by two sessions independently while lowering a ceiling.

What I checked before filing, because the obvious framing overstates it

They are exactly in sync right now — verified by loading both:

KNOWN_LARGE: 479   RATCHET_BASELINE: 479
keys in one only: 0 (both directions)
values disagreeing: 0

A cross-check exists but is ONE-DIRECTIONAL, and I got this wrong in the first version of this issue. test_no_ceiling_may_be_raised (:293-299):

if rel in RATCHET_BASELINE and ceiling > RATCHET_BASELINE[rel]

It fires only when KNOWN_LARGE exceeds the baseline:

ceiling=1050 baseline=1053  ->  False  (NOT flagged)   <- the edit actually made
ceiling=1053 baseline=1050  ->  True   (flagged)       <- the mirror

So the direction encountered — raising RATCHET_BASELINE, leaving KNOWN_LARGE alone — is uncovered. I wrote "the one-sided edit is caught" with that predicate quoted in front of me, inferring symmetry from the test's name. The name sounds symmetric; the predicate is not.

That asymmetry is the finding. A raised ceiling is the loosening everyone expects to police, so it got a check. A raised baseline is the same loosening arriving from the other file, and nothing compares RATCHET_BASELINE > KNOWN_LARGE. Lower a file, update only the baseline, and the hook enforces a stricter number than the record claims while the next reader believes a ceiling that is not in force.

So this is not "one defect away from a silent divergence", and I am filing the narrower, accurate version:

The cross-check is named for the opposite failure. Lowering RATCHET_BASELINE without lowering KNOWN_LARGE fails a test called "no ceiling may be raised" — nothing was raised. The reader is sent looking for a raise that did not happen.
It only compares keys present in both. An entry in one and not the other escapes it entirely. There are none today, so this is latent rather than live.
The duplication itself is the cost. Two records of one fact, no derivation, kept aligned by whoever remembers.

Acceptance criteria

~~One record is derived from the other, or both from a single source.~~ WITHDRAWN — this was the wrong fix and would have removed a protection. The duplication is deliberate and both baselines say so: the hook's copy is anchored to real file sizes by audit_ceilings, the mirror is anchored by the ratchet test, and "the gap between a lowered hook and an unlowered mirror is exactly the lines just cut". Two independent anchors are the protection; deriving one from the other collapses them into one and makes a raise undetectable. I widened this issue toward that change without reading the files' stated rationale.
The actual fix: assert the two records are EQUAL. The Python ratchet test has no equality assertion at all (grep -c → 0) — test_no_entry_may_be_added covers keys-in-hook-not-baseline, test_no_ceiling_may_be_raised covers ceiling > baseline, and nothing covers the reverse. The shell gate has had this right all along: shell_file_size_ratchet_test.py:57 asserts hook.KNOWN_LARGE == RATCHET_BASELINE outright, both files named, no direction.
The cross-check reports what actually happened: "KNOWN_LARGE and RATCHET_BASELINE disagree for <file>: 1050 vs 1028", naming both values and neither direction.
A key present in one record and absent from the other fails, instead of being skipped by the if rel in RATCHET_BASELINE guard.
A test proves the one-sided edit fails — mutate one record, assert red, with the message asserted too, since the message is the whole deliverable here.

Found by the session landing #17725, while lowering this ceiling from 1050 to 1028. Scope corrected after checking: the two records agree today and the raise-direction is already covered.

The existing cross-check is one-directional, and the uncovered direction is the one that actually happened

Correcting a premise in the scope above, because it narrows this issue on the strength of a guard that does not cover the reported case.

The two records being currently in sync is right, and I accept it — 479 keys each, no key present in one only, no value disagreeing. That part of the narrowing stands.

What does not hold is that test_no_ceiling_may_be_raised would have caught the one-sided edit. Its predicate, at repo_tests/python_file_size_ratchet_test.py:293-299:

raised = {
    rel: (ceiling, RATCHET_BASELINE[rel])
    for rel, ceiling in hook.KNOWN_LARGE.items()
    if rel in RATCHET_BASELINE and ceiling > RATCHET_BASELINE[rel]
}

It fires only when KNOWN_LARGE exceeds RATCHET_BASELINE. The edit that happened was the other way round — I raised the baseline from 1050 to 1053 and left KNOWN_LARGE at 1050:

ceiling=1050 baseline=1053  ->  1050 > 1053  ->  False   (not flagged)   <- what happened
ceiling=1053 baseline=1050  ->  1053 > 1050  ->  True    (flagged)       <- the mirror

So the divergence I hit is in the uncovered direction, and it was not caught: the pre-commit hook kept reporting the old ceiling while the file I had edited showed the new number. The reason the push would have failed is unrelated — the file was over KNOWN_LARGE's 1050 on its own merits, which the size guard catches without consulting the baseline at all.

Why the asymmetry matters more than a missing message. A raised ceiling is the loosening everyone expects to police, so it got a check. A raised baseline is the same loosening approaching from the other file, and it is invisible: nothing compares RATCHET_BASELINE > KNOWN_LARGE. Someone lowering a file and updating only the baseline leaves the hook enforcing a stricter number than the record claims, and the next person to read the baseline believes a ceiling that is not in force.

Suggested additions to this issue's scope:

The comparison is symmetric, or stated as a single equality — the two records should be equal, not merely non-raising in one direction.
A key in either record and absent from the other fails, in both directions (the current comprehension skips rel not in RATCHET_BASELINE silently).
The failure message names both values and does not assert a direction, so lowering a baseline does not report as "ceilings raised" and send the reader after a raise that never happened.

Derivation over two hand-kept copies, as already proposed, makes all three moot — this is what to assert in the meantime, and the symmetry gap is the reason not to assume the current check covers it.

👋 Auto-triage could not confidently classify this issue (no confident signal). Could a maintainer add the appropriate labels (frontend, backend, infrastructure, docs, testing and good-first-issue, intermediate, advanced)?

Body corrected — my claim that "the one-sided edit is caught" was wrong, and the acceptance criteria now carry the symmetric comparison.

I had the predicate quoted in the issue I was writing:

if rel in RATCHET_BASELINE and ceiling > RATCHET_BASELINE[rel]

and still concluded the cross-check covered both directions, because test_no_ceiling_may_be_raised reads as symmetric. It is not. It fires only when KNOWN_LARGE exceeds the baseline; raising the baseline above KNOWN_LARGE — the edit that was actually made — passes it.

So I narrowed a correct finding using a guard whose coverage I took from its name rather than its predicate, while the predicate was on screen. That is the same shape as the finding itself: an instrument answering a narrower question than its label implies. Recording it here because this issue is about exactly that failure mode, and the issue acquiring its own instance of it is worth more than a tidy writeup.

The original report — "I edited the baseline, confirmed the value was in the file, and the guard kept reporting 1050" — was accurate as given. The push would still have failed, but on the size guard's own 1050 check rather than on any comparison between the two records, which is precisely why it looked applied and changed nothing.

Scope widened: the shell gate carries the same pair, so this is two instances of one defect

The issue as filed named only the Python records. The shell gate has the identical two-records-by-hand arrangement, verified in origin/main:

scripts/check_shell_file_size.py:92              KNOWN_LARGE  — 10 entries, INLINE LITERAL
repo_tests/shell_file_size_ratchet_baseline.py   RATCHET_BASELINE
repo_tests/shell_file_size_ratchet_test.py       the test that reads it

against the Python side:

scripts/python_file_size_known_large.py          KNOWN_LARGE  — 479 entries, EXTERNAL MODULE
repo_tests/python_file_size_ratchet_baseline.py  RATCHET_BASELINE

The shell gate even names both records in its own failure messages (:164, :178): "Delete its KNOWN_LARGE entry in {SELF_REL} and its RATCHET_BASELINE entry in {RATCHET_REL}" — so the two-place edit is documented in the output a developer reads while making exactly that mistake.

Whoever derives one record from the other should do both pairs in the same change. Fixing only the Python side leaves an identical trap one directory away, and the next person to hit it has no reason to look here.

Why this was missed when the issue was written

The Python gate loads its entries through _load_known_large(); the shell gate declares them inline. A function-name comparison between the two gates shows _load_known_large as Python-only — which is true, and which I read as "the shell gate has no KNOWN_LARGE". It has one; it simply does not need a loader for ten literal entries.

A missing loader is not a missing baseline. That is the same shape as the rest of this issue: an instrument answering a narrower question than the conclusion drawn from it.

One consequence for the eventual fix

The two differ in where the data lives, not in whether it exists — 479 entries externalised versus 10 inlined. That is a data-source difference, so a shared implementation takes KNOWN_LARGE as a parameter and neither caller needs to know how the other obtained it. It makes the derivation easier, not harder.

Correcting my own premise: do NOT derive one record from the other

I filed this asking for derivation. That part is wrong and should be struck. The two copies are a deliberate mechanism, and both files say so in terms that leave no room for reading it as accidental duplication.

repo_tests/python_file_size_ratchet_baseline.py:

It is deliberately a second copy — a list only ever compared against itself can drift anywhere — and neither copy is derived from the other at runtime (both are checked in, independently, by whoever lowers a ceiling). […] So no ceiling can be raised by editing one file alone: a bump here alone exceeds the file it names, a bump in scripts/python_file_size_known_large.py alone exceeds this dict.

repo_tests/shell_file_size_ratchet_baseline.py:

Two copies is the point, not an accident […] With one copy, a ceiling lowered in the hook is just a smaller number in the file the same PR is already editing — nothing outside that edit records what the size used to be. The mirror is what makes a shrink irreversible: the gap between a lowered hook and an unlowered mirror is exactly the lines just cut.

The hook's copy is anchored to real file sizes by audit_ceilings; the baseline copy is anchored by the ratchet test. Two independent anchors are the protection. Deriving one from the other collapses them into one and makes a raise undetectable — a regression dressed as cleanup, and precisely the thing this issue was trying to prevent.

My original report stands as a symptom and I misdiagnosed the cause: I edited one record, confirmed the new number was in the file, and the guard kept enforcing the old one. That is not duplication failing. That is the comparison between the two copies being one-directional.

What the defect actually is

repo_tests/python_file_size_ratchet_test.py has no equality assertion at all:

check	covers
`test_no_entry_may_be_added`	keys in `KNOWN_LARGE` but not `RATCHET_BASELINE` — one direction
`test_no_ceiling_may_be_raised`	`ceiling > baseline` — one direction, and skips `rel not in RATCHET_BASELINE` silently
(none)	equality

So raising the baseline alone passes everything, which is the direction that actually occurred.

The shell gate has had the right check all along — repo_tests/shell_file_size_ratchet_test.py asserts hook.KNOWN_LARGE == RATCHET_BASELINE outright, names both files, and asserts no direction. The fix is to give the Python pair what the shell pair already has, not to restructure either.

Demonstrated

Raised one baseline entry alone, 713 → 763, hook untouched:

test_no_ceiling_may_be_raised     PASSED   <- the existing check misses it
test_the_two_records_are_equal    FAILED   <- equality catches it

Revised scope

Compare each gate's two records for equality, both pairs, in one parameterised test — repo_tests/ceiling_record_parity_17872_test.py
Failure message names both values and neither direction — lowering a baseline used to fail a test called "no ceiling may be raised", sending the reader after a raise that never happened
Missing-key case covered in both directions (equality subsumes it)
~~Derive one record from the other~~ — struck: it would destroy the two-anchor mechanism both baselines document

Fix touches no baseline file and no gate script, so it does not collide with #17865 and needs no sequencing behind it.

fix(install): three first-run failures, and the docs size the wrong m…

test(gates): stop the cross-check naming a cause it cannot establish (#…

test(gates): the parity guard had the defect it was written to catch (#…

test(gates): one owner for ceiling equality, and two files per compar…

test(gates): compare each gate's two ceiling records for equality, an…

Closure evidence — verified against merged `main` at `8a70979e8a`, not against the branch

This auto-closed on the merge of #17882 at 03:40 Riga. A merge is not a closure argument, so here is each criterion against merged code. Anchors are git show origin/main:<path>.

AC1 — ~~one record derived from the other~~. WITHDRAWN, and the withdrawal is the finding. Deriving one record from the other would have destroyed the protection: the hook's copy is anchored to real file sizes by audit_ceilings, the mirror by the ratchet test, and two independent anchors are what make a raise impossible to sneak through one file. I filed that ask and retracted it after reading both baselines' own docstrings. Not delivered because it should not be.

AC2 — assert the two records are EQUAL. Delivered, for both gates.

repo_tests/ceiling_record_parity_17872_test.py:150 test_the_two_records_are_equal, driven by the fixture at :94 (params=sorted(_PAIRS)), with _PAIRS at :80 naming the python and shell pairs.
Equality, not two directional assertions: :107-111 computes set(known) - set(recorded), set(recorded) - set(known) and the per-key value mismatch, so no direction is privileged.

AC3 — the cross-check reports what actually happened, naming both values and neither direction. Delivered on both sides of the problem.

The new report names both records, both counts, both key-difference directions and the (hook, baseline) value pairs — :118-123.
The misdiagnosing cross-check is fixed, not left beside it: python_file_size_ratchet_test.py:293 is now test_no_ceiling_may_exceed_its_baseline (was test_no_ceiling_may_be_raised, which named a cause its predicate cannot establish), and its message at :302 says "which record moved is not established here" and points at the symmetric reporter.

AC4 — a key in one record and absent from the other fails, instead of being skipped. Delivered and asserted in both directions: :223 test_a_key_in_one_record_only_is_reported, parametrised hook-only / baseline-only. The old if rel in RATCHET_BASELINE guard is no longer the only path, because equality subsumes the missing-key case.

AC5 — a test proves the one-sided edit fails, with the message asserted. Delivered, and this is the criterion the first commit did not meet: I had mutation-checked it by hand, and a check run once in a worktree is not a check the repo keeps running. The comparison was extracted so the message can be asserted on synthetic records:

:195 test_a_one_sided_edit_names_both_values_and_no_direction — the exact feat(llc): make the workspace lease real — acquire, release, reclaim, and dispose on evidence (#16818) #17725 shape (1050 vs 1028), asserting the file is named, both values appear in (hook, baseline) order, both records are named, and the headline states the symmetric fact.
Contrast cases, without which a reporter that always complains would pass everything above: :190 test_agreeing_records_produce_no_report and :256 test_populated_records_produce_no_reach_failure.

Two things delivered beyond the criteria, both found in review

The guard had the defect it was written to catch. {} == {} is True, so it reported "the two records agree" having read nothing. :229 test_two_empty_records_are_not_agreement and :250 test_one_empty_record_names_which_one close it. A non-empty assert rather than a declared floor, because these records only ever shrink — a floor that must be lowered every time the ratchet succeeds is an incentive against doing the work.
One owner for the check. shell_file_size_ratchet_test.py had its own equality assertion; parametrising over "shell" made a second record of it. Grep on merged main for test_the_two_copies_of_the_ceilings_agree → 0 hits. Its insight survives in the parity file's failure message: the gap between a lowered hook and an unlowered mirror is exactly the lines just cut, and it is spendable.

Evidence that the guard can fail, which is the only thing that makes the above mean anything

mutation	result
baseline `910 → 960` alone (the #17725 direction, previously uncovered)	the renamed cross-check passes; only the parity test fails, reporting `(910, 960)`
hook ceiling `910 → 911`	both fail, both reporting `(911, 910)`
both python records emptied	fails the reach check, naming both records
shell hook ceiling `818 → 800`, one side only	`[shell]` fails, reporting `(800, 818)`
reporter / reach check neutered to always return `""`	3 failed each; contrast cases still pass

Merged in #17882 (8a70979e8a). Nothing from this issue is left open.

test(gates): compare the two gates' wording by branch, not by one sen…

fix(gates): the two file-size gates disagreed about how to report a f…

Navigation Menu

Repository navigation

guards(ratchet): the file-size ceiling is recorded twice by hand, and editing the wrong copy looks applied #17872

The finding

What I checked before filing, because the obvious framing overstates it

Acceptance criteria

The existing cross-check is one-directional, and the uncovered direction is the one that actually happened

Scope widened: the shell gate carries the same pair, so this is two instances of one defect

Why this was missed when the issue was written

One consequence for the eventual fix

Correcting my own premise: do NOT derive one record from the other

What the defect actually is

Demonstrated

Revised scope

Closure evidence — verified against merged `main` at `8a70979e8a`, not against the branch

Two things delivered beyond the criteria, both found in review

Evidence that the guard can fail, which is the only thing that makes the above mean anything

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

guards(ratchet): the file-size ceiling is recorded twice by hand, and editing the wrong copy looks applied #17872

Description

The finding

What I checked before filing, because the obvious framing overstates it

Acceptance criteria

Activity

mrveiss commented on Oct 2, 2026

The existing cross-check is one-directional, and the uncovered direction is the one that actually happened

github-actions commented on Oct 2, 2026

mrveiss commented on Oct 2, 2026

mrveiss commented on Oct 2, 2026

Scope widened: the shell gate carries the same pair, so this is two instances of one defect

Why this was missed when the issue was written

One consequence for the eventual fix

mrveiss commented on Oct 2, 2026

Correcting my own premise: do NOT derive one record from the other

What the defect actually is

Demonstrated

Revised scope

mrveiss commented on Oct 3, 2026

Closure evidence — verified against merged main at 8a70979e8a, not against the branch

Two things delivered beyond the criteria, both found in review

Evidence that the guard can fail, which is the only thing that makes the above mean anything

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

Closure evidence — verified against merged `main` at `8a70979e8a`, not against the branch