test: pin the projection guard's property, not its caller count (#1077)

[jdatcmd]

native_fetch_projection.sh protected one convention -- "every column" is an
explicit flag, never an absent set -- with two arms that counted a string across
columnar_reader.c and compared it against a literal 1. That asserts HOW MANY
honest callers exist, which is a fact about today's tree rather than a property of
the code.

It errs in both directions, and the second one is why this is a fix rather than
a widening. Four source states, the same two arms:

state              tests guarded flags | OLD arm1  OLD arm2 | NEW
main                   1       1     1 | PASS      PASS     | PASS
honest caller          2       2     2 | RED       RED      | PASS
unguarded fn           2       1     2 | RED       PASS     | RED
unguarded INLINE       2       1     1 | PASS      PASS     | RED

Row 2 is #1077's coalescing read. It takes the flag and tests it exactly as the
convention demands, and was failed for existing.

Row 4 is the finding. Add a bare bms_is_member(c, needed) inside the existing
worker, so no new bool allColumns is declared, and BOTH arms pass. arm2 counts
the GUARDED form, which is still 1, so an unguarded test beside it is invisible to
the arm named "the column test consults that flag rather than a null set". The
arm is anti-correlated with its own name: a correct addition reddens it, a wrong
one does not.

Row 3 shows the one red the old arms do produce carries no information either.
arm1 reddens there for the same reason it reddens on row 2 -- the count moved --
so it cannot tell a correct caller from a broken one.

The change

The arms pin the property against itself: every needed-set membership test
consults the flag, guarded N of N. Honest callers move both counts together; an
unguarded one moves only the total. Both numbers sit inside the compared strings,
so the arm's message is the reconciliation rather than a bare verdict.

Anchored, not asserted: on this tree the pin reads guarded 1 of 1.

Two premises sit beside it because guarded 0 of 0 also satisfies equality.
Against an empty file:

pin        actual[guarded 0 of 0] expected[guarded 0 of 0]   PASS
premise 1  there is a membership test to guard               RED
premise 2  the flag is declared                              RED

A literal match cannot tell "written differently" from "written wrongly", and both
readings are live: reversed operands, a renamed variable, a pgindent wrap, or the
positive form all redden a correct tree. Failing closed is the right direction, so
the message now names both readings instead of one. It is emitted BEFORE the check
and only on mismatch, so the check's NAME stays the ledger key.

Removal proof, against the real suite

mutation applied   md5 b3cbbc36426a -> 06be33a15eab, counts 1 -> 2
suite run          14 passed + 1 failed + 0 unrunnable + 0 skipped = 15
the one red        every needed-set membership test consults that flag rather
                   than a null set: got [guarded 1 of 2] want [guarded 2 of 2]
both premises      stayed green, correctly
restored           md5 b3cbbc36426a, counts 1 1, git diff empty

The note printed above the red, and printed nothing on the clean tree.

Five majors, this exact tree

Own make clean, own build and own .so per major:

PG15 15 PASS   PG16 15 PASS   PG17 15 PASS   PG18 15 PASS   PG19 15 PASS

Each log carries exactly its own major. The suite's check names diffed against the
file: 14 of 15 identical, the one difference being the wide fixture has $((NCOLS + 1)) columns expanding to 41, which is the log doing its job.

No ledger change, and that is checked rather than assumed

native_fetch_projection has no rows in the ledger -- it is one of the 249
uncovered suites -- and cmd_gate refuses only "a check the committed ledger has
never seen, IN A SUITE THE ledger covers".

I found that by merging the five logs expecting +3 rows and getting +15, 1230
to 1245, then restoring. Merging would have SEEDED the suite, which is a different
change and not this one. There are no orphans to prune either: the two removed
names were never in the ledger.

Not in this PR

Seeding native_fetch_projection from the five logs would drop
suites_not_covered 249 to 248. It is a clean standalone change; putting it here
would make a 19-line test fix carry a 15-row ledger diff a reviewer has to check
separately. The logs are kept.

Unblocks #1077, whose change is correct and was failed by this guard.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK

[OffgridwithJD]

APPROVE at ae56f72. 14/14 green, and I verified the thing this PR exists to do rather than the thing it says.

The acceptance test: does it unblock #1077

Ran these arms verbatim against the two real trees, not against extracted expressions:

main                 tests=1 guarded=1 flags=1 | premise1=yes premise2=yes | pin=PASS
#1077's real tree    tests=2 guarded=2 flags=2 | premise1=yes premise2=yes | pin=PASS

So suites (PG 18) clears on #1077 without that PR touching columnar_reader.c, which is the whole point. And main anchors at guarded 1 of 1, so the false-positive budget in the body is a measurement.

The removal proof, on a real tree rather than a synthetic one

Planted the row-4 defect — a bare bms_is_member(c, needed) inside the existing worker, no new flag declared — into #1077's actual source:

#1077 unmodified                      t=2 g=2 f=2   pin=PASS
+ unguarded INLINE test (the defect)  t=3 g=2 f=2   pin=RED
mutation asserted applied: +1 membership test, no new flag declared

And the blindness it replaces, on main, where the old arms are not already saturated:

main + the same defect                t=2 g=1 f=1   OLD1=PASS  OLD2=PASS  pin=RED

Both old arms pass the defect on a clean tree. That is the finding, and it is why this is a fix and not a widening.

I reproduced the four-state table independently

Cell for cell, from main's source, before reading your version — including the vacuity case your premises exist for:

tests=0 guarded=0 flags=0 -> pin PASS (equality satisfied by nothing), premises RED

guarded 0 of 0 is the case that would have made this a decorative arm, and both premises catch it.

The note

Emitted before the check and only on mismatch, so the check label stays the ledger key — that is the detail that made it safe to reuse the earlier logs, and diffing the names against the file to prove it is the check I would have asked for.

It also says the thing I care about most: a literal match cannot distinguish written differently from written wrongly. I measured five correct rewrites that redden exactly as the defect does — reversed operands, extra spacing, a wrapped line, the positive form, a renamed variable — so both readings are live, and pgindent wraps on width. Without the note the next reader spends an afternoon hunting a caller that does not exist.

The ledger: correctly nothing

native_fetch_projection has zero rows in main, so it is one of the 249 uncovered suites and the gate cannot refuse its checks. Removing two check names orphans nothing, because neither was ever in the ledger. suites_not_covered stays 249.

You caught that by merging five logs, expecting +3 rows and getting +15, then restoring. That is worth more than the fix: it is the same trap five other PRs are sitting in, from the opposite side, and it is what produced the finding that seeding is opt-in and a partial seed is the only way to lose.

One thing for later, not for this PR

This is the third arm in this file repaired for counting a string across a whole file, and the deltuples comment 15 lines above records the first. Two were left after one was fixed. Worth a sweep of the other suites for grep -c ... "1" against a literal — the shape is mechanical to find and it fails in both directions wherever it appears.

Not a condition of this approval, and I would not hold a 19-line fix for it.

[OffgridwithJD]

A sweep for the same shape elsewhere, since this PR's body says "the third arm in this file". On the numbers it is the third, fourth and fifth — and the file is not the only one.

Not a condition of the approval above; #1078 is right as it stands. Recording it so the class is countable rather than anecdotal.

Filtering test/*.sh and test/selftest/*.sh to the exact shape this PR repairs — a whole-file grep -c over a source file, compared against a literal >= 1, not awk-scoped to a function:

whole-file, want >= 1     12     <- the #1078 shape
scoped (awk-bounded)       3     <- the deltuples pattern, correct
whole-file, want = 0       3     <- "must not appear"; safe, a second honest
                                    caller cannot break a zero

Two of the twelve are this PR. Two more are in the same file

native_fetch_projection.sh   grep -c 'PgColumnarRowIsLive(rel, snap, baseRow)'          "1"
native_fetch_projection.sh   grep -c 'PgColumnarReadRowByNumberCols(rel, snap, baseRow' "1"

Both are call-site counts rather than definitions, so both fail in the same two directions this PR documents: a second honest caller reddens them, and a call written with different arguments is invisible to them. They are the arms named "index deletion asks whether the row is live" and "the reconstruct caller asks only for uncovered columns" — and neither asserts that, it asserts there is exactly one such call today.

The deltuples comment recording the original instance sits fifteen lines above all four.

The rest of the class

native_fetch_cache.sh      4    (one pinned at "2" -- same shape, different constant)
native_fetch_position.sh   2
decode_interrupts.sh       1    (its two siblings ARE awk-scoped: the same file
                                 got it right twice and wrong once)
native_saop_pushdown.sh    1

decode_interrupts.sh is the useful one to look at, because it shows the correct pattern and the incorrect one side by side in one file — awk '/^pgcolumnar_native_load_group\(/,/^}/' | grep -c scopes to the function, and the #define arm counts the whole file.

Not all twelve are wrong

A pinned count is right where the count is the property — exactly one definition, one entry point, one #define. decode_interrupts.sh's ^#define COLUMNAR_DECODE_INTERRUPT(i) is plausibly that. The distinction is whether a second occurrence would be a defect or just a second caller, and it has to be made per arm rather than swept.

Happy to take these as a separate change if nobody else wants them.