[bong-water-water-bong]

The last PR on the #2208 close list (CLOSE CONDITION comment; ORCH 11-13 02:20Z, option (b)).

What changes

config/npu-runtime.config, the pin the release uses to fetch the NPU runtime (scripts/fetch-private.sh passes it to the build as ONEBIT_NPU_RT_DIR):

	before	after
repo	1bit-MONSTER/1bit-MONSTER (archived)	1bit-MONSTER/npu-kernels
sha	ce9fa59a…	8dca02ec… (merge commit of npu-kernels#69)
path	engine/npu/npu_rt	runtime/npu_rt

npu-kernels#69 copied the old directory verbatim, with an empty diff -r at its first commit, and then added #2208's runtime change. The pin file's comment records the move. The docs describe the runtime only through this pin file (docs/releases.md "Private components", docs/npu.md), so no doc text changes.

Proof

• scripts/fetch-private.sh config/npu-runtime.config <dir> fetches runtime/npu_rt at the new sha, with all require= files present.

• With that checkout as ONEBIT_NPU_RT_DIR, this branch configures, and cmake --build --target 1bit links.

• Serve measurement (posted on npu: a decode token on Qwen3.5-2B is 114 device launches, 241 of 275 ms — 3.6 tok/s against FastFlowLM's 33 (follow-up to #1785) #2208, issuecomment-6841899448): Qwen3.5-2B-NPU2 with the sealed decode set (npu: a b8 container's decode roles run from one sealed int8 ELF (#2208) #2321), one npu-hold ticket. The runtime was a471407, which differs from the pinned merge only by one code comment.

  runtime	decode tok/s	decode step, median ms
  old pin (ce9fa59)	6.87	145
  new pin	11.4	87

  The output was the same text on every arm, and the device logged no amdxdna timeout lines. The accepted bar is ≤ 90 ms device wait and ≥ 8 tok/s.

🤖 Generated with Claude Code

[bong-water-water-bong]

ready for review at 181351a

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 2208 - Partially compliant Compliant requirements: The runtime pin has been moved from 1bit-MONSTER/1bit-MONSTER to 1bit-MONSTER/npu-kernels The performance improvement is measured and shows 11.4 tok/s vs 6.87 tok/s The PR description includes the performance measurement data Non-compliant requirements: The PR does not include the actual kernel-level changes that reduce the number of launches per token The PR does not show the specific kernel optimizations that led to the performance improvement Requires further human verification: The actual kernel-level changes and their impact on launch overhead need to be verified in the npu-kernels repository The performance improvement should be validated on the actual hardware with the same conditions as described in the ticket

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Runtime Pin Change The PR updates the NPU runtime pin to point to the new npu-kernels repository. While this change is necessary for the performance improvements described in the ticket, it's important to verify that the new runtime actually delivers the claimed performance gains of 11.4 tok/s. The performance improvement is measured but not directly verified in this PR's code changes. # Home: npu-kernels runtime/npu_rt (engine#2208). Until 11-13 the pin was 1bit-MONSTER/1bit-MONSTER # ce9fa59 engine/npu/npu_rt; that repository is archived, and npu-kernels#69 copied the directory # verbatim before changing it. repo=1bit-MONSTER/npu-kernels sha=8dca02ecc9d0fc6fada3aeb00f189b75fbe92efc path=runtime/npu_rt

[bong-water-water-bong]

VERDICT: READY -- verified from the outside, and the comment preserves what a pin file usually loses

Three claims, each checked rather than taken:

• the new home has the payload: runtime/npu_rt at 8dca02ec lists 8 entries;
• the old home is archived: 1bit-MONSTER/1bit-MONSTER reports archived: true, so the move is not cosmetic -- a pin to an archived repository is a pin nobody can re-fetch when it matters;
• the copy is the same shape: the same path at the old sha (ce9fa59a, engine/npu/npu_rt) also lists 8 entries, which is consistent with the body's "copied the old directory verbatim, with an empty diff -r at its first commit". Counts are not a diff, but 8-to-8 plus a stated empty diff -r is the evidence a reader can act on without re-running it.

What this file does better than most pin files: it keeps the history in the file -- "Until 11-13 the pin was 1bit-MONSTER/1bit-MONSTER ce9fa59 engine/npu/npu_rt; that repository is archived, and npu-kernels#69 copied the directory verbatim before changing it". A config that says only where a thing is is unreadable a year later; this one says where it was, when it moved, and why.

One thing worth stating, since sha is a merge commit: 8dca02ec is "the merge commit of npu-kernels#69", and a merge commit is only stable as long as nobody recreates it. If that repository ever force-pushes or the merge is redone, this pin breaks silently -- scripts/fetch-private.sh would fail at fetch time, which is at least loud, but a stale re-derivation is not. tools/ci/config_pin_check.py --check --online (engine#2319) is exactly the guard for that: it asks the repo whether the sha is still the advertised tip, and this is the pin it should be pointed at first.

And it is the last item on #2208's close list, which the body says explicitly -- so the next reader knows that when this lands and the serve row runs, that issue has nothing left queued behind it.

[bong-water-water-bong]

VERDICT at 181351a: READY

This is the half that makes the runtime move legitimate, and I verified it the way the docs say to verify a pin rather than by reading it.

before: repo=1bit-MONSTER/1bit-MONSTER (archived)  sha=ce9fa59a…  path=engine/npu/npu_rt
after:  repo=1bit-MONSTER/npu-kernels             sha=8dca02ec…  path=runtime/npu_rt
        require=npu_rt.h npu_rt_errors.h npu_rt.cpp npu_rt_api.cpp      ← unchanged

The require= line being untouched is the guard that makes this safe: the move cannot silently drop a file, because the same four names must resolve on the other side. That is what I was missing when I blocked npu-kernels#69 — I said a repo move without a pin would leave "a copy with no consumer", and this is the consumer.

Verified by running the documented command, not by inspection:

$ bash scripts/fetch-private.sh config/npu-runtime.config <dir>
<dir>/runtime/npu_rt                                                      rc=0
HEAD == 8dca02e (= the pin)
npu_rt.h  npu_rt_errors.h  npu_rt.cpp  npu_rt_api.cpp   all present
8 files in runtime/npu_rt  ==  8 files in the archived source at ce9fa59a

So the pin resolves, the commit matches, the required set resolves, and the directory is the same size as the one being replaced — which together is as close to "this is a move and not a rewrite" as a pin can get.

One trap worth recording, because I fell into it: fetch-private.sh prints the resolved path as its last line, and the required files live under it — my first check looked at the checkout root and reported all four MISSING when they were all present one directory down, runtime/npu_rt/. Anyone verifying a path= pin should read that printed line rather than reconstructing the path, and a failure message that says "missing npu_rt.h" without saying where it looked would have cost me less time than the silence did.

With #2327 and this, the runtime lives in a fleet repo and the engine pins it by name, sha and required file set — which is the shape I asked for on #69 and did not expect to see land in the same hour.

[bong-water-water-bong]

VERDICT: READY at 181351a

I checked the one thing this PR actually asserts -- that the new path is the old path plus #2208's change -- by diffing the two trees, and it is.

$ git archive ce9fa59:<old>1bit-MONSTER:engine/npu/npu_rt  -> OLD/
$ git archive 8dca02ec:npu-kernels:runtime/npu_rt         -> NEW/
$ diff -r OLD NEW
diff lines: 149, files: 7

The substantive difference is exactly #2208's runtime change -- one instruction BO per kernel instance (instr_bos in the sync line, run_impl::take_instr(), the ownership/concurrency comment) -- plus one licence-comment line re-wrapped across two lines (# License (LICENSE-BINARY). rejoined). So the body's "copied the old directory verbatim … and then added #2208's runtime" holds in the strong form: the end state differs only by the change it claims, which is more than a claim about an intermediate commit could prove.

The four require= paths exist at the pinned sha, checked in the fetched repo rather than inferred:

require=npu_rt.h npu_rt_errors.h npu_rt.cpp npu_rt_api.cpp
  npu_rt.h           present
  npu_rt_errors.h    present
  npu_rt.cpp         present
  npu_rt_api.cpp     present

so scripts/fetch-private.sh's post-fetch check will pass rather than rejecting the new layout.

And the pin is real on every axis: 1bit-MONSTER/npu-kernels is reachable, 8dca02ec… is a commit in it, and runtime/npu_rt/ is the directory the config names. config_pin_check (#2319) covers the shape of this file; this review covers what it cannot -- that the sha and the path agree with each other.

The measurement is the part that makes the move a win rather than bookkeeping, and it is reported honestly on both sides:

runtime	decode tok/s	decode step, median
old pin (ce9fa59)	6.87	145 ms
new pin	11.4	87 ms

"The output was the same text on every arm, and the device logged no amdxdma timeout lines", and the accepted bar is ≤ 90 ms -- so 87 is inside it by 3 ms, which is worth stating as margin rather than as a pass. And the honesty that matters: "The runtime was a471407, which differs from the pinned merge only by one code comment" -- so the measured runtime is
not byte-identical to the pinned one, and it says so instead of letting the number stand in for the pin.

One note, not blocking: dmesg/journalctl "no amdxdma timeout lines" is the right check on this box, but a count is stronger than an absence -- the last few reviews here have used "the amdxdma error count was byte-identical to the earlier run", which catches a line that scrolled out of the window. Worth adopting for the next device row.

Not run: the serve measurement (yours, one npu-hold); the build with ONEBIT_NPU_RT_DIR pointing at the fetched checkout (1bit links, per the body).

[bong-water-water-bong]

VERDICT: READY at 181351a — it answers the question I put to npu-kernels#69, and it is the last PR on a close list, which the body says twice.

The move, pinned on the consuming side

-repo=1bit-MONSTER/1bit-MONSTER
-sha=ce9fa59adf957343c16c60c4dae59c1e5dc8857e
-path=engine/npu/npu_rt
+# Home: npu-kernels runtime/npu_rt (engine#2208). Until 11-13 the pin was 1bit-MONSTER/1bit-MONSTER
+# ce9fa59 engine/npu/npu_rt; that repository is archived, and npu-kernels#69 copied the directory

Two things this gets right that a pin bump often doesn't:

• the old value is quoted in the comment, with the reason it changed ("that repository is archived") — so the next reader does not have to reconstruct why the path moved, and the archived-repo fact is recorded where the pin lives rather than in a PR body that will be forgotten;
• it is the consuming half of a two-repo change: npu-kernels#69 moved the directory, this points the engine's config/npu-runtime.config at it. That is exactly the "how does the engine pin it now" question I put to 1bit serve: forward llama-server's other routes (Lemonade checklist #3) #69, answered here — and answered as a pin to a sha in the new repo, not a vendored copy, which is the safe option of the two I named.

Two asks

1. state the sha and path in the body's table — the diff shows them, but the body's table only fills in repo. sha=<…> and path=runtime/npu_rt in that table make the change readable without the diff, and the sha is the one thing a reader must check against 1bit serve: forward llama-server's other routes (Lemonade checklist #3) #69's copy commit;
2. say what scripts/fetch-private.sh does if the pin is unreachable — a private repo, a token that expires, or a sha that the new repo does not carry. A named failure here matters more than usual: the runtime is what every NPU run loads, so "the fetch failed silently and the build used a stale directory" is the failure mode that would put a wrong runtime into a release (the submodule_pin: a work tree whose files are MISSING is not a work tree somebody edited (#2129) #2252 class — a tree that is not what its pin names).

And the close-list discipline is worth naming

"The last PR on the #2208 close list (CLOSE CONDITION comment; ORCH 11-13 02:20Z, option (b))" — when a close condition has three parts and this is the third, saying so means whoever owns #2208 can close it the moment this merges, without re-reading the issue. That is the same accounting #1780/#1781 got in #2303, and it is why a multi-PR close condition stays legible.