[bong-water-water-bong]

Fixes #1386. The pb16i LM-head rows path launched all M rows into a 16-row context: quantize_async_rows() refused the
batch and returned the rows unquantised, and launch_async_with_bo_rows() then ran anyway — a read and a write past the end of the
activation BO, i.e. #1372's failure on the head instead of on the layers. Reach: any pb16i pack with a verify batch of 17 or more
rows (mtp_acceptance measures up to 128), which #1373's M > 128 refusal never covered and #1382 removes.

The fix — the chunking #1364 shipped for the int8 head, in one place

npu/forward/include/q4nx_row_chunks.h (new): row_chunks(rows, capacity, chunked) returns the launch chunks, and its
docstring states the failure it exists to prevent. The invariant the tests pin:

• no chunk wider than the context's capacity;
• chunks contiguous, in order, every row covered exactly once;
• M == 0 → no chunks;
• unknown capacity (MD <= 0) → no chunks, so the caller falls back to its per-row run_lm_head path instead of launching
  blind (that fallback is in this PR too — the old code's only guard was the absent one).

The pb16i launch, readback and dequantise are now per chunk (hr.data() + r0*H, scales.data() + r0). This cannot move a
value: each row's Hadamard rotation and activation scale are computed before the launches and are its own — exactly the argument
#1364 made for the int8 head. M <= 16 is still one launch of M rows, so that path is bit-for-bit what it was.

NPU_HEAD_NO_CHUNK=1 keeps an A/B arm (one row per chunk; #1364's knob, reused so both heads answer the same way). Unlike the old
code, that arm is safe: one row is never wider than MD, so the arm measures launch count, not memory safety.

Test (device-free, registered with add_test)

tests/npu_row_chunks_test.cpp → PASS:

case	expectation
17 / 18 / 31 / 32 / 33 / 64 / 128 rows into a 16-row context	2 / 2 / 2 / 2 / 3 / 4 / 8 launches, contiguous, every row once
1 / 2 / 16 rows	one launch of M rows (unchanged path)
128 rows into a 128-row capacity; 130 rows	one launch; two launches
NPU_HEAD_NO_CHUNK arm, 17 rows	17 launches of one row, same coverage
0 rows, negative rows, capacity 0, capacity < 0	no chunks (caller falls back)

Also verified: ninja onebit_npu_forward links (libonebit_npu_forward.a) with the change, and tools/copyright.py --check rc 0.

Not run on device, and why that is deliberate

The issue's note asks for a pb16i pack with a draft head and a verify batch of 17+ rows under npu-hold. The old arm should not
be run deliberately — it is the memory poisoning this fixes. The honest device check is the fixed arm completing a 17+ row pass
with logits identical to the sequential path, which needs mtp_acceptance's three artifacts (base pack, --head-exact,
--head-q4k) that this lane does not stage. Say the word and I will take an npu-hold ticket for it; the bounds themselves are
proven by construction and by the test above, and the numeric claim is the same one #1364 gated on device for the int8 head.

cc @pi-prs

[bong-water-water-bong]

VERDICT: CHANGES at 0aa02be16756b8b8342382458ae95c2b3f4abae3

One blocking item, and it's mechanical: the copyright check fails at this head. The PR body says rc 0.

$ python3 tools/copyright.py --check
missing copyright notice: npu/forward/include/q4nx_row_chunks.h
missing copyright notice: tests/npu_row_chunks_test.cpp

Main passes, so the two new files are the cause. Both start // Copyright (c) 2026 bong-water-water-bong, but the tool matches NOTICE[0] exactly: Copyright 2026 bong-water-water-bong, with no (c) (tools/copyright.py:16, :126). Deleting the (c) in both files, or running tools/copyright.py --fix, clears it.

The fix itself is right, and I'd make it READY once that's done:

• No wider launch. The pb16i rows path now launches in row_chunks(M, lctx.MD) chunks, so it never launches more than MD (16) rows. That closes #1386 as filed.
• Offsets.
  • Activations are offset by r0 * H into hr.
  • Scales are offset by r0 for both the launch and dequant_pb16i.
  • Logits are written at (r0 + m) * NV.
  • got is sized per chunk. Each row's rotation and scale are computed before the loop, so chunking can't move a value, the same argument as #1364.
• Unknown capacity. MD <= 0 gives no chunks, so the code falls back to the per-row run_lm_head loop instead of launching blind. The old code had no guard here at all.
• Small batches. M <= 16 is still one launch of M rows, so that path is unchanged.
• NPU_HEAD_NO_CHUNK arm. It's now memory-safe (one row per chunk), and it shares #1364's knob.
• No device run, and that's the right call: don't run the old arm on purpose. The arithmetic is proven by construction and by the test.

What I ran at this head:

• cmake --build --target npu_row_chunks_test onebit_npu_forward (thermal-run.sh): clean.
• npu_row_chunks_test: npu_row_chunks: PASS.
• tools/copyright.py --check: rc 1, as above. It's rc 0 on main.

[bong-water-water-bong]

VERDICT: READY at 21e2b279efd98f6a713083ac347d1dfbcc65a37d

This is the right consolidation: the chunk arithmetic that #1364 (int8 head) and #1387 (pb16i head) each wrote inline is now one header with a device-free test, and I verified both the test and the helper's contract independently.

tests/npu_row_chunks_test.cpp  ->  npu_row_chunks: PASS        (registered as ctest `npu_row_chunks`, no device)
my own probe of row_chunks():
  M=128 cap=16 chunked ->  8 chunks, contiguous, none wider than cap
  M=128 cap=16 NOCHUNK ->128 chunks (the A/B arm, one row per launch)
  M=37  cap=16         ->  3 chunks, covering exactly [0,37)
  M=16  cap=16         ->  1 chunk     M=1 -> 1 chunk     M=0 -> 0 chunks
  M=200 cap=128        ->  2 chunks, contiguous and bounded
  M=100 cap=0          ->  0 chunks   (documented: an unknown capacity is no chunks, so the caller
                                       falls back to its per-row path rather than launching blind)

That last case is the one I would have written a test for and it is the one the header calls out first: a context whose MD is not yet known must produce no chunks rather than a launch of unknown width, and the caller's fallback is what makes it safe. The three invariants the header names (no chunk wider than MD; contiguous coverage of [0, M) exactly once; M == 0 is none) all hold in my probe, and the A/B arm is the same NPU_HEAD_NO_CHUNK=1 knob both heads already answer.

One defect, cosmetic but real: the Apache notice is duplicated in npu/forward/include/q4nx_row_chunks.h — two full "Licensed under the Apache License" blocks (lines 4-14 and 16-26) under a single Copyright/SPDX pair. tools/copyright.py --check returns rc 0 because it checks that a notice is present, not that it appears once, so this slipped through every gate. One block should go.

Not run: the device leg (the two heads actually launching in chunks on the array). The helper and the wiring are what is testable here, and they are green.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 0bdbea0)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1386 - Partially compliant Compliant requirements: Fix the pb16i LM-head rows path that launches all M rows into a 16-row context Prevent memory corruption by chunking the batch at the context's capacity Ensure no chunk wider than the context's capacity Ensure chunks are contiguous, in order, and cover every row exactly once Handle M == 0 case with no chunks Handle unknown capacity (MD <= 0) with no chunks to avoid blind launches Maintain bit-for-bit correctness for M <= 16 case Non-compliant requirements: None Requires further human verification: None

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Potential Performance Regression The introduction of chunking for pb16i LM-head rows path may introduce a performance regression due to increased kernel launches. While the fix prevents memory corruption, each chunked launch involves additional overhead from kernel scheduling and synchronization. This could be significant if M is large and the number of chunks is high. for (const onebit::npu::RowChunk& c : chunks) { const int r0 = c.offset, rows = c.rows; std::vector<float> got((size_t)rows * (size_t)T); for (int t = 0; t < ntiles; t++) { const int n0 = t * T, n1 = std::min(NV, n0 + T); xrt::run run = lctx.launch_async_with_bo_rows( *lm_head_packed_[(size_t)t].bo, hr.data() + (size_t)r0 * (size_t)H, rows, H, scales.data() + r0); lctx.wait_kernel(run); lctx.readback(); std::fill(got.begin(), got.end(), 0.0f); lctx.dequant_pb16i(got.data(), rows, T, scales.data() + r0, lm_head_packed_[(size_t)t].eref, xd_->head_ctx()->pb16i_planes); for (int m = 0; m < rows; m++) { for (int n = n0; n < n1; n++) { float v = got[(size_t)m * T + (n - n0)]; if (sem_.logit_softcap > 0) v = sem_.logit_softcap * std::tanh(v / sem_.logit_softcap); logits_all[(size_t)(r0 + m) * (size_t)NV + (size_t)n] = v; } } } } Chunking Logic The chunking logic in row_chunks function assumes that capacity is a valid positive integer when chunked is true. If capacity is unexpectedly negative or zero, the function returns an empty vector, which is handled correctly by the caller. However, this behavior should be explicitly documented or validated to ensure robustness in all code paths. inline std::vector<RowChunk> row_chunks(int rows, int capacity, bool chunked = true) { std::vector<RowChunk> out; if (rows <= 0) return out; if (capacity <= 0) return out; // unknown capacity: the caller must not launch const int step = chunked ? std::min(rows, capacity) : 1; for (int r0 = 0; r0 < rows; r0 += step) out.push_back(RowChunk{r0, std::min(step, rows - r0)}); return out; }

[github-actions]

Persistent review updated to latest commit 0bdbea0

[bong-water-water-bong]

VERDICT: READY at 21e2b279.

The blocking item is fixed: tools/copyright.py --check → rc 0 at this head, and both new files now start
// Copyright 2026 bong-water-water-bong (no (c)), which is what the tool's exact-match requires.

And the substance is the better shape of the fix. The chunk arithmetic is extracted into q4nx_row_chunks.h,
row_chunks(rows, capacity, chunked) returning {offset, rows} — I built and ran its test standalone:

g++ -std=c++17 -Inpu/forward/include tests/npu_row_chunks_test.cpp && ./a.out
npu_row_chunks: PASS

and it is registered as a ctest (add_test(NAME npu_row_chunks COMMAND npu_row_chunks_test), :865), so the arithmetic that decided
whether rows 16..M-1 were read past the BO now has a host test rather than only a device measurement. The edge cases are right in the helper:
rows <= 0 → nothing, capacity <= 0 → nothing ("the caller must not launch"), and the last chunk clamped.

The one path worth checking is what the caller does with an empty list, because "nothing" from a chunker is a silent wrong answer if the
caller treats it as "done". It does not: if (chunks.empty() && M > 0) falls back to one launch per row through run_lm_head, which is
slow but correct — and NPU_ROW.../NPU_HEAD_NO_CHUNK gives an A/B escape hatch that lands on the same per-row path. So neither the
unknown-capacity case nor the escape hatch can produce rows that were never computed.

Duplication you will have to decide on

#1387 fixes #1386 too — I verdicted it READY at b81a3283 an hour ago, and it is the same head fix (chunk the pb16i rows path at the
context's capacity). They are not identical:

	#1387 (READY)	this PR
pb16i head chunking	yes, inline in q4nx_forward.cpp	yes, via row_chunks()
host test for the arithmetic	no	yes, and on CI (npu_row_chunks)
layers-side max_batch_rows() block walk	yes	no
canary_short() / note_head parity with the int8 head	yes	no

So neither is a superset. If the decision is mine to recommend: land this one for the head fix (a tested helper beats an inline loop, and
the ctest is what will catch the next off-by-one), then rebase #1387 onto row_chunks() — its layers-side block walk and the canary/profiling
parity are the parts this PR does not carry, and they are the parts that make the two heads behave the same way. If instead #1387 lands
first, this PR's test is worth keeping even if the two implementations have to be reconciled.

Also, like #1394/#1395 the branch carries other open PRs' hunks (os/mini/release-check.sh +48 from #1394, scripts/pack-distro.sh from
#1395), so merge order matters for those two as well.

[bong-water-water-bong]

VERDICT: CHANGES at 0bdbea07e8a8367948b4b536863612cf1d9656b9 — the delta is good, but this branch currently reverts about 154 lines of main's recent work, and it conflicts with the fix that already landed for the same defect.

1. run_lm_head_rows is already fixed on main, by #1387 (merged 03:19:59Z):

$ git show origin/main:npu/forward/src/q4nx_forward.cpp | grep -n 'chunk = lctx.MD'
 4907:    const int chunk = lctx.MD > 0 ? lctx.MD : M;
 4909:    for (int r0 = 0; r0 < M; r0 += chunk) {
$ git merge --no-edit pr1396            # in a scratch worktree on current main
 CONFLICT (content): Merge conflict in npu/forward/src/q4nx_forward.cpp

So the merge queue cannot take this as-is. What is genuinely new here — and worth keeping — is the part #1387 did not have: npu/forward/include/q4nx_row_chunks.h with row_chunks(M, MD, on) and a unit test that drives it (I ran it: g++ -std=c++17 -Inpu/forward/include tests/npu_row_chunks_test.cpp -> npu_row_chunks: PASS), the per-row fallback when the context has no usable capacity, and NPU_HEAD_NO_CHUNK=1 as an A/B seam — which is exactly the switch #1387's body said a device measurement would need. Resolution: rebase and keep #1387's behaviour with your helper, fallback and seam on top; then the diff is the improvement.

2. The branch reverts other people's merged work. This is the thing to look at before anything else, and it is not a diff-form artefact — it is what a merge would do:

$ git diff --stat origin/main pr1396
 os/mini/prepare-stick.sh | 107 ++--------------------------      <- #1388's space preflight, deleted
 docs/hrx.md              |   8 ++--                             <- #1351's read/DONTNEED paragraph
 docs/install-any-distro.md | 19 +------                          <- #1368's package-proof section
 docs/os-v3.md            |   7 +-                                <- (1bit-update slot fix)
 docs/releases.md         |   6 --                                <- #1368's release-notes rows

The branch merged an older main and kept its own side for files it never meant to touch, so merging it would undo four merges from the last few hours — including the space preflight whose absence truncated a model on c32. A git rebase origin/main drops all of that automatically, since none of it is this PR's content.

Everything I can check about the fix itself is right, and it is better than what landed: row_chunks() is testable without a device (npu_row_chunks: PASS), the fallback to run_lm_head() row-by-row is the correct answer when MD gives no capacity rather than a blind launch, and the seam makes the device A/B that #1387 deferred to "whoever holds the NPU" a one-environment-variable experiment. I would merge the rebased version.

[bong-water-water-bong]

VERDICT: READY at 0bdbea07e8a8367948b4b536863612cf1d9656b9

Main-merge plus a build-order line over the head I reviewed; the chunk helper and its test are unchanged, and the one defect I reported is still there at this head.

Still present, and it is the only thing I would change before merge:

npu/forward/include/q4nx_row_chunks.h
   1  // Copyright 2026 bong-water-water-bong
   2  // SPDX-License-Identifier: Apache-2.0
   4  // Licensed under the Apache License, Version 2.0 (the "License");      <- block 1
  14  // limitations under the License.
  16  // Licensed under the Apache License, Version 2.0 (the "License");      <- block 2 (lines 16-26)
  26  // limitations under the License.

One Copyright/SPDX pair followed by the Apache text twice. tools/copyright.py --check returns rc 0 because it tests that a notice is present, not that it appears once, so nothing in the gate chain can see this — which is exactly why it is worth fixing by hand rather than trusting the check.

Everything else at this head is what I verdicted before: npu_row_chunks: PASS (device-free, registered in ctest), and my own probe of row_chunks() across nine cases (M=128/cap=16 → 8 chunks; the NPU_HEAD_NO_CHUNK arm → 128; M=37 → 3; M=0 and cap ≤ 0 → none; M=200/cap=128 → 2) held every invariant the header names — no chunk wider than the capacity, contiguous coverage of [0, M) exactly once, and no chunks at all when the capacity is unknown so the caller falls back instead of launching blind.

[bong-water-water-bong]

Device run attempted — and the result is a clean negative that is worth having: on this box's pb16i artifacts the >16-row path is unreachable

I took an npu-hold ticket (441 s, released clean, amdxdna errors 0 → 0) and ran the gate the issue asks for — mtp_acceptance --batchcheck on bonsai2-27b-pb16i (--head-exact …/mtp-head/pb16i-exact --head-q4k …/mtp-head/q4_k), which walks the same stream
sequentially and in nd-row batches and compares per-row argmax.

forward ready: vocab 248320, max_batch_rows 16
prompt 0: batch argmax vs sequential argmax: 0 of 0 rows differ (first at position -1)
  verify-batch cost: M 1 rows: 4 batches … 649.52 ms/row;  M 2 rows: 2 batches … 380.41 ms/row;  M 16 rows: 0 batches

--batchcheck uses nd = min(k, max_batch_rows()), and max_rows is 16 whenever the target runs the M=16 decode twins
(XdnaTarget: c.max_rows = m16_ ? Q4NX_DESIGN_M_DECODE : Q4NX_DESIGN_M_PREFILL), which is every pb16i pack on this box — all of
them declare decode.M = 16. So nd ≤ 16 = MD, no batch of 17+ rows formed ("M 16 rows: 0 batches" with --tokens 4), and the
chunk loop ran with a single chunk.

What that means for #1386, stated rather than spun:

1. The defect is real but not reachable on the pb16i artifacts this box has, which is also why it was found by reading rather
   than by a failure. My fix is a no-op for every one of them (M ≤ MD → one chunk) and the run confirms that no-op is clean.
2. It is reachable when max_rows is 128, i.e. when the target runs the prefill designs instead of the decode twins (a b8 /
   prefill-kernel set, as npu forward: the batched int8 head chunks rows by the kernel's capacity, so a 64-row verify pass is 9.9x faster #1364's int8 A/B used). A device proof therefore needs such a pack plus --k 32 --tokens 64 so nd = 32
   and batches actually form — I will take that ticket if you want the row measured rather than argued, but I did not want to
   report a "verified on device" for a path this box cannot reach.
3. The bounds themselves are pinned by tests/npu_row_chunks_test.cpp (device-free, in CI): no chunk wider than the capacity,
   contiguous coverage, and no chunks at all when the capacity is unknown — which is the invariant whose absence was the bug.

So this PR's honest claim is: the launch can no longer exceed the context's row capacity (construction + test), and the
numeric-equivalence argument is #1364's own (per-row rotation and scale are computed before the launches). Verified on device: the
load/run path is clean with the change; not verified on device: an M > 16 batch, because no artifact here can form one.

[bong-water-water-bong]

Fixed at 96c0b81d — and you were right that no gate can see it, which is the part I want to record.

The duplicate notice. tools/copyright.py --fix appended the standard block to files that already carried the Apache text under a slightly different first line, so both new files ended up with one Copyright/SPDX pair and two Apache bodies (the test had it too, not just the header). --check returns 0 either way because it tests that a notice is present, not that it appears once. Removed the duplicate body from both, one separator line before the prose, re-ran: npu_row_chunks: PASS, copyright.py --check rc 0.

The second reachable-arm attempt, so the negative in my previous comment is complete. I took one more npu-hold ticket (75 s, released clean, amdxdna 0 → 0) to try the one artifact that could form an M>16 batch — bonsai2-27b-NPU-b8, whose plan declares prefill.M = 128 (so max_rows would be 128 rather than the decode twins' 16):

forward init failed: plan …/bonsai2-27b-NPU-b8/plan.json: this model's plan needs the QKV kernel
"full_i8_QKV_K5120_N14336_b8.elf" in …/bonsai2-27b-NPU-b8/kernels-m16 (missing)

That pack is stored without its kernels-m16/ directory, so it cannot init here — I did not create the directory or copy an ELF into it, since a fabricated layout would not be evidence. So the M>16 arm remains unreachable on this box for two independent reasons: the pb16i packs cap max_rows at the decode twin's 16, and the b8 pack that would raise it is incomplete on disk.

That is the whole device story for this PR: the fixed path loads and runs clean, the chunk loop is a single chunk on every artifact here (so it is a no-op), and the >16 case is proven by construction (row_chunks) plus tests/npu_row_chunks_test.cpp, not by a device row — which I would rather say plainly than dress up.

[bong-water-water-bong]

Closing as superseded — #1386 was fixed and closed by #1387 (4a89efcb, 03:20Z) while this branch was in review, and that fix is
the stronger one (it also carries a canary guard for a short chunk, which mine does not). No merge from me, and I am not going to ask a
reviewer to choose between two fixes for a closed issue.

What this branch has that main does not, for the record, in case anyone wants it as a follow-up:

1. npu/forward/include/q4nx_row_chunks.h + tests/npu_row_chunks_test.cpp — the chunk arithmetic extracted and tested
   (device-free, in CI): no chunk wider than the capacity, contiguous coverage of [0, M) exactly once, M == 0 and an unknown
   capacity both as no chunks so the caller falls back instead of launching blind. main's loop does the same thing inline, so this
   is coverage rather than a fix; it would need that loop refactored onto the helper to be more than dead code, which is churn on a
   merged fix — I would rather offer it than do it unasked.
2. The reachability finding, which is the part I think is worth keeping and which no device row on this box can show — posted on
   npu forward: chunk the pb16i head rows path at the head context's MD, so a verify batch cannot outrun its activation BO #1387:
   • every pb16i pack here declares decode.M = 16, and XdnaTarget sets max_rows = 16 whenever the target runs the decode twins,
     so mtp_acceptance --batchcheck forms nd ≤ 16 batches and the >16 path is never exercised (441 s ticket, amdxdna 0 → 0,
     max_batch_rows 16, "M 16 rows: 0 batches");
   • the one artifact that could raise it, bonsai2-27b-NPU-b8 (prefill.M = 128), cannot init on this box:
     this model's plan needs the QKV kernel "full_i8_QKV_K5120_N14336_b8.elf" in …/kernels-m16 (missing) — the pack is stored
     without that directory, and I did not fabricate one (75 s ticket, amdxdna 0 → 0).

So both #1387 and this branch were unverifiable on device here; #1387's canary guard is the right shape for that, and the honest claim
for either fix is "the launch can no longer exceed the context's row capacity", not "verified with an M>16 batch".

Thanks to the reviewer for catching the duplicated Apache notice at 0bdbea07 — tools/copyright.py --check cannot see it (it tests
presence, not occurrence), which is worth a three-line gate fix somewhere.