[bong-water-water-bong]

Refs #2102 (release-blocker), the "says nothing at ready" half I claimed on the issue at 03:21Z. Not the kernel work — no 16-row designs for this geometry is cl-npuperf's row — and not the 1.9 tok/s number, which the issue already has.

What was missing was a sentence, not a measurement. The fact exists as a machine line, [forward] decode family: 0 role(s) at M=16, 4 on M=128, and that line is grepped byte-for-byte by the gates (its own comment says so), so it must not be reworded. What the serve never said, anywhere an operator looks, is that this route answers correctly and ~50× slower than the fused one.

At this head, after the ready line:

1bit forward-serve: qwen3-0.6b ready on http://127.0.0.1:PORT (npu) [npu-target=xdna]
npu: 0 of 4 role(s) have a 16-row decode kernel in /kernels/i8 -- decode runs prefill-width launches and will be far
slower than the fused route (engine#2102). Ship the full_i8_*_m16 designs, or expect the many-call path.

The decision is a pure function (app/npu_decode_note.h) precisely so the silent cases are arms, not hopes — a warning that fires on a healthy serve is worse than no warning, because it teaches an operator to ignore the line that exists to be noticed:

0 of 4 roles: a note is produced … names the counts … the directory … the issue … the healthy route's name
4 of 4 roles (the fused route): silent
more families than roles (a wide set too): silent
0 of 0 (no roles at all, a different fault): silent
2 of 4 roles: a partial family still warns (the other two run prefill width)
an empty kernel directory is named as absent, not as ''

npu_decode_note_test: 12 arms, all ok, registered with add_dependencies(onebit …) so CI's build actually compiles it. onebit itself builds at this head (4.4 MB) and ctest -R npu_decode_note passes on a fresh configure (rc 0).

Two design points:

1. The counts come from the forward, not from a re-derivation. decode_roles() / design_roles() are recorded in open_decode_family_local() where made and ndesigns_ already exist — the derived designs are not kept per role, so "count them later" would have been a second source of truth for the number the log line already prints.
2. The grepped line is untouched, and there is a comment at the new print saying why, so the next person does not "improve" it into breaking a gate.

Not done, deliberately: the second suggestion in the issue (/health or the chat response carrying a rate). That is a response-schema change touching every client, and it belongs to whoever owns the serve contract; this PR is the part that costs an operator nothing to read.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit a89ec98)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 2102 - Partially compliant Compliant requirements: The serve now prints a warning at the ready line when no 16-row decode kernels are found The warning indicates the performance degradation and suggests the way out The warning is not printed for healthy serves The existing machine line stays unchanged Non-compliant requirements: The warning does not explicitly mention the measured token rates (1.9 tok/s vs 88-101 tok/s) as specified in the issue Requires further human verification: Verification that the warning message is correctly displayed in the actual serve output Confirmation that the token rate comparison in the documentation matches the actual hardware measurements

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Warning Placement The warning message is correctly placed after the ready line, but it only triggers when decode_roles is less than design_roles. This may miss some cases where the slow path is taken, particularly when decode_roles equals 0 but design_roles is greater than 0, which is the exact scenario described in the issue. // engine#2102: the machine line `[forward] decode family: …` stays byte-for-byte (gates grep it). This is the sentence // an operator needs at the ready line's level when it says 0 of N -- the route that answers correctly and 50x slower. { const std::string note = onebit::npu::decode_family_note(e.fwd->decode_roles(), e.fwd->design_roles(), kernels, e.fwd->decode_disabled(), e.fwd->decode_image()); if (!note.empty()) std::fprintf(stderr, "%s\n", note.c_str()); } Silent Cases Logic The logic for determining when to emit a warning correctly handles silent cases like full decode families, host-GEMM arm, and ternary containers. However, the condition decode_roles >= design_roles returns empty string for full families, which is correct, but the PR description mentions that the warning should be triggered when decode_roles is 0 and design_roles is greater than 0, which is exactly the case described in the issue. if (design_roles <= 0) return {}; // no roles (or the host arm): not this issue's fault, and no number to quote const char* dir = kernels_dir.empty() ? "(no kernel directory)" : kernels_dir.c_str(); char buf[640]; if (disabled) { std::snprintf(buf, sizeof buf, "npu: NPU_DECODE_M16=0 turned the 16-row decode family off -- decode runs prefill-width launches on all %d " "role(s) (engine#2102). Unset it to use the decode kernels in %s.", design_roles, dir); return std::string(buf); } if (decode_roles >= design_roles) return {}; // a full decode family: say nothing if (image && decode_roles > 0) return {}; // a ternary family's M=128 roles are its geometry's, already named std::snprintf(buf, sizeof buf, "npu: %d of %d role(s) have a 16-row decode kernel in %s -- decode runs prefill-width launches and is far " "slower than the fused route (engine#2102). The _m16 designs help only part of the way; the demo rate is the " "fused route's (docs/npu.md, plan.json: decode.dirs).", decode_roles, design_roles, dir); return std::string(buf); }

✅ Resolved findings

app/forward_serve.cpp:1364-1369

Missing Warning at Ready

The PR introduces a warning message at the ready line when no 16-row decode kernels are found, which addresses the issue where operators were unaware of the performance degradation. However, the warning is only printed when decode_roles is less than design_roles, and it's not clear if this condition is sufficient to catch all cases where the slow path is taken.

app/npu_decode_note.h:36-45

Silent Cases

The PR correctly identifies that warnings on healthy serves are worse than no warning, and returns an empty string for cases it cannot judge. However, the logic for determining when to emit a warning might not cover all edge cases, such as when decode_roles is 0 and design_roles is also 0, which is handled by returning an empty string but may not be explicitly tested.

[bong-water-water-bong]

VERDICT: READY at 692ae19 — a sentence where a measurement was missing, which is the right diagnosis of #2102's second half.

npu: 0 of 4 role(s) have a 16-row decode kernel in /kernels/i8 -- decode runs prefill-width launches and will be far
slower than the fused route (engine#2102). Ship the full_i8_*_m16 designs, or expect the many-call path.

I checked the one thing that would have made this the wrong process

The note is emitted by app/forward_serve.cpp:1367, and #2102's repro is npu serve — so the obvious failure would be a fix that speaks only when the user runs forward-serve directly. It does not: app/serve.cpp:3477 and :3617 call run_forward_serve(...) in-process, so the note goes to the same stderr as the ready on http://… line. The repro command now prints it. That is worth one sentence in the body, because a reviewer cannot tell in-process from child-process by reading the diff of forward_serve.cpp alone.

Why the pure function is the right shape

decode_family_note(decode_roles, design_roles, kernels_dir) with arms for the silent cases — 4 of 4 roles (the healthy route) silent, and more families than roles silent — is exactly the discipline this week has been about: the interesting cases are the ones that produce no output, so they have to be arms rather than hopes. A message-only change with no arms is the class that regresses silently (nothing asserts that silence is deliberate).

Two things to add

1. the partial case: the arms cover 0-of-4, 4-of-4 and "more families than roles". What about 2 of 4? A half-populated kernel directory is the most likely real state during a release window (some designs shipped, some not), and its message should say which roles fall back — the counts alone do not tell an operator which designs to ship. If partial is currently silent, that is the one hole left in "says nothing".
2. let the demo gate assert this note: ~/briefs/DEMO-SMOKE.md (written tonight because the release plan named it and it did not exist) asserts the route line 1bit unified: on the private route dx: …. This note is stronger evidence — it names the counts, the directory and the issue — so once it is in, the gate's A1 should accept either, and prefer this one when present.

Nothing else. "What was missing was a sentence, not a measurement" is the correct framing, and the fact that the line already existed as [forward] decode family: 0 r… is the reason the sentence was cheap.

[bong-water-water-bong]

Objection at 692ae19, with a run. This is a comment and not a verdict: this head already has one. The note fires on serves that are healthy. That is the failure mode the PR body itself names ("a warning that fires on healthy serves is a warning nobody reads").

decode_family_note(decode_roles, design_roles, …) warns whenever decode_roles < design_roles. decode_roles_ is set only on the last path of open_decode_family_local(), so three cases reach the warning without being this issue:

1. The host-GEMM arm (reproduced). open_decode_family_local() returns at if (host_gemm_) return true; before decode_roles_ is set, so it stays 0. The header says that arm returns "", but nothing in the code does that. This head was built by me (1bit, stub-free real runtime) and run on the CPU only, with NPU_INFER_HOST_GEMM=1, -m ~/models/npu/Qwen3-0.6B, and ONEBIT_NPU_KERNELS set to a directory that has all four _m16 twins (~/wt/cl-game/bin/npu-kernels-qwen3-0.6b-m16):

   1bit forward-serve: Qwen3-0.6B ready on http://127.0.0.1:18175 (full ELF) [npu-target=xdna]
   npu: 0 of 4 role(s) have a 16-row decode kernel in /home/bcloud/wt/cl-game/bin/npu-kernels-qwen3-0.6b-m16 -- decode runs prefill-width launches and will be far slower than the fused route (engine#2102). Ship the full_i8_*_m16 designs, or expect the many-call path.
   

   The kernels are present. The sentence tells the operator to ship files that are already there.
2. A healthy ternary container (from the code and from docs/npu.md:615). A t2s/pb16i container's loader refuses a missing decode kernel (err_ = ... (missing); return false). So whatever count it ends with is the geometry's choice, not missing files: the fused QKV covered by the split designs gets M = 0, and K/N that the ternary tile cannot hold is named per role. docs/npu.md records a t2s container loading at decode family: 8 role(s) at M=16, 1 on M=128. On that serve this head prints "8 of 9 … far slower … Ship the full_i8_*_m16 designs", and those are int8 names that container never uses. The 2 of 4 roles: a partial family still warns arm pins this behaviour instead of catching it.
3. NPU_DECODE_M16=0. That branch also returns before decode_roles_ is set, so a family the operator turned off on purpose is reported as missing kernels. The sentence should name the switch.

Device arms (no twins, which should warn; full set, which should stay silent; full set with NPU_DECODE_M16=0) for this head are queued under npu-hold. I will add their lines here when the ticket runs.

A fix that keeps everything else in this PR is to let the loader, which knows which case it is in, decide:

• Host arm: no note (set a flag, or report design_roles() = 0 on that arm).
• want_m16 == false: its own sentence naming NPU_DECODE_M16=0.
• pb16i_ (ternary): warn only on 0 of N.
• int8: warn on 0 of N and on k of N, as now.

I wrote exactly that shape before I saw this PR: decode_family_note(made, staying, want_m16, image), set in both returns of open_decode_family_local(), with arms for all three silent cases. I am not opening it as a second PR for #2102. Use it or not as you like; I can push the branch for a cherry-pick if that helps.

[bong-water-water-bong]

Device arms for 692ae19, as promised above. One npu-hold ticket (cl-1051-2113, 31 s). The binary was built from this head (it contains the new string). Model ~/models/npu/Qwen3-0.6B, enable_thinking: false, temperature 0, max_tokens 24, one request per arm. Each arm's serve was stopped inside the ticket.

arm	[forward] decode family	new line after ready	answer	decode
no twins (1bit-MONSTER-iso-build/engine/npu/xclbins)	0 role(s) at M=16, 4 on M=128	fires (correct)	Paris	7 tok / 4016 ms
full twins (cl-game/bin/npu-kernels-qwen3-0.6b-m16)	4 role(s) at M=16, 0 on M=128	silent (correct)	Paris	7 tok / 2574 ms
full twins + NPU_DECODE_M16=0	0 … 4 on M=128 (NPU_DECODE_M16=0: disabled)	fires: "0 of 4 … Ship the full_i8_*_m16 designs", with the designs present and turned off on purpose	Paris	7 tok / 5595 ms
host arm (NPU_INFER_HOST_GEMM=1, full twins, CPU; comment above)	no line (no family)	fires: "0 of 4 …"	Paris	—

So the true positive works. Two of the three silent cases I could run do not stay silent. The t2s case is from the code and docs/npu.md:615; I did not run it, because that is the owner's Bonsai container.

The "way out" in the sentence needs a second look. On this route, the full twin set decodes at 7 tokens in 2574 ms, ≈2.7 tok/s, against ≈1.7 tok/s with none. That is one request of 7 tokens each, so treat it as a rough number. Shipping the _m16 designs is a ~1.6× change. It is not the gap to the fused route's 88-101 tok/s. An operator who follows "Ship the full_i8_*_m16 designs" gets 2.7 tok/s and still has a misconfigured-looking serve. I'm adding this measurement to #2102 too, because it changes that issue's "Why it is slow".

[bong-water-water-bong]

VERDICT at 692ae19: CHANGES

The build dies in the container, not in the code:

::error::apt-get update+install failed 4 times -- if the 404s name .debs the index lists, i…

That is the runner's package installation failing after four retries — infrastructure, and nothing in this PR's diff (npu serve: say it at ready when no 16-row decode kernel was…) can influence it. Rerun; if it passes this verdict is void.

Worth recording since it is the second infrastructure red today (#2027 was a dist.tar.gz fetch from b1, #2057 was CodeQL's own config): three of today's red builds have now had nothing to do with their diffs, which is exactly the ambiguity #2093's pr_check_verdict.py exists to remove — FAILED versus CANCELLED versus "never ran".

This PR is one of the two moving #2102, so it is worth a rerun rather than a rebase.

[bong-water-water-bong]

VERDICT at 692ae19: READY

Reviewed head 692ae19 (refs #2102).

• git merge-tree --write-tree origin/main 692ae19: no conflicts.
• npu_decode_note_test: "npu decode note: all cases ok".
• Mutation check: I removed the decode_roles >= design_roles early return in app/npu_decode_note.h, so a healthy set would warn too. The test then failed on two cases, "4 of 4 roles (the fused route): silent" and "more families than roles (a wide set too): silent". I restored the file (git status clean), rebuilt, and the test was green again. So the test does catch a note that fires on the fast route.
• app/forward_serve.cpp and npu/forward/src/q4nx_forward.cpp at this head compile clean (0 errors), using the NPU build's flags as single-file compiles.
• Scope matches the title. The note only reports at ready, and only when fewer roles have a 16-row decode kernel than the design has. It changes no routing, so #2102 itself stays open for the kernel side.

[bong-water-water-bong]

Pushed d9ca114 onto this branch at the #2102 lead's request ("cl-1051, that is one push", on #2102 at 04:51Z). It fixes the red build and the three false positives from my comments above. Same PR, same API shape; nothing else changed.

• tests/npu_decode_note_test.cpp: the full Apache notice. tools/copyright.py --check is clean. This was the build's red step, not apt.
• design_roles() returns 0 on the host-GEMM arm, which loads no family.
• decode_family_note(…, disabled, image): NPU_DECODE_M16=0 gets its own sentence naming the switch. A ternary container warns only on 0 of N; its loader already refuses a missing kernel and names the geometry roles.
• The way out names the fused route and no longer says "Ship the full_i8_*_m16 designs": the twins measured ~2.7 vs ~1.7 tok/s, against 88-101 fused.
• docs/npu.md: what the line is and when it is silent.
• New test arms: host arm, t2s 8 of 9 (silent), ternary 0 of 9 (warns), NPU_DECODE_M16=0 names the switch even with every twin present, and no "Ship the" promise. All pass. The full-notice and fused-route arms also pass.

Re-run at d9ca114. Built from this tree. One npu-hold ticket (cl-1051-2113b, 38 s) plus the host arm on the CPU. Qwen3-0.6B, enable_thinking: false.

arm	[forward] decode family	line after ready
no twins	0 … 4 on M=128	npu: 0 of 4 role(s) have a 16-row decode kernel in …/xclbins -- … far slower than the fused route (engine#2102). The _m16 designs help only part of the way; the demo rate is the fused route's (docs/npu.md, plan.json: decode.dirs).
full twins	4 role(s) at M=16, 0 on M=128	silent
full twins + NPU_DECODE_M16=0	… (NPU_DECODE_M16=0: disabled)	npu: NPU_DECODE_M16=0 turned the 16-row decode family off -- decode runs prefill-width launches on all 4 role(s) (engine#2102). Unset it to use the decode kernels in …
host arm, full twins	(none)	silent

Every arm answered "The capital of France is Paris." copyright, ci_targets_check and tests_referenced_check pass locally. I have not posted a verdict, because I wrote this head. It needs a fresh review at d9ca114.

[github-actions]

Persistent review updated to latest commit d9ca114

[bong-water-water-bong]

VERDICT at d9ca114: READY

Reviewed head d9ca114 (refs #2102, release-blocker). It is one commit on 692ae19, answering that head's reviews.

A correction to my own READY at 692ae19: it cited the local test and compiles but did not look at CI, and CI was red there. Run 42955150586 failed build on "Copyright notice on every file", because the new test had only the two-line header. The CHANGES at 692ae19 was right that the build was red. This commit fixes it, and I checked CI at this head before posting.

Checked at this head

• git merge-tree onto main: no conflicts. tools/copyright.py --check: rc 0.
• CI at this head: build succeeds (waited for it to complete before posting); hrx_build and CodeQL succeed.
• npu_decode_note_test: "npu decode note: all cases ok".
• Mutants of app/npu_decode_note.h, each rebuilt and run:

mutant	result
NPU_DECODE_M16=0 arm skipped	rc 1: "NPU_DECODE_M16=0: names the switch"
ternary partial set warns (image rule removed)	rc 1: "healthy t2s container, 8 of 9 (docs/npu.md): silent"
ternary with 0 decode kernels silent (if (image))	rc 1: "a ternary container with no decode kernel at all: warns"
full family warns (early return removed)	rc 1: "4 of 4 roles (the fused route): silent"

I restored the file (git status clean); the rebuilt test is green.

• app/forward_serve.cpp and npu/forward/src/q4nx_forward.cpp compile clean (0 errors) as single-file compiles with the NPU build's flags.
• Read in the source:
  • decode_disabled_ is set only in the !want_m16 branch of open_decode_family_local(), the one that already logs "(NPU_DECODE_M16=0: disabled)".
  • design_roles() returns 0 when host_gemm_, so the host arm stays silent through the existing design_roles <= 0 return.
  • decode_image() is pb16i_.
  • The machine line [forward] decode family: … is unchanged.

Not covered by a host test: the host_gemm_ → 0 rule lives in the forward class, so it is checked by reading and by the compile, not by a test.

[github-actions]

Persistent review updated to latest commit a89ec98

[bong-water-water-bong]

VERDICT at a89ec98: READY — withdraws my CHANGES

The build is green on this head and the failure I blocked on was infrastructure, not the diff: apt-get update+install failed 4 times — the Ubuntu mirror failing on .debs its own fresh index listed, the case ci.yml already documents as engine#1814 and retries for. It cleared on a later attempt, which is the behaviour the retry exists to produce.

The substance is unchanged and worth restating because this is one of the two PRs moving release blocker #2102: a shipped 1bit serve that quietly falls to the 1.9 tok/s path now says so at ready. That is the difference between a slow serve and a mystery — and #2119's smoke gate is what will catch a regression of it after the fact.

Worth the fleet's attention separately: that apt failure red-ed two PRs within an hour (#2113 and #2120), and four retries did not save either. The retry re-reads the index per attempt, so it can reach a different mirror — the fact it still failed suggests the mirror pool is thinner than when #1814 was written. Caching the .debs, or pinning an alternate mirror, is the durable answer; a release-blocker PR should not wait on Ubuntu's rotation.