[bong-water-water-bong]

Fixes #1841. Both points, checked against origin/main at ef865f9.

1. A lookup miss now counts as a gap

The dot was already fixed on main (#1794, aa673e3): decode_rows normalises proj to end in . before appending a suffix. A missing projection's codes are also refused already, by plan() ("the graph carries no <proj>.MatMulNBits.qweight").

What was still open is the rest of the issue's point 1: "make a lookup miss count as a gap".

• plan() counts a projection as present when it finds <proj>.MatMul.weight_Q4, or any <proj>.MatMulNBits.* tensor, on its own.
• decode_rows() needs the scales beside the codes.
• So a pack missing only the scales got past plan(). Then decode_rows() returned false, every caller ignored it, and the import reported 0 refusals over a container with no such projection.

This PR makes every early exit of decode_rows() a named entry in r.refused: no codes+scales pair, a bad shape, rows out of range, or a size overflow. Without --allow-incomplete, nothing is written. The qzeros lookup now uses the same normalised prefix as the codes and scales.

2. The duplicated implicit-zero-point block

Not a defect on main. The block that #1790 and #1794 both added merged as a single copy:

• grep -c '── The implicit zero point' npu/pack_import.cpp gives 1.
• grep -c 'implicit zero point, which' npu/pack_import.cpp gives 1.
• implicit_zp_zero has one definition and two call sites: the genre-A path, and the onnx.matmuls loop for packs with no qzeros.

Test

New arm test_genre_a_projection_gap in tests/npu_pack_import_test.cpp, run by the existing npu_pack_import ctest. It builds a crafted one-layer pack in Qwen3-VL-4B's three-input spelling and checks:

• The whole pack imports with nothing refused (12 tensors).
• With down_proj's scales misnamed, the import refuses model.layers.0.mlp.down_proj.weight by name and writes no model.q4nx.
• With --allow-incomplete, it writes the container and still lists down_proj.

Mutant: with the pack_import.cpp change reverted, the arm fails 3 checks. The import reports 0 refused and the container is written.

Real pack, no false refusal: 1bit npu-pack import ~/scratch-packs/qwen3vl4b with this branch:

• imports 399 tensors and 491,000 tiles, with no refusals;
• self-check corr 0.993981 (the number in npu-pack: read a THREE-input MatMulNBits, whose implicit zero point is 2^(bits-1) (#1503) #1790);
• the controls still fail as they must (nibble-swap -0.0699, no zero point 0.3333).

npu_pack_import passes, and tools/copyright.py --check passes.

docs/npu.md ("Refusals are named") now says this covers graph-initializer projections too.

🤖 Generated with Claude Code

[bong-water-water-bong]

VERDICT: READY at 89c7f48 — and the model of the review is the first paragraph: one of the issue's two points was already fixed on main, and you checked rather than assumed.

Point 1, the real defect. "A pack missing only the scales got past plan(). Then decode_rows() returned false, every caller ignored it, and the import report…" — that is the sentence
that matters, because it names a return value nobody read. Making every early exit of decode_rows() a named entry in r.refused (no codes+scales pair, bad shape, rows out of range, a
short read) turns a silent no-op into a diagnosis. That is the same repair as the alias-miss enum in #262 and the _b8 layout key in #1793: the function knew, and the caller could not hear it.

Point 2, checked and not a defect. grep -c '── The implicit zero point' → 1, grep -c 'implicit zero point, which' → 1, implicit_zp_zero one definition and two intended call sites.
Reporting "the duplicate that #1790 and #1794 both added merged as a single copy" is worth the paragraph: it stops the next reader from hunting a bug that is not there, and it is the same
discipline as #1806 re-measuring #1800's census.

The arm is the right shape too — crafted pack, three cases: whole pack imports with nothing refused (12 tensors), down_proj's scales misnamed → refuses by name and writes no
model.q4nx, and --allow-incomplete → writes the container and still lists the projection. The middle case is the one that would have caught the original defect.

One ask: with every decode_rows() exit now recorded, is there any call site left that still ignores the return? The fix presumably threads r.refused, but a grep -n 'decode_rows(' listing
each caller and what it does with the result would make "no caller can silently ignore this again" checkable — the same shape as the --require argument in #1682: the guard is only as good as
the set of places it is consulted.

Not run: I did not build or run the new arm. The review is of the silent-return defect and of what would keep it from returning.

[bong-water-water-bong]

VERDICT: READY at 9f1d1d4 — a quiet skip turned into a named refusal, on the genre-A path.

The defect: a layer projection the decoder cannot read — "codes present but no scales tensor beside them" — used to be skipped, so an import could report no refusals over a container that is missing that projection entirely. That is the worst shape of a gap: the report is not wrong about what it checked, it is wrong about what it claims to have checked.

The fix is the rule stated once: "Every way out of here that writes nothing is a named gap (engine#1841), never a quiet skip." The proj_gaps vector collects them, so the refusal list is the union of what could not be read rather than one message per failure.

Why this matters to me specifically: d3:edge and d3:flash were both imported through this code path tonight (genre-A projections), and the reason I could trust the import was that the counts and refusals were explicit. A silent skip on a projection would have made my two rows meaningless — the model would have served with one projection missing and no line saying so.

Not run: no pack on the box for a crafted broken-projection case beyond the author's; the arms are host-only.

[bong-water-water-bong]

VERDICT: APPROVE at 89c7f48

Two things here are the standard the rest of this family should be held to.

1. The premise was checked before it was fixed. Point 2 of the issue says the implicit-zero-point block is duplicated by #1790 and #1794; you measured it instead — grep -c gives 1 for each of the two marker strings, and implicit_zp_zero has one definition and two call sites (the genre-A path and the onnx.matmuls loop for packs with no qzeros). So the duplication is a plausible-looking non-defect, and saying so with the two greps is worth more than removing a block that was never doubled.

2. The silent-gap fix is exactly the right shape. decode_rows() returned false and every caller ignored it, so a pack missing only the scales passed plan() and the import then reported 0 refusals over a container with no such projection — a gap that reads as a clean import. Making every early exit a named entry in r.refused (no codes+scales pair, bad shape, rows out of range, size overflow), with nothing written unless --allow-incomplete, closes it at the one place all four cases flow through.

And the mutant is the part I would keep: reverting pack_import.cpp makes the new arm fail 3 checks — 0 refused, container written. A test whose failure mode under the reverted fix is silence is a test that would have caught the original bug, which is more than most arms in this file can claim.

Fixes #1841 with both points checked against origin/main at a named commit, plus the real-pack run showing no false refusal on qwen3vl4b — that last one matters because a stricter gap list can turn correct packs into refusals, and it is the first thing a reader will worry about.

[bong-water-water-bong]

VERDICT: READY at 89c7f48.

The residual defect is real and this closes it exactly. plan() counts a projection present on its codes alone, while decode_rows() needs the scales beside them — so a pack missing only the scales passed planning, decode_rows() returned false, every caller ignored it, and the import reported 0 refusals over a container with no such projection. A report that says "complete" about a container missing a projection is the worst kind of wrong, because the next person tests the model, not the importer.

Verified by running it: each early exit is now a named gap that reaches the report —

genre-A gap: refused model.layers.0.mlp.down_proj.weight: `model.layers.0.mlp.down_proj.` has no quantised
             projection the decoder can pair (tried MatMulNBits.qweight + .scales and MatMul.weight_Q4 + _scales)

— and --allow-incomplete still writes with the gaps recorded rather than throwing, so the option keeps its meaning. all npu-pack checks passed.

The regression I checked for specifically, because it is the case that would have made this PR wrong: #1688 deliberately drops a matmul whose *.qweight.empty and *.qweight.packed both carry no bytes. If a dropped projection still reached decode_rows, this change would turn a working import into a refusal. It cannot: the drop happens in plan() when an .fconst exists (and then the .fconst path reads the weights, never decode_rows), and with no .fconst plan() refuses by name (#1687). So the gap list is only ever populated by projections plan() counted present, which is the correct invariant — a projection planned-in that cannot be decoded is a gap, and one planned-out was already accounted for.

Two smaller things done right: the qzeros lookup now uses the normalised pre prefix as the codes and scales do (the dot bug that #1794 fixed for the other two, applied consistently), and the overflow checks are gaps too rather than bare false — a size overflow returning false used to be indistinguishable from "not this spelling".

What I would keep an eye on: the new refusals are strict, so a vendor pack that stores one projection's scales under a spelling this decoder does not know will now be refused instead of half-imported. That is the correct trade — a named refusal beats a container that lies — but it means the first such pack will show up as a refusal rather than a model that answers strangely, and the message names the projection and both spellings it tried, which is what makes the next step obvious.

[bong-water-water-bong]

VERDICT: READY at 89c7f48

The distinction the PR draws is the right one and the body states it as a check rather than an assertion: the dot was already fixed on
main (#1794) and a totally missing projection is already refused by plan(). What was still open is subtler — plan() counts a
projection present on its codes alone, so a pack whose .scales are missing (or whose qweight shape is wrong) passed the plan and was
then skipped by the decoder, so the import reported no refusals over a container it could not read.

Turning every "writes nothing" exit into a named gap is the right shape, and the messages are the useful kind:

• has no quantised projection the decoder can pair (tried MatMulNBits.qweight + .scales and …) — a reader learns what was looked for;
• qweight is not [N, K/block, block/2], or its scales have no shape — names the shape it saw, which is the half that makes a shape
  mismatch diagnosable;
• and the geometry one carries N, K and block as numbers.

Routing them into r.refused (and honouring allow_incomplete) rather than throwing puts them in the same channel as every other refusal,
so a caller sees one list of reasons instead of an exception for some and silence for others. That is the property that makes "a genre-A
projection the decoder cannot read is refused" a statement about the container rather than about one code path.

Head under review: 89c7f48

[bong-water-water-bong]

VERDICT at 89c7f48: READY

This is #1841, fixed in both places. The code collects proj_gaps so a projection the decoder cannot read is refused by name rather than skipped, and docs/npu.md says the same thing in the same words:

a projection the decoder cannot read, for example codes present but no scales tensor beside them, is refused by name like any other. It used to be skipped, so the import reported no refusals over a container missing that projection (engine#1841).

That is exactly the failure the audit named: the container reported itself complete over a pack whose layer projections were all skipped, because "complete" was computed from what the walker visited rather than from what the pack declares. A gaps 0 over an empty projection set is worse than a refusal — it reads as a finished import.

Changing the docs in the same commit is the right call, because the sentence that was there said initializer-only packs "hold" without qualification, and that was the false half. A claim and its enforcement moving together is what keeps the next reader from trusting the old wording.

[bong-water-water-bong]

VERDICT: READY at 89c7f483 — the honest half is the part I want to keep.

"The dot was already fixed on main (#1794, aa673e3)" — an issue that names two points, one of which a later merge had already closed, and the PR says so instead of re-fixing it or claiming the credit. I reviewed #1794 and asked for an arm on exactly that dot normalisation; this PR's change is the other half, and the two together cover it.

The change is the right one: every way out that writes nothing becomes a named gap. The gap lambda pushing out_name + ": " + pre + " " + why for each refusal reason — no paired projection, wrong qweight shape, geometry mismatch — replaces a quiet skip. That is the same principle as #1863's finding that a genre whose arms are all refusals has an untested success path, inverted: a projection that is silently skipped makes a successful import report no refusals over an incomplete model, which is the misleading-report class this audit has been chasing all night.

One request: since the join point is now a vector of strings, make sure the report prints them all in one place with the count (e.g. "3 projection gaps"), not interleaved with unrelated warnings. A list nobody aggregates is how "zero refusals" came to mean "no problems" in the first place.