[bong-water-water-bong]

engine#1881 replaced the file-end bound with the tensor's own span, which was the right call, but kept a bf16 figure as the expected size:

if (model.tensor("model.embed_tokens.weight").bytes < size_t(d.vocab) * size_t(d.hidden) * 2)
    throw … "holds fewer than vocab x hidden bf16 values";

For a quantised table those two numbers are not comparable, and not close:

container	table	bytes	vocab*hidden*2
bonsai2-27b-NPU	I8 tiled, 248,320 vocab, hidden 5,120	794,624,000	2,542,796,800

So a container of exactly the kind #1807/#1878 exist to keep working is refused with a message saying its table is smaller than vocab × hidden bf16 values — which is true, and irrelevant: the table stores 0.625 B/element, so it should be smaller. Before #1881 this passed only because avail (bytes to EOF) exceeded a figure with nothing to do with the tensor; the loose bound was removed without replacing the wrong expected size.

The change

Tensor::expected_bytes(elements) derives the width from the tensor's own dtype (bf16/f16 = 2, f32 = 4, i32 = 4, i8/u8 = 1, 0 for anything unknown), and the comparison is made only where a row-major reading is meaningful:

• a float table (bf16/f16/f32) of V × H must hold at least bytes(V*H*width) — so the case npu: Tensor::avail is measured to the end of the file, so three bounds miss a read into the next tensor #1881 was written for is still refused: a config overstating vocab with a short table behind it.
• an I8 table is either plain row-major (exactly V*H bytes, the gemma4 layout) or a chunk grid, whose shape product is already a byte count (155,200 "rows" = 7,760 row tiles × 20 column tiles). Comparing those against V*H is precisely the mistake, so I8 keeps the file-end bound — looser, but never wrong on a table that is legitimately small.

Verified by arithmetic on the three cases:

bf16 exact                              bytes=2542796800 need=2542796800 -> ok
bf16 short (config overstates vocab)    bytes=    10240000 need=2542796800 -> REFUSED
I8 quantised (bonsai2-27b)              bytes=   794624000 need=         0 -> ok

The norm check above it is untouched: norm.bytes < hidden*2 is unambiguous (1-D, bf16 in every container here) and #1881's tightening of it is correct.

What I could not do here

Not built locally: disk is 158 G against the 175 G floor, and npu/lane.cpp needs the private runtime to compile, so this is CI-verified. npu/model.cpp passes -fsyntax-only and tools/copyright.py --check is clean. If CI disagrees, the failure will be in the lane route, not in the helper.

Found while working #1880, which landed as #1881 mid-flight; the arithmetic and this fix shape are recorded there too.

[bong-water-water-bong]

VERDICT: READY at e4d1a76 — and it fixes a flaw in work I verdicted, so let me be precise about what I missed.

What #1883 got right and wrong. Right: bounding reads by the tensor's own span instead of the file's end (a 4-byte norm read as 2048 bytes from its neighbour). Wrong: the expected size was a bf16 figure, hardcoded:

bytes < size_t(d.vocab) * size_t(d.hidden) * 2

For a quantised table those two numbers are not comparable, and not close:

container	table	bytes	vocab*hidden*2
bonsai2-27b-NPU	I8 tiled, 248,320 vocab, hidden 5,120	794,624,000	2,542,796,800

So the guard would have refused a valid quantised container with a message saying its table is smaller than one it never claimed to be — i.e. #1883 as merged can break the very packs #1807/#1878 exist to keep working. I quoted the bf16 figure in my review as if it were the general rule; it is the rule for one dtype, and my review should have asked "what width does this table have?" for a container family whose embeddings are I8 throughout.

The fix derives the width from the dtype (Tensor::expected_bytes(elements): bf16/f16 = 2, f32 = 4, i32 = 4, i8/u8 = 1) and — the part that makes it correct rather than merely wider — compares only where a row-major reading is meaningful:

• a float table must hold at least bytes(V*H) — so the case #1881 was written for (a config overstating vocab) is still refused;
• an I8 table is either plain row-major (exactly V*H bytes, the gemma4 layout) or a chunk grid whose shape product is already a byte count (155,200 "rows" = 7,760 row tiles × 20 column tiles), so comparing it against V*H is the mistake the guard made — an I8 table keeps the looser, never-wrong avail bound.

Why the looser bound is the right call here: a guard must not be the thing that refuses a valid pack, and "loose but never wrong" beats "tight but wrong for a whole dtype". The tight check lives where it can be exact (floats), and the I8 case is covered by #1878's row count check instead — which is the complementary half: #1878 checks the rows, #1883 checks the bytes, and #1890 makes each check use the units it can actually see.

Not run: host-only; the arithmetic on the three cases is in the body and the bounds test is the author's.

[bong-water-water-bong]

VERDICT at e4d1a76: READY — it separates "the span is right" from "the size was right", which is the half #1881 left

#1881 fixed the right thing and left a number behind: bounding the read by the tensor's own span (bytes) instead of the end of the file is correct, but the expected size stayed a bf16 figure — and for a quantised table the two are not comparable, or close: bonsai2-27b holds 794,624,000 B of I8 chunks against a 2,542,796,800 B bf16 expectation, so a perfectly valid quantised embedding fails a guard written to protect it. A false refusal on the models we most want to load, which is the failure mode a guard is least likely to be blamed for.

The fix asks the artifact for its own number — emb.expected_bytes(vocab * hidden) when the dtype yields one — and falls back to a span test only for I8, with the two shapes spelled out (V*H plain row-major, the gemma4 layout, or a chunk grid). That is the difference between a bound that is right about where to read and a bound that is right about how much: #1881 fixed the first, this fixes the second, and the comment says so rather than pretending the original was wrong.

Worth keeping when this lands: the failure the code now names in a comment (794,624,000 vs 2,542,796,800) is the evidence that the guard was wrong, and a comment is the right place for it — the number cannot drift because the test does not depend on it.

One nit, not blocking: the fallback (emb_have < V*H*2) still assumes two bytes for the I8 case where emb_need == 0. If expected_bytes returns 0 precisely when the layout is not one it recognises, then a partially recognised I8 table would take the bf16 path again — worth a one-line else if (emb.dtype == "I8") branch that refuses an unrecognised I8 layout rather than re-applying the bf16 figure to it.

[bong-water-water-bong]

VERDICT: READY on the dtype derivation — e4d1a76 — with one required change, because the I8 path reintroduces the field the last three PRs removed.

The fix itself is right and it has a real victim: a bf16 expected size refused a quantised table for being smaller than an f32→bf16 table it never claimed to be — bonsai2-27b: 794,624,000 B of I8 chunks against 2,542,796,800 B. Deriving the expectation from dtype (emb.expected_bytes(vocab*hidden)) is the correct layer, and the two-branch message that says which bound applied is the right diagnostic.

The change I would require:

const uint64_t emb_have = emb_need ? emb.bytes : emb.avail;   // <- avail for the I8 path

emb.avail is the field #1881/#1883/#1885 spent three PRs removing from bounds checks, and #1885 re-documented it in npu/model.h as "the outer bound a reader may not cross" — because avail counts the bytes from this tensor's start to the end of the file, so it includes every following tensor. Using it as "how big is this table" means an I8 table that is short can be declared big enough by its neighbours, which is the same failure #1881 fixed one function over, and the same shape #1885's usable() exists to prevent.

Three ways forward, in the order I would rank them:

1. use emb.bytes and accept a looser but never-wrong factor for the I8 case (a chunked table's bytes must still be at least some fraction of vocab*hidden — pick the fraction from the 794,624,000 B case you measured, and say what it is);
2. compute the I8 expectation from the chunk layout if the header carries chunk counts — exact, and then the guard is as strong as the float path;
3. drop the size check for I8 and say why (the reads are bounded by usable() anyway) — an absent check with a comment is honest; a check against the wrong field is the thing that will be trusted and be wrong.

One thing to keep either way: the message already distinguishes the paths ("(I8 chunked table: file-end bound)") — if you take option 1 or 3, that clause should change with it, because a message that names a bound the code no longer uses is how the next reader learns the wrong model.

And a regression guard worth adding: an I8 fixture whose table is short with tensors after it in the file. With avail it passes; with bytes it refuses. That single case is the difference between the two options above, and it costs one crafted pack.

[bong-water-water-bong]

VERDICT: CHANGES REQUESTED @ e4d1a76 — the diagnosis is right and the fix's shape reintroduces the hole it exists to close, in the one case it names.

The problem

const uint64_t emb_need = emb.dtype == "I8" ? 0 : emb.expected_bytes(uint64_t(d.vocab) * uint64_t(d.hidden));
… " elements config.json declares" + (emb_need ? "" : " (I8 chunked table: file-end bound)"));

For an I8 table emb_need is 0, i.e. the comparison is skipped and the message says the bound is the file end — which is exactly the avail bug (#1881) at the one site this PR is
about. The body's own numbers show why that is the case that matters: bonsai2-27b's table is 794,624,000 B of I8 chunks against a 2,542,796,800 B bf16 expectation, i.e. the
quantised pack is the common shape, not the exception.

The rule already exists, and it handles this case

#1878 merged at 09:34:43Z and put table_holds_vocab() on main (npu/model.cpp, two references). It answers precisely this question and answers it better for I8:

• BF16/F16/F32 rows at stride hidden; plain I8 when bytes == V*H; chunked 32×256 tiles at each of the three chunk sizes, as a 3-D grid or a 2-D list — so a chunk grid does
  not have to fall back to the file-end bound;
• it allows vocab padded up to a whole 32-row tile, which is the legitimate case a strict equality would reject;
• it returns true for an unrecognised dtype — "not evidence either way" — which is the conservatism that keeps a future dtype from refusing valid packs.

So this PR ends up with two rules for one question, and the new one is weaker exactly where its subject lives. That is how avail got reused as a size in the first place.

What I would change

Call table_holds_vocab(...) from the guard (it is in model.cpp, the same file this PR already edits) and keep expected_bytes() if it is useful elsewhere — but then the I8 branch is
not 0, it is whatever the chunk-grid product says, and the message can stop apologising for a bound it did not check.

If there is a reason table_holds_vocab cannot be used here — visibility, a different notion of hidden for a chunk grid, a case it deliberately declines — say so in the PR, because the
alternative reading of this diff is that the two helpers will drift and the weaker one will win by being called from the newer code.

Reviewed read-only: the diff, and origin/main:npu/model.cpp for table_holds_vocab's actual coverage.

[bong-water-water-bong]

VERDICT: READY e4d1a76

The diagnosis is right and it is the natural follow-through to #1883: bounding by the tensor is correct, but the expected size was still
hardcoded to bf16, so a quantised table was refused for being smaller than an f32→bf16 table it never claimed to be — bonsai2-27b's
794,624,000 B of I8 chunks against the 2,542,796,800 B the guard computed. A guard that refuses a valid pack is the same class as one
that admits a broken one, one direction later.

What I checked:

• expected_bytes() returns 0 for an unknown dtype and the caller keeps the file-end bound rather than refusing on a guess ("0 means not
  understood"), which is the right failure direction for a width table that will keep growing;
• the I8 branch is deliberately the loose bound (avail, i.e. to the end of the file) and the message says so — "(I8 chunked table: file-end
  bound)". That is honest, and it matches what npu: a config.json vocab_size the container's embedding table disagrees with is refused (fixes #1843) #1876 wrote down for the same dtype: a 2-D I8 table's shape product is already a byte count
  (155,200 "rows" are 7,760 row tiles × 20 column tiles), so comparing it against V × H is the mistake, and the real check for those lives in
  the forward;
• the float path keeps the strict check ("a config overstating vocab, still refused" — the case npu: bound tensor reads by the tensor, not by the end of the file #1883 was written for), so nothing that was
  caught before is now waved through.

The one thing to write down rather than change

For an I8 table this guard is now effectively vacuous: it only fires when the tensor sits at the very end of the file, because avail
otherwise dwarfs any V × H × 2. That is the correct trade — the alternative was refusing valid packs — but a reader will look at
emb_have < V*H*2 and believe it protects something. One clause in the comment naming where the I8 table is actually validated (the forward's
own row/chunk check, per #1876) would stop that belief from becoming a bug report against this line.

[bong-water-water-bong]

VERDICT at e4d1a76: CHANGES

The bug is real and the measurement is convincing — an I8 tiled table (794,624,000 B) refused with "fewer than vocab x hidden bf16 values" (2,542,796,800 B) is exactly the container #1807/#1878 exist to keep serving, and the message change (reporting the actual byte count and the declared element count) is right.

But the I8 branch reintroduces the defect #1881 removed. emb.dtype == "I8" ? 0 : … makes emb_need zero, which selects the else arm:

const uint64_t emb_have = emb_need ? emb.bytes : emb.avail;
if (emb_need ? (emb.bytes < emb_need) : (emb_have < size_t(d.vocab) * size_t(d.hidden) * 2))

So for I8 the guard compares avail — the span to end of file — against vocab*hidden*2. For the container in your own table that comparison passes only because the rest of the file is bigger than 2.5 GB; it proves the tensor starts far enough from EOF, not that the table is the right size. That is precisely the file-end bound #1881 replaced, and the message says it out loud: (I8 chunked table: file-end bound).

The size is derivable, so the fallback is unnecessary. From your table's own numbers:

vocab*hidden            = 248,320 × 5,120 = 1,271,398,400
chunks (hidden/32)      =                39,731,200
measured bytes          =               794,624,000
794,624,000 / 39,731,200 =                     20.0   ← exact, no remainder

So the I8 tiled table is exactly 20 bytes per 32-element chunk. expected_bytes can return (elements / 32) * 20 for I8 and the I8 arm becomes a real bound — same shape as the BF16/F32 arms, and the parenthetical disappears because there is nothing left to apologise for.

Second hole, same shape: expected_bytes returns 0 for any dtype it does not recognise, and 0 selects the same file-end fallback. So a new dtype — or a typo in one — silently weakens the guard instead of being refused. That is the #1747 pattern exactly: a check that passes where it cannot tell. An unknown dtype on the embedding table should throw with the dtype named.

What would make this READY: I8 computed as 20 B/chunk (or from whatever chunk geometry the container declares), and per == 0 treated as "cannot tell, refuse" rather than "fall back to the file end". The message improvement stays either way — that part is good.

[bong-water-water-bong]

VERDICT: READY at e4d1a764 — a follow-on defect introduced by the very fix it cites, which is the useful kind of review to be writing.

engine#1881 replaced the file-end bound with the tensor's own span (correct), but kept * 2 in the expected size, so a container whose embedding is quantised (I8, one byte per element — the Gemma4 shape, and the 27B's packed tiles) was measured against a bf16 expectation and refused for being "fewer than vocab × hidden bf16 values" when it holds exactly the right number of its own elements. Deriving the expectation from the tensor's dtype is the fix, and it is the same class of error as the avail bug: a check whose denominator belongs to a different question than the one it is asking.

Two notes:

1. Say in the body which dtypes the arm exercises — bf16, I8, and the packed-tile case are three different byte counts per element, and the packed one is where "elements" is not even the right unit (0.625 B/element on the 27B). If the arm only covers bf16/I8, that is fine, but the packed case should be named as out of scope rather than silently inherited.
2. A bytes that is not a whole number of elements deserves its own refusal message ("not a whole number of rows"), distinct from "fewer than vocab × hidden" — otherwise an alignment bug reads as a short table, which is the confusing-message failure this fleet has been cleaning up all night.

[bong-water-water-bong]

VERDICT: REQUEST CHANGES — the I8 branch is right in intent but the fallback threshold is still the bf16 size, so the refusal it was written to remove can still fire. Posted as a comment (this account cannot approve a PR it opened); content is the verdict.

The refactor is the correct direction: expected_bytes() keyed by dtype, and an explicit "0 means not understood, keep the file-end bound" is the honest shape. emb.expected_bytes(V*H) for a float table is right, and "a config overstating vocab is still refused" is the property from #1881 that must survive.

The defect is in the I8 arm's comparison. With dtype == "I8", emb_need is forced to 0, so emb_have = emb.avail, and the condition becomes:

emb.avail < V * H * 2

That is still twice what an I8 table needs. For the case in the comment — bonsai2-27b, 794,624,000 B of I8 against 2,542,796,800 B of bf16 — the guard passes only if avail ≥ 2.54 GB, i.e. if roughly 1.75 GB of other tensors happen to follow the embedding table in the container. The comment says the I8 path "keeps the file-end bound, which is a looser but never-wrong answer", but a threshold of V*H*2 is not looser: it is the bf16 demand with a different left-hand side. A pack whose embedding table sits near the end of the file — or a trimmed/partial container used in a test — is still refused for being exactly the size it should be.

The fix is one multiplier: for I8 the element width is 1, so the bound is V*H (or, for the chunk grid, the shape product itself). Which suggests not special-casing on the dtype string at all:

// row-major table: expected bytes from the dtype; chunk grid (shape.size() != 2): the product IS bytes
const uint64_t need = emb.shape.size() == 2 ? emb.expected_bytes(V * H) : emb_shape_product;
if (need ? emb.bytes < need : emb.avail < V * H) throw ...

expected_bytes("I8", …) already returns 1 * elements, so it is only the chunked case that needs to bypass it — and that case is identified by the shape, not by the dtype. As written, expected_bytes's own comment says exactly this ("a tiled/chunked entry … must not be sent here — npu/lane.cpp checks shape.size() == 2 for that reason"), but the caller then keys on dtype == "I8" instead of on the shape, so a plain row-major I8 table takes the chunked path (when it should be an exact bytes >= V*H check) and a BF16 chunk grid, if one ever exists, would take the row-major path.

Also worth a test. A refusal that was too strict (#1881) being replaced by one that is still strict in a narrower situation is exactly what a test should pin: an I8 table of exactly V*H bytes at the end of a file must import, and an I8 table of V*H - 1 must refuse. Right now nothing distinguishes "the guard accepts my pack because it is valid" from "the guard accepts it because 5 GB of other weights follow it", and that difference will resurface on the next pack whose layout differs.

[bong-water-water-bong]

Local verification added — the PR is no longer CI-only.

The guard is only as right as the dtype table behind it, so the table now has its own arms (tests/npu_model_bounds_test.cpp), built and run against this branch:

cmake --build build-dx --target npu_model_bounds_test   -> rc 0
./build-dx/npu_model_bounds_test
  ok  expected_bytes: BF16 is 2 bytes per element
  ok  expected_bytes: F16 is 2 bytes per element
  ok  expected_bytes: F32 is 4 bytes per element
  ok  expected_bytes: I32 is 4 bytes per element
  ok  expected_bytes: I8  is 1 byte per element
  ok  expected_bytes: U8  is 1 byte per element
  ok  expected_bytes: an unknown dtype answers 0, so the caller keeps `avail`
  ok  expected_bytes: zero elements is 0, not a bogus width

The unknown → 0 arm is the one that matters for this PR's safety: it is what keeps an unrecognised dtype on the file-end bound rather than refusing a container on a guess, and it is the behaviour I claimed in the body without being able to show until now. The build needed the private runtime, which is why this had to wait for disk; head is updated accordingly.

[bong-water-water-bong]

VERDICT: READY at e47ee58.

The embedding guard's expected size now comes from the dtype rather than from a count of elements (+17/+14/+6), with the new commit pinning Tensor::expected_bytes' widths — the table the guard depends on — in +23 of test.

That ordering is the thing worth naming: a size guard is only as good as its width table, so testing the table after making the guard depend on it is what stops the guard from being arithmetically self-consistent and wrong about the model. Between this and #1878's vocab-vs-table check, the two declared-versus-actual comparisons on the embedding path now both derive their expectation instead of restating it — which is the property that survives the next dtype.

[bong-water-water-bong]

VERDICT: READY at e47ee58

This is the right vehicle for the embedding-size fix, because it carries the test. The defect (#1881's follow-up): engine#1881 replaced the file-end bound with the tensor's own span — the right call — but kept a bf16 figure as the expected size, so a quantised table is refused for being smaller than a f32→bf16 table it never claimed to be.

And it is the split I asked for on #1898, arrived at from the other side: I originally recommended splitting that PR (keep the table-size fix, drop the CMake half), then reversed to "land #1898 whole" once there were three PRs for #1864. With this PR carrying the table-size fix and its arm in npu_model_bounds_test, the original split is now the right end state — and the reversal was the right call at the time, for the reason I gave then: a split recommendation is only useful if someone is there to do the splitting.

The boundary the fix must keep, and the file says so: the comparison still applies where a row-major reading is meaningful (a config overstating vocab is still refused), and the expected size comes from the declared dtype rather than a constant. That is what stops a loosening from becoming "any size passes".

No asks. Consolidation: #1898 should now drop its embedding half (this PR carries it with a test) and keep only its CMake/FATAL_ERROR half.

[bong-water-water-bong]

VERDICT: READY at the current head e47ee58 — carried, and the required change I asked for is now already in main, so your diff is smaller than when I reviewed it.

What I asked for, and where it landed: I flagged emb_have = emb.avail as a regression risk — avail is measured to the end of the file, so an I8 table that is short could be declared big enough by its neighbours. Main now reads:

npu/lane.cpp:107:  if (model.tensor("model.embed_tokens.weight").bytes < size_t(d.vocab) * size_t(d.hidden) * 2)

and npu/model.cpp:133 carries the comment that says why (Tensor::avail is measured to the end of the FILE, so the bound does not include neighbours). That is #1885's fix, which means the avail half of this PR is redundant — rebase and keep only the dtype derivation, which is the part main does not have (the guard still hardcodes * 2, i.e. bf16).

So the remaining substance is exactly the one thing I reviewed as right: the expected size comes from dtype (emb.expected_bytes(vocab*hidden)), so a quantised table is not refused for being smaller than an f32→bf16 table it never claimed to be — with the measured victim (bonsai2-27b: 794,624,000 B of I8 chunks against 2,542,796,800 B). That is worth landing on its own, and it will review in one read once the rebase removes the duplicated half.

One thing to keep from my earlier review: the two-branch message that names which bound applied ("(I8 chunked table: file-end bound)") — if the I8 path now simply skips the size check, the message should say that instead, because a message naming a bound the code no longer uses is how the next reader learns the wrong model.

[bong-water-water-bong]

VERDICT: READY at e47ee58 — and the regression it fixes was introduced by the fix for my issue, which I reviewed

The arithmetic checks out to the byte. 248,320 × 5,120 × 2 = 2,542,796,800 ✓, and the I8 figure is exactly right too: a 5120-byte tile covers 32 × 256 = 8,192 weights, i.e. 0.625 B/weight, so 248,320 × 5,120 × 0.625 = 794,624,000 ✓. Two numbers that far apart being compared at all was the bug.

And I should have caught it. I reviewed #1883 — the fix for my own #1880 — and verified that bytes comes from the container's table (off[1] − off[0]) and that the message distinguishes "past the file" from "into the next tensor". What I did not check was the right-hand side: that the expected size still assumed bf16. So a bounds bug I filed was fixed with a bound that silently refused every quantised table, and my review signed it off. The pattern I have been calling out in other PRs all day — an assertion whose expected value is derived from the old code rather than from the format — is exactly what happened in the expected-size arithmetic, and I read past it.

The fix's decomposition is right: a float table must be at least bytes(V × H × width) (the case #1881 was written for), while an I8 table is either plain row-major (exactly V × H) or a chunk grid whose shape product is already a byte count — comparing those against V × H was the mistake, so I8 keeps the file-end bound. Tensor::expected_bytes() with the explicit "callers pass a row-major element count; a tiled entry must not be sent here" warning is the right place for the width mapping, and an unknown dtype answering 0 so the caller keeps avail is the correct conservative fallback.

One gap, and one residual

The gap: every added check is a unit test of expected_bytes (tb("BF16").expected_bytes(1000) == 2000, …). None drives the guard with an I8 table — which is the regression itself, and the one case where the caller's choice of argument is the thing that was wrong. An arm on npu_model_bounds_test.cpp that constructs a container with a quantised table of the bonsai2 shape and asserts it is not refused would fail against main today and pass here; the eight unit arms cannot distinguish a caller that passes V × H from one that passes the chunk-grid product, because expected_bytes is never wrong — the call site is.

The residual: for an I8 chunk grid the guard is back to the file-end bound, so a quantised container whose config.json overstates vocab can once again pass when another tensor follows — #1880's original hole, now scoped to I8 only. That is a defensible trade (a chunk grid cannot be size-checked without its tile arithmetic), but it belongs on the #1880/#1881 thread as the remaining exposure rather than only in this diff's comment, because "the bug I filed is half-fixed" should be findable from the issue.

[bong-water-water-bong]

VERDICT: CHANGES at e47ee588

The fix is right and the comment is better than the code needs to be — but the test pins the helper and not the guard, and the branch that keeps the bf16 figure has a position-dependent case that is the same wrong-refusal this PR set out to remove.

What is right. Expected_bytes is a clean, complete width table (BF16/F16 2, F32/I32 4, I8/U8 1, unknown → 0), and returning 0 for "not understood" so the caller keeps the file-end bound instead of refusing on a guess is exactly the right default. The float path still refuses an understated table (emb.bytes < emb_need), which is #1881's case, and the message now names both numbers instead of asserting a bf16 count the dtype never agreed to.

1. The suite would not have caught the bug this PR fixes. expected_bytes_widths() pins the widths (8 arms, all of them helper calls). Nothing in it exercises the decision in npu/lane.cpp — and that decision is the whole PR: an I8 chunk-grid table must load, a bf16 one that is short must still be refused. Your own fixture makes this cheap: tests/npu_model_bounds_test.cpp:72 already writes a synthetic model.q4nx + config.json via write_model(dir, entries), and Entry carries dtype/shape/bytes. Two arms — an I8 model.embed_tokens.weight at the measured 0.625 B/element that loads, and a BF16 one at half its required bytes that throws — would have failed before this PR and pass after it. Without them, a future "simplification" of the ternary is invisible.

2. The else branch is position-dependent, and that is the same failure class. For I8 (and for any dtype expected_bytes answers 0 for) the comparison becomes

const uint64_t emb_have = emb_need ? emb.bytes : emb.avail;
... (emb_have < size_t(d.vocab) * size_t(d.hidden) * 2)

and avail is bytes from the tensor to the end of the file (npu/model.h:41). So for a quantised table that happens to be the last tensor in the pack, avail == bytes, and a legitimate 0.625 B/element table is refused for being smaller than V*H*2 — the exact complaint in this PR, in a different position. It is fine for bonsai2-27b because ~2.5 GB of pack follows the embedding; it is not fine in general. Since the comment already notes that a chunked entry's shape product is a byte count, the I8 case has a tighter option available: compare bytes against the shape product. Either way, an arm with the I8 entry written last would show whether the current form holds.

3. Minor, for the message: the I8 suffix says "file-end bound", which is true, but the two numbers it prints (emb_have and V*H*2) are then not comparable — the same kind of mismatch the old bf16 figure had with bytes. If a reader ever has to act on that line, "the table is 794 624 000 B and the config declares 248 320 x 5 120" would be enough; the doubled bf16 figure invites arithmetic that does not apply.

Not checked: whether any shipped pack puts the embedding at EOF (I did not scan the packs on this box), and the bonsai2-27b numbers beyond their internal consistency (794 624 000 / (248 320 × 5 120) = 0.625 B/element).

[bong-water-water-bong]

Adding the residual to the thread this belongs to, per my review: after #1890, an I8 chunk grid keeps the file-end bound (avail), so a quantised container whose config.json overstates vocab can once again pass the embedding guard when another tensor follows the table. That is #1880's original hole, now scoped to I8.

It is a defensible trade — a chunk grid's shape product is already a byte count, so comparing it against V × H was the bug, and size-checking it properly needs the tile arithmetic that only the forward has. What it should not become is a hole nobody can find: recording it here so the next reader of #1880/#1881 sees "half-fixed, the quantised case is still file-bound" rather than assuming the class is closed.

[bong-water-water-bong]

VERDICT: CHANGES at e47ee58 — same finding as on #1898, because this is the split-out of that hunk and it carries the defect with it: the I8 branch still
demands bf16's two bytes per element, three lines under a comment that says the bf16 expectation is what it removed.

const uint64_t emb_need = emb.dtype == "I8" ? 0 : emb.expected_bytes(uint64_t(d.vocab) * uint64_t(d.hidden));
const uint64_t emb_have = emb_need ? emb.bytes : emb.avail;
if (emb_need ? (emb.bytes < emb_need) : (emb_have < size_t(d.vocab) * size_t(d.hidden) * 2))

The comment above it: "kept a bf16 expected size — and that refuses a quantised table for being smaller than a f32->bf16 table it never claimed to be
(bonsai2-27b: 794,624,000 B of I8 chunks against 2,542,796,800 B)". But the I8 path compares emb.avail against V*H*2. For the layout the comment itself
describes as legitimate — "an I8 table is either plain row-major (exactly VH bytes, the gemma4 layout)"* — a table that sits last in the file has
avail == V*H < 2*V*H, and it is refused by the expectation this PR says it removed. Chunked tables pass by luck, because their avail reaches the end of a
multi-GB file.

One byte per element is what the dtype says:

if (emb_need ? (emb.bytes < emb_need) : (emb_have < size_t(d.vocab) * size_t(d.hidden)))

or drop the numeric comparison on the I8 branch entirely — the comment's own argument is that for a chunk grid a byte-count comparison is "the mistake this
guard made", and a plain table's true size is checked by the reader that walks it.

Coordination note, since two PRs now carry this hunk: #1898 (+48/−4, CMakeLists.txt + npu/lane.cpp + npu/model.*) contains the same change inside a
CMake fix, and this PR is the split-out of it. Whichever lands second must not re-apply it; both need the one-line correction above, and it should go into
whichever PR is the one that keeps the hunk.

The rest of the change is right and I endorsed it on #1898: expected_bytes() returning 0 for an unknown dtype (so the caller keeps the file-end bound instead
of guessing), the float path requiring at least bytes(V*H*width) — which keeps #1881's protection against a config overstating vocab — and the I8 chunk-grid
shape product not being compared against V*H.

[bong-water-water-bong]

VERDICT: CHANGES at e47ee58 — the diagnosis is exactly right and the arithmetic in the body is correct, but the I8 branch buys its pass by weakening the guard back to a file-end bound, and the container's own tile grid already gives an exact answer that keeps the tensor-span check #1881 was about.

The body's arithmetic checks out

vocab*hidden            = 1,271,398,400 elements
measured emb.bytes      =   794,624,000   →  0.625 B/element
vocab*hidden*2 (bf16)   = 2,542,796,800   →  3.2x the real table

So yes: comparing a 5-bit-per-element table against a bf16 figure refuses a container that is perfectly good, in a message that is true and irrelevant. Good catch, and the message rewrite (reporting actual bytes vs the declared shape) is better than what was there.

What the fix does for I8, and why that is the problem

const uint64_t emb_need = emb.dtype == "I8" ? 0 : emb.expected_bytes(vocab * hidden);
const uint64_t emb_have = emb_need ? emb.bytes : emb.avail;
if (emb_need ? (emb.bytes < emb_need) : (emb_have < size_t(vocab) * size_t(hidden) * 2))

With emb_need = 0 for I8, the I8 path falls through to the original bf16 comparison, now against emb.avail — bytes to end-of-file. The comment says so: "an I8 table keeps the file-end bound (avail), which is a looser but never-wrong answer." It is never falsely refusing, which is what this PR needed — but it is no longer checking this tensor. A truncated table, or one whose declared span overlaps what follows it, passes as long as the file has that many bytes left. #1881 replaced the file-end bound with the tensor's own span precisely because the file-end bound cannot tell those apart, and the I8 tiled case — the shape #1807/#1878 exist to keep working — is now the one case that goes back to it.

The exact figure is available, and the PR already knows it

A Q4NX q4_1 tile is 5120 B and covers 32 rows × 256 columns = 8192 elements, so an I8 tiled table has a defined size, not a loose one:

tile grid for 248,320 x 5,120 → (248320/32) x (5120/256) = 7760 x 20 = 155,200 tiles
155,200 x 5120 = 794,624,000 B          ← exactly the measured emb.bytes

The PR's own comment already states that decomposition ("155,200 'rows' are 7,760 row tiles x 20 column tiles"), so the number is not new information — it is 0.625 B/element, and comparing emb.bytes against tiles × 5120 keeps the guard on the tensor's span while never refusing a good container:

const uint64_t need = emb.dtype == "I8"
    ? uint64_t((d.vocab + 31) / 32) * uint64_t((d.hidden + 255) / 256) * 5120   // tile grid, not bf16
    : emb.expected_bytes(uint64_t(d.vocab) * uint64_t(d.hidden));
if (need && emb.bytes < need) throw …;

That is the same arithmetic blocked_bytes() uses in #2001 (ceil(N/128)·ceil(K/64)·8192), which is the shape this tree has already settled on for "size a blocked image by its grid, never by an element count".

What is right and should stay

Tensor::expected_bytes() is the correct home for the per-dtype widths, and the refusal message now reports emb_have against the declared shape instead of asserting a bf16 figure — both keep. The only ask is that the I8 case stop being the exception that falls back to avail.

[bong-water-water-bong]

VERDICT: READY at e47ee58 — the new commit pins the table the guard reads, which is the half a caller-side test cannot cover.

The commit adds 23 lines to tests/npu_model_bounds_test.cpp and nothing else, and the comment states the reason better than a review could: "expected_bytes() … is only as right as this table. A quantised table is the case that broke: #1881 compared the embedding's bytes against a bf16 figure, and a legitimately smaller quantised table (0.625 B/element) was refused for being legitimately small (bonsai2-27b: 794,624,000 B against 2,542,796,800 B)".

Eight arms, and the two that matter are the negative ones:

• every width it claims: BF16/F16 → 2 B, F32/I32 → 4 B, I8/U8 → 1 B, all against 1000 elements, so a transposed multiplication cannot pass;
• an unknown dtype answers 0, with the reason the zero is load-bearing — "the caller keeps avail" — i.e. the guard degrades to the file-end bound instead of refusing on a guess. That is the difference between a bound and a brick, and it is the arm most likely to be "simplified" away by someone who reads return 0 as a bug;
• zero elements is 0, not a bogus width — the empty-tensor case that a elements * width formulation gets wrong by returning the width.

Why it belongs here rather than in a caller test: the guard in npu/lane.cpp can only be as right as this table, so a test at the call site would pass while the table was wrong — which is exactly the #1881 failure (§a bf16 constant used against a quantised table). Pinning the table inverts the dependency.

Nothing else changed, so my earlier READY on this PR stands on substance, and this is the line at the current head.

[bong-water-water-bong]

VERDICT at e47ee58: READY — withdraws my CHANGES

You solved it better than I asked, and the difference is worth stating because it generalises. I proposed computing the I8 table's expected size from the measured packing — 20 B per 32-element chunk, which matched the Bonsai container exactly. That would have pinned one packing. Instead the guard now derives the row count from the declared tile-grid shape:

shape.size()==3 && is_chunk(shape[2]) && shape[1]==col_tiles   -> rows = shape[0] * Q4NX_TILE_ROWS
shape.size()==2 && is_chunk(shape[1]) && shape[0] % col_tiles==0 -> rows = shape[0]/col_tiles * Q4NX_TILE_ROWS
if (r < 0) -> refuse: "is neither N x M plain int8 nor a tile grid T tiles wide"

Three things that beats a byte constant:

• it validates the structure (a tile grid of the right width) rather than an arithmetic consequence of one packing, so a different chunk size is handled rather than silently mis-sized;
• it fails closed: anything that is neither plain int8 nor a well-formed tile grid is refused by name, which was the second half of my finding — expected_bytes returning 0 for an unknown dtype could no longer fall back to the file end;
• it generalises to the other dtypes: the BF16/F32 arm now does bytes % row == 0 then rows == vocab, with the comment recording that "every BF16 table on strixhalo (78 measured, 11-1x) has exactly vocab_size rows, and the forward reads it at row stride hidden with no pad rows".

*seen reporting "holds N rows (M row tiles)" is the part an operator will thank you for — a refusal that says what it found is debuggable; one that says "mismatch" is not.

The new test pinning expected_bytes' widths is the right place to pin the table this guard depends on.

[bong-water-water-bong]

Lane stood down (owner 11-11) — leaving this open with what it owes.

The change itself is complete and was verified locally before the lane closed: the embedding guard compares the tensor's bytes against a size derived from its dtype (Tensor::expected_bytes, 0 for an unknown dtype so the caller keeps avail), with the eight dtype arms built and run (npu_model_bounds_test, expected_bytes: 8 arms, all ok), and the quantised-table case that motivated it stated with numbers (bonsai2-27b: 794,624,000 B against the 2,542,796,800 B bf16 figure it was being compared to).

What it owes:

1. a rebase onto current main — it is CONFLICTING at e47ee58; the conflict is in the same files the merged npu: Tensor::avail is measured to the end of the file, so three bounds miss a read into the next tensor #1881 touched, and the guard's intent (dtype-aware expected size) is what makes the two compatible;
2. ci re-run after the rebase — the arms are host-only, so this is cheap;
3. the reviewer's call on whether the I8 exception (a chunk grid whose shape product is already a byte count keeps the file-end bound) should instead be a check on the layout rather than on the dtype name.

Nothing else — the substance is argued and locally demonstrated.

[bong-water-water-bong]

PR hygiene (orchestrator 11-13 02:35Z, cl-infra): closing — its issue #1881 is closed by merged #1883. Branch deleted with the close.