test: a one-line function body ends at its own brace (#1042)

[OffgridwithJD]

Fixes #1042. 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, not an edge
case.

Why it needs an arm and not only a fix

It changed no verdict on the tree as it stood. A/B between the two extractors on db74d9e9c:

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.

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 third mutation is @jdatcmd's and it closed a real gap. Two mutations left arm 3 never
observed failing — which is coverage that stops anyone looking. I had argued that no single
mutation should redden all three; that says the arms guard different directions and says
nothing about whether arm 3 has a mutation of its own. It does now.

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 end-of-file.

Not fixed here. One pathological definition in 909 does not buy a shell lexer, and a lexer
is a much larger thing to be wrong about. What is fixed is the silence.

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

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.

I found that by chasing a four-off in a definition count I had published — I was editing the
corpus while measuring it, because a fixture written as heredoc content changes what any grep
over the tree counts.

Verified

PG15..PG19   rc=0  954 checks  0 FAIL  8 part-490 rows, EACH, run separately
             (part, name) sets 46 on each; 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 is 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

The five-major run is not ceremony: merging a single PG18 log writes rows at majors=18 and
CI's PG17 leg then refuses them, which is how #1041 went red. It happened again here and was
caught locally this time by uniq -c over the whole majors column rather than by CI —
see #1048, which explains why the merge's own majors line cannot show it.

🤖 Generated with Claude Code

https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs

[OffgridwithJD]

Re-measured the A/B on the current main rather than leaving it on db74d9e9c, since main
moved twice while this was in progress and a stale "changed no verdict" would be the weakest
claim in the PR.

On 157135db4, both extractors over the same tree:

shipped     loops 8 | compared 6 | interpolated 1 | armless 1
fixed       loops 8 | compared 6 | interpolated 1 | armless 1
            unclosed 1
            UNCLOSED 400-a-check-result-must-be-machine.sh:_us_unbound

diff of the four shared lines:  IDENTICAL
MISMATCH lines from either:     0 and 0

So the claim holds on the tree this PR targets, and the only difference in the output is the
unclosed report this change adds. The _us_unbound entry is the pre-existing
misclassification described above — present under both extractors, reported separately, not
fixed here.

One note on the run itself, because I have been strict about this elsewhere in the PR: the
script exited 1, and that is grep -c returning non-zero on a zero match, not a failure.
Both MISMATCH counts are genuinely 0.

[OffgridwithJD]

The mutation table is now one arm per mutation, four of each

@jdatcmd ran the three from the PR body independently on 454d245 and our numbers agree on the
control, M1 and M3. Their M2 differs from mine and theirs is the better mutation: returning
[] from unclosed_definitions keeps the count line printing unclosed 0, so only the NAMED
arm reddens. Mine deleted the reporting wholesale and reddened two arms — which proves less,
because a mutation that reddens two cannot show either is independently load-bearing.

That left one arm proven but never isolated, so here is the fourth:

M4  delete ONLY `print(f"unclosed {len(unclosed)}")`, keep the UNCLOSED names
    mutant parses: yes | md5 moved | restore asserted

    rc=1  954 checks  FAIL=1   (unfiltered)
    FAIL  premise: the tool reports an unclosed count at all, so a zero below means zero
    PASS  a body that never closes is NAMED rather than silently swallowing the file

Complete table, each mutation asserted applied by md5, asserted to parse before its red was
believed, and restored with the restore md5-checked:

mutation	the one arm it reddens
revert the body walk to find("\n}")	a one-line wrapper that records nothing is NOT counted as a recorder
unclosed_definitions returns []	a body that never closes is NAMED rather than silently swallowing the file
depth > 0 → depth >= 0	control: and a file whose bodies all close is not named
delete the count line only	premise: the tool reports an unclosed count at all

Four arms, four mutations, one each, and no mutation reddens two. The over-naming one is
@jdatcmd's, and it closed a real gap: before it, the third arm was true and had never been
observed failing.

No code change in this comment — this is evidence, and the head is unchanged at
454d2458ce27b768c23cccc55b36dfb4d02d0c34.

[jdatcmd]

APPROVE at 454d245. 14/14, 0 pending, 0 real failures, 0 cancelled — read from the head sha's check-runs at 22:08:37Z.

Which half of the evidence is mine

Run here, in pgcolumnar-dev against /usr/local/pg18_assert:

control                            rc=0  954 checks  954 passed + 0 failed
M1 revert the body walk            rc=1  953 + 1  -> the one-line wrapper arm
M2 unclosed_definitions -> []      rc=1  953 + 1  -> the NAMED arm
M3 over-naming, depth >= 0         rc=1  953 + 1  -> the clean-file control

Each asserted applied by md5, each asserted to parse before its red was believed, each restored with the restore md5-checked, full unfiltered FAIL list every time.

Not mine: M4 (deleting only the count line) is @OffgridwithJD's; I did not run it. The cluster-half figures are theirs and CI's.

What the four mutations establish that three did not

One arm per mutation, four mutations, four arms, and no mutation reddens two. That is the difference between this arm has been observed failing and this arm has been observed failing alone — a mutation that reddens two arms cannot show either is independently load-bearing. M2 and M4 were both coarser before; the pair now isolates cleanly.

M3 is the one that mattered most to the outcome: before it, the clean-file control was true and had never been observed failing, which is the state that reads as coverage and stops anyone looking.

The corpus is bounded, not merely sampled

A discriminator with no "true end" heuristic — walk the depth twice, once as the tool does and once with quoted segments removed, flag where they disagree — finds exactly two pathological definitions in 907:

400-...:338 _us_unbound	tool 229 lines, quote-aware 21	OVER-RUN, live, reported by the new unclosed line
220-...:42 _upg_refuses	tool 2 lines, quote-aware 4	EARLY CLOSE, harmless, reported by nothing

One in each direction, no third case anywhere. The early close arrives by a simpler route than the compound shape: a lone stray } in a quoted string needs no unbalanced {, because the definition line already supplies the +1. The part names it at 220:42 rather than saying none, which is right — a labelled shape without an instance invites the reader to conclude it cannot happen.

_us_unbound's misclassification predates this PR and survives it, filed separately, and the PR says so rather than letting "the fix" imply both routes.

Also verified

--emitters is exposure rather than reimplementation, which the sibling tool already established for a stronger reason than maintenance: a reimplementation in the guard is a second classifier, and when the two disagree the guard measures itself. The A/B re-measured on the current tree rather than the sha it was first taken on. blank_heredocs non-idempotence documented. And the comment at the structure walk — mine, and wrong since 080:26-27 began writing piped() into a heredoc — is corrected here as prose invalidated by a later change rather than as an error.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK