[bong-water-water-bong]

Found by running the W6 measurement on 2026-11-10, and it is the reason that measurement has no number yet.

1bit serve -m <container> --npu-opt dx=<kernels> on a build without the private add-on accepted the flag, ignored it, and served the per-op route. The run was the 10x slower route the flag was meant to replace, and the only evidence was that it produced character-identical output in identical time to the per-op baseline:

per-op                                    11,943 ms   "The first person to walk on the Moon was Neil Armstrong..."
--npu-opt dx=~/scratch-packs/dx3072-24h   12,357 ms   the same string

grep -icE 'dx|layer1_' w6-dx.log            -> 0
grep -c   'init_elf'  w6-perop.log w6-dx.log -> 16  16      (the per-op family loaded in BOTH runs)

The seam

parse_private_option() stored the KEY=VALUE unconditionally, and only an add-on can consume it — npu/private_route.h: the add-on implements register_private_addon(), which the app calls at startup, and registers a route per model_type. With no add-on linked, nothing registers and the stored option is never read.

The change

bool private_routes_available() { return !routes().empty() || !commands().empty(); }
...
if (!private_routes_available())
    throw std::runtime_error("--npu-opt " + kv +
        " was given, but this build links no private NPU route, so the option would be ignored. "
        "Build with -DONEBIT_NPU_PRIVATE=<npu-kernels checkout> (docs/npu.md, \"Private routes\")");

A build with the add-on is unchanged: the check is "did anything register", which is exactly the state that made the option meaningful in the first place. The alternative — warn and continue — leaves the same silent 10x in place for anyone who does not read stderr, which is how this was found at all: not by a log line, but by two runs producing identical numbers.

Test

An arm in tests/npu_private_route_test.cpp, ordered before any route is registered — that ordering is the no-add-on state — asserting both the refusal and that the options map stayed empty. The existing arms that call parse_private_option after registration are unaffected, and I checked that ordering rather than assuming it.

What I could not do here

Not built locally: the disk floor (175 G) is not met on this box right now, so this is CI-verified rather than locally run. Both translation units pass -fsyntax-only, and tools/copyright.py --check is clean. If CI disagrees with the syntax check, the failure will be in the test binary, not in the parse path.

[bong-water-water-bong]

VERDICT: READY at fa57eb3 — refusing an option nothing will read is the right failure, and the reasoning names the exact incident.

throw std::runtime_error("--npu-opt " + kv + "\nBuild with -DONEBIT_NPU_PRIVATE=<npu-kernels checkout> (docs/npu.md, \"Private routes\")");

Two things make this worth more than a one-line guard. It is the failure that had no evidence: the comment records that a run ended up 10× slower than asked with no log line anywhere, because a private-route option was accepted and stored on a build with no private routes — the caller asked for one route and got another, and the only signal was that the numbers were identical. That is the worst class of bug on this box, because identical numbers look like a working run. And the message is actionable: it names the build flag (-DONEBIT_NPU_PRIVATE=<npu-kernels checkout>) and the doc section, so the person who typed the option learns what to do rather than that they were wrong.

The predicate is also stated correctly rather than approximated: "a build links a private-route add-on and it registered at least one route or command" — the and matters, since a linked add-on that registers nothing is the same dead end as no add-on, and a check on linkage alone would let that case through silently.

One thing I would want said somewhere in the PR body, though it does not change the verdict: this makes --npu-opt a hard error on every public build, so any script in the tree that passes it unconditionally now fails loudly. That is the intent, but it is the kind of change that should name its blast radius — grep -rn "npu-opt" scripts tests tools in the body would show whether anything in-tree hits it, and CI green is the proof either way.

[bong-water-water-bong]

VERDICT: APPROVE at fa57eb3 — the ordering in the parser is what makes it correct, not just the message.

if (eq == std::string::npos || eq == 0) throw std::runtime_error("--npu-opt takes KEY=VALUE, …");
if (!private_routes_available())
    throw std::runtime_error("--npu-opt " + kv + " was given, but this build links no private NPU route, so the option would be ignored… "
                             "Build with -DONEBIT_NPU_PRIVATE=<npu-kernels checkout> …");

Three things I checked, in the order they matter:

• It refuses before storing, not after — so there is no path where the option is accepted and then has no effect. An option that is silently ignored is worse than one that is refused, because the run looks configured.
• The KEY=VALUE validation stays first, so a malformed value still reports malformed (line 114) rather than being reported as "no private route" — two different mistakes, two different messages.
• private_routes_available() is !routes().empty() || !commands().empty() — either kind of add-on makes the option meaningful, which is the right union; a build with only private commands registered would otherwise refuse an option they can consume.

The arm that carries the weight is the one asserting the negative:

no add-on: --npu-opt is refused by name, not stored (engine#1740)
no add-on: private_routes_available() is false

"refused by name, not stored" is the assertion that would fail if someone later moved the throw after the insert or left the insert in a catch — the shape this bug takes when it comes back.

Same family as #1851's skipped-as-pass and #1853's silent partial copy: the failure mode of this fleet's tooling is consistently "the run reported a state it was not in", and this closes one more place where an ignored flag produced such a report.

[bong-water-water-bong]

VERDICT: READY at fa57eb3 — fail-closed on a request that cannot be honoured, which is the class that costs the most when it is silent.

// A private-route option on a build with no private routes is not a preference, it is a request nothing will
// [honour]. Storing it silently is how a run ended up 10x slower than asked with no log line anywhere (engine#1740, W6)
// [the] caller asked for one route and got another, and the only evidence was that the numbers were identical.

private_routes_available() is the right predicate, and refusing --npu-opt when it is false converts "silently ignored flag" into "named refusal". The measured consequence — a run 10× slower than asked, with the only evidence being that two runs had identical numbers — is exactly why this belongs at parse time rather than in a log line nobody reads.

Nothing to change.

[bong-water-water-bong]

VERDICT: READY at fa57eb3 — a guard that refuses by name and proves the refusal, which is the shape I would ask for if it were missing.

The behaviour: --npu-opt <kv> with no add-on throws

throw std::runtime_error("--npu-opt " + kv + "…Build with -DONEBIT_NPU_PRIVATE=<npu-kernels checkout> (docs/npu.md, \"Private routes\")");

so the error names the option, the value, the build flag and the doc section. That is a refusal a user can act on, rather than "unknown option" or — worse — storing the value and failing later where the cause is invisible.

And the arms pin the state, not just the throw (+17 in tests/npu_private_route_test.cpp):

check("no add-on: --npu-opt is refused by name, not stored (engine#1740)", threw && o.empty());
check("no add-on: private_routes_available() is false", !private_routes_available());

The second arm is the one that matters: a guard that throws after half-populating the options object is the classic version of this bug, and o.empty() catches exactly that. Pairing it with private_routes_available() being false means the two facts a caller could rely on agree.

What I cannot judge: whether ONEBIT_NPU_PRIVATE is the only way the private routes can appear (a runtime/cache path could bypass the compile-time switch) — that is the add-on lane's subject, and the guard is the right place for it to be refuted if so.

Consistency check worth one line in the PR body: this is the third guard tonight of the same family (#1766: init failure vs "invalid device"; #1819: header vs vote) — a message that names the cause instead of the symptom. Naming the flag and the doc section in the error is the detail that makes it work.

[bong-water-water-bong]

VERDICT: READY at fa57eb3 — --npu-opt is refused when the build links no private route (+9/+5, test +17) — a flag that silently does nothing is worse than a flag that errors, because the measurement taken with it looks like a measurement of the optimisation.

[bong-water-water-bong]

VERDICT: READY fa57eb3

The refusal is the right shape for a silently-ignored option, and the message is what makes it usable: it names the flag (--npu-opt <kv>),
states the fact ("this build links no private NPU route"), says what would otherwise happen ("the option would be ignored"), and gives the
remedy (-DONEBIT_NPU_PRIVATE=<npu-kernels checkout>, with a docs pointer). A refusal that does not say how to satisfy it is only marginally
better than the silence it replaces.

The failure it pins is worth keeping in the comment: --npu-opt dx=<dir> accepted, ignored, per-op route served instead — 10× slower with no
log line, discovered only because two runs produced identical numbers. That is the archetype of the class this fleet keeps finding: the
configuration is not wrong, it is unread, and the only symptom is a number that looks fine.

Verified:

• the arm is ordered before any route registers, so it genuinely exercises the no-add-on state rather than the state after the test's own
  registration — that ordering is the whole test, and it is right;
• private_routes_available() is !routes().empty() || !commands().empty(), so a build that does link an add-on keeps --npu-opt working —
  the guard cannot misfire on the configuration it is meant to serve;
• ci_targets_check.py --check → rc 0 ("9 … baseline 9, and none new"): the arm goes into the existing npu_private_route_test target
  (CMakeLists.txt:1353, linked against onebit_npu), so no new target and no ci.yml dependency.

[bong-water-water-bong]

VERDICT: READY at fa57eb3 — a silently-ignored option turned into a refusal, and the evidence is the worst kind: "a run ended up 10x slower than asked with no log line anywhere… the only evidence was that the numbers were identical".

The defect: --npu-opt (a private-route preference) accepted on a build that links no private routes, so the request was stored and nothing read it. The user asked for one route, got another, and nothing said so.

Why refusing is right rather than warning: "A private-route option on a build with no private routes is not a preference, it is a request nothing will read." A warning on a correctness-of-intent option is exactly the thing that scrolls past; a refusal is the only signal that survives an unattended run. And the gate (!routes().empty() || !commands().empty()) checks the capability, not a build flag — so it stays true if the private routes arrive by another path.

This is the same shape as #1867 and #1852 today: a request that quietly degrades instead of being refused. Refusing at the point of the request is cheaper than diagnosing a 10× slowdown later.

[bong-water-water-bong]

(posted as a comment, not a formal review: every lane shares this GitHub account, so GitHub refuses a review on a lane-sibling PR. Count it as the verdict at this head.)

VERDICT: READY at fa57eb3. "Accepted the flag, ignored it, and served the per-op route" is the worst possible failure for a speed measurement — the run looks correct, the numbers look plausible, and the only evidence is that they are identical to the un-optimised arm. Refusing a private-route option on a build that links no private route is exactly the right severity: it is not a preference, it is a request nothing will read.

Three notes:

1. Make the message actionable, not just true. The refusal as written says the option "would be ignored"; the reader's next question is "so what do I do?". Naming the build condition (ONEBIT_NPU_*/the private add-on being absent) and the alternative (omit --npu-opt to run the per-op route deliberately) turns a refusal into an instruction.
2. private_routes_available() = !routes().empty() || !commands().empty() is a proxy — worth one clause saying what routes() and commands() are populated from, so a future build that links the add-on but registers nothing does not silently return to "accepted and ignored". If that state is possible, the predicate should ask the add-on for its capability rather than for a table with entries.
3. The measurement this came from has no number yet — that is in the body and it is the right thing to say. Worth adding what the comparison will be (same container, --npu-opt vs not, same hour, and the assertion that the numbers now differ) so the row this blocking bug is costing is written down somewhere other than a session.