compare_to_bash.py cannot read 203 names in 25 suites that define their own recorder (#432)

[OffgridwithJD]

compare_to_bash.py reads the recorders lib.sh shares. #1052 widens that list from spelling to behaviour. The same bug exists one scope down, in the arm three lines below it: 25 suites define their own recorder, and test_the_suite_local_helpers_are_known_and_excluded derives them by the name check_*.

This is not the same as #1051. There the fix was a table entry, because lib.sh is one file every suite shares. Here each suite has its own, so the question is what the extractor should do about a name it can only find by reading the suite's own definitions.

The measurement

203 names across 25 suites, derived by behaviour (a function forwarding a bare positional into a known recorder's name slot):

suite	local recorders	read	unread	total
phase6	eq_on_off	4	39	43
native_groupagg	oracle, toggle_diff	37	21	58
sorted_pathkeys	ans, ansp	94	19	113
native_agg_addcolumn	both	2	16	18
alter_column_type	conv	8	15	23
native_groupagg_batch	agree, agree_in, tog	37	9	46
projections	expect_fail	68	9	77
vector_agg_tlist_shape	ansq	21	9	30

and 17 more with 1-7 each. phase6 is 91% unread.

The two populations overlap; neither contains the other

by spelling (check_*, what the arm uses today)	4 helpers in 2 suites
by behaviour (forwards a bare positional)	27 helpers in 25 suites

check_float is in both. parallel_copy.sh's three are found only by spelling, because they compose the name rather than forward it:

check_structure()  ->  check "$label: offsets well-formed and newline-aligned" "$ok" 1

The extractor reads {}: offsets well-formed and newline-aligned as a template out of parallel_copy.sh itself, so the SHAPE is visible while the 12 labels the suite supplies are not. That is a third shape and a behaviour sweep looking for a bare "$1" correctly rejects it. Any fix has to decide about it deliberately rather than inherit whichever sweep was written first.

Why this gates #432's queue

sorted_pathkeys.sh is the next port at 113 names, and the 19 it loses are not a random 17%. The suite pairs every plan assertion with an answer assertion, and says why:

check "ORDER BY the sort key plans no Sort" "$(sorts 'SELECT k FROM c ORDER BY k')" "no"
ans   "and returns the same rows in the same order as heap" \
      'SELECT id, k, j FROM %T ORDER BY k, j, id'

The ordering is real on these shapes, so the Sort must be gone. Each is paired with the answer, because losing the Sort is only correct if the rows still come back in that order.

Counted:

readable   (check*)   24  plan assertions -- "plans no Sort", "REFUSE: ..."
unreadable (ans/ansp) 18  answer arms     -- "and ... still answers correctly"

The grader would see every claim about the PLAN and none about the ANSWER. A port that dropped all 18 would grade one-for-one. The answer arms are the half that catches a wrong answer rather than a slow plan, which is the more serious of the two defects and the reason the suite has them.

sorted_pathkeys.sh already knows, at line 280:

Two of them go through ansp, which records under its first argument -- they are arms like any other and were missed by a sweep that looked only for check.

A different sweep hit this exact bug in this exact suite, and the note stayed local to the skip loop.

Shape, if anyone takes it

Three options, and I do not think it is mine to pick:

1. Read them. Derive each suite's own recorders the way test: the grader could not read a name passed through a lib.sh wrapper (#1045) #1052 derives lib.sh's, and extract through them. Most complete, and it means the extractor parses the suite's function definitions rather than matching a fixed list.
2. Refuse them. Have the grader FAIL on a suite whose names it cannot fully read, naming the helper, so a pair cannot be declared complete on a partial reading. Turns a silent gap into a loud one without teaching the tool anything new.
3. Declare them. Leave the extractor alone and require any such pair to carry an INCOMPLETE reason naming the unread helper.

1 and 2 are complementary rather than alternatives: 2 is the guard that makes 1's coverage checkable. I would rather this be decided than have me implement one half and close the question, which is the note @jdatcmd made on #1048.

Measured on bf31e2f plus #1052. Scripts are reproducible from the derivation #1052 adds; the per-suite numbers come from running it per file with lib.sh's recorders subtracted.

🤖 Generated with Claude Code

https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs

[OffgridwithJD]

Correcting my own numbers before building: 147 in 14 suites, not 203 in 25

@jdatcmd asked me to take this. Measuring the refuse half's cost first turned up that the issue above over-counts, because it treats two different wrapper shapes as one. They are not, and only one of them is a gap.

COMPOSE is already covered

native_ownership.sh   refused()  ->  check "non-owner refused: ${1%%(*}"

_BASH_INTERP reduces ${1%%(*} to {}, so the grader reads non-owner refused: {} straight out of the wrapper's own definition, and that one template covers all 9 call sites. Measured on four of them:

suite	helper	what the grader reads from the definition	call sites
native_ownership	refused	non-owner refused: {}	9
hilbert_locality	crun	premise: {} ran without raising	5
parallel_copy	check_structure	{}: offsets well-formed and newline-aligned	5
sorted_pathkeys	ansp	{}	10

The first three are real templates naming a real property. That is why native_ownership grades missing: 0 today and why my port of hilbert_locality matched without my noticing this existed. No gap, and nothing to refuse.

FORWARD is the gap, and it is narrower

ansp passes its $1 straight through, so the definition yields a bare {} — a "name" that names nothing. Recounted with composers excluded:

	issue above	measured
suites	25	14
names unread	203	147
graded pairs affected	—	0

phase6                  eq_on_off              39      row_triggers           both             7
sorted_pathkeys         ans, ansp              19      native_groupagg_batch  agree, agree_in  6
native_agg_addcolumn    both                   16      wal_envelope           ordered          6
alter_column_type       conv                   15      audit                  expect_error     5
projections             expect_fail             9      phase5                 assert_plan      5
vector_agg_tlist_shape  ansq                    9      parallel_export_parquet expect_error    2
phase4                  assert_plan, expect_fail 7     parquet_export_stats   check_float      2

All 14 are registered suites. None is a graded pair, so reading them changes no current verdict — which makes this safe to land either way round.

A sub-finding: 17 bare {} names are published today

A forwarding wrapper does not contribute nothing. It contributes {}, and the corpus carries 17 of them. That is a bash "property" with no content, and it is not inert:

port name 'a completely unrelated property'   matched by the bare template: False
port name '{}'                                matched by the bare template: True
port name '{} rows agree'                     matched by the bare template: False

Narrow — it only matches a port name that is entirely one f-string — but it is a spurious match in the one direction that matters, and the other 16 sit in the MISSING column naming nothing a port could ever assert. Same family as pgc_skip reading a capability in #1051: a wrong name is worse than an absent one.

What I propose, following @jdatcmd's read-and-refuse

1. Read a suite's own forwarding wrappers, derived by the rule that already works for lib.sh — a function reaching a recorder transitively with a bare positional in the name slot, seeded from pgc_record. No declaration list: the population is defined by what a function does. That is what handles ans (a one-line body, the skip-loop-arms.py reads a one-line function body as everything below it, so a psql wrapper counts as a check recorder (latent, A/B'd) #1042 shape) and ansp (forwards through a local), and it is why a first detector returns NONE for sorted_pathkeys.
2. Refuse a suite whose helper reaches a recorder with a name position that cannot be resolved at all. Silently skipping is how this got here.
3. Drop the bare {} rather than publish it. A forwarding wrapper's definition states no property; its call sites do.
4. Do not refuse composers. They are readable and correct today, and refusing them would turn hilbert_cluster, hilbert_locality and native_ownership — three pairs that are COMPLETE right now — into refusals for no defect. That was the cost of the version I had in my head before measuring, and it is the reason I measured first.

Point 4 is the one worth the paragraph. The refuse half is right in principle and, aimed at the wrong population, it would have broken three green pairs to fix nothing.

Measured on d1ff6e4 (#1054's merged tree). Taking this unless @jdatcmd would rather split it.