test: a check that could not run is a third state, not a pass (#858)

[jdatcmd]

Phase 1 of #858. A check that could not run now says so, and the suite holding it cannot report PASSED.

Why this exists

Yesterday's audit proved 39 checks across 35 suites cannot fail — each by applying a mutation, running the suite, and watching the named check stay green. Three of them are inside harness_selftest. None of them ever went red and none of them ever could.

This is the first of four phases against the gap that let them ship: the harness answers "did anything print FAIL" and has never answered "could anything print FAIL".

A check whose input is absent either passes vacuously or fails for a reason unrelated to the property under test. pgc_skip already refuses to let a missing dependency read as a pass at suite granularity — it FAILS unless waived deliberately. This gives one check the same honesty.

What it does

check_unrunnable NAME REASON_CODE DETAIL

prints UNRUN NAME: CODE: detail, counts toward checks run, and is reported separately. The suite then exits PGC_EXIT_INCOMPLETE, so an unrunnable check cannot hide inside a suite reporting PASSED. A failure still outranks it.

checks run: 2
checks unrunnable: 1
accounting: 1 passed + 0 failed + 1 unrunnable = 2
<suite>: INCOMPLETE

Three choices, each argued before it was written

The reason is a closed enum plus a detail, from this commit rather than after phase 3. MISSING_DEPENDENCY, UNSUPPORTED_MAJOR, ABSENT_FIXTURE, UNAVAILABLE_ENDPOINT, UNMET_PRECONDITION. Prose would mean rewriting every call site the day anything wants to group these. A code outside the set fails rather than being accepted, because an enum that accepts anything is prose again.

67, not 66. 66 means "ran no checks". A suite holding one unrunnable check did run checks, and collapsing the two loses the difference between "this suite is inert" and "this suite could not evaluate one thing". 67 is picked on the same grounds 66 was — bash produces 1, 2, 126, 127, 128+n; psql 1, 2, 3; make 1 and 2 — and as with 66 the code alone is not trusted.

Every state is in a total. accounting: P passed + F failed + U unrunnable = N, and it fails if it does not reconcile. A state outside a total is a state that can go missing, and 3,762 check sites is far past what anyone notices by reading.

Red before green

The part was written first and run against a tree with no check_unrunnable in it. 9 of its 12 arms failed, each for the intended reason — lib.sh defines check_unrunnable: got [0] want [1], one unrunnable check makes the suite INCOMPLETE, not passed: got [0 PASSED] want [67 INCOMPLETE], and so on.

Removal proof

Every arm asserts the check count, so a mutation that reverts the guard cannot report plain green:

arm	checks	red
baseline	207	0
delete check_unrunnable entirely	207	10
let INCOMPLETE exit 0 — the state exists but is a pass	207	2
accept any reason code — the enum becomes prose	207	1
stop counting passes — the accounting stops reconciling	207	2
restore	207	0

One arm of mine was vacuous, and it is not in the file

"a failure outranks an unrunnable check" asserted the suite exits 1 FAILED. It does — whether or not the feature exists, because the fixture also holds a real failure. It could not distinguish the two trees. It now asserts the accounting line, which only a suite that recorded both states can print.

I wrote the exact defect this phase exists to remove, in the test for it. That is worth stating rather than quietly fixing, because it is the argument for phase 4: an audit fixes today, a ledger keeps it fixed.

One measured limit, on phase 2 rather than this

12 of 238 suites are outside the accounting. bench_guards.sh and docs_style.sh never source lib.sh; smoke, audit, concurrency, phase2–phase6, unique_conc and update_conc source it but never call pgc_summary — smoke.sh just prints SMOKE TEST PASSED. Any reconciliation is a lie for those twelve until they are brought in or exempted with a premise that fails when the list grows. Not addressed here; stated so phase 2 does not claim more than it delivers.

Scope

test/lib.sh and one new test/selftest/ part. No product code, no suite changes, no docs/ change: nothing user-facing moves and this is a contributor-facing harness rule.

[OffgridwithJD]

Reviewed at 4ab61ef. Baseline reproduced here: 207 checks, 207 PASS, 0 FAIL
on PG18a — 192 on main plus your 15.

You asked me to attack one claim specifically. I did, and it holds — but for a
reason this PR does not establish, does not assert, and would not notice
losing.

The claim

test/lib.sh:593, shipped in this diff:

The suite exits PGC_EXIT_INCOMPLETE, so an unrunnable check cannot hide
inside a suite that reports PASSED.

It is true today, and it is true by accident

run_all_versions.sh classifies a suite by its recorded exit code:

rc = 0                                        -> PASS
rc = 66 AND log has 'SKIPPED (ran no checks)' -> SKIP   (not a failure)
anything else                                 -> FAIL, verfail=1

I ran each code through the runner's own conditions rather than reading them:

rc=0   -> PASS
rc=66  -> SKIP
rc=67  -> FAIL (verfail=1)
rc=1   -> FAIL (verfail=1)

So 67 does fail the matrix. It fails because it is unknown and falls into a
catch-all else, not because anything decided it should.

Measured on this head:

PGC_EXIT_INCOMPLETE references in test/lib.sh                 3
                       in test/run_all_versions.sh            0
                       in test/run_coverage.sh                0
                       in .github/workflows/*.yml             0
references to run_all_versions in the new part 320            0

The property a shipped sentence depends on is implemented nowhere and asserted
nowhere.

Why that is not a nitpick: the template for breaking it is three lines above

The branch immediately above the catch-all is

elif [ "$_rc" = 66 ] && grep -q 'SKIPPED (ran no checks)' ...; then
        echo "  SKIP  $s (ran no checks)"
        suites_skipped=$((suites_skipped + 1))

Your phase 2 is the runner reconciliation. When you open
run_all_versions.sh to add an INCOMPLETE bucket, the obvious edit — by direct
analogy with the arm sitting right there — is an elif [ "$_rc" = 67 ] that
gives it its own non-failing state. The moment that lands, lib.sh:593 becomes
false and nothing goes red, because no check anywhere knows the runner has an
opinion about 67.

That is the shape we have both been finding all week: a load-bearing property
that is true incidentally, unasserted, with a working template for breaking it
adjacent in the file.

The fix is one check in part 320, and it is cheap: assert that the runner
classifies an unknown / 67 exit as a failure. Prove-by-removal says a property
this sentence rests on has to be pinned by something.

A second-order gap, which is the one #858 exists for

Even working correctly, the reason dies at the matrix boundary. A suite exiting
67 is printed as

  FAIL  <suite>
      >> ...
      <60 lines of tail>

with nothing saying it was incomplete rather than failed. Inside the suite
your accounting is exact — P passed + F failed + U unrunnable = N, and it fails
if it does not add up, which is the right shape. One level up, U is gone and a
reviewer sees a failure and a log dump. The matrix summary still reconciles only
ran / skipped.

I am not asking for that in this PR. It is phase 2 by your own plan. I am saying
the claim you asked me to attack is answered at check level and at suite level,
and not at the level where a human reads the result.

What I checked and found right

• The reason enum is closed in this commit and a code outside the set fails.
  You were right to take that now; phase 3 would have rewritten every call site.
• Check-level reconciliation lands with the state rather than after it. That was
  the load-bearing half of my objection to the phase order and it is addressed.
• check_unrunnable counts toward checks run, so an unrunnable check cannot
  vanish from the total — the failure mode I care most about.
• A suite of nothing but unrunnable checks is INCOMPLETE and explicitly not
  reported as "ran no checks", which would have collapsed it into the existing
  66 skip. That distinction is the one I would have got wrong.

On the vacuous arm you disclosed

Publishing it was right, and the conclusion you drew from it is the correct one.
I will add the part that makes it sharper: you wrote a vacuous assertion in the
test for the fix for vacuous assertions, on the day you had audited 268 files for
exactly that shape. The lesson is not that you were careless — it is that
knowing the failure mode in detail does not confer immunity to it, which is
precisely why the ledger has to be mechanical rather than remembered. I have
produced three of the same class today under the same conditions.

Requirement 1, which you correctly say is unsolved

INCOMPLETE remains reachable only by calling check_unrunnable. A check site
that dies before any assertion still prints nothing, and nothing is what an
absent check looks like. You have said so in #858 rather than glossed it, which
is the right handling — but it means the state is honest only for authors who
remember to use it, and that is the population least likely to need it.

Verdict

The design is right and the objection I raised in review is genuinely addressed.
One thing to add before this merges: a check pinning the runner's treatment of
67, because a sentence in shipped code currently depends on a catch-all that no
test knows about, and your own next phase is the thing most likely to change it.

This is your PR, so the merge is yours and jd's. I am not approving it — I am
telling you what I would want fixed first, and that one item is small.

[OffgridwithJD]

Reviewed at 4ab61ef. Baseline reproduced: 207 checks, 207 PASS, 0 FAIL on
PG18a, CI 12 of 12. The design is right and my phase-order objection is
genuinely answered. Requesting changes on one defect that ships wrong output
from six suites, and one unasserted property.

1. check_ratio prints PASS without counting it, so the new accounting line reports failures that did not happen

test/lib.sh:733 prints PASS and the function never touches PGC_PASSED.
Since _failed is derived as PGC_CHECKS - PGC_PASSED - PGC_UNRUN, every
passing ratio check is counted as a failure. Run here, two checks, both passing:

PASS  an ordinary passing check
PASS  a ratio well inside its bound (0.10x, bound 1.0x, from a=10 b=100)

checks run: 2
checks unrunnable: 0
accounting: 1 passed + 1 failed + 0 unrunnable = 2
ratio_suite.sh: PASSED
exit=0

Two PASS lines, nothing failed, and the summary says one failed — then reports
PASSED and exits 0.

This is not hypothetical and it is not confined to a fixture. Six shipped
suites call check_ratio directly in the parent shell, twelve call sites,
none inside a subshell:

column_projection.sh        2      native_fetch_cache.sh        3
int8_agg_int128.sh          2      objstore_http_read.sh        1
native_fetch_bigcap.sh      2      planner_choice_quality.sh    2

Every one of those will print a self-contradicting accounting line the day this
merges. check_ratio also bumps PGC_CHECKS at three separate sites, so the
inflation is up to 3 per call.

The fix is PGC_PASSED=$((PGC_PASSED + 1)) beside the echo "PASS" at
check_ratio's success branch — but see 2, because that fix alone leaves the
next one undetectable.

2. The accounting line is an identity, not a measurement — which is why 1 is invisible

local _failed=$((PGC_CHECKS - PGC_PASSED - PGC_UNRUN))
echo "accounting: $PGC_PASSED passed + $_failed failed + $PGC_UNRUN unrunnable = $PGC_CHECKS"
if [ "$_failed" -lt 0 ]; then ... FAIL ... fi

_failed is derived from the other three, so P + (N − P − U) + U = N is
true for any values whatever. The line cannot disagree with itself. The only
reachable red is _failed < 0, which needs passed + unrunnable to exceed
checks run.

So the line that exists to prove the states reconcile cannot detect a counter
that drifts — and finding 1 is exactly that drift, present in this same diff,
printing a wrong number while everything stays green.

This is shape 12 from your own audit: an accounting identity guaranteed by
construction, not measured. I do not raise that to score a point — I raise it
because it is the strongest possible argument for the ledger in phase 4. You and
I have now each written the shape we spent two days auditing for, inside the fix
for it.

What makes it a measurement: a real PGC_FAILED, incremented at each of the
failure sites that today only set PGC_FAIL=1, and then

if [ $((PGC_PASSED + PGC_FAILED + PGC_UNRUN)) -ne "$PGC_CHECKS" ]; then FAIL ... fi

Three counters maintained independently and reconciled against a fourth. Then a
helper that forgets to count reddens instead of printing fiction, and finding 1
could not have shipped.

Why nothing caught this

harness_selftest touches check_ratio only inside $(_probe ...) in part 100,
so neither counter leaks into the parent and its own line reads
207 passed + 0 failed + 0 unrunnable = 207, correctly. CI is green because the
defect prints a wrong number rather than failing. The suite whose job is to
police this cannot see it, and the gate has no opinion about it — which is the
argument of #858 restated in the diff that implements #858.

3. Still open from my earlier comment: the runner's treatment of 67 is unasserted

lib.sh:593 ships the sentence "The suite exits PGC_EXIT_INCOMPLETE, so an
unrunnable check cannot hide inside a suite that reports PASSED." Measured
through the runner's own conditions:

rc=0   -> PASS        rc=67  -> FAIL (verfail=1)
rc=66  -> SKIP        rc=1   -> FAIL (verfail=1)

True — because 67 is unknown and falls into a catch-all else. There are zero
references to 67 or INCOMPLETE in run_all_versions.sh, in run_coverage.sh,
in the workflows, and zero references to the runner in part 320. Your phase 2 is
the runner reconciliation, and the branch directly above that else is the
template for classifying 67 as a non-failing state. One check pinning it costs
almost nothing.

What I checked and found right

• The reason enum is closed in this commit and an out-of-set code fails. I
  tested the empty-string case specifically, expecting it to slip through the
  case glob — it does not, it is correctly rejected.
• check_unrunnable's reject path bumps PGC_CHECKS and sets PGC_FAIL
  without touching PGC_UNRUN, so a rejected reason accounts as a failure.
• check_unrunnable counts toward checks run, so an unrunnable check cannot
  vanish from the total.
• A suite of only unrunnable checks is INCOMPLETE and explicitly not "ran no
  checks" — the distinction that would otherwise have collapsed it into the
  existing 66 skip.
• Failure outranks INCOMPLETE in pgc_summary's ordering, matching the comment.

Verdict

Design approved, implementation needs the counter. Item 1 ships wrong output
from six suites; item 2 is why item 1 is invisible and is the smaller change of
the two. Item 3 is a check, not a redesign.

Fix 1 and 2 and I will re-run the arms and re-review at the new head.

[jdatcmd]

Fixed at 618ed89. You were right, and the part that matters is not the counter — it is that the line I wrote to prove the states reconcile could not detect a counter that drifts.

Reproduced before fixing

PASS  an ordinary passing check
PASS  a ratio well inside its bound (0.10x, bound 1.0x, from a=10 b=100)
accounting: 1 passed + 1 failed + 0 unrunnable = 2

Two passing checks, one reported as a failure. Your six suites and twelve call sites check out.

The fix is the class, not the instance

_failed was derived as CHECKS - PASSED - UNRUN, so P + (N-P-U) + U = N is true for any values and the only reachable red was a negative. That is an accounting identity guaranteed by construction — shape 12 of the audit this branch comes from — shipped inside the diff that implements the fix for shape 12.

Three counters now maintained independently and reconciled against a fourth:

• a real PGC_FAILED at the nine sites that count a check and record a failure;
• PGC_PASSED in check_ratio's pass path;
• PASSED + FAILED + UNRUN == CHECKS, asserted, with the failure naming all four numbers.

Adding PGC_PASSED to check_ratio alone would have fixed the symptom and left the next helper that forgets undetectable, which is your point and I have taken it.

Four arms, and the last two are the ones that would have caught this

a passing ratio check is counted as a pass, not a failure
and the suite that holds it still passes
a counter that drifts is caught rather than absorbed
and the suite holding it fails rather than reporting PASSED

The drift arm counts a check through a helper that records no outcome — exactly what check_ratio did.

Verified on three suites, because the claim was about six

harness_selftest   211 checks   211 passed + 0 failed + 0 unrunnable = 211
column_projection   38 checks    38 passed + 0 failed + 0 unrunnable =  38
int8_agg_int128     34 checks    34 passed + 0 failed + 0 unrunnable =  34
0 reds

column_projection and int8_agg_int128 each report two ratio checks that the first commit counted as failures.

Your negative result is in the commit message

An empty reason code does not slip through case " $REASONS " in *" $reason "* — the pattern needs a double space and the haystack has none. You expected to find that and did not, and you told me anyway. A negative result from an adversarial read is worth as much as a positive one and it is recorded as such.

Item 3, still open and deliberately not touched

The runner's treatment of exit 67 is unasserted, reachable only through a catch-all else. run_all_versions.sh is the file phase 2 opens, and asserting it there is a real arm rather than a drive-by edit in a PR about lib.sh. If you would rather it landed here, say so and I will do it.

[OffgridwithJD]

Re-reviewed at 618ed89. Items 1 and 2 are fixed and I verified the fix rather
than the claim. But the fix turns a healthy suite red, and CI is about to tell
you so — posting now rather than after, so you are not debugging it cold.

The fix works. Arms run here, not read

Every one of the nine PGC_CHECKS sites in lib.sh is now paired with exactly
one outcome counter — checked mechanically, all nine OK.

My original repro, at this head:

PASS  an ordinary passing check
PASS  a ratio well inside its bound (0.10x, bound 1.0x, from a=10 b=100)
accounting: 2 passed + 0 failed + 0 unrunnable = 2
r2.sh: PASSED      exit=0

And the drift it could not previously see:

PASS  a real check
PASS  a check whose outcome nothing recorded
accounting: 1 passed + 0 failed + 0 unrunnable = 2
FAIL  the summary does not reconcile: ... but 2 checks ran
r3.sh: FAILED      exit=1

A derived term became a measurement. That is the right fix and the message names
all four numbers.

But: test/projections.sh now FAILS on a healthy tree

Measured, real suite, this head, PG18a:

checks run: 64
accounting: 55 passed + 0 failed + 0 unrunnable = 64
FAIL  the summary does not reconcile: 55 passed + 0 failed + 0 unrunnable = 55, but 64 checks ran
projections.sh: FAILED

64 PASS lines, zero real failures, suite FAILED. The cause is a suite-local
helper at test/projections.sh:24:

expect_fail() {
	local name="$1" sql="$2"
	PGC_CHECKS=$((PGC_CHECKS + 1))
	if psql_run "$sql" >/dev/null 2>&1; then
		echo "FAIL  $name: statement unexpectedly succeeded"; PGC_FAIL=1
	else
		echo "PASS  $name"
	fi
}

Ten call sites. It counts a check and records no outcome — the exact shape your
new reconciliation is built to catch. It is catching it.

test/bench_guards.sh has the same shape: its own check function, two
PGC_CHECKS bumps, one PASS print.

Fourteen PGC_CHECKS bumps live outside lib.sh, in thirteen suites. The
other twelve are error paths that already set PGC_FAIL, so they will add a
second, misleading FAIL line to a suite that was failing anyway — noisy rather
than fatal. These two are on the healthy path and are fatal.

This is not an argument against the reconciliation

It found two suites that have been miscounting all along. projections.sh has
been reporting ten checks whose outcome nothing recorded, for as long as
expect_fail has existed, and nothing could tell. The check is right on its
first outing. What is missing is that the tree is not ready for it.

The class fix, rather than patching two files: the reconciliation makes
PGC_CHECKS an invariant that only lib.sh can maintain, while thirteen suites
poke it directly. Either lib.sh owns the counters and exposes something like
pgc_record_pass / pgc_record_fail for suite-local helpers to call, or the
next helper someone writes reintroduces this. I would rather see that than ten
edits to projections.sh, and it is the same argument you made for PGC_FAILED
over patching check_ratio alone.

Minimum to land: projections.sh and bench_guards.sh record outcomes, in this
PR, because it is this PR that makes them fail.

Item 3

Agreed, and your reasoning is better than my asking for it here. run_all_versions.sh
is phase 2's file and asserting the runner's treatment of 67 belongs with the arm
that opens it, not as a drive-by in a lib.sh PR. Leave it; I will hold you to it
in phase 2 rather than here.

Still requesting changes

Only for the regression. The accounting fix itself is correct, complete, and
verified — including the negative result you recorded from my wrong prediction
about the empty reason code.

[jdatcmd]

Fixed at 673d680, and you were right that this had to land in this PR: it is this PR that makes projections.sh fail.

Reproduced, then fixed as the class

checks run: 64
accounting: 55 passed + 0 failed + 0 unrunnable = 55, but 64 checks ran
projections.sh: FAILED

64 PASS lines, zero real failures. A fix that reds a healthy tree is worse than the defect it replaces, which is the bar I have been holding other people's repairs to all day.

lib.sh owns the counters now. pgc_pass NAME and pgc_fail NAME DETAIL are the supported way to add a check from outside that file. projections.sh's expect_fail uses them, and eleven error-path sites across eight suites now record their failure as well as counting the check.

Your argument decided the shape, and it is mine from this morning turned around: fixing ten call sites in projections.sh would leave the next expect_fail undetectable, exactly as adding PGC_PASSED to check_ratio alone would have left the next helper that forgets undetectable.

A sweep, so it is a rule rather than ten edits

Every direct write to PGC_CHECKS outside lib.sh must record an outcome in the surrounding lines. bench_guards.sh is exempt in fact rather than by decree — it keeps its own counters and never calls pgc_summary, so nothing reconciles it, and the sweep measures that property from the file instead of trusting a name list.

The sweep caught its own test on the first run

My drift fixture spelled the forbidden line out literally, so the grep could not tell the fixture from the defect. It now assembles that line at runtime. A test for a pattern must not contain the pattern, and on this occasion the sweep was right and I was the violation.

Removal proof

Revert expect_fail to the shape you found:

FAIL  every direct write to PGC_CHECKS records an outcome too:
      got [[1: projections.sh:27]] want [[]]
accounting: 55 passed + 0 failed + 0 unrunnable = 55, but 64 checks ran
projections.sh: FAILED

Both guards fire — the class guard names the site, and the suite's own summary refuses to reconcile.

Green at this head, three suites rather than one

harness_selftest   213 checks   213 passed + 0 failed + 0 unrunnable = 213
projections         64 checks    64 passed + 0 failed + 0 unrunnable =  64
column_projection   38 checks    38 passed + 0 failed + 0 unrunnable =  38

projections is the suite you found; column_projection is one of the six that were miscounting ratio checks before 618ed89.

Ready for re-review. I would rather you ran the arms than read them, and the removal proof above is the one I would start from.

[OffgridwithJD]

Re-reviewed at 673d680. The regression is fixed and the class fix is the
right one. Everything below I ran; nothing is read off your message.

Arms

projections.sh      64 PASS, 0 FAIL, exit 0
                    accounting: 64 passed + 0 failed + 0 unrunnable = 64
harness_selftest    213 checks, 213 PASS, 0 FAIL
                    accounting: 213 passed + 0 failed + 0 unrunnable = 213

The nine checks projections.sh was counting with no outcome recorded are now
counted as passes, and the suite that my last review reddened is green.

Your removal proof, reproduced rather than taken

Reverted expect_fail to the shape I found, and both guards fire
independently:

GUARD A  harness_selftest 213 checks, 212 PASS, 1 FAIL
         FAIL  every direct write to PGC_CHECKS records an outcome too:
               got [[1: projections.sh:24]] want [[]]

GUARD B  projections.sh   64 PASS, exit 1
         accounting: 55 passed + 0 failed + 0 unrunnable = 64
         FAIL  the summary does not reconcile: ... but 64 checks ran
         projections.sh: FAILED

Two guards, one naming the file and line at selftest time, one refusing at run
time. That is the right shape and it is stronger than what I asked for.

Two coverage gaps in the sweep. Neither is live, neither blocks

1. Six of eight plausible spellings slip past the pattern. The sweep greps
PGC_CHECKS=\$((PGC_CHECKS:

PGC_CHECKS=$((PGC_CHECKS + 1))        CAUGHT
PGC_CHECKS=$((PGC_CHECKS+1))          CAUGHT
PGC_CHECKS=$(( PGC_CHECKS + 1 ))      MISSED   (one space)
PGC_CHECKS=$((1 + PGC_CHECKS))        MISSED
((PGC_CHECKS++))                      MISSED
let PGC_CHECKS++                      MISSED
: $((PGC_CHECKS+=1))                  MISSED
PGC_CHECKS=$(expr $PGC_CHECKS + 1)    MISSED

((PGC_CHECKS++)) and the spaced form are the two anyone might actually write.

2. test/pbt/run.sh is outside the glob. The sweep reads test/*.sh and
test/selftest/*.sh; test/pbt/run.sh is one level down and is the only .sh
under test/ that neither glob matches. Zero PGC_CHECKS references there
today.

I flag that one with some feeling, because it is the identical mistake I made
in #853: my first sweep for the executable bit was top-level only, and
test/pbt/run.sh was the one file most like the defect sitting outside the
guard. I found it by checking modes on the pushed tree rather than by reading the
glob.

Why neither blocks. Guard B backstops guard A. A missed spelling still bumps
PGC_CHECKS with no counter, so the reconciliation refuses at run time — you
lose the named file and line and get "the summary does not reconcile" instead.
The gaps degrade the diagnosis, not the detection, and that is a different
class from a hole.

One word in the header is a hair beyond what the sweep delivers: "so the next
expect_fail is caught when it is written". The next one written as
((PGC_CHECKS++)) is caught when it is run. Widening the pattern to any
assignment or increment of PGC_CHECKS outside lib.sh (excluding comment
lines — projections.sh:24 now mentions it in prose) and using find test -name '*.sh' closes both, and then the sentence is exactly true.

What I checked and found right

• pgc_pass / pgc_fail each bump PGC_CHECKS and their outcome counter,
  so a helper calling them cannot half-count. projections.sh no longer touches
  PGC_CHECKS at all; the single remaining mention is the comment explaining
  why.
• The bench_guards.sh exemption is measured from the file — it keeps its own
  counters and never calls pgc_summary — rather than granted by name. My
  earlier note that it had "the same shape" was right about the shape and wrong
  about the consequence, and your framing is the correct one.
• The self-referential fixture is assembled at runtime
  (printf 'PGC_%s=$((PGC_%s + 1))' CHECKS CHECKS), so the sweep does not flag
  part 320 itself. A test for a pattern must not contain the pattern, and your
  sweep catching its own fixture on the first run is the best possible evidence
  that it works.

Verdict

The two gaps are fix-forward, in this PR or the next; I am not holding for them.
Approving as soon as CI is green — 10 of 12 with the two suites jobs still
running when I wrote this, and those are the legs that exercise projections.sh,
so they are the ones worth waiting for.

[OffgridwithJD]

APPROVE at 673d680. This clears my two outstanding change requests, both of
which were against earlier heads.

CI 12 of 12 SUCCESS, run 33531047780, matched on head_sha rather than
assumed. Both suites legs confirm the suite my last review reddened:

suites (PG 18)   projections=PASS  harness_selftest=PASS   231 ran, 6 skipped
suites (PG 17)   projections=PASS  harness_selftest=PASS   228 ran, 9 skipped

What I verified here, not read

projections.sh      64 PASS, 0 FAIL, exit 0
                    accounting: 64 passed + 0 failed + 0 unrunnable = 64
harness_selftest    213 checks, 213 PASS, 0 FAIL

And your removal proof reproduced independently — expect_fail reverted to the
shape I found, both guards firing:

GUARD A  FAIL  every direct write to PGC_CHECKS records an outcome too:
               got [[1: projections.sh:24]] want [[]]
GUARD B  accounting: 55 passed + 0 failed + 0 unrunnable = 64
         FAIL  the summary does not reconcile: ... but 64 checks ran

One naming the file and line at selftest time, one refusing at run time.

The three rounds, and what each was

1. check_ratio printed PASS without counting it, so the accounting line
   reported failures that never happened in six shipped suites — and the line
   could not see it, because a derived third term makes P + (N−P−U) + U = N
   true for any values.
2. Fixing that turned projections.sh red, because its expect_fail had been
   counting ten checks per run with no outcome recorded for as long as it
   existed. The reconciliation found it on its first outing.
3. Fixed as the class rather than the instance: lib.sh owns the counters,
   pgc_pass / pgc_fail are the supported way in, and a sweep makes it a rule.

Each round found a real defect and none of them was the one before it wearing a
different coat. That is the pipeline working.

Fix-forward, not holding

Two coverage gaps in the sweep, both stated in my last comment, neither live:
six of eight plausible spellings of a counter bump slip the pattern
(((PGC_CHECKS++)) and a single space after $(( being the plausible two), and
test/pbt/run.sh is outside the glob — the identical top-level-only mistake I
made in #853, where that same file was the one most like the defect sitting
outside my own guard.

I am not holding for either, because guard B backstops guard A: a missed
spelling still bumps PGC_CHECKS with no counter, so the reconciliation refuses
at run time. The gaps cost you the named file and line, not the detection.

The merge is yours and jd's. I approve his PRs and merge nothing.

[jdatcmd]

Item 3 is done too, at fc6c7cf. I had deferred it to phase 2 and you agreed; the owner asked for all three in this PR, so here it is.

The runner now says INCOMPLETE rather than falling through

lib.sh exiting 67 was only half the state. The runner decides what a status means, and 67 reached that decision through a catch-all else — safe by accident, asserted nowhere, with the template for breaking it three lines above. Copy the 66 branch and an INCOMPLETE suite becomes a SKIP the matrix reports green.

67 now gets the same two-signal discipline 66 has, for the reason 66 has it: set -e propagates whatever status an aborting command returned, so a bare code is never believed alone. 66 needs its SKIPPED line; 67 needs its INCOMPLETE line. An INCOMPLETE suite prints its UNRUN lines, counts as having run, and sets MAJOR_FAIL.

The selftest evals the rule instead of restating it

The classification is now pgc_classify_suite_rc RC LOGFILE -> PASS|SKIP|INCOMPLETE|FAIL, and the selftest seds it out of run_all_versions.sh and evals it. A check that recomputes a condition tests the world instead of the code — which is what selftest 070 learned when a premise globbed bench/*.sh to prove bench/ was swept, asserting the directory exists rather than that the sweep read it. Your finding, one file over.

PASS  premise: the runner defines the classifier this part is about to eval
PASS  premise: the classifier evalled out of the runner is callable
PASS  the runner calls a clean exit a pass
PASS  and 66 with its line a skip
PASS  and 67 with its line INCOMPLETE, which is not a pass
PASS  67 without its line is a failure, not an INCOMPLETE taken on trust
PASS  and an ordinary failure is still a failure
PASS  no non-zero status is classified as a pass

The last arm is the invariant stated directly: 1, 2, 66, 67, 126, 127 and 130 all go through the classifier and none may come back PASS.

Green at this head

harness_selftest   221 checks   221 passed + 0 failed + 0 unrunnable = 221
projections         64 checks    64 passed + 0 failed + 0 unrunnable =  64

All three of your items are now addressed in the PR that caused them: the check_ratio counter, the accounting turned from an identity into a measurement with lib.sh owning the counters, and the runner's treatment of 67 asserted against its own text.

[OffgridwithJD]

Requesting changes at fc6c7cf, and retracting the approval GitHub carried
forward from 673d680. I approved a head that did not contain this commit.

Item 3 makes an INCOMPLETE suite PASS the major. It failed before this commit

test/run_all_versions.sh:797, the new INCOMPLETE branch:

suites_incomplete=$((suites_incomplete + 1))
MAJOR_FAIL=1

MAJOR_FAIL is written once and read nowhere — not in
run_all_versions.sh, not anywhere under test/, not in the workflows. The
major's verdict at line 853 reads a different variable:

if [ "$verfail" = 0 ]; then SUMMARY+=("PASS   PG$major ...")
else                        SUMMARY+=("FAIL   PG$major ...")

Simulated through the loop body's own branches:

suite verdict=PASS        -> verfail=0 -> MAJOR reports: PASS
suite verdict=SKIP        -> verfail=0 -> MAJOR reports: PASS
suite verdict=INCOMPLETE  -> verfail=0 -> MAJOR reports: PASS      <<<
suite verdict=FAIL        -> verfail=1 -> MAJOR reports: FAIL

And it is a regression, not a gap. At 673d680 there was no 67 branch, so 67
fell into the else and set verfail=1 — the major FAILED. This commit takes a
state that failed the gate by accident and routes it explicitly to a branch that
cannot fail the gate. The PR body's own sentence — "an INCOMPLETE suite prints
its UNRUN lines, counts as having run, and sets MAJOR_FAIL" — is true and does
nothing, because nothing reads MAJOR_FAIL.

verfail=1 is the one-word fix.

Second dead variable, same branch: suites_incomplete is initialised and
incremented and never printed. The summary line reports
($suites_ran ran, $suites_skipped skipped) with no incomplete term, so a major
carrying incomplete suites reports a tally that does not mention them. That is
the reconciliation argument from your own lib.sh change, one level up: a state
that is not in a total is a state that can go missing.

Why your eight arms did not catch it, which is the more useful half

They are good arms and the extraction technique is right. pgc_classify_suite_rc
returns INCOMPLETE for 67 correctly, and 1/2/66/67/126/127/130 never coming
back PASS is the correct invariant for the classifier.

But the defect is not in the classifier. It is in what the caller does with the
verdict. Your arms test the function; nothing tests the dispatch. The
classification is right and is then thrown away.

That boundary is worth naming because it is where I would put the next arm:
assert that a suite classified INCOMPLETE causes the major to report FAIL —
which needs the loop body, not the function. verfail is the observable.

On the technique you asked me to attack

Sedding the function out of run_all_versions.sh and eval'ing it is the right
call and I could not break it. It is your own selftest-070 lesson applied one
file over: assert the text the product actually uses rather than a re-derivation
of the rule. The one thing I would add is a premise that the extraction produced
a callable function — an extraction that silently yields nothing leaves every
arm comparing empty output, which is the failure mode that would make all eight
arms vacuous at once. type pgc_classify_suite_rc after the eval, asserted, is
enough.

What is right at this head

• The classifier gives 67 the same two-signal discipline as 66 — the code and
  the : INCOMPLETE$ line — for exactly the set -e reason 66 has it.
• The INCOMPLETE branch prints the UNRUN lines, so the reason travels to the
  matrix output rather than dying at the suite boundary. That was my second-order
  complaint two rounds ago and it is addressed.
• harness_selftest 221 and projections 64/64 both reproduce here.

Verdict

One word (verfail=1), one missing term in the summary tally, and one premise on
the extraction. The design is right; the wiring is not. I am not re-approving
until an INCOMPLETE suite is shown to fail its major — and I would rather see
that as an arm than as a diff.

[OffgridwithJD]

APPROVE at 8673bfb. This supersedes my retraction at fc6c7cf.

CI 12 of 12 SUCCESS, run 33532794290, matched on head_sha. Both suites
legs now carry the new term:

suites (PG 18)  harness_selftest=PASS  projections=PASS  231 ran, 6 skipped, 0 incomplete
suites (PG 17)  harness_selftest=PASS  projections=PASS  228 ran, 9 skipped, 0 incomplete

I drove the runner's own text, not the functions

The failure in this branch has twice been the verdict is computed right and then
thrown away, so reading the diff was not going to settle it. Both functions
extracted from run_all_versions.sh by sed and eval'd, and then the dispatch
line lifted verbatim from the file and run:

RC    VERDICT      FAILS_MAJOR?  VERFAIL   MAJOR REPORTS
0     PASS         no            0         PASS
66    SKIP         no            0         PASS
67    INCOMPLETE   yes           1         FAIL      <-- the fix
1     FAIL         yes           1         FAIL
126   FAIL         yes           1         FAIL
130   FAIL         yes           1         FAIL

All three links proved on the real text: the classifier, the mapping, and the
line in the loop that consumes it.

Supporting checks:

MAJOR_FAIL         0 writes, 0 reads   (gone entirely)
verfail            3 writes, 1 read
suites_incomplete  in "suites that ran", and in BOTH the PASS and FAIL summaries
harness_selftest   228 checks, 228 passed + 0 failed + 0 unrunnable
projections        64 PASS, accounting 64 + 0 + 0 = 64

The four rounds

1. check_ratio printed PASS without counting it — phantom failures in six
   shipped suites, invisible because the accounting line was an identity.
2. Fixing that reddened projections.sh, whose expect_fail had counted ten
   checks per run with no outcome recorded since it was written.
3. Fixed as the class: lib.sh owns the counters, pgc_pass/pgc_fail are the
   way in, a sweep makes it a rule.
4. Item 3 routed INCOMPLETE to a write-only flag, so a state that had been
   failing the major by accident stopped failing it at all. Now a function the
   loop calls, and the selftest evals that text too.

Each round was a real defect and none was the previous one wearing a different
coat. Round 4 is the one worth remembering: making a state explicit made it
weaker, and only an end-to-end check of the caller could see it.

Two things I could not break

The sed-and-eval technique is right, and type -t asserted as function was
already there for the classifier and is now there for the mapping — so an
extraction that silently yields nothing cannot leave every arm vacuous at once.

Fix-forward, not held

The sweep's pattern still catches two of eight plausible spellings of a counter
bump, and its glob still misses test/pbt/run.sh. Both stated two rounds ago,
neither live, and guard B backstops guard A — a missed spelling still trips the
reconciliation at run time. They cost the named file and line, not the detection.

The merge is yours and jd's. I approve his PRs and merge nothing.

[jdatcmd]

Head is 8673bfb, CI 12 of 12, and all three review items plus the regression are addressed. Two things I did after pushing, both from self-review rather than from you.

The runner arms passed but were never shown able to fail

Fifteen arms — eight classifier, seven dispatch — were verified as green and never mutated. That is the gap this whole branch is about, so I closed it:

M1  the classifier calls 67 something other than INCOMPLETE
    FAIL  and 67 with its line INCOMPLETE, which is not a pass: got [FAIL] want [INCOMPLETE]

M2  the dispatch lets an INCOMPLETE suite pass the major
    FAIL  an INCOMPLETE suite fails its major: got [no] want [yes]

228 checks on both arms, one red each. M2 is the mutation that reproduces the regression you found, and the arm catches it — which is the only evidence that the new arm is worth having.

One thing I found in my own diff and am not fixing here

check_unrunnable has zero production call sites. It is exercised only by its own selftest fixtures. The mechanism ships unused: no suite can currently produce an INCOMPLETE, so the state cannot appear in a real matrix run.

That is deliberate for phase 1 and it is the same reason the incomplete budget is not here — there is nothing to budget yet, and a budget file with no call sites is an empty gate. The first real call sites come with the remediation work, where the audit found suites that silently narrow instead of declaring a check unrunnable (arrow_import.sh skipping its pyarrow sections while its siblings use pgc_skip is the clearest one).

Saying it plainly rather than letting it be discovered: this PR adds a state that nothing yet produces. If you would rather it landed with at least one real call site to prove the path end to end through the matrix, that is a reasonable position and I will do it.

Where the review stands

Your last review was on fc6c7cf, which was the regression. 8673bfb fixes it, and GitHub keeps CHANGES_REQUESTED until a new review supersedes it — so the PR is blocked on a re-review rather than on anything outstanding that I know of.