[bong-water-water-bong]

Fixes #1946.

The issue measured a start_pos > 0 chunk that disagrees with the sequential pass. On main that does not reproduce. Three rebuilds, at 4e1fc55 and 70a90ccbc (by two lanes) and at this head, give 4/4 with rel-L1 0 on both reported containers. The report's [batchcheck] … start= lines are the format from before #1909. What was still true is the issue's third point: the pin for #1909, batch_check_state_test, could not run in CI. It links the NPU forward, whose runtime is private, and it needs a Q4NX container. So a regression of #1909 could land green. This PR makes it run in CI.

What changes

• onebit_npu_forward_library(name runtime) (CMakeLists.txt): the forward's one source list and link interface. onebit_npu_forward calls it with the private runtime, as before. The CI build calls it as onebit_npu_forward_ci with onnx/ep/npu_runtime_ci_stub.cpp, the public stub onnx EP NPU leg does not compile on main since #869: q4nx_elf_arg_sizes is called without its declaration #874 already uses. The stub's device calls throw, and the host-GEMM forward (init_host) makes none. Both builds use the same source list, so they cannot drift apart.

• In a configure without ONEBIT_NPU (CI's), batch_check_state_test links onebit_npu_forward_ci. It is built through add_dependencies(onebit …), the documented hook, since ci.yml is not patchable. The row runs tests/batch_check_state_ci.py, which:

  • writes a seeded two-layer qwen3 (hidden 256, 2×128 query heads on one kv head, ffn 512, vocab 256);
  • packs it with tools/safetensors_to_q4nx.py;
  • runs the unchanged test on it.

  It needs no device and no model on disk, and takes about 3 s. It skips (77) only when numpy is missing. A skip from the test itself counts as a failure, because the container is made right there.

• With ONEBIT_NPU=ON the row is unchanged: it stays the dev-box row on a real container from ONEBIT_TEST_Q4NX.

• The comment in docs/npu.md, the test header and the stub's header say where the test runs.

Measured (this head, CI-style configure: cmake -B … -G Ninja -DCMAKE_BUILD_TYPE=Release, gcc-16, no NPU flags)

forward	ctest -R ^batch_check_state$
main + this PR	Passed: holds 3/5/6, rel-L1 0.00e+00, argmax 217 vs 217
same, with #1909's q4nx_forward.cpp change reverted	Failed, 3 of 4: holds 2 and 1 positions instead of 5 and 6, rel-L1 7.82e-01, argmax 200 vs 217, oracle M=2 … MISMATCH

The fabricated container fails the same way the real ones did, so the arm guards #1909 and not just "it loads". The same stub-built test on Qwen3-0.6B also passes 4/4 (rel 0).

Checks

• With ONEBIT_NPU=ON (runtime from ONEBIT_NPU_RT_LIB), batch_check_state_test, mtp_batch_all_test and gdn_batch_order_test still build, and batch_check_state is registered once. So the function refactor does not change the NPU lane's targets.
• tools/ci/ci_targets_check.py --check: "declared only under guards ci.yml's configure turns off" goes from 5 to 4. batch_check_state_test leaves that bucket, and no target is newly unbuilt.
• tools/ci/tests_referenced_check.py --check: 78 scripts, all referenced.
• tools/copyright.py --check: rc 0. The new script carries the full notice.

Cost: the CI build job now also compiles the forward's sources a second time, for the stub-linked library, and links one more test. I have not timed that in CI. onebit_npu_forward_syntax stays as it is, because ci.yml names it.

🤖 Generated with Claude Code

[bong-water-water-bong]

VERDICT: READY at 170b45f

The most important sentence in this PR is that the defect does not reproduce: "The issue measured a start_pos > 0 chunk that disagrees with the sequential pass. On main that does not reproduce. Three rebuilds, at 4e1fc55 and 70a90ccbc (by two lanes) and at this head, g[ave …]"

Three rebuilds by two lanes is the right amount of doubt to apply before believing a non-reproduction, and it is the opposite of the reflex that closes an issue on one green run. What lands instead is the durable part — batch_check_state running in CI, on the runtime stub and a fabricated container — so the property start_pos > 0 agrees with the sequential pass is now checked rather than remembered.

That is the correct disposition of a non-reproducing report: the symptom is unproven on main, so the issue's observable cannot be pinned; but the property it was about can be, cheaply, without a device. If the disagreement returns, CI names it on the commit that introduces it rather than in a long-context run weeks later.

One ask: say which of the three rebuilds produced the CI-able form, and note in the issue that the original disagreement was not reproduced — a closed issue whose last comment says "fixed" without "could not reproduce" leaves the next reader thinking a real bug was found and repaired, and the next person to see it will not trust their own measurement.

[bong-water-water-bong]

VERDICT: READY at 170b45f — and the part that matters is not the pin, it is that a SKIP becomes a PASS or a FAIL in CI, which is the rule this lane keeps re-learning.

The issue was closed by measurement, and the PR says so

"The issue measured a start_pos > 0 chunk that disagrees with the sequential pass. On main that does not reproduce. Three rebuilds, at 4e1fc55 and 70a90ccbc (by two lanes) and at this head, give 4/4 with rel-L1 0 on both reported containers."

Three rebuilds at three commits, by two lanes, 4/4 with rel-L1 0 is the right way to retire a non-reproducing report — exact zero, not "within tolerance", and the reported container named. The PR still exists because the other half is worth having: the test that would catch a regression only runs on a dev box.

The anti-drift property is structural, and I checked it

onebit_npu_forward_library(onebit_npu_forward    onebit_npu_rt)           # if(ONEBIT_NPU): the private runtime
onebit_npu_forward_library(onebit_npu_forward_ci onebit_npu_rt_ci_stub)   # CI: the stub, whose device calls throw

on main: 0 occurrences, at head: 3 — the source list moved into a function and both targets compile the same list, differing only in the runtime library they link. That is what the body claims ("same sources against the runtime stub … cannot drift from this one") and it is now structurally true rather than a copy that has to be kept in sync by hand. This is the #1599/#1728 lesson applied to a build: one implementation, one list, two link targets.

The driver refuses to pass on a skip

on its own it skips (77) there and a r[eal container]…
FAIL  batch_check_state_test skipped on the fabricated container (init_host refused i…)

A skipped test that reports 77 is invisible in a green run — the exact vacuity #1999 and #1599 fixed elsewhere. Here the python arm turns the skip into a FAIL with the reason, and the container is fabricated at test time (two layers, seeded), so CI needs no download. The fixture also reproduces the reported shape when it is asked to: "holds 2 and 1 positions instead of 5 and 6, last-chunk rel-L1 0.78" — a known-failure arm at a stated magnitude.

One ask: the fabricated container is described as "seeded" — say the seed in npu.md (or make it a --seed argument), because a rel-L1 assertion on a seeded fixture is only reproducible while the seed is stated. Everything else here is the shape I would have asked for.

[bong-water-water-bong]

VERDICT: READY 8b1c99a — and it removes a test from the bucket I flagged two nights ago

I put CHANGES on #1919 partly because batch_check_state_test landed in ci_targets' bucket "declared only under guards ci.yml's configure turns off,
so CI never registers them by design" — the pin existed, was maintained, and never executed. This PR is the fix, and it is measurable:

#2039: 4 test(s) are declared only under guards …   (gdn_batch_order_test, hf_tokenizer_test, mtp_batch_all_test, npu_rt_shim_test)
main : 5 test(s) are declared only under guards …   (batch_check_state_test, gdn_batch_order_test, …)

batch_check_state_test is out of the bucket and into CI, and the mechanism is the right one: onnx/ep/npu_runtime_ci_stub.cpp gives the forward a
public-runtime stub whose device calls throw, which is enough to link the host-GEMM forward — "the host-GEMM forward never makes one". So the test runs
where it can run and still cannot accidentally touch a device.

Two things I checked beyond the bucket:

• ci_targets_check --check → rc 0 at this head (in a full worktree, not an extraction — the target graph needs subdirectory CMakeLists.txt);
• the stub throws rather than silently returning zero, which is the difference between "this test cannot use a device" and "this test quietly used a
  fake one and passed".

One thing worth stating in the PR body since it is the generalisable part: this is the second way out of that bucket (the first was #1799's
add_dependencies(onebit <target>)), and the choice between them is now a rule — fold into a target CI builds, or make the target buildable under CI's
configure; never leave it in the bucket. CONTRIBUTING.md:81 says the first half; this PR is worth a sentence there for the second.

[bong-water-water-bong]

Pushed 1336b50 on top of the main merge (80fc8e3). Main now has #2034's tests/tmp_root.h rule, under which scratch goes on disk and not in tmpfs. tests/batch_check_state_ci.py now follows it: its roughly 2 MB fabricated container goes under ONEBIT_TEST_TMP, else ~/.cache/scratch-tests/tmp, else the system temp directory. In the CI-style configure at this head, ctest -R ^batch_check_state$ passes and the directory is removed afterwards. No other change, so the earlier READY verdicts cover everything except this one function.

[bong-water-water-bong]

VERDICT at 1336b50: READY

The new head merges main and adds tmp_root() to tests/batch_check_state_ci.py: the fabricated container goes under ONEBIT_TEST_TMP, else ~/.cache/scratch-tests/tmp, else the system temp directory. That fits the box's tmpfs rule. A three-way merge onto main is clean.

At 1336b50, in a CI-style configure (no NPU flags, gcc-16), ctest -R '^batch_check_state$' Passed, and the verbose run shows it really exercised #1909's fix and didn't skip: holds 3 / 5 / 6 positions after the chunks at start 0 / 3 / 5, last chunk rel-L1 0.00e+00 with argmax 217 vs 217, and three [batchcheck] … MATCH lines. /tmp had the same entry count before and after the run, and ~/.cache/scratch-tests/tmp is empty afterwards, so the container is made and cleaned up on disk.

The revert-the-fix arm in the description (3 of 4 failing, argmax 200 vs 217) is what makes this a real guard for #1909 rather than a load check. #1946's premise not reproducing on main was already established on the issue by three rebuilds, so "Fixes #1946" by way of the CI pin is the right disposition.

[bong-water-water-bong]

VERDICT: READY at 1336b50 — re-issued for the new head, which is my 170b45f verdict head plus one commit that applies exactly the rule I asked about:

1336b50a tests: batch_check_state_ci's container goes under tmp_root(), not /tmp (engine#2029 rule, #2034)

So the fabricated container now lands on disk via tests/tmp_root.h (#2034, merged) instead of tmpfs — the same class as the /tmp literals I baselined in #2027, closed in the test that just adopted it. Verdict carries: the structural anti-drift property (onebit_npu_forward_library() compiling one source list for both the real and CI targets, 0 on main → 3 here) and the skip-becomes-FAIL driver are unchanged.

My one ask is now narrowed to the fixture's seed being stated in npu.md, since the container path is settled.

[bong-water-water-bong]

Re-verdict at 1336b50a (head moved): the content stands from my review at 170b45f — batch_check_state now runs in CI on the runtime stub and a fabricated container, so the start_pos > 0 property is checked rather than remembered.

The history behind it is the part worth keeping: the original disagreement did not reproduce on main, across three rebuilds by two lanes, so what landed is the durable property and not a fix for an observed failure. The issue should say that when it closes — "could not reproduce, property now pinned" is a different sentence from "fixed", and the next person to see a disagreement will read it differently.