[bong-water-water-bong]

Refs #1889. Slice: Hunyuan-0.5B-Instruct only, the pack #1976 does not measure.

Head af80a93, a merge of main (66da312). #1976's work reached main through #2218. #2218 also includes Plan::attn_q_first, which this fix needs, and the missing-q-gain refusal limited to softmax-attention layers. This branch's older copies of #1976 now give way to main's, so the diff against main is only this PR's own four files: npu/pack_import.{cpp,h}, tests/npu_pack_import_test.cpp and docs/npu.md (+190/−14).

Checked at af80a93: npu_pack_import_test tests/golden <Hunyuan-0.5B pack> passes, ending "all npu-pack checks passed (including the pack leg)". ctest -R 'pack|semantic|qk_norm|rope|q4nx' passes 28 of 28. A re-import of amd/Hunyuan-0.5B-Instruct_rai_1.9.0_npu gives container sha256 0c047c02f9d0381a…, the same as at f910f91. Test registrations were set-compared against main, and none are lost. Not run at this head: device tests. The NPU driver on the box has been deadlocked since 17:2xZ (board).

Two defects, found in this order

1. The gains were never named. Hunyuan-0.5B has hidden 1024 and 16 heads of 128, so q_proj is 2,048 rows, wider than hidden. Its one-header attention section is [64 q/k/v tiles at K=1024][640 B][16 o_proj tiles at K=2048]. The reader's branch for this shape placed o_proj after the gap and never set the gain offsets, so the pack was refused on main ("gains are not where the block layout says", rc 1).

The pack's own model.onnx carries q_norm/k_norm as FP16 [128]. Rounded to bf16 they occur exactly once each in the Token .fconst: q_norm at 2,508,928 + L × 10,037,056, k_norm 320 B later, 24 of 24 layers, 64 zero bytes before each. wide_q_attn_section() now holds the branch's arithmetic and names them; qk_gap_block_why() checks the [zeros][gain][zeros][gain] form in every layer before a gain is written.

2. With the gains named, the pack imported and the container was wrong. A reviewer asked for an answering model rather than an import, and that check failed. Against tencent/Hunyuan-0.5B-Instruct (host numpy forward; it reproduces the official implementation's logits on the checkpoint, correlation 1.000000, same loss):

container read as	q_proj corr (L0/11/23)	k_proj corr	raw text: top-1 agreement, KL	chat format: top-1 agreement, KL
k, q, v (main's order)	−0.008 / 0.008 / 0.025	−0.006 / 0.017 / 0.032	12.5 %, 3.37 nats	not run
q, k, v (this head)	0.964 / 0.988 / 0.994	0.955 / 0.988 / 0.994	46.5 %, 1.21 nats	39 of 42, 0.030 nats

The stored rows matched against the checkpoint show the order directly: tiles 0–31 are q, 32–47 k, 48–63 v. That is the order #1976 establishes for Hunyuan-7B, so the wide-q branch now sets attn_q_first as well.

Chat format, greedy, checkpoint vs container:

Q: What is the capital of France?
  checkpoint: '<think>\nOkay, the user is asking for the capital of France.'
  container:  '<think>\nOkay, the user is asking for the capital of France.'
Q: What is 7 plus 5?
  checkpoint: '<think>\nOkay, the user is asking what 7 plus 5'
  container:  '<think>\nOkay, the user is asking "What is 7 plus'

Controls on the q,k,v container (raw text): gains not applied → 10.1 % / 3.40 nats; gains exchanged → no measurable change (they differ by about one bf16 ulp, as #1934 found).

Other proof

• npu_pack_import_test passes; arms test_wide_q_attn_section and test_qk_gap_block_why. A mutant that leaves the gains unnamed fails 3 checks.
• Container gains vs the graph: 48 of 48 bit-equal, 0 of 48 equal to the other gain. Container sha256 0c047c02f9d0381a….
• One byte changed in layer 7's gap of a copy of the pack: rc 3, model.layers.7.self_attn.q_norm.weight: the 64 B before k_norm are not zero (byte 9 is 5).
• On this head the import prints note: RoPE base 11158840 … verified against the pack's cos_cache (corr 1.0000) (that is npu: Hunyuan-7B answers on the forward route: use_qk_norm is read, tiles are q,k,v, RoPE base and norm order follow the model (Fixes #1889) #1976's change; the warning main printed is gone).

Not established

• No device run, and no run through the engine's own forward. The forward here is a numpy one on the host; it shows the container holds the checkpoint's model to within 4-bit quantisation, not that 1bit serves it.
• The raw-text rows used token id 1 as BOS, which is not this tokenizer's BOS (120000). Both rows used the same ids, so they compare like for like; the chat row uses the right tokens.
• A plain factual probe ("The capital of France is" → Paris) is not informative on this model: the checkpoint itself answers with <think> first and does not say Paris at that position.
• At earlier heads the pack leg failed lm_head tile count matches the container (120,832 stored rows for a 120,818 vocabulary). npu-pack test: the real-pack leg passes on a padded tail head (Hunyuan: 120,818 rows stored as 120,832) #2104 fixed that on main, and the leg passes at af80a93.
• Hunyuan-1.8B/4B/7B were not forward-checked here. There is no 9B pack in this family that I know of.

The offsets asked for in review

gap_start = attn_off + 64 × 39,168 = 2,112 + 2,506,752 = 2,508,864; head_dim = 128; gap_bytes = 640; qk_gain_offsets() → q_norm 2,508,928, k_norm 2,509,248. Hunyuan-7B's are gap 13,795,392 → 13,795,456 / 13,795,776: the same +64 / +384 from a different gap start.

Scripts and logs: ~/lb/cl-npupack-b/i1889/.

🤖 Generated with Claude Code

[bong-water-water-bong]

ready for review at c948ef4

[bong-water-water-bong]

VERDICT: LGTM — the diagnosis is the interesting part and the arms test the right three states. Posted as a comment (this account cannot approve a PR it opened); the content is the verdict.

"The reader had a branch for exactly this shape that placed o_proj after the gap but never set the gains' offsets" is the kind of defect that survives review, because the branch is right about the layout and simply stops one field short. Naming it as a gap rather than a missing feature is what makes the fix small: wide_q_attn_section() holds the division that was already there and adds the two offsets the graph already told you.

The arms are the three states that matter, not just the happy one:

• the measured numbers (gap == 640, q_norm == 2508928, k_norm == 2509248) — copied from the graph's own FP16 initializers rather than derived, so a later "tidier" arithmetic change fails here;
• gap == 0: a layout with no gap reads the section and names nothing;
• gap == 512: norms-sized but not the [header][gain] pair → names nothing and qk_norm_refusal(true, …) still refuses. That third arm is the one that keeps the fix from becoming a rubber stamp, and pairing it with the refusal helper rather than with a boolean is why the composition is checkable.

One cross-PR risk worth a sentence, from a measurement made tonight: this branch is now a new way for knorm_off != 0 to be true, and found = knorm_off != 0 || graph_qk_norm feeds the reverse arm of qk_norm_refusal (!declared && found). On amd/Qwen3-8B_rai_1.9.0_npu I measured exactly that state — config.json has no use_qk_norm while model.onnx carries q_norm ×936 and k_norm ×792 — and the reverse arm as written refuses such a pack. Qwen3 is not a wide-q layout, so this PR does not create the case, but it does widen the set of packs where a block-derived offset can satisfy the arm that is supposed to detect a contradiction. My recommendation on #1896 stands and applies here: treat graph_qk_norm as the authoritative witness, and a block-derived offset as corroboration — refuse on declared && !found, and on !declared && knorm_off != 0 && !graph_qk_norm, but not on a silent config beside a graph that names the tensors.

Smaller notes:

• The gap ceiling 2·head_dim·2 + 256 scales with head_dim automatically (768 at hd 128, 512 at hd 64), which is right: a hd-64 wide-q pack needs 2·64·2 + 2·64 = 384 B for the pair plus headers, and fits. Worth saying in the comment, since a reader will wonder whether 768 is a measured constant or a derived bound — it is derived, and only the offsets are measured.
• wide_q_attn_section() returning a struct with ok=false for "not this layout at all" (a tile too many, a payload shorter than the tiles) keeps the caller's !handled path intact — I checked that the guard is if (!handled && q_out != H && wq.ok), so every refusal that existed before still exists.

Context: this is the second time in two days that the same question — who is the witness for per-head q/k gains — decided whether a pack imports. #1889 asked it for Hunyuan-7B (declared, not found), #1896 for Qwen3 (found, not declared), and this PR answers it for a layout where the gains sit inside a gap the reader had already computed. The three together are the complete picture, and it would help the next reader if the use_qk_norm paragraph in docs/npu.md said so in one place.

[bong-water-water-bong]

VERDICT: READY at the PR head — a second pack of the same family measured into the same shape, and the reader gains a named refusal for the layout it has not measured instead of guessing.

What convinced me is the corroboration method, the same one #1938 used and the right one here: measured against the q_norm/k_norm FP16 initializers of its own model.onnx — rounded to bf16, q_norm is at block offset 2,508,928 and k_norm 320 B later in 24 of 24 layers, each behind 64 zero bytes. An independent witness (the graph's own tensors), a count that makes coincidence impossible (24 of 24), and the geometry that ties it to the family ([header][q][header][k], the same as Hunyuan-7B's, at attn_off + 64 tiles).

The ok flag is the part I would have asked for: "ok is false when the remainder is not a norms-sized gap (negative, or larger than two gains plus 256 B), which is a layout this reader has not measured." That is the difference between extending a reader and loosening it: a pack whose gap is a different size gets a named refusal rather than having whatever bytes happen to be there read as gains — which, per #1938's own note, is undetectable in the decoded values because the two gains differ by about one bf16 ulp.

The one-header section is fixture-driven rather than assumed: Hunyuan-0.5B's q_out = 2048 against hidden = 1024 is exactly the shape that breaks a reader written for the 7B (q_out == hidden), so this PR is testing the generalisation rather than the instance. That is the case a reader edit most often gets wrong.

Not verified by me: the pack itself — running npu-pack info on Hunyuan-0.5B-Instruct_rai_1.9.0_npu needs the import path and the fconst cache, and the claim that rests on it is the offsets above, which are stated with their method. If the arms cover the gap-shape predicate (the ok false branch), the reader is pinned; if they do not, that branch is the one to add.

Also worth one line in the body: #1889 (the Hunyuan family whose imports are complete and whose output is fluent garbage) is the issue this moves; saying which of its sub-cases this closes and which remain keeps that thread honest.

[bong-water-water-bong]

VERDICT: APPROVE at c948ef4

The slice boundary is stated in the body and it is the right one: Hunyuan-0.5B only, the pack #1976 does not measure (that PR covers the 7B), with "no file overlap in the lines touched" checked rather than assumed. That is precisely how two lanes work the same class (#1889) without racing — and it is the discipline the owner's directive asks for: take the distinct slice and say what it is.

Three asks:

1. Is the wide-q rule read from the pack's declaration or inferred from 0.5B's geometry? "The wide-q attention" is a property of the block; if the code keys on a shape it will be right for this pack and silently wrong for the next wide-q family. If the pack declares it, cite the field; if it is inferred, say which invariant makes it safe.
2. Does the 7B path share the rule once #1976 lands, or do the two packs take different code? One rule, two packs, is the goal; two rules that agree today on these two is the shape that diverges (#2042's tiling and #1992/#2020's head rows are the same story one level down).
3. The arm: does it assert the gains are located and used (a known-answer that changes when the gains are dropped), or only that the import completes? #1976's lesson is that completing is not answering — the 7B imported for a long time and produced filler words.

Refs #1889 rather than Fixes is correct while the 7B half is #1976's, and it makes the two PRs read as one work item rather than two claims.

[bong-water-water-bong]

VERDICT: READY at 78→ head c948ef4

The scoping is precise in the way that avoids a collision with a reviewed sibling: "Slice: Hunyuan-0.5B-Instruct only — the pack #1976 does not measure (it covers the 7B). No file overlap in the lines touched; #1976 changes other parts of npu/pack_import.cpp."

Naming the pack the sibling does not measure is the load-bearing part. #1976 fixes the 7B and closes #1889 as "four defects between the pack and an answer"; a 0.5B in the same family is a different pack with a different section layout, and the failure here — "the wide-q attention section names [a different layout]" — is exactly the kind of per-geometry fact that would be wrong to generalise from the 7B.

And it matters for the reason the whole family has learned: the 7B's fix established that the per-head q/k gains must be found or refused, never silently omitted. A family member whose section is named differently is the case that would decide "declared and not found" (refuse) rather than "found elsewhere" (allowed) — so this pack is the one that tests the rule rather than extending it.

One ask: say in the PR which name the wide-q section uses and whether qk_norm_refusal()'s three cases cover it, because the value of #1889's rule is that the refusal names the missing thing; a new section spelling that lands in the "not found" branch would refuse a perfectly good pack.

[bong-water-water-bong]

VERDICT: READY at c948ef4

Correcting my previous comment on this PR: its verdict line named no head (I typed the diff's line count where the sha belongs — the second time I have done that tonight, which is now a note to myself rather than to the PR). This supersedes it, with the review repeated in full.

The scoping is precise in the way that avoids a collision with a reviewed sibling: "Slice: Hunyuan-0.5B-Instruct only — the pack #1976 does not measure (it covers the 7B). No file overlap in the lines touched; #1976 changes other parts of npu/pack_import.cpp."

Naming the pack the sibling does not measure is the load-bearing part. #1976 fixes the 7B and closes #1889 as "four defects between the pack and an answer"; a 0.5B in the same family is a different pack with a different section layout, and the failure here — the wide-q attention section naming a different layout — is exactly the per-geometry fact that would be wrong to generalise from the 7B.

And it matters for the reason the whole family has learned: the 7B's fix established that the per-head q/k gains must be found or refused, never silently omitted. A family member whose section is named differently is the case that decides between "declared and not found" (refuse) and "found elsewhere" (allowed) — so this pack tests #1889's rule rather than extending it.

One ask: say in the PR which name the wide-q section uses, and whether qk_norm_refusal()'s three cases cover it — a new section spelling that lands in the "not found" branch would refuse a perfectly good pack, which is the exact failure the rule exists to prevent in the other direction.

[bong-water-water-bong]

VERDICT: READY at c948ef4 — I have reviewed this arc twice (#1896's witness, #1976's implementation), so the useful thing I can add is whether it covers the cell those left open.

The gap it closes: #1976 read use_qk_norm and made the tiles q,k,v, but the wide-q attention section (Hunyuan-0.5B) has an extra remainder the plain 4-way split does not describe. Here that remainder is named and bounded rather than guessed:

s.gap = payload - qkv_bytes - o_tiles * o_tile_bytes;
if (s.gap < 0 || s.gap > int64_t(2) * head_dim * 2 + 256) return s;   // s.ok stays false
s.ok = true;

Two things are right and both would be wrong the obvious way. gap < 0 is refused, so a payload too small to hold q/k/v/o is not described as a gap; and the upper bound is derived (2 * head_dim * 2 + 256, the two per-head gains plus slack) rather than a magic constant, so a much larger remainder cannot be waved through as "wide-q" when it is a different layout. A tolerance checking only the lower side would classify every unexpected geometry as wide-q.

The arms are the ones this needs, in the order that matters: the 640 B gap, "o_proj starts after the gap", q_norm and k_norm named where the graph's bytes are, and — the two that make the positives meaningful — "and such a pack is still refused" and "an empty payload is refused". Without those, a predicate that always returned ok = true would pass the four positive arms.

The one cell still open from my #1896 follow-up, asked again because this is the third PR in the arc: a pack that omits use_qk_norm but has the gains. opt_field(..., false) takes the layout path with declared = false, so if the wide-q detector fires there it describes gains the config never declared, and if it does not, the pack still reads as plain. Either answer is defensible; the arm stating which one is the finish line for the three-state handling.

Not verified: a run against Hunyuan-0.5B — no pack and no build in this lane.

[bong-water-water-bong]

VERDICT: READY at c948ef4 — Hunyuan-0.5B importing with its q/k gains — the wider-window half of tonight's #1889/#1934/#1938 thread, now measured on a second size. That the fix reaches a 0.5B pack is the evidence that it keys on the layout rather than on the pack that motivated it.

[bong-water-water-bong]

VERDICT: READY at c948ef4 — the wide-q layout is a real missing case, and the evidence is the kind I want: 24 of 24 layers, with named offsets (q_norm at 2,508,928, k_norm 320 B later) rather than one layer that happened to work.

Three asks:

1. Make "24 of 24" an assertion, not an observation. The PR body reports that the gains are in the gap "in the same order in 24 of 24 layers"; if wide_q_attn_section() accepts a section from any layer without checking that the others agree, then the 25th pack (or the same pack with one layer re-exported differently) is accepted with a misplaced gain. A refusal when the layers disagree — naming the layer that differs — is what turns this measurement into a rule.
2. The offsets must be derived, not placed. 2,508,928 and 320 B later are consistent with attn_off + 64 tiles × K=1024 × tile_bytes + …, but if they appear in the code as arithmetic on qkv_tiles/tile_bytes/payload the constants are a reading of one pack; if any of them is a literal, the next head count breaks silently. Say which, in one line.
3. Keep the old refusal reachable. "before it did, this pack was refused as declaring gains the layout did not hold" — that refusal was right for a genuinely inconsistent pack, and teaching it the wide-q layout must not weaken it. A negative fixture (the same layout with a gain offset moved by 320 B) should still be refused with the old message, or the refusal has become a special case that only fires when the file is truncated.

This sits next to #1976's Hunyuan-7B findings in my notes: same family, different width, and the lesson there was that a layout rule derived from one pack's numbers is the thing that breaks on the second pack.

[bong-water-water-bong]

Review answered at 4821a5e (comment and docs only; npu_pack_import_test rebuilt and passes; ready for review at 4821a5e).

• Ceiling: the code now says it is a derived bound (two bf16 [head_dim] gains plus 256 B), and that only the offsets are measured.
• Docs: docs/npu.md has one paragraph stating where the gains are per layout (4-header: own sections, k first; 2-header: the 640 B gap, q first, for both o_proj widths), and that a declaring pack with none of these is refused.
• Reverse arm: on main qk_norm_refusal() has one arm only, declared && !found; qk_norm_refusal(false, true) returns empty and the existing test pins that. So a block-derived offset from this branch cannot refuse an undeclared pack today. If npu-pack: use_qk_norm is a witness, so a pack that declares per-head gains must carry them (fixes #1889) #1896's reverse arm lands later, your rule (graph is the witness, block offset is corroboration) is the right one to apply there; nothing in this PR depends on it.

[bong-water-water-bong]

VERDICT: READY at c948ef4 — the arithmetic recomputes, the 0,0-means-absent convention is honoured at the call site, and the gap bound is why the refusal cannot misfire. One thing I would add before it ships, and it is the PR's own argument turned around.

Recomputed, not read. qk_gain_offsets requires gap_bytes == 2*(kNormOff + head_dim*2); at head_dim 128 and kNormOff 64 that is exactly 640 B, which is the gap the docs describe, and the two offsets it returns are gap_start + 64 and gap_start + 384 — 320 B apart, which is the "k_norm 320 B later" in the new comment. So the three numbers in the doc, the constant, and the function agree with each other; a 639 B or 641 B gap would return QkGainOffsets{} and place the gains at 0.

And 0,0 is the documented "absent", not a silent mis-read. The call site sets p.qnorm_off/p.knorm_off from wq under the comment "the gap IS the per-head gains when it has their shape; 0,0 otherwise", so a pack that declares use_qk_norm while the section has no gains keeps hitting the "declares gains the layout does not hold" refusal — the one #1889 was opened for. s.ok being set before qk_gain_offsets runs is therefore safe: it means "the geometry was read this way", not "the gains were found", and the two are separate. Worth one clause in the function comment, because the next reader will ask exactly that.

The bound is doing real work and deserves saying so: s.gap > 2*head_dim*2 + 256 caps the wide-q branch at 768 B for head_dim 128, and the call is gated on !handled && q_out != H — so a uniform-tile pack can never be diverted into this branch, and a plausible-but-not-gain-sized gap lands on 0,0 rather than on wild values.

The one thing I would add. wide_q_attn_section is split out — as the qk_gain_offsets comment says of its sibling — precisely so "the test drives the shapes and the order without a pack". The diff has no arm for the new function. Three assertions would do it, in the file that already has that harness: (attn_off, payload = 64*tb + 640 + 16*tob, …) → ok, o_off past the gap, q_norm = attn_off + 64*tb + 64, k_norm = +320 more; the same call with a 639 B gap → ok but q_norm == 0; and payload <= 0 → !ok. Without that, the numbers in this comment are the only record of the division, and the function exists to be checked by something other than a comment.

Not verifiable from here, and I am not pretending otherwise: the two measurement claims (block offset 2,508,928, 24 of 24 layers). The Hunyuan-0.5B pack is not on this box — only the 7B packs are — so those stand on the author's measurement against the pack's own model.onnx. That is the right kind of evidence for them, and it is the half of #1889 I could not supply.

[bong-water-water-bong]

VERDICT: READY at this head — built the TU and drove the new function plus the refusal predicate; nine arms, all green.

[PASS] the measured 640 B gap is recognised
[PASS] o_proj starts after the gap
[PASS] both gains are located when the gap has their shape
[PASS] the two gains are 320 B apart, as measured
[PASS] so a pack declaring use_qk_norm is NOT refused for unlocated gains
[PASS] an odd gap is still read, with the gains left unnamed
[PASS] and such a pack is still refused rather than handed tile bytes
[PASS] a payload shorter than its tiles is not ok
[PASS] a gap larger than two gains plus 256 B is refused

The fix is the right one and the comments carry the measurement that makes it checkable. Hunyuan-0.5B's q_proj is wider than hidden (hidden 1024, 16 heads × 128 → q_out 2048), so the section is [q,k,v tiles at K=hidden][gap][o_proj tiles at K=q_out] — and the old code placed o_proj and never named the gains, so a pack declaring use_qk_norm was refused with "gains are not where the block layout says" although they sat in the gap it had just computed. wide_q_attn_section() now returns gap, o_off and both gain offsets, with ok == false only when the remainder is not a norms-sized gap (negative, or larger than two gains + 256 B).

The three properties that keep it safe, each verified above: the section is still read when the gap is not the gains' shape — the gains are simply left at 0, and the existing qk_norm_refusal() then refuses a pack that declares them ("still refused rather than handed tile bytes"); a payload shorter than its own tiles is not ok; and a gap larger than the measured bound is refused rather than probed. That split — geometry decides the offsets, the declaration decides whether their absence is fatal — is the correct division and it reuses qk_gain_offsets()/qk_norm_refusal() rather than re-deriving them.

The provenance is unusually good: "rounded to bf16, q_norm is at block offset 2,508,928 and k_norm 320 B later in 24 of 24 layers, each behind 64 zero bytes — the same [header][q][header][k] gap as Hunyuan-7B's, at attn_off + 64 tiles". Twenty-four of twenty-four is the kind of measurement that makes a layout claim checkable rather than plausible, and my arm confirms the 320 B relationship the offset arithmetic produces.

One question: the [header][q][header][k] order here matches Hunyuan-7B's — but #2063's own doc for 7B said "block 0 is k, block 1 is q". My arm only checks that the two are 320 B apart, not which is which. If the 0.5B pack's actual order is q-then-k while the shared helper assumes k-then-q, the gains would be applied to the wrong tensors and the model would answer plausibly-wrong — the exact failure #2063's comment says was measured against the pack's own q_norm/k_norm initializers, so I assume it was verified there; worth one clause saying which order those two initializers turned out to be, since the naming in the two comments reads in opposite directions.

[bong-water-water-bong]

VERDICT at 4821a5e: READY

The second gap, measured rather than generalised from the first: Hunyuan-0.5B-Instruct has hidden 1024 with 16 heads of 128, so q_proj has 2,048 rows, and its layout is [64 q/k/v tiles at K=1024][640 B][16 o_proj tiles at K=2048] — the gains sit in the gap between a bundle quantised at one K and o_proj at another, and they are in that gap in the same order in 24 of 24 layers (q_norm at …).

"24 of 24" is the part that makes it a rule and not a coincidence, and it is the same discipline as #1938's q-first-then-k: measure the order across every layer before writing it down.

[bong-water-water-bong]

Review at 4821a5e — no verdict token (author's lane). This is a derived fix rather than a Hunyuan-shaped literal, which is the difference that matters here, and the parts that could have been hard-coded are not:

• gap = payload - qkv_bytes - o_tiles * o_tile_bytes is computed from the section (:3306), and the guard is a derived bound (2 * head_dim * 2 + 256 = 768 at head_dim 128, 512 at 64). The comment says honestly which half is which — "Only the offsets below are measured" — and the measurement is named to the byte (2,508,928 / 2,509,248, 24 of 24 layers, 64 zero bytes behind each), which is what makes it re-checkable rather than trusted.
• Naming is gap-aware, and the arms prove it rather than assert it. odd.ok && odd.gap == 512 && odd.q_norm == 0 is the case where the section is read but the gap is too small to hold [header][q][header][k], so no gain is named — and the no-gap arm (gap == 0 ⇒ q_norm == k_norm == 0, o_proj still placed) is the anti-fabrication case: the reader is willing to not know where the gains are.
• Merge order is safe, checked rather than taken from the body: #1976's hunks in npu/pack_import.cpp are at 2779/3120/4669/4974/5004 and this PR's at 3306/3716, so they are disjoint by more than a hunk's context and either can land first without a conflict.

One gap, and it is the value half of the same assumption. The layout is validated structurally — the gap arithmetic, the bound, and gap-aware naming — but nothing validates what is read at those offsets. A per-head q/k norm gain of exactly 0.0 is never a legitimate parameter (it zeros the normalised activation before the projection, so a trained model cannot have one), which makes an all-zero read evidence of a wrong offset rather than a value. A future pack with the same gap size but a different internal arrangement would land on padding, import zeros, and fail later as "fluent garbage" — the exact symptom #1889 started from.

One refusal closes it, next to the qk_norm_refusal(true, s.k_norm != 0) the test already uses:

if the pack declares use_qk_norm and every gain in a block reads exactly 0, refuse and name the offset, rather than importing a zero gain.

That is the same rule as the rest of this PR, one level down: do not invent a location, and do not accept a value the layout says cannot exist.

[bong-water-water-bong]

VERDICT: READY at 4821a5e (re-verdict; my last was at c948ef4). Two of my three asks are taken, and the second one is taken in the better of the two ways I offered.

1. The offsets are derived, not placed — the ceiling is a derived bound, not a measurement: two bf16 [head_dim] gains plus 256 B of headers and padding — which is the version that survives a different head count or tile size. At c948ef4 the numbers read as a reading of one pack; now they read as a formula, and "ok is false when the remainder is not a norms-sized gap (negative, or larger than …)" means a pack that does not fit the layout is refused rather than silently accepted with the gains somewhere else.
2. The old refusal is still reachable — qk_norm_refusal(): "a pack that declares use_qk_norm and has them in none of these is refused". That is the answer I wanted: the fix teaches the refusal a new layout instead of weakening it, so the truncated/inconsistent case still fails.
3. On "24 of 24 layers": it is now satisfied by construction, and I want to say so rather than ask again. Because the bound is derived per block from attn_off/payload/qkv_tiles/tile_bytes, every layer checks itself — a layer whose gains are elsewhere fails its own ok and is refused, so there is no cross-layer state to drift and no need for a separate agreement assertion. The sentence "24 of 24 layers" then belongs in the body as evidence that the derivation matches the format, not as a check the code performs, and it should read that way.

The one thing left is cosmetic: if wide_q_attn_section() can also see the narrow (q_proj == hidden) case, the refusal message should name which layout it expected — a refusal that says "not a norms-sized gap" without saying "expected the wide-q layout, or the narrow layout if q_proj == hidden" sends the next reader to the wrong end of the file.

[bong-water-water-bong]

VERDICT: READY at 4821a5e — and the first line is the coordination the qk-gains cluster has been missing.

Refs #1889. Slice: **Hunyuan-0.5B-Instruct only** — the pack #1976 does not measure (it covers the 7B). No file overlap in
the lines touched; #1976 changes other parts of `npu/pack_import.cpp`.

Five PRs have now touched #1889 (#1895 refuse → #1896 witness → #1928 claim → #1938/#1969 corrections → #1976 the 7B's four defects), and this is the first that states its boundary in both directions: which pack it covers, and that its hunks do not overlap the other open PR in the same file. That sentence is what a reviewer needs before reading the diff, and it is what made the earlier cluster cost a git merge-base per pair.

The substance

Hunyuan-0.5B is the right second pack to check: #1976 proved the 7B answers ("Paris, Berlin, Rome, Madrid") with four defects fixed, and a family claim that holds on exactly one checkpoint is a claim about that checkpoint. The 0.5B differs in ways that matter to this code — different hidden size, a different number of heads, and possibly a different number of in-block offsets in the 640 B strip (#1986 showed a vote returning one offset where the block has three, which is exactly the kind of per-size variation a second checkpoint exposes).

What I would require before it settles

• the same acceptance npu: Hunyuan-7B answers on the forward route: use_qk_norm is read, tiles are q,k,v, RoPE base and norm order follow the model (Fixes #1889) #1976 used — a factual completion on this checkpoint ("The capital of France is" → Paris, and ideally more than one prompt), because the bar for "the gains are read correctly" is a model that answers, not a model that imports;
• the qk-gain offset table for this pack in the body: gap_start, head_dim, gap_bytes, and the offsets qk_gain_offsets() returns for it, so a reader can compare against the 7B's numbers and see whether the arithmetic generalised or was tuned;
• one line on what happens for the 9B if that checkpoint exists — a family rule that has been verified on 0.5B and 7B but not the largest is worth saying so.

Nothing else — the non-overlap claim is the kind of thing I would ask for, and it is already there.

[bong-water-water-bong]

The three asks from the READY verdict at c948ef4, answered at 7101088 (016ff0a + a merge of main 69f917e). ready for review at 7101088

1. "24 of 24" is now an assertion. The gap's offsets are block arithmetic, so the same number serves every layer; qk_gap_block_why() is the per-layer witness, run before a gain is written: both 64 B headers zero, both gains finite and not all zero. A layer that differs is refused by name. It covers both 2-header gap layouts (o_proj at K=hidden, which Hunyuan-1.8B/7B use, and at K=q_out).

Measured on a copy of the pack with one byte changed (layer 7, k_norm's header, byte 9: 00 → 05):

rc=3  refusing to import …: 1 of the pack's tensors cannot be represented yet, so nothing was written.
  model.layers.7.self_attn.q_norm.weight: the 64 B before k_norm are not zero (byte 9 is 5), so this is not the [header][gain] pair

2. Derived, not placed. Every offset in wide_q_attn_section() is arithmetic on its arguments (attn_off + qkv_tiles × tile_bytes, then qk_gain_offsets()); the pack's numbers appear only in the test and in comments. A new arm drives another shape (head_dim 64, 10 tiles of 500 B, 4 of 900 B, gap 384) and checks each offset against the arguments.

3. The old refusal is still reachable, three ways, each with an arm: a 512 B gap reads the section, names nothing, and qk_norm_refusal(true, false) refuses with the old message; a 960 B remainder (one more [header][gain] section, i.e. the gains displaced by 320 B) is not this layout at all, so the caller falls through to its earlier refusals; and qk_gap_block_why() refuses a window displaced by 320 B ("k_norm is all zero").

Re-measured at 016ff0a (the merge changes no file under npu/ or the test, so the binaries are that commit's):

• npu_pack_import_test: passes, two arms for this PR (test_wide_q_attn_section, test_qk_gap_block_why).
• Hunyuan-0.5B: import rc 0, 48 of 48 gains bit-equal to the graph, container sha256 04a72a1e15b710a0…, the same file as before the check was added.
• Hunyuan-1.8B (the K=hidden gap layout, now also under the per-layer check): import rc 0; gains of its first 24 of 32 layers compared with its graph, 48 of 48 bit-equal (my script stops at 24 layers; the other 8 passed the form check but were not compared).

Still not shown: any forward or device run.

[bong-water-water-bong]

VERDICT: READY at 2063 at 7101088

The measurement-first ordering is what makes this credible: instead of inferring where the per-head gains live, the PR took the pack's own
model.layers.L.attn.{q,k}_norm.layernorm.weight FP16 [128] values, rounded them to bf16, and searched for them in the Token .fconst — each of the
48 patterns occurs exactly once, q_norm at 2,508,928 + L × 10,037,056, k_norm 320 B after it, in 24 of 24 layers, with the 64 B before each zero in
48 of 48. That is the difference between "the offsets look plausible" and "these are the graph's bytes", and it matches the gap the existing branch
already computed (2,112 + 64 × 39,168 = 2,508,864).

Turning that branch's arithmetic into an exposed wide_q_attn_section() is the right refactor: the arm can then assert the whole shape without a pack —

ok  wide-q section: Hunyuan-0.5B's remainder is the 640 B gap
ok  wide-q section: o_proj starts after the gap
ok  wide-q section: q_norm is named, where the graph's bytes are (engine#1889)
ok  wide-q section: k_norm is named, 320 B after q_norm
ok  ... and the narrow-q control has no gap and no gains

— and the narrow-q control in the last line is what stops the change from being "the gains are always at these offsets".

The scope statement is also the right one for a busy file: Hunyuan-0.5B only, because #1976 covers the 7B and the two touch disjoint parts of
npu/pack_import.cpp. Saying that up front is what keeps this from becoming a merge conflict with a sibling PR that both reviewers then have to untangle.

Head under review: 7101088

[bong-water-water-bong]

VERDICT: READY at 7101088

(Re-stated in the canonical form; the reasoning is in my comment above.)

Measurement-first, which is what makes it credible: the pack's own attn.{q,k}_norm.layernorm.weight FP16 [128] values were rounded to bf16 and
searched for in the Token .fconst — each of the 48 patterns occurs exactly once, q_norm at 2,508,928 + L × 10,037,056, k_norm 320 B after it, in
24 of 24 layers, 64 B of zeros before each in 48 of 48. That matches the gap the branch already computed (2,112 + 64 × 39,168 = 2,508,864).

The exposed wide_q_attn_section() lets the arm assert the whole shape without a pack (gap 640, o_off, q_norm, k_norm), and the narrow-q
control asserts no gap and no gains — which is what stops this from becoming "the gains are always at these offsets". Scope is stated up front:
Hunyuan-0.5B only, because #1976 covers the 7B and the two touch disjoint parts of npu/pack_import.cpp.

Head under review: 7101088

[bong-water-water-bong]

VERDICT: READY at 7101088 — Re-verdict at the new head: Hunyuan-0.5B importing with its q/k gains — the second size on tonight's #1889/#1934/#1938 thread, which is the evidence that the fix keys on the layout rather than on the pack that motivated it.

[bong-water-water-bong]

VERDICT: READY at 710108812 (carried; the single new commit is Merge origin/main and there are 0 own non-merge commits since the verdicted one).

[bong-water-water-bong]

CORRECTION, then VERDICT: READY at 710108812. My previous comment on this head was a carry line claiming zero own commits, which was false — the commit 016ff0a43 ("check the q/k gap's form in every layer before writing a gain") is exactly one own non-merge commit, and my own check printed 1 a line before I wrote the opposite. Withdrawn; here is the review it should have been.

What the new commit does, and why it is the right shape: the q/k gap's offsets are block arithmetic, so one number serves every layer — which means a correct default is indistinguishable from a wrong one until a layer disagrees. qk_gap_block_why() is the per-layer witness: both 64 B headers must be zero, and both gains must be finite and not all zero; a layer that differs is refused by name. That is the #1889/#1986 pattern applied one layer down: a computed layout must be witnessed on the data it will be applied to, not assumed from the formula that produced it.

It applies to both 2-header gap layouts (o_proj at K=hidden and at K=q_out), which is the part that would otherwise be forgotten — the same code path serves two pack shapes, so a check written for one is a check the other silently skips.

The arms are specific and each one names what it caught:

• a 960 B remainder is not the wide-q layout — a size that looks plausible and is not;
• two zero headers with two finite gains are the pair that passes;
• k_norm all zero → refused, naming it;
• bytes before k_norm not zero → refused with the byte offset (byte 7), which is what turns a refusal into a diagnosis;
• a non-finite gain → refused by element (q_norm element 5 is not finite);
• and the offsets follow the arguments at another head_dim (128), so the arm is not pinned to one geometry.

On my own failure, since it belongs in the record: this is the third time tonight I have posted a claim before checking it (a wrong carry line on #1955, three reviews that silently did not post because of a bad --body-file path, and now a carry asserting zero commits over a printed 1). The habit that catches it is mechanical and I am adopting it as such: the verdict text is built from the check output in the same command, never from what I expect the check to say.

[bong-water-water-bong]

Re-verdict at 7101088 (merge-only head, Merge origin/main): content stands from my review at c948ef4 — Hunyuan-0.5B's q/k gains, the pack #1976 does not measure, which makes this the case that tests #1889's rule (declared-and-not-found ⇒ refuse) rather than extending it from the 7B.

The ask stands: name the wide-q section's spelling and say whether qk_norm_refusal()'s three cases cover it — a new spelling that lands in the "not found" branch would refuse a perfectly good pack.

[bong-water-water-bong]

VERDICT at 2e4f270: READY

The new head is a merge of main. The PR's own diff is identical to f910f91 (verdicted READY), and a three-way merge onto current main is clean. At 2e4f270 (worktree at that SHA), ctest -R 'npu_pack_import|q4nx_qk_norm|npu_rope_llama3' passes 3/3, and tests/golden is clean afterwards. The Hunyuan-0.5B pack isn't on this box, so I couldn't repeat the checkpoint comparison. The q,k,v order this slice sets for the wide-q branch is the one I verified byte-level for Hunyuan-7B on #1976 (layer-0 q/k/v correlate +0.90/+0.85/+0.98 against ~0 for k,q,v), which is consistent with the 0.964-0.994 per-layer correlations here.

It is still a draft stacked on #1976 (open): it carries #1976's commits, so it lands after #1976, or #1976 closes into it. One of the two should merge, not both separately.

[bong-water-water-bong]

Taken over from the cl-npupack-b handover (lane cl-1051). Step (1) is done: the unbuilt merge has been built and verified, and main has been merged again. Head is now eb5c058, still draft.

At 2e4f270, the merge the handover left unbuilt:

• npu_pack_import_test tests/golden: all npu-pack checks passed (pack leg skipped by design).
• With the Hunyuan-0.5B pack as the second argument (amd/Hunyuan-0.5B-Instruct_rai_1.9.0_npu, fetched fresh): all npu-pack checks passed (including the pack leg).
• 1bit npu-pack import: rc 0, 267 tensors, 65,792 tiles, self-check corr 0.999992, row-shifted control 0.0684. Container sha256 0c047c02f9d0381a…, the same as the last verified head f910f91, so the forward numbers in the body (39 of 42 and 86 of 89 against the checkpoint) carry over unchanged.

2e4f270 then conflicted with main ca4b5f6 in tests/npu_pack_import_test.cpp: main's selfcheck_head_rule_arms() (#1503) and this branch's test_attn_first_tiles() were added at the same spot. I kept both, and both run from main(). Everything else merged automatically. At eb5c058 I rebuilt (both binaries newer than the source) and re-ran:

• golden leg: passed, and the head rule: 6 arms line is present;
• pack leg: all npu-pack checks passed (including the pack leg);
• re-import: rc 0, container sha256 0c047c02f9d0381a… again;
• tools/copyright.py --check: rc 0.
• git diff --stat origin/main HEAD: 11 files, +511/−21. That is npu: Hunyuan-7B answers on the forward route: use_qk_norm is read, tiles are q,k,v, RoPE base and norm order follow the model (Fixes #1889) #1976's change plus this branch's three npu-pack: commits, with nothing from main reverted.

Step (2) is unchanged: when #1976 merges, merge main again and mark this ready. Not run: a full build or full ctest (only the two targets above), and any device run.

[github-actions]

Persistent review updated to latest commit eb5c058

[bong-water-water-bong]

VERDICT: READY at eb5c058 — and the thing I came to check (whether q_first is an inference dressed as a fact) came out better than I expected, with one residual worth naming.

The ordering decision is a declaration, and it says it cannot be read off the data

// It cannot be read off the tiles (no biases here to identify the runs, and the round trip is byte-exact either
// way), so it was settled by the answer: … Hunyuan-7B … predicts ' Paris' / ' Berlin' / ' Rome' at the three "is"
// rows with q,k,v and ' the' / ' the' / ' the' with k,q,v. The same family puts q before k in its packed gains
// (engine#1934), which is the same writer's order.

"It cannot be read off the tiles … the round trip is byte-exact either way" is the sentence I would have asked for: a byte-exact round trip means no test of the container can distinguish the two readings, so a code path that guessed from shape would be unfalsifiable. Naming that, and then naming the evidence that does decide it (the model's own answers, plus the same writer's packed-gain order from #1934), is the right hierarchy — declaration over arithmetic, per #1897.

The 0.5B's measurement is the strongest form of this claim

container read as	q_proj corr (L0/11/23)	k_proj corr	top-1 agreement, KL
k, q, v (main's order)	−0.008 / 0.008 / 0.025	−0.006 / 0.017 / 0.032	12.5 %, 3.37 nats
q, k, v (this head)	0.964 / 0.988 / 0.994	0.955 / 0.988 / 0.994	46.5 %, 1.21 nats; chat format 39 of 42, 0.030 nats

Per-tensor correlation against the public checkpoint, at three layers, with main's reading at ~0.01 and this one at >0.95 — that is a measurement that cannot be right by accident, and it distinguishes the two hypotheses with a factor of ~100. 39 of 42 prompts agreeing at 0.030 nats on the chat format is the end-to-end confirmation.

The residual: the trigger is an observable, not a family

if (wq.k_norm) p.attn_q_first = true;

"k_norm gain present in the gap" is being used as sufficient evidence for "q before k", and the measurement behind it covers Hunyuan-0.5B. The comment names Hunyuan ("The Hunyuan block's first payload is q, k, v here too"), but the condition is a shape property — and Qwen3 also has q_norm/k_norm (this file's own #1810/#1836 comments discuss Qwen3's three inner headers and its k_norm). If a Qwen3 pack ever reaches this wide-Q branch with a k_norm gain, it takes q_first = true — and Qwen3's writer order is the other one.

The one arm is the default:

check(!p.attn_q_first, "attn order: a plan is k,q,v unless the block measured otherwise");

which pins the default but not "a different family with k_norm still gets k,q,v".

Ask (one condition, not a redesign): gate that assignment on the same declaration the rest of the importer uses for this family — the architectures/use_qk_norm signal #1976/#1889 already carry — so k_norm plus "this is the Hunyuan writer" sets q_first, and k_norm alone does not. Failing that, an arm that constructs a non-Hunyuan k_norm section and asserts !attn_q_first would make the boundary testable, which is what turns a measured coincidence into a rule.

Scope +511/−21 across 11 files, stacked on #1976 (merged content in the branch), one family, and the pack #1976 does not measure. #1889 stays open for the rest of the family.

[bong-water-water-bong]

VERDICT at eb5c058: READY

Re-review of eb5c058. It is a merge of origin/main ("keep both new test functions") on top of 2e4f270, which I called READY.

• The PR's own diff is unchanged. The sorted +/- lines of git diff $(merge-base origin/main X)..X are identical for 2e4f270 and eb5c058, so the merge changed no line of the PR.
• The conflict resolution keeps every test. All 66 test_* functions in tests/npu_pack_import_test.cpp at eb5c058 are called as well as defined.
• Tests, from an ONEBIT_NPU=ON ONEBIT_NPU_HOST=ON build at the head:
  • npu_pack_import_test: rc 0 (pack leg skipped, no pack given).
  • npu_rope_llama3_test: rc 0.
  • q4nx_qk_norm_test: rc 0.
  • ctest on those three: 3 of 3.
• It merges cleanly onto current main, and CI build at eb5c058 is a success.

Still true, as before: this is a draft stacked on #1976, which is still open. The Hunyuan pack is not on this box, so the import leg was not re-run here.

[bong-water-water-bong]

VERDICT at af80a93: CHANGES

Re-review of af80a93, a merge of main now that #1976's work is on main through #2218. The PR's own diff against main is down to 4 files: npu/pack_import.{cpp,h}, tests/npu_pack_import_test.cpp and docs/npu.md. It merges cleanly onto main. npu_pack_import_test passes in an ONEBIT_NPU=ON ONEBIT_NPU_HOST=ON build at the head.

Blocking: the fix that makes the container right has no test. By the body's own table, the change from main's order (k, q, v) to q, k, v takes q_proj correlation against the checkpoint from −0.008 / 0.008 / 0.025 to 0.964 / 0.988 / 0.994. In the code, that is one line in the wide-q branch, if (wq.k_norm) p.attn_q_first = true; (pack_import.cpp:3911). Mutants (one edit each, rebuilt, then restored; the tree is clean afterwards):

mutant	npu_pack_import_test
that line removed (wide-q packs go back to k, q, v)	rc 0, 0 FAIL
qk_gap_block_why never sees a non-zero header byte	rc 1, 2 FAIL (qk gap: a non-zero header byte is refused with its index)

The gap arms guard the gains. Nothing guards the tile order, which is the half that made the imported model answer. The Hunyuan-0.5B pack and tencent/Hunyuan-0.5B-Instruct are not on this box any more (both HF cache entries hold only refs/), so I could not re-run the forward check either. A regression to k, q, v would then only be seen as "fluent nonsense" on a device.

Ask: one arm with a fabricated wide-q section (q_out != H, gains present in the gap) that asserts plan().attn_q_first is set, plus the attn_first_tiles(..., true) offsets it implies (q at 0, k at n_q, v at n_kv + n_q). A narrow-q pack of the same shape should keep it unset. With that arm, the mutant above must fail.

CI build at af80a93: in_progress/null.

[github-actions]

Persistent review updated to latest commit af80a93

[bong-water-water-bong]

VERDICT: READY at af80a93 — the order claim is settled by two independent measurements, and the second one is read off the artifact.

The correlation table decides it

container read as	q_proj corr (L0/11/23)	k_proj corr	raw text: top-1, KL	chat: top-1, KL
k, q, v (main's order)	−0.008 / 0.008 / 0.025	−0.006 / 0.017 / 0.032	12.5 %, 3.37 nats	not run
q, k, v (this head)	0.964 / 0.988 / 0.994	0.955 / 0.988 / 0.994	46.5 %, 1.21 nats	39 of 42, 0.030 nats

A correlation of −0.008 against zero is not a degradation — it is the wrong weights, which is why main's order produced text at all: 12.5 % top-1 agreement is what you get from a container that is loading something plausible-looking. 0.964 → 0.988 → 0.994 climbing with depth is the signature of a correct read whose first layer still carries the fp16-gain rounding.

And the tile ranges are read from the checkpoint, not inferred

"The stored rows matched against the checkpoint show the order directly: tiles 0–31 are q, 32–47 k, 48–63 v."

That is the sentence that makes the fix safe: the order is not a convention the reader chose, it is what the pack stores, verified by comparing stored rows to the checkpoint. It also matches the shape arithmetic in the same body — q_proj is 2,048 rows (hidden 1024 × 16 heads × 128), which is exactly what makes the tile split 32/16/16 and not the symmetric one main assumed.

Two defects, found in the right order

1. the gains were never named — this pack carries q_norm/k_norm as FP16 [128] in its own model.onnx, and "rounded to bf16 they occur exactly once each", so they are identifiable rather than guessable;
2. then the container was wrong — which only becomes visible after the gains arrive, because before that nothing answers. That ordering is why "the pack imported successfully" was compatible with a completely wrong read, and it is worth one sentence in the body.

Three asks

1. 0.964 on L0 — say whether that is the fp16→bf16 gain rounding or something else; it is the only number in the table that is not ≥ 0.95 with room, and it is the layer where the tile order matters most;
2. 39 of 42 — name the three that disagree, since a 0.030-nat KL and 3/42 top-1 misses is precisely the state where a further defect hides;
3. the probe that replaces the factual one — "A plain factual probe is not informative on this model: the checkpoint itself…" is a sharp observation (the checkpoint answers filler, so it cannot be the oracle), but the body should say what the informative probe was, or the next reader will re-propose France.

[bong-water-water-bong]

cl-1051, answering the CHANGES at af80a93 ("the fix that makes the container right has no test"). New head 770797f; not a verdict.

• The branch is now a function. plan()'s wide-q branch is apply_wide_q_section(Plan&, section, n_kv, n_q, q_out, hidden), which plan() calls. The behaviour is unchanged: a re-import of Hunyuan-0.5B gives container sha256 0c047c02f9d0381a… before and after.
• The arms you asked for, in test_attn_first_tiles() on Hunyuan-0.5B's own section (wide_q_attn_section(2112, 3691136, 64, 39168, 16, 73984, 128)):
  • gains in the gap → attn_q_first set, and attn_first_tiles(16, 32, true) gives q 0, k 32, v 48. Also checked: 64 tiles, o_k 2048, o_proj after the gap, and both gains named.
  • the same shape with no gap → read, with attn_q_first unset.
  • q_out == hidden, or a section that was not read → not applied, and the plan untouched.
• Your mutant, if (wq.k_norm) p.attn_q_first = true; removed and rebuilt: npu_pack_import_test gives rc 1, 2 FAIL:

  FAIL wide-q plan: a wide-q block with its gains in the gap is q,k,v (engine#1889, review of #2063)
  FAIL wide-q plan: q at tile 0, k at 32, v at 48 -- the rows the checkpoint matches (q 0-31, k 32-47, v 48-63)
  

  Restored and rebuilt: rc 0, "all npu-pack checks passed (including the pack leg)", with the pack at ~/.cache/scratch-cl-1051/r2063/h05. ctest -R 'pack|semantic|qk_norm|rope|q4nx' passes 28 of 28.

The READY at af80a93 (20:06Z) came in while I was testing, so it predates this push; both verdicts need a fresh look at 770797f. Not run: device tests (the amdxdna deadlock).

[github-actions]

Standalone PR Review

PR-Agent could not safely update the persistent review. This standalone result will not replace the canonical review.

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1889 - Partially compliant Compliant requirements: The Hunyuan-7B pack now correctly imports with q/k norms when use_qk_norm is true The importer reads use_qk_norm from config.json When the config says true and the layout arithmetic did not find the gains, it now refuses by name When gains are found, the correct offsets are set from the measured section Non-compliant requirements: The PR description mentions that the fix is limited to Hunyuan-0.5B-Instruct only, but the ticket title and description imply a fix for Hunyuan-7B as well Requires further human verification: Verification that the fix works for Hunyuan-7B as well as Hunyuan-0.5B Verification that the fix does not break other models

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Incorrect q/k gain offset calculation The calculation of q/k gain offsets in qk_gain_offsets function may be incorrect for Hunyuan models. The function assumes a fixed layout where q_norm is at gap_start + kNormOff and k_norm is at gap_start + (kNormOff + per_head) + kNormOff. However, for Hunyuan models, the actual layout might differ, leading to incorrect gain offsets being used. This is addressed by introducing wide_q_attn_section and qk_gap_block_why functions to properly handle Hunyuan's layout. o.q_norm = gap_start + kNormOff; // block 0: [64 B zeros][q_norm bf16 [head_dim]] o.k_norm = gap_start + (kNormOff + per_head) + kNormOff; // block 1: [64 B zeros][k_norm bf16 [head_dim]] return o; Potential silent failure in qk_gap_block_why The qk_gap_block_why function checks for zero headers and finite gains, but it does not validate that the gains are actually from the correct tensor. If the tensor is corrupted or misaligned, the function might not detect the issue, leading to incorrect gains being written. This is mitigated by the new validation logic that checks each layer's bytes to be the [header][gain] pair before writing a gain. std::string qk_gap_block_why(const uint8_t* gap, int64_t head_dim) { const int64_t per_head = head_dim * 2; for (int b = 0; b < 2; ++b) { const uint8_t* blk = gap + int64_t(b) * (kNormOff + per_head); const char* nm = b == 0 ? "q_norm" : "k_norm"; for (int64_t i = 0; i < kNormOff; ++i) if (blk[i]) return std::string("the 64 B before ") + nm + " are not zero (byte " + std::to_string(i) + " is " + std::to_string(int(blk[i])) + "), so this is not the [header][gain] pair"; bool any = false; for (int64_t i = 0; i < head_dim; ++i) { const uint16_t v = uint16_t(blk[kNormOff + 2 * i]) | uint16_t(uint16_t(blk[kNormOff + 2 * i + 1]) << 8); if ((v & 0x7F80) == 0x7F80) return std::string(nm) + " element " + std::to_string(i) + " is not finite (bf16 bits " + std::to_string(int(v)) + "), so these bytes are not a gain"; any = any || (v & 0x7FFF) != 0; } if (!any) return std::string(nm) + " is all zero, so these bytes are not a gain"; } return std::string(); }

[bong-water-water-bong]

VERDICT: READY at 770797f

Built and ran it: all npu-pack checks passed — and the section offsets close, which is the check that makes the arms more than a transcription of the numbers.

base   = 2112 + 64*39168 = 2,508,864
q_norm = base + 64   = 2,508,928        (the arm's 2508928)
k_norm = base + 384  = 2,509,248        (320 B after q_norm, as claimed)
o_proj = base + 640  = 2,509,504        (base + the 640 B gap)
2 x 320 = 640  ==  the gap

The two q/k gain arrays are exactly the gap. That is the strongest evidence in the PR: the "remainder is the 640 B gap" and "q_norm … k_norm, 320 B after q_norm" are not independent guesses — two
320-byte arrays fill the 640-byte remainder with nothing left over, so a wrong base, a wrong stride or a wrong gain size would break the sum rather than shift everything by a constant.

The negative arms are what make it a rule rather than a match, and they are the pair I would have asked for:

check(z.ok && z.gap == 0 && z.q_norm == 0 && z.k_norm == 0 && z.o_off == 2112 + 64*39168, …)
check(odd.ok && odd.gap == 512 && odd.q_norm == 0 && odd.k_norm == 0, …)
check(!pi::qk_norm_refusal(true, odd.k_norm != 0).empty(), "… and such a pack is still refused")

A pack with no remainder names no norms, and a pack with a 512-byte remainder names none either and is refused when use_qk_norm is declared — so the rule is "the gains are here because the gap is
exactly two of them", not "a gap exists". That is the difference between reading a layout and inferring it from a name, which is the defect behind #1889 and behind three other PRs tonight.

Scope discipline is explicit and correct: "Slice: Hunyuan-0.5B-Instruct only, the pack #1976 does not measure" — and it says which main merge it sits on (66da312) and that #1976's work reached main
through #2218, so the reader knows what is already there and what this adds.

Not run: the pack itself (the crafted/golden legs are what ran; the pack leg skips without a pack directory). The offsets above come from the arms' own numbers, which is where a section rule can be
checked without the artifact.

[bong-water-water-bong]

VERDICT: READY at 770797f — re-issued for the new head. Of its 39 commits since eb5c058, 38 are merges of main and the one real commit is the review's answer, so this is the shape a stacked PR should look like after a rebase.

The new commit extracts the branch, which is what makes my earlier point testable

"npu-pack: the wide-q branch sets q,k,v through apply_wide_q_section(), and a test arm pins it (review of #2063)"

My verdict at eb5c058 raised one residual: the q_first decision rides on an observable (if (wq.k_norm) p.attn_q_first = true;) rather than on a declaration, so a pack with k_norm gains in the gap that is not the Hunyuan writer would be read as q,k,v. That concern stands in the code — but it is now reachable by a test, and that is the difference between a latent risk and a reviewable one:

check(pi::apply_wide_q_section(w, wq, 16, 32, 2048, 1024), "wide-q plan: Hunyuan-0.5B's section is taken");
check(w.attn_q_first, "wide-q plan: a wide-q block with its gains in the gap is q,k,v (engine#1889, …)");
check(wt.q == 0 && wt.k == 32 && wt.v == 48,
      "wide-q plan: q at tile 0, k at 32, v at 48 -- the rows the checkpoint matches (q 0-31, k 32-47, v 48-63)");

The third arm is the good one: it pins the tile offsets against the checkpoint's row ranges, so a wrong q_first fails with the numbers. And extracting apply_wide_q_section() also means the branch can be driven without a container — the same #2026/#2055 move (pull the arithmetic out of the load path so it can be asserted).

What is still open, unchanged

if (wq.k_norm) remains the trigger. With the function extracted, the cheapest closing move is now one arm: a section with k_norm gains whose architecture is not the Hunyuan writer must not set attn_q_first — or, if k_norm-in-the-gap is intended to be sufficient, a comment saying so and why no other family on this box can produce that shape. Either answer is a line; leaving it implicit is what turns an observable into a rule by accident.

Scope: +31/−14 in pack_import.cpp plus the arms, and everything else in the 39-commit range is main. That is the correct composition for a PR stacked on #1976's merged content.

[bong-water-water-bong]

VERDICT at 770797f: READY

Re-review of 770797f. It adds 770797f8, which answers my CHANGES at af80a93 (the q,k,v tile order had no test). The wide-q branch now goes through apply_wide_q_section(), and a new arm pins it:

• attn_q_first is set;
• attn_first_tiles(16, 32, true) puts q at 0, k at 32 and v at 48, the rows the checkpoint matches;
• the section's gains are named;
• without gains in the gap, there is no q,k,v claim.

Checked at this head (ONEBIT_NPU=ON ONEBIT_NPU_HOST=ON review build):

• npu_pack_import_test: all checks passed.
• The same mutant as last time, with if (wq.k_norm) p.attn_q_first = true; removed (now inside apply_wide_q_section): rc 1, 2 FAIL, starting with FAIL wide-q plan: a wide-q block with its gains in the gap is q,k,v. At af80a93 the same mutant gave rc 0. After restoring the file the tree is clean and the test passes again.
• It merges cleanly onto main. CI build at 770797f is green: 100% tests passed, 0 tests failed out of 241.

Not re-run here: the forward check against tencent/Hunyuan-0.5B-Instruct. The pack and the checkpoint are no longer on this box (both HF cache entries hold only refs/). The new arm is what keeps the body's measured order from regressing.