test: a ledger of which checks have ever been red (#918) - #925
Conversation
Closes #918, phase 4 of #858, and the honest version of it. Nothing recorded whether a check had ever been red. That is the gap that let 39 checks across 35 suites ship unable to fail, three of them inside the suite whose whole purpose is to stop exactly that. The gate answered "did anything print FAIL" and had never answered "could anything print FAIL". WHAT THIS LEDGER CLAIMS, AND WHAT IT DOES NOT. It records that a named check WAS OBSERVED RED in a recorded run. It does NOT claim the check is proven able to fail: that is a stronger statement, it needs a named mutation applied deliberately, and conflating the two would put a claim in the ledger that nothing measured -- the `defeated: 0` shape from VACUITY_MODES section 1, a number that reads as evidence and is not. SO EVERY ENTRY CURRENTLY READS `never`, and that is the finding rather than an embarrassment. #918 asks "nothing records whether a check has ever been red"; the answer this ships is "and now something records that almost nothing has". A ledger of 608 rows, none ever observed red, is a measurement of how much of the corpus has never been attacked, and that measurement is worth having on day one. WHAT FILLS IT. Not only deliberate mutation runs. Every real CI red fills it, every flake, every bisect, and those arrive whether anyone remembers or not. A mutation run is the deliberate accelerator, not the only source. Raised by OffgridwithJD, and it matters because "only a mutation run can retire debt" invites someone to build a mutation gate before it is needed. THE MUTATION COLUMN EXISTS FROM v1 with nothing filling it automatically, because adding a column later means rewriting every entry. If an entry can record WHICH mutation reddened a check, the catalogue a mutation gate would need builds itself out of work people already do by hand -- the vacuity branches are writing nine to eleven per change, each chosen to revert one property. A RENAME IS REPORTED, NOT SILENTLY ABSORBED. The ledger is keyed by check name, and names here are prose that gets rewritten freely -- which is most of why #917 exists. So a rename loses the check's history and reads exactly like a brand-new check that has never been red, the ONE state this ledger exists to distinguish. It cannot be prevented without a synthetic id someone would have to maintain, and this repository removed a hand-maintained list today for that reason. So a name that appeared while another disappeared is NAMED. Both directions are required: reporting a rename on every added check is noise that gets it ignored. A DUPLICATED NAME SHARES ONE ROW, so one of the two going red would mark BOTH as observed red -- a claim about a check nothing attacked. Also reported rather than prevented. The real corpus carries four today, which is how it was noticed at all: 612 records reduce to 608 rows. TWO TRACKED FILES CARRY THE DEBT, per #858's own constraint. check_ledger.tsv and check_ledger_budget.txt are in the tree, so a change to either is a diff a reviewer sees. PGC_SKIP_TIMING is the precedent for why this is not an environment variable: set in two workflow files, it suppressed whole suites for months and no diff ever showed it. Both numbers may only go down. The second number is the one that is easy to forget: suites_not_covered is 250 of 251, because the ledger can only be seeded from suites whose logs exist, and the matrix does not preserve them. Counting it separately stops "we ledger 608 checks" reading as "we ledger the corpus". It is a real limit, stated rather than hidden, and it burns down as suites are seeded. The gate refuses a check the ledger has never seen, so a new check cannot enter as silent debt -- while the existing 608 are grandfathered, because a gate that fails on 3,762 unledgered sites is one somebody disables under deadline. Evidence: selftest exit 0, 612 checks, 0 failures; the gate rc=0 against the committed files; 27 pytest; shellcheck rc=0; docs_style PASSED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
…onvention (#918) Reported by OffgridwithJD, who found it by asking whether their own six branches added duplicate check names. All six did. THE FOUR DUPLICATES I REPORTED ARE NOT FOUR INSTANCES, THEY ARE A CONVENTION. `premise: the pytest layer is where THIS PART thinks it is` says "this part" precisely so the sentence can be copied into any part, and main already carries two copies of it and two of `premise: the harness library is where this part thinks it is`. The count is the count TODAY and grows with every part anyone adds. So a ledger keyed on (suite, name) is not a ledger of checks. It is a ledger of check NAMES, and the two differ by however many parts share a boilerplate premise -- with the consequence this ledger cannot have: one sharer going red marks every other as observed red, a claim about a check nothing attacked. THE KEY IS NOW (suite, part, name). The part comes from pgc_record, derived from BASH_SOURCE, which #917 adds in the commit below this one. Not a convention change, so the next part written the same way is keyed correctly without anyone remembering, and it closes a blind spot in the rename detector: a premise moving between parts was indistinguishable from a rename and is now an appearance and a disappearance in two different parts, which the detector does not pair. Measured over a real run, 583 records: distinct (suite, name) 579 distinct (suite, part, name) 582 One duplicate survives, and it is a GENUINE one rather than a convention artifact: 340-the-binary-must-be-built-from asks `premise: the fixture fingerprints at all` twice within the same part. Naming that precisely, instead of losing it among three copied premises, is the point. The reviewer also renamed the premises in all six of their own in-flight branches so each names its own subject -- thirteen renames, each part now contributing zero duplicated names -- which is why this number does not grow by six the moment those land. They left main's copies alone as this PR's scope, which is right: this change reports rather than prevents. Re-seeded from a green run, keyed the new way: 613 rows, none ever observed red. Evidence: selftest exit 0, 614 checks, 0 failures; gate rc=0 against the committed files; 140 pytest passed; shellcheck rc=0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
…tor caught the rename (#918) OffgridwithJD looked at the collision that survived the (suite, part, name) key and found it is not a repeat of one question. At 340:268 and 340:894, `premise: the fixture fingerprints at all` is asked once of the SOURCE-PARTITION fixture and once of the LOCALE SWEEP: 268 _fs_before="$(pgc_source_fingerprint "$_fs_rp")" -> did the a.c/b.c partition fixture produce a fingerprint at all 894 _lc_vals="$_lc_vals $(LC_ALL=... pgc_source_fingerprint "$_lc/tree")" -> did every locale in the sweep produce one Two questions about two different code paths, sharing a sentence. That is the case where a shared ledger row is WRONG rather than merely imprecise: one failing marks the other's premise as observed red, and they are not about the same code. I FIXED IT RATHER THAN REPORTING IT, which departs from this PR's stance, and the distinction is worth stating. "Report rather than prevent" is right about the CONVENTION -- `premise: the pytest layer is where THIS PART thinks it is` is phrased to be copied, rewriting the corpus's shared premises is churn, and the tool exists to surface them. This is not that. It is one file, two lines, and the two premises are about different subjects. Naming them apart takes the corpus to ZERO collisions: 614 records, 614 distinct keys. AND IT IS THE RENAME DETECTOR'S FIRST TEST ON REAL DATA rather than fixtures. Run against the ledger seeded before the rename: possible rename: premise: the fixture fingerprints at all -> premise: every locale produced a fingerprint (in harness_selftest/340-the-binary-must-be-built-from, history: last red never) rename scan: appeared=2, vanished=1 appeared=2 against vanished=1 is honest: the two premises shared ONE row before, so one of the new names has no predecessor to pair with. Re-seeded: 614 rows, none ever observed red. Evidence: selftest exit 0, 614 checks, 0 failures, 0 collisions; gate rc=0 against the committed files; shellcheck rc=0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
|
Reviewed The finding:
|
linuxhikerpm
left a comment
There was a problem hiding this comment.
Blocking at exact head 671a946059ddaf88b3289d28ee891c20ab8fdb0d: the ledger is not yet an operational or fail-closed gate.
-
pgc_ledger.pyis never invoked by the runner or CI. No workflow consumesgate,merge, orrename-scan, and runner logs are deleted. CI reds therefore cannot populate the ledger and the gate cannot reject a new check. -
Missing/malformed inputs false-green.
read_records()ignores unreadable files, short records, and unrecognized content. Reproduced rc=0 for a nonexistent log, an empty log, and a known record missing its verdict. Require nonempty reconciled input and fail closed on read/parse errors. -
Mutation attribution is invocation-wide. One
--mutationvalue is copied to every failure in every input log; two independent failures merged withMUTATION_Aboth acquired that mutation. The issue requires per-check attribution derived from the run. -
Multi-log handling is incorrect. Two ordinary logs containing the same check are reported as a duplicate. A rename detected with the after-log alone disappears when before+after are supplied. Global positional
zip(appeared, vanished)also misses a real rename when unrelated movement in other parts shifts ordering; group by(suite, part)before pairing and distinguish runs when checking duplicates. -
The tracked budgets are not monotonic. Debt=2 passes with budget 2 and still passes after increasing the committed budget to 999.
suites_not_coveredcan also be bypassed by omitting optional--registered. Compare against the prior tracked ceiling and require registry input for coverage claims. -
git diff --checkreports 632 trailing-whitespace errors, including all 614 ledger rows because an empty mutation is encoded as a trailing tab.
The (suite, part, name) key improvement is sound and fixes cross-part collisions, but the paired tests use only valid single logs and do not exercise these failure modes. Please add both shell and pytest red/control arms for each integrity boundary and wire the resulting gate into retained runner/CI artifacts.
|
Two additional exact-head findings from the full adversarial pass:
These supplement the changes-requested review at the same head; the existing six blockers still stand. |
|
Adversarial pass on 1. The mutation column silently overwrites, so it cannot accumulate a catalogueThat column exists so the mutation catalogue builds itself out of work people already do by hand. It keeps only the last one: No warning, no append, no record that M1 ever existed. A check that two different mutations reddened ends up claiming one of them, and which one depends on run order. That defeats the stated purpose rather than limiting it: six branches of mine carry 55 one-property mutations with their reddened arms named, and merging those runs one at a time would leave a single mutation per check. The fix is a decision, not code: either the column is a set (append, deduplicated) or it is explicitly "the most recent mutation that reddened this check" and the docstring says so. I would take the set — the value is in the accumulation — but either is honest and the current state is neither. 2. Drift is one-directional: a ledger row whose check no longer exists is kept silentlyThe gate reports
3. A stale number inside the file whose whole subject is stale numbers
Trivial to fix and worth fixing precisely because of where it is. On the budget file being typed at allI went looking for this as a defect and came away agreeing with it, so I will say so. Verified and soundMerging one run twice is byte-identical. A later date advances last-red; a subsequent green run does not erase it. A mutation supplied on a run where the check stayed green is correctly not recorded. The gate refuses a record with no ledger row by name, with a control at rc=0. 614 records give 614 distinct keys after the 340 rename, and the key's part comes from One merge note, recorded here rather than only in messages: |
…annot deadlock (#918) jd's direction: shipping the recording half alone would be shipping something that does nothing. A ledger nothing feeds and nothing reads is not a staged feature, it is a data file with no producer and no consumer -- the `defeated: 0` shape one level up. So the gate is fixed rather than dropped. Ten findings, from @linuxhikerpm and OffgridwithJD, every one reproduced first. IT IS NOW FED AND IT NOW REFUSES. Nothing in the repository called the tool: zero references in .github/, zero in the runner, so "the gate refuses a check the ledger has never seen" was false as written. run_all_versions.sh now runs it before it removes the build directory, which is the only place a matrix run can reach every suite's log. CI verifies; humans commit the ledger, because a ledger CI rewrote by itself would be a file nobody reads changing under everybody. THE DEADLOCK WAS THE DESIGN, NOT THE NUMBER. Bounding `checks_never_observed_red` means every added check breaks the gate: a new check enters as `never`, so the only way to land one was to raise a number the file says in capitals may only fall. It shipped at 614 rows, 614 never, ceiling 614. The two numbers are different kinds of thing and the file now says so. `checks_never_observed_red` is a CENSUS, asserted to match the ledger in both harnesses so it cannot drift. `suites_not_covered` IS a ceiling, because adding a check to a covered suite does not move it, and the gate compares it against the previously committed value and refuses an increase -- so "may only fall" is mechanism rather than prose. AND THE REFUSAL IS RESTRICTED TO SUITES THE LEDGER COVERS, which is the MEANING of that ceiling rather than a softening of the gate. Without it the gate refuses every check of all 250 uncovered suites and reddens the whole matrix on its first run, which is a gate somebody turns off within the week. It tightens on its own as suites are seeded, and an arm pins that a new check in a NOW-covered suite is refused again. FAIL CLOSED. A nonexistent log, an empty one and a record missing its verdict each returned rc=0. An integrity failure that reads as a clean run is worse than no gate because it certifies. They return 2, distinguishable from a real refusal at 1, and --registered is required rather than silently skipped. THE MUTATION COLUMN ACCUMULATES a set rather than overwriting, because keeping only the last one records the most recent attack rather than the catalogue the column exists to become. One --mutation cannot be attributed across several logs at once. MULTI-LOG HANDLING was wrong in two directions. The same check in two logs is two RUNS and was reported as a duplicate; the same name twice in ONE log is the duplicate, and is what is reported now. Renames are grouped by (suite, part) before pairing, because a global positional zip misses a real rename whenever unrelated movement elsewhere shifts the ordering -- and a before-log and an after-log together are refused rather than silently finding nothing, since the vanished name is present in the union. 614 rows ended in a tab, because an empty mutation was an empty last field. An absent mutation is now `-`; `git diff --check` reports nothing. Evidence: selftest exit 0, 638 checks, 0 failures; 9 pytest; shellcheck rc=0; docs_style PASSED; git diff --check clean. Both harnesses carry red-and-control arms for every integrity boundary above, including the deadlock as its own arm. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
…s' into feat/918-mutation-ledger # Conflicts: # CHANGELOG.md
|
Treated as new, as you asked. I ran the thing the redesign turns on — a real PG 17 matrix on The blocker: CI is not running, and will not start
The cause is the base. This PR targets One file, one region: your entry and #923's under the same heading. It is fed, and I can prove it from a real runFull So the gate reached the real logs of a real matrix, and And the refusal works. Driven with a record the ledger has never seen, in a covered suite: Fail-closed holds where you said it does — a nonexistent log, an empty log, a malformed record, and a missing The finding: "the ceiling may only fall" is true of the tool and false of the wiring
The monotone block is guarded by
So the tool is right, and 410's scratch-repo arms prove it. What nothing does is compare the real committed budget against git. And adding the flag at that call site does not fix it. The arm that closes it belongs where the repository is rather than where the build copy is: compare the working budget against What I checked and am not relitigatingThe runner does invoke the gate, it runs before On shape, since you askedThe census/ceiling split is right, and the reason is stated correctly: every new check enters as One consequence worth naming out loud: today the ledger covers one suite, so the gate refuses unknown checks in A main-tree defect this run surfaced, which is not yours to fix hereThe same report prints two different "accounted" totals: 237 and 235, differing by exactly 2. The population line uses the wide reader ( |
…ng (#918) Reported by OffgridwithJD, measured on the shipped form. I wrote that the gate "now refuses to see the ceiling raised above its previously committed value". That was true of pgc_ledger.py and false of run_all_versions.sh: the runner's exact invocation, no --against rc=0 the raise is not refused --against HEAD, absolute path rc=0 "no prior ceiling to compare" --against HEAD, repo-relative path rc=1 correctly refused THE MIDDLE LINE IS THE ONE THAT MATTERS. `git show REF:PATH` needs a repo-relative path and the runner passes an absolute one inside a copied build directory, so _committed_budget returned None and the gate printed a note that READS LIKE A PASS while the ceiling it was asked to enforce went unchecked. Asked to compare, unable to compare, is not the same as nothing to compare -- and that is the fail-open shape this whole change is about, in the code that closes it. So the tool resolves the path itself, through the budget's own git toplevel, and every failure to resolve it is an ERROR. The caller no longer has to know. WHICH REF IS NOW A DECISION RATHER THAN A DEFAULT. `--against HEAD` compares a committed file against ITSELF: for any change already committed the working budget and HEAD's are identical, so it catches only an uncommitted raise. The property that matters is that a branch may not raise the ceiling relative to MAIN. The runner prefers origin/main, falls back to HEAD, and PRINTS the fallback and what it costs, because a silent fallback is a gate quietly enforcing less than it claims. THE SCRATCH-REPO ARMS WERE NECESSARY AND NOT SUFFICIENT, which is the gate-nothing-invokes finding one level down: they proved the tool while no wired invocation exercised it. There are now arms in the REAL tree at the REAL path -- an absolute path resolves rather than shrugs, a budget git has never seen is an integrity failure rather than a note, and raising the tracked ceiling in place is refused, with the file restored byte-exact. Also in this commit: the merge of #923's base, whose CHANGELOG entry conflicted with this one. Both entries are kept, #917 then #918, since they describe two changes under one heading. #925 was CONFLICTING against its base, which is why no CI had run on it. Evidence: selftest exit 0, 678 checks, 0 failures; 145 pytest passed (the 35 errors are /usr/local/pg18a absent on this host, identical on main); shellcheck rc=0; docs_style PASSED; git diff --check clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
Reported by OffgridwithJD, and it is my own sentence one level up. `origin` is not a fixed thing. In a contributor's clone it is their FORK -- they measured theirs 446 commits behind upstream -- so `--against origin/main` compared the ceiling against a 16-day-old main and printed that it had compared. THE DIRECTION IS WHAT MAKES IT FAIL OPEN. The ceiling may only fall, so an older main carries one that is higher or equal, and a raise passes whenever the stale prior is high enough. Never falsely red, silently weaker, with a line that reads like the enforcement happened. "Asked to compare, unable to compare, printing a note that reads like a pass" was the previous finding; this is the same shape with "compared against the wrong thing" in place of "could not compare". AND THE FALLBACK WAS THE SAME SHAPE AGAIN. When origin/main did not resolve the runner printed the cost and proceeded with `--against HEAD`, which compares a committed file against itself and therefore catches nothing for any change under review. A fallback that enforces less while saying so is still a gate enforcing less, and one level down I had already made an unresolvable prior an error. SO THE PRIOR IS RESOLVED, AND NEVER GUESSED. `--against auto` takes GITHUB_BASE_REF, which in CI names the PR's target and IS the prior by definition, or the local main's configured upstream outside CI, which is the per-clone answer to "which main is mine". Neither available is rc=2. The ref used is printed, so a reader can see which prior the comparison actually made. AND CI MUST FETCH THAT BASE. actions/checkout takes one ref at depth 1 and the suites job set no fetch-depth, so the base branch is absent and `auto` would stop the run -- correctly, but for a reason the workflow owns rather than the author. The suites job now fetches it at depth 1, guarded on github.base_ref so a push build does not fail on it. Only the file at that commit is read. Arms for all of it, including the two that would have caught me: no base ref and no upstream is an integrity failure with its reason named, and a GITHUB_BASE_REF whose ref is absent says the checkout needs to fetch it rather than falling back. Three arms testing the design this replaces were deleted rather than left to pass against nothing. Evidence: selftest exit 0, 686 checks, 0 failures; 9 pytest; shellcheck rc=0; docs_style PASSED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
|
The CI half of the
So in CI Fixed at One residual on the non-CI path, measured in the clone the new docstring cites:
I am not arguing for a different ref — "the configured upstream of the local main" is the right answer to "which main is mine" and I cannot see a better one that does not guess. I would add the distance: git rev-list --count <prior>..HEADprinted beside the ref. "comparing against origin/main (446 commits behind HEAD)" tells the reader what the comparison is worth; "comparing against origin/main" does not. Same move as printing the ref, one step further, and it costs nothing when the number is zero. Everything else in my earlier review stands as written, including the matrix evidence on |
…anged and the destination did not (#918) Reported by OffgridwithJD, who measured it in their own clone rather than predicting it. `main@{upstream}` is the per-clone answer to "which main is mine", and in a contributor's setup it resolves to their FORK -- `git push -u origin main` is what sets that config. Theirs is 446 commits behind upstream, so `--against auto` outside CI lands on exactly the ref the hardcoded `origin/main` did. The route changed; the destination did not. It is not the same defect: the ref used is printed, so a reader can see `origin/main`. But a reader cannot see that it is 446 commits stale, and the direction still fails open -- the ceiling may only fall, so an older main carries a higher one and a raise passes whenever the stale prior is high enough. There is no better ref to pick that does not guess, and I am not going to guess. So the weakness is made VISIBLE instead: ceiling against origin/main (446 commits behind HEAD): 250 -> 250 Naming the ref told a reader WHICH prior was used. This tells them what the comparison is worth. It costs nothing when the number is zero, where the label is omitted entirely. AND THE CI HALF IS NOW MEASURED RATHER THAN CONSTRUCTED, from my own run 34502198282 on fb87237: origin/main does not resolve here, so the ledger ceiling is ceiling against HEAD: 250 -> 250, which does not rise The fallback fired, the prior was HEAD, and the comparison was a committed file against itself -- catching nothing for any change under review, in the only place this gate runs for real, with a line above it that reads like enforcement. The previous commit removed that fallback and made CI fetch the base; this is the evidence it needed to be removed rather than kept with a warning. Evidence: selftest exit 0, 690 checks, 0 failures; 9 pytest; shellcheck rc=0; docs_style PASSED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
|
CI is red on From the I reached the same failure independently before seeing this. A full matrix run on So Three states, not twoThe distinction needed here is the one this branch already drew one level down:
The middle row is the first-landing case, and it is genuinely nothing to compare rather than could not compare. It is also self-clearing: once the file is on A second, smaller one in the same four linesThe runner prints for an integrity failure. The gate takes care to distinguish The half that is verifiedBoth of those runs reached the gate with real logs from a real matrix, printed the census and the coverage, and reported |
…wo failure kinds are different (#918) THE GATE CAUGHT ITS OWN BOOTSTRAP the first time it ran in CI, which is the right outcome for a rule that could not be satisfied. From run 34503924812: ledger integrity failure: --against refs/remotes/origin/feat/917-machine-readable-results was given, but test/check_ledger_budget.txt does not exist at that ref #925's base is #923's branch, where the budget does not exist because THIS change adds it. So `auto` resolved the base correctly, fetched it, found no prior, failed closed, and reddened the matrix -- and a PR introducing the file could never pass its own gate. OffgridwithJD hit the identical failure independently in their own clone, resolved to their fork, where the file is also absent. THREE STATES, NOT TWO, which is the distinction this change already draws one level down: the prior ref does not resolve ERROR. Asked to compare, unable to. the ref resolves, file absent there NO PRIOR. Nothing could have been raised relative to a file that did not exist. the ref resolves, file present COMPARE. The middle one is the first-landing case and it is genuinely "nothing to compare". It is expressed as the PROPERTY -- absent at the prior -- rather than as a flag or a date, so it clears itself: once the file is on main every future base carries it, and there is no exemption left for anyone to forget to remove. It is not a hole. Deleting the budget on a branch and re-adding it higher does not reach it, because the file still exists at the prior and the comparison happens. AND THE RUNNER COLLAPSED THE TWO FAILURE KINDS. The gate distinguishes rc=1, a real refusal whose fix is to regenerate the ledger, from rc=2, the gate unable to do its job at all. The runner reported both as "has a check the ledger has never seen" -- sending the reader at a repair that cannot help, three lines below the gate's own "new this run=0", which says the opposite. Also OffgridwithJD, from the CI log of this branch. It now branches, and both arms still fail the major. THE ARMS FOR IT WERE WRONG TWICE, both times in the same way. They counted occurrences inside `grep -A6`, `-A8` and `-A12` windows, and every one broke the moment the call site gained a comment: a window's size is a fact about formatting. The block is now EXTRACTED and tested, as selftest 320 already does with the runner's classifier. Then the extraction counted the block's own explanation as a second occurrence of the sentences it was counting, so comments are stripped -- selftest 080's control problem, met in a fifth file tonight. One arm was deleted rather than fixed: it read a variable defined ninety lines below it, and the block that defines the variable already asserts the same thing. Evidence: selftest exit 0, 696 checks, 0 failures; 145 pytest passed; shellcheck rc=0; docs_style PASSED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
Reported by OffgridwithJD, who scoped it precisely rather than leading with the
headline: they checked the production path FIRST and confirmed it was sound, so
"the fail-open hole is back" would have been wrong.
The three states this round exists to draw had collapsed back to two on one path.
The ref-resolution check lived inside the `auto` resolver, so it covered the
production call site and nothing else. An EXPLICIT ref that did not resolve fell
through to the file-absent branch:
--against refs/heads/no-such-ref-xyz -> rc=0
"no budget at refs/heads/no-such-ref-xyz (distance from HEAD unknown):
this change introduces it, so there is no prior ceiling it could have raised"
Two things wrong in one line. The verdict is "nothing to compare" where the truth
is "could not compare". And it asserts a change introduces a file at a ref that
does not exist, one clause after saying the distance from HEAD was unknown -- the
code knew it could not resolve the ref and contradicted itself inside one
sentence.
Resolving now belongs to the reader, where every caller passes through, so the
file-absent branch describes only what it claims: a file missing at a ref that IS
there. All three states are pinned by arms against one scratch repo carrying a
branch with the budget and a branch without it -- unresolvable is rc=2 and never
says "introduces", absent-at-an-existing-ref is rc=0 and does, and present
compares. Without the last two the fix would be satisfied by refusing everything.
Not reachable from the runner, which always passes `auto`. It was a trap for these
arms, for anyone driving the tool by hand, and for whoever later passes a concrete
ref because `auto` was inconvenient.
AND THE PREVIOUS HEAD WENT 12/12 IN CI, with the gate's own output confirming
every piece of the redesign:
ledger census: rows=693 | never observed red=693, ever red=0, new this run=0
ledger coverage: registered=251 | covered=1, not covered=250, ceiling=250
no budget at refs/remotes/origin/feat/917-machine-readable-results
(1 commit behind HEAD): this change introduces it
Fed from real matrix logs, refusing nothing because nothing is unledgered, the
prior's distance printed beside it, and the bootstrap recognised rather than
fatal.
Evidence: selftest exit 0, 704 checks, 0 failures; 9 pytest; shellcheck rc=0;
docs_style PASSED.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
OffgridwithJD
left a comment
There was a problem hiding this comment.
Approving at exact head 71a7c9bcd02a57201d9e010bbfb94abd11835071 — 12/12 SUCCESS, mergeStateStatus=CLEAN.
Four rounds of findings from me, every one fixed, and I re-measured the last round on this head rather than reading the diff.
The three states, measured on this head in my own clone
--against refs/heads/no-such-ref-xyz rc=2 "that ref does not resolve here, so the
prior ceiling cannot be read"
--against auto (origin/main lacks it) rc=0 "no budget at origin/main (464 commits
behind HEAD): this change introduces it"
--against HEAD (this tree carries it) rc=0 "ceiling against HEAD: 250 -> 250, which
does not rise"
Three distinct verdicts, three distinct messages. The one that was wrong is right in the way that matters most: the unresolvable ref no longer claims "introduces". A message asserting something the code cannot know was the real defect there, more than the exit status was.
Why the chain is worth recording, since the final state hides it
| round | the finding | how it failed |
|---|---|---|
| 1 | the monotone ceiling was unwired — if args.against: and the runner never passed it |
with the absolute build-dir path it would have printed "no prior ceiling to compare" and returned 0 |
| 2 | the fix named origin/main, which is 446 commits stale in my clone |
the ceiling only falls, so a stale prior is always higher: fails open while printing that it compared |
| 3 | auto resolved correctly and the budget cannot exist at the prior, because this PR introduces it |
CI red on both suites |
| 4 | an explicitly passed unresolvable ref read as "no prior" | said "this change introduces it" about a ref that does not exist |
Round 2's CI half was settled by this PR's own log — origin/main does not resolve here then ceiling against HEAD: 250 -> 250, a committed file compared against itself — which is the evidence that the fallback needed removing rather than keeping with a warning.
The claim the redesign turns on, verified three times
The gate is fed from real matrix logs, not fixtures:
my run on 2289f10d ledger census: rows=638 | never observed red=638, new this run=0
ledger coverage: registered=251 | covered=1, not covered=250, ceiling=250
ALL VERSIONS PASSED, rc 0
my run on 3d880f49 same census and coverage, then the bootstrap failure (round 3)
CI on 380f9f68 rows=675, and the fallback line that proved round 2
not covered=250 sits exactly on the ceiling, which is the tight place to be: one newly registered suite nobody seeds reddens it.
Fail-closed holds where it should — a nonexistent log, an empty log, a malformed record and a missing --registered are all rc=2, distinguishable from the rc=1 refusal. rename-scan groups by (suite, part), so two parts each renaming in one run pair correctly instead of positionally, and a before+after union in one call is refused. The ledger is LC_ALL=C sorted, so an added check inserts at a content-decided point rather than at the end — the #554 conflict lesson applied, and it matters because every PR adding a harness_selftest check will regenerate this file.
On shape
The census/ceiling split is right and the reason is stated correctly: every new check enters as never, so bounding that number makes landing a check require raising a number the file says may only fall. That is a deadlock, not a budget. Restricting the refusal to covered suites is the meaning of suites_not_covered rather than a softening, and the arm that a new check in a now-covered suite is refused again is what makes it a tightening.
Putting the ref check in the reader rather than in auto is also right: a fact belongs where every caller passes through it, and a policy belongs where it can change without touching the fact.
What I did not verify
I did not run the PG 18 matrix locally for this head; CI's suites (PG 18) is green and that is the evidence for it. My runs were PG 17 on an assert build. And the census of 693 never rows means nothing has been observed red yet — the ledger's value arrives as checks are attacked; what is verified today is that it refuses an unknown check and cannot be quietly widened.
@linuxhikerpm your block stands at 671a9460, four heads back. The monotone enforcement you would have been looking at then did not exist in working form — it was unwired at that commit, which is finding 1 above.
Closes #918 — phase 4 of #858, the last of the three. Stacked on #923; base is
feat/917-machine-readable-results, so review in order.The honest version, and why that is the finding
Nothing recorded whether a check had ever been red. That is the gap that let 39 checks across 35 suites ship unable to fail, three of them inside the suite whose whole purpose is to stop exactly that.
This ledger records that a named check was observed red in a recorded run. It does not claim the check is proven able to fail. That is a stronger statement, it needs a named mutation applied deliberately, and conflating the two would put a claim in the ledger that nothing measured — the
defeated: 0shape fromVACUITY_MODES§1, a number that reads as evidence and is not.So every entry currently reads
never. #918 asks "nothing records whether a check has ever been red", and what this ships answers "and now something records that almost nothing has". A ledger of 608 rows, none ever observed red, is a measurement of how much of the corpus has never been attacked — and that measurement is worth having on day one.What fills it
Not only deliberate mutation runs. Every real CI red fills it, every flake, every bisect — and those arrive whether anyone remembers to run something or not. A mutation run is the deliberate accelerator, not the only source. Worth stating, because "only a mutation run can retire debt" invites someone to build a mutation gate before it is needed.
The format
The mutation column exists from v1 with nothing filling it automatically, because adding a column later means rewriting every entry. If an entry can record which mutation reddened a check, the catalogue a mutation gate would need builds itself out of work people already do by hand.
Two losses that are reported rather than prevented
A rename. The ledger is keyed by check name, and names here are prose that gets rewritten freely — most of why #917 exists. So a rename loses the check's history and reads exactly like a brand-new check that has never been red: the one state this ledger exists to distinguish. It cannot be prevented without a synthetic id someone would have to maintain, and this repository removed a hand-maintained list today for that reason. So a name that appeared while another disappeared is named. Both directions are required — reporting a rename on every added check is noise that gets it ignored.
A duplicated name. Two checks with the same name in one suite share a row, so one going red would mark both as observed red. Reported for the same reason. The corpus carries four today, which is how it was noticed: 612 records reduce to 608 rows.
The debt is two tracked files
Per #858's own constraint.
test/check_ledger.tsvandtest/check_ledger_budget.txtare in the tree, so a change to either is a diff a reviewer sees.PGC_SKIP_TIMINGis the precedent for why this is not an environment variable: set in two workflow files, it suppressed whole suites for months and no diff ever showed it. Both numbers may only go down.The second is the one that is easy to forget, and it is a real limit. The ledger can only be seeded from suites whose logs exist, and the matrix does not preserve them — so 250 of 251 suites have no rows at all, and the gate cannot refuse a new check in a suite it has never seen. Counting it separately stops "we ledger 608 checks" reading as "we ledger the corpus". It burns down as suites are seeded.
The gate refuses a check the ledger has never seen, so a new one cannot enter as silent debt — while the existing 608 are grandfathered, because a gate that fails on 3,762 unledgered sites is one somebody disables under deadline, and then we are back at
PGC_SKIP_TIMINGwith extra steps.Evidence
What I did not build, deliberately
A harness-driven mutation gate. It needs a mutation catalogue per check to be anything other than random line-deletion, and that catalogue does not exist in machine-readable form yet — it is being hand-written, nine to eleven mutations per change, in the vacuity work happening now. This ledger is the right place for it to start accumulating, which is why the column is here. Building the gate first would be building the machine before the input.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK