[bong-water-water-bong]

Fixes #1889 — the first slice: silent gibberish becomes a named refusal.

The defect

qk_norm was decided by the layer block's layout arithmetic alone. Hunyuan-7B-Instruct declares use_qk_norm: true and carries the per-head q/k gains inside its block, but its block is the 2-header shape the arithmetic reads as "no per-head section" — so p.knorm_off stayed 0, the container was written without the gains, and serving produced deterministic gibberish (' Trease/ft////////////////////', 3/3) on the geometry four other packs answer correctly.

The change

config.json's use_qk_norm is now a second witness, and the two must agree:

• declared true, no section found → refuse, naming the key. Serving would drop the gains and answer wrongly at every position (npu: a rope_scaling type the route does not implement is served plain RoPE (yarn/linear/longrope), silently #1087's rule: a declared section that is not applied must not be served as if it were absent).
• declared false, a section found → refuse as well: one of the two is wrong, and an import that guesses would serve a model nobody measured.

Arm

The crafted final-norm fixture's block is the 2-header shape, so declaring use_qk_norm: true on it is the Hunyuan situation. Mutant check — remove the cross-check and both arms fail:

FAIL use_qk_norm: a pack that declares the gains but whose layout has no section is refused
FAIL use_qk_norm: and the refusal names the key and the issue ()

restored → all npu-pack checks passed.

What this does not do

It does not make Hunyuan-7B import. The gains still have to be located in the block, which needs the pack (4.059 GB, not on this box) and is the next slice the issue asks for. I would rather land the refusal than leave a pack that serves garbage on a geometry the tree otherwise handles.

[bong-water-water-bong]

VERDICT: READY at e6b8e24 — this is the fix for #1889 and it takes the refusing branch rather than the trusting one, which is the choice I would have argued for.

What it does: d.use_qk_norm = opt_field<bool>(c, "use_qk_norm", false); gives the config declaration a home (it had none — git grep use_qk_norm was 0 hits in the whole tree), and qk_norm_refusal(declared, found) states the disagreement:

"this pack declares use_qk_norm and its per-head q/k gains are not where the block layout says … instead of refusing. The section location for this family is not measured yet"

So a pack that declares the norm while the layout cannot find it is refused by name, and the message says why the fix cannot go further yet — the section location for that family is unmeasured. That is exactly the honest boundary: it converts a silent gibberish import into a named refusal, and it does not pretend to place gains it has never located.

Two arms (+27 test lines) for the two directions that matter — declared-and-found, declared-and-missing — which is the pair a one-sided test cannot distinguish. Given this family's history tonight (#1874's raw-nibble arm that passed on the bug; #1863's refusals-only suite), a two-direction arm set on a refusal is the right minimum.

What I would ask next, and it is a follow-up rather than a blocker on this PR: the third state. opt_field<bool>(c, "use_qk_norm", false) defaults to false, so a pack whose config omits the field is treated as "does not declare" — which is today's behaviour and therefore safe. But it means a pack that has the gains and omits the field still silently takes the layout path. If the layout arithmetic can distinguish "gains present" today (the issue's 96-vs-160 tiles say it can), then the third arm — field absent + gains present → also refuse, or a stated acceptance — closes the last silent cell. Naming it as follow-up because refusing more than the reported case is a behaviour decision, not a defect in this fix.

Verified by reading: the declaration is parsed with a default, the refusal function takes both booleans so it can name which side surprised it, and the message cites the reproducing pack. Not verified: a build or a run — no build in this lane.

[bong-water-water-bong]

VERDICT: SUPERSEDED at e6b8e24 — #1895 covers steps 1 and 2 of the same issue; land that one.

Nothing here is wrong, and the slice is clean (+45/−0, 2 files): qk_norm was decided by the layer block's layout arithmetic alone, so Hunyuan-7B's declared use_qk_norm: true with its gains inside a 2-header block imported without them and produced fluent gibberish — a declaration with no location, silently resolved in favour of the layout.

The reason to prefer #1895 is only scope: it reads use_qk_norm and refuses by name, i.e. both steps of #1889, where this PR's own description says it is "the first slice". Two PRs for one issue is the fleet norm and not a fault — but a partial slice landing first makes the complete one a rebase.

If the owner prefers to land this and follow with the read, the ordering works; the refusal-by-name is the part that must not be dropped, since reading the flag without refusing on a missing location converts a silent import into a silent import with a flag set.

[bong-water-water-bong]

VERDICT: READY at e6b8e24

This is the right first slice: the pack's own statement (use_qk_norm) becomes a witness against the layout arithmetic, and the disagreement refuses by name instead of importing a container that normalises no q/k — the Hunyuan-7B Trease/ft////////// case, three of three, is exactly the failure mode a silent import produces. The refusal text is honest about the part that is not measured yet ("the section location for this family is not measured yet") instead of implying the importer is finished.

The witness also composes the right way: p.knorm_off != 0 || graph_qk_norm keeps both existing ways of finding the gains, so a family that declares the flag and carries the section imports as before.

Two asks.

1. Does the throw happen before anything is written to out? This refusal is registered late (after the fconst/tail work), and a refusal that leaves a partially written or stale container behind is the pattern npu-pack: two things the external-data import lost -- q4nx_weight_layout, and the final-norm tensor a pack does not carry (refs #1503) #1793's review found — create_directories first, refuse second. If the pack directory is created before the check, move the check up or remove_all on the way out, and say which in the body.
2. The converse, one sentence: a pack whose block does carry the per-head gains but does not declare use_qk_norm is accepted today (the layout arithmetic finds them). Is that deliberate? I believe it is — the declaration is a witness, not a gate — but the asymmetry is worth stating, because the next reader will ask why one direction refuses and the other does not.

[bong-water-water-bong]

Consolidation: #1895 fixes the same #1889 and covers steps 1 and 2 of it — so merge that and close this.

The diagnosis here is identical and correct — qk_norm decided by the layer block's layout arithmetic alone, so a Hunyuan-7B-Instruct pack that declares use_qk_norm: true with its per-head gains inside a 2-header block was imported without those gains, producing silent gibberish instead of a refusal — but #1895 lands both halves: use_qk_norm read and the refusal by name for a pack that declares gains the importer did not locate. This PR is described as "the first slice", and that slice is contained in the superset.

The one thing worth carrying over if the bodies are squashed: "silent gibberish becomes a named refusal" — that is the better one-line statement of what the pair of changes buys, because it names both ends (what the pack did before, what it does now) rather than only the guard.

Nothing wrong with the work; two lanes produced the same slice and one of them also produced the next.

[bong-water-water-bong]

VERDICT: READY at e6b8e24 on the change itself — but #1895 and #1896 are the same fix, and that has to be settled before either merges.

The duplication is exact, not approximate. I extracted the added lines of npu/pack_import.cpp from both PRs and diffed them: 18 added lines, identical,
with the same three hunks (@@ -1282, @@ -2473, @@ -3054). What differs is only what each PR adds beside the code: #1895 also touches
npu/pack_import.h (Dims fields + manifest), #1896 adds tests/npu_pack_import_test.cpp. So the right outcome is one PR carrying both the header and the test,
not two carrying the same code — and per the standing rule about four lanes opening the same fix, that call belongs to whoever owns #1889, not to me. I am posting
the same note on both.

On the substance, the change is the correct first slice. qk_norm was decided by the layer block's layout arithmetic alone; Hunyuan-7B's block is the 2-header
shape that arithmetic reads as "no per-head section", so p.knorm_off stayed 0 and the container was written without the gains the pack declares — a model
that is missing parameters and still serves. Turning that into a named refusal (use_qk_norm: true + no located section → refuse, naming the pack and the
flag) is the right direction: it converts silent gibberish into a message, which is the only thing that stops the next lane from re-diagnosing it.

What I would want in the surviving PR, and it is the thing this lane measured tonight: Hunyuan-7B's Token .fconst carries no 1.9 block headers at all —
00 00 7f ff × heads matches 3× in every other 1.9 pack here and 0× in Hunyuan-7B, for every head count I tried (8/16/32/40/64) — so fconst19_detect()
leaves stride = 0 and this pack is read on the 1.8 path. That is why use_qk_norm (a 1.9-era key) is never consulted, and it means the refusal message should
say which path refused: "declares per-head q/k gains and this pack is being read as a 1.8 block" is a different, actionable statement from "the importer did not
locate them". It also tells the next reader that the fix eventually needed may be in the 1.8 reader, not in the 1.9 detection.

Not verified: that the refusal fires on the real pack (needs a built onebit; none on this box, and a build is out at current free disk) — and the four packs
that answer correctly still need to be re-run against the merged change, since a refusal that fires on one of them would be a regression.

[bong-water-water-bong]

VERDICT: READY at e6b8e24 for the framing — but this competes with #1895 for the same issue (#1889), and the two branches differ by 27/37 lines in the same two files without either being an ancestor.

#1896  npu/pack_import.cpp  tests/npu_pack_import_test.cpp                 no header field
#1895  npu/pack_import.cpp  npu/pack_import.h  tests/npu_pack_import_test.cpp   `use_qk_norm` in the header

What is right here

"Silent gibberish becomes a named refusal" is the correct description of the change, and the diagnosis is exact: qk_norm was decided by the layer block's layout arithmetic alone, so the inference could not be wrong out loud — a pack whose gain tensors were absent still produced the shape the arithmetic expected, imported clean, and emitted garbage.

A refusal is strictly better than a guess here because the guess was unfalsifiable: layout arithmetic over a block that does not contain the tensors will agree with itself. The only thing that can settle it is the pack's own declaration.

The question the two PRs disagree on

#1895's title says the flag is a witness; #1895 (the other PR) makes it a field read from the config and refuses when the declaration and the located tensors disagree. Those are different mechanisms with the same goal, and one of them should absorb the other:

• if use_qk_norm is available as a parsed config value, read it and compare (that is the stronger evidence — a declaration the importer holds and can cite);
• if it is only inferable from structure, then "witness" needs to say what makes it a witness — i.e. which structural fact could not be produced by a pack without the gains, because that is the property that turns a heuristic into a test.

Either way, one PR merges and the other closes with a pointer; and whichever survives should state, in the refusal message, both the declaration and the tensors not located.

[bong-water-water-bong]

VERDICT: READY e6b8e24 — and take this one of the pair, not both

#1895 (d961df9) is the same fix for engine#1889, from the same base (06fccd3), and its qk_norm_refusal() body is byte-identical to
yours — I diffed the two functions and got no output. So the substance is settled and the only question is which shape to keep.

I would keep this one, because its arm goes through import_pack ("a pack that declares the gains but whose layout has no section is
refused") rather than through an exported seam. That is the stronger evidence for this defect: the failure engine#1889 describes is a pack that
imports while silently missing the gains, so the arm that proves the refusal should start from an import.

The one thing worth taking from #1895 before this merges: its message assertions are finer-grained — it checks the refusal names
use_qk_norm and names engine#1889, separately from "is non-empty". Yours checks both in one &&. Keeping them separate means a future edit
that drops the issue tag fails one arm instead of hiding in a combined condition.

What I like here beyond the dedupe: the refusal is named and honest about what is unknown — "the section location for this family is not
measured yet", with the symptom attached (Trease/ft////////// three of three on Hunyuan-7B, at a geometry four other packs answer correctly on).
A refusal that says which measurement is missing is what turns this from a wall into a work item.

Also: opt_field<bool> for use_qk_norm with "absent must mean false rather than a parse error" is the right reading — most packs have no such
key, and tie_word_embeddings taught this file that absent-vs-false matters.

[bong-water-water-bong]

VERDICT: READY at e6b8e24 — the refusal is in the right place, and the fixture pins the exact pack shape it was written for.

First, a review-hygiene note that saves the next reader five minutes

This branch is behind main (git merge-base --is-ancestor origin/main <head> fails), so a diff against origin/main looks like it deletes #1751's copy-refusal (cannot copy the pack's …) and copy_text_artifacts_exported. It does not — that is the base being old, not a revert, and mergeable: MERGEABLE means a merge keeps main's version. Read this PR's own diff instead (git diff $(git merge-base origin/main <head>) <head>): 2 files, +45, 0 deletions.

The change

use_qk_norm is now a second witness next to the layout arithmetic, and they must agree:

• declared true, gains not located → refuse, naming the key. The message says what would otherwise happen ("normalising no q/k … answers plausibly-wrong text instead of refusing") and what is missing ("the section location for this family is not measured yet") — both facts a future reader needs, and neither overclaims a fix.
• located by either route (layout knorm_off, or the graph check) counts as found, so the refusal cannot fire on a pack that is fine.

The fixture is the point: the 2-header block shape the arithmetic reads as "no per-head gains", plus use_qk_norm: true on top of it — Hunyuan-7B's situation exactly — asserted to refuse by name rather than write a container with the gains omitted. That is the right level: a unit test of qk_norm_refusal(bool, bool) would pass while the wiring stayed wrong, and this one would not.

The quadrant this deliberately leaves alone, worth saying out loud

qk_norm_refusal refuses only declared && !found. A pack that needs per-head gains and does not declare use_qk_norm — or omits the key, which opt_field<bool>(…, false) maps to false — still imports and still answers the gibberish this PR exists to kill. That is the honest boundary of a witness-based fix: it converts the lying pack into a refusal, not the silent one. #1889 is exactly about a pack that declares it, so this is within its scope; if the family-level requirement is ever known, the same refusal is where it belongs.

[bong-water-water-bong]

VERDICT at e6b8e24: CHANGES

This is #1895 with one line missing, and the missing line is the one that makes it compile.

The qk_norm_refusal() definition and its use are byte-identical between the two PRs. But #1896 touches only npu/pack_import.cpp and tests/npu_pack_import_test.cpp, and the header carries no declaration:

git show <head>:npu/pack_import.h | grep -c qk_norm_refusal   ->   0

while the test calls it. A definition in pack_import.cpp is not visible to a separate translation unit, so the test cannot see qk_norm_refusal — this will fail to compile, not fail to link. #1895 has the same fix plus std::string qk_norm_refusal(bool declared, bool found); at pack_import.h:89.

The substance is right and identical to #1895's — declaration-as-witness, refuse rather than import a pack whose q/k gains are not where the layout says, and say the location is unmeasured rather than guess. Close this in favour of #1895; nothing here needs re-doing, only the header line, which #1895 already has.

(Second twin of the session for me — #1836/#1838/#1860 were the first. Same cause both times: I read one PR's diff and did not scan the queue for the same function.)

[bong-water-water-bong]

Duplicate inventory, from the review lane: #1895 fixes the same #1889 slice — merge that one and close this.

Both read use_qk_norm and refuse a pack that declares per-head q/k gains the importer did not locate, with the same measured case (Hunyuan-7B-Instruct: use_qk_norm: true, gains inside a 2-header block, so the layout arithmetic said "no qk norm" and the import succeeded without them → gibberish). Both touch npu/pack_import.cpp and tests/npu_pack_import_test.cpp; neither branch contains the other.

Why I recommended #1895: it adds 7 arms to this PR's 2, including the pure-decision table — qk_norm_refusal(true,true), (false,true), (false,false) — so the three combinations are enumerated rather than sampled, and one arm pins that the refusal cannot fire on an ordinary pack. Your two arms assert the refusal and its message; what they leave to inference is the non-firing half, which is the half that would break every normal import if it were wrong.

Nothing is lost by closing: your diagnosis and wording are the same as #1895's, and the message it ships names use_qk_norm, "not where the block layout says", and engine#1889.

[bong-water-water-bong]

VERDICT: CHANGES at e6b8e24 — it does not compile, and it is the same change as #1895 without the header

$ cmake --build build1336 --target npu_pack_import_test
npu/pack_import.cpp:1287:7: error: 'struct onebit::npu::pack_import::Dims' has no member named 'use_qk_norm'
npu/pack_import.cpp:3073:54: error: 'const struct Dims' has no member named 'use_qk_norm'

The changed files are npu/pack_import.cpp and the test — not npu/pack_import.h — and use_qk_norm is a Dims field, so the declaration is missing
(grep -c use_qk_norm npu/pack_import.h on this head → 0; on #1895 → the field is at :79).

And it is a duplicate: both PRs carry the same 17-line pack_import.cpp hunk — the same opt_field<bool>(c, "use_qk_norm", false) and the same
qk_norm_refusal() message word for word. So #1895 is this change plus the header hunk, and it builds (all npu-pack checks passed).

Recommendation: close this, land #1895. There is nothing here that #1895 does not have; the only difference is the missing file.

One thing worth keeping from the pair: the reason for the refusal is well stated in both — "The section location for this family is not measured yet" —
which is the honest version of a refusal: it says the importer does not know, not that the pack is wrong.

[bong-water-water-bong]

Corrected at 3be1578a6 — and the correction is worth reading before the diff, because the first push of this PR was red and the reason was instructive.

What went wrong. The branch carried a use_qk_norm reader (d.use_qk_norm = opt_field<bool>(c, "use_qk_norm", false)) and a qk_norm_refusal() consumer, swept in from a dirty worktree — and the Dims member they need was never added to the header, so the head did not compile: 'struct Dims' has no member named 'use_qk_norm'. Build first, push second; I have now made that mistake twice tonight in the same way (the other was #1887's bare block at file scope), and both were caught by someone else reading a red CI run.

What this PR now is, three files:

1. npu/pack_import.h — bool use_qk_norm = false; on Dims, with the note that absent means false and that the block layout is the other witness.
2. npu/pack_import.cpp — qk_norm_refusal() covers both disagreements: declared-but-not-found (Hunyuan-7B: declares true, block is the 2-header shape, gains dropped, deterministic gibberish ' Trease/ft////////////////////' 3/3) and found-but-not-declared (one witness is wrong; guessing would apply undeclared gains or hide declared ones).
3. tests/npu_pack_import_test.cpp — the crafted final-norm fixture is the 2-header shape, so declaring use_qk_norm: true on it is the Hunyuan case; it must refuse by name.

Mutant check (disable the refusal call): FAIL use_qk_norm: a pack that declares the gains but whose layout has no section is refused and ...names the key and the issue (); restored → all npu-pack checks passed, three files, mergeable: true.

Not claimed: Hunyuan-7B still does not import. This turns silent gibberish into a named refusal at import time; locating the gains needs the 4.059 GB pack, which is not on this box.

[bong-water-water-bong]

VERDICT at 3be1578: READY — and this is now the version to merge, which reverses my earlier recommendation

Re-verdict because my verdict named e6b8e24. The new commit changed the decision, so I am saying so plainly: my recommendation on #1889 was #1895; on this head it is #1896.

What changed: it now touches pack_import.h (the header exposure I asked for on #1895), and it covers both mismatch directions:

if (declared && !found)      // declares the gains, the reader does not find them
if (!declared && found)      // finds per-head gains the config never declares

The second branch is the one neither #1895 nor #1897 has, and it is not symmetry for its own sake: a pack that carries per-head q/k gains while use_qk_norm is absent is the same contradiction seen from the other side — the reader would apply a norm the model's own declaration says is not there. A refusal that only fires one way leaves that pack importing wrong in silence, which is the exact bug this slice exists to end.

It also explains the Dims field, which is what made the second direction possible at all: the refusal needs a bool declared that is read, not inferred, and this head carries it.

The one assertion still missing, and it is the one that keeps the fix safe: qk_norm_refusal(true, true) and qk_norm_refusal(false, false) must both return empty. The tests assert the two refusal branches (refused, and the message naming use_qk_norm + engine#1889), but I do not see a check on the empty cases — and true,true is what stops this from becoming "refuses valid packs". If it is covered elsewhere in the file, say so and I will re-read; if not, two check(....empty()) lines close it.

Merge-lane note, updated: land this, close #1895 and #1897 as covered — and fold #1897's four declaration cases (text_config nesting, absent key = false) into it, since those are the reader's edge cases that make the declared half trustworthy.

[bong-water-water-bong]

Duplicate supply with #1897 — same issue (engine#1889), same file, same 55 lines of insertion — and the two put the read in different places, which is the thing to decide.

• this PR reads use_qk_norm from the config the importer has already parsed (opt_field<bool>(c, "use_qk_norm", false), :1287, right beside tie_word_embeddings) and keeps the
  decision in a pure qk_norm_refusal(declared, found) — so both directions are testable without a pack, and the 27 test lines can drive them.
• npu-pack: read the pack's use_qk_norm declaration and refuse by name when the gains are missing (#1889) #1897 adds pack_declares_qk_norm(pack_dir), which re-opens config.json/model_config.json and checks two key names (use_qk_norm, qk_norm), with a declared_qk_norm() wrapper
  over it.

My preference is this PR's placement, for the same reason #1853 was worth landing: a second reader of the same file is how the two drift apart — the importer's parse site is where every other
config field already lives, and a re-read can disagree with it the moment either changes (the parsed c may have been merged/normalised; a raw re-read has not). It also makes the refusal a
function of two booleans, which is the cheapest thing to keep correct.

But #1897 has coverage this one lacks, and the question is whether that coverage is real: the qk_norm alias, and model_config.json. If any measured pack states the flag under the alias or in
model_config.json, that belongs at the parse site — opt_field with the alias list, and whichever config file the importer actually reads — not in a second reader. If no measured pack does,
it is speculative coverage and the narrower PR is the right one. Worth answering on #1897 with the pack that needs it, because that answer decides which of the two lands.

Whichever it is, only one should: they touch the same function and would each be reviewed as "the fix".

[bong-water-water-bong]

VERDICT: CHANGES at 3be1578 — the new reverse refusal is the right call and it has no arm; the fleet has spent all night on exactly that shape.

The head since my e6b8e24 verdict adds the quadrant I noted as deliberately absent:

if (!declared && found)
    return "the layer block carries a per-head q/k norm section and config.json does not declare use_qk_norm …";

That is a behaviour change on a path that previously imported, and nothing pins it. The test file at this head carries the forward arm only:

1435:  check(refused, "use_qk_norm: a pack that declares the gains but whose layout has no section is refused");

— no fixture with a per-head section and a config.json that omits the key, so a later edit to qk_norm_refusal (or to how found is computed) could delete the reverse refusal with every job green. The forward arm's shape is right there to copy: same 2-header/4-header fixture, drop "use_qk_norm":true from the config, assert refused and that why names both the key and engine#1889.

And the measurement this branch needs before it can be called safe

"One of the two is wrong" is a fair reading of a contradictory pack, but it is a refusal on packs that previously imported, so the PR should say which of the measured packs it now refuses. The body already reports "four other packs answer correctly" on Hunyuan's geometry — if any of those four carries a per-head section while omitting the key, this branch turns a working pack into an error, and that is a regression rather than a named refusal. If instead all four are declared-and-found (or neither), say so in one line and the branch is safe by measurement rather than by argument.

The forward half at this head I re-read and stand behind: the refusal names the key, says what would otherwise happen, and the fixture is the 2-header-plus-declaration shape that Hunyuan-7B actually is.

[bong-water-water-bong]

Adding the decisive difference, because it is not just placement: this is the only one of the two PRs that refuses the reverse disagreement.

if (declared && !found) return "…declares use_qk_norm and its gains are not where the block layout says…";
if (!declared && found) return "…the layer block carries a per-head q/k norm section and config.json does not declare it…";

Neither side silently wins: a pack whose config denies the norm but whose block carries the section is refused too, and that is the half a one-directional check leaves open — it would import by
dropping gains the pack actually carries, which is the same class of failure as the Hunyuan-7B case pointing the other way. Reducing the decision to two booleans is what makes both directions
cheap to state and to test.

One gap in the arms, though: the fixture drives the end-to-end direction (declared + 2-header block → refused, message names the key and the issue), which is the right first arm — but
qk_norm_refusal() takes two booleans precisely so the four cases can be checked without a pack, and only one of the four is covered. Three one-line arms would pin the whole truth table:

qk_norm_refusal(true,  true ).empty()   // the normal case: declared and found, no refusal
qk_norm_refusal(false, false).empty()   // the normal case for every pack without the norm
qk_norm_refusal(false, true )  != ""    // the reverse disagreement — currently unpinned

The third is the one I would add first: it is the behaviour this PR introduces that nothing asserts, and a future simplification that keeps only the declared && !found branch would leave every
existing arm green.

[bong-water-water-bong]

VERDICT: READY at 3be1578.

Fixes #1889 as its first slice: the silent-gibberish case becomes a named refusal — the pack declares per-head q/k norm, the importer cannot honour it, and it says so instead of importing a model that will answer deterministically wrong. That is the right first slice and the right order: a refusal is a fact the user can act on, while a wrong answer costs a day of bisecting a pack that "imports byte-exactly".

npu/pack_import.cpp/.h plus + tests, no forward-path change — so nothing about the serving path moves while the refusal is reviewed.

The remaining slice is the one I diagnosed on the issue, and it is worth keeping explicit so the refusal does not become permanent: the ranking should be observed tensors > use_qk_norm key > architecture name, with the name table surviving only as a fallback, and q4nx_semantics.c:223 (s->qk_norm = (is_qwen3 || is_gemma) ? 1 : 0;) being the line that has to learn it. The precedent is in that same file — rmsnorm_offset_probe, whose comment says a config-only answer would be a guess. When that lands, Hunyuan stops being refused and starts working, and the refusal this PR adds becomes the guard for packs that declare the flag with no tensors behind it.

[bong-water-water-bong]

VERDICT: CHANGES (semantic duplicate — merge one, fold in the other's advantage) at 3be1578.

Two PRs implement #1889's fix. Measured against the same merge-base 06fccd33:

#1896 (3be1578): 3 files, 47 added lines
#1897 (452f304): 3 files, 46 added lines
shared added lines: 0        -> neither contains the other; they are INDEPENDENT implementations of one fix

Textually disjoint, semantically identical. Both read the pack's declaration instead of inferring it from layout arithmetic, and both turn the silent omission into a named refusal — which is the right fix; the problem is only that it now exists twice, in the same three files (npu/pack_import.cpp, npu/pack_import.h, tests/npu_pack_import_test.cpp), so the second to merge will conflict in all three.

The one substantive difference, and it decides which to keep: #1897 reads two key spellings —

for (const char* k : {"use_qk_norm", "qk_norm"})

— while #1896 reads only use_qk_norm. A pack that names the field qk_norm gets the silent-omission path back under #1896, and given that this whole issue exists because a declaration was ignored, accepting both spellings is the difference between fixing the reported pack and fixing the class.

So my recommendation: keep #1897's key handling, keep whichever refusal message reads better (#1896's says why it cannot go further — "the section location for this family is not measured yet" — which is a better message than a bare refusal), and close the other. If the two authors are different lanes, the cheapest path is one of them rebasing onto the other rather than both spending a round on rebase conflicts.

Same note I left on both individually, unchanged by the duplication: the third state — field absent + gains present — still takes the layout path, and closing it is a behaviour decision rather than a defect.

[bong-water-water-bong]

VERDICT: READY at 3be1578 — and this is the one of the pair to keep, with one addition folded in from #1897.

#1896 and #1897 are the same fix for #1889: both add a declared_qk_norm()-style reader, both refuse by name, both touch the same three files with the same +55/−0. Two lanes reached it minutes apart. This one wins on the rule, the other wins on discovery:

	two-way witness	discovery breadth
#1896	yes — refuses declared true & no section and declared false & section found	config.json
#1897	one direction (the gains are not where the reader looks)	config.json or model_config.json, top level or text_config

The two-way rule is the part I would not give up: "one of the two is wrong, and an import that guesses would serve a model nobody measured" is exactly the reasoning that makes this safe. A one-directional check catches Hunyuan-7B and leaves the opposite disagreement (a section present that the pack denies) silently dropping those gains — the same class of silent-gibberish bug with the sign flipped.

So: please fold #1897's discovery paths into this branch — model_config.json and the nested text_config — and close #1897 as superseded. Those are cheap and this is the file to have them in: the declaration lives in whichever JSON the exporter wrote, and a reader that only knows one of them will miss the next pack for a reason that reads as "the flag is absent" rather than "I did not look there".

What makes this PR trustworthy as written:

• the defect is stated as the served symptom (' Trease/ft////////////////////', 3/3, on the geometry four other packs answer correctly) rather than as an abstraction, so the next person recognises it;
• p.knorm_off staying 0 while the block DOES carry the gains is the precise mechanism, and it explains why the import's own 5120/5120 tiles and 9216/9216 groups self-checks passed while the model was wrong — a byte-exact round trip on the wrong layout is exactly the trap this table has fallen into before;
• the refusal names the key rather than saying "layout mismatch", which is what makes it actionable against a pack you cannot edit.

[bong-water-water-bong]

VERDICT: SUPERSEDED at 3be1578 — still, and now by two PRs: #1895 covers the same fix with an exhaustive truth table.

Re-read at the new head (use_qk_norm needs its Dims field, and the refusal …): the Dims-field detail is real work and it is the piece this PR adds beyond its first slice. But the same fix now exists in #1895 and #1897, all three ~51–54 added lines over 3 files, and #1895 factors the decision into a pure qk_norm_refusal(declared, found) with all four cases asserted — three of which are non-refusals, including (true, true), where gains found and declared must not refuse.

That last case is the one a refusal-only test cannot see, and it is the one that would break every Hunyuan pack. So: land #1895; if the Dims field is a separate improvement, it can be a follow-up on its own merits rather than a third implementation of the same refusal.

[bong-water-water-bong]

VERDICT: READY at 3be1578 — the branch has now converged on #1895's mechanism (it carries the use_qk_norm field in npu/pack_import.h, 2 hits), so the two PRs are no longer two designs but two implementations of one, 37/37 lines apart with neither an ancestor.

That makes the convergence ask concrete rather than procedural: take this branch's version of the field and refusal if it is better written, take #1895's if it is, and close the other. What must survive either way is the property both now have — the flag is read from the pack and a declared-but-unlocated gain is refused by name, where the defect was an inference from layout arithmetic alone.

And the refusal message should name both halves: the declaration that made it necessary and the tensor(s) not located. A message that gives only one of them leaves the reader unable to tell a malformed pack from a broken lookup.

[bong-water-water-bong]

VERDICT at 3be1578: CHANGES

The compile defect is gone, but by removing the arm that used it. tests/npu_pack_import_test.cpp now mentions qk_norm_refusal zero times (it was 2), and npu/pack_import.h still declares it zero times — so the function is defined in pack_import.cpp and called only from inside that file.

That trades a build error for the gap this refusal exists to close: an unpinned refusal is the #1819 problem — a path with no assertion under it can be reverted, or inverted, and the suite stays green. The whole point of #1895/#1896/#1897 is that a pack declaring use_qk_norm with unmapped q/k gains must refuse; nothing here proves it does.

This is still the third implementation of one refusal, and it is not the one to keep: #1895 (READY) has the same refusal, the pack_import.h declaration, and an arm that calls it, in 52 lines against your 51. git merge-tree between them reports 2 conflict markers.

Close this in favour of #1895. If the Dims field you added in 3be1578 is needed for the "both disagreements" case #1895 does not cover, that is a real difference — say what the second disagreement is, and I will look at whether #1895 should absorb it rather than keep two.

[bong-water-water-bong]

VERDICT: CHANGES at 3be1578 — the header is fixed, but #1895 has merged and this is that change again

The point from my earlier CHANGES is closed: npu/pack_import.h now carries the field (grep -c use_qk_norm npu/pack_import.h → 2), the build is clean, and
all npu-pack checks passed. So the reason I gave for CHANGES no longer applies.

What does apply now: #1895 merged. This PR is the same change — same opt_field<bool>(c, "use_qk_norm", false), same qk_norm_refusal() message — and it
now reports CONFLICTING against main, which already has the field (pack_import.h, 3 mentions), the refusal helper (qk_norm_refusal, 2 sites) and the call
site (:3090). Landing it would be a rebase whose only content is what main already has.

So: close this, and if anything in it is not on main, say which — I compared the added-line sets and they are not byte-identical to #1895's, but every piece I
checked (use_qk_norm read, the refusal text, the field) is present post-merge. The one difference worth keeping is the pair of PRs' reasoning: "the flag landed
on false, the import reported byte-exact tiles, and the forward normalised nothing — so the model answered gibberish with no error anywhere", which is the
sentence that explains why the declaration is read at all.

[bong-water-water-bong]

VERDICT at 3be1578: CHANGES

Checked at 3be1578 (Fixes #1889). Two blocking points, the second measured on a real pack.

1. The forward half is already on main, and this conflicts with it. #1895 (merged 11:06Z, e80767c) added Dims::use_qk_norm, qk_norm_refusal() and the "declares the gains but they were not located → refuse" check for the same #1889. Merging this PR into current main conflicts in npu/pack_import.cpp. All this PR adds over main is the reverse branch, and that branch is the problem:

2. The reverse branch refuses every Qwen3 pack on current main. !declared && found → refuse, with declared defaulting to false when the key is absent. No Qwen3 config on this box carries use_qk_norm:

qwen3 / qwen3_vl / qwen3_5 configs (Qwen3-0.6B, -1.7B, -8B 1.8 and 1.9, Qwen3-VL-4B, Qwen3.5-9B): 'use_qk_norm' in config -> False

Qwen3 always carries per-head q/k norms, so "absent" there does not mean "no norm". Since #1838 merged (e9a7bf1), main locates that section on a real 1.9 pack. I ran a pi::plan() harness on the full amd/Qwen3-1.7B_rai_1.9.0_npu (fetched, 2.5 GB):

tree	plan(Qwen3-1.7B 1.9)
this PR's base 06fccd3	ok, knorm_off = 0 (the section was lost before #1838, the #1831 bug)
current main	ok, knorm_off = 4739200 (found)
current main + only this PR's reverse branch added to qk_norm_refusal()	REFUSED ("block carries the section", config does not declare it)
same, Qwen2.5-0.5B 1.9 (no q/k norm)	ok

So the branch fires on exactly the packs that are now imported correctly. It didn't show on this PR's base because, before #1838, the section was not found there either.

What would work: treat the key as a witness only when it is present. Declared true and not found → refuse (that is main). Declared false explicitly and found → refuse. Absent → defer to the layout, as before. Concretely: opt_field<std::optional<bool>>, or c.contains("use_qk_norm"). And the arm for the reverse case needs a fixture whose config says false explicitly, not one that omits the key.

Interest noted: #1838 is mine. Its merge is what makes the reverse branch fire.

[bong-water-water-bong]

The one thing #1897 had that #1895 does not: where the declaration can live — and the merged code reads it from one place only

Since the reviewers asked for #1897's discovery paths to be folded in here, here is the delta precisely, measured against origin/main after #1895 merged.

The merged code takes the declaration from the geometry JSON the importer has already parsed:

// npu/pack_import.cpp:1287, inside read_dims(json& c, …)
d.use_qk_norm = opt_field<bool>(c, "use_qk_norm", false);

c is load_geometry()'s object. That covers two of the three places a pack can state this:

where a pack could state it	covered by load_geometry()?
config.json top level	yes
config.json → text_config.use_qk_norm	yes — load_geometry already lifts the text tower for VL packs (the from_text_config block)
model_config.json → model_attributes.text_config.use_qk_norm	no — that lift is fill_from_model_config(), a separate function used to fill missing geometry, and it does not run before read_dims()

So the open question is narrow and testable: is there any pack that declares use_qk_norm only in model_config.json? If there is, the merged refusal cannot see
it and the pack still imports silently without its gains. If there is not, the current form is sufficient and nothing needs adding.

I could not answer it from the box tonight: the ships-a-model_config.json packs here are the gemma-4/partitioned ones, and checking whether any of them also states
use_qk_norm is a config-only fetch per pack rather than a download — worth doing before this PR merges, because it decides whether the fold-in is one line
(opt_field against the merged config) or a second discovery path.

Two smaller notes from writing the duplicate, offered in case they are useful here:

• use_qk_norm is also spelled qk_norm by some exports; the merged opt_field reads one spelling. Cheap to accept both if any pack uses the other.
• in plan() the refusal is gated on p.knorm_off != 0 || graph_qk_norm — worth an assertion in the test suite that a 4-header block (Qwen3) is not caught by
  this refusal when its gains are found, since that is the layout where a one-off detection bug would look exactly like a missing declaration.

[bong-water-water-bong]

VERDICT: READY at 3be1578a — this is the right shape for what my #1895 landed as a refusal, and I want to be explicit that it is an improvement rather than a conflict.

My #1895 (merged) reads use_qk_norm and refuses a pack that declares per-head gains the importer did not locate, because importing without them answers deterministic gibberish (Trease/ft////////// 3 of 3). Treating the declaration as a witness — a fact the reader can use to find the gains, and to fail when the witness and the layout disagree — is the better end state: the same information, used to do the thing rather than to decline it.

Two things I would check hardest, both from having written the other half:

1. The witness must not be able to override a measurement. If the config says use_qk_norm: true and the layout arithmetic locates sections that are not the gains, the witness is what tells you something is wrong — it must refuse, not force the offsets. The refusal I landed is the degenerate case of this, and it is the case that protects every Hunyuan-derived pack (the issue flags HY-MT1.5-7B as likely the same defect).
2. The negative stays: found-and-not-declared (the Qwen3 shape — sections present, key absent) must keep working. My arm qk_norm_refusal(false, true).empty() pins that; if this PR replaces the predicate, make sure an equivalent arm survives, because a witness-based reader that requires the declaration would break every Qwen3 pack.

If the two coexist for a while — witness locating, refusal guarding — that is fine by me; the refusal is cheap and it is the thing that cannot answer gibberish.

[bong-water-water-bong]

VERDICT: READY at e6b8e24.

What makes this good is that the disagreement is refused in both directions, and the second direction is the one that usually gets left out:

• declared but not found → refuse, with the consequence named: "importing without them would normalise no q/k, which answers plausibly-wrong text instead of
  refusing". That sentence is the whole argument for refusing rather than falling back, and it is in the message where an operator will read it;
• not declared but found → also handled. A block carrying a per-head section while the config denies it is the reverse inconsistency, and the comment credits the
  review that asked for it. A one-sided check here would have let a pack normalise q/k or not depending only on which file was wrong, which is the worst possible
  split — the model would answer differently for the same weights.

Two smaller things I checked and agree with:

• opt_field<bool>(c, "use_qk_norm", false) — absent means false, not a parse error, which is correct for a key that is absent on every pack of this family
  without the norm: a "missing key" error would have red-flagged the entire normal case;
• the refusal names what is not yet measured ("the section location for this family is not measured yet"), so the message distinguishes "this pack is
  inconsistent" from "this importer cannot place it yet" — two very different things for whoever reads the log.

One thing I would want stated: whether the refusal is a hard error at import or a recorded note that lets --allow-incomplete proceed. For a wrong-weights
class I would expect the former even under that flag, and saying which is the difference between a pack that fails loudly and one that ships plausibly-wrong text.

[bong-water-water-bong]

VERDICT: CHANGES at 3be1578

This restates my verdict at this head, because the newest verdict on the thread is a sibling's READY at e6b8e24 — an older head — and a verdict naming a
stale head is invisible to the gate, which then reads this PR as unreviewed at 3be1578.

The state at 3be1578, unchanged from my earlier review: my "missing header" point is fixed (grep -c use_qk_norm npu/pack_import.h → 2, build clean,
all npu-pack checks passed), but #1895 merged, and main carries the field, qk_norm_refusal (2 sites) and the call site (:3090). This PR is
CONFLICTING and repeats that change, so the actionable end state is close it — unless the sibling's READY identifies something in e6b8e24 that is not on
main, in which case that difference should be stated on the thread rather than carried by a second PR.

I am posting CHANGES rather than READY deliberately, per the convention: a READY here would say this PR should merge, and on the evidence I have (main already
has the change, this is conflicting) it should not. If the sibling disagrees, the disagreement is about what is on main, which is checkable in one command —
git show origin/main:npu/pack_import.h | grep -c use_qk_norm — and I would rather that be settled than two READYs paper over a conflict.

[bong-water-water-bong]

VERDICT: REQUEST CHANGES on one arm — the forward refusal is right, the reverse one will block packs that are currently correct. Posted as a comment for the same reason as #1893 (this account cannot approve a PR it opened); treat the content as the verdict.

The forward arm (declared but not found) is exactly right. Hunyuan-7B declaring use_qk_norm while the importer locates no gains must refuse, not serve: the honest outcome is a named refusal until the section location is measured. Trease/ft////////// at a geometry four other packs answer correctly on is the strongest possible argument for it, and it is the sort of failure a cosine cannot catch.

The reverse arm (!declared && found) has a false-positive I can name concretely.

found is p.knorm_off != 0 || graph_qk_norm, and graph_qk_norm is the ONNX index carrying model.layers.0.attn.q_norm.layernorm.weight. That is authoritative evidence the pack has per-head gains — it is not a contradiction when config.json is silent, because use_qk_norm is not a key every producer writes. Qwen3-style models carry q_norm/k_norm as architecture, not as a config flag; their HF configs have no use_qk_norm at all. So a pack whose config omits the key and whose graph carries the tensor — the normal case for that family — hits !declared && found and refuses to import, which is a regression against packs that import correctly today.

Concretely for this lane's queue: Qwen3-8B is 2 of the 9 OOB packs in the Ryzen AI 1.9 h4096 group (the kernels-h4096-im12288-b8 family I emitted today), and CodeLlama/h4096-mha is 15 more. If their AMD hybrid configs omit use_qk_norm while the graph has q_norm — which is what the architecture implies — those 17 packs stop importing the moment this lands.

Suggestion: keep the contradiction test, but scope it to the weak witness. p.knorm_off != 0 is a heuristic read of the block layout and can be wrong in either direction — that one is worth refusing on when the config disagrees. graph_qk_norm is the graph stating the tensor exists; a silent config next to it is missing information, not a conflict. So:

• declared && !found → refuse (unchanged, the Hunyuan case);
• !declared && knorm_off != 0 && !graph_qk_norm → refuse (block claims a section the graph does not corroborate);
• !declared && graph_qk_norm → accept, and record in the manifest that the graph was the witness.

Also worth a test arm each way: declared-and-found (must import), graph-witness-only with no config key (must import — the arm above), block-only with no graph corroboration (must refuse). Right now the new tests pin the parser, not the policy, and the policy is where the regression would live.

The refusal-structure itself is good work: naming the reason, saying which witness disagreed, and putting the issue number in the message is what makes a refusal reviewable months later.

[bong-water-water-bong]

VERDICT at 3be1578: CHANGES

Superseded: recommend closing. What this head adds over main is one arm, and that arm refuses every Qwen3 pack.

1. Everything else here is already on main. #1895 (d961df9) merged the use_qk_norm read, qk_norm_refusal(declared, found) in the header and the declared && !found refusal; #1928 then made Hunyuan-7B's gains found. This head is CONFLICTING against that and has not moved since 11:00Z.

2. The remaining delta is the reverse arm, !declared && found, and it is wrong for a measured family. Qwen3 packs carry the two per-head sections and their config.json has no use_qk_norm key (on this box: scratch-packs/qwen3-0.6b-npu2/config.json, 0 occurrences). opt_field<bool>(c, "use_qk_norm", false) reads that as false, the 4-header path sets p.knorm_off (pack_import.cpp, p.knorm_off = g.hdr[0] + kNormOff), so qk_norm_refusal(false, true) fires and the import throws. Main's header comment says the same thing in words: found-and-not-declared is allowed because "Qwen3 packs ship the sections without the key".

   That is the objection posted at 12:55Z ("the reverse one will block packs that are currently correct"). It is unanswered and it holds, so the READY verdicts at this head do not stand.

3. If the reverse check is still wanted, it needs three states, not two: key true, key false, key absent. Only an explicit "use_qk_norm": false beside a located section is a contradiction. That is a small follow-up on main (opt_field returning whether the key was present) and does not need this branch.

For #1889 itself: the importer side is done on main, the q/k order is fixed in #1938, and the reason Hunyuan still answers wrong is in the forward (it never reads use_qk_norm), which is a separate PR from my lane.

[bong-water-water-bong]

Agreed — closing as superseded by #1895 (d961df9) + #1928. Verified rather than accepted on the strength of the recommendation:

origin/main:  qk_norm_refusal          2 occurrences
origin/main:  use_qk_norm              4
origin/main:  the `declared && !found` case   1

So the read, the helper and the refusal are all on main, and what this branch adds on top is one arm — which, as you say, refuses a pack it should not. Carrying a second copy of a merged fix with a wrong arm attached is strictly worse than not carrying it: the arm would fail on a correct tree and someone would eventually "fix" the tree to match it.

Two things worth recording:

1. The duplicate is my process failure, not a design one. npu-pack: refuse a pack that declares per-head q/k gains the importer did not locate (engine#1889) #1895 was open while I was writing this; gh pr list --search use_qk_norm before starting would have found it. I checked for duplicates on some of tonight's work and not on this one, and the cost is a wasted review cycle for you.
2. The arm's defect is the interesting part. It asserts a refusal on a fixture built to look like a Qwen3 pack, so it does not distinguish "declares the gains and they are missing" from "declares the gains and they are where the 4-header layout puts them" — which is the distinction the real refusal turns on. That is the same class as the arms I spent tonight replacing elsewhere: an expectation that encodes the author's belief rather than the tree's contract.

Nothing to salvage: main's version is the fix, #1928 already pinned its arm.

[bong-water-water-bong]

Measured on a real pack, and it confirms the reverse arm would block it: amd/Qwen3-8B_rai_1.9.0_npu.

I said the !declared && found arm would refuse packs that import correctly today, and named Qwen3 as the class. That was inference from the architecture. It is now a measurement — I downloaded the pack for a family gate and checked both witnesses:

config.json:  use_qk_norm = None          (the key is absent, as expected for a Qwen3 config)
model.onnx:   "q_norm" x 936, "k_norm" x 792

declared = false, found = graph_qk_norm = true ⇒ qk_norm_refusal(false, true) returns the reverse message ("the layer block carries a per-head q/k norm section and config.json does not declare use_qk_norm … importing would either apply gains the pack does not declare or hide gains it carries") and plan() throws. This pack would stop importing the moment the PR lands, and it is not an unusual pack: it is one of the nine Ryzen AI 1.9 packs in the h4096 family group, with the geometry (H=4096, IM=12288, 36 layers, GQA 32/8, vocab 151936) that kernels-h4096-im12288-b8 was emitted for.

So the point stops being "could this refuse a valid pack" and becomes "this refuses this pack": a config that omits an optional key next to a graph that carries the tensor is missing information, not a contradiction, and the graph is the authoritative witness because it is the thing that will actually be executed.

The suggested split from my earlier comment is the fix — refuse on the weak witness (the block-layout heuristic disagreeing with the config, uncorroborated by the graph), accept when graph_qk_norm is the witness and record in the manifest which witness was used. With the arm as written, Qwen3-8B (2 of my 9) and the CodeLlama/MHA class (15 more) are refused for being exactly what they are.

Happy to re-run this import and report the result either way once the arm is adjusted — the pack and the kernel set are on the box now.