Skip to content

test: an unrunnable record names the check it stands in for (#1040) - #1041

Merged
jdatcmd merged 3 commits into
commandprompt:mainfrom
OffgridwithJD:fix/1040-unrunnable-names-its-check
Sep 13, 2026
Merged

jdatcmd merged 3 commits into
commandprompt:mainfrom
OffgridwithJD:fix/1040-unrunnable-names-its-check

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

What this changes

Two of the four check_unrunnable calls in one loop in test/hilbert_locality.sh carried
a shortened name; the other two matched their runnable twins exactly:

:574  check_unrunnable "box $box: groups read, Z-order"
:597  check_num        "box $box: groups read over $PLACEMENTS placements, Z-order"

:578  check_unrunnable "box $box: Hilbert reads fewer groups than Z-order"
:602  check_text       "box $box: Hilbert reads fewer groups than Z-order"     <- same name

So the property had two ledger keys, and which one appeared depended on whether that
box's two partitions came out different that day. The key was a function of the data.

Scope, stated rather than implied: hilbert_locality is not a ledger-covered suite
(covered = differential, harness_selftest, native_join_runtime_filter,
native_join_vector_agg), so this is a wrong key waiting to be seeded, not a wrong number
in check_ledger.tsv today.

This is half B of the split agreed with @jdatcmd on #1040. They hold half A, the
grader. B lands first so A lands green.

Why a guard and not just the two-line rename

The convention is already near-universal, and that is exactly what made the lapse
invisible. Of the 25 check_unrunnable call sites in test/*.sh, 23 carry the runnable
name
hilbert_cluster 9 of 9, projection_rewrite 11 of 11 counting its pgc_pass
twin. A shorter name in a new refusal branch reads as ordinary, and both halves of a
two-branch site are rarely read together.

Selftest part 480 drives .github/scripts/unrunnable-arm-names.py over test/*.sh. It is
part 470's property one helper over: 470 asks whether a skip loop still names the arms its
sibling branch emits, this asks whether an unrunnable record names a check the same file
asserts anywhere.

The tool derives three things a list of them got wrong first

derived because
which functions record from the pgc_record call in the body. Two hand-written lists omitted pgc_pass, and projection_rewrite.sh then read as a false orphan: its twin records through pgc_pass, not a check_* helper.
which argument is the name pgc_skip records pgc_record FAIL "$2". Argument one is the CAPABILITY, so reading it takes arrow where the check is called arrow support is present — the bash-side mirror of the name-position defect #1036 and #1038 closed on the python side, across 22 call sites. pgc_require_tools records a FIXED name and takes none.
what counts as a refusal from the verdict recorded, not the helper's spelling. check_unrunnable records FAIL when the reason code is not in the closed list, which made it its own twin in a draft and blocked wrapper detection.

check_skip is deliberately not swept, and that is a finding rather than an omission.
A skipped arm has no runnable counterpart by construction — the name IS the arm — so 21 of
its 23 call sites have no twin and always will. Sweeping it would report 21 mismatches
that are all correct code.

The positive control is the point of the part

This guard's steady state is zero mismatches, and a sweep that matched nothing reports zero
too. Two arms drive the real tool over a fixture whose refusal branch names something the
file never records, and over a control where the names agree. Three further arms pin values
that were wrong in a draft of the tool rather than restating the code: the refusal set,
pgc_skip:2, and pgc_pass/pgc_fail being twins.

The removal proof

The rename is the only variable between the two runs. Both on this branch, db74d9e base.

BEFORE the rename   harness_selftest RC=1, 943 checks, exactly ONE FAIL:

  FAIL  every unrunnable record names a check its own suite asserts (#1040):
        got [hilbert_locality.sh:574 box $box: groups read, Z-order
             hilbert_locality.sh:576 box $box: groups read, Hilbert] want []

        the other 8 arms of the new part all PASS, including both control arms

AFTER  the rename   harness_selftest RC=0, 943 checks

Also in this PR, and why it is not a separate one

The census re-derivation printed in check_ledger_budget.txt reads the wrong field and
returns zero:

awk -F'\t' '$4=="never"' test/check_ledger.tsv | wc -l     -> 0
awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l     -> 1189   on db74d9e
stated in the budget                                          1189

#1010 inserted the majors a row claims as field 4, moving the last-red to field 5, and the
recipe stayed on field 4. The CHANGELOG entry for #1010 states that
check_ledger_budget.txt carries the corrected form — it does not. The gate is
unaffected: it computes the census itself and never runs this command. So the harm is a
reviewer re-deriving the number by the printed method, getting 0, and "correcting" a budget
that was right — the same fail-closed direction as #1004, and the same reason to remove it.

Fixed here rather than filed because this change moves that very number (1189 -> 1198),
and a wrong recipe beside a number nobody can check is worse than no recipe. The two
historical $4 forms in CHANGELOG.md are left alone: they are deliberately preserved wrong
instructions, not live ones.

Verified

harness_selftest    RC=0, 943 checks              (red/green above, PG18a)
sweep               25 sites, 24 compared, 1 dynamic, 0 mismatches
                    unchanged with --keep-heredocs, so the answer does not depend on it
shellcheck -S error over test/*.sh test/selftest/*.sh    clean
docs_style          14/14 PASSED
ledger              rows 1197 -> 1206 (+9, the new part)
                    census 1189 -> 1198, re-derived by the CORRECTED command
                    ceiling 249 -> 249, does not rise
                    orphans 0, unprunable 0
                    gate rc=0 against db74d9e
widened extractor   all 7 ported pairs rc=0 missing=0

That last line is the one that matters for #1040's other half: with these two names fixed,
widening compare_to_bash.py to all eight helpers lands green, with no change to the
port and none to pgc_vacuity.py.

Closes nothing on its own — #1040 stays open for the grader half.

🤖 Generated with Claude Code

https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs

…rompt#1040)

Two of four check_unrunnable calls in one loop in hilbert_locality.sh
carried a shortened name while the other two matched their runnable twins
exactly:

    :574  check_unrunnable "box $box: groups read, Z-order"
    :597  check_num        "box $box: groups read over $PLACEMENTS placements, Z-order"

    :578  check_unrunnable "box $box: Hilbert reads fewer groups than Z-order"
    :602  check_text       "box $box: Hilbert reads fewer groups than Z-order"

So the property had TWO ledger keys and which one appeared depended on
whether that box's two partitions came out different that day. The key was
a function of the data. hilbert_locality is not ledger-covered, so this was
a wrong key waiting to be seeded, not a wrong number today.

THE CONVENTION IS ALREADY NEAR-UNIVERSAL, which is what made the lapse
invisible: 23 of 25 check_unrunnable call sites carry the runnable name
(hilbert_cluster 9 of 9, projection_rewrite 11 of 11 counting its pgc_pass
twin). A shorter name in a new refusal branch reads as ordinary, and both
halves of a two-branch site are rarely read together.

AND A GUARD, because a rename in either branch reintroduces it silently.
Selftest 480 drives .github/scripts/unrunnable-arm-names.py over test/*.sh.

THE TOOL DERIVES THREE THINGS A LIST OF THEM GOT WRONG FIRST:

  which functions record   from the pgc_record call in the body. Two
                           hand-written lists omitted pgc_pass, and
                           projection_rewrite then read as a false orphan.
  which argument is the    pgc_skip records "$2": argument one is the
  name                     CAPABILITY. Reading it takes `arrow` where the
                           check is called `arrow support is present` --
                           the bash mirror of the defect commandprompt#1036 and commandprompt#1038
                           closed on the python side, 22 call sites.
                           pgc_require_tools records a FIXED name.
  what is a refusal        from the verdict recorded, not the spelling.

check_skip is deliberately NOT swept. A skipped arm has no runnable
counterpart by construction -- the name IS the arm -- so 21 of its 23 call
sites have no twin and always will; sweeping it would report 21 mismatches
that are all correct code.

THE PART CARRIES A POSITIVE CONTROL, because the guard's steady state is
zero mismatches and a broken sweep reports zero too. It drives the real
tool over a fixture whose refusal branch names something the file never
records, and over a control where the names agree.

Three arms pin values that were WRONG in a draft of the tool rather than
restating the code: the refusal set, pgc_skip's argument TWO, and pgc_pass
and pgc_fail being twins.

AND THE CENSUS RECIPE IN check_ledger_budget.txt READ FIELD FOUR AND
RETURNED ZERO. commandprompt#1010 inserted the majors as field 4, moving the last-red to
field 5. The CHANGELOG entry for commandprompt#1010 says this file carries the corrected
form; it did not. The gate is unaffected -- it computes the census itself --
so the harm is a reviewer re-deriving 0 against a stated number and
"correcting" a budget that was right. Fixed here rather than filed because
this change moves that very number.

Verified, red then green with the rename as the only variable between the
two runs:

    before the rename  harness_selftest RC=1, 943 checks, exactly one FAIL,
                       naming both offending sites; all 8 other arms of the
                       new part green, including the positive control
    after  the rename  harness_selftest RC=0, 943 checks
    sweep              25 sites, 24 compared, 1 dynamic, 0 mismatches
    shellcheck -S error over test/*.sh test/selftest/*.sh   clean
    docs_style         14/14
    ledger             rows 1197 -> 1206 (+9, the new part), census 1189 ->
                       1198 re-derived by the corrected command, ceiling
                       249 -> 249, orphans 0, gate rc=0 against db74d9e
    widened extractor  all 7 ported pairs rc=0 missing=0, which is the one
                       red commandprompt#1040's grader half would otherwise land with

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs
…commandprompt#1040)

The tool printed MISMATCH lines and returned 0. Measured, unpiped, before
changing anything:

    on main    2 mismatches   rc=0
    on branch  0 mismatches   rc=0

Part 480 gates on the PARSED OUTPUT and so was never fooled -- but that is a
fact about the wiring I happened to write, not about the tool. Anyone who
runs this in CI and trusts $? gets a gate that cannot fail. Reported by
@jdatcmd, who re-measured it without a pipe first, because their initial
reading was tail's status and not the program's.

THREE ARMS, NOT ONE. "Exits non-zero on a mismatch" alone passes on a tool
that always exits 1. "Exits zero when clean" alone passes on a corpus with
nothing to look at. So the clean run uses a SECOND fixture directory holding
only the agreeing file, with a premise asserting that directory really has a
refusal site:

    the sweep EXITS non-zero when it reports a mismatch
    and exits zero on a corpus that has refusal sites and no mismatch
    premise: that clean run had a refusal site to be silent ABOUT

Neither fixture run is piped, and the comment says why.

An em dash reached the changelog and docs_style refused it, which is the
guard doing its job.

Verified on top of b3433be:

    harness_selftest   RC=0, 946 checks (943 + 3), 0 FAIL
    part 480           12 checks, all pass
    sweep unpiped      main 2 mismatches rc=1 | branch 0 mismatches rc=0
    shellcheck -S error over test/*.sh test/selftest/*.sh   clean
    docs_style         14/14
    ledger             rows 1206 -> 1209 (+3), census 1198 -> 1201
                       re-derived by the corrected $5 command, ceiling 249 ->
                       249, orphans 0, gate rc=0 against db74d9e

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Head moved to 75a3447c7c699688e42591cb2f0e67c2364a94b3 — the sweep's exit code was a lie

@jdatcmd found that the tool prints MISMATCH and returns 0. I reproduced it unpiped before
changing anything, because $? after a pipeline is the last stage's:

on main    2 mismatches   rc=0
on branch  0 mismatches   rc=0

Part 480 gates on the parsed output, so it was never fooled. That is a fact about the
wiring in this PR, not about the tool: anyone who runs it in CI and trusts $? gets a gate
that cannot fail. So it is fixed here rather than left as an observation.

Three arms, not one. "Exits non-zero on a mismatch" alone passes on a tool that always
exits 1; "exits zero when clean" alone passes on a corpus with nothing to look at. The clean
run therefore uses a second fixture directory holding only the agreeing file, with a
premise asserting that directory really has a refusal site:

PASS  the sweep EXITS non-zero when it reports a mismatch
PASS  and exits zero on a corpus that has refusal sites and no mismatch
PASS  premise: that clean run had a refusal site to be silent ABOUT

Neither fixture run is piped, and the comment records why.

The delta, so the previous verification does not have to be redone from scratch

.github/scripts/unrunnable-arm-names.py   |  8 +   one-line return, plus its comment
test/selftest/480-...                     | 28 +   three arms, second fixture dir
test/check_ledger.tsv                     |  3 +   the three new checks
test/check_ledger_budget.txt              |  2 +-  census 1198 -> 1201
CHANGELOG.md                              |  9 +   one paragraph

Nothing else moved between b3433be and 75a3447; that stat is the whole difference.
test/hilbert_locality.sh and test/selftest/parts.manifest are untouched by this commit,
so the rename and its removal proof stand exactly as reviewed.

Re-verified on the new head

harness_selftest    RC=0, 946 checks (943 + 3), 0 FAIL
part 480            12 checks, all pass
sweep unpiped       main 2 mismatches rc=1 | branch 0 mismatches rc=0
shellcheck -S error over test/*.sh test/selftest/*.sh   clean
docs_style          14/14
ledger              rows 1206 -> 1209 (+3), census 1198 -> 1201 re-derived by
                    the corrected $5 command, ceiling 249 -> 249, orphans 0,
                    gate rc=0 against db74d9e

One thing the guard caught on me: an em dash reached the changelog and docs_style refused
it. That is the guard doing its job, and worth saying rather than quietly fixing.

Also filed while here: #1042, the same body-extraction defect in skip-loop-arms.py,
A/B'd as latent (byte-identical output on db74d9e9c). Not fixed in this PR — different
tool, different caller, and its own arm is worth more than the fix.

…mmandprompt#1040)

CI's `suites (PG 17)` leg failed on 75a3447 while `suites (PG 18)` passed.
Every suite PASSED in the failing leg; the census agreed at 1201, the ceiling
did not rise, the population reconciled at 253. The only failing condition was
one field:

    major field across harness_selftest rows
      934   15;16;17;18;19
       12   18            <- part 480's, and only part 480's

    PG17: not in the ledger: harness_selftest 480-... (x12, on major 17)
          PG17 has a check the ledger has never seen, which is not a pass

pgc_ledger.py merge stamps the majors of the LOG it is given, and the rows were
merged from a single PG18 run.

NOT FIXED BY SETTING THE FIELD. Writing 15;16;17;18;19 into those rows to match
their 934 neighbours produces a value that is consistent, plausible and
UNOBSERVED. That field is what the gate compares against, so a hand-written row
is a claim the gate can never catch being wrong -- the standard rather than a
measurement checked against one. Proposed by @jdatcmd and withdrawn by them on
that reasoning; their summary is the better one, that copying the neighbours is
the version which looks like diligence.

So harness_selftest was run on all five assert builds and all five logs merged.
The majors field accumulates as a SET (pgc_ledger.py:283), checked in the source
before relying on it, so five merges union rather than overwrite.

The second reason is the stronger one: this answers whether part 480 passes on
15 and 19 AT ALL. Nobody had asked. The red-then-green removal proof for this
whole PR was PG18a, and a defect in the major field is invisible there by
construction.

Agreed with @jdatcmd BEFORE the runs, so it was not decided after seeing the
number: if a major legitimately produced fewer checks, the rows would say what
each run observed. A check existing on four majors and not the fifth is a
finding about the part, not a number to smooth by union or arithmetic.

Verified, all five run on this tree at 75a3447:

    PG15  rc=0  946 checks  0 FAIL  12 rows from part 480
    PG16  rc=0  946 checks  0 FAIL  12
    PG17  rc=0  946 checks  0 FAIL  12
    PG18  rc=0  946 checks  0 FAIL  12
    PG19  rc=0  946 checks  0 FAIL  12

    the five runs' (part, name) sets: 45 each, and PG18 differs from PG15,
    PG16, PG17 and PG19 by ZERO lines -- so no check exists on some majors and
    not others, and no row was added by the four new logs

    ledger gate, run once per major's log:  rc=0 on all five, zero
                                            "never seen" lines
    ledger rows 1209 unchanged | census 1201 unchanged | ceiling 249 -> 249
    the whole diff: 12 insertions, 12 deletions in check_ledger.tsv, and every
    changed row differs ONLY in field 4 (checked by normalising that field and
    asserting each line then appears exactly twice)
    the ledger is now uniform: 1209 of 1209 rows carry 15;16;17;18;19

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Head 3a640b00eda2f3041e6dde0650513d8cde2b195c — the PG17 red was one field

suites (PG 17) failed on 75a3447 while suites (PG 18) passed. Every suite PASSED in the
failing leg.
The census agreed at 1201, the ceiling did not rise, the population reconciled at
253. The only failing condition was the majors field on my twelve new rows:

major field across harness_selftest rows
  934   15;16;17;18;19
   12   18            <- part 480's, and only part 480's

PG17: not in the ledger: harness_selftest 480-... (x12, on major 17)
      PG17 has a check the ledger has never seen, which is not a pass

pgc_ledger.py merge stamps the majors of the log it is given, and I merged one PG18 run.

Not fixed by setting the field

@jdatcmd spotted it independently and proposed writing 15;16;17;18;19 into the twelve rows to
match their 934 neighbours. That goes green, and it is the wrong fix: the value would be
consistent, plausible and unobserved. This field is what the gate compares against, so a
hand-written row is a claim the gate can never catch being wrong — the standard rather than a
measurement checked against one. They withdrew it on that reasoning; their summary is better
than mine, that copying the neighbours is the version which looks like diligence.

The stronger reason is the second one: this answers whether part 480 passes on 15 and 19 at
all.
Nobody had asked. The red-then-green removal proof for this whole PR was PG18a, and a
defect in the major field is invisible there by construction.

So harness_selftest ran on all five assert builds and all five logs were merged. The majors
field accumulates as a set (pgc_ledger.py:283), checked in the source before relying on it.

Agreed with @jdatcmd before the runs, so it was not decided after seeing the number: if a
major produced fewer checks, the rows would say what each run observed. A check existing on
four majors and not the fifth is a finding about the part, not a number to smooth.

Measured

PG15  rc=0  946 checks  0 FAIL  12 rows from part 480
PG16  rc=0  946 checks  0 FAIL  12
PG17  rc=0  946 checks  0 FAIL  12
PG18  rc=0  946 checks  0 FAIL  12
PG19  rc=0  946 checks  0 FAIL  12

(part, name) sets: 45 on each major, and PG18 differs from 15, 16, 17 and 19 by ZERO lines,
so no check exists on some majors and not others and the four new logs added no row

ledger gate, once per major's log:   rc=0 on all five, zero "never seen" lines
rows 1209 unchanged | census 1201 unchanged | ceiling 249 -> 249
diff: 12 insertions, 12 deletions in check_ledger.tsv and nothing else
every changed row differs ONLY in field 4 -- checked by normalising that field and
  asserting each line then appears exactly twice
the ledger is now uniform: 1209 of 1209 rows carry 15;16;17;18;19

The delta from 75a3447 is test/check_ledger.tsv alone, so every earlier verification on that
head stands: the rename, its removal proof, the exit-code arms, shellcheck, docs_style, and
the widened-extractor result.

The lesson worth more than the fix

A green on the major you develop on is not a green on the matrix, and this defect is invisible
there by construction.
CI runs suites on 17 and 18 only, so 17 was the only place it could
surface before merge — and it did.

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE at 3a640b0. 14/14, both suites legs green, read on this sha at 18:04:36Z and not from any earlier rollup. Everything below was re-run here rather than taken from the PR.

The guard reddens and greens, both directions, unpiped

sweep on main db74d9e    rc=1   2 mismatches
sweep on 3a640b0         rc=0   0 mismatches
buckets                  sites 25 = dynamic 1 + compared 24

rc is the verdict now, and that matters beyond this PR: the first version printed MISMATCH and exited 0. Part 480 was never fooled, because it gates on the parsed output — but the next caller is the one that trusts $?, and a tool whose exit code cannot fail is a trap regardless of today's only consumer.

Three arms, not one, which is the part I would point a reader at. "Exits non-zero on a mismatch" alone passes a tool that always exits 1. "Exits zero on a clean corpus" alone passes a corpus with nothing to find. The third arm is what separates them, and it has its own premise:

check "premise: that clean run had a refusal site to be silent ABOUT"   compared == 1

A clean corpus and an empty corpus both report rc=0; only one is evidence.

The ledger fix was derived, not written

The previous head failed suites (PG 17) because its 12 new rows carried major 18 while every other row carried 15;16;17;18;19. I proposed setting the field by hand and that was wrong — the field is what the gate compares against, so a hand-written row is a claim the gate can never catch being wrong. Running all five majors and merging the logs is the right answer, and it answered a question neither of us had asked: whether part 480 runs at all on 15 and 19. Five (part, name) sets of 45, PG18 differing from the other four by zero lines.

Verified here:

diff 75a3447 -> 3a640b0   test/check_ledger.tsv alone, 12 insertions 12 deletions
                          normalise field 4: 12 distinct lines, each exactly twice,
                          ZERO asymmetric -- so the rows differ only in the major
major field, WHOLE ledger 1209 of 1209 carry 15;16;17;18;19 -- no straggler anywhere
census                    stated 1201, $5-re-derives 1201, 1209 rows, 12 for part 480
part 480                  12 real checks == 12 ledger rows
                          (16 code invocations minus 4 that are FIXTURE CONTENT
                           written into offender.sh/control.sh for the tool to read)

The whole-column check is the one worth keeping as a habit rather than an accident: a fix that corrected these twelve and left a thirteenth row elsewhere at 18 would look identical in the diff.

The census recipe

$4=="never" returned 0 against a stated 1189; $5=="never" returns the stated number. Correct to fix it here rather than separately, because this PR moves that number and a recipe nobody can run beside a number nobody can re-derive is worse than no recipe. Leaving the two historical $4 forms in CHANGELOG.md is right — docs_style.sh's scope note is explicit that landed entries are a record, not a live document.

What this PR unblocks

The widened extractor (#1040's other half) is green on this head: all seven pairs rc=0 missing=0, hilbert_locality at 34 bash names. That half goes up separately once this lands, re-derived against the moved main rather than carried from here.

The methodological point, which outlives the PR

A complete red-then-green removal proof could not see this defect, because the proof ran on one major and the field records the major it ran on. Being more careful locally could not have found it; only the matrix could. Generalised: a mutation proof is evidence about behaviour under the conditions it ran in, so any value that RECORDS those conditions is outside what it can reach — a recorded version, date, environment, or build path. The fix for that is another condition, not more rigour.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK

@jdatcmd
jdatcmd merged commit 0d17a87 into commandprompt:main Sep 13, 2026
14 checks passed
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 13, 2026
)

skip-loop-arms.py decides which functions record a check by reading each
one's body, and it took that body as everything up to the next brace at
column zero:

    test/audit.sh:122   q() { run_pg "$PSQL -c \"$1\""; }

q's brace is not at column zero, so its body ran on into the NEXT function's
and swallowed every check call in between. q -- a psql wrapper that records
nothing -- then classified as a RECORDER, and its first argument, SQL text,
entered a set of valid check names. The corpus has 199 one-line definitions,
so this is the common form and not an edge case.

IT CHANGED NO VERDICT ON THE TREE AS IT STOOD, which is why it needs an arm
and not only a fix. A/B between the two extractors on db74d9e:

    loops 8 | compared 6 | interpolated 1 | armless 1

from both, byte-identical. Two extractors that agree on today's corpus are
indistinguishable from the tool's own output, so a regression would have been
invisible to every existing check. That is a property of TODAY'S CORPUS and
not of the tool.

THE TOOL NOW SAYS WHEN A BODY NEVER CLOSES. Counting braces is defeated by an
unbalanced one inside a quoted string, and the corpus has exactly one:
_us_unbound in test/selftest/400-a-check-result-must-be-machine.sh, whose
grep -oE '\$\{?...' and tr -d '${' leave the walk unterminated, so its body is
229 lines and runs to EOF. NOT fixed here -- one pathological definition in
909 does not buy a shell lexer. What is fixed is the silence.

That misclassification predates this change and survives it: _us_unbound is a
"recorder" under the shipped tool AND under this one, and the symmetric
difference of the two emitter sets is empty. Reported separately rather than
folded in, so this PR's claim stays about one route.

AND HEREDOC BODIES ARE BLANKED BEFORE SCANNING FOR DEFINITIONS, which this
file already did for its structure walk and did not do here. The guard for
this very defect writes q() { ... } into a fixture as heredoc content, so
without it the tool reported the test's own fixture as a finding in the real
tree: unclosed 2, the second being the guard's own swallower.

THREE MUTATIONS, BECAUSE NO SINGLE ONE CAN REDDEN ALL THREE ARMS:

    revert the body walk        the one-line wrapper arm            1 FAIL
    delete the unclosed report  the count premise and the NAMED arm 2 FAIL
    report unconditionally      "a clean file is not named"         1 FAIL
                                and the other two stay green

Each mutant was asserted to still PARSE before its red was believed, and each
restore was md5-asserted. The over-naming mutation was @jdatcmd's: two
mutations left the third arm never observed failing, which is coverage that
stops anyone looking.

TWO COMMENTS CORRECTED IN THE SAME CHANGE, both @jdatcmd's findings:

blank_heredocs is NOT IDEMPOTENT -- a second pass meets an opener whose
terminator the first already blanked, finds none, and blanks to EOF. Measured
on lib.sh: 112 lines blanked by one pass, 372 by two. This change calls the
helper from three places where it was called from one, and all three hand it
raw text, so it is clean by construction and nothing enforces that. The
docstring now says so. It cost @jdatcmd four phantom "lost recorders" within
a minute of meeting the function.

And the justification at the structure walk said "a heredoc never contains a
check". True when written; 080-no-suite-pipes-a-captured-string.sh:26-27 now
writes piped() and cased() into one. Corrected in the same change that makes
the code stop relying on it, because otherwise the next reader meets a
justification the tree contradicts and cannot tell which is stale. Third
instance of that shape today, with commandprompt#1043's header and commandprompt#1041's census recipe.

Verified, all five assert builds run separately:

    PG15..PG19   rc=0  954 checks  0 FAIL  8 part-490 rows, each
    (part, name) sets 46 on each major; PG18 differs from the other four
    by ZERO lines
    ledger       rows 1209 -> 1217, census 1201 -> 1209, re-derived by the
                 budget file's own recipe; majors distribution ONE line,
                 1217 at 15;16;17;18;19
    gate         rc=0 on each major's log, zero never-seen lines
    orphan scan  rc=0
    shellcheck -S error over test/*.sh test/selftest/*.sh   clean
    the tool under -W error::SyntaxWarning                  clean
    docs_style   14/14

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 13, 2026
)

skip-loop-arms.py decides which functions record a check by reading each
one's body, and it took that body as everything up to the next brace at
column zero:

    test/audit.sh:122   q() { run_pg "$PSQL -c \"$1\""; }

q's brace is not at column zero, so its body ran on into the NEXT function's
and swallowed every check call in between. q -- a psql wrapper that records
nothing -- then classified as a RECORDER, and its first argument, SQL text,
entered a set of valid check names. The corpus has 199 one-line definitions,
so this is the common form and not an edge case.

IT CHANGED NO VERDICT ON THE TREE AS IT STOOD, which is why it needs an arm
and not only a fix. A/B between the two extractors on db74d9e:

    loops 8 | compared 6 | interpolated 1 | armless 1

from both, byte-identical. Two extractors that agree on today's corpus are
indistinguishable from the tool's own output, so a regression would have been
invisible to every existing check. That is a property of TODAY'S CORPUS and
not of the tool.

THE TOOL NOW SAYS WHEN A BODY NEVER CLOSES. Counting braces is defeated by an
unbalanced one inside a quoted string, and the corpus has exactly one:
_us_unbound in test/selftest/400-a-check-result-must-be-machine.sh, whose
grep -oE '\$\{?...' and tr -d '${' leave the walk unterminated, so its body is
229 lines and runs to EOF. NOT fixed here -- one pathological definition in
909 does not buy a shell lexer. What is fixed is the silence.

That misclassification predates this change and survives it: _us_unbound is a
"recorder" under the shipped tool AND under this one, and the symmetric
difference of the two emitter sets is empty. Reported separately rather than
folded in, so this PR's claim stays about one route.

AND HEREDOC BODIES ARE BLANKED BEFORE SCANNING FOR DEFINITIONS, which this
file already did for its structure walk and did not do here. The guard for
this very defect writes q() { ... } into a fixture as heredoc content, so
without it the tool reported the test's own fixture as a finding in the real
tree: unclosed 2, the second being the guard's own swallower.

THREE MUTATIONS, BECAUSE NO SINGLE ONE CAN REDDEN ALL THREE ARMS:

    revert the body walk        the one-line wrapper arm            1 FAIL
    delete the unclosed report  the count premise and the NAMED arm 2 FAIL
    report unconditionally      "a clean file is not named"         1 FAIL
                                and the other two stay green

Each mutant was asserted to still PARSE before its red was believed, and each
restore was md5-asserted. The over-naming mutation was @jdatcmd's: two
mutations left the third arm never observed failing, which is coverage that
stops anyone looking.

TWO COMMENTS CORRECTED IN THE SAME CHANGE, both @jdatcmd's findings:

blank_heredocs is NOT IDEMPOTENT -- a second pass meets an opener whose
terminator the first already blanked, finds none, and blanks to EOF. Measured
on lib.sh: 112 lines blanked by one pass, 372 by two. This change calls the
helper from three places where it was called from one, and all three hand it
raw text, so it is clean by construction and nothing enforces that. The
docstring now says so. It cost @jdatcmd four phantom "lost recorders" within
a minute of meeting the function.

And the justification at the structure walk said "a heredoc never contains a
check". True when written; 080-no-suite-pipes-a-captured-string.sh:26-27 now
writes piped() and cased() into one. Corrected in the same change that makes
the code stop relying on it, because otherwise the next reader meets a
justification the tree contradicts and cannot tell which is stale. Third
instance of that shape today, with commandprompt#1043's header and commandprompt#1041's census recipe.

THE PATHOLOGICAL POPULATION IS BOUNDED AT TWO, one in each direction, and
the bound is worth more than either instance. Walk each definition's depth
twice -- once as the tool does, once with quoted segments removed -- and flag
where the walks end on different lines. No guess about where a body ought to
end. Over all 907 definitions:

    400-...:338 _us_unbound   OVER-RUN     229 lines vs 21 quote-aware
                              in emitters(): YES, reported by the new line
    220-...:42  _upg_refuses  EARLY CLOSE    2 lines vs 4
                              in emitters(): no, reported by NOTHING

An early close truncates rather than swallows, and unclosed_definitions
cannot report it because the body DID close. This change's visibility covers
over-running only, and the note in the part says so with the instance named.

The discriminator is @jdatcmd's. Neither of our earlier probes could state
the bound: mine over-reported every one-line definition (214 findings, 196+
of them correct behaviour), theirs was blind to a body running to EOF. Each
was blind to exactly the case the other found.

Verified, all five assert builds run separately:

    PG15..PG19   rc=0  954 checks  0 FAIL  8 part-490 rows, each
    (part, name) sets 46 on each major; PG18 differs from the other four
    by ZERO lines
    ledger       rows 1209 -> 1217, census 1201 -> 1209, re-derived by the
                 budget file's own recipe; majors distribution ONE line,
                 1217 at 15;16;17;18;19
    gate         rc=0 on each major's log, zero never-seen lines
    orphan scan  rc=0
    shellcheck -S error over test/*.sh test/selftest/*.sh   clean
    the tool under -W error::SyntaxWarning                  clean
    docs_style   14/14

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 14, 2026
commandprompt#1048)

`pgc_ledger.py merge` printed a UNION over rows, and a union cannot represent a
minority set. Merge rows carrying {18} into a ledger whose rows carry
{15,16,17,18,19} and the union does not move, so the line was byte-identical on a
correct merge and an incorrect one. It was the one statistic that could not see
the only defect this summary has ever had, and it was the only one emitted.

The defect occurred twice in three hours, to the same person, with a written note
about it in between: commandprompt#1041 wrote 12 rows at `18` against 934 uniform ones, caught
only by CI's `suites (PG 17)` leg; commandprompt#1042 wrote 8 against 1209, caught by a manual
`uniq -c`. Both times the merge printed `majors ... 15, 16, 17, 18, 19`. The
operator was not ignoring the output. The output agreed with them.

It now prints the distribution, says NOT UNIFORM when there is more than one set,
and prints `rows N = sum of buckets N` beside it, per the house rule that a
list-derived claim carries its reconciliation.

The arm holds the DISCRIMINATION rather than the wording: it merges the same two
checks two ways and requires the summaries to differ. Asserting on one output
alone would pass against the union for any string containing the five majors. The
correct arm uses five LOGS, not one log naming five majors, because the same name
twice in one log is a duplicate sharing a row and would make the control unfaithful.

Removal proof: restoring union semantics while KEEPING the new output shape
reddens the discrimination assertion, so what is load-bearing is the distribution
and not the rewording. Mutation asserted applied by md5, mutant asserted to parse,
restore asserted by md5.

Reporting only. Whether merge should REFUSE a non-uniform result is a live design
question and is deliberately not settled here.

Also: TESTS.md described the ledger as FIVE tab-separated columns and omitted
`majors` from the list, from the day that column landed (commandprompt#1010) until now.

guard_tests 317 -> 318, re-derived by collection against bf31e2f, never by
arithmetic.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants