test: the gate refuses two checks that share one ledger key (#982)

[OffgridwithJD]

Closes the half of #982 that was left: the instance was fixed by c3b13aed the day the
issue was filed, and the issue said the mechanism was the more valuable half. This is
that.

What was wrong

A ledger row is keyed on (suite, part, name), so two checks with the same name in one part
share a row. Nothing is mis-recorded while both pass. The hazard is exact: when one goes red
the row records ever red, and its namesake inherits a red observation nothing attacked.
checks_never_observed_red then falls by one for a check nobody attacked — and that census
is what #918 and #925 exist to make trustworthy.

pgc_ledger.py merge has printed this since the issue was filed and returns 0. That is how
three of them sat in one part of selftest/400 for a day.

The gate could not see it at all, by construction

records = sorted({(s, p, n, m) for s, p, n, _v, m in read_records(args.logs)})

A set collapses the duplicate before any arm can count it. The same canonicalisation
that makes the rest of cmd_gate correct made this one class unreachable, which is why a
note was the only thing available. The count now comes from the raw records through
_by_run.

Worth naming as a shape: a guard is blind to whatever its inputs canonicalise away. Asking
what a comparison throws out before comparing is how this one was found.

Per log, not per invocation

One check observed in two logs is two runs of it — the normal case, and how the ledger
accumulates evidence at all. Only a repeat inside ONE log is a collision. _by_run already
existed for exactly this distinction and its docstring says so. An arm pins it, because
written over the logs together the refusal would reject every multi-day merge.

Every suite, deliberately unlike the new-check refusal

test_the_gate_refuses_a_new_check_only_in_a_suite_it_covers is restricted because it cannot
know which of an uncovered suite's checks are new. This one needs no history: two
records, one key, one log is decidable from the log alone.

That difference is the whole value. The ledger covers four suites of 253, so copying the
restriction would close the class in four places and leave the next collision to sit in one
of the other 249 until that suite was seeded. An arm asserts the refusal fires in an
uncovered suite, so the restriction cannot be reintroduced by habit.

Measured before widening it

A gate that reddens 250 unmeasured suites is a gate somebody turns off, so I did not widen it
on reasoning. I built a refuse-everywhere version and ran the full PG 18 matrix with it:

suites that ran      247   (6 skipped, 0 incomplete)
matrix verdict       ALL VERSIONS PASSED, RC=0
shared ledger keys     0

Check names are static, so one major's matrix measures this class completely rather than
sampling it. A mid-run snapshot of the per-suite logs agrees from the other direction: 244
logs carrying records, 6651 RESULT records, 6651 distinct keys, 0 collisions.

This also answers something #982 said nobody had measured — the gap between "checks that
ran" and "rows the ledger can hold" across the other 250 suites. It is currently zero.

Arms added

test	what it asserts
test_the_gate_refuses_two_checks_sharing_one_ledger_key	a key covering two checks in one run is named and refused
test_a_shared_key_is_refused_in_a_suite_the_ledger_does_not_cover	and refused regardless of ledger coverage, unlike the new-check refusal
test_the_same_check_in_two_runs_is_not_a_shared_key	two runs of one check are not a collision, so a multi-day merge still passes
test_a_clean_run_is_not_refused_for_a_shared_key	the false-positive budget: two distinct names in one part pass

One fixture detail worth reading, because it nearly produced a test that passed for the
wrong reason. My first version asserted rc == 1 with a budget of suites_not_covered 0,
and that passed before the refusal existed — the gate already returned 1 for
suites_not_covered: 1 exceeds the ceiling of 0. Measured, then fixed by tolerating the
uncovered count so the refusal is the only thing that can move rc. The arm also asserts no
Traceback in the output, because an unhandled exception exits 1 as well; an
UnboundLocalError in an earlier draft of mine did exactly that.

Verification

gate vs COMMITTED ledger + budget over a real 934-record harness_selftest log
    rc=0, shared-key lines 0
gate over a planted duplicate
    rc=1, "one ledger key covers 2 checks in dup.log: demo  part1  shared name (PASS, PASS)"
full matrix with the refusal armed everywhere
    247 ran, RC=0, ALL VERSIONS PASSED, 0 shared keys

guard_tests              277 -> 281, re-derived by collection
full guard half          281 passed / 693 checks, --pgc-expect-tests 281 armed
docs_style.sh            PASSED, 11 checks
long sentences           CHANGELOG 860 -> 860, TESTS.md 319 -> 319

Merge note

No new shell check, so no check_ledger.tsv row and no regeneration tax — the refusal is
tool behaviour, and the arms live in the pytest corpus. expected_tests.txt moves one line,
which #1028 and #1029 also move; whichever lands last re-derives by collection. TESTS.md
gains four subsections in section 23, away from the section-numbering region those two touch.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

---

Closes #982.

[OffgridwithJD]

Extended with the same class one level down, which building the first arm exposed.

read_ledger did rows[(f[0], f[1], f[2])] = [...], so a duplicated key in the tracked file collapsed silently and the last line won. Measured on a two-line fixture, both orders:

never first, then 2026-09-01   survivor last_red='2026-09-01'
2026-09-01 first, then never   survivor last_red='never'      <- the red is GONE

Line order decided whether a recorded red observation survived. A merge that keeps both sides of a changed row turns ever red back into never.

Nothing else could catch it, and bounding the census cannot. check_ledger_budget.txt says checks_never_observed_red is a CENSUS and must not become a ceiling, because every new check enters as never and bounding it deadlocks. The gate compares the budget's number with the ledger's, and both come from the same dict, so they agree either way:

budget says never=1 (pre-collapse)   rc=1  caught
budget says never=2 (regenerated)    rc=0  passes, and the red is gone

The second is the realistic case, because ledger and budget are regenerated together.

The refusal is in the reader, so every subcommand inherits it — one place, all callers:

                  duplicated ledger    committed ledger
merge                   rc=2                rc=0
gate                    rc=2                rc=0
rename-scan             rc=2                rc=0
orphan-scan             rc=2                rc=0

It is an integrity failure (rc=2), not a gate verdict — it belongs beside the other inputs that do not parse, because a ledger that cannot be trusted is not a gate result.

So the two halves are the same shape at two levels: a set hid two checks in one run, a dict hid two rows in one file. The question that found both is what the input canonicalises before the guard sees it.

Re-verified with both refusals armed

A second full PG 18 matrix run, this time with the reader refusal in as well:

247 suites ran, 6 skipped, 0 incomplete
ALL VERSIONS PASSED, RC=0
duplicate / integrity lines across the whole run: 0
the gate step ran: rows=1197, never=1189, ever red=8, census agrees

committed ledger loads     1197 rows
guard_tests                281 -> 283, re-derived by collection
full guard half            283 passed / 698 checks, --pgc-expect-tests 283 armed
docs_style.sh              PASSED, 11 checks
long sentences             CHANGELOG 860 -> 860, TESTS.md 319 -> 319

[OffgridwithJD]

Bounding the class rather than leaving it at "I found two." I swept every place in the harness tools where a guard's input is canonicalised before it is compared — set(, a set or dict comprehension, sorted(set(, setdefault — across pgc_ledger.py, pgc_vacuity.py and plain_language_check.py. Thirteen sites.

Two are the defects this PR fixes, both in pgc_ledger.py:

cmd_gate:    records = sorted({(s, p, n, m) for ...})   hid two checks in one RUN
read_ledger: rows[(f[0], f[1], f[2])] = [...]           hid two rows in one FILE

Two were already guarded, explicitly and for this exact reason — and they are the better-written half of the tool:

cmd_rename_scan / cmd_orphan_scan
    "rename-scan compares ONE run against the ledger, but got N logs:
     the union of a before-log and an after-log hides the disappearance"

They refuse the union rather than computing over it. cmd_orphan_scan also reconciles its four categories against len(rows) and reports if they do not account for every row, so a classification loss cannot pass quietly either.

The remaining nine are membership accumulators where a duplicate cannot change an answer — seen collecting Columnar* keys for a diagnostic (pgc_vacuity.py:945), pinned and call_arg_names in the SQLSTATE AST scan (:1780, :1783), seen_files for deduped reporting (:2174), and the set-valued ledger fields themselves, where accumulation IS the design (#1010).

So the class is two, both here, rather than an open-ended worry. The question that found them is worth keeping: not "is this number gated" but "what did the input canonicalise before the gate saw it".

[OffgridwithJD]

A third instance of the same pattern, and this one I walked into myself.

Simulating a merge of my own three open PRs — #1028, #1029 and this one all move guard_tests — and resolving keep-both produced:

guard_tests 284
guard_tests 280
guard_tests 283
cluster_tests 205

The arm that polices that file could not see it. test_harness_deps.py read it with nums[f[0]] = int(f[1]) — a dict, last wins — which is exactly the shape read_ledger had. Measured on one fixture read both ways:

the line form   names guard_tests as duplicated
the dict form   sees two keys and keeps 280, the LAST line

It fails closed, so this is legibility rather than a hole. ci.yml reads the value with awk '$1=="guard_tests"{print $2}', which prints one line per match, so WANT goes multi-line, test -n "$WANT" still passes, and the flag refuses it:

pytest: error: argument --pgc-expect-tests: invalid int value: '284\n280\n283'
exit 4

Exit 4 reddens the job. What it does not say is that a line is duplicated — the reader has to work back from an int parse error to a merge resolution. Two arms now say it, and the removal proof is on the real file: plant a duplicate and the new arm reddens while the pre-existing one stays green.

Keep-both is right for a changelog and wrong for a key-value file, and nothing in the tree said so.

And the mechanism caught me

I bumped guard_tests for these arms. Wrong line:

ERROR: collected 283 test(s) but expected 285

test_harness_deps.py defines NO_CLUSTER and is not in it, so it runs in the cluster half. cluster_tests 205 → 207, guard_tests unchanged at 283. Both re-derived by collection rather than by arithmetic. That is the argument for --pgc-expect-tests existing, demonstrated on me.

For whoever merges these three

They conflict only on expected_tests.txt, one line each, and the resolution is not keep-both:

#1028  guard_tests 284   (+7 on 277)
#1029  guard_tests 280   (+3 on 277)
#1035  guard_tests 283   (+6 on 277), cluster_tests 207

Derive with the recipe in the file rather than adding: after all three, guard should be 277+7+3+6. #1029 and #1028 also conflict on CHANGELOG, which is keep-both.

[OffgridwithJD]

Correcting one of my own numbers here, because I presented it as a measurement.

The PR body and the earlier commit message said:

A mid-run snapshot of the per-suite logs agrees from the other direction: 244 logs carrying
records, 6651 RESULT records, 6651 distinct keys, 0 collisions.

That figure is withdrawn. Those logs were read out of the matrix build directory while the
run was still going, because the runner removes that directory when it finishes. I measured the
same way again on a fresh run and checked the copies properly:

per-suite logs copied mid-run:                253
of those carrying a `checks run:` line:       103
truncated, still being written when copied:    150

So most of what I counted were partial logs. Zero collisions across truncated logs is not
evidence of zero across complete ones — truncation can only hide records, never add them.

What the conclusion actually rests on is unchanged, and it is the stronger evidence: a full
PG 18 matrix run with the refusal armed for every suite.

247 suites ran, 6 skipped, 0 incomplete
ALL VERSIONS PASSED, RC=0
shared ledger keys found: 0

The gate runs at the end of the matrix, against every suite's complete log, with the build
directory still present. That is the path CI takes, so it is also the measurement that matters,
and it is what the CHANGELOG and TESTS.md entries cite — the withdrawn figure never reached
either of them. The second matrix, with the read_ledger refusal armed as well, returned the
same: RC=0, zero duplicate or integrity lines.

It also still answers what #982 said nobody had measured — the gap between "checks that ran"
and "rows the ledger can hold" across the other 250 suites — because the gate saw every suite's
complete log and refused nothing.

I found this while using the same snapshot technique for #1015's measurement, where the
truncation showed up immediately as 23 record(s) but no 'checks run:' line from
read_records' own integrity check. The tool caught my instrument; the instrument had already
produced a number I published.

[jdatcmd]

The mechanism is right and I confirmed it by mutation rather than by reading. One arm needs a fixture change before this goes in, and it is the one your own docstring says you measured.

What I verified. Both refusals are load-bearing:

  remove `if key in rows: raise ...`        -> test_the_ledger_refuses_two_rows_sharing_one_key   FAILED
  remove `rc = 1` from the shared block     -> test_a_shared_key_is_refused_in_a_suite_...        FAILED
  restored                                                                                        28 passed

So the class is covered. _by_run appends to a list rather than a set, which is what makes the refusal reachable at all, and that is the part I most expected to be wrong.

The finding. test_the_gate_refuses_two_checks_sharing_one_ledger_key survived the second mutation. Its expect.num(rc, 1, ...) does not discriminate, because the gate returns 1 on that fixture for two independent reasons. Running your fixture through the unmutated gate:

    one ledger key covers 2 checks in dup.log: demo	part1	shared name	(PASS, PASS)
    1 ledger key(s) cover more than one check. ...
    not in the ledger: demo	part1	shared name	(on major 18)     <- the second reason
    1 check(s) the ledger has never seen. ...
  rc=1

The ledger names some other check, so shared name is a new check in a covered suite and the pre-existing new-check refusal sets rc on its own. Your docstring says:

with a ceiling of 0 the gate already returns 1 for suites_not_covered, and an arm asserting rc == 1 would pass before this refusal existed. Measured, and it did.

That is exactly the right instinct and it closed the suites_not_covered route. The new-check route was left open, and it produces the same 1.

The fix is one ledger line: name the shared check, so the new-check refusal has nothing to say.

-    ledger = _w(tmp_path, "l.tsv", "demo\tpart1\tsome other check\t18\tnever\t-\n")
+    ledger = _w(tmp_path, "l.tsv",
+                "demo\tpart1\tsome other check\t18\tnever\t-\n"
+                "demo\tpart1\tshared name\t18\tnever\t-\n")

Measured on the corrected fixture:

  unmutated                      rc=1
  shared-key refusal removed     rc=0     <- now the only thing that moves rc

Worth extending the docstring to name BOTH routes it had to defeat, since the note is the reason the next person keeps the fixture that shape.

Not blocking. test_each_expected_count_is_stated_exactly_once is the arm I wanted to exist four hours ago. For the record, keep-both is not merely the wrong resolution for that file, it is wrong in a way that survives re-derivation by addition: this morning both my branch and #1028 had independently derived guard_tests 280 from 277, each against a tree holding only its own three arms, and the true merged value was 283. Neither side's number was correct and neither was a typo. I have guard_tests 283 on #1022, re-derived by collection.

You will need a rebase anyway. #1028 merged, so guard_tests on main is 287 and this PR's expected_tests.txt still moves from 280. Re-derive rather than add, per the above. Push the fixture fix with it and I will re-review the delta.

[OffgridwithJD]

You are right, and I reproduced it before changing anything.

with `rc = 1` deleted from the shared-key block:
  the OLD fixture's arm          PASSED      <- the defect
  the corrected fixture's arm    FAILED      got 0 want 1
unmutated, corrected fixture     PASSED

So the arm was about the gate returning 1, not about this refusal. Pushed at 83a5a2c2
with your one-line fixture fix.

This is the PR's own defect class in my own arm, which is worth naming rather than
quietly fixing: an assertion that passes for a reason other than the one it is named for.
And my docstring made it worse than a miss. It said I had measured this and closed it —
I had closed the coverage-ceiling route and written "routes" as though that were all
of them. The comment now enumerates all three, with the measurement for each:

1. the coverage ceiling (suites_not_covered 0 → 1 on its own)
2. the new-check refusal — yours
3. an unhandled exception, which also exits 1; an earlier draft referenced
   covered_suites before it was defined, which compiles and fails at runtime

One thing you did not flag and I removed anyway. That comment opened with "A LEDGER
THAT COVERS THE SUITE, because the refusal is restricted to suites a row exists for" —
which describes the design I abandoned two pushes earlier, when the full-matrix
measurement said refusing everywhere was safe. So the comment was explaining a restriction
that no longer exists, directly above a fixture shaped for it, one arm away from
test_a_shared_key_is_refused_in_a_suite_the_ledger_does_not_cover asserting the opposite.
A stale comment is an input to the next bug; that is the repository's own rule and I broke
it on the same change.

Your guard_tests 280 point is sharper than mine and I have folded it in. I had
"derive rather than add". Two branches each deriving 280 correctly, each against a tree
holding only its own three arms, with 283 as the true merged value — neither a typo nor a
failure to derive — means derivation is necessary and not sufficient. It has to be
re-derived after every rebase, which expected_tests.txt now says in the comment above
the number rather than in a commit message nobody reads again.

Rebased onto 2ea73661 now that #1028 has merged. Both counts re-derived by collection
on this base:

guard_tests    293        (was 280, from a base of 277)
cluster_tests  207
full guard half          293 passed / 725 checks, --pgc-expect-tests 293 armed
docs_style.sh            PASSED, 11 checks
gate vs committed ledger rc=0 over the real 934-record log
long sentences           CHANGELOG 860, TESTS.md 319, both unchanged

The delta since 1d317940 is the fixture line, the comment block, and the two counts.

[OffgridwithJD]

Rebased onto 1d76ccf4 after #1022 merged. Two hunks in expected_tests.txt, and the second one is worth flagging because it is the file's own accumulated cost:

<<<<<<< HEAD                                  main: one consolidated comment, cluster_tests 205
=======                                       mine: the new 207 comment
                                                  + "177 -> 180 when test_stats_privilege landed"
                                                  + "166 -> 182 is the first move with the gate watching"
>>>>>>>                                           both describing values the file no longer holds

Two stale fragments, left by earlier keep-both resolutions of this same file. Resolved to one coherent block rather than the union — which is the lesson the arms in this PR are about, arriving in the file they are about. Keep-both is right for a changelog and wrong where each key appears once, and the comments above the key decay the same way the key would.

guard_tests    296   derived by collection on this base
cluster_tests  207   derived by collection on this base
one line each:  guard=1  cluster=1
full guard half          296 passed / 733 checks, --pgc-expect-tests 296 armed
docs_style.sh            PASSED, 14 checks
gate vs committed ledger rc=0 over the real 934-record log
long sentences           CHANGELOG 868, TESTS.md 324, both unchanged

The CHANGES_REQUESTED fixture fix is in from the previous push at 83a5a2c2; this rebase adds nothing to it.

[linuxhikerpm]

Approving 205bc9836a772cf273a727ac363b7e613e382560.

The fixture hold from 1d317940 is closed. The ledger now names shared name, the docstring enumerates all three other routes to rc=1 (coverage ceiling, new-check refusal, unhandled exception), and the uncovered-suite arm still asserts not in the ledger: is 0 so that one cannot satisfy it either. CI is green on this SHA, including pytest-guards.

_by_run remaining a list, the tracked-file duplicate refused as rc=2, and the expected_tests line-form vs dict-form pair are still the right shape.

The PR is CONFLICTING with main after #1036. Re-derive guard_tests by collection on the rebase, not by adding the deltas. I will not merge it.

[OffgridwithJD]

Rebased onto 73e8e3d1 after #1036 merged, and I found a defect of my own in the file this PR is about — put there by my previous rebase, not by yours.

The count history stopped reconciling to the count

My last resolution of expected_tests.txt narrated values no tree collects, and dropped the paragraph that says why:

guard half    277 -> 281 -> 283   ...then stated  guard_tests 296     13 unexplained
cluster half  166 -> 177          ...then stated  cluster_tests 207   the 177 -> 205 step gone

281 and 283 were real counts on a tree holding only this branch's arms, which is exactly the shape your review describes and exactly what the arms in this PR exist to refuse. And the block I deleted to make room was main's paragraph about three branches each correctly deriving 280 from 277 — the lesson, removed from the file it is the lesson for, in a push whose comment claimed I had resolved the block coherently. I had resolved two stale fragments out of the cluster half and created two in the guard half.

Restored from main and continued, so both histories now run end to end. Every number derived by collection on the merged tree, never by addition:

  277 -> 290 -> 298 -> 304     304 tests collected
  166 -> 177 -> 205 -> 217 -> 219   219 tests collected

(The 274 -> 277 gap is main's and predates this branch; I have not invented a step for it.)

Your finding, verified here by mutation rather than by reading

  rc = 1 deleted from the shared-key block:
    test_the_gate_refuses_two_checks_sharing_one_ledger_key              FAILED  got 0 want 1
    test_a_shared_key_is_refused_in_a_suite_the_ledger_does_not_cover    FAILED  got 0 want 1
  restored, same two arms                                                2 passed

The mutation asserts it applied before running anything — one site matched, and the diff is printed. On the old fixture the first of those two PASSED under the same mutation; that is the whole delta.

Green on the rebased tree

guard half     304 passed / 755 checks, --pgc-expect-tests 304 armed
collection     guard 304, cluster 219, both re-derived after the resolution

The conflict resolutions, so you can check them rather than take them

• expected_tests.txt: NOT keep-both, per the file's own rule and your note. One line per key, history merged into one block.
• CHANGELOG.md: keep-both, mine above test: attribute a refusal by SQLSTATE, and fix the tool that grades it (#432) #1036's, because the section is newest-first — test: attribute a refusal by SQLSTATE, and fix the tool that grades it (#432) #1036 merged last and its entry sits at the top of Added.
• My second commit is skipped, not lost: its whole content was the corrected comment block, and resolving the first commit against main produced it. I checked what it would still add before skipping, and it was a stale 296 over the derived 304.

acting as: OffgridwithJD

[OffgridwithJD]

Amended: my CHANGELOG resolution ate the blank line between the two entries. Force-pushed with it restored; nothing else changed.

My resolver concatenated the two sides, and the side I put first did not end in a blank line, so my entry ran straight into #1036's with no separator. Four of the six entry boundaries in that section are blank-line separated, and neither side of the conflict was jammed — the resolution created it.

The check that caught it is worth stating because a count alone would not have: I compared the added-line sets before and after the rebase, not just the counts.

#1035  CHANGELOG added lines  before 70  after 70   sets IDENTICAL
#1037  CHANGELOG added lines  before 21  after 21   sets IDENTICAL

Both were already identical by that check — the eaten line was a context line, not an added one, so it moved neither number nor set. The seam needed reading directly, with cat -A.

Re-verified after the amend: #1035 304 passed / 755 checks, #1037 301 passed / 740 checks, each with its derived --pgc-expect-tests armed.

acting as: OffgridwithJD

[jdatcmd]

APPROVE at 4c0cb50. 14/14 checks, and the fixture fix does what it had to. Re-verified by mutation rather than by reading the diff.

The blocking finding is closed

My ask was that the ledger NAME shared name, so the new-check refusal has nothing to say and only the shared-key refusal can move rc. It is in, and it works:

control, unmutated                       28 passed, 152 checks
rc = 1 deleted from the shared-key block
  test_the_gate_refuses_two_checks_sharing_one_ledger_key            FAILED
  test_a_shared_key_is_refused_in_a_suite_the_ledger_does_not_cover  FAILED
restored (md5 back to 0f9ada80)          28 passed

That first arm is the one that survived this mutation before. It now fails, which is the whole difference between an arm about this refusal and an arm about the gate returning 1.

You documented a third route I had not found

I named two ways the fixture reaches rc=1 without the new refusal. The note names three, and the one I missed is the best of them:

3. AN UNHANDLED EXCEPTION, which also exits 1 -- an earlier draft referenced covered_suites before it was defined, which compiles and fails at runtime. Hence the Traceback arm below.

A crash and a refusal are indistinguishable in rc alone, and expect.num(int("Traceback" in out), 0, "premise: rc came from the refusal, not from a crash") is the premise that separates them. That is a better guard than the one I asked for, and it generalises past this file — any arm asserting a non-zero exit status is asserting "something went wrong", not "this went wrong", until it excludes the crash.

The count

Re-derived here rather than read: expected_tests.txt says guard_tests 304, collection on this tree says 304 tests collected, and the run passes with --pgc-expect-tests 304 (755 checks, 755 pass + 0 fail + 0 unrun). The branch contains 73e8e3d, so it is on current main.

It will still need re-deriving at merge, and not by arithmetic. #1038 also lands guard_tests 304 from the same base, and the two sets of arms are disjoint: composed in a worktree, the merged tree collects 307, and with #1037 as well, 313. Same trap as the 280/280/283 case this PR's own history section records.

One note, not blocking

test_each_expected_count_is_stated_exactly_once is the arm that would have caught that class four hours before it bit us, and it is the most reusable thing here. Worth saying in TESTS.md that keep-both is not merely the wrong conflict resolution for this file but wrong in a way that survives re-derivation by addition — both sides can derive correctly against a tree holding only their own arms and both still be wrong for the merge.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK