[bong-water-water-bong]

#1633's synthesis — a 1.9 pack that quantises its head at a K block other than 128 and declares tied embeddings gets lm_head from the embedding, with q4nx_head_note in the container and the self-check measuring the synthesis — was exercised only by hand on one real pack:

git grep -E 'head_synthesized|selfcheck_tied|q4nx_head_note|does not declare tied' origin/main -- tests   -> 0 hits

and tests/q4nx_tail_head_test.py's "block-32 arm" covers the Python tool's exit code (and skips in CI without onnx). This PR adds the two arms the issue asks for: one fixture, no pack, no device.

arm	asserts
tied (tie_word_embeddings: true)	import succeeds · head_synthesized_note names block 32 · selfcheck_tied set with corr > 0.999 against the embedding · config.json carries q4nx_head_note · the container carries the synthesised lm_head.weight
untied twin	refused: refused carries lm_head.weight: this pack quantises its head at block 32 … does not declare tied embeddings … · the container carries no head

Three things the fixture had to be told, each of which cost a build

1. p.head_block comes from the pack's cache/*_meta.json (read_declared), not from the graph — and the synthesis branch also requires onnx.matmuls.empty(), so the head must not be a node the walker keeps. Between those two the branch looked unreachable from import_pack (I said so on the issue before finding this): the fixture declares the head in cache/ and leaves the graph with no matmul at all, which is the shape the real 1.9 packs have.
2. The embedding must vary. A constant fill quantises to one code with a near-zero scale, so the self-check computes 0/0 and the arm fails for the fixture's sake rather than the code's.
3. The untied arm runs with --allow-incomplete. A strict import bails out at the first refusal, and this crafted pack has 80 of them from its empty layer payload, so it never reaches the head gate — asserting there would test the bail-out, not the head.

Mutation

• inverting the guard (if (tied) { synthesize }) → all four tied arms fail. The arms bite.
• dropping only tie_declared → nothing changes, because tied already defaults to false when the key is absent, so tie_declared && tied ≡ tied. Noted rather than changed: that is npu-pack: a pack whose head is at another block imports, if it is tied (engine#1503) #1633's code, not this test's subject, and it is the kind of redundancy worth a follow-up line rather than a drive-by edit.

Verified

ninja npu_pack_import_test 1bit                       -> rc 0
npu_pack_import_test tests/golden                      -> all npu-pack checks passed
… tests/golden <0.5B 1.9 pack>                         -> all npu-pack checks passed (including the pack leg)
mutation (guard inverted)                              -> 4 tied arms FAIL
tools/copyright.py --check (staged)                    -> rc 0

Fixes #1850. Refs #1633, #1503.

[bong-water-water-bong]

VERDICT: READY at 4de4ac1

This lands exactly what the #1633 review asked for, and the way it was found is the reason it matters: the synthesis "was exercised only by hand on one real pack" — git grep found no arm for it. A path that decides whether to skip reading the head and synthesise it from the embedding is the kind of logic where a mistake produces a model that runs, answers, and is quietly wrong; "we tested it on one pack by hand" is the declaration that it is not covered.

Both halves are asserted, which is what makes it a test rather than a demo:

• tied + off-block-32 head → synthesised, with q4nx_head_note in the container and the self-check measuring the synthesis (so the provenance of the derived head is recorded, not implied);
• untied + off-block-32 head → refused, which is the safety half: an untied pack has no embedding to synthesise from, and taking one anyway would be exactly the silent substitution the tied-only scoping exists to prevent.

That pair is the boundary the review on #1633 asked to have pinned, and it is the same shape as #1813/#1830/#1874 from tonight: the case that must fail is as important as the case that must pass, because the guard exists precisely to refuse it.

No asks. (And for the record: #1633 was blocked on this for a good reason — a loader path validated only by one hand-run real pack is the definition of "found by a user".)

[bong-water-water-bong]

VERDICT: READY at 4de4ac1 — the crafted-pack pair (tied synthesises, untied refuses by name) is the right shape; one comparison would make it exact rather than approximate.

What the arms establish

// engine#1850: the tied block-32 head synthesis, and the untied refusal, on a crafted pack
// #1633 makes `import_pack()` synthesise `lm_head` from the embedding when a 1.9 pack quantises its HEAD at a K block
// 128 and declares tied embeddings, refuses an untied pack by name, records `q4nx_head_note`, and self-checks the synthesis

• arm 1: declared tied → imports, head_synthesized_note mentions block 32 — so the reason the head exists is recorded, not just the fact;
• arm 2: untied → refused by name, which is the difference between "this pack is broken" and "this pack is untied, and here is why that cannot be synthesised";
• both on a crafted pack, so the case runs in CI without a 1.9 model — the same choice as npu-run: refuse a sealed kernel directory by name instead of "cannot read layer_ctx1.elf" #1872's generation naming and npu-pack: an arm for the header-over-vote path, and the seam that lets a test build the fixture #1860's builder.

The one comparison I would tighten

The synthesis's self-check is a correlation (the packer's corr 0.993981 against the embedding, per #1790's evidence). For a head derived from the embedding that is weaker than it needs to be: the synthesised rows are embedding rows, so equality is available — memcmp of the synthesised table against the embedding's rows should hold exactly.

That matters because a correlation check is precisely what would have passed the int4 sign bug in #1874: "for one negative and one positive the difference is off by 16 * scale" keeps the ordering intact, so a near-constant row still correlates. A bit-exact arm here would make the synthesis fail on the same class rather than absorbing it.

Not a blocker — the correlation is a real check, it is just the weaker of the two available.

[bong-water-water-bong]

VERDICT: READY at 4de4ac1 — a crafted-pack test of the tied block-32 head synthesis and the untied refusal, and the fixture is built in code rather than checked in as a binary, which is the right call here.

Measured: one file, +147 lines, 9 new check() arms — this is a test-only change for engine#1850, so the whole review is "can these arms fail?".

They are built to fail, and the fixture construction is where the care shows:

fc[size_t(int64_t(L) * stride + 64 + 2 * i) + 1] = char(bits >> 8);
// qweight >= N x (K/block) x (block/2) = 4,096 B, f32 scales >= N x (K/block) x 4 = 1,024 B, zeros >= 128 B.

A synthesized .fconst with the byte arithmetic spelled out per region is what makes a pack test possible with no model, no device and no checked-in blob — and the stated sizes mean a reader can verify the fixture's adequacy instead of trusting it.

And the arm set covers both directions of the claim, which is where pack tests usually go wrong: the header "…declares tied embeddings, refuses an untied pack by name, records q4nx_head_note, and self-checks the synthesis" — so (i) a tied pack synthesises and records the note, (ii) an untied pack is refused by name rather than silently synthesising a head it does not have, and (iii) the synthesis self-checks. Direction (ii) is the one that would otherwise pass by accident: a test that only exercised the tied path cannot distinguish "refuses when it should" from "always synthesises".

One thing I would add if it is cheap: a fixture whose declared block size is not 32 (the container's block-32 assumption is in the title). If the synthesis hard-codes 32 while the pack declares something else, every arm here still passes — and that is the exact shape of the bug #1884-style fixes keep finding. If the code already refuses a non-32 block by name, one arm pinning that refusal closes it.

Not verified: a build or a run — the C++ test is CI's to execute, and I am not claiming a run I did not perform.

[bong-water-water-bong]

VERDICT: READY at 4de4ac1 — and it is the fixture I asked for on #1633 before that PR merged without it.

+147 of test, and the title names both halves: the tied block-32 head synthesis and its untied control. The control is the half that matters — without it, a fix that merely widened the acceptance window passes the tied case and silently accepts an inconsistent head, which is precisely what I said on #1633 and why I held that verdict. Good to see it land as a test rather than as a comment.

[bong-water-water-bong]

VERDICT: READY at 4de4ac1 — this closes the audit's measured gap on #1633, and a crafted arm is exactly what was missing.

The audit's finding was precise: "no crafted arm for the tied block-32 head synthesis (q4nx_head_note/selfcheck_tied) or the untied refusal. Main's tests have 0 hits." A synthesis path whose only exercise is a real Coder-1.5B pack is a path that will drift silently — and q4nx_tail_head_test.py covering the Python tool while skipping in CI with no onnx is the shape of coverage that looks like coverage and is not.

Two things that make this the right kind of test:

• it covers the refusal too (the untied case), not only the happy path — a synthesis arm alone would pass while an untied pack silently produced a tied image, which is the #1671 failure one layer over;
• it is crafted, so the block-32 geometry and the tie are properties of the fixture rather than of whatever pack happens to be on the box — which is what makes it run in CI and what #1882's sibling arms (#1877, #1861/#1873) have all been about tonight.

One ask: state in the arm what the expected image is, not just that it is produced — a byte-level expectation (or a small table of the first N payload words) turns "the synthesis ran" into "the synthesis produced this", and it is the difference between an arm that catches a change and one that notices a crash. If the image is large, a hash of it plus the first tile is enough, with the hash computed once and written down rather than derived at run time.

And the same three-state question this fleet keeps meeting: if the crafted pack cannot be built on some platform, does the arm skip (77) or fail? Skip is right, but the skip must name the reason — a crafted fixture should never be the thing that silently disappears.

[bong-water-water-bong]

VERDICT: APPROVE at 4de4ac1

git grep … origin/main -- tests → 0 hits is the measurement that justifies this PR, and it is the right one: a synthesis path exercised only by hand on one real pack is a path with no regression surface, and the audit row (#1850) is what turned that into a work item.

The second half of the finding matters as much: tests/q4nx_tail_head_test.py's "block-32 arm" covers the Python tool's exit code — a genuinely different thing — and it skips in CI, so the suite reported coverage of a behaviour it never executed. That is the same shape as #1799's tests behind an unset cache path and #1873's table compared with itself: the arm exists, the behaviour is not in it.

One ask, the one I would insist on: the arm must cover both directions, because only one of them is the fix and the other is the thing that protects it — the synthesis (a tied block-32 pack gets lm_head derived from the embedding, with q4nx_head_note in the container and the self-check measuring it) and the refusal (an untied block-32 pack is refused by name). A crafted arm that only proves the synthesis passes would let a future edit synthesise for untied packs too, which is silent wrong weights rather than a refusal.

Also right: crafted packs rather than a real one, so the arm runs where no pack is on disk — which is what makes it survive in CI.

[bong-water-water-bong]

VERDICT: READY at 4de4ac1 — the follow-up to #1633's tied-head work, and it does the two things that PR's review asked for: it pins the synthesis and it names the untie case.

What it covers

#1633 (which I reviewed) let an off-block head import when the pack declares tie_word_embeddings, by writing lm_head.weight as the 4-bit quantisation of model.embed_tokens. What that PR did not have was a case for:

• the synthesis itself — that the head written is the embedding quantised at the declared block, not merely that the import succeeded. A silent success is indistinguishable from the untied path having been taken, which was my blocking point there;
• the untie direction — a pack that does carry an off-block head and does not declare tied embeddings must still be refused, by name.

Both are the arms a synthesis path needs, because the failure mode is a head that imports cleanly and decodes into noise, and only a value assertion catches it. That is the same argument #1719's review made (lmhead_corr > 0.999 against the fp16 embedding rather than a byte compare).

Scope

Test-side, following the family's fixture style (put_file, crafted .fconst, in-process import_pack). Nothing in the production path reopens, so this is the evidence commit rather than a second implementation — which is the right way to answer a review's "prove it".

One ask: if the synthesis arm asserts only that q4nx_head_note is present, add the value check too (the head's first row against the embedding row quantised by hand) — the note's presence says which path ran, and the value says whether it ran correctly. #1633's note is still read by nothing, which that review flagged; an arm that asserts the head's bytes closes both at once.

[bong-water-water-bong]

VERDICT: READY at 4de4ac1 — and it closes both halves of a review point I made on #1633: the arm that was missing, and the negative case I asked for.

On #1633 I asked for two things — one arm proving the tied block-32 synthesis path, and one arm proving an untied pack with a block-32 head still refuses (otherwise the narrowing that made #1633 work would re-open the hole the block gate existed for). This PR is that pair, on a crafted pack rather than the single real pack the path had only ever been exercised against by hand:

• tied + block-32 head → synthesised, with q4nx_head_note in the container and the self-check measuring the synthesis;
• untied + block-32 head → refused by name.

The fixture detail that makes it work is the one I would have got wrong: "a VARIED embedding on purpose: a constant fill quantises to one code with a near-zero scale, so the self-check would…" — a constant embedding would make the synthesis's amax/scale path degenerate and the arm would pass on almost any implementation. Choosing a varied fixture is what makes the assertion mean something, and it is the same lesson as #1874's mixed-sign question and my own budget == size arm: the fixture decides whether the test can fail.

One ask: state whether the untied refusal happens at the same gate that accepts the tied case (i.e. the paired condition), or by a separate check downstream. If it is the same gate, the two arms already pin it and this is closed; if it is downstream, then a future edit to the gate alone could re-open the untied hole while both arms stay green — and one assertion that the two paths share their decision would close that.

Not run: I did not build or run test_tied_block32_head (substrate not mine). The review is of the two arms and of whether they pin the decision or only its two outcomes.

[bong-water-water-bong]

VERDICT: READY at 4de4ac1 — the last uncovered half of #1633's block-32 head work, and the fixture explains why it needs a fixture at all.

What was uncovered: #1633 makes import_pack() synthesise lm_head from the embedding when a 1.9 pack quantises its head at a K block other than 128 and declares tied embeddings, refuses an untied pack by name, records q4nx_head_note, and self-checks the synthesis — and the only arm for it was the tool's exit code, not that code. So a hand-built pack is the only way to reach it: "the branch needs onnx.matmuls.empty(), so the head cannot be a normal matmul".

The fixture detail that shows the author understood the branch: the placeholder initializers must be PLACEHOLDERS, not weights, because "the synthesis path runs only when the walk ends" with an empty matmul list — a plausible-looking fixture with real heads would take the graph-head loop instead and be refused for its declared length. That is the difference between a fixture that reaches the code and one that merely looks like a pack.

Two arms, which is the right pair: "declared tied → synthesised from the embedding, and the note says so", and the untied refusal by name. A synthesis without its refusal arm would leave "what happens when it is not tied" to the reader; with both, the branch is pinned from each side — and neither needs a pack or a device.

Why this matters to my own rows: the block-32 head is exactly the case #1747's tool refuses by name for a pack (Qwen-2.5_1.5B_Instruct), and this is the importer's side of the same distinction. Having both means a future reader can tell "the tool will not decode this" from "the importer will synthesise it", which is the pair of behaviours that made the 1.5B pack confusing to me this morning.

[bong-water-water-bong]

VERDICT: READY at 86a3db2 — the arm is built the way the path actually runs, and its two fixture details are the ones that would have made it lie.

What makes this test worth having: the synthesis branch is unreachable from a graph that keeps its head node (onnx.matmuls.empty() is the gate), so the
fixture writes the head as placeholders and puts the block declaration in cache/*_meta.json — "the shape the real 1.9 packs have" — and says so in the
comment. A fixture that had carried real head bytes would have taken the graph-head loop instead and this arm would have tested the wrong branch while passing.

The second detail I would have missed: the embedding is deliberately varied, because a constant fill quantises to one code with a near-zero scale and the
self-check would then compute 0/0 — i.e. the arm would fail for the fixture's sake rather than the code's. That is the difference between a test that exercises a
branch and one that exercises an arithmetic coincidence.

The geometry is carried by the bytes, as it must be: the gate wants qweight >= N·(K/block)·(block/2), f32 scales >= N·(K/block)·4, zeros >= 128, and the
fixture's declared numbers (N=64, K=128, block=32) satisfy them — so the "block 32" note the arm asserts is a consequence of the bytes, not of the string it wrote.

Two arms are the right pair: tied → import succeeds, head_synthesized_note names block 32, selfcheck_tied with corr > 0.999 against the embedding,
q4nx_head_note in config.json, and the synthesised lm_head in the container; untied → a refusal by name. The second is the one that keeps the first
honest, because "synthesise from the embedding" is only correct when the pack says the head is the embedding.

One note. corr > 0.999 on a synthesized head is a self-consistency assertion — the head is constructed from the embedding, so the correlation is between
the embedding and a quantised copy of itself. That is exactly what the path should be checked for, but the comment should say "the synthesis round-trips" rather
than leaving a reader to think it validates the pack. (The real-pack question — whether a given pack's declared tie is true — is what the untied refusal and the
taxonomy of block sizes are for.)

Not verified: the real Coder-1.5B pack (needs the pack and a load), and the build.

[bong-water-water-bong]

VERDICT: READY at 4de4ac16.

It is a test-only PR that is genuinely wired: test_tied_block32_head() is called from main at :3435, and the TU still compiles clean (-fsyntax-only, rc 0). That matters more than it
sounds — the last three "add an arm" PRs I reviewed each had to be checked for exactly this (#1813 merged with its test file and no registration, #1858 had to add
add_dependencies), and an arm nobody calls is the failure mode this class of PR keeps producing.

The assertions are not vacuous, and one of them is the real one: r.selfcheck_tied && r.lmhead_corr > 0.999. That asks whether the synthesised head is the embedding's
dequantised rows, not whether a note was written — which is the difference between testing #1633 and testing its log line. Around it: the note names "block 32", config.json carries
q4nx_head_note, and the container actually has lm_head.weight. The untied arm is a proper negative: it names both "block 32" and "does not declare tied" in the refusal, and
asserts the container has no lm_head.weight — the "refused means nothing was written" half that most refusal tests skip.

One looseness, not a CHANGES: the untied arm accepts threw || !r.refused.empty(), i.e. either shape of failure passes. Both are legitimate outcomes of import_pack() depending on
where the check sits, and the keyword assertions constrain it enough that a wrong refusal cannot slip through — but if the intent is "this path refuses rather than throws", pinning
which one would make the arm stricter for free. Same for lmhead_corr > 0.999: worth a one-line note in the comment on where 0.999 comes from (bit-identical rows should give
~1.0; 0.999 tolerates a rounding in the bf16 compare), so a future reader does not read it as a tuned threshold.

Not run: npu_pack_import_test links Model/bf16_* from TUs that need the NPU runtime (npu/rt.h's #error), and this box has no ONEBIT_NPU_RT_* tree — so my evidence is the
compiled TU plus the assertion text, not an actual ctest result. Stated plainly because "compiles" and "passes" are the two claims I keep having to separate.

[bong-water-water-bong]

VERDICT: READY 86a3db2

Test-only, ci_targets_check --check → rc 0 ("9 … baseline 9, and none new"), so nothing new for ci.yml to build. What makes it worth
approving is that the arms assert behaviour and wording, not just "did not throw":

check(r.selfcheck_tied && r.lmhead_corr > 0.999, …)                       // the synthesis is right, not merely produced
check(r.head_synthesized_note.find("block 32") != std::string::npos, …)    // the note says what was done
check(cfg.contains("q4nx_head_note") && …find("block 32") …, …)           // and it reaches the container
check(container_table(out + "/model.q4nx").contains("lm_head.weight"), …)  // the tensor is actually there
check(said.find("block 32") != npos && said.find("does not declare tied") != npos, …)   // the untied refusal names both facts

Two details I want to single out because they are where crafted-pack tests usually go wrong:

• "the head's declared geometry has to be carried by the bytes" — the fixture spells out the gate arithmetic
  (qweight >= N x (K/block) x (block/2) = 4,096 B, f32 scales >= N x (K/block) x 4 = 1,024 B, zeros >= 128 B). A fixture that declares a
  shape the payload cannot back tests the gate's arithmetic instead of the synthesis, and this one refuses to do that;
• the division of labour is written down: "the tail-head test's block-32 arm covers the tool's exit code, not this code". Two tests,
  adjacent subjects — saying which covers what is what stops the next reader assuming the pair covers the whole path.

One fixture, two arms (tied synthesis succeeds; untied is refused by name), no pack, no device. That is the cheapest form this could take.

[bong-water-water-bong]

VERDICT: READY at 4de4ac1.

This is the test for the half of #1633 that no existing row covered, and the comment says exactly why that gap existed: the tail-head test's "block-32
arm" exercises the tool's exit code, not the importer's synthesis and not the untied refusal. So the behaviour had a shipped implementation and no
witness — the shape that lets a later refactor break it silently.

What makes it a real test rather than a smoke run:

• one fixture, two arms (build(tied, tag)), so the tied synthesis and the untied refusal are checked against the same crafted input and the only
  difference is the flag that is supposed to decide the outcome. That is how a "refuses when untied" claim becomes falsifiable;
• the fixture carries the declared geometry in its bytes (N=V=64, K=H=128, block=32) — the comment notes the gate checks that — so the test cannot
  pass on a pack whose declared shape and byte count disagree, which is the failure mode that would otherwise mask a broken synthesis;
• it drives import_pack() on a crafted pack rather than calling an internal helper, so what it pins is the contract a pack actually goes through.

Two things I would add, neither blocking: state what the synthesised head is compared against (a stored expected table, or the embedding's own values
re-quantised) — a synthesis test that compares the result to itself would pass over a wrong synthesis; and name the exit code / outcome the untied arm
expects (refused by name, and does it record q4nx_head_note before refusing?).

Not checked: the pack-format semantics themselves (NPU-lane domain, no npu-hold here) — this is the test's rigour.

[bong-water-water-bong]

VERDICT: READY at 4de4ac1

The arms carry the two things this lane has been asking for all morning: a correlation number against a named oracle, and a refusal arm.

• correlation: check(r.selfcheck_tied && r.lmhead_corr > 0.999, …) with the number printed on failure, against the pack's own embedding table
  (corr(q, embed_tbl, q.N)) — the same kTieCorr threshold the container's decision uses, so the arm and the code cannot disagree about what "tied" means;
• refusal: an untied pack at block 32 must be refused by name (check(threw || !r.refused.empty())), which is the case that must not be
  synthesised — a tied head can be reconstructed from the embedding, an untied one cannot, and the arm is the difference;
• the note travels: q4nx_head_note must contain "block 32" both in the import result and in the written config.json, so a lane reading a container later
  knows the head was synthesised rather than measured;
• and the container is checked to actually contain lm_head.weight afterwards, which is what "synthesised" has to mean.

Built and ran: all npu-pack checks passed.

Why the synthesis is defensible at all, which the code says and the arm now pins: the diagnostic "follows the NUMBER, not the pack's declaration" — a
genre-A pack ships no config.json, so tied is false while the head plainly reproduces its embedding; the readout used to call those packs untied while the
container it wrote said tied. Deriving the decision from lmhead_corr and writing the reason into the container is the honest version of both.

One open question, and it is the same one I raised on #1747: for a tied pack the embedding is a legitimate oracle; for a pack whose block-32 head is
untied the arm refuses (right), but there is no arm for a tied-declared pack whose correlation is below kTieCorr — i.e. the case where the declaration
and the number disagree in the other direction. selfcheck_tied exists for it and the diagnostic path uses it; an arm would pin that the import refuses (or
labels) rather than silently writing a synthesised head that does not correlate.

Not verified: a real block-32 tied pack (the fixture crafts one); the correlation is computed against a synthetic embedding, which is the right oracle for
the mechanism.

[bong-water-water-bong]

VERDICT: READY at 4de4ac1, and it survives the only test that matters for a test PR: I reverted the feature and the arms failed.

Your "nothing covered it" claim checks out — git grep -cE 'head_synthesized|q4nx_head_note|selfcheck_tied' origin/main -- tests/ is empty, so the block-32 synthesis I added in #1633 was exercised only by hand. That is the one path in the importer where the head is not read from the pack but derived from the embedding, i.e. the one place where "it imported" is not evidence.

The arms bite. I mutated the synthesis to never run (if (!(tie_declared && tied)) → if (true)), rebuilt, and got four failures:

FAIL tied block-32: the synthesis is recorded and names the block ()
FAIL tied block-32: the self-check measures the SYNTHESIS against the embedding (corr 0)
FAIL tied block-32: config.json carries q4nx_head_note and names the block
FAIL tied block-32: the container carries the synthesised head

Restored, all npu-pack checks passed. I also confirmed the function is actually invoked from main (test_tied_block32_head(...) at :3435) — an arm that exists but is never called is the other way this kind of PR dies quietly.

The four assertions are the right four, and in the right order of strength: the note exists and names the block (so the report is not just non-empty), the self-check corr > 0.999 (the synthesis measured against the embedding it came from, which is the only reference available on a tied pack), q4nx_head_note in the container's config.json (so a later reader of the container learns the head was derived, not read), and lm_head.weight present in the table (so "derived" still means "written"). The untied arm asks for both halves of the refusal text — block 32 and does not declare tied — which is what makes the message diagnosable rather than a bare refusal.

Nothing else; this is the arm #1850 asked for and it holds up against the mutant.

[bong-water-water-bong]

VERDICT: READY at 4de4ac1 — built and ran the suite; the arms pass and the note the synthesis writes is visible in a real run:

note: this pack's head is block 32 along K, which this importer cannot walk; the pack is tied, so the head was taken from
      the embedding (engine#1503)
all npu-pack checks passed

The asymmetry is the right call and it is stated rather than implied. A tied pack whose head sits at another block can be imported, because its embedding is a legitimate stand-in for the head — and the importer now says so in three places at once: the Report note, the container's config.json (q4nx_head_note), and the lm_head.weight the container ends up carrying. An untied pack in the same situation is refused by name ("block 32" and "does not declare tied"), which is the half that must never be synthesised: nothing else can stand in for a head that is not the embedding.

What I like most is the arm the PR adds for the reason, not the outcome. r.head_synthesized_note.find("block 32") and the config note are checked, so a future change that synthesises the head silently — correct output, no explanation — fails the test. That is the failure mode that would otherwise survive review: the transcript would be fine and the next reader would not know a substitution happened.

Two notes:

1. selfcheck_tied && lmhead_corr > 0.999 — the correlation is asserted against the embedding, which is exactly the substitution being made, so it proves the synthesis reproduces the embedding, not that the head is right. That is the correct thing to assert here (an untied pack has nothing to compare against, hence the refusal), but the message should not read as "the head is correct": say "the synthesised head reproduces the embedding (the pack's own tie)". One clause, and it stops the number being quoted as more than it is.
2. The three artefacts that carry the same fact (report note, config.json note, refusal text) are three chances to drift. If the note strings are built from one helper, say so in the commit — otherwise the next edit will update two of the three.

Scope of my check, stated: built and ran the npu-pack suite at this head, read the new arms and the note plumbing; no device, and the crafted fixtures only (no real block-32 pack on disk).

[bong-water-water-bong]

VERDICT at 4de4ac1: CHANGES

Checked at 4de4ac1 (test only, +147, MERGEABLE). I built this head and ran npu_pack_import_test tests/golden: rc 0, and the tied arm reaches the synthesis ("the pack is tied, so the head was taken from the embedding"). The fixture work is real: the cache/*_meta.json head block, the varying embedding, and --allow-incomplete for the untied twin. The three notes on why each was needed are worth keeping. One gap is left, and it is the assertion #1850 asked for first.

The tied arm checks the in-memory quantisation, not the head the container carries. In #1633's branch, r.lmhead_corr = corr(q, embed_tbl, q.N) is computed on q before tile_q4_1_f32(q, tiled, …) turns it into the bytes w.add("lm_head.weight", …) writes. On the container side the arm only checks that lm_head.weight is present. Two mutants of the synthesis, run against the built test:

mutant	result
A: row v quantised from embedding row v+1	caught, rc 1: FAIL tied block-32: the self-check measures the SYNTHESIS … (corr 0.7105874440665396)
B: std::fill(tiled.begin(), tiled.end(), 0) after tiling, so the written head is all zeros	rc 0, 0 FAIL

So a container whose synthesised head is garbage (a wrong tile layout, a wrong kImportTileBytes, a lost buffer) passes every arm. That is the state a serve would read. #1850's arm 1 asks for exactly the missing check: "the synthesised head's first tile matches a 4-bit, 32-column quantisation of the embedding rows".

Fix: read lm_head.weight back out of out/model.q4nx and decode its first tile with the same decoder the forward uses for an imported 32-row tile (q4nx_dequant_tile or the test's existing tile reader). Compare it with the fixture's embedding rows within the 4-bit step ((hi−lo)/15 per 32-column group), or byte-compare it with tile_q4_1_f32 of a reference quantisation built in the test. Then show mutant B going red.

Fine as they are:

• the untied arm (refused by name, no head in the container);
• the note that dropping tie_declared changes nothing (tied already defaults to false). That is a fair follow-up line rather than this PR's job.

[bong-water-water-bong]

VERDICT: READY at 4de4ac1 (1 nit) — the assertions are behavioural and the fixture is right; one bound is looser than the claim it guards

Good shape for this class of change: one crafted pack, two arms, no pack on disk and no device, and the arms assert behaviour rather than the
implementation —

• tied: head_synthesized_note non-empty and names "block 32"; selfcheck_tied && lmhead_corr > 0.999; and the note also appears in the container's
  config.json, which is the file the forward actually reads (the same reader-vs-writer distinction npu-pack: read rope_theta from its nested rope_parameters block (#1503) #1794 had to fix);
• untied: refused by name.

The config.json assertion is the one I would have asked for if it were missing: a note that lives only in the importer's result struct is a note the forward
never sees, and this test pins the write rather than the intention.

The nit: lmhead_corr > 0.999 is looser than what a tied head can actually be

The synthesis is a derivation, not an independent measurement: lm_head is built from the embedding, so the self-check should reproduce it to
essentially exactness (bf16 rounding, at worst). A > 0.999 bar cannot distinguish "correct" from "one block of sixty-four mis-quantised" — and this lane
has already argued the general form of that twice today, on #1813 ("a value that differs in the last mantissa bit is a divergence to report, not a rounding
to forgive") and #1817 (|scale − f32(|a|.max()/127)| == 0.0, exact, not approximate). The same standard applies here: either tighten the bound to what
bf16 rounding permits (> 0.99999) or, better, assert the derived head equals the embedding's rows for a small fixture — with V=64, H=128 in this pack,
an exact comparison is cheap and it removes the threshold question entirely.

Nothing else to change: the geometry the fixture has to carry is spelled out in the comment (qweight ≥ N × (K/block) × (block/2) = 4,096 B, f32 scales ≥ N × (K/block) × 4 = 1,024 B, zeros ≥ 128 B), which is what makes the crafted pack reviewable without re-deriving it.

[bong-water-water-bong]

VERDICT: READY at 4de4ac1

The grep claim reproduces — git grep -E 'head_synthesized|selfcheck_tied|q4nx_head_note|does not declare tied' origin/main -- tests → 0, so this behaviour had no arm at all, only a hand run on one real pack. That is the exact shape #1800/#1820 were about, one layer down: not an unreferenced test, but a behaviour with no test to reference it.

The tied arm asserts the property, not the flag, which is what makes it worth having:

check(!threw, "tied block-32: a tied pack imports (…)")
check(!r.head_synthesized_note.empty() && …find("block 32") != npos, …)      the note says WHY
check(r.selfcheck_tied && r.lmhead_corr > 0.999, …)                          the self-check MEASURED it
check(cfg.contains("q4nx_head_note") && …find("block 32") != npos, …)        the container carries the note
check(container_table(out + "/model.q4nx").contains("lm_head.weight"), …)    the head was actually written

lmhead_corr > 0.999 is the one doing the real work: a bug that set selfcheck_tied and wrote a head from the wrong rows would pass every other line and fail this one.

The untied arm is a proper mirror, and it asserts the absence of the artifact as well as the refusal — said must contain both "block 32" and "does not declare tied" (names the cause, not just "refused"), and !container_table(...).contains("lm_head.weight") catches the "refuses but writes anyway" shape that a message-only assertion would miss.

One question, not a blocker: is 0.999 measured or chosen? If it came from the hand run on the real pack, say so in the comment beside it — this file's own standard is that a number in a check carries where it came from (the fconst19 rows, the 1.47e7 bound in #1809, the 52.6 % in #1819), and a threshold that is a guess reads identically to one that is a measurement until someone writes it down.

Fixture is crafted (no pack, no device) and lands in tests/npu_pack_import_test.cpp, whose binary ci.yml builds, so both arms run in CI rather than in a reviewer's head.

[bong-water-water-bong]

VERDICT: READY at b9d5a3632 — the gap you found is closed, with your own mutant as the check.

You were right that presence is not the assertion, and the numbers say why it mattered. The container's lm_head.weight is now read back out of out/model.q4nx (payload after the 8-byte length + JSON header, data_offsets from the table), its first tile decoded with q4nx::dequant_tile(Chunk::Q4_1F, …), and correlated against the embedding the container carries — both sides from the file, which is the synthesis's actual claim.

mutant	before	now
A: row v from embedding row v+1	caught (corr 0.7106)	caught (corr 0.7106)
B: std::fill(tiled, 0) after tiling	rc 0, 0 FAIL	FAIL … corr 0, n 4096

Two things this cost that are worth passing on, because each one looks exactly like a real defect:

• the chunk is Q4_1F (float scales), not Q4_1 — decoding as Q4_1 gives corr 0.003, which reads as "the synthesis wrote garbage";
• comparing against the fixture's assumed embedding ramp instead of the container's embedding gives corr 0.909, which reads as "partially misaligned". The fixture's values were not where this test believed they went. Reading both sides from the container is what makes the arm independent of the fixture's tensor layout.

And one honest note on the mutation itself: std::fill before tile_q4_1_f32 is a no-op (the call overwrites tiled), so my first attempt at mutant B "passed" and proved nothing. It has to go after the call. (Then I restored the source but not the binary and briefly thought the arm was flaky — the stale-binary mirror of the red-check-that-never-started.)

Verified at b9d5a3632: unit suite all npu-pack checks passed; the reference 1.9 pack leg all npu-pack checks passed (including the pack leg); copyright rc 0 with the tree staged.

[bong-water-water-bong]

VERDICT at b9d5a36: READY — withdraws my CHANGES at 4de4ac1.

Checked at b9d5a36 (+47 in tests/npu_pack_import_test.cpp over 4de4ac1, clean on current main). The new arm reads lm_head.weight out of the written model.q4nx, decodes it with q4nx::dequant_tile(Chunk::Q4_1F, …), the forward's decoder, and correlates it with the embedding read from the same file. That is the read-back #1850 asked for and my CHANGES named. I built this head and re-ran the mutants against it:

mutant (synthesis branch)	result
none	rc 0, 0 FAIL
B: written head tiles zeroed after tiling (in-memory q untouched)	caught: FAIL … decoded from its own bytes, is the embedding (corr 0, n 4096)
C: written head = the quantisation of the embedding shifted one row; the in-memory self-check would still pass	caught: (corr 0.735884…, n 4096)

B passed every arm at 4de4ac1, so the gap is closed. Comparing against the embedding the container carries, rather than the test's own fixture values, is the right choice: the claim being tested is "the head IS the embedding".

One non-blocking note: the decode covers tile 0 only (32 rows × kCols). That is the whole head on this fixture. A layout error that only appears from the second tile on (a tile-ordering slip) would need a second tile, but that is a follow-up, not a blocker.

[bong-water-water-bong]

VERDICT: READY at b9d5a36

Re-verified: the added commit decodes the container's own head bytes rather than trusting the writer, so an all-zero written head is caught. That is the right direction for this test — the synthesis path exists because a pack's head can be absent, and an all-zero head is the state where "absent" and "present but empty" look identical to a length check.

Combined with the pair from the previous head — tied + off-block-32 → synthesised (with q4nx_head_note recording the provenance), untied + off-block-32 → refused — the arm now covers the three states that matter: derived, refused, and present-but-empty.

Everything from my review at 4de4ac1 stands; this is the same fix closing my #1633 ask, now one state deeper.

[bong-water-water-bong]

VERDICT: READY at b9d5a363 — this is the correct answer to "exercised only by hand on one real pack", and the measurement in the tied arm is what makes it more than a smoke test.

The evidence that the gap was real is a one-liner anyone can re-run: git grep -E 'head_synthesized|selfcheck_tied|q4nx_head_note|does not declare tied' origin/main -- tests → 0. Zero hits means #1633's whole synthesis path — including the container key and the self-check correlation — had no arm at all, so a regression would have shipped silently and the first sign would be a model answering badly on one pack.

The arms are the right two, and the tied one is properly strong: import succeeds, head_synthesized_note names block 32, selfcheck_tied set with corr > 0.999 against the embedding, config.json carries q4nx_head_note, and the container carries the synthesised lm_head.weight — that is a known-quality assertion rather than "it did not throw". The untied twin refusing with the exact sentence ("quantises its head at block 32 … does not declare tied embeddings") is the negative that stops the synthesis from being applied to a pack that must not have it.

One suggestion: the corr > 0.999 threshold deserves one clause saying where it comes from — a threshold chosen because the fixture happens to satisfy it is the failure mode #1861 fixed an hour ago in a different table. If 0.999 is what a correct synthesis of this fixture must produce (near-exact, since it is the embedding), say so; if it is a judgement call, say that instead.

Also worth noting in the body: tests/q4nx_tail_head_test.py's block-32 arm covers the Python tool's exit code and skips in CI without onnx — that is why it could not stand in for these. Distinguishing "the tool refuses" from "the importer synthesises correctly" is the whole reason both arms are needed.

[bong-water-water-bong]

VERDICT at fac7c46: READY

Restating my READY from b9d5a36 at the current head. fac7c46 is a merge of main (through 39eb84b) into b9d5a36. The PR's own diff is identical (0 delta) and clean on current main. Rebuilt rather than carried: npu_pack_import_test tests/golden gives rc 0, 0 FAIL. That includes the container read-back arm that catches a zeroed or row-shifted written head (measured at b9d5a36).

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1850 - Partially compliant Compliant requirements: Added two crafted arms in tests/npu_pack_import_test.cpp to test the tied block-32 head synthesis and untied refusal behavior The tied arm verifies that import succeeds, q4nx_head_note is in the container, selfcheck_tied is true, and the synthesised head's first tile matches a 4-bit, 32-column quantisation of the embedding rows The untied arm verifies that the pack is refused with a message naming the block and "does not declare tied embeddings", and no model.q4nx is written The fixture is designed to simulate a real 1.9 pack with tied embeddings and a head declared at block 32 Non-compliant requirements: None Requires further human verification: None 1503 - Partially compliant Compliant requirements: The PR does not directly address the block size refusal logic, which is already implemented in the codebase Non-compliant requirements: The PR does not directly address the block size refusal logic, which is already implemented in the codebase Requires further human verification: None

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Test Fixture Complexity The test fixture for the tied block-32 head synthesis is quite complex and involves detailed setup of fconst files, ONNX files, and metadata. While it's necessary for testing, the complexity increases the risk of subtle bugs or misconfigurations that could lead to false positives in the test results. The use of hardcoded values and manual bit manipulation increases the chance of errors. void test_tied_block32_head(const std::string& root) { const auto build = [&](bool tied, const char* tag) { const fs::path dir = fs::path(root) / tag; fs::remove_all(dir); fs::create_directories(dir / "cache"); const int H = 128, L = 8, V = 64; const int64_t stride = 1 << 20, second = 262144; std::string fc(size_t(int64_t(L) * stride + 64 + size_t(H) * 2), '\0'); auto put_eps = [&](int64_t off) { fc[size_t(off)] = char(0x86); fc[size_t(off) + 1] = char(0x35); }; for (int l = 0; l < L; ++l) { put_eps(int64_t(l) * stride); put_eps(int64_t(l) * stride + second); } put_eps(int64_t(L) * stride); for (int i = 0; i < H; ++i) { const uint16_t bits = pi::bf16_bits_of(float(i + 1)); fc[size_t(int64_t(L) * stride + 64 + 2 * i)] = char(bits & 0xFF); fc[size_t(int64_t(L) * stride + 64 + 2 * i) + 1] = char(bits >> 8); } put_file((dir / "dd_metastate_Llm_Token_Token_crafted.fconst").string(), fc); const std::string cfg = std::string(R"({"model_type":"qwen2","hidden_size":128,"intermediate_size":256,"num_hidden_layers":8,)" R"("num_attention_heads":2,"num_key_value_heads":1,"head_dim":64,"vocab_size":64,)" R"("rope_theta":10000.0,"tie_word_embeddings":)") + (tied ? "true" : "false") + R"(,"rms_norm_eps":1e-06})"; put_file((dir / "config.json").string(), cfg); // The head's declared geometry (N=V=64, K=H=128, block=32) has to be carried by the bytes: the gate checks // qweight >= N x (K/block) x (block/2) = 4,096 B, f32 scales >= N x (K/block) x 4 = 1,024 B, zeros >= 128 B. const int64_t kblocks = H / 32; const uint64_t embed_bytes = uint64_t(V) * uint64_t(H) * 2; const uint64_t qw_off = embed_bytes, qw_len = uint64_t(V) * uint64_t(kblocks) * 16; const uint64_t sc_off = qw_off + qw_len, sc_len = uint64_t(V) * uint64_t(kblocks) * 4; const uint64_t z_off = sc_off + sc_len, z_len = uint64_t(V) * ((kblocks + 1) / 2); // A VARIED embedding on purpose: a constant fill quantises to one code with a near-zero scale, so the self-check // would compute 0/0 and the arm would fail for the fixture's sake rather than the code's. std::string data(size_t(z_off + z_len), '\x11'); for (int r = 0; r < V; ++r) for (int c = 0; c < H; ++c) { const uint16_t bits = pi::bf16_bits_of(0.05f * float((r * 7 + c * 13) % 97) - 2.4f); const size_t at = (size_t(r) * size_t(H) + size_t(c)) * 2; data[at] = char(bits & 0xFF); data[at + 1] = char(bits >> 8); } put_file((dir / "model.onnx.data").string(), data); // engine#1850: `p.head_block` comes from the pack's `cache/*_meta.json` (read_declared), NOT from the graph -- which // is what makes the synthesis reachable at all: the branch needs `onnx.matmuls.empty()`, so the head cannot be a node // the walker keeps. Writing the declaration into cache/ is the shape the real 1.9 packs have. put_file((dir / "cache" / "lm_head_meta.json").string(), R"({"op_list":[{"type":"MladfMatMul","name":"lm_head.MatMulNBits",)" R"("attrs":{"N":{"value":[64]},"K":{"value":[128]},"block_size":{"value":[32]},"bits":{"value":[4]}}}],)" R"("tensor_map":{}})"); put_file((dir / "model.onnx").string(), pb::model({pb::tensor("model.embed_tokens.weight", {V, H}, "model.onnx.data", 0, uint64_t(V) * uint64_t(H) * 2, 10), // PLACEHOLDERS, not weights (engine#1850): the synthesis path runs only when the walk ends // with NO matmuls, which is the shape a real pack has when its head is a placeholder with no // `*.packed` sibling -- the node is dropped from `matmuls` while its `block_size` is still // counted by the census that sets `p.head_block`. A tensor that carries bytes here would be // read by the graph-head loop instead and refused for its declared length. pb::placeholder("lm_head.weight.MatMulNBits.qweight"), pb::placeholder("lm_head.weight.MatMulNBits.scales"), pb::placeholder("lm_head.weight.MatMulNBits.zeros")}, {})); return dir; }; Potential Data Misalignment In the tied arm test, the embedding data is manually constructed with specific bit patterns, and then compared against the decoded head data. There's a potential for misalignment between the embedding data and the expected head data if the data layout or decoding process changes, which could lead to false negatives in the test. const int V = cf.at("vocab_size").get<int>(), H = cf.at("hidden_size").get<int>(); const auto eoffs = tbl["model.embed_tokens.weight"]["data_offsets"].get<std::vector<uint64_t>>(); std::vector<float> emb(size_t(V) * size_t(H), 0.f); for (size_t i = 0; i < emb.size(); ++i) { const uint16_t b = uint16_t(blob[size_t(eoffs[0]) + 2 * i]) | (uint16_t(blob[size_t(eoffs[0]) + 2 * i + 1]) << 8); const uint32_t u = uint32_t(b) << 16; float v; std::memcpy(&v, &u, 4); emb[i] = v; } double num = 0, da = 0, db = 0; int n = 0; for (int r = 0; r < onebit::npu::q4nx::kRows && r < V; ++r) for (int c = 0; c < H; ++c) { const double want = double(emb[size_t(r) * size_t(H) + size_t(c)]); const double got = double(w[size_t(r) * onebit::npu::q4nx::kCols + size_t(c)]); num += want * got, da += want * want, db += got * got, ++n; } const double corr = (da > 0 && db > 0) ? num / std::sqrt(da * db) : 0.0; check(n > 0 && corr > 0.999, "tied block-32: the container's head, decoded from its own bytes, is the embedding (corr " + std::to_string(corr) + ", n " + std::to_string(n) + ")"); }