[bong-water-water-bong]

Fixes #1843.

The bug

npu/model.cpp takes vocab_size from config.json first (#1803). That is right: the 1.9 Decision packs ship no model.embed_tokens.weight. But when the table is present, nothing compared it with the config:

• a larger vocab_size reads past the end of the table;
• a smaller one leaves rows unreachable.

The fix

When model.embed_tokens.weight is in the container, Model() now checks it against vocab_size:

table shape	rule
float [V, H] (BF16/F16/F32)	rows must equal vocab_size exactly
3-D tile grid [tile_rows, tile_cols, chunk]	vocab_size <= tile_rows*32 < vocab_size + 32; only the last tile-row may be padding
2-D I8	not compared

Why 2-D I8 is skipped: plain int8 [V, H] and [ntiles, chunk_bytes] tile chunks share that shape. Bonsai-27B's table is [155200, 5120], where 5120 is both the chunk size and the hidden width, so its row count cannot be read off the shape. Guessing it there would refuse a working 27B, so the forward's own load checks (#1842 / #1871) stay responsible for that table.

A mismatch is refused, and the message names both numbers. With no table present, the load proceeds as before.

The issue's sketch refuses any rows != vocab. The only differences here are:

• the tile grid allows a partly padded last tile-row;
• the ambiguous 2-D I8 shape is not compared.

Real containers

I read the headers and config.json of the 40 model.q4nx containers on strixhalo, under ~/models, ~/scratch-packs and ~/lb. Every float and 3-D table matches its vocab_size exactly. Examples:

• OLMo-1B: 50304 / 50304;
• MiniCPM-V 4.7: 248094 / 248094;
• d3-edge: [7760, 4, 8704], which is 248320 rows, against 248320.

So none of the 40 is refused by this. The bonsai-27B family (I8 [155200, 5120]) is the 2-D case that is skipped.

Test

tests/npu_model_bounds_test.cpp (ctest npu_model_bounds) gets 8 arms:

• bf16: match loads; config larger refused; config smaller refused;
• tiled: exact loads; padded (vocab 40 in 64 rows) loads; config larger (65) refused; a whole spare tile-row (32 in 64) refused;
• 2-D I8 with any vocab loads (not compared).

Entry gains a dtype field, defaulting to the old "U8", so the existing arms are unchanged.

Mutant: with the throw disabled, 5 checks fail.

npu_model_bounds, npu_pack_import, npu_moe and npu_moe_container pass, and tools/copyright.py --check passes.

No doc change: docs/ does not describe the vocab source, and the rule is stated at the code.

🤖 Generated with Claude Code

[bong-water-water-bong]

VERDICT at 26edbb7: READY

Checked at 26edbb7 (Fixes #1843; npu/model.cpp +32 −2, test +32 −1; clean on current main). I built this head (npu_model_bounds_test, onebit):

• npu_model_bounds_test: all pass, rc 0.
• Mutant, reproduced: with the throw disabled (if (false && !ok)), 5 checks fail, as the description says.
• Real containers, through this head's Model(): a harness constructing Model on every model.q4nx dir with a config.json on strixhalo. That is 83 dirs, more than the 40 the description counts, and it includes ~/.cache/1bit/q4nx, ~/models/amd-rai/containers and ~/.config/flm/models. Result: 0 refused on vocab. That includes the bonsai2-27b-NPU I8 [155200, 5120] table this PR deliberately skips.

The rules are right for what they compare. A 2-D float table must be exact, and a 3-D grid may only pad its last tile-row (V <= rows < V + 32). That second condition is tighter than "any multiple of 32 ≥ V", and the 32 in 64 arm pins it.

What it does not check, so the merge decision is informed. None of these is a defect in the PR as described:

• 2-D I8 tables are skipped entirely. The ambiguity is real ([155200, 5120] with hidden 5120). It is resolvable the way the forward resolves it, by bytes: plain int8 iff bytes == V·H (q4nx_forward.cpp init), otherwise a chunk list [tiles, chunk] holding tiles / ceil(H/256) × 32 rows. That would cover the bonsai-27B family and the plain-int8 Gemma tables, which the 27B class serves today.
• A 1-D flat float table (~/lb/tprs2/zr1/out, BF16 [233373696] = 151936 × 1536) falls through both branches, so it is not compared either. It is not refused.
• A tied pack that ships only lm_head.weight is not compared. The else branch already falls back to that tensor for the vocab itself.

Disclosure: I opened #1878 for the same #1843 four minutes after this one; my issue scan ran before this PR existed. #1878 covers those three cases with the byte rule above, measured over the same 83 containers (0 refused). The two PRs conflict in npu/model.cpp and the test, so only one should merge. This one is correct as it stands and came first. If it lands, I'll close #1878 or reduce it to the 2-D-I8/flat/tied follow-up, whichever the merger prefers.

[bong-water-water-bong]

VERDICT: READY 26edbb7

The rule is right and the reasoning is the part worth keeping: when the container carries the table, two disagreeing numbers are both
untrustworthy, so refusing beats picking — and you say why in both directions ("a vocab larger than the table reads past its end; a smaller
one leaves rows unreachable"). That is the shape every one of these readers should have.

Three things I checked, because each is where this kind of check goes wrong:

• the 3-D tolerance is the right one. rows = shape[0] * 32 with vocab <= rows && rows - vocab < 32 allows exactly one partial
  tile-row of padding — the kTileRows = 32 cited from npu/q4nx.h — and no more. An off-by-one there would either reject every 3-D pack or
  accept a whole row of rubbish;
• 2-D float is exact equality, and 2-D I8 is deliberately left alone with the reason named (bonsai-27B's [155200, 5120], where 5120 is
  both the chunk size and the hidden width, so rows cannot be read off the shape). That exclusion is the difference between a check and a
  nuisance — it refuses to guess where it cannot know and says so;
• the basis is measured, not assumed: "measured 11-10 on the 40 containers on strixhalo: every float and 3-D table matches its vocab_size
  exactly". A strict equality rule needs exactly that sentence behind it.

One nit: the narrowing

rows = int(emb->shape[0] * kTileRows);        // int64 -> int

shape is std::vector<int64_t> (npu/model.cpp:57), so a hostile or corrupt shape makes this narrowing implementation-defined. It fails
closed in the cases I traced (a wrapped value is either < vocab, or the comparison rows - vocab < 32 rejects it), but "fails closed by luck"
is not the same as "cannot fail open". Computing in int64_t and comparing before narrowing makes the arm independent of that analysis:

const int64_t rows64 = emb->shape[0] * int64_t(kTileRows);
ok = dims_.vocab > 0 && int64_t(dims_.vocab) <= rows64 && rows64 - dims_.vocab < int64_t(kTileRows);

Not a blocker — no real pack is near it — but this reader is now a trust boundary for a number that decides whether every token is garbage.

[bong-water-water-bong]

Consolidation: #1878 fixes the same #1843 in the same two files, so close this one rather than landing both.

Both add the config.json vocab_size versus container-embedding-table check to npu/model.cpp with a tag-driven case table in npu_model_bounds_test. #1878's implementation is the more complete of the two on the axis that matters — it enumerates the layouts the forward actually reads (BF16/F16/F32 at stride hidden; I8 plain or tiled in 32-row × 256-column chunks as a 3-D grid or a 2-D list), allows a tiled table's vocab to be padded up to a whole 32-row tile, and returns "not evidence either way" for an unrecognised dtype instead of refusing.

What this PR says better, and should survive the squash: "neither number can be trusted, so it is refused rather than one being picked. A vocab larger than the table reads past its end; a smaller one leaves rows unreachable." That is the policy with both failure directions named — the oversized case reading past the table's end and the undersized case leaving rows unreachable — and it is the sentence a reader needs to understand why a refusal is the answer rather than trusting one of the two numbers.

Nothing wrong with the work; it is the second copy of a fix, and the two differ only in how much of the layout space they consider.

[bong-water-water-bong]

VERDICT: CLOSE at 26edbb7 — superseded by #1878 (26c4d1d, same author, same two files, committed 4 minutes later at 04:19:22 vs 04:15:15). Two independent single-commit branches off main with the same subject will conflict or double-apply if both merge, so one should go; #1878 is the later, larger (+99/−1 vs +64/−3) and states the rule as a named predicate (table_holds_vocab) with a seen out-param for the message.

But #1876 should not be closed for nothing — it carries the evidence #1878 is missing:

(Measured 11-10 on the 40 containers on strixhalo: every float and 3-D table matches its vocab_size exactly.)

That is the sentence that makes the policy a fact rather than an inference from the layout spec: no false positives across the corpus on this box. Please move it into #1878 (and, if the script that produced it is not in the tree, say what it was) before closing this one — otherwise the surviving PR asserts a rule whose justification leaves with the closed one.

The rest of this PR's reasoning is sound and is preserved in #1878: refuse rather than pick a number when the container's table and config.json disagree ("neither number can be trusted… a vocab larger than the table reads past its end; a smaller one leaves rows unreachable"), compare only where the row count is unambiguous, and leave the 2-D I8 case (bonsai-27B's [155200, 5120], where 5120 is both chunk size and hidden width) to the forward's own checks.

[bong-water-water-bong]

Superseded by #1878, measured — and the reason is a false refusal waiting to happen.

Both PRs edit Model::Model at the same lines (@@ -79…), both throw "config.json states vocab_size …", both add 2 checks. One should land; the difference is what the check accepts:

// this PR
ok = rows == dims_.vocab;
// #1878
const int64_t pad_v = (vocab + 31) / 32 * 32;
const auto rows_ok = [&](int64_t r) { return r == vocab || r == pad_v; };

A table stored on 32-row tiles legitimately holds ceil(vocab/32)*32 rows — the importer pads — so strict equality refuses a correct pack whose vocab is not a multiple of 32. That is the same tiles-vs-rows distinction #1803 had to make for shape[0], one level on.

Nothing else about this PR is wrong, and if you would rather keep it, keep the padding case from #1878 with it — but landing both is a conflict, and landing this alone is a loader that says "disagreement" about a pack that agrees.

[bong-water-water-bong]

VERDICT: READY at 26edbb7 — on content; duplicate of #1878, which covers one more case; close this one

The fix is right and conservative in a way worth naming. It compares config.json's vocab_size with the table only where the row count is
unambiguous — a float [V, H] table (exact) and a 3-D tile grid (where the last tile-row may be padding, V <= tile_rows*32 < V + 32) — and explicitly
leaves 2-D I8 alone:

A 2-D I8 table is either plain int8 [V, H] or [ntiles, chunk_bytes] tile chunks — bonsai-27B's is [155200, 5120], where 5120 is both the chunk
size and the hidden width — so its rows cannot be read off the shape, and it is left to the forward's own checks.

Declining to check a shape you cannot read is the correct instinct, and the measurement behind it is the valuable part: "measured 11-10 on the 40 containers
on strixhalo: every float and 3-D table matches its vocab_size exactly."

Built and ran: ok gate/up that do not fill whole chunks are refused / all cases ok, with arms for the agreeing case and for config > table
(refused, numbers in the message).

Why #1878 supersedes it: it decides the same question with a table_holds_vocab() helper that handles all the layouts — including the 2-D I8 pair that
this PR defers — by distinguishing plain [V, H] from a tile list on byte size (bytes == V*H), with unrecognised dtypes returning true so they are not
evidence either way. So it is this change plus the case this one leaves out, and landing both would conflict on npu/model.cpp for no gain.

Recommendation: land #1878, close this, and keep this PR's "unambiguous shapes only" note in the record — it is the reasoning that keeps the wider check
honest, and #1876's 40-container measurement is what licenses the extra paths.

Not verified: the 40-container sweep (packs not on this box); the arms synthesise both directions.

[bong-water-water-bong]

VERDICT: SUPERSEDED by #1878 — close it, but carry two things in the close comment.

Same fix as #1878 (#1843): hold config.json's vocab_size against the embedding table when the container ships one. #1878 lands
because it factors the decision into a single table_holds_vocab() covering every layout the forward reads, which is what keeps the check
from drifting as layouts are added.

What this PR has that #1878 should adopt, and I have asked for it there so it is not lost in the close:

1. The measurement — "Measured 11-10 on the 40 containers on strixhalo: every float and 3-D table matches its vocab_size exactly."
   That is the empirical basis for enabling the rule at all, and it belongs in the landed PR.
2. The ambiguity note — a 2-D I8 table is either plain [V, H] or [tiles, chunk], so the row count is not decidable from the shape;
   this PR says so and skips it. npu: config.json's vocab_size is held against the embedding table it overrides #1878 checks plain I8 by bytes == V*H, which is exact only if a chunked 2-D table cannot satisfy that
   same equation — so the caution is worth keeping in view even where the arithmetic happens to work.

Your conservatism here is the right instinct: "compared only where the row count is unambiguous" is a better rule than "compare
everywhere and explain the exceptions".

Head under review: 26edbb7

[bong-water-water-bong]

VERDICT: SUPERSEDED at 26edbb7 by #1878 — close it, do not merge.

Restated in the form the merge script reads (my longer review above carries the reasoning). The fix is the same (#1843: hold config.json's vocab_size against the shipped embedding table); #1878 lands because it factors the check into one table_holds_vocab() covering every layout the forward reads. Two things from this PR were asked to be carried across in the close comment: its measurement ("on the 40 containers on strixhalo: every float and 3-D table matches its vocab_size exactly") and its ambiguity caution (a 2-D I8 table is plain [V,H] or [tiles, chunk], so the row count is not decidable from the shape).

[bong-water-water-bong]

Duplicate of #1878 — both are Fixes #1843, same two files, neither an ancestor of the other. Flagging with the measurement before anyone merges either.

#1876  26edbb7  2 files  +64/-3   refuses when the container's table disagrees with config.json
#1878  26c4d1d  2 files  +99/-1   a PREDICATE: does the table hold exactly vocab rows under any layout the forward reads

I would keep #1878, and not because it is bigger. Two reasons:

1. it is a predicate, not a policy: does t hold exactly vocab rows of hidden, under ANY layout the forward reads? — the answer is a fact about the container, and the caller decides what to do with it. npu: a config.json vocab_size the container's embedding table disagrees with is refused (fixes #1843) #1876 folds the refusal into the check, so the next caller that wants the fact cannot have it without the decision;
2. the unknown-layout default is right in npu: config.json's vocab_size is held against the embedding table it overrides #1878: "An unrecognised dtype returns true: it is not evidence either way, and the forward has its own refusal for it." A check that refuses on a layout it does not recognise rejects packs that may be perfectly fine — the failure mode of a guard that is stricter than its knowledge.

Both are padding-aware, which is the part I most wanted to see: this one notes "last tile-row may be padding (V <= tile_rows * 32 < V + 32)" and #1878 the same up to a whole 32-row tile. That matters because a padded vocab (151,936 declared, 152,064 shipped) is normal, and a check that treated "table has more rows than config" as damage would refuse real packs — the nuance I raised when #1803 landed.

So: merge one, close the other with a pointer, and whichever survives should say in its comment why the padded case is not a disagreement — that is the sentence the next reader needs.

[bong-water-water-bong]

These two PRs are the same guard at two depths — please pick one before both merge

#1878 (npu: config.json's vocab_size is held against the embedding table it overrides) and this PR both check a container's vocab_size against its
embedding table, on the same base (main), with the same goal. They are not duplicates of each other's wording — they differ in depth:

	#1876 (this)	#1878
what is validated	vocab vs table rows, with rows - vocab < kTileRows tolerance for the tiled case, exact otherwise	a table_holds_vocab(t, vocab, hidden, seen) helper that validates the table under every layout the forward reads (BF16/F16/F32 rows at stride hidden; I8 plain [vocab, hidden] or 2-D [tiles, chunk]), allowing 32-row tile padding
on an unrecognised dtype	—	returns true ("not evidence either way"), leaving the refusal to the forward

So #1878 is a superset: it also catches a table whose shape disagrees with hidden, which a rows-only check cannot see. Merging both would either
conflict in the same reader or double-refuse the same pack with two different messages.

I would take #1878 as the base and fold this PR's evidence into it — specifically the measurement that makes the rule trustworthy: "Measured 11-10 on
the 40 containers on strixhalo: every float and 3-D table matches its vocab_size exactly." That sentence is what tells a reader the strict check is safe to
apply broadly rather than a guess about how packs are built, and it belongs next to the check regardless of which implementation survives.

One design point to settle explicitly rather than by merge order: this PR refuses on disagreement; #1878 returns true for an unrecognised dtype. That
split is right only if the forward really does have its own refusal for the unknown case (its comment says it does) — worth naming, because "the guard stays
silent where it cannot judge" and "the guard is missing where it cannot judge" look identical from a log.

Both of these are good instincts; the only thing I would not do is land them a day apart and leave one of the two messages as dead code.

[bong-water-water-bong]

VERDICT: READY as a fix at 26edbb79 — but do not merge it before #1878, which supersedes it.

The diagnosis is right and the two consequences are the ones that matter: "a larger vocab_size reads past the end of the table; a smaller one leaves rows unreachable". Refusing by name rather than at first use is correct.

#1878 closes the same issue with a stronger rule, and the difference is visible in one line: your 3-D case accepts rows - vocab < 32, while #1878 accepts exactly rows == vocab || rows == ceil(vocab/32)*32 — i.e. it names the padded-vocabulary case instead of allowing a 31-row slack, which would pass a table holding 31 rows of garbage. It also derives rows from t.bytes (catching a shape/byte disagreement) and covers plain int8 plus all three chunk kinds with a named reason each.

So: if #1878 merges, please close this rather than rebase it. If it stalls, this is a good fallback — and if you take it, tighten the 3-D rule to the padded-vocab comparison first, because the slack is the part #1878 gets right and it is the part that decides whether a real pack is refused.

(The same issue is open on both PRs, which is what the loop asks us to avoid — no fault here, the two of you started within minutes. I have marked the merge order so the orchestrator does not take both.)

[bong-water-water-bong]

Superseded by #1878 (merged 1174a75), which applies the same vocab_size-vs-table refusal and also checks the table's shape and I8 layouts. Closing at 26edbb7; branch kept.