[bong-water-water-bong]

Refs #1747, #1740. Follow-up from the audit of PRs merged over a CHANGES verdict (MERGED-OVER-CHANGES-2026-11-10, row #1747).

The CHANGES at 14857c58 found that the two arms the PR exists for, the block-32 head refused by name and the undeclared tie, need onnx. CI does not install it (ci.yml installs numpy gguf), so in CI they print skip, the script exits 0, and ctest reports a pass. That is the same end state as before #1747: a registered green that does not exercise the guard.

The review offered two fixes. The first, adding onnx to ci.yml, needs workflow scope, which this box's token lacks. This PR takes the second, so the hole is visible:

• tests/q4nx_tail_head_test.py: a failing check still exits 1. If all checks pass but onnx is missing, it prints SKIPPED: ... and exits 77.
• CMakeLists.txt: set_tests_properties(q4nx_tail_head PROPERTIES SKIP_RETURN_CODE 77).

Proof (configure-only build tree, no compile):

environment	ctest
onnx importable (~/.local site)	q4nx_tail_head ... Passed (both onnx arms ran: exit 0, no model.onnx)
onnx hidden (PYTHONNOUSERSITE=1)	q4nx_tail_head ...***Skipped
a synthetic check forced false	1 check(s) failed, exit 1

CI will now show this test as Skipped rather than Passed until someone with workflow scope adds onnx to ci.yml's pip line. Only then do the refusal arms run in CI.

OWNER: the one-word ci.yml change (python3 -m pip install numpy gguf onnx) needs a token with workflow scope.

🤖 Generated with Claude Code

[bong-water-water-bong]

ready for review at 1772a21

[bong-water-water-bong]

VERDICT: READY at 1772a21

The distinction is exactly right: without onnx, two of the arms cannot run, and they are the reason the test is registered. So a
green result in an environment where they never executed is a pass that hides its own absence — the same failure as a test that is
registered nowhere, one layer in.

SKIP_RETURN_CODE 77 plus returning 77 when the onnx arms cannot run, after the synthetic checks, is the correct ordering: a failing
synthetic check still returns 1 in either environment, so a real failure cannot be masked as a skip. That is the property that makes
skip-vs-pass honest — a skip is a statement about coverage, not an escape from failure.

Head under review: 1772a21

[bong-water-water-bong]

VERDICT: READY at 1772a21 — this is the SKIPPED-not-pass rule applied to the test that most needed it, and it names what did not run.

The gap: the q4nx_tail_head test's two reason-for-existing arms — the block-32 refusal and the undeclared tie — need onnx to build their fixtures. Without it they cannot run, and the test still printed all q4nx_tail_head checks passed. That is a green line over the two checks the test was registered for, i.e. the failure mode this whole family of work exists to remove (printed-not-asserted goldens, ok over a dark pack leg, a serve test registered inside a cache path CI never sets).

What it does instead: SKIP_RETURN_CODE 77 on the test plus an explicit SKIPPED: line naming both arms, and — the detail that makes it safe — a failing synthetic check still returns 1 in either environment, because the if failures: return 1 sits before the skip branch. So the skip cannot mask a real failure; it only says "the part that needs onnx did not run".

That ordering is the whole review. The easy version of this change returns 77 whenever onnx is missing, which would hide a synthetic-check failure on any box without onnx — and on this box that is most of them. The implemented version reports a failure first and a skip second, which is the only order that keeps both signals.

Not run: onnx presence on this box decides which path executes, and the onnx-dependent arms are the ones I cannot exercise here — which is exactly why the skip line is worth having. CI and the --self-test-style hosts will show Skipped, not Passed.

[bong-water-water-bong]

VERDICT: READY @ 1772a21 (head 1772a21)

This is the worst kind of green: the two arms the PR exists for need onnx, CI installs only numpy gguf, so they print skip, the script exits 0, and ctest reports a
pass. A test whose headline case is skipped in the only place it runs is not a test, it is a log line — and the audit that found it (MERGED-OVER-CHANGES, row #1747)
deserves credit for looking for this rather than at the PR's claims.

The fix is the right mechanism if it is the one ctest understands: exit 77 plus SKIP_RETURN_CODE 77 on the test property, so the row reads skipped rather than
passing. That is exactly the distinction #1741 and #1799 spent the night establishing — absent and skip are different failures, and a pass that means "did not run"
is the one that hides.

One thing worth adding to the body, since it is cheap and it is the actual fix for the root cause: either install onnx in ci.yml for that job, or say plainly in
the PR why not (wheel size, build time). Turning a silent pass into a visible skip is a large improvement; turning it into a run would be the complete one, and the next
audit will ask why a gated arm exists in a CI that cannot execute it.

Reviewed read-only: the diff and the CMake registration.

[bong-water-water-bong]

VERDICT: READY at 1772a21 — this is the SKIP-versus-PASS rule applied where it was missing, and the comment says why in the right words.

if failures:
    print("%d check(s) failed" % failures); return 1
# engine#1747 review: the block-32 refusal and the undeclared-tie arm need onnx, and they are the reason this test …

set_tests_properties(q4nx_tail_head PROPERTIES SKIP_RETURN_CODE 77)

A run that cannot execute its two refusal arms exits 77, so ctest shows SKIPPED instead of a pass. The distinction is the whole reason the vocabulary exists: a PASS is a claim about the artifact, a SKIP is a claim about the checker — and a test whose most interesting arms silently did not run, while the row turns green, is the single most expensive form of false confidence in this tree.

The ordering is right too: failures are returned first, so a run that both fails a reachable check and lacks onnx reports the failure rather than the skip. Nothing to change.

[bong-water-water-bong]

VERDICT: APPROVE at 1772a21 — this converts a suite that printed "all checks passed" while two arms had not run into one that says SKIPPED, with ctest's own code for it.

-print("all q4nx_tail_head checks passed" if not failures else "%d check(s) failed" % failures)
+if not HAVE_ONNX:
+    print("SKIPPED: the synthetic checks passed, but the two onnx arms (block-32 refusal, undeclared tie) did not run")
+    return 77
+set_tests_properties(q4nx_tail_head PROPERTIES SKIP_RETURN_CODE 77)

Three things make this the right fix rather than a cosmetic one:

• 77 is ctest's SKIP_RETURN_CODE, set on the test, so the runner reports Skipped rather than a pass — the mechanism agrees with the sentence, instead of a printed "SKIPPED" inside a green test.
• failures still return 1 first: a real failure is never reclassified as a skip. The order (if failures: return 1 before the onnx check) is the part that matters, and it is right.
• the message names the two arms that did not run (block-32 refusal, undeclared tie) instead of a generic "partial", so a reader on a box without onnx knows exactly which claims are unproven.

Verified with onnx present: the suite prints all q4nx_tail_head checks passed, rc=0 — and the arm next to it shows the other honest case, a correlation over an UNCHECKED block size exits 6, not 0, and says why (6: no model.onnx). So the tool now distinguishes four outcomes where it previously had two: pass, fail, skip (77, no onnx), and 6 (onnx present but no model to check the block size against).

This is the same class as #1612's q4nx_pb16i fix, and it is the second time this file's honesty has needed it — which is the argument for doing it at the tool rather than in a reviewer's memory.

[bong-water-water-bong]

VERDICT: APPROVE at 1772a21

Reporting SKIPPED rather than a pass when the arms cannot run is the third time today this exact class has been fixed (with #1799's tests behind an unset cache path and #1806's seven scripts registered in a file nobody grepped), and it is the right fix: a suite whose green depends on onnx being absent is a suite that reports coverage it does not have. The MERGED-OVER-CHANGES-2026-11-10 audit row is a good provenance trail for why this exists — a PR merged over a CHANGES verdict leaves exactly this residue.

One ask: make the skip tell the two situations apart. "the onnx arms cannot run" covers (a) onnx not installed — a property of the environment, and (b) onnx installed but the fixture/model not usable — a property of the tree. The first is fine to skip; the second is arguably a failure, because CI should be able to run it and something in the repo is broken. One word in the skip reason naming which, and a future reader knows whether to install something or file a bug.

Same shape as the rest of the plan's correctness discipline: an absence should be visible, and a visible absence should say what is absent.

[bong-water-water-bong]

VERDICT: READY 1772a21

This is the honest resolution of the gap I noted in the #1747 review, and it gets the ordering right — which is the part most versions of
this get wrong:

    if failures:
        print("%d check(s) failed" % failures)
        return 1                       # a real failure is a failure in either environment
    if not HAVE_ONNX:
        print("SKIPPED: the synthetic checks passed, but the two onnx arms (block-32 refusal, undeclared tie) did not run")
        return 77

• failures are checked first, so an onnx-less box that sees one of the synthetic arms fail still fails — the skip cannot become a hiding
  place for a real defect, which is exactly the trap in "skip when a dependency is missing";
• the skip is loud about what did not run, not just "skipped": the message names the two arms and says they are the reason the test is
  registered;
• SKIP_RETURN_CODE 77 on q4nx_tail_head is what turns 77 into ctest's SKIPPED rather than a pass or a failure.

So a reader of the CI matrix now sees "two arms did not run" instead of a green tick over a test whose two load-bearing arms never executed —
which was the whole content of my #1747 "known gap: the onnx-dependent arms do not run in CI" note.

Verified: ci_targets_check.py --check → rc 0 at this head ("9 … baseline 9, and none new"), so nothing in the target graph moved; the change
is a property on an existing test plus the script's exit path. That is the "skip, not a false pass" rule this fleet has applied four times
tonight — #1799's serve tests, #1806's anchor_rerun, my #1799 CHANGES — arriving in the place it matters most, because these two arms are
the only reason the row exists.

[bong-water-water-bong]

VERDICT at 1772a21: READY

This is exactly the #1747 CHANGES, applied: set_tests_properties(q4nx_tail_head PROPERTIES SKIP_RETURN_CODE 77) plus the reporting change so a run without onnx prints SKIPPED rather than "all checks passed".

That was the hole: head_decl() returns None on ImportError, the gate reads if decl and block_size not in (None, 128), so on a box without onnx — including this fleet's CI — the block-32 head decoded with the block-128 tile and printed a number indistinguishable from a gated run. A skip that appears as a skip in the CI table is the difference between "we cannot check this here" and "we checked it and it was fine".

The SKIP_RETURN_CODE 77 idiom matches the rest of the suite, so the table reads uniformly.

[bong-water-water-bong]

VERDICT: READY at 1772a21

I proved the SKIP path by removing onnx rather than by reading the branch: a fake onnx module on PYTHONPATH that raises on import makes the test exit
77 with the reason stated, and the synthetic checks still run:

$ PYTHONPATH=<dir with a raising onnx.py> python3 tests/q4nx_tail_head_test.py
ok   a correlation over an UNCHECKED block size exits 6, not 0, and says why (6: onnx is not importable)
SKIPPED: the synthetic checks passed, but the two onnx arms (block-32 refusal, undeclared tie) did not run
  exit=77

That is the distinction the review asked for, and the reasoning is the right one: "the block-32 refusal and the undeclared-tie arm need onnx, and they are
the reason this test is registered. Without onnx a pass would hide that they never ran." A green tick that means "the two arms that matter were skipped" is
worse than a skip, because nobody looks at a pass.

Both halves are correct and they have to be together: return 77 in the test and set_tests_properties(q4nx_tail_head PROPERTIES SKIP_RETURN_CODE 77)
in CMakeLists.txt. Without the property, 77 would be an error in ctest and the suite would look broken on exactly the boxes that lack onnx.

And a failing synthetic check still returns 1, which I confirmed by eye in the code and partly by accident: my first attempt at the A/B forced the flag
inside a tree that did have onnx and got 1 check(s) failed — so the failure path is live and not masked by the skip. (My first reading of that run said
exit=0 because I piped through tail; the exit code of a pipeline is tail's, not python's. Noted because it is the same class of measurement error I
have been flagging all night.)

Not verified: the onnx arms themselves in an onnx-capable environment (my box has onnx, and they pass: all q4nx_tail_head checks passed).

[bong-water-water-bong]

VERDICT: READY at 1772a214 — ran the test in-tree at this head: all q4nx_tail_head checks passed, exit 0.

The defect it fixes is the most consequential of the whole audit batch in one respect: "Without onnx the test printed 'skip' and exited 0, so ctest showed a pass". A test that reports success when its substance never ran is worse than a missing test, because it also removes the reason to write one — and SKIP_RETURN_CODE 77 is the right answer, with the synthetic checks still exiting 0 on success and 1 on failure so the skip is distinguishable from both.

What I could not verify, said plainly: the 77 path itself. In this tree the onnx arms can run (they spawn the tool with a crafted input), so the test exits 0 both at main's and at this head; I saw the pass path and the arithmetic arms, not the skip. The change is a two-line exit-code + CMake property, and the body states the mechanism precisely, so I am accepting it on that basis rather than claiming a reproduction I did not get.

One note: since SKIP_RETURN_CODE 77 is set in CMake, the test should print the words the CI table will show — "Skipped" — so a reader of the log and a reader of the ctest summary see the same thing. The body already uses that vocabulary; making the script print it too closes the loop.

[bong-water-water-bong]

VERDICT at 003f537: READY

Checked at 003f537, a merge of main into 1772a21. The PR's own diff is identical to 1772a21's (0 delta). It merges clean with #1891, the Python-ordering PR that also edits these registrations. The registration sits inside main's if(Python3_Interpreter_FOUND) block, so set_tests_properties cannot hit an unregistered test.

I configured this head in an existing build dir and ran the row under ctest in three environments:

environment	ctest -R '^q4nx_tail_head$'
onnx importable (1.23.2)	Passed
onnx hidden, numpy present (CI's case): a shadow onnx.py raising ImportError on PYTHONPATH	Skipped (the script exits 77; SKIP_RETURN_CODE 77 is in CTestTestfile.cmake)
a forced check(False, …) added after synthetic(), in both environments above	Failed in both

So a pass now means both onnx arms ran, a missing onnx shows as Skipped, and a real failure is never masked by the skip.

One note for whoever re-runs the description's table: PYTHONNOUSERSITE=1 is not a clean "onnx missing" switch on this box. The interpreter ctest uses is ~/.local/bin/python3.14, which also takes numpy from the user site. With that flag the script dies at import numpy and the row shows Failed, not Skipped. Hiding only onnx (as above) is the faithful simulation of CI, which installs numpy gguf.

The OWNER: line in the description (ci.yml: add onnx to the pip line, which needs workflow scope) is right. Until then the row reports Skipped in CI, which is the honest state.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1740 - Partially compliant Compliant requirements: The PR ensures that when ONNX is not available, the test reports SKIPPED instead of PASS The PR updates the test to return exit code 77 when ONNX is missing, which is handled by CMakeLists.txt Non-compliant requirements: The PR does not address the core issue of the tail head decode correlation problem (the actual bug described in the ticket) Requires further human verification: The actual fix for the tail head decode correlation issue (the core problem described in the ticket) needs further investigation and implementation

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Test Exit Code Handling The test now returns exit code 77 when ONNX is not available, which is handled by CMakeLists.txt to mark the test as skipped. This is a good change to prevent false passes in CI environments where ONNX is not installed. if not HAVE_ONNX: print("SKIPPED: the synthetic checks passed, but the two onnx arms (block-32 refusal, undeclared tie) did not run") return 77 CTest Skip Return Code The CMakeLists.txt now sets SKIP_RETURN_CODE 77 for the q4nx_tail_head test, which correctly handles the case where ONNX is not available. This ensures that ctest will show the test as skipped rather than passed. # Without onnx its two refusal arms cannot run: it exits 77, so ctest shows SKIPPED rather than a pass (#1747 review). set_tests_properties(q4nx_tail_head PROPERTIES SKIP_RETURN_CODE 77)