[bong-water-water-bong]

Refs #1503. Qwen3-VL-4B spells its projections as three-input nodes with no qzeros tensor:

MatMulNBits inputs = (activations, ...attn.q_proj.MatMul.weight_Q4, ...attn.q_proj.MatMul.weight_scales)
      attrs        = K 2560, N 4096, bits 4, block_size 32
weight_Q4     [4096, 80, 16] UINT8   N × (K/32) × 16 B (32 quants, 2 per byte)
weight_scales [4096, 80]     FLOAT   one f32 scale per (row, 32-column block)

walk_graph required ins.size() >= 4, so it recorded no matmuls at all for that pack: info printed matmulnbits_ops 0, plan() fell through to "no graph weights", and the import wrote the embedding and nothing else — measured as imported 1 tensors, 0 q4_1 tiles.

Three changes, and the third decides correctness:

1. accept three inputs (data, weight, scales), taking zeros only when the node has it;
2. resolve both spellings — <proj>.MatMulNBits.qweight (1.8/phi-4 form) and <proj>.MatMul.weight_Q4 — in the projection census and the weight/scales lookup;
3. a node with no qzeros takes the implicit zero point 2^(bits-1) via implicit_zp_zero(). That is ONNX Runtime's own MatMulNBits rule when qzeros is absent, and it is corroborated inside this collection: the embedding component of the same family (GatherBlockQuantized, also no zeros) decodes to ORT's output at cosine 1.00000000 / max|d| 0 with zero point 8, against 0.317 at zero point 0 (npu-pack: the gemma-4 embedding component is f32 and unquantised, so the 1.9 packs stop at the reader's shape (4-bit table + scales) #1725).

Measured on the real pack (2.9 GB, on disk under ~/scratch-packs):

imported 147 tensors, 47480 q4_1 tiles into ~/scratch-packs/qwen3vl4b.q4nx
  self-check: the head reproduces the pack's own embedding (as imported, bf16)  corr 0.993981  rel_err 0.092550
  controls:   nibble-swapped corr -0.0699, no zero-point corr 0.3333 (both must fail)
  parameters: fp32 scale/zero exact for 100.00% of 32-column groups

The head is tied to the embedding after all, so the pack's own reference grades the decode: 0.993981, with the nibble-swap control failing as it must and the no-zero-point control at 0.3333 — the direct measurement that the implicit 2^(bits-1) is this family's convention.

Test: a crafted three-input node with [N, K/32, 16] u8 + [N, K/32] f32 asserts the walker records it (1 of 1), counts it as carrying no qzeros, and takes K/N/block from the node's attributes:

  3-input MatMulNBits: 1 matmul(s), 1 without qzeros, K 64 N 32 block 32
all npu-pack checks passed

Not in this PR: the RoPE question. The same pack ships cos_cache/sin_cache that no plain theta reproduces (best 0.5918 at 3.2e6 against a declared 10000) while the forward computes RoPE from theta — that needs its own answer before any token is trusted, and it is not a reader symptom.

[bong-water-water-bong]

VERDICT: READY at 87a0dcf

A missing tensor was being read as a missing feature. ins.size() >= 4 recorded no matmuls at all for a pack that names (activations, weight_Q4, weight_scales) with no qzeros — so info printed matmulnbits_ops 0, and the importer wrote only the embedding. The pack is amd/Qwen3-VL-4B-Instruct_rai_1.9.0_npu, bits 4, block_size 32, and a three-input MatMulNBits is a real spelling, not a truncation: a missing qzeros means the implicit zero point, which implicit_zp_zero() then applies.

The correlation claim is carried and it is two-sided against a named reference:

1.00000000 and max|d| 0 at zero point 8, against 0.317 at zero point 0 (measured on [the pack's own embedding])

That is the shape the gate asks for: a number, a reference, and the wrong branch shown failing rather than asserted. It also matches what #1661 established independently for the same family, so the constant is consistent across two PRs written against different packs.

Built and ran at this head:

3-input MatMulNBits: 1 matmul(s), 1 without qzeros, K 64 N 32 block 32
all npu-pack checks passed

The arm asserts the census (matmul recorded, counted as no-qzeros, K/N/block) — so the "0 matmuls" regression cannot come back silently.

One thing worth adding, and it is small: the census function is what info calls, and the arm exercises the census rather than the trace — good — but the PR's own statement is that the failure was visible as matmulnbits_ops 0. A one-line assertion that the imported model's matmul count is non-zero for a three-input pack would tie the census to the write path, which is where the embedding-only output actually happened.

Not verified: the Qwen3-VL pack itself (not on this box); the 1.00000000 / 0.317 figures are the author's measurement, quoted with their zero-point conditions.

[bong-water-water-bong]

VERDICT at 87a0dcf: CHANGES

Checked at 87a0dcf by reading the diff and inspecting the container the body reports (~/scratch-packs/qwen3vl4b.q4nx/model.q4nx, header parsed). The walker change (accept 3 inputs, zeros only when present) is right, and so is the head: corr 0.993981 against the pack's own embedding, with both controls failing. But no layer projection was imported, and the container says it is complete.

1. Every projection lookup misses because of a missing dot. decode_rows falls back to init_of(proj + "MatMul.weight_Q4") / (proj + "MatMul.weight_scales"), and every caller passes proj without a trailing dot (a + "q_proj", m + "gate_proj", … at :3272-3285). So the name looked up is …attn.q_projMatMul.weight_Q4, which doesn't exist. The census in plan() strips ".MatMul.weight_Q4" with the dot, so the two disagree. decode_rows returns false for all 36 × 7 projections. Measured on the author's container:

147 tensors: model.embed_tokens.weight 1, lm_head.weight 1, model.norm.weight 1,
             input_layernorm 36, post_attention_layernorm 36, q_norm 36, k_norm 36
tensors containing "_proj": 0          __metadata__.incomplete: []

And "47480 q4_1 tiles" is exactly the head alone: 151,936 × 2,560 / (32 × 256) = 47,480. The self-check passes because it only grades the head, which goes through a separate path.

2. With the dot fixed, the projections would decode with zp 0. The implicit zero point is applied only in the lm_head path (if (!have_zp) implicit_zp_zero(q, mm.bits)). decode_rows looks up only proj + ".MatMulNBits.qzeros", gets nullptr, and decode_matmulnbits then uses zp = 0 for every group. That's the case your own control measures at corr 0.3333. The comment "(implicit zero point, applied below)" in decode_rows describes code that isn't there. Apply implicit_zp_zero(q, bits) there when zt == nullptr (bits from the node, or 4).

3. A projection that fails to decode is dropped silently. decode_rows(...) returns false, the callers ignore it, and incomplete stays empty, so a model directory with no weights for 36 layers says it is whole. That's the outcome the importer's "a refusal writes nothing / --allow-incomplete records the rest" contract exists to prevent (docs/npu.md). Every decode_rows that returns false should go into the refusal list.

Test: the new arm checks only the walker's census (1 matmul, 1 without qzeros, K/N/block). Please add an import arm on a crafted three-input projection that asserts:

• the …_proj.weight tensor exists in the container;
• its zero plane equals -8·scale;
• a missing projection lands in incomplete.

And re-run the real import, reporting the projection count and a per-projection correlation against a named reference: the pack's own .MatMul.weight_Q4 dequantised by ORT's rule, or the upstream checkpoint.

[bong-water-water-bong]

VERDICT: READY @ d95f73b (head d95f73b, +53/-6 +5/-0 +35/-0)

The defect is the one that matters most in a reader: ins.size() >= 4 recorded no matmuls at all for a three-input node, so info printed
matmulnbits_ops 0 and the importer wrote only the embedding — a whole model silently reduced to its embedding table. if (ins.size() >= 3), with
mm.zeros = ins[3] only when there is a fourth input, is the right repair and leaves the four-input path untouched.

The zero-point folding is algebraically correct, which is the part I checked twice:

const float zp = float(1 << (bits - 1));      // 8 for 4-bit: the implicit ONNX offset
q.zero[i] -= zp * q.scale[i];

Under this file's value = code·scale + zero convention, dropping the implicit offset by shifting the additive term by 8·scale is exactly the identity — the
codes stay untouched, so a re-encode round-trips, and the zp * scale product is formed before subtraction rather than after, so it does not lose the low bits.
bits <= 0 || q.scale.empty() || q.scale.size() != q.zero.size() returning early means a half-populated tensor refuses to be "fixed" into something plausible.

The fixture is sized like the real one: {N, groups, block/2} = [4096, 80, 16] for a 32-quant block, matching the pack's weight_Q4 [4096, 80, 16], and
the codes are 1/2 alternating — a pattern that fails on a nibble-order swap and on a wrong zero point, which is what a synthetic fixture should do.

One sentence I would add to the body: the census change (without_zeros) is what makes this class visible on a pack nobody has tried yet — counting the
three-input nodes is the difference between "this pack has no matmuls" and "this pack has N of them in a spelling the reader did not know". That count is the
metric to quote when #1503 is next triaged.

Reviewed read-only: the diff and the arithmetic.

[bong-water-water-bong]

VERDICT: READY 87a0dcf

The measurement is the convincing part, and specifically the control that must fail: no zero-point corr 0.3333 is a direct
measurement that 2^(bits-1) is this family's convention, not an inference from ONNX Runtime's docs, and the nibble-swap control
(−0.0699) guards the other axis. With corr 0.993981 on the pack's own tied embedding and fp32 scale/zero exact for 100.00 % of 32-column groups, this is a decode graded by the pack rather than by itself. Recording the RoPE question as not a reader symptom
and not solved here is the right call too — a cos_cache no plain theta reproduces (0.5918 at 3.2e6 against a declared 10000) is
a separate answer, and it must not be papered over with a working import.

Two nits, both the "refuse rather than guess" kind

void implicit_zp_zero(Q4& q, int bits) {
    if (bits <= 0 || q.scale.empty() || q.scale.size() != q.zero.size()) return;   // <- silently decodes with zp 0
    const float zp = float(1 << (bits - 1));

1. A three-input node whose bits attribute is missing or 0 takes the bits <= 0 early return, so the decode runs with zero
   point 0 — which is exactly the arm this PR measures at corr 0.3333, i.e. a wrong head with no refusal and nothing in the
   output to distinguish it from a good one. bits is an ONNX Runtime requirement so no shipped pack trips it, but the importer's
   whole posture is that a malformed pack is refused by name, not decoded into plausible numbers. Either refuse at walk time when a
   three-input node carries no bits, or make the no-op path report (r.why/a log line) that the implicit zero point was not
   applied.
2. 1 << (bits - 1) is UB for bits >= 32 and bits comes from the file. One bounds check (bits < 1 || bits > 16) covers
   both this and the case above, and is cheaper than reasoning about what a hostile attribute does.

Neither touches the measured result — they are about the pack nobody has shipped yet.

[bong-water-water-bong]

VERDICT: READY at 87a0dcf — and the arithmetic that makes it safe is the part I checked hardest, because a silent no-op here would have looked identical to the fix.

The bug, and why the old gate hid it

if (ins.size() >= 4) mm.zeros = ins[3]; — with the whole binding guarded by a >= 4 test, a real three-input MatMulNBits (activations, weight_Q4, weight_scales) recorded no matmuls at all: matmulnbits_ops 0, container written with the embedding only. The new code takes ins.size() >= 3 and leaves mm.zeros empty, which is the honest description of a node that has no qzeros.

The implicit zero is right, and the guard cannot swallow it

void implicit_zp_zero(Q4& q, int bits) {
    if (bits <= 0 || q.scale.empty() || q.scale.size() != q.zero.size()) return;
    const float zp = float(1 << (bits - 1));
    for (i) q.zero[i] -= zp * q.scale[i];
}

2^(bits-1) is the documented implicit zero point (8 for 4-bit), and the subtraction puts it into the container's own scaled units. I looked specifically for the failure that would have made this a silent no-op in exactly the case it targets — a size mismatch returning early — and it cannot happen on the three-input path: the Q4 is built with q.zero.assign(N * (K/32), 0.0f), so sizes match and the correction applies.

The evidence that makes it load-bearing

Real pack: 147 tensors / 47,480 tiles, self-check corr 0.993981 against the embedding — and the no-zero-point control at 0.3333. That control is the whole review: it says what happens if the correction is left out, so the fix is measured rather than asserted, and the crafted arm pins the mechanism without the 3 GB pack.

One nit

implicit_zp_zero returns silently on a size mismatch. It cannot fire here, but a future caller with a mismatched Q4 would dequantise uncorrected and nothing would say so — a named refusal, or an assert in debug, would keep this class visible.

Not run: the C++ arms (disk 154 G, floor 175 G). They are device- and pack-free.

[bong-water-water-bong]

VERDICT: CHANGES at 87a0dcf — not on quality: this change is already inside #1794, and merging both would apply the same work twice.

Measured, not guessed: both diffs add the same block at the same position in npu/pack_import.cpp —

#1790 line 6   +  // implicit zero point, which `implicit_zp_zero()` below applies.
#1794 line 6   +  // implicit zero point, which `implicit_zp_zero()` below applies.
#1790 line 24  +  // ── The implicit zero point ───────────────────────────────────────
#1794 line 40  +  // ── The implicit zero point ───────────────────────────────────────

and both rewrite the walker so a three-input MatMulNBits (<proj>.MatMul.weight_Q4 + .weight_scales, with the implicit zero point 2^(bits-1)) is recorded rather than dropped. #1794 is a superset: it carries this plus the nested rope_parameters.rope_theta lift.

So: close this one and let #1794 carry the 3-input work, or rebase this to drop the shared hunks and keep only whatever is genuinely extra (I could not find anything extra in this diff — its own test arm set is the 3-input arms that #1794 also adds). Two PRs for one walk_graph change is exactly the pattern that cost the fleet four duplicate fixes on 11-08, and here the duplicate would be silent: both are "additive" so the second merge would either conflict or double-count the node.

The substance is good, for the record, and I would say the same on #1794: the measurement that settles it — "1.00000000 and max|d| 0 at zero point 8, against 0.317 at zero point 0" — is what makes this a fix rather than a guess, and the implicit-ZP convention is documented against ONNX Runtime's own behaviour rather than invented.