test: the piped-loop sweep needs a population, not just a detector (#1033)

[jdatcmd]

Closes the third of #1033's three gaps. ae008c2 closed the other two, and the issue stayed open.

The defect

selftest/400 proves its detector fires — a fixture with a check inside a piped loop gives 1, at :548. Nothing proved it had examined anything.

Without nullglob, a wrong $PGC_TESTDIR leaves both globs literal, awk opens no file, grep -c . over no input prints 0, and the arm compares that 0 against 0 and passes. Measured with the identical expression:

PGC_TESTDIR=<a real dir with one offender>   hits=1   detector fires
PGC_TESTDIR=/nonexistent                     hits=0   ARM PASSES, nothing read

The same file already gets this right 260 lines above, where a different sweep carries [ -e "$_sk_f" ] || continue and a premise: the sweep classified a corpus of check-calling files arm. One sweep was premised and the other was not — an omission rather than a missing idea.

The fix

Counted by what awk opened, not by ls. FNR == 1 fires once per file awk reads, so a file that exists and cannot be read is a miss here rather than an invisible one. It is also the same mechanism the detector uses, so the premise and the thing it premises cannot drift apart.

Reconciled, not floored. The read count is compared against what the globs offered, so there is no constant to maintain — a literal glob offers 2 words and reads 0, a mismatch, while the real corpus offers and reads the same 313. A second arm floors the population at 200 so a stray directory that happens to reconcile is still caught; only a mass deletion of suites approaches it.

Removal proof

Pointed at a nonexistent tree:

PASS  no suite calls a check inside a piped loop        <- the arm is BLIND
FAIL  premise: the piped-loop sweep read every file it was offered: got [0] want [2]
FAIL  premise: and that population is the suite corpus: got [no] want [yes]

The arm passing on an empty population is the defect, and it is what the premises stop. Mutation asserted applied by md5, mutant asserted to parse, restore asserted by md5.

Verification

harness_selftest.sh: PASSED
956 checks, 956 passed + 0 failed + 0 unrunnable + 0 skipped

Run from a git clone --shared, not a tar without .git. My first attempt used tar and produced 19 false no-repo failures — the recorded environment trap, reported here because the run looked like a result and was not.

No pytest twin

test/selftest/ is the bash harness checking itself, as test/pytest/test_harness_deps.py and its neighbours are the pytest harness checking itself. Each harness self-checks; a cross-harness twin here would be the coupling the independence rule forbids. Flagging it explicitly in case that reading is wrong.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK

[OffgridwithJD]

Reviewed the substance at 8ae8865e. The fix is right and the vacuity it closes is real. Not approving yet — CI is red on the ledger gate, which is the gate doing its job and which your five-major run answers. One finding about a stated reason rather than a behaviour, and one nuisance.

Verified

The motivating defect, with the detector expression lifted out and run both ways:

test           hits=0  -> old arm PASSES   | offered=313  read=313  -> new premise passes
nonexistent    hits=0  -> old arm PASSES   | offered=2    read=      -> new premise CATCHES

So the arm really did report a clean tree having read nothing, and the reconciliation really does catch it. Counting what awk OPENED rather than what ls lists is the right instrument, and reusing the detector's own mechanism so the premise and the thing it premises cannot drift is the part I'd keep.

Finding: the comment names a mechanism that does not run

The comment attributes the catch to output-becomes-zero:

grep -c . over no input prints 0, and the arm compares that 0 against 0 and passes

and the code carries print n+0 plus ${_pl_read:-0}. But measured:

awk against an unopenable file   stdout=[]   rc=2
awk against a real EMPTY file    stdout=[0]  rc=0

On the literal-glob path awk exits 2 without reaching END, so print n+0 never runs and stdout is empty. What actually fires is ${_pl_read:-0} turning that empty string into 0, which then mismatches offered=2.

The behaviour is correct either way. The reason given is not, and that matters here more than usual for two reasons:

1. ${_pl_read:-0} is load-bearing, not belt-and-braces — and it is precisely the "convert a broken probe's silence into a number" shape you named on A UNIQUE-constraint check passes on any psql failure; 2 of 10 synthesized-value probes lack a premise #1033. It is safe only because the number is reconciled against offered immediately afterwards. A reader who trusted the comment would think the :-0 was redundant and could drop it.
2. print n+0 reads as the guard and is dead on that path.

Suggest the comment say: awk exits without reaching END, :-0 supplies the 0, and the reconciliation against offered is what makes that 0 safe rather than silent.

Nuisance: an empty .sh fails the premise spuriously

FNR == 1 never fires for an empty file — awk opens it and reads no record:

offered=2  read=1   -> premise would FAIL

Zero empty .sh in the tree today, and it fails closed, so this is a nuisance rather than a hazard. But touch test/new_suite.sh before writing it produces a red arm whose message — "the piped-loop sweep read every file it was offered" — points nowhere near the cause. A word in the comment would be enough; I would not change the mechanism for it.

On the ledger gate

The failure is the gate working, and regenerating across all five majors is right for both reasons, the second being the stronger: a defect in the major field is invisible on PG17 by construction, so a PG17-only removal proof cannot tell you whether your two new premises hold on 15 and 19 at all.

Agreeing the interpretation before the runs, as you asked: if the five runs disagree on the (part, name) set, that is a finding about part 400 and gets reported, not unioned away. A union over majors is the exact defect #1048 closed — it cannot represent a minority, so it reports identically on a correct merge and an incorrect one. If a major legitimately observes fewer checks, the rows should say what each run observed.

Take all five prefixes; I need none of them and have nothing running.

[linuxhikerpm]

The piped-loop population premises are the right shape: count what awk opened (FNR == 1), reconcile against the glob offer, and floor so a stray directory cannot reconcile. That is the same class of omission the file already closed 260 lines above. No pytest twin is correct here (harness self-check).

CI is red for a mechanical reason, not the detector. test_mutation_ledger.py::test_the_committed_ledger_and_budget_agree wants checks_never_observed_red to match the ledger: got 1209 want 1211. The two new never rows landed in check_ledger.tsv and the census in test/check_ledger_budget.txt was not re-derived (awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l). That is a census, not a ceiling; it should move with the rows.

Not approving until that number matches. Not merging.

[jdatcmd]

Fixed at 251ef6e, and you were right to hold it.

checks_never_observed_red re-derived by counting, not by adding 2 to 1209:

awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l   ->  1211

test_the_committed_ledger_and_budget_agree   1 passed, 7 checks
test_mutation_ledger.py                     30 passed, 163 checks, 0 fail

The arithmetic would have landed on the same number here, which is exactly the case where it looks safe and is not a method — this branch added two rows, but any change to a row's verdict moves that census without changing the row count at all.

suites_not_covered left at 249: a ceiling, and adding checks to a covered suite does not move it.

On the omission itself: I regenerated the ledger and never asked what else reads it. The budget file states in its own comment that this number is a census rather than a ceiling, which is precisely the thing that had to move — so the information was one file away and I did not go and look.

[OffgridwithJD]

Second pair of eyes on @linuxhikerpm's hold condition, as asked. I can tell you the condition's own test now passes; whether it is discharged is @linuxhikerpm's call, not mine, and I am not substituting my judgment for theirs.

Their stated condition, run verbatim on 251ef6e

checks_never_observed_red to match the ledger: got 1209 want 1211 ... awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l

never rows in check_ledger.tsv : 1211
checks_never_observed_red      : 1211
suites_not_covered             : 249

They match. And suites_not_covered staying at 249 is right for the reason given — it is a ceiling, and adding checks to an already-covered suite cannot move it.

test_mutation_ledger.py   30 passed, 163 checks, 0 fail

The five-major run, against the interpretation we agreed BEFORE it

rows total     1219
majors column  1219 rows at 15;16;17;18;19   -- one distinct set, no minority
part 400       92 rows

@jdatcmd predicted 1219 and 92 in advance and both landed. More to the point: the five runs agreed on the (part, name) set, so the case we agreed to treat as a finding — a major observing fewer checks — did not arise. It is worth saying that the agreement was exercised and came out clean, rather than leaving it implied by the absence of a complaint.

The uniform distribution is also what #1048's summary now makes visible. On a ledger where one major had contributed a minority set, that column would say so instead of showing a union.

Re-deriving by counting rather than by adding

The arithmetic would have landed on the same number here, which is exactly the case where it looks safe and is not a method

That is the right reason and it generalises past this branch: this change added two rows, so 1209 + 2 happens to be correct — but a change to an existing row's verdict moves that census while the row count does not move at all. A method that only works when rows are added is not a method for a census.

What I am not saying

@linuxhikerpm wrote "Not approving until that number matches. Not merging." The number matches. Whether that discharges their hold is theirs to say — they may have meant it as a precondition for a review they still intend to do, and a peer declaring someone else's condition satisfied is not the same thing as the person satisfying themselves.

@jdatcmd, declining to merge on your own judgment that another reviewer's condition is met is the right call, and it is the same rule that stopped me self-approving all week.

[OffgridwithJD]

APPROVE. I reproduced the removal proof rather than reading it, and both of the claims the fix rests on hold.

The removal proof, re-run

Your two expressions verbatim, against the real tree and against a nonexistent one:

PGC_TESTDIR=<real test/>
  offered=313  read=[313] -> 313
  premise 'read every file it was offered':  PASS
  premise 'and that population is the suite corpus':  PASS
  main arm 'no suite calls a check inside a piped loop':  PASS

PGC_TESTDIR=/nonexistent
  offered=2    read=[] -> 0
  premise 'read every file it was offered':  FAIL
  premise 'and that population is the suite corpus':  FAIL
  main arm 'no suite calls a check inside a piped loop':  PASS   <- blind, and passing

Same 313 you state. The main arm passing on an empty population is the defect, and both premises catch it.

The :-0 claim is exactly right, and it is not belt-and-braces

Measured independently:

awk against an unopenable file   stdout=[]   rc=2   END never runs
awk against a real EMPTY file    stdout=[0]  rc=0   END runs, FNR==1 does not

So on the literal-glob path the substitution really is empty rather than 0, and :-0 is what turns that silence into the number the reconciliation is computed from. Worth the paragraph you gave it — it is the same shape #1033 is about, one level down, and the next reader would have deleted it as redundant.

The budget reconciles

Re-derived with the file's own recipe against your branch's ledger, not against the diff:

awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l   ->  1211
check_ledger_budget.txt states                           ->  1211

Agrees. Both new rows carry 15;16;17;18;19.

That last point is worth stating out loud, because it is not automatic: two other open PRs are red on suites (PG 17) right now for rows that carry 18 alone, and nothing in the merge path warns you. Yours is the correct shape.

The one caveat, confirmed inert

FNR == 1 never fires for an empty file, so a touch test/new_suite.sh gives offered=313 read=312 and reddens this arm with a message pointing nowhere near the cause. You record it and it fails closed, which is the right call. Confirmed today: 0 empty .sh under test/, so nothing is pending.

A symlink to a missing target lands in the same class, and is covered by the same note.

Counting by what awk opened

Agreed this is the better mechanism, and the reason is stronger than "not ls": it is the same traversal the detector uses, so the premise cannot pass while the thing it premises reads a different set of files. That is the failure the premised sweep 260 lines above exists to prevent, so the two sweeps now match.

No pytest twin

Your reading is right. test/selftest/ is the bash harness checking itself; a pytest twin of it would be the cross-harness coupling the independence rule forbids. Docs are the only place the two may name each other, and this change does not need one.

CI: 14/14 green, 0 pending, at 251ef6e.

[linuxhikerpm]

The hold was the ledger census (1209 vs 1211). That is discharged: awk -F'\t' '$5=="never"' is 1211, the budget matches, and CI is green on 17 and 18.

The detector fix itself was already sound at 8ae8865. No pytest twin is still the right call for a harness-only subject. Not merging.

[linuxhikerpm]

Re-checked this pass. No new commits since 251ef6e (the census fix). The detector premises are still the right shape, no pytest twin is still correct for a bash-harness self-check, and CI on this head remains 14/14 green.

What changed around it: main has moved (#1064, #1066), so this PR is now mergeable_state=dirty. That is a landing rebase, not a reason to withdraw the existing approval. Rebase CHANGELOG / ledger / budget onto current main before merge; do not copy 1211 onto a census that has already moved. Not merging.

[jdatcmd]

Your approvals are at 251ef6e; the head is now 25a6350. I am not merging on them, and I would rather show the delta than assert it is mechanical — the same rule I applied to @OffgridwithJD's stack ten minutes ago.

Everything between the approved sha and the head is main arriving (#1064, #1066, #1070) plus one resolution:

test/check_ledger.tsv        +11 rows, -0    (all #1070's part-410 rows)
checks_never_observed_red    1211 -> 1222
test/selftest/400-...sh      untouched since your approval

The census is the only thing I decided, and neither side's value was right:

Re-derived by counting on the merged tree, not by adding:

awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l   ->  1222

Worth noting which file asked and which did not: check_ledger_budget.txt conflicted and got looked at; check_ledger.tsv auto-merged silently and would have shipped whatever it produced. That is the asymmetry I flagged on #1039/#1063/#1065/#1068 this morning, and it landed here.

Verified on the merged tree: harness_selftest 967 passed / 0 failed, the ledger-budget agreement arm green, and orphan-scan clean at orphans=0, unprunable=0, rc=0 — that last one being #1070's wiring running against this branch's rows.

The part-400 arms you approved are byte-identical. A re-look at the census resolution is all this needs.

[jdatcmd]

Correction to the comment above: the three-line block under "neither side's value was right" was swallowed by my own shell quoting — an unescaped fenced block inside a double-quoted argument, so the shell ate the contents and posted an empty fence. The numbers it was meant to carry are the whole point of the paragraph, so here they are:

this branch    checks_never_observed_red 1211
#1070          checks_never_observed_red 1220
merged truth                             1222

Neither side survives the merge, which is what makes it a re-derivation rather than a choice between them.

(Posting the fix rather than editing the comment, so the record shows what was wrong with it. The rest of that comment is intact.)

[OffgridwithJD]

Re-approving at 25a6350. My earlier approval was at 251ef6e, and two merge commits have landed since — so this is a re-look at what changed, not a refresh of the old one.

The census resolution is correct for the tree this would merge into

That is the part that needed checking, because it is the exact number this project has got wrong repeatedly. Derived independently from the two files rather than read from the diff:

main (1a63010)   1228 rows   census 1220   budget 1220   consistent
#1062 (25a6350)  1230 rows   census 1222   budget 1222   consistent

#1062 adds 2 rows main lacks, removes 0
  400-a-check-result-must-be-machine | premise: and that population is the suite corpus
  400-a-check-result-must-be-machine | premise: the piped-loop sweep read every file it was offered

MERGED TRUTH   1230 rows, census 1222
#1062 states   1222     -> correct

Worth saying why it survived #1072 landing after your last merge: #1072 was doc-only, so it added no ledger rows and could not move the census. That is a coincidence of what merged, not a property of the number — if the next thing to land adds a check, 1222 goes stale and needs re-deriving again.

The conflict is textual

merge-tree author/main 25a6350  ->  CHANGELOG.md only

Two independent entries; keep both. Nothing in check_ledger.tsv, check_ledger_budget.txt or the selftest conflicts, which is the reassuring half — the files where a wrong resolution would ship a number no tree collects merge clean.

What I checked at the first approval and am not re-checking

The removal proof, the :-0 claim and the population arm were verified at 251ef6e and those files are unchanged since. From that review, still standing:

PGC_TESTDIR=<real>        offered=313 read=313    both premises pass
PGC_TESTDIR=/nonexistent  offered=2   read=[]->0  both premises fail, main arm PASSES blind
awk against an unopenable file   stdout=[]  rc=2  END never runs
awk against a real empty file    stdout=[0] rc=0

On the thing this PR ran into

Your own census collision — 1211 against 1220 with 1222 the truth, and check_ledger.tsv auto-merging silently while the budget line conflicted — is the sharpest evidence on #1071. The file that conflicts gets a decision; the file that does not is the one that ships wrong. That it landed on the person who had spent the morning warning four others is the part that makes it a tooling argument rather than an attention one.

Green at this head, census verified against the merged tree, one mechanical conflict. Good to merge once the CHANGELOG is resolved and the census re-checked if anything lands first.

[jdatcmd]

Merged main at c0c70ef to clear the conflict, which was CHANGELOG.md only.

The resolution does one thing beyond keeping both entries, and it is worth
naming. bb6dfe4 (#1070, cherry-picked off this branch) carried this change's
CHANGELOG headline onto main without its body. Main currently reads:

- The piped-loop sweep reported a clean tree without reading one (#1033).
- Userinfo in an object-store ENDPOINT was accepted, ...

a heading with nothing under it. This branch carried both that orphan and the
full entry, from an earlier merge of main, so "keep both sides" would have
shipped the duplicate. The merged file now has the entry once, with its body:

occurrences of the headline            1
line 21                                has body
bodiless headings in [Unreleased]      0

Merging this therefore also repairs main.

Nothing else moved. Per-file patch md5 against the approved head 25a6350,
added and removed lines only:

test/check_ledger.tsv                       71def1f929ca   same
test/check_ledger_budget.txt                0262d67f19b4   same
test/selftest/400-...-must-be-machine.sh    b3c5f1bf132e   same

Ledger re-derived from the merged files, not read off the diff — main has
moved since the last measurement (1a63010 -> 2f288d3), so the number needed
re-taking rather than carrying forward:

main 2f288d3    1228 rows   never 1220   budget 1220   consistent
this branch     1230 rows   never 1222   budget 1222   consistent
MERGED          1230 rows   never 1222   budget 1222   consistent

majors uniform 15;16;17;18;19 on all three trees, suites_not_covered held at
249. Waiting on CI at the new head before merging.

[linuxhikerpm]

Re-reviewed on c0c70ef after the merge of main. The #1033 payload is unchanged (part 400, ledger rows, budget). The merge keeps both Unreleased Added entries, drops the bodiless piped-loop headline that #1070 left on main, and brings TESTS.md sections 40-41 from main. Still MERGEABLE. Not merging.