Repository navigation
fix(tooling): file-size audit discovers oversized files instead of only re-checking known ones (#14547) - #14629
Conversation
✅ SSOT Configuration Compliance: Passing🎉 No new hardcoded values of either class — Known backlog in |
Review — BLOCK, on the decision rather than the codeThe mechanism is right and I could not fault it. The measurement is exact — I re-derived it independently and got 505 / 3 / 502, with both checked-in dicts matching my own tree walk on all 505 paths and all 505 values, zero diff. The algebra survives extraction into sibling modules: I traced all six one-sided edits and every one fails loudly. The empty-walk case genuinely errors for two independent reasons. The boundary fix is exact on both sides, and both branches are live (2 of the 505 sit at ceiling 601). The extraction into data modules rather than self-exemption was the right instinct. What blocks is that this converts a 3-file freeze into a 505-file freeze over the repo's most actively-edited modules, and the PR never quantifies that. The number the PR is missing
So "a no-op for existing files" is not what this does. It is a growth freeze on 383 production modules and 122 test files that nobody is currently decomposing, arriving as a side effect of a discovery fix. It may well be the right policy — but it is a policy, and 505 violations against a 600-line limit is itself evidence the limit is aspirational. That is the owner's call, and I have posted it on #14547 with options. Second blocker: it grandfathers a file born oversized
The hook's own comment, in this very PR, says:
Grandfathering files that postdate the rule is the one thing the mechanism explicitly forbids. Split them or file them; do not exempt them. Related and worth stating: the three files that motivated #14547 grew between filing and this PR — High — the exclusions do not match the hook, and three docstrings say they do
Medium, cheap, same pass
One correction to the evidenceThe mutation claim of "3 red" holds for a whole-body revert, but a narrower mutation — swapping only the loop source while keeping Also: the Verification block quotes To unblock
The tree walk itself — the actual #14547 fix — is well built and I want it to land. |
…#14547) audit_ceilings() could only re-check files already in KNOWN_LARGE, so a file that grew past 600 lines without ever being added stayed invisible to every audit run — autobot-slm-backend/services/reconciler.py reached 2222 lines this way. A fresh tree walk found 502 more files already over the limit with no entry at all. audit_ceilings() now walks every in-scope git-tracked *.py file instead of the dict's own keys, and all 505 currently-oversized files are grandfathered at their measured size so the walk can run clean today; the shrink-only ratchet (grow fails, shrink forces the ceiling down, a vanished file fails) now applies to the whole set. KNOWN_LARGE/RATCHET_BASELINE moved to sibling data modules so the guard's own files stay under its own limit.
…14547) The --audit-ceilings step's comment described the old KNOWN_LARGE-only re-check; update it to describe the tree walk that replaced it.
- Flag testing_pattern_analyzer.py and reconciler_remediation_tracker_expiry_ 14465_test.py distinctly: both were created after the guard's own "never add an entry to make a new file pass" rule already existed. Filed #14630 and annotated their KNOWN_LARGE/RATCHET_BASELINE entries so they read as a tracked violation, not anonymous #5060 legacy debt. - EXCLUDED_PREFIXES was missing .worktrees/, present in .pre-commit-config. yaml's own exclude for this hook; added it, and replaced the self- referential exclusion test (built from the same constant it checked) with one that parses the YAML independently and asserts set equality. - audit_ceilings()'s reached count was len(tracked) rather than files actually read, silently including files that failed count_lines; a helper now counts only successful reads, restoring the original invariant. - count_lines() didn't catch UnicodeDecodeError (not an OSError, raised lazily during iteration) -- one non-UTF-8 .py file would have crashed the whole audit. - Added a cardinality ratchet on KNOWN_LARGE's own entry count: the existing set-difference test only catches an entry added to one dict and not the other, not a new entry added to both together.
|
Neither file is in this PR's diff. The ceilings live in
The branch is 27 commits behind base. Both files grew after the snapshot; So the guard is doing exactly its job — the new Options
Recommendation: 1. The part worth fixing rather than repeatingA committed baseline snapshot races every other merge. This PR will go red The durable form is to derive the baseline in CI from the merge-base rather This is separate from, and does not block, the outstanding owner decision on |
mrveiss
added
the
needs-decision
Evidence for the pending decision: the discovered files are real, and three are mine
Five crossings are attributable to a single commit each:
Every one starts at exactly its ceiling. That is the finding. A ceiling recorded All five landed with green required checks, including What this says about the decisionThe question on this PR was whether discovery mode is worth the noise. The answer I would not re-derive the ceilings again to clear the build. That is what Recommend instead: keep discovery, and grant explicit margin rather than Filed separately as #14762 so the live overruns are tracked independently of this PR. Not re-opening the decision — this is input to it, not a substitute. Leaving |
…rged tree (#14547) The snapshot this branch carries was measured before 21 commits of base landed under it. Fifty-six of the files it grandfathers grew since, two shrank, and seven more crossed 600 lines with no entry at all, so the discovery walk this change exists to switch on reported all sixty-five as violations and took `code-quality` and `python-suite` red with it. Re-measured against the merged tree instead of hand-patched: 512 entries, each equal to the file's line count as the walk sees it now. The three ceilings that exist on base -- orchestrator.py, chat_workflow/manager.py, chat_workflow/tool_handler.py -- are unchanged and still equal their files, so no recorded ceiling moves up; every other entry is this change's initial grant, and an initial grant is measured, not chosen. The two shrinks move down, which is the direction the ratchet turns. MAX_KNOWN_LARGE_ENTRIES follows the population it pins, 505 -> 512, with the reason inline: it is the size of the set the walk finds the day discovery is switched on. From here it only falls -- once this lands an unlisted oversized file fails the audit rather than joining the list.
…last measure (#14547) CI checks out `Merge <head> into <base tip>`, not the branch, so the snapshot has to agree with base as it stands when the check runs, not as it stood when the branch last merged. Between the two, base grew `autobot-backend/services/agent_terminal/service.py` from 994 to 1015 and took the ceilings audit red on that one file alone. Re-measured on the merged tree again: that entry follows its file, nothing else moved, and the population is still 512, so MAX_KNOWN_LARGE_ENTRIES is untouched. The three ceilings recorded on base remain equal to their files.
The parked decision, resolved: every check is greenAll 46 checks pass on What was actually wrongBase merged in (21 commits). That alone fixed nothing, and confirmed the earlier The decision taken, and why it is not a ratchet raiseThe snapshot is re-measured against the merged tree, not hand-patched. The
The timing constraint this exposes, worth knowing before mergingCI checks out Verification
Local counts are from CPython 3.10.12 and agree file-for-file with what CI measured on Overlap to note#14976 touches |
|
Decision taken: accept the snapshot. Confirmed with the owner. Recording all 509 entries at measured size and raising The reasoning that settled it: this strictly strengthens the guard. Today the hook is Removing One thing changed under you: #14976 merged as The two are complementary, not competing: #14976 closed the fail-open on the commit path for a file that cannot be read; this closes the blind spot on the audit path for a file that was never listed. Both must survive the merge — do not drop #14976's Also worth folding in while you are there: now that |
mrveiss
removed
the
needs-decision
Reconciles #14976 with this branch's rewrite of the same hook. The two changes are complementary and both survive: #14976 closes the fail-open on the COMMIT path (a file that cannot be read is reported, not skipped), this branch closes the blind spot on the AUDIT path (an oversized file nobody listed is discovered, not invisible). Resolution, both hunks kept-both: * the module docstring keeps #14975's "unmeasured is not within-the-limit" paragraph and this branch's tree-walk paragraph that supersedes the old KNOWN_LARGE-iteration description; * `unmeasured()` and this branch's `_vanished_entry_problem()` both stay; git's hunk had dragged the old `audit_ceilings` header in behind `unmeasured`, and that header is the one thing dropped -- the walk-based `audit_ceilings` this branch replaced it with is what remains. `main()` merged cleanly and still emits `unmeasured(rel)` before returning 1. This branch widened `count_lines` to catch `UnicodeDecodeError` as well as `OSError`, so a file that does not decode is now a fourth way to reach `unmeasured`. Its message named three causes; it names encoding as well now, and `test_a_non_utf8_file_is_unmeasured_rather_than_measured` pins that path -- the probe is deliberately over MAX_LINES, so anything that measured it instead would trip the size verdict rather than agree with the test.
Reconciled with #14976 — green again on
|
| Hunk | Kept |
|---|---|
| module docstring | #14975's "unmeasured is not within-the-limit" paragraph and this branch's tree-walk paragraph, which supersedes the old KNOWN_LARGE-iteration description it replaced |
| helper block | unmeasured() and _vanished_entry_problem(), side by side |
The only thing dropped is the old audit_ceilings def header that git's hunk dragged in
behind unmeasured — the walk-based audit_ceilings this branch replaced it with is what
remains. main() merged cleanly and still does violations.append(unmeasured(rel)) before
returning 1. repo_tests/python_file_size_ratchet_test.py auto-merged with all three of
#14976's tests intact.
#14976's path, proved still live rather than merely present
Mutations, each restored, on the reconciled tree:
| Mutation | Went red |
|---|---|
unmeasured(rel) call reverted to a bare continue |
test_an_unreadable_argument_is_reported_not_skipped, test_the_unmeasured_finding_is_not_dressed_as_a_size_violation, test_a_non_utf8_file_is_unmeasured_rather_than_measured |
unmeasured message made to quote (max 600) |
test_the_unmeasured_finding_is_not_dressed_as_a_size_violation, test_a_non_utf8_file_is_unmeasured_rather_than_measured |
UnicodeDecodeError dropped from count_lines |
test_a_non_utf8_file_is_unmeasured_rather_than_measured |
| ceiling above the file (both dicts) | test_recorded_ceilings_match_the_files_today, test_the_baseline_is_relowered_by_every_shrink |
| ceiling below the file (both dicts) | test_recorded_ceilings_match_the_files_today |
entry in KNOWN_LARGE alone |
test_no_entry_may_be_added |
| entry in both dicts | test_known_large_entry_count_may_not_grow |
| baseline raised alone | test_the_baseline_is_relowered_by_every_shrink |
Restored: 30/30 green. Population starved to 0 files, then 1: run_audit exits 1 both
times — a sweep that reaches nothing still cannot report a clean tree.
The fourth cause
This branch widened count_lines to catch UnicodeDecodeError as well as OSError, so a
file that does not decode as UTF-8 is a fourth way to reach unmeasured. The message named
three causes and would have sent someone looking in the wrong three places. It names
encoding now, and the new test pins the path, not the wording: the probe is deliberately
longer than MAX_LINES, so a change that measured it instead would trip the ordinary size
verdict and fail rather than agree.
Snapshot
4819 files scanned, 512 grandfathered, all live and at size., exit 0. No entry moved on
this merge, so MAX_KNOWN_LARGE_ENTRIES stays at 512.
Base has since advanced to seven dependency bumps (#14946-#14954). git diff --name-only HEAD...origin/Dev_new_gui -- '*.py' is empty — no Python file changed size, so the
merge-ref measurement is unaffected and the branch was deliberately not re-merged to avoid
re-queueing the whole suite for no change. That will not hold indefinitely: the snapshot has
to agree with base at check time, so this still wants merging while the queue is quiet.
Local counts are CPython 3.10.12; CI is authoritative and agrees.
…asure (#14630) (#14547) Re-measuring the snapshot rewrites the dict literal from `git ls-files` and emits `"path": count` and nothing else, so it destroyed both hand-written annotations that separated two entries from #5060's legacy backlog. A generator cannot preserve what it does not know about: anything hand-added inside generated output is destroyed on the next run, not kept by default. Both blocks are restored in both copies, and the loss is now guarded two ways. `ANNOTATED_AS_LIVE` in the baseline data module holds the annotated paths OUTSIDE the dict literal, where a literal rewrite cannot reach them, and `test_an_entry_singled_out_as_live_keeps_its_note` fails if either copy stops carrying the inline comment or drops its #14630 anchor. The regeneration is no longer the only thing standing between the annotations and silence. The guard's own test file lands at 599 of its 600 lines, so the trimmed `MAX_KNOWN_LARGE_ENTRIES` note pays for the new test. Anything further added here fails `test_recorded_ceilings_match_the_files_today` rather than sliding past, which is the intended way to find out.
Review finding fixed: the two #14630 annotations are back, and can no longer be lost silentlyHead The finding was right, and the claim in the body was false
Why restoring them was not enoughA generator cannot preserve what it does not know about. Anything hand-added inside
Run twice in a row, the re-measure reproduces the annotations byte-for-byte. Mutation evidence for the new guard
The first attempt at that last mutation passed, and it was the mutation that was wrong, The eight earlier mutations — five ratchet, three on #14976's unmeasured path — were re-run Two things a reviewer should know
Local counts are CPython 3.10.12; CI is authoritative and agrees. |
) Review of #15013 found four red checks and one dishonest deferral. File-size ratchet. takeover_manager_e2e_test.py shrank 663 -> 411, which is under MAX_LINES (600), so its grandfather entries were DELETED from both scripts/python_file_size_known_large.py and repo_tests/python_file_size_ratchet_baseline.py rather than lowered -- the audit's own message says an entry naming a compliant file exempts nothing while looking authoritative. #14629 turned discovery on for this audit, which is why it surfaced now. The explanatory comment on the 77 -> 59 change grew the uncollected-class guard 921 -> 926, past its recorded ceiling. A grandfathered exemption freezes the size it was granted for, so the comment was condensed back to 921 instead; the same two facts are still there, folded into the paragraph above the dict -- including the correction that 'every one of the remaining offenders is a live-service driver' is no longer true. Stale citation. atomic_facts_extraction_test.py was deferred citing #6994. #6994 closed on 2026-05-06 via #7076, and the TODO(#6994) in that file was planted by that same closing commit, which excluded this work from #6994's scope on its face. Nothing has tracked it since. Filed as #15043 and the comment now points there, saying plainly that self.extraction_agent is never assigned so all 8 methods would raise AttributeError if collected. The dead no-op statement goes with it. No behaviour change; the file's 8 stay inside the ceiling. Decomposition (#560). temporal_invalidation_test.py was rewritten in this PR, so the two functions over the 70-line threshold were split now rather than carried: _setup_test_facts (79) is a table of seven rows plus a loop, and test_invalidation_rule_matching (74) delegates to _assert_rule_matches and _fresh_fact. Longest function in the file is now 31 lines. The three mutations that cover it -- age, confidence and source-pattern rules -- were re-run after the split and all three are still red.
…#14979) (#15013) * fix(test): collect two live-service driver suites that pytest refused (#14979) SessionTakeoverTestSuite and TestTemporalInvalidation both defined __init__, so pytest declined the class and 18 test_* methods were dead text while the *_test.py name, the collected tree and the clean import all said they ran. Neither actually needed a live service, which is what made them convertible ahead of the rest of the #14979 population: - takeover_manager_e2e_test.py drives WorkflowAutomationManager in-process and TerminalWebSocket through an AsyncMock socket. Its try/except ImportError wrapper is gone (every method opened with a silent return when it tripped), test_chat_integration now mocks the orchestrator instead of asserting "if workflow_id:" against a live planner, and test_command_risk_assessment asserts SecureCommandExecutor.assess_command_risk instead of a copy of the frontend classifier defined inside the test body. The run_demo_workflow driver became test_status_reports_every_step_of_a_multi_step_workflow. - services/temporal_invalidation_test.py drives the service against an in-process fakeredis rule store. test_actual_invalidation_sweep had a hidden ordering dependency on the dry-run test; the fixtures built naive datetimes that make matches_fact raise TypeError; three assertions were conditional on the behaviour they checked; two rule branches tripped on age before the confidence and source-pattern paths were ever read. Every method ended on a return value, which would have moved all 18 into #14920's population on collection; those are gone. Ratchet: autobot-backend 77 -> 59 uncollected test_* methods, lowered in this commit with the reason inline. * Auto-fix: code formatting Applied Black, isort, and autoflake to match code-quality checks. Triggered by workflow auto-fix-formatting.yml. * fix(test): clear the file-size ratchet and re-point a stale TODO (#14979) Review of #15013 found four red checks and one dishonest deferral. File-size ratchet. takeover_manager_e2e_test.py shrank 663 -> 411, which is under MAX_LINES (600), so its grandfather entries were DELETED from both scripts/python_file_size_known_large.py and repo_tests/python_file_size_ratchet_baseline.py rather than lowered -- the audit's own message says an entry naming a compliant file exempts nothing while looking authoritative. #14629 turned discovery on for this audit, which is why it surfaced now. The explanatory comment on the 77 -> 59 change grew the uncollected-class guard 921 -> 926, past its recorded ceiling. A grandfathered exemption freezes the size it was granted for, so the comment was condensed back to 921 instead; the same two facts are still there, folded into the paragraph above the dict -- including the correction that 'every one of the remaining offenders is a live-service driver' is no longer true. Stale citation. atomic_facts_extraction_test.py was deferred citing #6994. #6994 closed on 2026-05-06 via #7076, and the TODO(#6994) in that file was planted by that same closing commit, which excluded this work from #6994's scope on its face. Nothing has tracked it since. Filed as #15043 and the comment now points there, saying plainly that self.extraction_agent is never assigned so all 8 methods would raise AttributeError if collected. The dead no-op statement goes with it. No behaviour change; the file's 8 stay inside the ceiling. Decomposition (#560). temporal_invalidation_test.py was rewritten in this PR, so the two functions over the 70-line threshold were split now rather than carried: _setup_test_facts (79) is a table of seven rows plus a loop, and test_invalidation_rule_matching (74) delegates to _assert_rule_matches and _fresh_fact. Longest function in the file is now 31 lines. The three mutations that cover it -- age, confidence and source-pattern rules -- were re-run after the split and all three are still red. * Auto-fix: code formatting Applied Black, isort, and autoflake to match code-quality checks. Triggered by workflow auto-fix-formatting.yml. --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 participant
Thinking Path
audit_ceilings()iteratedKNOWN_LARGE.items(), so it could only ever re-verify a file someone had already added to the exemption dict — it had no path to discover a file that grew past 600 lines and was never added. A fresh tree walk (excluding the same directories.pre-commit-config.yaml'spython-file-sizehook already excludes) measured 505 Python files over 600 lines, 3 already exempted, 502 completely undiscovered — includingautobot-slm-backend/services/reconciler.pyat 2222 lines, the file that surfaced this issue.The fix makes
audit_ceilings()walk every in-scope git-tracked*.pyfile and rule on it with the existing ratchet logic, rather than iterating the dict's own keys. Making that walk pass today requires grandfathering all 505 currently-oversized files at their measured size — the backlog the missing walk let accumulate silently, now visible and frozen under the same shrink-only ratchet as the original three (grow past ceiling fails, shrink not reflected in the ceiling fails, a vanished/renamed entry fails, an unlisted file over the limit fails).KNOWN_LARGEand its mirrorRATCHET_BASELINEmoved into sibling data modules (scripts/python_file_size_known_large.py,repo_tests/python_file_size_ratchet_baseline.py) so the guard's own files and its test stay under the 600-line limit they enforce on everything else, rather than needing a self-referential grandfather entry.Review follow-ups (second commit round):
testing_pattern_analyzer.pyandreconciler_remediation_tracker_expiry_14465_test.py— were created after the guard's own "never add an entry to make a new file pass" rule already existed, so silently sweeping them into the same bucket as decades-old arch(backend): extraction-first task primitives — decompose god classes by composition, not inheritance #5060 debt was wrong. Filed file-size guard: two files were grandfathered while already violating the guard's own rule #14630 for their decomposition and annotated both entries so they read as a tracked, live violation, not anonymous legacy debt.EXCLUDED_PREFIXESwas missing.worktrees/, present in.pre-commit-config.yaml's own exclude for this hook — added it, and replaced the exclusion test (which built its own expectation from the same constant it was checking) with one that parses the YAML independently and asserts set equality.audit_ceilings()'s reached count waslen(tracked)rather than files actually read, silently inflating the floor check with files that failedcount_lines. Extracted a_scan_tracked_fileshelper that only counts successful reads, restoring the original invariant.count_lines()didn't catchUnicodeDecodeError(not anOSError, raised lazily while the line generator is consumed) — one non-UTF-8.pyfile would have crashed the whole audit.KNOWN_LARGE's own entry count (MAX_KNOWN_LARGE_ENTRIES): the existing set-difference test only catches an entry added to one dict and not its mirror, not a new entry added to both together in the same change — which is exactly the shape this PR's own bulk addition took.The freeze-size question (505 modules under a growth freeze, 48 of the last 100 merged commits already touched one) is an owner decision and is tracked separately on #14547, not acted on here.
What Changed
scripts/check_python_file_size.py:audit_ceilings()now walks the tracked-file tree (tracked_python_files(), agit ls-filescall scoped byEXCLUDED_PREFIXES, mirroring the pre-commit hook's ownexclude:— cross-checked against the YAML by a dedicated test) instead ofKNOWN_LARGE.items(). A_scan_tracked_fileshelper counts only successfully-read files.count_lines()catchesUnicodeDecodeErroralongsideOSError.run_audit()'s reach check compares against a floor (MIN_TRACKED_PY_FILES) over the whole tree rather thanlen(KNOWN_LARGE).KNOWN_LARGEis loaded from a sibling data module by path.scripts/python_file_size_known_large.py(new): the grandfather dict — 505 entries, one per file the walk found over 600 lines, each at its measured size; the two recently-created entries are called out distinctly with an issue reference.repo_tests/python_file_size_ratchet_baseline.py(new): the mirroredRATCHET_BASELINE, kept as an independent second copy per the existing two-copy design, same annotations.repo_tests/python_file_size_ratchet_test.py:test_recorded_ceilings_match_the_files_todayupdated for the new reach semantics; two audit-simulation tests isolateKNOWN_LARGE/tracked_python_filesfor their synthetic scenarios;test_a_shrunk_file_must_lower_its_ceilinghandles the boundary where a ceiling of 601 shrinks to exactlyMAX_LINES; new tests pin the discovery fix (an unlisted oversized file is caught, an unlisted compliant file stays silent), theEXCLUDED_PREFIXES/YAML cross-check, and theKNOWN_LARGEcardinality ratchet..github/workflows/code-quality.yml: updated the--audit-ceilingsstep's comment to describe the tree walk.Verification
All execution was done in scratch copies under
/tmp, never in the worktree or main tree.python3 -m pytest repo_tests/python_file_size_ratchet_test.py -q— 26 passed.python3 scripts/check_python_file_size.py --audit-ceilingson the merged tree (post-rebase, with the new data modules tracked) —python-file-size ceilings: 4702 files scanned, 505 grandfathered, all live and at size., exit 0.KNOWN_LARGEand tracked it: the audit correctly failed (... 650 lines (max 600), exit 1) — the exact case it could not previously see. Removed it: clean again (exit 0).test_recorded_ceilings_match_the_files_today,test_audit_discovers_an_unlisted_oversized_file,test_audit_is_clean_for_an_unlisted_compliant_file), 23 passed.audit_ceilings, keep the return statement): also 3 red, 23 green — the same three. Extracting_scan_tracked_filestiesreacheddirectly to whatever list is scanned, so there is no longer alen(tracked)computed independently of the loop for a narrow mutation to hide behind; both mutation shapes are caught by the same margin.black --check(matching the repo's pinnedline-length = 120/target-version = py312config) is clean on all touched/new files; two pre-existing lines it also flags in the ratchet test predate this change (confirmed againstorigin/Dev_new_gui) and were left untouched as out of scope.Reconciled with #14976
#14976 landed on base and touches the same hook. Both changes survive the merge and are
complementary: #14976 closes the fail-open on the commit path (a file that cannot be
read is reported, not skipped); this one closes the blind spot on the audit path (an
oversized file nobody listed is discovered, not invisible).
The one conflicted file was
scripts/check_python_file_size.py, resolved keep-both inboth hunks: the docstring holds #14975's "unmeasured is not within-the-limit" paragraph
alongside this branch's tree-walk paragraph, and
unmeasured()sits next to_vanished_entry_problem(). The only thing dropped is the oldaudit_ceilingsheadergit's hunk dragged in behind
unmeasured— the walk-basedaudit_ceilingsthat replacedit is what remains.
main()merged cleanly and still emitsunmeasured(rel)beforereturning 1. The ratchet test auto-merged with all three of #14976's tests intact.
This branch widened
count_linesto catchUnicodeDecodeErroras well asOSError, so afile that does not decode is a fourth way to reach
unmeasured. The message named threecauses; it names encoding too now, and
test_a_non_utf8_file_is_unmeasured_rather_than_measuredpins that path — the probe is deliberately over
MAX_LINES, so anything that measured itinstead would trip the size verdict rather than agree with the test.
Re-measured on the reconciled tree:
4819 files scanned, 512 grandfathered, all live and at size., exit 0 — no entry moved on this merge, soMAX_KNOWN_LARGE_ENTRIESstays 512.repo_tests/python_file_size_ratchet_test.py— 30 passed (26 + #14976's 3 + the new one).Mutations re-run on the reconciled tree, each restored: the five ratchet mutations as
above; reverting #14976's
unmeasured(rel)call to a barecontinue→ 3 red; making theunmeasured message quote
(max 600)→ 2 red; droppingUnicodeDecodeErrorfromcount_lines→ the non-UTF-8 test red. Restored: 30/30. Population starved to 0 files,then 1:
run_auditexits 1 both times.The annotations, and why they nearly went missing
Review caught that the programmatic re-measure in
8cc83b7ebe4rewrote both dict literalsfrom
git ls-filesand emitted"path": countand nothing else — destroying bothhand-written annotations above. The claim in "Review follow-ups" above was false for three
commits. Both blocks are restored in both copies, so it is true again.
The generic lesson is the durable part: a generator cannot preserve what it does not know
about, so anything hand-added inside generated output is destroyed on the next run rather
than kept by default. There will be another re-measure, because the snapshot must agree with
base at check time. So the annotations are now defended two ways that do not depend on
whoever runs it remembering:
ANNOTATED_AS_LIVEin the baseline data module holds the annotated paths outside thedict literal, where a rewrite of that literal cannot reach them.
test_an_entry_singled_out_as_live_keeps_its_notereads both files and fails if eitherstops carrying the inline comment, or drops its
#14630anchor.The re-measure itself now harvests comment blocks out of the literal and re-emits them above
the entry they annotated, and refuses outright if an annotated path has left the dict. Run
twice in a row it reproduces the annotations byte-for-byte.
Mutations on the new guard, each restored: annotation stripped from the
KNOWN_LARGEcopyalone → red; from the baseline copy alone → red; from both, the exact regression that
happened → red; comment kept but the
#14630anchor removed → red; the annotated entrydropped from both dicts with the side-table intact → red. The eight earlier mutations
(five ratchet, three on #14976's unmeasured path) were re-run on this tree and all still
kill their guard. Restored: 31 passed.
One thing for a reviewer to know:
repo_tests/python_file_size_ratchet_test.pynow sits at599 of its 600 lines. The new test was paid for by trimming this branch's own
MAX_KNOWN_LARGE_ENTRIESnote. Anything further added there failstest_recorded_ceilings_match_the_files_todayrather than sliding past — loud, but worthknowing before writing the next test in that file.
Closes #14547
Model Used
Claude Opus 5 (1M context)