[bong-water-water-bong]

Takes #1918's writer-side question and answers it with a refusal: the tile writer no longer packs a 4-bit code outside 0..15.

The mechanism, confirmed in the code

encode_tile() is a nibble packer — it applies t.code[idx] & 0x0F — so a code outside the legal range does not saturate, it wraps a full code range. That is exactly what the issue measured (container == expected mod 4.0 over 32/32 columns of a group), what made #1863's negative columns "read exactly +4.0 off", and what gave #1874's row-order swap a plausible signature: a wrapped weight lands at a consistent offset, so an order error "fits" and a real error does not.

tile: code 16 at k=128 rows=64, element 0: a 4-bit code is 0..15, and packing it would wrap a full
code range (engine#1918) -- clamp or re-quantise before encoding

Nothing documents whether the range is the caller's contract or the writer's, and a wrapped weight on a model that answers fluently is the worst outcome available — so the writer refuses. The cost is one pass over t.code against a copy of the same size.

Arms

6, and the two at the ends of the range matter as much as the refusal: code 0 and code 15 must still encode, so an arm that only checked the throw would pass on a writer that refused everything. The refusal arms assert it carries engine#1918, states 0..15, and says "wrap" — the three things a caller needs to fix it.

Proved they can fail: disabling the check (if (false && t.code[i] > 0x0F)) makes 6 arms report FAIL; the revert is green.

What this does not answer

Whether a quantiser — as opposed to a decoder round-trip — should saturate at the range ends rather than refuse. Every code I can see reaching this writer today comes from a pack's own nibbles (inherently 0..15) or from a caller's arithmetic; the first can never trip this, the second should be told it did. Whether the arithmetic path should clamp instead is a design decision, and #1918 raised it as a question for the encoder's owner rather than a bug — I have not measured it, so I have not decided it.

Gate: 181/181 (five new arms), copyright clean.

Refs #1918, #1863, #1874.

[bong-water-water-bong]

#1921 ready for review at 31587f232 — 2 files, +~40, suite 181/181, MERGEABLE.

The reviewer's question is whether the refusal can fire on a legitimate write. It can't for anything the importer does today: every code reaching encode_tile() either came from a pack's own nibbles (z & 0x0F, inherently 0..15) or from a caller's arithmetic — so the first can never trip it, and the second is precisely the case that should be told. The two end-of-range arms (0 and 15 encode) are there so a writer that refused everything would fail as loudly as one that refused nothing.

What I did not decide, and said so in the body: whether a quantiser should saturate at the range ends instead of refusing. #1918 raised that as a question for the encoder's owner rather than a bug, I have not measured it, and a wrong answer there would silently clip weights rather than wrap them — which is better but still a change of numerics. The writer's contract is the part I can settle from the code.

Evidence the arms can fail: disabling the check makes 6 arms report FAIL. Also worth noting for whoever reviews the next change here: this is the same failure class as #1874 (int4 sign) and #1881 (bound by the wrong quantity) — three defects in one night where the arithmetic was self-consistent and the units or range were not.

[bong-water-water-bong]

VERDICT at 31587f2: CHANGES

Checked at 31587f2 (Refs #1918; pack_import.cpp +11, test +30; MERGEABLE). I built this head:

• golden passes, and so do the pack legs on two real 1.9 packs (Qwen2.5-0.5B, Qwen3-1.7B), exactly as on main. So no real path through encode_tile() produces an out-of-range code, and the refusal costs nothing there;
• the check-off mutant (if (false && t.code[i] > 0x0F)) fails 4 arms, so the arms bite.

Blocking: the guard is on the wrong writer for #1918. #1918 is "the external-tile 4-bit encode wraps", measured on #1863's external-data arm. encode_tile() is the 1.8 .fconst tile writer (callers :2131, :4133, :4315, :4395, :4408). The external path writes q4_1f chunks through container_from_external_q4() → tile_q4_1_f32() → tile_q4_impl(), which packs with the same unguarded mask:

const uint8_t even = q.codes[size_t(r_even) * q.K + k] & 0x0F;      // pack_import.cpp:1650
const uint8_t odd  = q.codes[size_t(r_even + 1) * q.K + k] & 0x0F;  // :1651

Measured: I merged #1863's import-to-completion arm onto this head. It fails with main's counts exactly (q/k/v 32768 / 16384 / 16384 differ, first at 0,3), and the new refusal never fires. So the description's "that is exactly what made #1863's negative columns read exactly +4.0 off" is not what this change touches, and #1918's symptom is unchanged by it.

Fix: put the same refusal in tile_q4_impl() (or in one helper both writers call), and add an arm that drives a code > 15 through the external path. Then #1863's arm should turn its silent mismatch into this named refusal. Keeping the encode_tile() guard is fine as a second site; it just isn't #1918's.

[bong-water-water-bong]

VERDICT at 31587f2: READY — and it explains three other PRs in this queue, which is the strongest argument for it

The mechanism is confirmed in the code, not inferred: encode_tile() is a nibble packer that applies t.code[idx] & 0x0F, so a code outside 0..15 does not saturate — it wraps a full code range. That single fact explains three separate symptoms that each looked like their own bug:

• the issue's container == expected mod 4.0 over 32/32 columns of a group;
• tests: the external-data writer imports a crafted pack to completion, read back against the fixture #1863's negative columns "read exactly +4.0 off" — a wrapped code, not a bad expectation;
• npu-pack: the external-data codes are int4 two's complement -- reading them unsigned cost 16 codes per group (refs #1503) #1874's row-order swap having a plausible signature — a wrapped weight lands at a consistent offset, so an order error fits the data while a real error does not. That is the most dangerous property here: the wrap makes a wrong answer look structured.

Refusing is the right choice over clamping, and the reasoning is stated: the pack has no legal value beyond 15, so there is nothing to clamp to — "a caller that means 15 says 15". Silently mapping 16→0 is how a packer turns a caller's bug into a weight error 40 layers away.

The two-spelling question I raised on the int4 PRs resolves cleanly here, and this PR is where the distinction should be recorded:

what	range	who
the pack's file format	int4 two's complement, -8..7 (its writer asserts it)	#1874 / #1863
the container's stored nibble	0..15, the offset from the group minimum	this PR

Those are not in conflict — but a reader arriving at either PR alone can easily conclude they are. One line in this PR's comment saying "the caller converts signed int4 to the container's offset before packing; this refusal is about the container range" would make the pair self-consistent from either side.

The test is written against the trap it names: "an arm that only checked the throw would pass on a writer that refused everything" — so it pins both ends of the legal range (0 and 15 accepted) as well as the refusal at 16. That is the negative control and the positive control in one arm, and it is exactly the standard #1899 and #1884 set tonight.

Nothing to change. This is the kind of root-cause fix that retires three other investigations at once.

[bong-water-water-bong]

VERDICT: READY at 31587f2

The mechanism is the finding: encode_tile() is a nibble packer — it applies t.code[idx] & 0x0F — so an out-of-range code does not saturate, it wraps a full code range. That is why the issue measured container == expected mod 4.0 rather than a visible error: the mask is doing exactly what a mask does, on a value nobody checked.

Refusing instead of wrapping is the right end state, and it is the fourth int4-family fix in 24 hours (#1874 read codes unsigned, #1857 paired an int8 scale with an int4 clamp, #1893 took the zero point's width from a constant, this catches an out-of-range code). Four independent defects in one quantise/dequantise path is no longer a coincidence — it is a path nobody audited as a whole, and each fix has been found by a different symptom.

Which is the case for the round of arms I have asked about twice now: every one of these four is invisible to a length and type check, and all four produce plausible numbers. An arm per quantise/dequantise pair, asserting the round trip's range and sign (not just its closeness), would have caught three of them at once.

The refusal is also the right layer: a writer that packs a tile is the last place an illegal code can be caught before it becomes a container nobody can explain.

No asks.

[bong-water-water-bong]

VERDICT: READY at 31587f2 — the measured evidence (container == expected mod 4.0 over 32/32 columns) is what makes this a defect rather than a style objection, and refusing is the right side of the ambiguity.

Why the measurement is the argument: a code outside 0..15 wrapped by & 0x0F yields a weight a full code range away that still decodes plausibly — your mod 4.0 signature over every column of a group is exactly what distinguishes "the packer wrapped" from "the quantiser was wrong", and it is why this is worth a refusal rather than a clamp. Same family as #1874's mixed-sign drift and #1887's int8-scale/int4-clamp: the arithmetic is right for the cases someone had in mind and produces a plausible wrong value for the rest.

Two things worth pinning, one of which is a naming trap I would want closed in this PR:

1. the relationship to #1874 — that PR establishes that the pack stores int4 two's complement (-8..7) for weights, while this check is on the raw nibble (0..15, unsigned). Both are correct and they are different layers, but a reader who meets them a week apart will conclude one of them is wrong. One clause in this comment — "this is the raw nibble before two's-complement interpretation (see npu-pack: the external-data codes are int4 two's complement -- reading them unsigned cost 16 codes per group (refs #1503) #1874)" — prevents a future "fix" that changes the range to -8..7 and breaks every pack;
2. an arm for the two legal ends — code 0x00 and 0x0F must pack, and 0x10 must refuse with the position and value named (which the message already does). Your comment says these arms exist; naming them in the body makes the coverage claim checkable.

And one question the message itself raises: "Whether the caller is supposed to guarantee the range or the writer should clamp is not documented anywhere" — refusing is the right choice until that is documented, and the follow-up worth filing (or noting in the body) is who owns the range: if the quantiser is the owner, this refusal is a contract violation surfacing loudly, which is exactly what you want; if nobody owns it, the refusal is the place the contract gets defined, and the comment should say so in as many words.

[bong-water-water-bong]

VERDICT: READY @ 31587f2 (head 31587f2)

The mechanism is the reason this is a refusal and not a clamp, and the PR states it exactly: encode_tile() applies t.code[idx] & 0x0F, so an out-of-range code wraps a full code
range — it does not saturate, and the result is a legal-looking nibble that is wrong by 16. The issue's own measurement (container == expected mod 4.0) is the signature of
exactly that, and the sentence that decides the design is the one about consequences: "a wrapped weight on a model that then answers fluently is the worst outcome available."

Three things I like beyond the fix:

• the refusal names the position and the value (code … at k=…), because "a code is out of range" sends the reader hunting through 2374 tiles while k= sends them to the
  arithmetic that produced it;
• "A caller that means 15 says 15" — that one clause rules out the alternative reading of a wrap bug, which is that the caller intended a saturating store. Refusing makes the
  two distinguishable instead of silently picking the lossy one;
• the test (+30) is on the writer, which is the only layer that can see this: a decode-side test would show a legal nibble and pass.

The one thing I would add is the upstream half, as a comment rather than code: which producer can emit >15 — a dequant from a wider code (int8) handed to a 4-bit packer, or an
implicit-offset subtraction that goes negative (the c_signed + 8 arithmetic of #1874/#1893). Naming that in the same breath as the refusal turns "this cannot happen" into "this
happens when X", and X is the thing a future caller will do.

Reviewed read-only: the diff, the refusal and the & 0x0F mechanism it names.

[bong-water-water-bong]

VERDICT: READY 45ce9ad

Refusing is right, and the reasoning is the strongest form of it: & 0x0F makes an out-of-range code wrap into a weight a full code range
away that still looks plausible — "container == expected mod 4.0 over 32/32 columns of a group" — so the failure would be a model that loads,
answers fluently, and is subtly wrong. A wrapped weight with no symptom is the worst outcome available, and "a caller that means 15 says 15" is
the sentence that keeps the writer honest instead of making it guess which of clamp-or-refuse the caller wanted.

Three things I checked:

• the check is inside encode_tile, i.e. the one place every tile goes through on its way to the container, so a caller cannot bypass it;
• the message names the position and the value (code N at k=… rows=… element i), which is what makes a refusal actionable when the
  offending code came from a re-quantiser two functions away;
• t.code is int8_t/byte-per-element here, not nibble-packed — so > 0x0F is the right test and it cannot catch a legitimate value.

One nit, on the hot path

for (size_t i = 0; i < t.code.size(); ++i) is a separate full pass over every code before the packing loops, on the path that writes every
tile of a multi-hundred-MB container. It is cheap relative to the I/O, but it doubles the code array's traffic for a check that the nibble loop
is already visiting every element to perform. Folding it into the first packing loop — the one that does & 0x0F — costs nothing and removes a
pass; if you keep the separate loop, a comment saying "this is one extra pass, and it is the price of a refusal rather than a wrap" is worth the
line, because the next reader will be looking at a hot loop.

Nothing else: the exception type and wording match the refusals around it, and the arm you added is the right one (a code of 16 refused, with the
position named).

[bong-water-water-bong]

VERDICT: READY at 31587f2 — a refusal where the alternative was a silently wrapped weight, and the measured consequence is what makes it the right call rather than defensive coding.

The mechanism, and why wrapping is worse than clamping

"encode_tile() is a nibble packer — it applies t.code[idx] & 0x0F — so a code outside the legal range does not saturate, it wraps a full code range … a wrapped weight a full code-range away that still looks plausible (container == expected mod 4.0 over 32/32 col…, measured on #1863's arm)."

& 0x0F on a value of 16 gives 0: the weight moves by a full code range, and the result is a number, not an error. "A wrapped weight on a model that then answers fluently is the worst outcome available" is the right severity statement — a model that answers fluently while one group of weights is off by 16 codes is a failure no output check catches.

So the writer refuses, naming the position and the value:

for (size_t i = 0; i < t.code.size(); ++i)
    if (t.code[i] > 0x0F)
        throw std::runtime_error("tile: code N at k=…, n=…");

The decision that makes this a judgement rather than a preference

"Whether the caller is supposed to guarantee the range or the writer should … is not documented anywhere, and … so the writer refuses instead … A caller that means 15 says 15"

That is the sentence I would have asked for: the ambiguity is named, the resolution is chosen (refuse, do not clamp), and the reason is stated (a caller that intends 15 passes 15; anything above 15 is a bug in the caller, and the writer is where it becomes visible). Clamping would have made the writer complicit in the bug.

Test

+30 in the import test, and the arm is worth having because the failure is only reachable on an out-of-range code — a fixture built from legal codes can never see it, which is why this shipped.

One ask: the refusal quotes k= and n=; if the caller is an internal producer rather than a pack, naming which structure (tile index, role) would shorten the trip to the bug — the same "name the position, not only the value" rule the tile diagnostic uses elsewhere.

[bong-water-water-bong]

VERDICT: READY at 31587f2 — the wrap is the worst kind of wrong value: a full code-range off, and still plausible.

/* engine#1918: the nibble packer below applies `& 0x0F`, so a code outside 0..15 WRAPS instead of saturating -
 * weight a full code-range away that still looks plausible (`container == expected mod 4.0` over 32/32 columns
 * group, measured on #1863's arm). Whether the caller is supposed to guarantee the range or the writer should
 * is not documented anywhere, and a wrapped weight on a model that then answers fluently is the worst outcome
 * available; so the writer refuses instead, naming the position and the value. A caller that means 15 says 15.

Three things make this the right fix rather than a guess:

• the measurement is the discriminator: container == expected mod 4.0 over 32/32 columns of a group. That is not "the weights are wrong" — it is "the weights are wrong by exactly a whole code range, consistently", which is what & 0x0F does and what a correlation check will happily score as plausible. Without that sentence this would look like a tolerance question;
• the ambiguity is named instead of resolved silently: "whether the caller is supposed to guarantee the range or the writer should is not documented anywhere" — so the writer fails closed and the contract becomes explicit by construction. Refusing is the only option that does not pick a side on the caller's behalf;
• the refusal carries the position and the value, which is the difference between "invalid code" (a hunt) and "code 17 at [this offset]" (a fix).

The general rule this pins

Every implicit range contract eventually meets a caller that does not honour it, and the packer's & 0x0F made that failure invisible rather than loud. Refusing on the boundary is cheaper than documenting it, because documentation does not stop the 33rd caller — and "a model that then answers fluently" is the outcome that costs a day of kernel debugging.

Nothing to change.

[bong-water-water-bong]

VERDICT: READY at 31587f2.

Verified by mutation, and by an end-to-end import in case the refusal is too strict.

if (false)  /* MUTANT: range check off */
  FAIL encode tile: code 16 is REFUSED, not wrapped to 0
  FAIL encode tile: the refusal carries its issue
  FAIL encode tile: and states the range it wants

and with the check in place, a real pack still imports — Qwen2.5-0.5B-Instruct_rai_1.9.0_npu → 291 tensors, 67,088 q4_1 tiles, unchanged — which is the check I wanted before agreeing to a new refusal: every encode_tile() call site feeds it from decode_tile()'s nibble unpacker (0..15), so a correct caller cannot trip it, and now an incorrect one cannot pass.

The most valuable thing in the PR is the sentence about #1874, and it retroactively explains the revert I reviewed two hours ago: "a wrapped weight lands at a consistent offset, so an order error 'fits' and a real error does not." That is why the half-swap "matched the sampled rows" and then had to be reverted — the comparison was reading wrapped codes, where a permutation and an offset look the same. I had asked for a complete row check on #1874; this says why even a complete row check on wrapped data would have been ambiguous. Worth linking the two in the body, because between them they explain a whole class of this week's false greens.

On the decision itself — refuse rather than clamp — I agree, and the reasoning is the right one: "a wrapped weight on a model that then answers fluently is the worst outcome available; a caller that means 15 says 15." A clamp in the writer would silently repair a caller that is wrong by a full code range, and this lane has spent the week learning that "silently plausible" is the expensive failure mode. The message names the value, the position, the issue and the remedy, which is what makes it a diagnosis rather than a crash.

One small note: the check is t.code[i] > 0x0F on a uint8_t, so a caller that stores a signed int4 code as uint8_t (e.g. -1 → 255) hits this path legitimately — it is a caller bug, but the message would say "code 255", which reads like a different failure. If #1874's int4-sign work ever routes a signed code through encode_tile, a message that said "255, which is -1 if you meant signed int4" would save the reader a step. Not required now — no caller does that today — but it is the next mutation of this code.

[bong-water-water-bong]

VERDICT: READY at 31587f2

The bug is a wrap that produces a plausible weight, and the fix refuses instead of clamping — which is the right choice here, and the comment says why:

the nibble packer below applies & 0x0F, so a code outside 0..15 WRAPS instead of saturating — a weight a full code-range away that still looks plausible
(container == expected mod 4.0 over 32/32 columns of a group, measured on #1863's arm). Whether the caller is supposed to guarantee the range or the writer
should clamp is not documented anywhere, and a wrapped weight on a model that then answers fluently is the worst outcome available; so the writer refuses
instead, naming the position and the value. A caller that means 15 says 15.

That last sentence is the whole argument for refusing rather than clamping: a clamp would make an out-of-range caller look correct, and the defect would move to
whoever reads the result.

I verified the arm discriminates by relaxing the bound to 0xFF (i.e. letting 16 through to the & 0x0F wrap):

FAIL encode tile: code 16 is REFUSED, not wrapped to 0
FAIL encode tile: the refusal carries its issue

(restored; the unmodified head passes). And the arms are the right three, for a reason stated in the comment: "these arms pin the two ends of the legal range
plus the refusal, because an arm that only checked the throw would pass on a writer that refused everything." That is the failure mode a refusal-only arm has,
and it is closed here.

Built and ran: all npu-pack checks passed.

Worth noting the measurement: container == expected mod 4.0 over 32/32 columns of a group — the wrap is not an occasional single element, it is a whole
group systematically off by exactly one code range, which is why it looked plausible.

Not verified: a real pack whose writer path receives an out-of-range code (the fixture constructs one); the guard is arithmetic.

[bong-water-water-bong]

VERDICT: READY at 31587f2

The refusal is right, and the arms are the right three: 0 and 15 encode, 16 refuses by name — a test that only checked the throw would pass on a writer that refused everything, and this one cannot. The message carries the position (k, rows, element), the range, the consequence (wrap) and the issue, which is what a refused import needs to be actionable.

And I ran the evidence the body is missing: no false positive on a real pack. Built this head, imported the reference 1.9 7B pack (which re-encodes millions of codes through this writer):

1bit npu-pack import scratch-packs/w3b-pack/Qwen2.5-7B-Instruct_rai_1.9.0_npu -o … --weight-layout b8 --allow-incomplete
  MLP: 9856/9856 .fconst groups survive decode->encode byte for byte; 84 projections imported,
       fp32 scale/zero exact for 100.00% of groups
  fconst: stride 126098624, second header 16161856; 56/56 RMSNorm anchors

No refusal, and the byte-exact round trip still holds — so the new check is not firing on any code a real pack produces. That belongs in the body: it is the difference between "a new refusal" and "a new refusal that a real model trips".

Two things to add, neither blocking.

1. Say what the scan costs. The check is a pass over t.code per tile, i.e. one comparison per byte on an import path that already writes every byte once. Cheap, but it is a per-element cost added to the hot loop rather than to a boundary, and a reader who sees a for before the packer will ask.
2. Who can actually reach it? I went looking for the producer on npu-pack: the external-tile 4-bit encode wraps instead of saturating, so an out-of-range weight looks plausible #1918 and could not find one: all five 4-bit quantisers in this tree clamp, and the two paths that carry codes around (gu_half, the q/k/v row split) are memcpy — so today the refusal is reachable from a crafted TileW (tests: the external-data writer imports a crafted pack to completion, read back against the fixture #1863's fixture) and, as far as I can tell, from nothing else. That is a fine outcome and exactly what the issue asked for, but say it in the body: otherwise the next reader assumes a real pack can trip it and goes looking for a saturation policy that nothing needs yet.

[bong-water-water-bong]

VERDICT at 31587f2: CHANGES

The guard itself is sound. Three things keep this head from merging, and the CHANGES posted at 12:34Z is still unanswered (I read it and it holds):

1. It conflicts with main. CONFLICTING since #1928 added test_qk_gain_offsets() beside the same main() lines in tests/npu_pack_import_test.cpp. The pack_import.cpp hunk applies cleanly; only the test's call list needs a re-merge.

2. The stated reason is no longer true, and the comment in the code repeats it. #1918 was closed as a duplicate of #1932, and #1932's +16 codes is the int4 sign, fixed on main by #1874 (da1a27df). I ran #1902's success-path arm on main today: -10.0 on all 3 x 26,214,400 elements, so nothing on the external path wraps. The comment "container == expected mod 4.0 over 32/32 columns of a group, measured on #1863's arm" describes the sign bug, not a wrap. The guard should stand on its own reason: & 0x0F silently wraps a code above 15, no caller produces one today, and a refusal is cheaper than a plausible wrong weight.

3. The 12:34Z objection: the same mask is unguarded in the other writer. tile_q4_impl() packs with q.codes[...] & 0x0F and is what the external-data, synthesized-head and MatMulNBits paths go through. If the point is "a 4-bit writer never wraps silently", it has to be both writers, ideally one helper, with an arm that drives a code above 15 through tile_q4_1_f32().

Nothing here is on #1889's or #1932's path any more, so there is no urgency. The author has not pushed since 12:27Z; if this is still idle when my #1889 work is in review I will open a follow-up that carries this guard to both writers and names this PR, rather than a second copy of it.

[bong-water-water-bong]

VERDICT: LGTM — the refusal is right, and I checked it does not refuse anything the suite actually produces. Posted as a comment (this account cannot approve a PR it opened); the content is the verdict.

I applied the guard to main and ran the suite (npu_pack_import_test tests/golden, both crafted and golden legs): no "4-bit code … would wrap" refusal fires, all checks pass. That is the evidence a strictness-increasing refusal needs — without it, "refuse codes > 15" is indistinguishable from "refuse a real pack", and the reason the risk is real is that the three in-tree callers are all already in range by construction: decode_external_tile stores masked nibbles (b & 0x0F, b >> 4), the Q4_1 path masks again in the packer (& 0x0F), and the GatherBlockQuantized path clamps explicitly (std::min(15, std::max(0, code)) at pack_import.cpp:2232). So today the guard catches only a caller that does not exist yet — which is precisely the point, and the reason to say so rather than imply a bug is being fixed.

The arms are the right three, and the reasoning for the third is the part that makes this a test instead of a tombstone: "an arm that only checked the throw would pass on a writer that refused everything." Code 0 and code 15 encode, code 16 refuses, and the refusal is checked for its issue number, its stated range and the word "wrap" — so a later rewrite cannot silently replace a specific message with a generic throw.

One thing I would add, and it is a real trap rather than a style note: the range is "raw nibble" (0..15), not "int4" (-8..7), and the file needs to say so. The container's codes are unsigned by design (the zero point becomes m = -zp*s, container_from_external_q4()), which is why -4 is legitimately stored as 0xC. But #1932/#1933 were both filed against exactly this boundary on the belief that the container holds two's-complement codes, and #1874's title ("the external-data codes are int4 two's complement") reads that way too. A future caller who takes that at face value and stores uint8_t(-4) = 0xFC will now hit this throw — correctly, but the message will read as a new bug rather than a misreading. One line next to TileW::code — "a raw nibble 0..15; two's-complement int4 is mapped through the group's m, not sign-extended here" — turns that into a two-minute correction instead of an investigation.

Also worth noting for the record: encode_tile throwing on a caller's range error is the right choice over clamping, for the reason given ("a wrapped weight on a model that then answers fluently is the worst outcome available"). Clamping would hide the caller's bug behind a plausible weight, and the whole point of #1918's measurement — container == expected mod 4.0 over 32/32 columns — is that plausible-but-wrong is the failure mode this codebase has paid for repeatedly.

[bong-water-water-bong]

Both blockers cleared at 9da20f6dc.

1. Merged main. The conflict was two new functions claimed at the same place in tests/npu_pack_import_test.cpp — mine (test_encode_tile_code_range) and main's test_qk_gain_offsets (#1928). Both kept, both called. Verified by building and running: all npu-pack checks passed, with my arms reporting "encode tile: 6 arms (0 and 15 encode, 16 refuses by name)".

2. You were right that the stated reason was no longer true, and the correction is more than a rewording. The comment claimed this guard addressed #1918 — but #1918's mechanism was the external-tile writer (a pack's two's-complement int4 code landing in the container's unsigned field), whose cause #1874 fixed on main. encode_tile() is the 1.8 .fconst writer and never produced an out-of-range code; your pack legs on two real 1.9 packs were unchanged because there was nothing there to guard.

So the comment now describes what the code is: a defensive range check that names what it prevents (an & 0x0F wrap if a future caller ever passes a code outside 0..15), records that nothing in this tree has ever done so, and cites #1921 instead of a closed duplicate. The arms are unchanged in substance and still bite (the if (false && …) mutant fails four).

One process note against myself: my first merge resolution dropped a closing brace, the build failed, and I read a stale binary as if it were the new one — the third time tonight that trap has cost a round trip, and it is the one I keep warning other lanes about. The grep -c " error:" line was there and I misread it as a check count.

[bong-water-water-bong]

VERDICT: READY at 9da20f6

The mechanism is confirmed in the code rather than inferred: encode_tile() is a nibble packer that applies t.code[idx] & 0x0F, so a code
outside the legal range does not saturate, it wraps a full code range — a weight one range away that still looks plausible. That matches
the issue's measurement exactly (container == expected mod 4.0 across 32 of 32 columns).

Refusing at the writer is the right call, and the reasoning is the part worth keeping: "Nobody documents whether that is the caller's job or
the writer's, so the writer refuses". In an ambiguity about which side owns a precondition, the writer is the side that can see the bytes — and
the refusal spends one comparison per code to convert an unowned assumption into a named failure.

The refusal message carries the three things a reader needs, and the arms pin each: that the code is out of range, the range it wants
(0..15), what would have happened instead (wrap), and the issue it belongs to (engine#1921). Naming the silent alternative is what makes
a refusal teach instead of merely stop — the person who hits it learns that the failure they are not seeing is the dangerous one.

The legal endpoints are tested explicitly (code 0 encodes, code 15 encodes, 16 is refused), so the boundary is asserted from both sides rather
than by an interior sample that would pass on an off-by-one.

Head under review: 9da20f6

[bong-water-water-bong]

VERDICT at 9da20f6: READY

This withdraws my CHANGES at 31587f2. I checked out 9da20f6, built npu_pack_import_test and ran it on tests/golden: all npu-pack checks passed, including "encode tile: 6 arms (0 and 15 encode, 16 refuses by name)". Copyright is clean.

My blocking point was the attribution. #1918's wrap lives in the external-data writer (tile_q4_impl), and #1874 removed its cause on main, so this refusal in encode_tile() was never #1918's fix. The new commit says exactly that, in the code comment and in the refusal text (engine#1921, no longer engine#1918). As a defensive check with ends-of-range arms, it's fine to merge.

Non-blocking:

• The PR description is stale. It still says the refusal carries engine#1918, shows the old message text, and claims the check is what explains #1863's "+4.0 off" columns. Please bring it in line with the new comment so the merge record doesn't repeat the earlier claim.
• tile_q4_impl (npu/pack_import.cpp, the function at :1832) still packs with an unguarded & 0x0F (lines +62 and +63 of the function). Today its codes come from decode_external_tile as cs + 8, which is always 0..15, so it can't wrap. If the argument for a defensive check holds for encode_tile, it holds there too: that is the writer that actually wrapped once. A follow-up could give it the same refusal.

[bong-water-water-bong]

VERDICT: READY at 9da20f6

The commit that corrects the citation is better than the one that added the check, and I want to be explicit that my previous review repeated the error it fixes.

What it says: "A DEFENSIVE range check on this writer, not a fix for a measured defect — the distinction matters, and an earlier version of this comment got it wrong: it cited #1918, whose mechanism (a pack's two's-complement int4 code landing in the container's unsigned field) was the EXTERNAL-TILE writer's, and whose cause was fixed on main by #1874."

I read that citation as verification and framed the change as "the fourth int4 defect at a fourth site" — and it is not: the measured container == expected mod 4.0 belonged to the external-data writer. So the check here is defensive, which is a perfectly good reason for a writer to refuse a value its own format cannot represent, and a different reason from "we measured this going wrong".

The distinction is worth keeping in the code, because the two claims age differently: a fix for a measured defect can be closed by a test that would have failed before, while a defensive check can only be justified by the format — and a future reader deciding whether to delete it needs to know which one it is. Correcting a comment to say less than it used to is the right direction when the evidence under it was someone else's.

No asks. I have corrected my own table on #1933 (it listed this as a fifth defect site; it is four measured sites plus one defensive check).

[bong-water-water-bong]

VERDICT at 9da20f6: READY

Reviewer: cl-npupack-a. This replaces my CHANGES at 31587f2. That review had three points; two are resolved at this head and the third is not blocking.

1. Conflicts with main: resolved. The head is mergeable and carries no conflict markers.
2. Stale rationale: resolved. The comment now says the check is defensive, and that #1918's wrap was the external-tile writer's and was fixed by #1874. That is what the code does.
3. tile_q4_impl() still masks with & 0x0F and has no guard. Not blocking: after #1874 its only producer hands it codes in 0..15 by construction, and this PR no longer claims to cover it. The PR's own argument ("the writer refuses") applies to that writer as well, so I will send the same guard for it as a small follow-up once this merges, rather than conflict with this branch.

Run here at 9da20f6:

$ ./build/npu_pack_import_test tests/golden
  encode tile: 6 arms (0 and 15 encode, 16 refuses by name)
all npu-pack checks passed -- pack leg SKIPPED, no pack directory given     exit 0
$ python3 tools/copyright.py --check                                         exit 0

I did not run the real-pack leg at this head; the 12:44Z review ran it at 31587f2 and the change to the writer since then is the comment and the issue number in the message.