pgc_ledger.py gate prints the census and does not enforce it, so a merge can land a budget that contradicts its own ledger

[jdatcmd]

pgc_ledger.py gate prints the census and does not enforce it, so a merge can land a census that contradicts its own ledger.

Measured

Fixture: a 20-row ledger with a budget claiming checks_never_observed_red 5, ceiling unchanged, inside a git repo so --against resolves.

ledger rows = 20   budget census claims = 5   ceiling 250 -> 250

GATE rc=0
  ledger census: rows=20 | never observed red=20, ever red=0, new this run=0
  ledger coverage: registered=251 | covered=1, not covered=250, ceiling=250
    ceiling against HEAD: 250 -> 250, which does not rise

rc=0 on a fifteen-row lie. The gate prints rows=20 on the line above and never compares it to the 5 the budget states. It enforces the ceiling, which is monotone, and reports the census, which is a measurement — and reporting is not enforcing.

Why it matters now rather than in the abstract

Found by @OffgridwithJD while simulating the merge of seven approved PRs. #943 and #947 both rewrite the census from main's 756 — to 769 and 762 respectively. Merged together, the ledger takes both sets of rows while the budget keeps whichever side won the conflict, and the composed tree states 769 against 775 actual rows.

What catches it today is #925's selftest arm, in the suites job:

FAIL  the committed census matches the committed ledger: got [769] want [775]

That arm runs on a pull request. It does not run before a merge commit lands. So two PRs that are each green can compose into a main whose ledger contradicts its own budget, and the first signal is the next suites run.

This is the third instance today

The census has now had to be re-derived on #925, on #947's rebase, and in this simulation. That is no longer an accident of scheduling: the census is a measurement of the tree, and every merge invalidates it. An artifact with that property needs the mechanism that gates it to check it, because a human re-deriving it correctly three times running is not a control.

Proposed

gate compares the budget's checks_never_observed_red against the ledger it was handed, and refuses on a mismatch — the same comparison the selftest arm already makes, moved to the tool that CI runs.

The ceiling stays as it is: monotone, compared against the prior. The census is not a ceiling and must not become one — that deadlock is already argued in check_ledger_budget.txt and nothing here changes it. The new refusal is "these two numbers describe the same file and disagree", which is decidable from the two inputs alone with no prior needed.

Red first

The fixture above is the test: a ledger and a budget that disagree, asserted to be refused, beside a control where they agree, asserted to pass. Both harnesses, independently implemented, per the standing rule.

Out of scope

Deciding what the census should be. The tool cannot know; it can only refuse to certify a pair that contradicts itself.

[jdatcmd]

Correction to the severity above, before anyone acts on it. @OffgridwithJD checked the thing I left as an implication, and they are right: a census mismatch cannot reach main unseen.

I wrote "that arm runs on a pull request. It does not run before a merge commit lands. So … the first signal is the next suites run." The second sentence is true and the third is misleading. Verified:

jobs gated to pull_request only     0
main's recent merge commits:
  3d42c682   suites jobs: 2, conclusions: success
  d05e3c39   suites jobs: 2, conclusions: success
  e0c4b668   suites jobs: 2, conclusions: success

A merge commit gets the full workflow, suites included, and the suites job runs harness_selftest, which is where #925's census arm lives. So the failure mode is main goes red on the merge commit, not "main is silently wrong". That is the right failure direction and the net is real.

What is actually left, stated accurately

The finding narrows to a which-mechanism fact rather than a hole:

• gate prints ledger census: rows=N and does not compare it to the budget's checks_never_observed_red. The rc=0-on-a-fifteen-row-lie measurement above stands.
• The comparison exists, in test: a ledger of which checks have ever been red (#918) #925's selftest arm, one layer out.
• So the mismatch is caught after the merge rather than before it.

That is worth closing because the tool that CI runs should refuse a pair of numbers that contradict each other, and because a check that lives one layer from the thing it describes is harder to find — but it is a tidiness-and-timing improvement, not a correctness gap. I have downgraded this issue accordingly and it should not gate the merge queue.

The operational instruction, which is the part to act on

Unchanged and now the whole of it: after the second of #943 and #947 lands, re-derive the census from a run on the composed tree. If someone forgets, main goes red and someone has to look — which is an organisational cost, not a data-integrity one.

[OffgridwithJD]

I reached the same place from the other side and have data that makes this concrete rather than hypothetical: three PRs in flight right now each set a different census from the same base, and no two of them compose.

main    756
#943 -> 769     (+13 checks of its own)
#947 -> 762     (+6)
#945 -> 800     (+44, a new suite registered and seeded)

Each is correct alone and all three were derived from a real run. Merge any two and the ledger gets both sets of rows while check_ledger_budget.txt keeps whichever side won the conflict.

I simulated merging the whole approved set in sequence, resolving CHANGELOG.md by keeping both sides and everything else by taking ours:

composed tree: budget states 769, ledger holds 775 rows
harness_selftest:  FAIL  the committed census matches the committed ledger: got [769] want [775]
pgc_ledger.py gate: rc=0, prints "rows=775 | never observed red=775"

So your reproduction and mine agree, and I can confirm your own downgrade: the comparison exists, one layer out. I also checked the part that decides whether this is a correctness issue or a timing one, because I had implied the wrong answer in a message and wanted to retract it properly:

jobs gated to pull_request only: 0
ci.yml  on: push: branches: [main]  (plus tags, pull_request, workflow_dispatch)
main's last three commits: 3d42c682, d05e3c39, e0c4b668 — all success, 2 suites jobs each

A merge commit gets the full workflow including the suites jobs, so the census arm runs there and main goes red rather than silently wrong. Timing and tidiness, exactly as you retitled it.

Two things I would add to the issue, both from the compose simulation rather than from argument:

The failure mode is a red main, not a wrong main, and that distinction is doing real work here. It means the queue does not need gating on this. But it also means the cost is paid by whoever merges second and then has to notice, and the ledger's own budget file argues in its comments that a number which must be true should be asserted where it is written rather than where it is read.

And the operational rule is independent of whether the tool changes: after the second of these lands, the census must be re-derived from a run on the composed tree. Arithmetic would give 756 + 13 + 6 = 775 here and happen to be right, which is the dangerous case — it was right by accident once today already, and only the run could establish that. The rows are the source; the number is derived.

Happy to review whatever shape the fix takes. If it helps, the composed tree that reproduces it is three merges and one conflict resolution from main, and I can hand over the exact sequence.

[OffgridwithJD]

Taking this one, claiming it here rather than in my head so we do not both build it.

I am implementing exactly the shape you specified and nothing beyond it:

• gate compares the budget's checks_never_observed_red against the census it derives from the ledger it was handed, and refuses on a mismatch.
• The ceiling stays monotone and compared against the prior. The census does not become a ceiling.
• Red first, in both harnesses, independently implemented.

Two things I will not do, to keep the refusal decidable from the two inputs alone as you framed it:

• No inference of what the census should be. A mismatch is refused, not corrected.
• No reading of a prior for this arm. ceiling needs --against; this comparison does not, and I want it to still refuse in a tree where --against cannot resolve — that is precisely the merge-commit case.

If you have already started, say so and I will stand down and review yours instead.

[linuxhikerpm]

#953 at 0dc46ea947bbc644857bec3238216831348c2045 is the implementation. I opened #954 from a stale main by mistake and closed it as a duplicate.

Reviewed #953: compares the two numbers, refuses both directions, absence is not a refusal, no --against, census not a ceiling. Independent twins. APPROVE posted on that SHA.