From b3433bed56050d48eef1bf9b074be4d67ad89943 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Sun, 13 Sep 2026 17:24:44 +0000 Subject: [PATCH 1/3] test: an unrunnable record names the check it stands in for (#1040) Two of four check_unrunnable calls in one loop in hilbert_locality.sh carried a shortened name while the other two matched their runnable twins exactly: :574 check_unrunnable "box $box: groups read, Z-order" :597 check_num "box $box: groups read over $PLACEMENTS placements, Z-order" :578 check_unrunnable "box $box: Hilbert reads fewer groups than Z-order" :602 check_text "box $box: Hilbert reads fewer groups than Z-order" So the property had TWO ledger keys and which one appeared depended on whether that box's two partitions came out different that day. The key was a function of the data. hilbert_locality is not ledger-covered, so this was a wrong key waiting to be seeded, not a wrong number today. THE CONVENTION IS ALREADY NEAR-UNIVERSAL, which is what made the lapse invisible: 23 of 25 check_unrunnable call sites carry the runnable name (hilbert_cluster 9 of 9, projection_rewrite 11 of 11 counting its pgc_pass twin). A shorter name in a new refusal branch reads as ordinary, and both halves of a two-branch site are rarely read together. AND A GUARD, because a rename in either branch reintroduces it silently. Selftest 480 drives .github/scripts/unrunnable-arm-names.py over test/*.sh. THE TOOL DERIVES THREE THINGS A LIST OF THEM GOT WRONG FIRST: which functions record from the pgc_record call in the body. Two hand-written lists omitted pgc_pass, and projection_rewrite then read as a false orphan. which argument is the pgc_skip records "$2": argument one is the name CAPABILITY. Reading it takes `arrow` where the check is called `arrow support is present` -- the bash mirror of the defect #1036 and #1038 closed on the python side, 22 call sites. pgc_require_tools records a FIXED name. what is a refusal from the verdict recorded, not the spelling. check_skip is deliberately NOT swept. A skipped arm has no runnable counterpart by construction -- the name IS the arm -- so 21 of its 23 call sites have no twin and always will; sweeping it would report 21 mismatches that are all correct code. THE PART CARRIES A POSITIVE CONTROL, because the guard's steady state is zero mismatches and a broken sweep reports zero too. It drives the real tool over a fixture whose refusal branch names something the file never records, and over a control where the names agree. Three arms pin values that were WRONG in a draft of the tool rather than restating the code: the refusal set, pgc_skip's argument TWO, and pgc_pass and pgc_fail being twins. AND THE CENSUS RECIPE IN check_ledger_budget.txt READ FIELD FOUR AND RETURNED ZERO. #1010 inserted the majors as field 4, moving the last-red to field 5. The CHANGELOG entry for #1010 says this file carries the corrected form; it did not. The gate is unaffected -- it computes the census itself -- so the harm is a reviewer re-deriving 0 against a stated number and "correcting" a budget that was right. Fixed here rather than filed because this change moves that very number. Verified, red then green with the rename as the only variable between the two runs: before the rename harness_selftest RC=1, 943 checks, exactly one FAIL, naming both offending sites; all 8 other arms of the new part green, including the positive control after the rename harness_selftest RC=0, 943 checks sweep 25 sites, 24 compared, 1 dynamic, 0 mismatches shellcheck -S error over test/*.sh test/selftest/*.sh clean docs_style 14/14 ledger rows 1197 -> 1206 (+9, the new part), census 1189 -> 1198 re-derived by the corrected command, ceiling 249 -> 249, orphans 0, gate rc=0 against db74d9e widened extractor all 7 ported pairs rc=0 missing=0, which is the one red #1040's grader half would otherwise land with Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs --- .github/scripts/unrunnable-arm-names.py | 300 ++++++++++++++++++ CHANGELOG.md | 56 ++++ test/check_ledger.tsv | 9 + test/check_ledger_budget.txt | 13 +- test/hilbert_locality.sh | 4 +- ...80-an-unrunnable-record-names-its-check.sh | 109 +++++++ test/selftest/parts.manifest | 1 + 7 files changed, 488 insertions(+), 4 deletions(-) create mode 100644 .github/scripts/unrunnable-arm-names.py create mode 100644 test/selftest/480-an-unrunnable-record-names-its-check.sh diff --git a/.github/scripts/unrunnable-arm-names.py b/.github/scripts/unrunnable-arm-names.py new file mode 100644 index 00000000..27bf7527 --- /dev/null +++ b/.github/scripts/unrunnable-arm-names.py @@ -0,0 +1,300 @@ +#!/usr/bin/env python3 +"""Every unrunnable record must name the check it stands in for. + +#1040. `check_unrunnable NAME REASON DETAIL` gives one check the honesty +`pgc_skip` gives a whole suite: the check did not run, and the reader is told +WHICH. That only works if the record carries the name the check uses when it DOES +run. Where the two spellings differ the property has two ledger keys, and which +one appears depends on runtime state -- on `hilbert_locality` it depended on +whether that box's two partitions came out different that day. + +This is part 470's property one helper over. 470 asks whether a skip LOOP still +names the arms its sibling branch emits; this asks whether an unrunnable record +names a check the same file asserts anywhere. + +`check_skip` IS NOT SWEPT, and that is a finding rather than an omission. A +skipped arm has no runnable counterpart by construction -- the name IS the arm -- +so 21 of its 23 call sites have no twin and always will. Sweeping it would report +21 mismatches that are all correct code. For the same reason a SKIP record is not +a twin: neither side ran. + +WHAT IS AND IS NOT COMPARED, printed rather than assumed: + + sites every name passed to a refusal emitter + dynamic the whole name is one shell expansion, so there is no literal to + compare (`arms_unrunnable` reads its names from a list) + compared the rest, where a missing twin is a real finding + +`dynamic` is printed for the reason 470 prints `interpolated`: a comparison +nobody makes and a comparison that passes are indistinguishable in a total. + +THREE THINGS THIS TOOL DERIVES RATHER THAN LISTS, each because a list of it was +wrong first: + +1. WHICH FUNCTIONS RECORD, from the `pgc_record` call in their body. Two + hand-written lists gave two wrong answers on this question. One omitted + `pgc_pass`, and `projection_rewrite.sh` then reported a false orphan because + its runnable twin records through `pgc_pass` and not a `check_*` helper. + +2. WHICH ARGUMENT IS THE NAME. `pgc_skip ` records + `pgc_record FAIL "$2"`: the first quoted argument is the CAPABILITY, not the + name. A reader assuming argument one takes `arrow` where the check is called + `arrow support is present` -- the bash-side mirror of the name-position defect + #1036 and #1038 closed on the python side. 22 call sites. + `pgc_require_tools` records a FIXED name and takes no name argument at all. + +3. WHAT COUNTS AS A REFUSAL, from the verdict recorded (`UNRUN`), not from the + helper's spelling. + +THE TWO SETS ARE ASYMMETRIC ON PURPOSE, because their failure directions are. +An over-broad REFUSAL set sweeps a site that did not need sweeping: extra work, +visible. An over-broad TWIN set supplies a name nothing records and turns a real +mismatch green: silent. So refusal wrappers are followed transitively, while the +twin set is restricted to `lib.sh` and to suite-local definitions spelled +`check` or `check_`. `check[a-z_]*` is NOT that spelling: it also matches +`checks_in` in `decode_interrupts.sh`, a counting utility that records nothing +(@jdatcmd). A population named after a prefix is not a population named after a +behaviour. + +AND THE SUBJECT MUST NOT BE IN THE REFERENCE SET. The first sweep of this +property subtracted the unrunnable names from the runnable set -- removing the +very names it was looking up -- so every name could only report absent and a +nine-for-nine file read as nine orphans. `--show-emitters` prints both sets so +the separation is inspectable rather than asserted. +""" + +import pathlib +import re +import sys + +_HELPER_DEF = re.compile(r"^([a-z_][a-z0-9_]*)\(\)\s*\{", re.M) +_SUITE_LOCAL = re.compile(r"^check(?:_[a-z_]+)?$") +_HEREDOC = re.compile(r"<<-?\s*'?\"?([A-Za-z_][A-Za-z0-9_]*)'?\"?\s*$") +_BARE_VAR = re.compile(r"^\$\{?([A-Za-z_][A-Za-z0-9_]*|[0-9]+)\}?$") + + +def blank_heredocs(lines): + """-> the lines with heredoc BODIES blanked out. + + A fixture written into a heredoc is text, not shell. Kept because + `skip-loop-arms.py` needs it on this corpus; `--keep-heredocs` runs without + it so the part can show the answer does not depend on it. + """ + out, term = [], None + for l in lines: + if term is None: + m = _HEREDOC.search(l) + out.append(l) + if m: + term = m.group(1) + else: + out.append("") + if l.strip() == term: + term = None + return out + + +def bodies(text): + """-> [(name, body)] with the body ended by ITS OWN closing brace. + + `text.find("\n}")` is wrong for a one-line definition: `q() { psql ...; }` in + `audit.sh` closes on its own line, so a search for a brace at column zero ran + on to the NEXT function's and swallowed every `check` call in between. `q` + then appeared in the twin set -- a psql wrapper offering SQL text as check + names. Depth is counted per line from the opening brace. + """ + out = [] + lines = text.splitlines() + for i, l in enumerate(lines): + m = _HELPER_DEF.match(l) + if not m: + continue + depth = l.count("{") - l.count("}") + body = [l[m.end():]] + j = i + 1 + while j < len(lines) and depth > 0: + depth += lines[j].count("{") - lines[j].count("}") + body.append(lines[j]) + j += 1 + out.append((m.group(1), "\n".join(body))) + return out + + +def split_args(line, fn): + """-> the arguments of `fn` on this line, as written. + + A quoted argument yields its contents; an unquoted one yields the word. The + POSITION is what matters, so counting only quoted strings would misread + `pgc_skip $cap "the message"`. + """ + m = re.match(r"^(?:.*?(?:;|&&|\|\||\||&))?\s*%s\s+(.*)$" % re.escape(fn), line) + if not m: + return None + rest, args, buf, q, esc = m.group(1), [], "", None, False + for ch in rest: + if esc: + buf += ch; esc = False; continue + if ch == "\\": + esc = True; continue + if q: + if ch == q: + q = None + else: + buf += ch + continue + if ch in "\"'": + q = ch; continue + if ch.isspace(): + if buf: + args.append(buf); buf = "" + continue + if ch in "#;&|" and not buf: + break + buf += ch + if buf: + args.append(buf) + return args + + +def _resolve(expr, body): + """-> the 1-based argument position the name comes from, or None. + + `pgc_record PASS "$1"` is position 1. `check()` writes `local name="$1"` and + then records `"$name"`, so a one-step assignment is followed. + """ + m = _BARE_VAR.match(expr.strip()) + if not m: + return None + tok = m.group(1) + if tok.isdigit(): + return int(tok) + a = re.search(r"\b%s=\"\$\{?([0-9]+)\}?\"" % re.escape(tok), body) + return int(a.group(1)) if a else None + + +def recorders(text, lib): + """-> (refusal, twins): {fn: name-position-or-None}, by the verdict recorded. + + None as a position means the function records a FIXED name, which still + contributes that literal (`pgc_require_tools`) but takes no name argument. + """ + defs, in_lib = [], set() + for src, is_lib in ((lib, True), (text, False)): + for name, body in bodies(src): + defs.append((name, body)) + if is_lib: + in_lib.add(name) + + refusal, twins, fixed = {"check_unrunnable": 1}, {}, {} + for _ in range(2): + for name, body in defs: + eligible = name in in_lib or _SUITE_LOCAL.match(name) + for verdict, expr in re.findall(r"pgc_record\s+(\w+)\s+\"([^\"]*)\"", body): + pos = _resolve(expr, body) + if verdict == "UNRUN": + refusal.setdefault(name, pos) + elif verdict == "SKIP": + pass # neither side ran; not a twin + elif eligible and name not in refusal: + # A REFUSAL EMITTER IS NEVER ITS OWN TWIN. `check_unrunnable` + # records FAIL when the reason code is not one of the closed + # list -- a harness-misuse record, not the check running. Left + # in the twin set it also blocked wrapper detection, because a + # wrapper's body naming `check_unrunnable` then looked like a + # body naming a runnable helper. + if pos is None and "$" not in expr: + fixed.setdefault(name, expr) + twins.setdefault(name, pos) + # A wrapper that reaches a refusal emitter and no recorder is itself a + # refusal: over-broad on the safe side. + if name not in refusal and not re.search(r"\bpgc_record\b", body): + if any(re.search(r"\b%s\b" % re.escape(r), body) for r in refusal): + if not any(re.search(r"\b%s\b" % re.escape(t), body) for t in twins): + refusal[name] = None + # A helper that DELEGATES its name reaches the twin set through the + # delegate, whether or not it also records directly: `check_timing` + # records SKIP itself and passes the same name on to `check`. + if name not in refusal and name not in twins and eligible: + if True: + for t, tpos in list(twins.items()): + c = re.search(r"\b%s\s+(.*)" % re.escape(t), body) + if c and tpos: + args = split_args("\t" + t + " " + c.group(1), t) or [] + # `check_ratio "$@"` forwards every argument, so the + # delegate's name position IS this one's. Without this + # `check_ratio_needs_quiet_machine` was absent from the + # twin set for no reason a reader could see, and an + # unexplained hole in a guard's own population is the + # thing the guard is supposed to be better than. + if args[:1] == ["$@"]: + twins.setdefault(name, tpos) + elif len(args) >= tpos: + p = _resolve(args[tpos - 1], body) + if p: + twins.setdefault(name, p) + break + for k in refusal: + twins.pop(k, None) + return refusal, twins, fixed + + +def names_for(lines, emitters): + """-> [(name, lineno)] taking each emitter's OWN name argument, not argument one.""" + out = [] + for i, l in enumerate(lines): + for fn, pos in emitters.items(): + if pos is None: + continue + args = split_args(l, fn) + if args and len(args) >= pos: + out.append((args[pos - 1], i + 1)) + break + return out + + +def main(testdir, show_emitters=False, keep_heredocs=False): + root = pathlib.Path(testdir) + files = sorted(root.glob("*.sh")) + lib = (root / "lib.sh").read_text(errors="replace") if (root / "lib.sh").exists() else "" + + sites = dynamic = compared = 0 + bad = [] + all_ref, all_twin, all_fixed = {}, {}, {} + for f in files: + text = f.read_text(errors="replace") + lines = text.splitlines() + if not keep_heredocs: + lines = blank_heredocs(lines) + refusal, twins, fixed = recorders(text, lib) + all_ref.update(refusal); all_twin.update(twins); all_fixed.update(fixed) + unrun = names_for(lines, refusal) + if not unrun: + continue + seen = {n for n, _ in names_for(lines, twins)} | set(fixed.values()) + for name, lineno in unrun: + sites += 1 + if _BARE_VAR.match(name): + dynamic += 1 + continue + compared += 1 + if name not in seen: + bad.append(f"{f.name}:{lineno} {name}") + + print(f"files {len(files)}") + print(f"sites {sites}") + print(f"dynamic {dynamic}") + print(f"compared {compared}") + for b in bad: + print(f"MISMATCH {b}") + if show_emitters: + print("refusal " + " ".join(f"{k}:{v}" for k, v in sorted(all_ref.items()))) + print("twins " + " ".join(f"{k}:{v}" for k, v in sorted(all_twin.items()))) + print("fixed " + " ".join(sorted(all_fixed))) + return 0 + + +if __name__ == "__main__": + args = [a for a in sys.argv[1:] if not a.startswith("--")] + sys.exit(main(args[0] if args else "test", + show_emitters="--show-emitters" in sys.argv, + keep_heredocs="--keep-heredocs" in sys.argv)) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ed4c46c..efc1471e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,62 @@ true until the next version shipped. ### Added +- An unrunnable record names the check it stands in for, so a refused check keeps + one ledger key instead of two (#1040). + + `check_unrunnable NAME REASON DETAIL` gives one check the honesty `pgc_skip` gives a + whole suite: the check did not run, and the reader is told which. That only works if + the record carries the name the check uses when it DOES run. Two of the four sites in + one loop in `hilbert_locality.sh` carried a shortened name: + + :574 check_unrunnable "box $box: groups read, Z-order" + :597 check_num "box $box: groups read over $PLACEMENTS placements, Z-order" + + while the other two matched their runnable twins exactly. So the property had two + ledger keys and which one appeared depended on whether that box's two partitions came + out different that day -- the key was a function of the data. `hilbert_locality` is not + a ledger-covered suite, so this was a wrong key waiting to be seeded rather than a + wrong number in `check_ledger.tsv`. + + The convention is already near-universal, and that is what made the lapse invisible: + 23 of the 25 `check_unrunnable` call sites in `test/*.sh` carry the runnable name + (`hilbert_cluster` 9 of 9, `projection_rewrite` 11 of 11), so a shorter name in a new + refusal branch reads as ordinary. Both halves of a two-branch site are rarely read + together. + + A new selftest part asserts the property over the whole corpus, driving + `.github/scripts/unrunnable-arm-names.py`. The tool DERIVES three things a list of it + got wrong first: which functions record, from the `pgc_record` call in their body; + which argument is the name, because `pgc_skip` records `"$2"` and reading argument one + takes the capability where the check is called `arrow support is present`; and what + counts as a refusal, from the verdict rather than the helper's spelling. `check_skip` + is deliberately not swept -- a skipped arm has no runnable counterpart by construction, + so 21 of its 23 call sites have no twin and always will. + + The part carries a positive control because the guard's steady state is zero and a + broken sweep reports zero too: it drives the real tool over a fixture whose refusal + branch names something the file never records, and over a control where the names + agree. + + This also closes the only pair `compare_to_bash.py` fails once its extractor is + widened to all eight `lib.sh` helpers (#1040): with the two names fixed, all seven + ported suites grade one-for-one under the widened extractor, with no change to the + port and none to `pgc_vacuity.py`. + +- The census re-derivation printed in `check_ledger_budget.txt` reads the wrong field + and returns zero (#1040). + + awk -F'\t' '$4=="never"' test/check_ledger.tsv | wc -l -> 0 + awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l -> 1189 on db74d9e + + #1010 inserted the majors a row claims as field 4, moving the last-red to field 5, and + the recipe stayed on field 4. The entry for #1010 in this file states that + `check_ledger_budget.txt` carries the corrected form; it did not. The gate is + unaffected -- it computes the census itself and never runs this command -- so the harm + is a reviewer re-deriving the number by the printed method, getting 0, and correcting a + budget that was right. Fixed here rather than filed because this change moves that very + number, and a wrong recipe beside a number nobody can check is worse than no recipe. + - `allow_empty` documented a rule the code did not enforce (#1031). `Expect.rows` documents the argument as taking *"a REASON, not a flag"*. The sentence even diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index de82e5c4..fbf2f9ad 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -1136,6 +1136,15 @@ harness_selftest 470-a-skipped-arm-records-under-its-own-name every comparable s harness_selftest 470-a-skipped-arm-records-under-its-own-name premise: and it actually compared some of them 15;16;17;18;19 never - harness_selftest 470-a-skipped-arm-records-under-its-own-name premise: the sweep found skip loops at all 15;16;17;18;19 never - harness_selftest 470-a-skipped-arm-records-under-its-own-name premise: the sweep tool is present 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check and it stays silent on the same file with the names in agreement 18 never - +harness_selftest 480-an-unrunnable-record-names-its-check every unrunnable record names a check its own suite asserts (#1040) 18 never - +harness_selftest 480-an-unrunnable-record-names-its-check pgc_pass and pgc_fail are twins, because a check can be recorded through them 18 never - +harness_selftest 480-an-unrunnable-record-names-its-check pgc_skip's name is read from argument TWO, not argument one 18 never - +harness_selftest 480-an-unrunnable-record-names-its-check premise: and it compared them rather than writing them all off as dynamic 18 never - +harness_selftest 480-an-unrunnable-record-names-its-check premise: the sweep found unrunnable sites at all 18 never - +harness_selftest 480-an-unrunnable-record-names-its-check premise: the sweep tool is present 18 never - +harness_selftest 480-an-unrunnable-record-names-its-check the refusal emitters are the two the tree defines 18 never - +harness_selftest 480-an-unrunnable-record-names-its-check the sweep REPORTS a name the file records only in its refusal branch 18 never - native_join_runtime_filter native_join_runtime_filter 3-table answer equals filter-off 15;16;17;18;19 never - native_join_runtime_filter native_join_runtime_filter 3-table join order matches filter-off 15;16;17;18;19 never - native_join_runtime_filter native_join_runtime_filter 3-table plan has coordinator 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 4e54a8ad..f3fa37d8 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -38,11 +38,20 @@ suites_not_covered 249 # HOW TO RE-DERIVE IT, written here because a changelog entry got it wrong and a # derivation is only useful where the number is: # -# awk -F'\t' '$4=="never"' test/check_ledger.tsv | wc -l +# awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l +# +# FIELD FIVE, and it said four until #1040. The command is only useful if it +# runs, and `$4` returned 0 on the shipped tree: #1010 inserted the majors a row +# claims as field 4, so the last-red moved to field 5 and the old form read a +# major where it expected a date. The CHANGELOG entry for #1010 says this file +# carries the corrected form; it did not, which is why the claim is worth as +# little as the recipe. Corrected here rather than filed because this change +# moves the number below, and a reviewer re-deriving it with the printed command +# would have got 0 against a stated 1198. # # Rows whose LAST-RED is `never`, not the row count. The two agree only while # nothing has ever been observed red, so a plain `grep -c` overcounts by exactly # the number of attacked checks -- and it overcounts from the first moment this # ledger does the job it exists for. The gate prints both quantities side by side # (`rows=N | never observed red=M`) because they are different questions. -checks_never_observed_red 1189 +checks_never_observed_red 1198 diff --git a/test/hilbert_locality.sh b/test/hilbert_locality.sh index dde3d51a..3c511d31 100755 --- a/test/hilbert_locality.sh +++ b/test/hilbert_locality.sh @@ -571,9 +571,9 @@ for pin in $PINS; do IFS=: read -r box want_z want_h floor <<<"$pin" if [ "$CURVE_DIFFERS" != different ]; then - check_unrunnable "box $box: groups read, Z-order" UNMET_PRECONDITION \ + check_unrunnable "box $box: groups read over $PLACEMENTS placements, Z-order" UNMET_PRECONDITION \ "the two partitions are not different ($CURVE_DIFFERS), so a ratio between them is not about the curve" - check_unrunnable "box $box: groups read, Hilbert" UNMET_PRECONDITION \ + check_unrunnable "box $box: groups read over $PLACEMENTS placements, Hilbert" UNMET_PRECONDITION \ "the two partitions are not different ($CURVE_DIFFERS), so a ratio between them is not about the curve" check_unrunnable "box $box: Hilbert reads fewer groups than Z-order" UNMET_PRECONDITION \ "the two partitions are not different ($CURVE_DIFFERS)" diff --git a/test/selftest/480-an-unrunnable-record-names-its-check.sh b/test/selftest/480-an-unrunnable-record-names-its-check.sh new file mode 100644 index 00000000..c4ab4e4b --- /dev/null +++ b/test/selftest/480-an-unrunnable-record-names-its-check.sh @@ -0,0 +1,109 @@ +# ---- an unrunnable record must name the check it stands in for -------------- +# +# #1040. `check_unrunnable NAME REASON DETAIL` gives one check the honesty +# `pgc_skip` gives a whole suite: the check did not run, and the reader is told +# WHICH. That only works if the record carries the name the check uses when it +# DOES run. Where the two spellings differ the property has TWO ledger keys and +# which one appears depends on runtime state -- in `hilbert_locality` it depended +# on whether that box's two partitions came out different that day, so the key +# was a function of the data. +# +# THIS IS PART 470's PROPERTY ONE HELPER OVER. 470 asks whether a skip LOOP still +# names the arms its sibling branch emits. This asks whether an unrunnable record +# names a check the same file asserts anywhere. +# +# WHY A GUARD AND NOT JUST THE RENAME. The convention is already near-universal +# and that is exactly what makes a lapse invisible: 23 of 25 call sites carry the +# runnable name (`hilbert_cluster` 9 of 9, `projection_rewrite` 11 of 11), so a +# reader skimming a new refusal branch sees nothing unusual in a shorter name. +# Both halves of a two-branch site are rarely read together. +# +# `check_skip` IS NOT SWEPT and that is a finding rather than an omission. A +# skipped arm has no runnable counterpart by construction -- the name IS the arm +# -- so 21 of its 23 call sites have no twin and always will. Sweeping it would +# report 21 mismatches that are all correct code. +# +# THE POSITIVE CONTROL BELOW IS THE POINT OF THE PART. A sweep that matched +# nothing reports the same zero as a corpus in agreement, and this guard's steady +# state is zero, so nothing else would ever distinguish the two. The fixture arms +# drive the real tool over a file whose answer is known. + +_un_py="$TESTDIR/../.github/scripts/unrunnable-arm-names.py" + +check "premise: the sweep tool is present" \ + "$([ -r "$_un_py" ] && echo yes || echo no)" "yes" + +_un_out="$(python3 "$_un_py" "$TESTDIR" --show-emitters 2>&1)" || _un_out="TOOL FAILED: $_un_out" + +_un_sites=$(printf '%s\n' "$_un_out" | sed -n 's/^sites \([0-9]*\)$/\1/p') +_un_dyn=$(printf '%s\n' "$_un_out" | sed -n 's/^dynamic \([0-9]*\)$/\1/p') +_un_cmp=$(printf '%s\n' "$_un_out" | sed -n 's/^compared \([0-9]*\)$/\1/p') +_un_ref=$(printf '%s\n' "$_un_out" | sed -n 's/^refusal //p') +_un_twins=$(printf '%s\n' "$_un_out" | sed -n 's/^twins //p') +_un_bad="$(printf '%s\n' "$_un_out" | sed -n 's/^MISMATCH //p' | tr '\n' ' ' | sed 's/ $//')" + +echo "-- unrunnable names: $_un_sites sites, $_un_cmp compared, $_un_dyn dynamic" + +# THE POPULATION IS THE PREMISE, twice. A sweep that found nothing and a corpus in +# agreement both print zero mismatches, and so does a sweep that found sites and +# compared none of them. +check "premise: the sweep found unrunnable sites at all" \ + "$([ "${_un_sites:-0}" -ge 20 ] && echo yes || echo "only ${_un_sites:-none}")" "yes" +check "premise: and it compared them rather than writing them all off as dynamic" \ + "$([ "${_un_cmp:-0}" -ge 20 ] && echo yes || echo "only ${_un_cmp:-none}")" "yes" + +# THREE PINS, each on a value that was WRONG in a draft of the tool, so each is a +# regression test rather than a restatement of the code. +# +# The refusal set decides what gets swept. A new refusal emitter must be looked at +# rather than silently sweep or silently not sweep; `arms_unrunnable` is here +# because it wraps `check_unrunnable` and reads its names from a list, so it must +# be recognised as a refusal and must NOT be offered as a twin. +check_text "the refusal emitters are the two the tree defines" \ + "$_un_ref" "arms_unrunnable:None check_unrunnable:1" + +# pgc_skip records `pgc_record FAIL "$2"`: its first quoted argument is the +# CAPABILITY, not the name. Reading argument one takes `arrow` where the check is +# called `arrow support is present` -- the bash-side mirror of the name-position +# defect #1036 and #1038 closed on the python side, across 22 call sites. +check "pgc_skip's name is read from argument TWO, not argument one" \ + "$(printf '%s\n' "$_un_twins" | tr ' ' '\n' | grep -c '^pgc_skip:2$')" "1" + +# pgc_pass was missing from two hand-written runnable lists, and its absence made +# projection_rewrite.sh report a false orphan: the twin of its unrunnable name is +# recorded by pgc_pass and not by any check_* helper. +check "pgc_pass and pgc_fail are twins, because a check can be recorded through them" \ + "$(printf '%s\n' "$_un_twins" | tr ' ' '\n' | grep -c '^pgc_\(pass\|fail\):1$')" "2" + +# ---- the positive control --------------------------------------------------- +# +# Driven over a fixture whose answer is known, because this guard's steady state +# is zero mismatches and a broken sweep reports zero too. +_un_fx="$(mktemp -d)" +cp "$TESTDIR/lib.sh" "$_un_fx/lib.sh" + +cat > "$_un_fx/offender.sh" <<'PGC_FX_BAD' +check_num "the property, said one way" "$a" "$b" +check_unrunnable "the property, said another way" UNMET_PRECONDITION "no fixture" +PGC_FX_BAD + +cat > "$_un_fx/control.sh" <<'PGC_FX_GOOD' +check_num "the property, said one way" "$a" "$b" +check_unrunnable "the property, said one way" UNMET_PRECONDITION "no fixture" +PGC_FX_GOOD + +_un_fxout="$(python3 "$_un_py" "$_un_fx" 2>&1)" || _un_fxout="TOOL FAILED: $_un_fxout" + +check "the sweep REPORTS a name the file records only in its refusal branch" \ + "$(printf '%s\n' "$_un_fxout" | grep -c '^MISMATCH offender.sh:2 the property, said another way$')" "1" +check "and it stays silent on the same file with the names in agreement" \ + "$(printf '%s\n' "$_un_fxout" | grep -c '^MISMATCH control.sh')" "0" + +rm -rf "$_un_fx" + +# ---- the tree --------------------------------------------------------------- + +check "every unrunnable record names a check its own suite asserts (#1040)" \ + "$_un_bad" "" + +unset _un_py _un_out _un_sites _un_dyn _un_cmp _un_ref _un_twins _un_bad _un_fx _un_fxout diff --git a/test/selftest/parts.manifest b/test/selftest/parts.manifest index b3a184ad..13653435 100644 --- a/test/selftest/parts.manifest +++ b/test/selftest/parts.manifest @@ -44,3 +44,4 @@ 450-a-red-nightly-must-be-findable.sh 460-the-controller-must-record-the-binary.sh 470-a-skipped-arm-records-under-its-own-name.sh +480-an-unrunnable-record-names-its-check.sh From 75a3447c7c699688e42591cb2f0e67c2364a94b3 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Sun, 13 Sep 2026 17:34:46 +0000 Subject: [PATCH 2/3] test: the sweep's exit code is the verdict, pinned in both directions (#1040) The tool printed MISMATCH lines and returned 0. Measured, unpiped, before changing anything: on main 2 mismatches rc=0 on branch 0 mismatches rc=0 Part 480 gates on the PARSED OUTPUT and so was never fooled -- but that is a fact about the wiring I happened to write, not about the tool. Anyone who runs this in CI and trusts $? gets a gate that cannot fail. Reported by @jdatcmd, who re-measured it without a pipe first, because their initial reading was tail's status and not the program's. THREE ARMS, NOT ONE. "Exits non-zero on a mismatch" alone passes on a tool that always exits 1. "Exits zero when clean" alone passes on a corpus with nothing to look at. So the clean run uses a SECOND fixture directory holding only the agreeing file, with a premise asserting that directory really has a refusal site: the sweep EXITS non-zero when it reports a mismatch and exits zero on a corpus that has refusal sites and no mismatch premise: that clean run had a refusal site to be silent ABOUT Neither fixture run is piped, and the comment says why. An em dash reached the changelog and docs_style refused it, which is the guard doing its job. Verified on top of b3433be: harness_selftest RC=0, 946 checks (943 + 3), 0 FAIL part 480 12 checks, all pass sweep unpiped main 2 mismatches rc=1 | branch 0 mismatches rc=0 shellcheck -S error over test/*.sh test/selftest/*.sh clean docs_style 14/14 ledger rows 1206 -> 1209 (+3), census 1198 -> 1201 re-derived by the corrected $5 command, ceiling 249 -> 249, orphans 0, gate rc=0 against db74d9e Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs --- .github/scripts/unrunnable-arm-names.py | 8 +++++- CHANGELOG.md | 9 ++++++ test/check_ledger.tsv | 3 ++ test/check_ledger_budget.txt | 2 +- ...80-an-unrunnable-record-names-its-check.sh | 28 +++++++++++++++++-- 5 files changed, 45 insertions(+), 5 deletions(-) diff --git a/.github/scripts/unrunnable-arm-names.py b/.github/scripts/unrunnable-arm-names.py index 27bf7527..4ee76937 100644 --- a/.github/scripts/unrunnable-arm-names.py +++ b/.github/scripts/unrunnable-arm-names.py @@ -290,7 +290,13 @@ def main(testdir, show_emitters=False, keep_heredocs=False): print("refusal " + " ".join(f"{k}:{v}" for k, v in sorted(all_ref.items()))) print("twins " + " ".join(f"{k}:{v}" for k, v in sorted(all_twin.items()))) print("fixed " + " ".join(sorted(all_fixed))) - return 0 + # THE EXIT CODE IS THE VERDICT. It was a flat 0 in the first version, so the + # tool printed two MISMATCH lines and reported success. Part 480 gates on the + # parsed output and was never fooled, but the next caller is the hazard: anyone + # wiring this into CI and trusting `$?` would get a gate that cannot fail. + # Reported by @jdatcmd, who re-measured it without a pipe before believing it, + # because the first reading was `tail`'s status and not this program's. + return 1 if bad else 0 if __name__ == "__main__": diff --git a/CHANGELOG.md b/CHANGELOG.md index efc1471e..bfc30042 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,15 @@ true until the next version shipped. ported suites grade one-for-one under the widened extractor, with no change to the port and none to `pgc_vacuity.py`. + The sweep's exit code is the verdict. It was a flat zero in the first version, so the + tool printed two `MISMATCH` lines and reported success. The part gates on the parsed + output and was never fooled, which is exactly why the exit code needed its own arms + rather than an observation: the next caller is the one that trusts `$?`, and a gate that + cannot fail is a trap whether or not today's only caller steps in it. Three arms, because + pinning the non-zero side alone passes on a tool that always exits 1, and the clean side + alone passes on a corpus with nothing to find -- so the clean run uses a second fixture + directory that HAS a refusal site, with a premise asserting it. Reported by @jdatcmd. + - The census re-derivation printed in `check_ledger_budget.txt` reads the wrong field and returns zero (#1040). diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index fbf2f9ad..f0a0675e 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -1136,14 +1136,17 @@ harness_selftest 470-a-skipped-arm-records-under-its-own-name every comparable s harness_selftest 470-a-skipped-arm-records-under-its-own-name premise: and it actually compared some of them 15;16;17;18;19 never - harness_selftest 470-a-skipped-arm-records-under-its-own-name premise: the sweep found skip loops at all 15;16;17;18;19 never - harness_selftest 470-a-skipped-arm-records-under-its-own-name premise: the sweep tool is present 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check and exits zero on a corpus that has refusal sites and no mismatch 18 never - harness_selftest 480-an-unrunnable-record-names-its-check and it stays silent on the same file with the names in agreement 18 never - harness_selftest 480-an-unrunnable-record-names-its-check every unrunnable record names a check its own suite asserts (#1040) 18 never - harness_selftest 480-an-unrunnable-record-names-its-check pgc_pass and pgc_fail are twins, because a check can be recorded through them 18 never - harness_selftest 480-an-unrunnable-record-names-its-check pgc_skip's name is read from argument TWO, not argument one 18 never - harness_selftest 480-an-unrunnable-record-names-its-check premise: and it compared them rather than writing them all off as dynamic 18 never - +harness_selftest 480-an-unrunnable-record-names-its-check premise: that clean run had a refusal site to be silent ABOUT 18 never - harness_selftest 480-an-unrunnable-record-names-its-check premise: the sweep found unrunnable sites at all 18 never - harness_selftest 480-an-unrunnable-record-names-its-check premise: the sweep tool is present 18 never - harness_selftest 480-an-unrunnable-record-names-its-check the refusal emitters are the two the tree defines 18 never - +harness_selftest 480-an-unrunnable-record-names-its-check the sweep EXITS non-zero when it reports a mismatch 18 never - harness_selftest 480-an-unrunnable-record-names-its-check the sweep REPORTS a name the file records only in its refusal branch 18 never - native_join_runtime_filter native_join_runtime_filter 3-table answer equals filter-off 15;16;17;18;19 never - native_join_runtime_filter native_join_runtime_filter 3-table join order matches filter-off 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index f3fa37d8..ff21e21a 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -54,4 +54,4 @@ suites_not_covered 249 # the number of attacked checks -- and it overcounts from the first moment this # ledger does the job it exists for. The gate prints both quantities side by side # (`rows=N | never observed red=M`) because they are different questions. -checks_never_observed_red 1198 +checks_never_observed_red 1201 diff --git a/test/selftest/480-an-unrunnable-record-names-its-check.sh b/test/selftest/480-an-unrunnable-record-names-its-check.sh index c4ab4e4b..244352da 100644 --- a/test/selftest/480-an-unrunnable-record-names-its-check.sh +++ b/test/selftest/480-an-unrunnable-record-names-its-check.sh @@ -92,18 +92,40 @@ check_num "the property, said one way" "$a" "$b" check_unrunnable "the property, said one way" UNMET_PRECONDITION "no fixture" PGC_FX_GOOD -_un_fxout="$(python3 "$_un_py" "$_un_fx" 2>&1)" || _un_fxout="TOOL FAILED: $_un_fxout" +# A SECOND FIXTURE DIRECTORY holding only the agreeing file, so the clean exit code +# is measured on a corpus that HAS a refusal site rather than on one with nothing to +# find -- those two report the same 0 and only one of them is evidence. +_un_fxok="$(mktemp -d)" +cp "$TESTDIR/lib.sh" "$_un_fxok/lib.sh" +cp "$_un_fx/control.sh" "$_un_fxok/control.sh" + +# NO PIPE ON EITHER RUN. `$?` after a pipeline is the LAST stage's, which is how the +# missing exit code first read as present (@jdatcmd). +_un_fxout="$(python3 "$_un_py" "$_un_fx" 2>&1)"; _un_fxrc=$? +_un_okout="$(python3 "$_un_py" "$_un_fxok" 2>&1)"; _un_okrc=$? check "the sweep REPORTS a name the file records only in its refusal branch" \ "$(printf '%s\n' "$_un_fxout" | grep -c '^MISMATCH offender.sh:2 the property, said another way$')" "1" check "and it stays silent on the same file with the names in agreement" \ "$(printf '%s\n' "$_un_fxout" | grep -c '^MISMATCH control.sh')" "0" -rm -rf "$_un_fx" +# THE EXIT CODE IS THE VERDICT, pinned in both directions. It was a flat 0 in the +# first version of the tool: two MISMATCH lines printed and success reported. This +# part gates on the parsed output and so was never fooled, which is exactly why the +# hazard needs its own arm -- the next caller is the one that trusts `$?`. +check "the sweep EXITS non-zero when it reports a mismatch" \ + "$([ "$_un_fxrc" -ne 0 ] && echo yes || echo "no, rc=$_un_fxrc")" "yes" +check "and exits zero on a corpus that has refusal sites and no mismatch" \ + "$_un_okrc" "0" +check "premise: that clean run had a refusal site to be silent ABOUT" \ + "$(printf '%s\n' "$_un_okout" | sed -n 's/^compared \([0-9]*\)$/\1/p')" "1" + +rm -rf "$_un_fx" "$_un_fxok" # ---- the tree --------------------------------------------------------------- check "every unrunnable record names a check its own suite asserts (#1040)" \ "$_un_bad" "" -unset _un_py _un_out _un_sites _un_dyn _un_cmp _un_ref _un_twins _un_bad _un_fx _un_fxout +unset _un_py _un_out _un_sites _un_dyn _un_cmp _un_ref _un_twins _un_bad +unset _un_fx _un_fxok _un_fxout _un_okout _un_fxrc _un_okrc From 3a640b00eda2f3041e6dde0650513d8cde2b195c Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Sun, 13 Sep 2026 17:53:42 +0000 Subject: [PATCH 3/3] test: the new ledger rows claim every major they were observed on (#1040) CI's `suites (PG 17)` leg failed on 75a3447 while `suites (PG 18)` passed. Every suite PASSED in the failing leg; the census agreed at 1201, the ceiling did not rise, the population reconciled at 253. The only failing condition was one field: major field across harness_selftest rows 934 15;16;17;18;19 12 18 <- part 480's, and only part 480's PG17: not in the ledger: harness_selftest 480-... (x12, on major 17) PG17 has a check the ledger has never seen, which is not a pass pgc_ledger.py merge stamps the majors of the LOG it is given, and the rows were merged from a single PG18 run. NOT FIXED BY SETTING THE FIELD. Writing 15;16;17;18;19 into those rows to match their 934 neighbours produces a value that is consistent, plausible and UNOBSERVED. That field is what the gate compares against, so a hand-written row is a claim the gate can never catch being wrong -- the standard rather than a measurement checked against one. Proposed by @jdatcmd and withdrawn by them on that reasoning; their summary is the better one, that copying the neighbours is the version which looks like diligence. So harness_selftest was run on all five assert builds and all five logs merged. The majors field accumulates as a SET (pgc_ledger.py:283), checked in the source before relying on it, so five merges union rather than overwrite. The second reason is the stronger one: this answers whether part 480 passes on 15 and 19 AT ALL. Nobody had asked. The red-then-green removal proof for this whole PR was PG18a, and a defect in the major field is invisible there by construction. Agreed with @jdatcmd BEFORE the runs, so it was not decided after seeing the number: if a major legitimately produced fewer checks, the rows would say what each run observed. A check existing on four majors and not the fifth is a finding about the part, not a number to smooth by union or arithmetic. Verified, all five run on this tree at 75a3447: PG15 rc=0 946 checks 0 FAIL 12 rows from part 480 PG16 rc=0 946 checks 0 FAIL 12 PG17 rc=0 946 checks 0 FAIL 12 PG18 rc=0 946 checks 0 FAIL 12 PG19 rc=0 946 checks 0 FAIL 12 the five runs' (part, name) sets: 45 each, and PG18 differs from PG15, PG16, PG17 and PG19 by ZERO lines -- so no check exists on some majors and not others, and no row was added by the four new logs ledger gate, run once per major's log: rc=0 on all five, zero "never seen" lines ledger rows 1209 unchanged | census 1201 unchanged | ceiling 249 -> 249 the whole diff: 12 insertions, 12 deletions in check_ledger.tsv, and every changed row differs ONLY in field 4 (checked by normalising that field and asserting each line then appears exactly twice) the ledger is now uniform: 1209 of 1209 rows carry 15;16;17;18;19 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs --- test/check_ledger.tsv | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index f0a0675e..6d62fbbb 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -1136,18 +1136,18 @@ harness_selftest 470-a-skipped-arm-records-under-its-own-name every comparable s harness_selftest 470-a-skipped-arm-records-under-its-own-name premise: and it actually compared some of them 15;16;17;18;19 never - harness_selftest 470-a-skipped-arm-records-under-its-own-name premise: the sweep found skip loops at all 15;16;17;18;19 never - harness_selftest 470-a-skipped-arm-records-under-its-own-name premise: the sweep tool is present 15;16;17;18;19 never - -harness_selftest 480-an-unrunnable-record-names-its-check and exits zero on a corpus that has refusal sites and no mismatch 18 never - -harness_selftest 480-an-unrunnable-record-names-its-check and it stays silent on the same file with the names in agreement 18 never - -harness_selftest 480-an-unrunnable-record-names-its-check every unrunnable record names a check its own suite asserts (#1040) 18 never - -harness_selftest 480-an-unrunnable-record-names-its-check pgc_pass and pgc_fail are twins, because a check can be recorded through them 18 never - -harness_selftest 480-an-unrunnable-record-names-its-check pgc_skip's name is read from argument TWO, not argument one 18 never - -harness_selftest 480-an-unrunnable-record-names-its-check premise: and it compared them rather than writing them all off as dynamic 18 never - -harness_selftest 480-an-unrunnable-record-names-its-check premise: that clean run had a refusal site to be silent ABOUT 18 never - -harness_selftest 480-an-unrunnable-record-names-its-check premise: the sweep found unrunnable sites at all 18 never - -harness_selftest 480-an-unrunnable-record-names-its-check premise: the sweep tool is present 18 never - -harness_selftest 480-an-unrunnable-record-names-its-check the refusal emitters are the two the tree defines 18 never - -harness_selftest 480-an-unrunnable-record-names-its-check the sweep EXITS non-zero when it reports a mismatch 18 never - -harness_selftest 480-an-unrunnable-record-names-its-check the sweep REPORTS a name the file records only in its refusal branch 18 never - +harness_selftest 480-an-unrunnable-record-names-its-check and exits zero on a corpus that has refusal sites and no mismatch 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check and it stays silent on the same file with the names in agreement 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check every unrunnable record names a check its own suite asserts (#1040) 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check pgc_pass and pgc_fail are twins, because a check can be recorded through them 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check pgc_skip's name is read from argument TWO, not argument one 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check premise: and it compared them rather than writing them all off as dynamic 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check premise: that clean run had a refusal site to be silent ABOUT 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check premise: the sweep found unrunnable sites at all 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check premise: the sweep tool is present 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check the refusal emitters are the two the tree defines 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check the sweep EXITS non-zero when it reports a mismatch 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check the sweep REPORTS a name the file records only in its refusal branch 15;16;17;18;19 never - native_join_runtime_filter native_join_runtime_filter 3-table answer equals filter-off 15;16;17;18;19 never - native_join_runtime_filter native_join_runtime_filter 3-table join order matches filter-off 15;16;17;18;19 never - native_join_runtime_filter native_join_runtime_filter 3-table plan has coordinator 15;16;17;18;19 never -