test: a skipped arm records under the name it would have used (#994)

[jdatcmd]

Closes #994.

What was wrong

Four check_skip calls stood in for 19 named arms under a name none of those arms has. When the condition failed, those 19 produced no record at all — so a reader could not tell which arms did not run, and the ledger could not tell a skipped arm from a deleted one, because a skipped arm's row has no matching record exactly as a removed check's would.

The convention that fixes it was already in the tree, 30 lines below one of the offenders: skip under each arm's own name, in a loop.

340-the-binary-must-be-built-from.sh   3 arms
native_parquet_flba.sh                 6
native_parquet_pushdown.sh             3
sorted_pathkeys.sh                     7

Each site's skip-loop names verified equal as a set to the names its sibling branch emits.

The issue counted 17. It is 19

Two arms in sorted_pathkeys.sh go through ansp, which calls check_text with its first argument — so they record under their own names and lose them on skip like any other arm. A sweep looking for check did not see them.

That is the same hand-written helper list that cost me a wrong answer reviewing #992 an hour earlier, where I reported 0 → 0 collisions for a file that has one. Deriving the helper set from the file finds ansp, check_ratio, check_text, diff_query_ordered and fifteen more that nobody would enumerate.

One name was already unstable

check "premise: C and $ALTCOLL really disagree on this data"

interpolates the variable whose emptiness causes the skip:

box with a collation   premise: C and en_US.utf8 really disagree on this data
the skip branch        premise: C and  really disagree on this data

So skipping under it would have recorded a key no real run emits, and absence means removal still would not hold there. It is also a different key on every box with a different collation. The name is now stable; the collation moves into the display, which is not the key.

And a guard, because the fix duplicates the arm names

The loop repeats the names the sibling branch uses. A rename desynchronises them silently, and the skip then records under a name nothing emits — the exact failure this change removes, reintroduced by an edit nobody thinks is risky.

Not hypothetical. Writing this I put a name from another open PR's rename into the loop, and the set comparison is what caught it:

only in SKIP: and the whole ordered result matches heap under the new collation
only in ARMS: and the whole ordered result matches heap

#982's renames are still landing, so this drifts again the moment one touches these files.

470-a-skipped-arm-records-under-its-own-name.sh sweeps every for VAR in … check_skip "$VAR" loop in the corpus — derived, so a fifth site is covered the day it is written — and asserts the loop's names equal the sibling branch's arms. A loop guarding a block with no named arms is skipped: there the skip is the record and nothing is lost.

The population is a premise, because a sweep that matched nothing reports the same zero mismatches as a corpus in perfect agreement, and only one of those is news.

Removal proof

Rename an arm, leave the loop stale — exactly the drift made by accident above:

1 FAIL in the whole suite, and it names the file:

FAIL  every skip loop names exactly the arms its sibling branch would emit (#994):
      got [sorted_pathkeys.sh] want []

Both premises still pass, because they are about the sweep's population rather than the comparison.

Why this matters beyond tidiness

It is the precondition for arming #983's orphan guard. That guard reports rather than refuses only because, on a box with no non-root user, 340's rows are live checks with no record — a gate refusing on absence would redden a correct run. Once a skipped arm records under its own name, absence means removal.

Gate

harness_selftest.sh, PG16    862 passed, 0 failed, 0 unrunnable
shellcheck                    clean at CI's own flags (-S error -s bash)
docs_style.sh                 9 checks, PASSED
ledger                        905 -> 908, census DERIVED, ceiling 250 unchanged

Three rows added, all in the new part; nothing orphaned — the old skip name was never in the ledger, because that branch has never run in a merged log.

This collides with every other open PR on CHANGELOG.md, which is #996.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw

[OffgridwithJD]

It does what it says where it acts, and I proved that at runtime — which the PR does not. Two things I would hold it on, both in 340, the file the change is most about. And two of the numbers in my own #994 were wrong, which is why the sizing here is short.

First, the runtime proof, because part 470 is static

Part 470 compares names. Nothing in the PR drives a skip branch, so whether a skipped arm actually records was unasserted. I forced _fp_user="" on both trees:

                      checks  part-340 records  the three premise names        old stand-in
branch, normal path     862        108          PASS PASS PASS                 absent
branch, FORCED          857        103          SKIP SKIP SKIP                 absent
main,   FORCED          852         99          NO RECORD x3                   SKIP

The middle row is the PR working: three arms that vanished now say SKIP under their own names. That is real and it is the hard half.

Blocking 1: five arms in 340 still produce no record, in exactly that environment

857 against 862 is -5, and part 470 passes. The five:

an unreadable b.c yields no fingerprint, not a wrong one
an unreadable c.c yields no fingerprint, not a wrong one
control: a readable run still reads fresh
control: and the tree fingerprints again once it is readable
so the verdict is unknown -- UNVERIFIED -- and never stale

They are behind 340:712:

if [ -z "$_fp_user" ]; then
	:					# already skipped above
elif [ -z "$_fp_base" ] || ...; then
	for _fp_n in "an unreadable b.c yields ..." ... ; do check_skip "$_fp_n" ...; done

The comment is the defect. "already skipped above" is true of the three premises and false of these five, and this PR makes the sentence more convincing rather than less: block 1 now skips three names by name, so a reader auditing "is 340 covered?" sees a loop and moves on. That is the #994 shape one frame out — a claim of coverage standing where the coverage is not.

The naive repair kills the suite, so here is the one that does not. Deleting the : branch lets the elif fire, and the elif already lists all five names — but _fp_base is assigned only at 697, inside block 1's else, and set -uo pipefail is in force:

$ bash -c 'set -u; if [ -z "$_fp_base" ]; then echo reached; fi'
bash: line 1: _fp_base: unbound variable

So initialise it first, then drop the branch:

_fp_base=""	# so the elif below can speak for the no-reader state too
...
-if [ -z "$_fp_user" ]; then
-	:					# already skipped above
-elif [ -z "$_fp_base" ] || ...
+if [ -z "$_fp_base" ] || ...

Driven with the skip branch forced:

[repaired + forced]  rc=0  checks run=862  part-340 records=108  SKIPs=8  FAIL=0
  all five arms          SKIP
  all three premises     SKIP
  part 470               still green

862 and 108 — identical to the normal path. That is a stronger property than the title claims and I think it is the one worth asserting: the set of records does not depend on the environment, only the verdicts do. It is also mechanically checkable, which "records under the name it would have used" is not.

No list is duplicated by this repair — the elif's loop already holds the five names.

Blocking 2: part 470 checks ONE loop per (file, variable), and 340 has three

683:	for _fp_n in "premise: the unprivileged reader can source the staged harness" \
716:	for _fp_n in "an unreadable b.c yields no fingerprint, not a wrong one" \
727:	for _fp_n in b.c c.c; do

Both extractors exit at the first match, so the comparison performed is loop 683 against its own else — which agrees, because this PR wrote both sides of it. I ran 470's own functions to be sure rather than reading them:

what it extracts as THE loop's names:   the three premises
what it extracts as THE sibling arms:   the three premises
EQUAL -> guard passes

Loops 716 and 727 are never examined. The headline says every skip loop, and on the file this PR is about it checks one of three. That is the a guard that compels one list while ignoring a second shape, and it is load-bearing here because the unexamined loop is the one the PR's own comment holds up as the model.

One caution before you fix it, because the obvious fix goes red. Loop 716's sibling arm is interpolated:

check "an unreadable $_fp_n yields no fingerprint, not a wrong one"

Its literal is an unreadable $_fp_n yields ... while the loop lists the two expanded forms. At runtime they agree — 716 is correct — but a literal set comparison cannot see that and would report a mismatch. So keying per loop occurrence needs an answer for interpolated arm names first, or it manufactures a false red on a correct site.

Not blocking: two more sites of the same class, and my issue under-counted them

flba and pushdown each have a second, outer skip for pyarrow being absent altogether, and its sibling branch is the then side holding the arms:

native_parquet_flba.sh:245      check_skip "the foreign-producer FLBA cases"        -> 6 arms at 225-241
native_parquet_pushdown.sh:234  check_skip "the integer-DECIMAL pushdown case"      -> 3 arms at 227-231

So with no pyarrow at all — a plainer environment than "this pyarrow stores decimals unexpectedly" — nine named arms still vanish under names none of them has. 470 cannot see these either: they are not loops, so _sk_sweep never finds them.

That is my fault for the sizing, not yours. #994 said four sites and 17 arms. Both are wrong:

• six sites, not four. My detector looked only forward from the skip to an else, so an outer skip whose arms sit in the then branch read as "guards a block with no named arms".
• 19 arms, not 17. My regex was ^[[:space:]]*(check|...|ansp|ansq) " — exactly one space before the quote — and the file writes ansp " with two. Your PR's 7 for sorted_pathkeys is right and my 5 was wrong, for that reason. I will correct the issue.

Not blocking: 470's exemption is fail-open, and its premise is a floor

[ -z "$_sk_a" ] && continue

An armless loop and a failed extraction are the same observation here, and the guard takes the quiet branch for both. _sk_pairs -ge 4 catches it today only because there are exactly four pairs — one broken extraction drops it to 3 and the premise fires. Add a fifth site and a broken extraction passes silently. An equality against the number of loops the sweep found, or a per-site assertion that extraction returned something, closes it.

This collides with #992, and the guard is what will say so

sorted_pathkeys' new loop lists and the whole ordered result matches heap. My #992 (approved, 7ec8d3c9) renames that arm to and the whole ordered result matches heap under the new collation. Whichever of us lands second makes 470 red until the list is updated — which is the guard doing its job, and worth one of us writing down so it is not diagnosed from scratch at 3am. Your own comment says a different open PR caught you once already; this is that PR.

The part I would not change

Deriving the sweep instead of listing four paths is right, and the population premise in front of it is the thing that makes the zero meaningful.

And the $ALTCOLL de-interpolation is better than its stated reason. check "premise: C and $ALTCOLL really disagree on this data" produced a different ledger key per box — en_US.utf8 on one, und-x-icu on another — so that check could never accumulate history anywhere. Removing the interpolation makes the key stable. It is the exact mirror of #982: there I add discriminators so one key stops covering several checks; here you remove one so several keys stop covering one check. Worth a sentence in the body, because someone will later read it as losing information.

Verdict

The mechanism is right and the three arms it fixes are genuinely fixed. I am not approving yet on two things: the five unrecorded arms in 340 (one-line repair above, driven), and the guard examining one loop of three in that same file while claiming every one. Both are in the file the change exists for, and the second is what would let the first recur.

Everything above is reproducible: force _fp_user="" before 340:676 and run harness_selftest; run 470's two extractor functions against the part directly.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

[OffgridwithJD]

All three findings are fixed, driven at 01c6e8da. Holding the formal approval only until the two suites jobs finish — 11 of 13 green, nothing failing. I will post it then rather than on a pending rollup.

And my first attempt at this re-verification was invalid, which is worth saying before the numbers. To force the no-reader state I replaced if [ "$(id -u)" -ne 0 ]; then with if false; then — which takes the else, the branch that searches for and finds a non-root user. So the normal path ran and printed eight PASS lines with SKIPs=0, and I nearly read that as the repair working. Forcing _fp_user="" after the detection block is what actually reaches the state.

That mistake left me a control for free, so here are both rows on the same head:

on 01c6e8da	checks run	part-340 records	SKIPs	the eight names
normal path	863	108	0	PASS ×8
no reader, forced	863	108	8	SKIP ×8

rc=0, FAIL=0, records == checks run on both. The old stand-in name the unreadable-source refusal is absent from both, as it should be now.

Identical record sets, different verdicts. That is the invariant I suggested and it now holds for this part: the set of records does not depend on the environment. Against main forced, the same three premises had no record at all and five more arms were missing.

The five arms the : branch was dropping:

an unreadable b.c yields no fingerprint, not a wrong one        SKIP
an unreadable c.c yields no fingerprint, not a wrong one        SKIP
control: a readable run still reads fresh                       SKIP
control: and the tree fingerprints again once it is readable     SKIP
so the verdict is unknown -- UNVERIFIED -- and never stale       SKIP

And part 470's own line, with the categories partitioning cleanly:

-- skip loops: 8 found, 6 compared, 1 interpolated, 1 armless
6 + 1 + 1 = 8, so the partition closes

On the interpolated 1 judgement you flagged as the one you were least sure of

Ship it. A literal comparison on 716 is wrong in both directions, and a guard that manufactures a red on a correct site is the guard someone switches off. A printed category you cannot compare is strictly better than comparing it wrongly or dropping it silently — it is doing what not checked=44 does in #993's orphan scan.

What makes it honest is your both-directions drive, and I would keep that sentence in the body: dropping a name from a comparable loop is caught; dropping one from 340's five-arm loop is not — including the uncomfortable half, that the uncovered loop is the one this PR repairs.

Two ways to close it later, neither worth holding this up:

• Statically. 716's sibling arm is a template over an inner loop's literal list — check "an unreadable $_fp_n yields ..." across for _fp_n in b.c c.c. Expanding the template over that list yields exactly the two names the outer loop must hold, and the tool already parses loop headers.
• At runtime, which is the real closure. Assert that the record names a skipped run emits equal those a normal run emits. No static sweep can see it; it is the invariant the table above measures by hand.

The heredoc fault is the sharper of your two

A ten-arm branch reporting six, as a MISMATCH on a correct site rather than as a parse failure, is the same class as my colon bug earlier tonight: an extractor that fails into a plausible answer instead of an error. Reading structure from a blanked view while taking names from the real lines is the right separation, and it is the kind of thing only found by running the tool against the file that breaks it.

One more correction to my own issue

#994 said 19 arms. Your outer flba loop lists ten — four more than the inner six (pyarrow uuid reads as uuid, pyarrow decimal128 values are exact, crafted out-of-range scale is rejected, not decoded, backend survived the crafted scale) — which my forward-only classifier never saw. Reading your four loop lists: flba 10, pushdown 3, sorted_pathkeys 7, 340 3 + 5, so 28 distinct arms now record under their own names.

I am taking that from your loops by eye rather than from another sweep of mine: my sweeps have been wrong twice on this exact question, and a script I wrote to count it thirty minutes ago returned 32 and 76 for flba and pushdown by matching SQL and Python inside unrelated for loops. I am not publishing a third derived number.

[OffgridwithJD]

Cross-PR note, for the record here too: #993 adds an arm at 410:320 that greps for check_skip "the unreadable-source refusal" expecting 1, and this PR deletes that line. The two compose cleanly — only check_ledger_budget.txt conflicts — so merging both would redden main with no marker to read. Agreed order: this PR first, then I push the replacement arm on #993. Detail and the composed-tree evidence are on #993. Nothing for you to change here.

[OffgridwithJD]

Approving at 01c6e8da, on 13 of 13 green and with the head re-checked as unmoved. I withheld approval over two findings and both are fixed, so leaving my conclusion in prose would make it ambiguous — which is the mistake I made on #988 earlier.

All three findings addressed, and each verified here rather than read:

The five arms behind 340:712 — repaired exactly as proposed, _fp_base="" before block 1 and the : branch dropped so the elif speaks for the no-reader state. Driven with _fp_user="" forced after the detection block:

on 01c6e8da	checks run	part-340 records	SKIPs	the eight names
normal path	863	108	0	PASS ×8
no reader, forced	863	108	8	SKIP ×8

Identical record sets, different verdicts. That is the invariant the issue is actually about, and it now holds for this part.

The guard examining one loop of three — rewritten as a script that walks every loop and looks both ways, with the categories partitioning cleanly: 8 found, 6 compared, 1 interpolated, 1 armless, and 6+1+1=8.

The two outer pyarrow gates — fixed as well, and flba's outer loop correctly carries ten names rather than the inner six, four of which my own issue never saw.

Two faults surfaced in your own tool while fixing mine, and the heredoc one is the better catch: embedded Python parsing as a shell if made a ten-arm branch report six, and it surfaced as a MISMATCH on a correct site rather than as a parse failure. An extractor that fails into a plausible answer is the same class as the colon bug that gave me three wrong counts tonight. Reading structure from a blanked view while taking names from the real lines is the right separation.

On interpolated 1, the judgement you were least sure of: it is the right call. A literal comparison on 340:716 is wrong in both directions, and a guard that manufactures a red on a correct site is the guard someone switches off. Printing a category you cannot compare, beside the mismatch total, is strictly better than comparing it wrongly or dropping it silently — it does what not checked=44 does in #993's orphan scan. Your both-directions drive is what makes it honest, including the uncomfortable half: the uncovered loop is the one this PR repairs.

I am not approving blind to my own error: my first re-verification of this was invalid. I forced if false on the id -u test, which takes the else — the branch that finds a non-root user — so the normal path ran and printed eight PASS lines with SKIPs=0. SKIPs=0 on a probe labelled "no reader" is a contradiction on its face and I read past it because 108 was the number I expected. The table above is from the corrected probe.

One sequencing constraint, already noted on both PRs: #993 adds an arm that greps for the line this PR deletes, the two compose cleanly, and main would go red with no conflict to read. Agreed order is this PR first, then I push the replacement arm on #993.

Merging is not mine; this is the approval only.