npu-pack: refuse a 4-bit code outside 0..15 instead of wrapping it (engine#1918)

npu/pack_import.cpp

@@ -2233,6 +2233,21 @@ TileW decode_tile(const uint8_t* tile, int k, int rows) {
 
 std::vector<uint8_t> encode_tile(const TileW& t) {
     if (t.k <= 0 || t.k % kKBlock) throw std::runtime_error("tile: K is not a positive multiple of 128");
+    /* 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. Nothing
+     * here ever produced an out-of-range code: the pack legs on two real 1.9 packs are unchanged with this guard in place.
+     *
+     * What it does buy: the nibble packer below applies `& 0x0F`, so IF a future caller ever hands this function a code
+     * outside 0..15, it would wrap a full code range rather than saturate -- a weight one range away that still looks
+     * plausible. Nobody documents whether that is the caller's job or the writer's, so the writer refuses, naming the
+     * position and the value, instead of silently packing a different number. A caller that means 15 says 15. */
+    for (size_t i = 0; i < t.code.size(); ++i)
+        if (t.code[i] > 0x0F)
+            throw std::runtime_error("tile: code " + std::to_string(int(t.code[i])) + " at k=" + std::to_string(t.k) +
+                                     " rows=" + std::to_string(t.rows) + ", element " + std::to_string(i) +
+                                     ": a 4-bit code is 0..15, and packing it would wrap a full code range "
+                                     "(engine#1921: clamp or re-quantise before encoding)");
     const int64_t sv = subv_bytes(t.rows);
     if (sv == 0) throw std::runtime_error("tile: rows is not a positive multiple of 64");
     const int nsubv = t.rows / 2, kbs = t.k / kKBlock;

tests/npu_pack_import_test.cpp

@@ -4184,6 +4184,35 @@ void test_qk_norm_refusal() {
     std::printf("  qk norm: 7 arms (declared+not-found refuses; found-only and neither are allowed)\n");
 }
 
+/* engine#1921: the nibble packer applies `& 0x0F`, so a code outside 0..15 WRAPS a full code range instead of
+ * saturating -- measured as `container == expected mod 4.0` over 32/32 columns of a group, i.e. a wrong weight that still
+ * looks plausible. The writer now refuses, and 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. */
+void test_encode_tile_code_range() {
+    const auto tile_with = [](int v) {
+        pi::TileW t;
+        t.k = 128;      // one 128-wide group, the minimum the layout allows
+        t.rows = 64;
+        t.code.assign(size_t(t.rows) * size_t(t.k), uint8_t(v));
+        t.scale.assign(size_t(t.rows), 1.0f);
+        t.zp.assign(size_t(t.rows), 0);
+        t.bias.assign(size_t(t.rows), 0);
+        return t;
+    };
+    bool ok0 = true, ok15 = true;
+    try { (void)pi::encode_tile(tile_with(0)); } catch (const std::exception&) { ok0 = false; }
+    try { (void)pi::encode_tile(tile_with(15)); } catch (const std::exception&) { ok15 = false; }
+    check(ok0, "encode tile: code 0 (the bottom of the legal range) encodes");
+    check(ok15, "encode tile: code 15 (the top of the legal range) encodes");
+    std::string why;
+    try { (void)pi::encode_tile(tile_with(16)); } catch (const std::exception& e) { why = e.what(); }
+    check(!why.empty(), "encode tile: code 16 is REFUSED, not wrapped to 0");
+    check(why.find("engine#1921") != std::string::npos, "encode tile: the refusal carries its issue");
+    check(why.find("0..15") != std::string::npos, "encode tile: and states the range it wants");
+    check(why.find("wrap") != std::string::npos, "encode tile: and says what would have happened instead");
+    std::printf("  encode tile: 6 arms (0 and 15 encode, 16 refuses by name)\n");
+}
+
 // engine#1889: the arithmetic that claims the per-head QK gains out of the 640 B Hunyuan-7B carries between its last qkv
 // tile and o_proj. A wrong offset there reads tile bytes as bf16 and mis-rotates every pack of the family, so both the
 // right shape (2 * (64 + head_dim*2) = 640 at head_dim 128) and the neighbours that must NOT be taken for it are driven.
@@ -4203,6 +4232,7 @@ void test_qk_gain_offsets() {
 }
 
 int main(int argc, char** argv) {
+    test_encode_tile_code_range();
     test_qk_norm_refusal();
     test_qk_gain_offsets();
     test_tail_head_placement();