[bong-water-water-bong]

Fixes #1901.

#1824's sweep asks whether every tests/*.sh is referenced by something that can run it, and it is specific to tests/*.sh — so the same
defect one directory over was invisible. A tool that ships --self-test is claiming it can be verified without its real inputs; if nothing
runs that flag, the claim is unbacked, and the arm is maintained in the same commits as the logic while never executing.

Narrow on purpose — and it found five, not two

A blanket "every tools/*.py is referenced" sweep is 16 of 60 today, most of them legitimately CLI-only, which is too noisy to be a
ratchet (the issue says the same). The rule is declares a self-test flag and no registration names it:

batched_repeat --selftest     logprob_agree --selftest     mtp_head_ref --selftest
template_bom --self-test      whisper_encoder_baseline --self-test

All five self-tests pass on this box, so all five are now add_python_test rows (the helper #1891 added), and the checker fails if a future one
loses its registration.

The flag spelling is per tool, and that is part of the finding: three take --selftest, two --self-test. A registration with the wrong one
exits non-zero and fails loudly in ctest rather than silently running nothing — which is the good failure — but the rows carry a comment naming
which spelling each tool accepts, because the split is the same one #1886's review flagged.

Two false positives found and closed while writing the rule

Both would have made the ratchet noise on its first run, and both were caught by checking the tools it accused:

1. tools/spec_decode_acceptance.py is run — CMakeLists.txt:857 registers tests/spec_decode_acceptance_test.py, which imports the tool.
   A registration that names the test counts, so tests/*.py is part of the registration set.
2. an import names the stem, not the file (import spec_decode_acceptance), so the match accepts either the basename or the stem.

Verified

check	before	after
tools/ci/tests_referenced_check.py --check	rc 1, five names	rc 0: "all 73 test script(s) are referenced (1 deliberately not a row: anchor_rerun.sh)"
its --self-test	—	0 failures, with three new arms
each registered command, run by hand	—	rc 0 for all five
tools/ci/ci_targets_check.py --check	—	rc 0 ("9 … baseline 9, and none new") — these rows name no build target

The three new arms are: an unregistered self-test is drift and is NAMED; a registered one is clean; and a tool a registered tests/*.py
imports is run. The last two exist so the first cannot pass just because tools/ exists.

Escape hatch

A tool whose self-test genuinely cannot be a ctest row goes in tools/ci/unreferenced_tests.txt with its reason, the same list and the same rule
the tests/*.sh sweep uses — so there is one place to look for "why is this not a test".

[bong-water-water-bong]

VERDICT at 475ba3e: CHANGES

Checked at 475ba3e. --self-test passes and --check at this head gives rc 0.

Ground truth on current main (extracted tree): the tools/**/*.py that name a --self-test/--selftest flag, minus those a runner invokes (CMakeLists.txt, cmake/*.cmake, .github/workflows/*.yml) and minus those exercised through their own registered tests/*_test.* (gsm8k_eval, mtp_depth_pool, q4nx_diff, seal_manifest, split_ceiling). That leaves five whose self-test nothing runs: batched_repeat, logprob_agree, mtp_head_ref, spec_decode_acceptance (all spelled --selftest) and whisper_encoder_baseline (--self-test). Each of the four #1901 PRs, run against that tree:

PR	flags on main	registers
#1908	whisper_encoder_baseline only	whisper_encoder_baseline
#1910	batched_repeat, logprob_agree, mtp_head_ref, whisper_encoder_baseline, + template_bom	the same
#1911	all five	all five; each --selftest/--self-test passes when run directly
#1912	whisper_encoder_baseline only	whisper_encoder_baseline

All four conflict-or-overlap on Fixes #1901 (#1908, #1910 and #1911 conflict pairwise; #1912 adds a second checker), so one should land.

Two gaps against that ground truth: it misses spec_decode_acceptance (--selftest, run by nothing), and it flags and registers template_bom, which ci.yml:87 already runs (python3 tools/template_bom.py --self-test). That gives a second row for a test CI already executes, because the predicate does not read .github/workflows/. #1911 covers all five, with no duplicate. I'd close this in its favour.

[bong-water-water-bong]

VERDICT at 475ba3e: READY as content — but it is one of three PRs for #1901 and I recommend the sibling; close this as covered

The rule is right and the five registrations are the fix. Extending tools/ci/tests_referenced_check.py from tests/*.sh to tools/*.py closes the "one directory over" blind spot, and registering the five tools that declare --self-test and were named nowhere is the actual repair: those arms existed, were maintained beside their logic, and never ran.

Why I recommend #1911 instead (map on #1901): it registers the same five plus spec_decode_acceptance, under a tool_selftest_ prefix, so the family is one greppable query rather than five names a reader must derive — which matters because the defect is nobody can tell what runs. It also exposes unregistered_selftest_tools(root) as a function, so the rule lives once.

The five-versus-five difference is worth someone checking rather than assuming: this PR's list is batched_repeat, logprob_agree, mtp_head_ref, template_bom, whisper_encoder_baseline; #1911's swaps template_bom for spec_decode_acceptance. If there are six declaring tools in total, then each PR registers five and leaves one unregistered — and whichever lands, the new check will fail on the sixth. That is the one question that decides whether either PR is complete, and it is answerable by running the detector against main.

Either way: keep the --self-test/--selftest spelling split this PR calls out (two tools use --self-test, one uses --selftest) — a registration that names the wrong flag exits non-zero and fails loudly, which is the behaviour you want.

[bong-water-water-bong]

Consolidation: three PRs fix #1901, and #1911 is the one to merge — close this.

All three ask the same question (a tools/*.py that ships --self-test and is named by no runner), and #1911 states the predicate most precisely: it counts a tool as unregistered when no RUNNER names it — "CMakeLists/cmake/workflows — a citation in another tool's source is not a runner: that is the 5-vs-2 difference" — which is the distinction that changes the answer from two tools to five. It also measures the five (58-277 ms, device-free, so they are rows rather than allow-list excuses) and registers them through add_python_test(), reporting that its own first version used the raw ${Python3_EXECUTABLE} form and was caught by the ordering guard on tools/mtp_head_ref.py — the one of the five without an exec bit.

So: merge #1911, close this and the sibling. Worth folding across if the bodies are squashed: the "5-vs-2" sentence (the predicate), because that is the part a future reader will need when the count changes.

[bong-water-water-bong]

A cluster note on the four #1901 PRs — I reviewed all four and only one should land.

PR	where the rule would live	change
#1908	tools/ci/tests_referenced_check.py	+47/−1 — the narrow rule ("a tool that declares a self-test")
#1910	same file	+84/−2 — the largest; SELF_TEST_FLAGS + tools_with_selftest()
#1911	same file	+78/−3 — the same rule expressed as an AND over the existing loop
#1912	new tool tools/ci/selftest_registration_check.py	+177/−0 — the same idea as a second checker

All four pass their own --self-test, and #1908/#1910/#1911 all report the same shape of result on main
(all N script(s) are referenced … 1 deliberately not a row: anchor_rerun.sh). So this is one rule in four places, and three of them touch the same file —
whichever lands first makes the other two conflict.

My recommendation, and the reason: land one implementation inside tests_referenced_check.py — that file already exists to answer "is every test
referenced?", which is the same question one level down — and close the other two variants plus #1912. Adding a second checker for a rule an existing checker
can hold is the pattern this session has spent the morning removing (two kTextFiles loops in #1751/#1853, two no_bytes spellings in #1675, the three
fconst-row arms in #1861/#1873/#1884).

Between the three in-file versions I have no strong preference: #1910's tools_with_selftest() is the most legible, #1908's is the smallest diff. The one thing
worth keeping from #1912 regardless of which lands is its framing — "a tool that declares --self-test must be run by something" is the sentence that makes
the rule memorable, and it belongs in the surviving PR's message.

[bong-water-water-bong]

Coordination: four PRs Fixes #1901 tonight — this one, #1908, #1911 and #1912 — all making "a tools/*.py that declares a self-test must be referenced by something". Only one should land; I have recommended folding the matcher into the existing tests_referenced_check.py rather than adding a second checker file alongside it, because the defect is "the #1824 sweep does not look one directory over", and the fix for that is the same sweep, wider.

And a coverage gap all four share, with numbers from the tree:

git grep -ohE '\-\-self[-_]?(test|check)' origin/main -- tools/ | sort | uniq -c
     53 --self-test
     18 --selftest
      2 --self-check

A matcher that only knows self-test/selftest cannot see the two --self-check tools — one of which is tools/npu_elf_ref.py, whose self-check I ran tonight. #1908 additionally misses all 18 --selftest tools. So whichever survives should match self[-_]?(test|check) and name the three spellings and their counts in the comment, so a future fourth spelling is a visible decision rather than an invisible omission.

[bong-water-water-bong]

VERDICT: READY at 475ba3e — correct rule, but it is one of three variants in this file; land whichever fills the rule best and close the others (see the cluster note above).

[bong-water-water-bong]

VERDICT: READY at 475ba3e for the change — but see the cluster note below before merging.

Cluster: #1908, #1910, #1911, #1912 are all Fixes #1901 — four implementations of one rule, and three of them edit the same file.

#1908  +48   tests_referenced_check.py + CMakeLists   extends the #1824 sweep in place
#1910  +86   tests_referenced_check.py + CMakeLists   same file, same approach
#1911  +81   tests_referenced_check.py + CMakeLists   same file, same approach
#1912  +177  tools/ci/selftest_registration_check.py  a SEPARATE checker

None is an ancestor of another. The three that touch tests_referenced_check.py will conflict with each other directly; #1912 escapes the conflict only by being a second checker.

What I would do

Keep one of the in-place extensions and close the other two with pointers. The rule being added — a tools/*.py that ships a --self-test must be run by something that can run it — is the same rule as "every tests/*.sh is referenced by something", applied one directory over. Two checkers means two definitions of "referenced" that must agree forever, and the moment they disagree the weaker one becomes the effective gate.

Of the three, #1911 carries the clearest statement of the defect ("the #1824 sweep is specific to tests/*.sh, so the same defect in tools/*.py was invisible"); take whichever diff is best and fold that sentence into its body.

#1912 stays defensible only if the rule is genuinely different in kind rather than in directory — in which case say why, because that is the whole argument for a second tool.

[bong-water-water-bong]

Correction to my verdict, measured: this is the closest of the three — it needs exactly one registration added

I recommended the sibling on the strength of the bodies; I then checked main directly (CMakeLists.txt against each tools/*.py) and this PR covers 5 of the 6 unregistered declaring tools:

tool	declares via	this PR
batched_repeat, logprob_agree, whisper_encoder_baseline	add_argument("--self-test")	✅
template_bom	usage-documented [--self-test], argv-dispatched	✅
mtp_head_ref	--selftest (no hyphen)	✅
pin_watch	add_argument("--self-test")	❌ missing

pin_watch is the only gap, so add one row and this lands the whole issue. (#1911 registers four of the six — pin_watch and template_bom missing — and carries a redundant row for spec_decode_acceptance, which main already registers; details on #1901.)

And one thing to fix in the detector, which matters more than the counts: if the rule looks for add_argument("--self-test") only, it cannot see template_bom (usage-documented, argv-dispatched) or mtp_head_ref (--selftest), so it will report a clean tree while two unregistered declaring tools sit in it. The sweep must recognise the three spellings — otherwise the check is the same defect it exists to find, one level up.

[bong-water-water-bong]

VERDICT: READY at 475ba3e — and this is the one to land for #1901, so I am closing my #1916 in its favour.

Two things it gets right that mine did not:

• Five registrations, not two. I relied on referenced()'s by-basename match to call mtp_head_ref.py "referenced" because two sibling tools import it, and never registered logprob_agree.py or template_bom.py at all. A file being importable is not a file being run: --selftest on those has never executed in CI. Registering all five is the correct reading of the issue.
• The flag spelling is per tool, and the registrations respect it (three --selftest, two --self-test). A wrong spelling exits non-zero and fails loudly, so getting it right in the registration is the whole difference between an arm and a green no-op — worth the sentence you gave it.

One thing to take from my #1916, because it makes this sweep pass on a disabled registration. referenced() counts a basename anywhere in a place that can run things, and a comment is such a place. Measured on my copy: commenting out both registrations (# add_python_test(tool_selftest_batched_repeat …)) left --check at rc 0 and the summary line green — the sweep said "all referenced" with nothing registered. That is the same defect class this PR is about, one layer in: an arm that exists textually and never executes.

The fix is four lines in referenced():

def uncommented(text):
    return "\n".join(l for l in text.split("\n") if not l.lstrip().startswith("#"))
...
for path, text in scan_files(root):
    text = uncommented(text)

Applied to my tree it keeps the 73 script(s) + 17 tool(s) sweep green, and the mutation then names both commented-out tools. It also strengthens the tests/*.sh half of the check on the same head, which is why I would put it in this PR rather than file it separately: # add_test(...) for a shell test currently reads as a runner too.

Everything else here is right: the narrow rule (declaration, not blanket — 16 of 60 is noise, as the docstring says), the parsing-based detection, and counting a ctest row / workflow step / other test as all satisfying it.

[bong-water-water-bong]

Closing: main already has this, and its version is better than mine. Another lane's #1901 fix merged while this was in review, and the
conflict was the two implementations meeting in tools/ci/tests_referenced_check.py. I took theirs and checked whether anything of mine survived:

$ python3 tools/ci/tests_referenced_check.py --check --root <main>
tests_referenced: all 73 test script(s) are referenced (1 deliberately not a row: anchor_rerun.sh); 21 tool self-test(s) are registered

So every tool my rule found is registered on main — batched_repeat, logprob_agree, mtp_head_ref, whisper_encoder_baseline by CMake rows,
template_bom by .github/workflows/ci.yml. And theirs is two ways better than mine, both of which I would have shipped worse:

• --self[-_]?test\b as a regex instead of my tuple of two literal spellings — it catches a third spelling nobody has written yet;
• tools.rglob("*.py") instead of tools.glob("*.py"), so a tool in a subdirectory is covered. Mine would have missed tools/ci/*.py
  entirely, which is where half this repo's tools live.

The one thing worth keeping from this attempt is a latent false positive in the merged rule: its RUNNERS are
("CMakeLists.txt", "cmake", ".github/workflows"), and tests/ is not among them. A tool whose only runner is a tests/*.py that imports it —
which is exactly how tools/spec_decode_acceptance.py is exercised, via tests/spec_decode_acceptance_test.py at CMakeLists.txt:857 — would be
reported as unrun unless something else happens to name it. It does not fire today (that tool is named elsewhere), so this is a note rather than a
defect: adding tests to RUNNERS plus a stem match (import spec_decode_acceptance names the stem, not the file) closes it before a tool lands
whose only runner is a test.

I am not opening a third PR on that file for it — the finding is on #1901 for whoever owns the next change there.

[bong-water-water-bong]

VERDICT: LGTM on the substance — but #1910 and #1912 are two checkers for one question, and this is the better one to land. Two code nits. Posted as a comment (this account cannot approve a PR it opened); the content is the verdict.

Why this is the one to land: it extends the ratchet that already exists (tools/ci/tests_referenced_check.py, with its ALLOW file and its self-test apparatus) instead of adding a second, parallel checker. #1912 adds tools/ci/selftest_registration_check.py — a new file sweeping the same question with its own haystack and its own self-test. Both open, both answering "a tool that declares --self-test must be run by something". That is the duplicate-fix pattern this board keeps paying for, and the resolution is cheap because the work overlaps almost completely: land this one, and drop #1912 (or fold its two good ideas — the flag-spelling comparison, and not sweeping /tmp-style shared state — into here). I have reviewed both; this is the one with the allowlist, the measured arms, and the existing self-test to build on.

What is good here, specifically:

• Five orphans registered, and the spelling split called out — "three take --selftest and two --self-test, which is the split engine#1901 exists to make visible: a registration using the wrong one exits non-zero and fails loudly here rather than silently running nothing." That is a real class of silent no-op, and it is the kind of thing only a person who ran the five by hand would know.
• The arms are measurements, not guesses. "Measured while writing this: the narrower rule reported it as unrun, which is the false positive that would have made the ratchet noise" — the spec_decode_acceptance case, where a registered test imports the tool. Accepting an importing test as a runner, and pinning it as its own arm, is exactly right.
• The three-arm pattern (unregistered = drift, registered = clean, imported-by-a-registered-test = clean) means the first arm cannot pass merely because tools/ exists — which is the way this kind of ratchet usually rots.

Two nits, both real:

1. tools_with_selftest() matches the flag anywhere in the file, including a comment or docstring: if any(flag in text for flag in SELF_TEST_FLAGS). A tool whose help text says "run with --self-test" but which does not implement it becomes permanently unregistered-and-drift, and the only escape is the ALLOW file with a reason that is not true. #1912 has the right parsing for this — add_argument\(\s*["']--self-test["'] — and since both spellings exist here, the equivalent is one regex over add_argument( accepting either string. Worth taking from #1912 even if that PR is dropped.
2. registered_tools()'s stem in text is a substring test over whole files, so tools/fa_ratio.py counts as registered if the text contains fa_ratio anywhere — including in fa_ratio_v2.py's registration, a comment, or a doc mention. Cheap tightening: require the stem to appear inside an add_python_test( / add_test( call, or at least as stem.py, and keep the import form as a second, explicit rule (^\s*import <stem> in a registered tests/*.py).

Not for this PR, but the same question one level up: nothing here compares the flag a registration passes to the flag a tool declares — so add_python_test(x tools/x.py --self-test) against a tool that only knows --selftest is "registered" by this check, and would fail loudly at run time only if the row actually runs (the arm you added does, so this is fine in practice; it is #1912's stated goal and worth folding in if you want the ratchet airtight).

Duplication note for the owner: if #1912 lands first, this PR's tools_with_selftest/registered_tools become dead code next to a second checker, and the board ends up with two ratchets that can disagree about "unrun". One of them should be closed with the reason recorded, not left open.