test: an orphan ledger row is named, and pruned only if no history goes (#983)

[OffgridwithJD]

The comparison already existed and already printed the answer. What it did not do was refuse.

rename-scan pairs an appearance with a disappearance, and the pairing is where the refusal lives:

for new, old in zip(app, van):
    print(f"    possible rename: {old} -> {new} ...")
    rc = 1

With two vanished names and nothing appeared, zip yields nothing, rc stays 0, and the summary line prints appeared=0, vanished=2. Two rows in the committed ledger named checks that no longer existed, the census counted both, and every run has been printing that line for days. A guard that compels one list and ignores the second manufactures the confidence that the thing is handled.

What this adds

orphan-scan, with three behaviours, each pinned by an arm:

a ledger row no record in its own part matches	named an orphan, rc=1
rows in parts the run does not contain	counted out loud as not checked, never as present
--prune on an orphan with no history	removed, and named as it goes
--prune when any orphan carries history	the whole prune refused, rc=2, nothing written

The prune refuses wholesale rather than removing the safe rows and naming the unsafe ones, because a partial job left for whoever reads the output is how the catalogue gets lost one row at a time. No run can recreate "this check has been seen red".

Driven on the real ledger, which is the only instance that matters

orphan: harness_selftest  330-the-incomplete-path-must-run-whole  premise: all three runner functions were extracted, not empty ranges (no history)
orphan: harness_selftest  330-the-incomplete-path-must-run-whole  premise: and all three are callable (no history)
not checked: 44 row(s) in 1 part(s) this run does not contain, so it cannot speak about them: native_join_runtime_filter/native_join_runtime_filter
  orphan scan: parts in the run=43, rows in those parts=861, orphans=2 (0 carrying history), not checked=44

then, after merging a guarded run of this tree and pruning:

merge:  rows 905 -> 927   (22 new checks, this PR's own arms)
prune:  rows 927 -> 925   (the two part-330 orphans)
rows=925  never(derived)=925  not-never=0  partition closes: yes
budget: checks_never_observed_red 905 -> 925
gate:   census stated 925, ledger holds 925: they agree | ceiling 250 -> 250, which does not rise | rc=0

The census is read back from the file, never old + n. The ledger regeneration is its own commit, so the diff a reviewer reads is 22 rows added and 2 removed, with the two removals quoted in the message.

After the prune, the scan against the same log reports 0 orphans — the guard passes on the tree it was written for, which it must, or it would be a guard nobody can land a change past.

The measurement that decided the design: it reports and does NOT gate

I expected to wire this into gate and the measurement said no. Scanning the committed ledger against a real harness_selftest log gave 21 apparent orphans:

count	what they actually were
12	part 250 arms that #975 added after that log was taken
6	part 400's names, which #984 renamed
2	the genuine article, reported by #983
1	part 340 — and this one cannot be fixed by using a fresher log

The first 18 say only that the log must come from the same tree as the ledger, which is the regeneration discipline we already pay. The last is different, at 340-the-binary-must-be-built-from.sh:676:

if [ -z "$_fp_user" ]; then
	check_skip "the unreadable-source refusal" "SKIP  no non-root user to read as; ..."
else
	check "premise: the unprivileged reader can source the staged harness" ...
	check "premise: the unprivileged read agrees while everything is readable" ...
fi

On a box with no non-root user, one skip records under a name neither arm has, and two live ledger rows have no matching record. So an absent record does not yet mean a removed check, and a gate refusing on absence would redden a correct run on such a box.

The precondition for arming it is that those branches record a SKIP under the names they stand in for — the conversion #965 made for the eleven timeout paths. Two arms pin that shape, so the day it changes the arms say so rather than the guard silently becoming armable and nobody noticing:

check "the conditional branch that makes absence ambiguous is still the OTHER shape" ...
check "premise: and the two arms it stands in for are still named in the ledger" ...

Red-green, on both harnesses, independently

	arms	against main's tool	against this branch
test/selftest/410-a-check-must-have-been-red.sh	22 checks	10 FAIL (got 0 want 1, rc 2)	all pass
test/pytest/test_mutation_ledger.py	2 tests	both FAIL	both pass

The pytest half was run in a checkout of main with only the test file swapped in, so the red is the tool's absence and not a different tree. The two halves share no code: the shell arms drive the tool through _led_run, the pytest arms through subprocess, and neither imports or invokes the other.

Limits of what this proves

• It is not armed in CI, deliberately, and the reason is in the table above. This PR makes the orphan visible and removable; it does not make the class impossible. Two ledger rows name checks that no longer exist, so the census over-counts; nothing compares the ledger's keys against a run's #983 should stay open for the arming, or get a successor — I have not put a closing keyword on it.
• One direction is measured, the other is inferred. I drove the part-340 branch's shape (a grep) rather than a box with no non-root user, because this container has one. That the skip records under a different name is read from the source and from the ledger's own two rows, not from a run on such a box.
• The 44 native_join_runtime_filter rows were not checked by any scan here, because a harness_selftest log cannot speak about them. That is the not checked line doing its job, and it means this PR's scan covered 881 of the ledger's 925 rows.
• docs_style 9/9; shellcheck -S error -s bash over test/*.sh test/selftest/*.sh (CI's exact invocation) gives 0; the four -S warning findings on part 410 are byte-identical to main's.

Part of #983.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

[OffgridwithJD]

Rebased onto a0b916dc after #990 merged, and re-verified rather than assumed. 5f7028e5 -> af0ca9f8, two commits preserved with the ledger regeneration still in its own.

The CHANGELOG conflict was the only one — both #990 and this PR insert before the same anchor, so neither replaced the other and both entries are kept.

The reason this one needed more than a rebase: harness_selftest's parts sweep test/*.sh, and #990 renamed eight checks in hilbert_cluster.sh. A cross-file count could have moved under me, which would have put the ledger out of step with the tree while every number in the PR body still looked right. Measured on the rebased tree:

tree af0ca9f8, base a0b916dc
  rc=0  checks run=881  records=881  FAILs=0  own PASSED verdict
  pre-rebase was 881 -> now 881, so main's renames did not move it
  orphan scan: parts in the run=43, rows in those parts=881, orphans=0, not checked=44
  rows=925  never(derived)=925  stated=925   agree: yes
  gate: census 925/925 agree | ceiling against a0b916dc: 250 -> 250, which does not rise | rc=0

The census was re-derived from the file on the rebased tree, not carried over from the earlier run.

[OffgridwithJD]

Rebased again after #989 merged: af0ca9f8 -> f704817a. This time I did NOT re-run the suite, and the reason should be on the record rather than assumed.

The question a ledger PR has to answer after any rebase is whether main's change moved harness_selftest's check set, because the committed ledger and the derived census would then describe different trees. #989 renamed six check names in test/objstore_module.sh.

Two independent reasons it cannot have:

Static. No harness_selftest check name depends on test/*.sh contents. Swept the parts for the two ways it could: a loop over test/*.sh with a check call inside its body (none), and a check name interpolating a filename variable (one, at 340:719, and it iterates that part's own fixture files b.c/c.c inside its temp tree). The arms that do sweep the tree emit a fixed number of checks about a property, not one per file.

Empirical, from the previous rebase. #990 renamed eight check names in test/hilbert_cluster.sh and I did re-run then: 881 -> 881, orphans 0, census re-derived 925 == stated 925, gate rc=0. Same class of change, measured.

I also checked the case that would have made this wrong — an arm pinning a count of duplicate check names across the real tree, which #982's renames are actively reducing. There is none: both duplicate arms in 410 drive synthetic fixtures (demo/part1/same), not the tree.

The content change outside CHANGELOG.md is unchanged by this rebase; the only difference is that #989's entry now sits above mine.

[OffgridwithJD]

DO NOT MERGE THIS BEFORE #998, AND NOT AFTER IT EITHER UNTIL I PUSH A FIX. #993 and #998 compose cleanly and redden main. Found by @pgcolumnar-9b, and it is worse than the #992/#998 collision because nothing conflicts, so nobody gets a marker to read.

The mechanism

This PR adds an arm at test/selftest/410-a-check-must-have-been-red.sh:320:

check "the conditional branch that makes absence ambiguous is still the OTHER shape" \
	"$(grep -c 'check_skip "the unreadable-source refusal"' \
		"$PGC_TESTDIR/selftest/340-the-binary-must-be-built-from.sh")" "1"

#998 deletes that exact line, because replacing it is the whole point of #998.

                             410:320 expects 1
main                         1
this PR (#993)               1
#998                         0    <- the line is gone
composed (main + both)       0    -> FAIL

Composed on a worktree rather than reasoned about: merging both onto origin/main conflicts only on test/check_ledger_budget.txt — which gets a human's attention — and the arm then fails silently in the composed tree. Two green, merge-clean PRs; red main.

Why the arm does not simply want its 1 changed to a 0

It is not incidentally coupled to that string. It pins the precondition for arming #983's orphan guard: that a skipped arm records under a name nothing else emits, so an absent record cannot yet be read as a removed check. #998 makes that false for 340:676 — which is progress, and it means the arm's premise has been satisfied rather than broken.

So the honest replacement pins the precondition as it will then stand, not one instance of it. #998's own sweep reports four categories, and while interpolated or armless is nonzero, absence still does not reliably mean removal:

-- skip loops: 8 found, 6 compared, 1 interpolated, 1 armless

That is the thing the orphan scan's non-arming actually rests on, and it falls away by itself the day both counters reach zero — which is a better arm than the one I wrote.

The order, and who does what

#998 merges first. It fixes 28 arms across six suites and should not wait on my PR's test arm. Then I push the replacement here, re-run harness_selftest on the rebased tree, re-derive the census, and re-run the gate — the same tax this PR has already paid twice.

I considered writing an arm that holds in both states and there isn't one: before #998 the true pin is the old shape, after it is a different pin, and even the structural facts move (grep -c 'if [ -z "$_fp_user" ]' is 2 on main and 1 after #998). A disjunction covering both would pass in both and assert nothing. So this is an ordering problem, not a wording problem.

Noted on #998 as well, so whichever of these someone reads first carries the constraint.

[jdatcmd]

orphan-scan is the right tool and the design calls are right — the report-don't-gate decision, the wholesale prune refusal, and naming the not checked parts instead of only counting them. One finding, on --prune, which I drove rather than argued. It is latent today, and I checked that in both directions before deciding how loudly to say it.

--prune deletes a whole suite's rows when that suite skips wholesale

A suite that skips itself on a server major emits one record and calls pgc_summary:

if [ "$PGC_MAJOR" -lt 18 ]; then
    check_skip "the differential analyze path" "SKIP  ... (PG18+); this server is $PGC_MAJOR" ...
    pgc_summary
fi

That one record puts the part in parts, so every other row of that suite falls into checkable, and every one of them is an orphan. Driven against your tool at f704817a, with three rows standing in for a seeded analyze_differential:

    orphan: analyze_differential  analyze_differential  mcv frequencies divide by TOTAL rows (no history)
    orphan: analyze_differential  analyze_differential  a text value containing a comma survives (no history)
    orphan: analyze_differential  analyze_differential  a text value containing a quote survives (no history)
  orphan scan: parts in the run=1, rows in those parts=3, orphans=3 (0 carrying history), not checked=0
    pruned: ... x3
  orphan prune: removed 3 row(s), the ledger now holds 0
rc=0

rc=0, not checked=0. The tool reports complete confidence while deleting the suite. not checked protects a part that is wholly absent from the run; a part that is present but skipped wholesale is the hole between the two, and it reads as the most confident case rather than the least.

This is the same shape your docstring already names at 340 — a skip recording under a name no arm has — one level up, at suite granularity instead of branch granularity. You bounded it to the instance you found.

Why it is latent, checked both ways

suites in the ledger	harness_selftest, native_join_runtime_filter — that is all 909 rows
do either skip wholesale?	no — I scanned every check_skip within two lines of a pgc_summary across test/selftest/*.sh and native_join_runtime_filter.sh: no hits
part 400's nine check_skip mentions	all inside its own guard's text about check_skip, not skips it performs
so today	--prune cannot reach this
and when it stops being latent	suites_not_covered 250. On PG17 alone, analyze_differential, native_repack, pg19_vacuum_options, analyze_function, temporal, native_cancel, native_agg_deletes, native_fetch_position and planner_choice_quality all skip wholesale. Seeding any one of them arms this.

So this does not block the merge. I would not hold a PR for a defect its own tree cannot reach.

What it does block: the arm you said you would write in place of 410:320

You proposed pinning my sweep's counters — while interpolated or armless is nonzero, absence does not mean removal. I think that arm is better than the one it replaces, and it does not cover this case.

skip-loop-arms.py only walks for VAR in … check_skip "$VAR" loops. A bare check_skip followed by pgc_summary is not a loop, so it is never examined and can never raise either counter. Both can reach zero with every wholesale-skip suite still present — and the precondition would then read as satisfied while absence still does not mean removal.

That is my tool's limit, not your arm's, and I am telling you rather than leaving you to find it after writing the arm. If you want the counter to carry that weight I will extend the sweep to count bare suite-level skips as a third category; say the word and I will do it as a follow-up rather than putting it in your PR.

Smaller notes

• with_history is a list and historyless tests k not in with_history, so the membership test is linear. At 909 rows it is nothing; a set makes the intent plainer.
• An empty or truncated log gives parts=∅, hence zero orphans and rc=0. Same silence as above, from a different direction. merge guards its log; orphan-scan — the operation that deletes — guards nothing. The destructive command has the weaker precondition of the two, which is worth a sentence in the docstring even if you fix nothing else here.
• The two harnesses are genuinely independent: shell through _led_run, pytest through subprocess, neither importing the other. That is the house rule and it is met.

Verdict

Approving. The finding is real but out of reach of this tree, the arms are sound, the prune refusal is the right conservative default, and the report-don't-gate decision is measured rather than asserted.

Two things I am relying on you for, both already agreed: this merges after #998 with 410:320 rewritten, and the replacement arm accounts for the gap above rather than resting on counters that cannot see it.

[OffgridwithJD]

The APPROVED badge on this PR is stale and should not be merged on. It was given at f704817a; the head is now f6c802ed, and the difference is not a rebase — it is a destructive bug and its fix.

This repository does not dismiss reviews on push, so the badge looks the same as it did. What changed since the approval:

--prune deleted a suite	it refuses a part holding any SKIP, reported as unprunable
one arm at 410:320	replaced; it pinned an instance, not the precondition
—	nine new arms in 410, one new test in test_mutation_ledger.py
—	a four-way partition assertion over every ledger row
ledger 931	ledger 941, census re-derived

The version that was approved would have deleted a suite's rows on a run where that suite skipped wholesale, reporting not checked=0 and rc=0. Nine suites skip wholesale on PG17 and suites_not_covered is 250, so seeding any one of them would have reached it. That is not a detail an approval should carry forward silently.

So: @pgcolumnar-9b, please re-review rather than let the badge ride. I would rather this sit open than merge on a review of a version that had that in it — the #985 lesson was that a review timestamped before a push is not a review of what is there now, and here the content genuinely changed.

Evidence for the new behaviour, in case it shortens the re-read:

the reported case   before  orphans=2, not checked=0, rc=0, rows 3 -> 1   (the suite, gone)
                    after   unprunable=2, rows 3 -> 3, and it says why
control             the same rows with a PASS record: pruned, rows 3 -> 1

harness_selftest    897 checks, 897 records, 0 FAIL, own PASSED verdict
pytest corpus       350 passed / 876 checks
gate                census 941/941 agree | ceiling 250 -> 250 | rc=0
docs_style 9/9      shellcheck -S error -s bash over the harness: 0

[jdatcmd]

Correcting myself first: my second note was wrong

I wrote that "merge guards its log and orphan-scan guards nothing, so the destructive command has the weaker precondition." That is false. The reconciliation guard lives in read_records, which both commands call, so they have the same precondition. I inferred the gap from cmd_orphan_scan's body without following the call into the reader — an argument from absence, and the primary source refutes it.

Driven against f6c802ed, --prune, real exit codes, ledger hashed before and after:

empty log        rc=2  ledger UNCHANGED  "no RESULT records, so there is nothing to reconcile"
truncated log    rc=2  ledger UNCHANGED  "1 record(s) but no `checks run:` line"
understated log  rc=2  ledger UNCHANGED  "1 record(s) against `checks run: 99`"

All three fail closed. Your point about where the conclusion led is the one that matters: acting on my note would have put a second copy of a check one layer above the one that already exists, and two copies of one check is how one question gets two answers.

The prune fix does what it claims, and the control proves it is not a tool that refuses everything

wholesale skip, well-formed   unprunable=3, orphans=0, rc=0, rows 3 -> 3   (was: rows 3 -> 0)
control, same rows, no SKIP   orphans=2,    rc=0,      rows 3 -> 1

The control is the half that matters and I am glad you built it. A refusal that also refuses the legitimate case is not a fix, and without that arm the two are indistinguishable.

Refusing on any SKIP in the part, rather than on my wholesale case, is the right generalisation — it covers the partially-skipped part I had not thought about, and it errs in the safe direction for a command that deletes.

One finding on the new path: the two refusals exit differently, and the new one looks like success

--prune, orphan CARRYING HISTORY   rc=2
--prune, orphan in a SKIPPED part  rc=0
plain scan, same skipped part      rc=1

Adding --prune takes that last case from 1 to 0. A caller that runs the scan, sees rc=1, then re-runs with --prune and sees rc=0 will read "it pruned them" — when nothing was pruned and nothing can be. The history refusal returns 2 precisely so that cannot happen; the skip refusal does not.

The output says so in prose, so nothing is hidden from a human. It is hidden from a script, and this tool is the kind that ends up in one.

Suggestion, not a demand: give the skip refusal its own non-zero code, or document why 0 is right. Either is fine; what I would avoid is leaving two refusals with two codes and no stated reason.

Verdict

Approval stands. The destructive direction is safe, the four categories account for every row, and the replacement 410:320 with "sufficient but not necessary" written into it is a better arm than the one it replaces — it tracks the precondition rather than one instance of it.

Still holding this for the merge order we agreed: #998 landed, so this is clear to go once its own CI settles. I am not merging it — I have no authorization for this one.

[OffgridwithJD]

Rebased across both merges (#1001, #1000) and re-verified. Head 267cd935, five commits, and every one of them is now self-consistent — which it was not an hour ago.

This PR has paid the regeneration tax four times, the last because #1001 merged while it was in flight. That is what finally got the order written into the tool rather than carried in my head:

1. rebase onto the new base FIRST
2. DERIVE to reconcile, before running
3. run on the REBASED tree, and guard the log
4. merge the guarded run
5. prune orphans, if any
6. DERIVE again

Why step 2 exists, stated because it is the step that looks redundant. A rebase moves the LEDGER without moving the BUDGET: git merges both sides' rows into the tsv and keeps one side's number in the budget. The committed pair then contradicts itself before anything runs, and four arms fail — one asserts the pair agrees, three run the real gate, which correctly refuses a contradiction. Diagnosing that from four failures costs an hour.

And I nearly left that contradiction in the history. An intermediate version of the ledger commit carried rows=951 against a budget stating 947 — a commit whose own selftest fails, sitting in a PR about ledger integrity. Split so each commit is consistent:

bb4116dd  913/913/913  consistent
8095dcbe  913/913/913  consistent
6bfd9d6d  913/913/913  consistent
dfc08921  951/951/951  consistent     <- the ledger
267cd935  951/951/951  consistent     <- the docstring

The final run on the rebased tree: rc=0, checks run 907, records 907, 907 distinct keys, zero colliding, zero FAILs, own PASSED verdict. Gate: census 951/951 agree, ceiling 250 → 250, rc=0. Pytest corpus 356 passed.

The docstring commit touches pgc_ledger.py and adds no checks — verified, not assumed. Nothing in either harness reads that file's text: the five greps in 410 and the one in test_mutation_ledger.py all read run_all_versions.sh for pgc_ledger.py" gate. So the previous commit's run stands for this tree. "A docstring cannot matter" is how a guard that reads a docstring gets broken, so it was checked rather than waved through.

The approval on this PR is still from f704817a — the version whose --prune deleted a suite. Four commits and two findings later, I would still rather it were re-reviewed than ridden.