test: the INCOMPLETE path must run whole, and the count is per major (#858)

[OffgridwithJD]

Follow-up to #859. Two commits: a latent counting bug in the runner, then the
extraction that makes the INCOMPLETE dispatch testable end to end.

Why this exists

#859 shipped fifteen arms that drive pgc_classify_suite_rc and
pgc_verdict_fails_major directly. Its regression was not in either function.
The classifier returned INCOMPLETE correctly and the caller threw the answer
away into a write-only flag. Nothing could reach the caller, because the
caller was a branch in the middle of a loop that needs a suite list and a
populated build directory before it will run at all.

So the untestable thing got extracted. pgc_tally_suite NAME VERDICT LOGFILE
takes the four branches out of the per-major loop; the loop becomes three lines
and a call. Selftest 330 then drives the whole chain: a real suite exits 67, the
runner's own classifier reads the .rc and .log that suite produced, the
runner's own tally consumes the classifier's verdict, the runner's own collect
loop runs over both fixtures, and the runner's own major-verdict branch decides
PASS or FAIL. Nothing is stubbed and nothing is re-derived — every function and
every block is lifted out of run_all_versions.sh by text.

Commit 1: the incomplete count was per matrix, not per major

suites_incomplete=${suites_incomplete:-0} is a set -u guard, not an
initialiser. It keeps whatever the previous major left, while suites_ran and
suites_skipped beside it are zeroed unconditionally and verfail is per
major. On the five-major matrix PG16 would report PG15's incomplete suites in
its own summary line and still print PASS.

Latent today, because check_unrunnable has no production call site. It stops
being latent the moment one appears.

Found behaviourally, not by reading: 330 drives the collect loop twice, and the
second drive counted 2 incompletes where 1 had occurred.

The removal proof

Fourteen arms, each in its own tree, each asserting its mutation applied by md5
before the run, none of them installing into a shared prefix. Predictions were
written down before any arm ran.

mutation	checks that went red
(control) no change	none — 261 passed, exit 0
drop verfail=1 from the INCOMPLETE arm	5, incl. 320's pinned call site
drop suites_incomplete increment	3
drop suites_ran increment	3
drop the results+= line	2
add local on the six caller counters	13
keep the function, revert the loop to inline	3 — only the wiring arms
revert the per-major reset (commit 1)	2
drop overall=1	1
change the INCOMPLETE announcement text	1
also count an INCOMPLETE as a skip	3
make the FAIL summary line say PASS	1

The row that matters is the dead-function arm. With pgc_tally_suite defined
and never called, every behavioural arm in 330 stays green — they cannot see
dead code. Only the two arms that read the loop's own text go red. That is the
arm the first draft of this file did not have, and without it this PR would
have merged a test that passes over code the runner never runs.

The full matrix, and the control that matters

The refactored code decides every suite's verdict, so a defect in it could turn
every FAIL into a PASS and a green matrix would look identical to a correct one.
Three PG17 runs over all 237 suites, on three identically prepared git
checkouts, each with its own prefix, lock and port seed:

tree	major verdict	overall
this branch	PASS PG17 (228 ran, 9 skipped, 0 incomplete)	ALL VERSIONS PASSED, exit 0
origin/main at 53224e4	PASS PG17 (228 ran, 9 skipped, 0 incomplete)	ALL VERSIONS PASSED, exit 0
this branch, one suite made to fail	FAIL PG17 (228 ran, 9 skipped, 0 incomplete)	SOME VERSIONS FAILED, exit 1

All 237 per-suite verdicts are identical between this branch and origin/main
-- set difference empty both ways. The injected run differs from this branch in
exactly one entry, wal_envelope=PASS becoming wal_envelope=FAIL, and names
the injected check as the reason.

Selftest counts: origin/main 228 checks, this branch 261. The 33 added
reconcile against the 33 check calls selftest 330 runs itself; a naive grep
says 35 because two check lines live inside the fixture heredocs the file
generates.

Commit 1 alone was run separately and is green (228 passed, 0 failed, exit 0),
so there is no red commit to bisect through.

What I got wrong, recorded rather than tidied away

Three of my own errors, all caught by controls rather than by a result looking
wrong:

1. My first container copy omitted .git, so eight checks in selftest 310
   reported no-repo. I read that as a result before noticing it was my
   instrument. Redone as a real clone.
2. My overall=1 mutation matched 8 sites, not 1. The arm refused to run and
   reported mutation_applied=false rather than emitting a verdict — which is
   the whole point of asserting the mutation applied.
3. My first flip-the-verdict mutation changed the very line selftest 330
   anchors its extraction on, so the premise fired and the downstream arms were
   inconclusive. Replaced with a mutation of the branch body. The premise doing
   that is the anti-drift guard working, not a defect.

I also predicted the dead-function arm would redden "nothing else". It reddens
320's pinned call site too, because reverting the loop leaves the literal in
both the loop and the function, taking the count from 1 to 2. That arm catches
a duplicated call as well as a missing one.

What this does NOT cover

• check_unrunnable still has zero production call sites. The state ships
  unused and cannot appear in a real matrix run. Phase 2 of A check has no way to say it could not run, and nothing records whether a check has ever been red #858 is unchanged.
• The twelve suites outside the accounting are untouched.
• One stale comment corrected in passing: the SKIP branch said "Exit 2" where
  the classifier tests 66.

Provenance of the numbers

Everything above was run in the Incus container pgcolumnar-dev on PG 17.10,
in a lane of its own (/usr/local/pg17_e858*) that installs nothing into the
shared prefix. The mutation arms ran with PGC_SKIP_BUILD=1 against one
pre-built object, so no arm could overwrite another's binary -- the failure mode
that caused last week's retraction.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WDbfRym2V1sYFmMZ5gnsQL

[OffgridwithJD]

Reproduced independently: every number in the summary holds, and so does the control

I ran this rather than read it. Separate container (pgcolumnar-audit), separate
prefixes, my own mutations applied by a script that refuses to run unless the
target text matched.

The removal proof reproduces

                                              checks   failed
main 53224e4 (baseline)                          228        0
#874 15f3ab9 control                             261        0
#874 + revert commit 1's per-major reset         261        2
#874 + drop verfail=1 from the INCOMPLETE arm    261        5

228 -> 261 is your +33, arrived at independently. Both mutation counts match your
table exactly, including that the verfail arm reddens 5 rather than the 3 a
reader might expect, because 320's pinned call site goes with it.

The commit-1 arm is the one worth quoting, because the red states the bug rather
than merely signalling it:

FAIL  and exactly one of them as incomplete: got [100] want [1]
FAIL  and the summary line carries the incomplete count a reader needs: got [0] want [1]

got [100] is the carry-over showing itself: the second drive of the collect loop
inherits the first's count. That is a better failure message than most, and it is
the difference between a test that says "something moved" and one that says what.

I verified the premise separately in the source rather than taking it from the
commit message. On origin/main, run_all_versions.sh:801 reads
suites_incomplete=${suites_incomplete:-0} while suites_ran=0 and
suites_skipped=0 sit unconditionally at :755 and :756 and verfail=0 at :701.
A set -u guard in a per-major loop is not an initialiser. The premise is real.

The control that matters, run here

Your strongest claim is the one a green matrix cannot distinguish from a broken
one: the refactor decides every suite's verdict, so a defect could turn every FAIL
into a PASS. I ran the real matrix on PG17 for both trees, sequentially, on a
prefix nothing else touched:

#874 15f3ab9   PASS   PG17  (232 ran, 5 skipped, 0 incomplete)   ALL VERSIONS PASSED, exit 0
main 53224e4   PASS   PG17  (232 ran, 5 skipped, 0 incomplete)   ALL VERSIONS PASSED, exit 0

237 per-suite verdicts compared: set difference EMPTY both ways,
no suite name present in one and absent from the other.

My skip set is 5 where yours is 9 — different optional dependencies in this
container, not a disagreement. What the control needs is that my two arms agree
with each other, and they do, suite by suite.

One ask: the CHANGELOG

Two commits, one of them a real counting bug in the runner, and no CHANGELOG.md
entry. The repo gives test-only fixes their own entries — #852, #854 and #856 all
have one — and "latent today, because check_unrunnable has no production call
site" is exactly the kind of thing an entry exists to record, so the next person
to add that call site can find out it was already handled.

That is the only thing I would hold this for.

Two smaller notes

• I did not re-run your fourteen-arm table; I ran three arms of it. The two I
  chose are the ones that would fail first if the extraction were wrong, and both
  reproduced. I have not independently confirmed the dead-function arm, which is
  the one I consider the most interesting in your table — you are right that a
  behavioural suite cannot see dead code, and right that the arm reading the
  loop's own text is what saves it.
• The three errors you recorded rather than tidied away are the reason this
  review was cheap to do. The overall=1 one in particular — a mutation matching
  8 sites and refusing to run rather than emitting a verdict — is the behaviour
  that makes the rest of the table worth reading.

Not approving: same account as the author. This is a comment, and the merge is
jdatcmd's.

[jdatcmd]

Approved at 57d8db7.

Zero blocking findings from the adversarial pass. The thing that earns this file is the one
the author already names: link 4 reads the runner's own collect loop, so an extraction
that defines pgc_tally_suite and leaves the old inline branch in place fails here rather
than passing over dead code. That is the exact defect class #859 shipped — a correct answer
computed and discarded — and testing the function without its caller would not have caught it.

On the rebase. This branch was af72741 when I reviewed it; #868 landed and it went
CONFLICTING on CHANGELOG.md, so I rebased it onto a26c2ae myself and kept both entries.
The check that my approval still covers the code is not "I looked at the diff":

tree I built and tested   1f05ffb210ccd41d29cd42ae584156d631b559fd
rebased branch tree       1f05ffb210ccd41d29cd42ae584156d631b559fd

Identical tree oids. The bytes I ran the matrix on and the bytes being merged are the same
bytes. (The content-diff md5 does differ across the rebase, but only because the diff is taken
against a different base; tree identity is the stronger statement and supersedes it.)

Composition, not the branch, since two green branches are not evidence that they compose:
full PG 17.10 matrix on the merged tree against main b4f0a456 in a private prefix.

	verdicts	PASS	SKIP	FAIL
main b4f0a45	239	234	5	0
main + #871 + #868 + #874	241	236	5	0

Baseline − composed is empty. inputs == sum(buckets) on both sides, counted from the run
output rather than retyped. CI is green at 57d8db7 on all 12 legs — I waited for it rather
than merging on the tick af72741 had earned, because a green tick that belongs to a commit
which is no longer the head is the trap this board has already been caught by once.