[bong-water-water-bong]

What

The MLP section of an OGA pack's layer block — the last part of the pack the importer refused (72 projections on Qwen2.5-0.5B) — is now decoded, byte-verified and imported. After this, the only tensor 1bit npu-pack still refuses is model.norm (final), whose position is genuinely unestablished.

The layout, and why it is a measurement

Derived from the writer's own headers (mladfQuantMatrix / mladfWgtSubv in the vendor wheel's ops_common/mladf_matmul_matrix.hpp, concat_gate_up in matmulbias_weights_gateup.cpp), then checked against the bytes:

part	layout
gate + up	ONE fused hidden x 2*intermediate tensor, rows interleaved in 32-column chunks (gate 32, up 32, …), in 128-column groups of 4,288-byte subvolumes — the same struct at Nsubv 64: [0,4096) quants, [4096,4160) zero points, [4160,4288) 64 bf16 scales
—	one 64-byte parameter subvolume (01 00 …), the same bytes in every layer block
down_proj	intermediate x hidden in 64-column groups of the 2,176-byte subvolume the attention payload already uses

The three parts must fill the block stride exactly, and that identity — not a constant — decides whether a pack is one this decoder understands. On Qwen2.5-0.5B they end at 8,708,288 B, the measured stride, to the byte. Qwen3-1.7B and Qwen3-4B do not add up and refuse by name with the size they did add up to (their writer used different kernel shapes — the same runtime-shapes table engine#734 ran into).

Code

The same decoder at a second width. TileW gains rows (64 or 128 columns); the subvolume size, its zero/scale offsets and its quant index follow from it, so the attention payload, down_proj and the fused width share one verified code path. gu_half() splits a fused 128-column group into the two projections (gate = rows 0..31 and 64..95, up = 32..63 and 96..127).

Verified on the shipped pack

• 2160/2160 MLP groups decode and re-encode to the pack's exact bytes (24 layers × 76 gate|up units + 24 × 14 down groups) — the same round-trip contract the attention tiles are held to.
• Every weight in them agrees byte for byte with an independent decode of the same layer: the decode program stores gate/up fused at Nsubv 64 while the prefill program stores them separately at Nsubv 32, and the two agree on codes, scales and zero points for all 24 layers × {gate, up, down}.
• New CI fixture tests/golden/oga_mlp_gu_group.bin — one 68,608-byte unit copied verbatim from the pack by tools/oga_mlp_gu_golden.py, plus the container tiles its two projections give — so the decode and the split are checked against the vendor writer's own bytes where no vendor stack exists, with two controls that must fail (a moved code nibble, a one-ulp scale).
• Container: 290 tensors / 67,088 q4_1 tiles (was 218 / 25,136), and the engine's own Model opens it.

The bf16 parameter bound is unchanged and reported, not hidden: zero points are bf16-exact for 52.9 % of the MLP's 32-column groups (48.7 % of the attention ones), residual 3.34e-3 relative.

Checks run

On this branch, fresh Release/ninja/gcc-16 configure with ONEBIT_OGA_PACK:

• tools/copyright.py --check → rc 0
• ctest -R npu_pack_import → 2/2 (npu_pack_import 0.00 s incl. the new golden, npu_pack_import_oga 2.05 s on the real pack)
• tools/oga_mlp_gu_golden.py <pack> --check → the fixtures re-derive from the pack

Not in this PR

model.norm (final) stays refused: its .fconst tail is [eps][pad][448 f32], not an RMSNorm weight. The MLP geometry for other hidden sizes is open (refused by name, with the arithmetic in the message). Nothing of the vendor's is linked, called or shipped — the reader is written from the documented format facts in amd-oss/npu-re/docs/FACTS-oga-pack-weight-layout.md §7.17.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit c4d8dfc)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 734 - Partially compliant Compliant requirements: The PR derives the .fconst layer-block stride and the two norm offsets from the pack's meta N/K instead of constants measured on one pack The PR verifies on the second family with the same epsilon-anchor check (48/48 on Qwen2.5-0.5B; 32 per bucket for Llama) The PR addresses the specific refusal of model.layers.0.input_layernorm.weight and model.layers.0.post_attention_layernorm.weight by generalizing the offset calculation Non-compliant requirements: The PR does not fully address the final norm refusal (model.norm.weight) as mentioned in the issue description Requires further human verification: Verification that the final norm refusal is indeed resolved after this PR (as it's mentioned as still being refused)

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review MLP Section Geometry Calculation The PR introduces a new mlp_geom function that calculates the MLP section geometry based on the pack's meta N/K values. This is a key change for generalizing the MLP section handling. However, the calculation relies on intermediate and hidden values being multiples of 64, which may not always be true for all model configurations. If these values are not multiples of 64, the function returns an error message, but it's unclear if this is handled gracefully in all cases. MlpGeom mlp_geom(int64_t hidden, int64_t inter, int64_t mlp_off, int64_t stride) { MlpGeom m; m.off = mlp_off; const int64_t gu_unit = group_bytes(hidden, kMlpRows); // 128 columns of K=hidden const int64_t down_group = group_bytes(inter, kTileRows); // 64 columns of K=intermediate if (inter <= 0) { m.why = "config.json gives no intermediate_size"; return m; } if (gu_unit == 0) { m.why = "hidden " + std::to_string(hidden) + " is not a positive multiple of 128, so the fused " "gate/up section is not 128-column groups of this shape"; return m; } if (down_group == 0) { m.why = "intermediate " + std::to_string(inter) + " is not a positive multiple of 128, so " "down_proj is not 64-column groups of this shape"; return m; } if (inter % kTileRows || hidden % kTileRows) { m.why = "intermediate " + std::to_string(inter) + " / hidden " + std::to_string(hidden) + " are not multiples of 64, which the fused gate/up split needs"; return m; } m.gu_units = inter / kTileRows; m.down_groups = hidden / kTileRows; m.gu = m.gu_units * gu_unit; m.down = m.down_groups * down_group; m.total = m.off + m.gu + m.gap + m.down; if (m.total != stride) m.why = "the MLP section ends at " + std::to_string(m.total) + " B in a " + std::to_string(stride) + "-byte layer block, so its geometry is not the one this decoder was established on"; return m; } MLP Section Byte Verification The PR adds byte verification for the MLP section by decoding and re-encoding groups of tiles. This is a good safety measure, but the verification process relies on the encode_tile function which now supports variable rows parameter. If there are inconsistencies in how the encoding/decoding is handled for different row sizes, it could lead to false positives or negatives in the verification. // ── the MLP section: gate|up fused, then down_proj ─────────────────────────────────── // Same contract as the attention payload: every group read here is decoded, re-encoded and // compared with the bytes it came from before any of it is written, and a group that does // not survive that is refused by name. The geometry is re-derived from this block rather // than taken from plan(), so the stride identity is checked against the file being read. if (p.mlp_gu_units) { const MlpGeom mg = mlp_geom(H, p.dims.intermediate, g.h1 + kNormOff + H * 2, g.stride); if (!mg.ok()) { late.push_back("the MLP section: " + mg.why); } else { const int inter = p.dims.intermediate; const int64_t gu_unit = group_bytes(H, kMlpRows); const int64_t down_group = group_bytes(inter, kTileRows); for (int L = 0; L < p.dims.layers; ++L) { const int64_t base_gu = int64_t(L) * g.stride + mg.off; const int64_t base_down = base_gu + mg.gu + mg.gap; std::vector<TileW> gate, up, down; bool gate_ok = true, up_ok = true, down_ok = true; bool gate_bias = false, up_bias = false, down_bias = false; for (int u = 0; u < int(mg.gu_units); ++u) { const uint8_t* src = at(fc, base_gu + int64_t(u) * gu_unit, gu_unit); const TileW grp = decode_tile(src, int(H), kMlpRows); ++r.mlp_groups_total; const std::vector<uint8_t> back = encode_tile(grp); if (back.size() == size_t(gu_unit) && std::memcmp(back.data(), src, size_t(gu_unit)) == 0) ++r.mlp_groups_ok; else gate_ok = up_ok = false; gate.push_back(gu_half(grp, false)); up.push_back(gu_half(grp, true)); } for (int gi = 0; gi < int(mg.down_groups); ++gi) { const uint8_t* src = at(fc, base_down + int64_t(gi) * down_group, down_group); const TileW grp = decode_tile(src, inter, kTileRows); ++r.mlp_groups_total; const std::vector<uint8_t> back = encode_tile(grp); if (back.size() == size_t(down_group) && std::memcmp(back.data(), src, size_t(down_group)) == 0) ++r.mlp_groups_ok; else down_ok = false; down.push_back(std::move(grp)); } auto emit = [&](const std::vector<TileW>& tiles, const char* suffix, bool exact, bool& bias_flag) { const std::string name = "model.layers." + std::to_string(L) + "." + suffix; for (const auto& t : tiles) for (uint16_t v : t.bias) bias_flag |= (v != 0); if (!exact) { late.push_back(name + ".weight: the .fconst group does not survive " "decode->encode, so its fields are not understood"); return; } if (bias_flag) { late.push_back(name + ".weight: the MLP bias subvolumes are not empty and the " "container has no MLP bias"); return; } Q4 q = q_from_tiles(tiles); std::vector<uint8_t> tiled; long long zinexact = 0; tile_q4_1(q, tiled, &zinexact); mlp_zero_inexact += zinexact; mlp_zero_groups += (long long)q.scale.size(); const int64_t nt = int64_t(tiled.size() / kTileBytes); r.tiles += nt; w.add(name + ".weight", "I8", {nt, int64_t(kTileBytes)}, std::move(tiled)); ++r.tensors; ++r.mlp_projections; }; emit(gate, "mlp.gate_proj", gate_ok, gate_bias); emit(up, "mlp.up_proj", up_ok, up_bias); emit(down, "mlp.down_proj", down_ok, down_bias); } r.mlp_zero_inexact_pct = mlp_zero_groups ? 100.0 * double(mlp_zero_inexact) / double(mlp_zero_groups) : 0.0; } } MLP Golden Test Coverage The PR adds a test case for MLP golden data, which is good for ensuring correctness. However, the test only covers one specific case (K=896) and doesn't verify the generalization for other values. It's important to ensure that the test covers a range of values to validate that the generalization works correctly. // ── 3c. the .fconst gate|up unit against shipped pack bytes ───────────────── void test_mlp_gu(const std::string& golden_dir) { if (golden_dir.empty()) return; const std::string path = golden_dir + "/oga_mlp_gu_group.bin"; std::ifstream f(path, std::ios::binary); if (!f) { std::printf("skip MLP golden: %s absent\n", path.c_str()); return; } std::vector<uint8_t> group((std::istreambuf_iterator<char>(f)), std::istreambuf_iterator<char>()); const int k = 896; const int64_t want = pi::group_bytes(k, 128); check(int64_t(group.size()) == want, "MLP golden: the fixture is one " + std::to_string(want) + "-byte gate|up unit of K=" + std::to_string(k) + " (got " + std::to_string(group.size()) + ")"); if (int64_t(group.size()) != want) return; const pi::TileW g = pi::decode_tile(group.data(), k, 128); check(g.rows == 128 && g.code.size() == 128u * size_t(k), "MLP golden: the unit decodes to 128 columns"); const std::vector<uint8_t> back = pi::encode_tile(g); check(back == group, "MLP golden: encode(decode(unit)) reproduces the pack's bytes exactly"); if (back != group) for (size_t i = 0; i < std::min(back.size(), group.size()); ++i) if (back[i] != group[i]) { std::fprintf(stderr, " first difference at byte %zu: ours %02x, pack %02x\n", i, back[i], group[i]); break; } { pi::TileW bad = g; bad.code[70] ^= 0x01; check(pi::encode_tile(bad) != group, "MLP golden: a single changed code nibble is caught"); pi::TileW bad2 = g; bad2.scale[100] *= 1.004f; check(pi::encode_tile(bad2) != group, "MLP golden: a one-ulp scale change is caught"); } // engine#849's class at the MLP width: the round trip above cannot see the zero-point pairing (it is // invariant under any permutation applied on both sides), and the fused unit's 4,288-byte subvolume // carries 64 rows per half instead of 32, so the same two checks are made here at `rows = 128`. { const int kbs = k / 128; const int64_t subv = pi::group_bytes(k, 128) / (2 * kbs + 2); const int subv_rows = 64; // = rows / 2 for the fused width std::vector<uint8_t> raw(size_t(pi::group_bytes(k, 128)), 0); for (int h = 0; h < 2; ++h) for (int kb = 0; kb < kbs; ++kb) for (int i = 0; i < subv_rows / 2; ++i) { // a nibble holds 0..15, and the fused width uses 32 of these bytes per half, so the // pattern repeats: what matters is which row each nibble lands on, not that the 32 // bytes are distinct. const int v = i & 0x0F; raw[size_t((2 + 2 * kb + h) * subv + 64 * subv_rows + i)] = uint8_t(v | ((15 - v) << 4)); } const pi::TileW z = pi::decode_tile(raw.data(), k, 128); bool pairs_even_odd = true; for (int h = 0; h < 2 && pairs_even_odd; ++h) for (int kb = 0; kb < kbs && pairs_even_odd; ++kb) for (int i = 0; i < subv_rows / 2; ++i) { const uint8_t v = uint8_t(i & 0x0F); pairs_even_odd = pairs_even_odd && z.zp[size_t(h * subv_rows + 2 * i) * size_t(kbs) + size_t(kb)] == v && z.zp[size_t(h * subv_rows + 2 * i + 1) * size_t(kbs) + size_t(kb)] == uint8_t(15 - v); } check(pairs_even_odd, "MLP golden: zero-point nibbles pair with rows 2i / 2i+1 (even/odd) at 128 rows"); // and on the pack's own unit: a correctly paired zero point sits where its 128 codes cluster double n = 0, sz = 0, sm = 0, szz = 0, smm = 0, szm = 0; for (int r = 0; r < 128; ++r) for (int b = 0; b < kbs; ++b) { double sum = 0; for (int c = 0; c < 128; ++c) sum += g.code[size_t(r) * size_t(k) + size_t(b * 128 + c)]; const double m = sum / 128.0, zz = double(g.zp[size_t(r) * size_t(kbs) + size_t(b)]); n += 1; sz += zz; sm += m; szz += zz * zz; smm += m * m; szm += zz * m; } const double cov = szm / n - (sz / n) * (sm / n); const double sd = std::sqrt(std::max(0.0, szz / n - (sz / n) * (sz / n))); const double sm2 = std::sqrt(std::max(0.0, smm / n - (sm / n) * (sm / n))); const double corr = (sd > 0 && sm2 > 0) ? cov / (sd * sm2) : 0.0; char buf[160]; std::snprintf(buf, sizeof buf, "MLP golden: the zero points sit under their own codes (corr %+0.4f, want > 0.9)", corr); check(corr > 0.9, buf); } // the fused unit holds gate and up interleaved in 32-column chunks: both halves have to reproduce the // container tiles recorded from an independent decode of the same bytes std::ifstream qf(golden_dir + "/oga_mlp_gu_q4_1.bin", std::ios::binary); std::vector<uint8_t> golden((std::istreambuf_iterator<char>(qf)), std::istreambuf_iterator<char>()); std::vector<uint8_t> tiled; for (const bool up : {false, true}) { const pi::Q4 q = pi::q_from_tiles({pi::gu_half(g, up)}); std::vector<uint8_t> part; pi::tile_q4_1(q, part); tiled.insert(tiled.end(), part.begin(), part.end()); } check(golden.size() == tiled.size(), "MLP golden: gate and up give " + std::to_string(tiled.size() / onebit::npu::kTileBytes) + " container tiles of 64 rows each"); check(golden == tiled, "MLP golden: container tiles byte-identical to the recorded ones"); if (golden != tiled) for (size_t i = 0; i < std::min(golden.size(), tiled.size()); ++i) if (golden[i] != tiled[i]) { std::fprintf(stderr, " first tile difference at byte %zu: golden %02x, ours %02x\n", i, golden[i], tiled[i]); break; } // the split has to be able to fail: swapping the two halves of one column-block must change the tiles { pi::TileW swapped = g; const size_t kb = size_t(k); for (int r = 0; r < 32; ++r) { std::swap_ranges(&swapped.code[size_t(r) * kb], &swapped.code[size_t(r) * kb] + kb, &swapped.code[size_t(32 + r) * kb]); } std::vector<uint8_t> other; // gate AND up, as `tiled` is: a gate-only stream differs in length for (const bool up : {false, true}) { std::vector<uint8_t> part; pi::tile_q4_1(pi::q_from_tiles({pi::gu_half(swapped, up)}), part); other.insert(other.end(), part.begin(), part.end()); } check(other.size() == tiled.size() && other != tiled, "MLP golden: a gate/up swap inside a column block is caught"); } std::printf("MLP golden: 1 unit, encode/decode exact, %zu container tiles identical\n", tiled.size() / onebit::npu::kTileBytes); }

[bong-water-water-bong]

VERDICT: CHANGES at c84110ae — one item, and it is a one-line cast: the aggregate CodeQL check is red on a new
alert in the file this PR adds. Everything else here reproduces on the shipped pack, including the parts I could check
independently of the importer's own code.

The blocker. CodeQL alert #70, rule cpp/integer-multiplication-cast-to-long (high),
tests/npu_pack_import_test.cpp:657:

check(r.mlp_groups_total == layers * (p.dims.intermediate / 64 + p.dims.hidden / 64),

r.mlp_groups_total is long long but layers is int, so the multiply happens in int and is converted afterwards
(the values here are 24 x 90, so nothing overflows — the query is right about the form, not about this pack).
Fix: int64_t(layers) * (...). #844 carries the same line, so fixing it in this branch fixes both; I re-verdict
both the moment a head moves.

Built and run here (fresh Release/ninja/gcc-16 configure, -DONEBIT_OGA_PACK=<the 0.5B pack>):
ninja npu_pack_import_test onebit rc 0, the only warning the pre-existing nlohmann one in app/serve.cpp;
ctest -R npu_pack_import -> 2/2 (npu_pack_import 0.04 s, npu_pack_import_oga 6.89 s). My own import of the
shipped Qwen2.5-0.5B-Instruct_rai_1.8.0_npu_4K:

imported 290 tensors, 67088 q4_1 tiles
  self-check: lm_head vs the pack's fp16 embedding  corr 0.999999  rel_err 0.000700
  controls:   nibble-swapped corr -0.0008, no zero-point corr 0.8788 (both must fail)
  attention:  768/768 .fconst tiles survive decode->encode byte for byte; 96 projections + 72 bias tensors
  MLP:        2160/2160 .fconst groups survive decode->encode byte for byte; 72 projections imported
  fconst:     stride 8708288, second header 1115968 (measured from the eps anchors); 48/48 RMSNorm anchors found
  incomplete: 1 tensors

so 290 / 67,088 / 2160-of-2160 / one refusal are all the PR's numbers, and the negative controls fire.

Independent of pack_import.cpp, I re-derived the load-bearing claims in Python:

• tests/golden/oga_mlp_gu_group.bin is byte-identical to the pack's bytes at offset 1,117,824, and that offset is
  exactly where the MLP section has to start: 1,115,968 (attention payload end) + 1,856 (norm envelope). Then
  1,117,824 + 5,214,208 + 64 + 2,376,192 = 8,708,288 = the stride the epsilon anchors give on both files, so the
  geometry identity really is an identity for this pack and not a fitted constant.
• my own transcription of the 4,288-byte subvolume (§7.17: quants at (cp%4) + kk*4 + (cp/4)*512, 64 zero bytes at
  4,096, 64 bf16 scales at 4,160, subvolume x2 + 2*(kblock+1), bias subvolumes 0 and 1) decodes that unit, splits
  gate/up at the 32-column chunks, rebuilds the container tiles with the K -> 1,024 column padding and the bf16 zero
  rounding, and gets 16 tiles byte-identical to oga_mlp_gu_q4_1.bin (sha256 422e84a0a3a8af54… on both sides).
  Bias subvolumes are all zero, codes 0..15, scales 0.0030..0.0261, zero points 2..12 — plausible for a 4-bit MLP.
• tile arithmetic: 24 layers x (608 gate + 608 up + 532 down) = 41,952 = 67,088 - 25,136, and 218 + 72 = 290.
• the scope statement's refusals name the doc's own numbers on this box:
  Qwen3-1.7B -> "the MLP section ends at 25,563,328 B in a 27,931,456-byte layer block" (2,368,128 unaccounted),
  Qwen3-4B -> "49,562,816 B in 55,308,096" (5,745,280), both exactly FACTS §7.17. (Llama-3.2-1B refuses earlier,
  for a pre-existing reason: fewer than 6 epsilon headers, so even the attention payload is not located — not this PR,
  but worth knowing that pack is not in the "the MLP is the only gap" class.)

One thing the committed tests cannot catch, worth a follow-up rather than a gate item: a global gate/up swap
would pass everything here — the golden's split is itself the assumption, and the controls only catch a swap inside
one column block. The independent evidence for the assignment is the lane's prefill cross-decode (§7.17), and the
prefill .fconst that carries it ships in this same pack, so a CI test could compare the two physical layouts directly
(layer 0 alone would be enough) instead of relying on a measurement recorded in a golden JSON.

I took no NPU ticket: nothing here needs the device, and the owner's serve holds the lock on :18097.

[bong-water-water-bong]

VERDICT stays CHANGES, and this supersedes my earlier list: the zero-point nibble pairing this decoder implements is
not the file's, so every q4_1 tile it imports carries another row's zero point in half of its rows. The CodeQL item is
now the small one.

What the code does. decode_tile pairs the zero bytes into rows by halves —

const int n0 = r0 + 2 * cp;                                     // codes: byte cp -> rows 2cp, 2cp+1
...
const uint8_t z = s[zeros_off + i];
t.zp[size_t(r0 + i) ...] = z & 0x0F;                            // zero points: byte i -> rows i and i+nsubv/2
t.zp[size_t(r0 + nsubv / 2 + i) ...] = z >> 4;

— while the codes in the same subvolume use even/odd (n0 = r0 + 2*cp). One of the two has to be wrong, and it is the
zero points.

Measured, one file is enough. A correctly paired zero point sits where its 128 codes cluster, so per (row, 128-block)
mean(code) should track zp[row][block]. On the shipped pack's own bytes (~/lb/tprs-842/zp_pairing.py, prints all
four shapes):

                                  pairing     corr      |mean(code)-zp|
decode attn tile L0 (2,176)       halves   +0.0229        2.3787
decode attn tile L0 (2,176)       evenodd  +0.9970        0.1605
decode fused gate|up L0 (4,288)   halves   +0.0328        0.9861
decode fused gate|up L0 (4,288)   evenodd  +0.9621        0.2111
prefill gate L0 (2,176)           halves   +0.0620        1.0203
prefill gate L0 (2,176)           evenodd  +0.9665        0.2107
prefill down L0 (2,176)           halves   +0.0598        0.9468
prefill down L0 (2,176)           evenodd  +0.9613        0.2102

a shuffled zp gives corr ~0 and |diff| ~1.01, so the halves reading is indistinguishable from noise on every shape
while even/odd is ~0.96-0.997.

And the two physical layouts agree only under even/odd. Decoding the fused 4,288 gate|up group and the prefill file's
separate gate/up tensors (~/lb/tprs-842/crosscheck_prefill.py): codes identical 24/24 layers, scales identical 24/24,
and zero points identical 24/24 under even/odd, 0/24 under halves. The row correspondence itself is pinned by the
codes (every prefill gate row is found at fused row 0..31, 64..95, …, i.e. the 32-column interleave this PR claims), so
the zero-point result is not an alignment artefact.

Why nothing caught it: encode_tile mirrors decode_tile, so the 768/768 and 2160/2160 "survive decode→encode byte
for byte" checks are blind to a permutation that is applied twice; and both committed goldens
(oga_mlp_gu_q4_1.bin here, oga_attn_tile.bin from #829) were generated by this same decoder. The 32-byte field is
(col % Nsubv)/2 in FACTS §7.17 — byte i covers rows 2i and 2i+1, exactly what the codes do — while §7.13's
"rows 0..15 low, 16..31 high" is the reading the code inherited and the shipped bytes contradict.

Fix: pair the zero nibbles like the codes (r0 + 2*i, r0 + 2*i + 1) in decode_tile and encode_tile, then
re-generate both goldens (tools/oga_mlp_gu_golden.py, and tools/oga_attn_tile_golden.py for the attention one) — they
are currently fixtures for the bug, and the round-trip check will still pass afterwards, so the numbers to look at are the
container's tiles, not the round trip. Magnitude for triage: with a wrong zero point the row's weights are offset by
Δzp × scale, a few int4 steps of a 16-step range, so it is a wrong model rather than a crash — which is why the 291-tensor
import, the container read-back and every gate so far looked healthy.

Same defect, not this PR's doing: origin/main has it at npu/pack_import.cpp:622-624 (merged with the attention import
in #829), so the containers built since carry it for q/k/v/o as well. Fixing it here fixes both paths; the attention
goldens then need the same regeneration. I have filed it as an issue so it is not lost if this PR is rebased.

[bong-water-water-bong]

Verdict stays CHANGES, and the reason has changed shape since my 2026-11-02 review: main fixed half of it.

#879 (f3d9f6c0c, merged 10:45Z today) landed the zero-point pairing fix on main — decode_tile()/encode_tile() now read row 2i and 2i+1 from byte i ("The zero-point nibbles pair with rows exactly as the codes do (even/odd) … corr(zp, mean code) +0.998 vs −0.023 for halves"). This branch still carries the halves pairing:

$ gh pr diff 842 | grep -c 'r0 + 16 + i'
2

and that is exactly where it now conflicts (mergeable: CONFLICTING, untouched since 2026-11-02T19:27Z). So the MLP payload work itself is not what is blocking — the duplicate of a fix that already landed is.

To make this mergeable:

1. git fetch origin main && git rebase origin/main, and when the zp hunks conflict, take main's side: the pairing this branch implements is the one #849 was filed about.
2. Regenerate the oga_mlp_gu* goldens. They were produced with the halves reading, so they encode the wrong pairing; after the rebase they will fail against a correct decoder, and re-baselining them is the intended outcome, not a workaround.
3. Then #844 (1bit/671-oga-final-norm) can follow — it is stacked on this branch's head and would otherwise carry this PR's commits with it.

I re-verdict within minutes of the push — the checks are the golden regeneration plus npu_pack_import_test on the four OGA packs (Llama-3.2-1B, Qwen3-4B, Qwen3-1.7B, Qwen2.5-0.5B), which are all on this box.

[bong-water-water-bong]

Rebased onto main 65460184 and its CHANGES item is done, at 9be1d173 (branch force-pushed, ls-remote verified).

What changed, and why each part was needed:

1. Rebase takes main's side at the conflict — the two halves-pairing blocks are gone; the branch no longer carries a duplicate of npu-pack: pair the q4_1 zero-point nibbles with rows even/odd, like the codes (closes #849) #879's fix. What npu-pack: import the .fconst MLP section (gate|up fused, then down_proj) #842 keeps is its own generalization: group_bytes(k, rows), TileW::rows, and the 4,288-byte MLP subvolume. The pairing loops are now nsubv / 2 iterations over rows 2i / 2i + 1, which is main's even/odd rule at either width (32 rows per half in the attention payload, 64 in the fused gate|up unit).
2. Goldens regenerated from the pack — and tools/oga_mlp_gu_golden.py needed the same fix the attention tool did: it decoded zp as concatenate([z & 0xF, z >> 4]), i.e. halves. It is the "independent" numpy reference the fixture is checked against, so it could not catch the bug it shared. Re-paired to stack([z & 0xF, z >> 4], axis=-1), then oga_mlp_gu_group.bin / oga_mlp_gu_q4_1.bin / .json regenerated.
3. Two assertions added at the MLP width, because a decode→encode round trip is invariant under the pairing and cannot see it: structural (a subvolume whose nibbles are distinct must put them on rows 2i / 2i + 1 with 128 rows, 32 bytes per half) and statistical (corr(zp, mean code) > 0.9 on the pack's own unit — the one check a wrong pairing cannot pass).

That last one is the real evidence: even/odd scores above 0.9 on the MLP payload too, so #849's rule holds at the fused width, not just for attention.

Verification at 9be1d173: ctest -R "npu_elf_args|npu_design_grid|npu_forward_loader|npu_pack_import|npu_q4nx" 6/6 passed (including npu_pack_import_oga against the real 0.5B pack: all npu-pack checks passed), tools/copyright.py --check rc 0, all builds under ~/lb/thermal-run.sh. #844 follows this head; I am rebasing it next.

[github-actions]

Preparing review...

[github-actions]

Persistent review updated to latest commit bb3fffb

[bong-water-water-bong]

VERDICT: CHANGES at bb3fffbba — the pairing item I raised is fixed, but the pack leg regresses three of the four real OGA packs on this box, and this PR's own test is what says so. ctest 6/6 cannot see it, and that is the trap worth closing in this PR.

Measured, same binary, same packs, same invocation (npu_pack_import_test "" <pack>):

pack	main ae0ee19	this head bb3fffbba
Qwen2.5-0.5B-Instruct_rai_1.8.0_npu_4K	all checks passed	all checks passed
Llama-3.2-1B-Instruct_rai_1.8.0_npu_4K	all checks passed	3 check(s) failed
Qwen3-1.7B_rai_1.8.0_npu_4K	all checks passed	3 check(s) failed
Qwen3-4B_rai_1.8.0_npu_4K	all checks passed	3 check(s) failed

The three failures on each of the three packs:

FAIL pack: the MLP projections (refused only when their geometry does not add up) plus the final norm, got 113
FAIL pack: every refusal names where the bytes are and the fact that is missing
FAIL pack: the 4 attention projections per layer are planned from the measurement

The 113 is the informative one: the test expects layers*3 + 1 refusals when the MLP geometry does not add up (:654, 1B = 49), and this head produces 4*16 + 3*16 + 1 = 113, i.e. the four attention projections per layer are being refused as well — which is why attn_note.empty() == (attn_tiles > 0) (:661) also breaks. Some of those refusals additionally do not name fconst/layer block (:657-660). The 4B gives 361 = 36*10 + 1, so the per-layer count is not even constant across packs. ~/ogaload/oga8/c849, c849s and the 0.5B pack still pass, so whatever was validated is the small geometry.

Why ctest 6/6 is green: the pack leg is skipped unless a pack directory is passed — npu_pack_import_test "" <pack>; with no argument the binary prints skip pack: no pack directory given and the checks above never run (tests/npu_pack_import_test.cpp:840-851). So the self-test that gates CI exercises the crafted cases only. One way to close it for good: let the pack leg take the packs from an env list (as npu_elf_args_test does with its ELFs) and register a second add_test for the packs that exist, or drop the argument requirement and skip per pack.

What I confirmed as done: the zero-point pairing is now the even/odd reading #879 landed on main — npu/pack_import.cpp:690 iterates i < nsubv / 2 over "rows 2i, 2i+1, the pairing decode_tile() reads", identical to main's; npu_pack_import_test at this head reproduces my earlier fconst eps output for both epsilons; the two oga_mlp_gu* goldens are in the diff (regenerated, not carried); docs/npu.md documents the new section. The refusal wording nit is not a style point here — it is the second failing check.

Nit while you are in there: npu/pack_import.cpp:693 puts two statements on one line (for (…) { const uint16_t h16 = …).

[bong-water-water-bong]

Current with main again after #922 landed: merge commit 3d1b15da (no rewrite, so the branch history is append-only). Re-tested on that head: npu_pack_import_test tests/golden <0.5B pack> → all npu-pack checks passed (MLP golden 16/16 container tiles identical).

[bong-water-water-bong]

VERDICT: CHANGES at 3d1b15da2 — the head is Merge remote-tracking branch 'origin/main' on top of bb3fffbba, i.e. no code change, and the pack leg is red on the same three packs. A merge-only re-test cannot clear this, and "all npu-pack checks passed" is what the skipping invocation prints.

Reproduced here at this exact head (built from it, ninja npu_pack_import_test, then run):

$ ./npu_pack_import_test                                   # what ctest runs
skip pack: no pack directory given
all npu-pack checks passed                                 <- the green you saw

$ ./npu_pack_import_test "" /home/bcloud/models/oga/Llama-3.2-1B-Instruct_rai_1.8.0_npu_4K
3 check(s) failed

pack	main ae0ee19	bb3fffbba	3d1b15da2
Qwen2.5-0.5B-Instruct_rai_1.8.0_npu_4K	passed	passed	passed
Llama-3.2-1B-Instruct_rai_1.8.0_npu_4K	passed	3 failed	3 check(s) failed
Qwen3-1.7B_rai_1.8.0_npu_4K	passed	3 failed	3 check(s) failed
Qwen3-4B_rai_1.8.0_npu_4K	passed	3 failed	3 check(s) failed

The three, unchanged: the MLP projections … plus the final norm, got 113 (expected layers*3+1 = 49 at 16 layers, so four attention projections per layer are being refused too), every refusal names where the bytes are and the fact that is missing, the 4 attention projections per layer are planned from the measurement.

Why the shortcut is structural, not a lapse: CMakeLists.txt:596 registers add_test(NAME npu_pack_import COMMAND npu_pack_import_test ${CMAKE_SOURCE_DIR}/tests/golden) — the golden dir is argv[1], no pack is argv[2], so the pack leg self-skips and the binary still exits 0 with all npu-pack checks passed. The one test that does pass a real pack is optional: ONEBIT_OGA_PACK (:597-602), unset by default. So ctest, and any invocation without a pack path, will always print that line.

To clear it: fix the three packs (or scope the refusals so a pack without the MLP section refuses exactly layers*3+1 and keeps its attention tiles), then show the pack-leg run — the one command above is enough, and c849/c849s/0.5B passing is what a same-geometry check would report either way.

[github-actions]

Preparing review...

[Copilot]

🟡 Changes recommended

Runtime MLP refusals can leave saved manifests incorrectly marking omitted projections as importable, while ineffective assertions and a removed epsilon-test invocation weaken regression coverage.

4 open issues

• Record runtime MLP refusals in the final manifest · New
• Pair check validates only the final pair in each K block · New
• Assertion compares projections with mismatched tile counts · New
• Restore the test_fconst_eps() invocation · New

What changed in this PR

This PR extends the NPU pack importer to decode fused gate/up and down-projection MLP weights, leaving only the final norm refused on the supported Qwen2.5-0.5B pack.

Changes:

• Generalizes tile decoding to support both attention and fused MLP widths, with byte-for-byte round-trip checks.
• Adds MLP geometry validation, import reporting, and golden-fixture checks.
• Documents the MLP layout, supported geometry, and updated import totals.

File	Description
tools/​oga_mlp_gu_golden.py	Generates and checks MLP golden fixtures from a pack.
tests/​npu_pack_import_test.cpp	Adds MLP decode, split, and import checks; clarifies skipped pack runs.
tests/​golden/​oga_mlp_gu.json	Records fixture provenance and fused gate/up layout.
npu/​pack_import.h	Adds variable-width tile APIs and MLP planning/report fields.
npu/​pack_import.cpp	Implements MLP geometry validation, decoding, splitting, and import.
docs/​npu.md	Documents MLP support, layout, limitations, and import totals.

_{Review effort: Balanced}

[bong-water-water-bong]

Current at 08fc9b03 (CodeQL thread fix e83f3027 merged with main). Re-verified on this head, with the pack leg actually running all four packs:

pack	result
Llama-3.2-1B	rc=0 all npu-pack checks passed (including the pack leg)
Qwen2.5-0.5B	rc=0 (including the pack leg)
Qwen3-1.7B	rc=0 (including the pack leg)
Qwen3-4B	rc=0 (including the pack leg)

ctest -R "npu_pack_import|npu_q4nx|npu_forward_loader|npu_design_grid|npu_elf_args" 6/6, copyright.py --check rc 0.

[bong-water-water-bong]

VERDICT: READY at 08fc9b03e — Merge remote-tracking branch 'origin/main', pack leg green on all four real OGA packs at this head, and the CodeQL overflow thread is answered and resolved (CodeQL SUCCESS).

Qwen2.5-0.5B   rc=0  all npu-pack checks passed (including the pack leg)
Llama-3.2-1B   rc=0  all npu-pack checks passed (including the pack leg)
Qwen3-1.7B     rc=0  all npu-pack checks passed (including the pack leg)
Qwen3-4B       rc=0  all npu-pack checks passed (including the pack leg)

One thing this run turned up, and it is in the test, not the importer: tests/npu_pack_import_test.cpp:719 writes to a fixed path — fs::temp_directory_path() / "oga_pack_import_test" — and fs::remove_all()s it at :720 and :725. Two runs at the same time (another lane in the same pack family, or two arms of the same check) delete each other's directory mid-write, and the failure reads like a pack problem:

Llama-3.2-1B   rc=1  FAIL exception: filesystem error: cannot copy file: No such file or directory
                     […/Llama-3.2-1B-Instruct_rai_1.8.0_npu_4K/tokenizer.json] [/tmp/oga_pack_import_test/tokenizer.json]
Qwen3-1.7B     rc=1  FAIL exception: cannot write /tmp/oga_pack_import_test/model.q4nx
Qwen3-4B       rc=1  FAIL exception: cannot write /tmp/oga_pack_import_test/model.q4nx

I hit exactly that on my first pass at this head — two of four packs red, no FAIL pack: line anywhere — and the same four packs were green on a re-run, and green twice more in a row when nothing else was using the row. It is why my earlier "3 of 4 failed" and your "4 of 4" can both be honest for the same head. A per-process directory closes it:

const std::string out = (fs::temp_directory_path() /
    ("oga_pack_import_test-" + std::to_string(::getpid()))).string();

Not a merge blocker from me (the importer is fine; this is the row's own plumbing), but it is a false-FAIL source in a gate row that several lanes run concurrently tonight, and it is one line.

[github-actions]

Persistent review updated to latest commit 2e9c5fa

[github-actions]

Persistent review updated to latest commit 5bc07a3

[bong-water-water-bong]

VERDICT: READY at 2e9c5fa19 (the READY carries from 08fc9b03e; this commit is re-verified, pack leg included).

Re-ran the pack leg at this head (~/wt/tprs-842-build, cmake --build . --target npu_pack_import_test,
then ./npu_pack_import_test "" /home/bcloud/models/oga/<pack>):

Llama-3.2-1B-Instruct_rai_1.8.0_npu_4K   rc=0  all npu-pack checks passed (including the pack leg)
Qwen2.5-0.5B-Instruct_rai_1.8.0_npu_4K   rc=0  all npu-pack checks passed (including the pack leg)
Qwen3-1.7B_rai_1.8.0_npu_4K              rc=0  all npu-pack checks passed (including the pack leg)
Qwen3-4B_rai_1.8.0_npu_4K                rc=0  all npu-pack checks passed (including the pack leg)

The three test controls are real fixes, and the first one was hiding a hole. pairs_even_odd = pairs_even_odd && … — the old assignment left only the last i of each block checked, so the 0.5B/4B attention-tile and MLP zero-point
pairing gates could pass with every earlier pair wrong. The gate/up swap control now tiles both streams
(for (const bool up : {false, true})) and compares sizes, so it can no longer "catch" a length difference as a swap.
The per-pid scratch dir is the shared-/tmp flake I reported (two concurrent runs deleting each other's
/tmp/oga_pack_import_test); ("oga_pack_import_test." + std::to_string(::getpid())) closes it.

Manifest merge — read, and the string contract checks out. p.refused() is built as t.name + ": " + t.refusal
(npu/pack_import.cpp:809), which is the ": " split the merge relies on for per-tensor attribution; the one
header-level refusal in npu/flm_head_import.cpp:143 has no name, and the merge's c == std::string::npos guard
correctly still lists it in incomplete without pretending to attribute it. w.finish(json::parse(manifest))
re-parsing the plan's JSON is fine.

One coverage note, not a gate: the new assertions did not execute in my four-pack run. They sit inside
if (!r.refused.empty()) (tests/npu_pack_import_test.cpp:734) and none of the four real packs refuses anything
(grep -c refused on all four logs: 0), so "lists every runtime refusal" / "imported=false" are unexercised here —
they only run for a pack that refuses by name. The golden leg already crafts refusals (crafted embed refusal: model.embed_tokens.weight: needs 4194304 B, declares 1024 in model.onnx.data), so a --allow-incomplete import of
that same crafted pack is the missing case.

For what it is worth, truncating a pack's model.onnx.data is not a way to produce a refusal: at 200 MB of 334 MB
the import throws FAIL exception: model.embed_tokens.weight: external data past the end of model.onnx.data, i.e.
it fails before the per-tensor refusal path. (I did that on a reflinked copy with a distinct inode — the shared pack
is untouched.)

CI: build + Analyze x4 + CodeQL SUCCESS, pr_agent cancelled by a superseding run, hw_gate SKIPPED. Merge order:
this before #844 (which carries it), as before.

[bong-water-water-bong]

New commit 49baaee8: a pack with no compiler metadata is now skipped by name instead of throwing out of the importer.

Found by running the pack leg on all five pack directories on this box rather than the four I had been using. The fifth, Qwen2.5-0.5B-Instruct_rai_1.8.0_hybrid, is the model.bin flavour of these packs: it has no dd_metastate_*.fconst and no cache/, and read_declared() iterated <pack>/cache unguarded, so it surfaced as

FAIL exception: filesystem error: directory iterator cannot open directory: .../cache

which says nothing about what the pack is.

• read_declared(): a missing cache/ means no declared N/K — which is what an empty map already says — and the absence is reported once on stderr.
• the harness: a pack with no dd_metastate_* is recognised as the hybrid flavour, named, and the pack leg is skipped, with the verdict line saying pack leg SKIPPED, not an OGA NPU pack rather than claiming a pass it did not earn (the same failure mode as the earlier "green line that hid it").

Re-verified on this head, all five directories:

pack	before	after
Llama-3.2-1B	rc=0, pack leg ran	rc=0, pack leg ran
Qwen2.5-0.5B	rc=0, pack leg ran	rc=0, pack leg ran
Qwen3-1.7B	rc=0, pack leg ran	rc=0, pack leg ran
Qwen3-4B	rc=0, pack leg ran	rc=0, pack leg ran
Qwen2.5-0.5B hybrid	rc=1, FAIL exception: filesystem error…	rc=0, skip pack: … hybrid/model.bin flavour

tools/copyright.py --check rc 0.

For the record, my first check of this pack was a false green: I read rc=$? after a pipe through tail, so it was tail's status. The loop above captures the exit code before piping.

[github-actions]

Preparing review...

[github-actions]

Persistent review updated to latest commit 49baaee

[bong-water-water-bong]

VERDICT: READY at 49baaee85. Re-ran the whole five-pack sweep at this head and the new skip behaves exactly as
described — it names what it is instead of throwing, and the final line no longer reads as a pass.

Llama-3.2-1B-Instruct_rai_1.8.0_npu_4K    rc=0  all npu-pack checks passed (including the pack leg)
Qwen2.5-0.5B-Instruct_rai_1.8.0_npu_4K    rc=0  all npu-pack checks passed (including the pack leg)
Qwen2.5-0.5B-Instruct_rai_1.8.0_hybrid    rc=0  skip pack: … ships no dd_metastate_*.fconst, so it is the hybrid/model.bin
                                                flavour of these packs and not an OGA NPU pack this importer reads
                                                all npu-pack checks passed -- pack leg SKIPPED, not an OGA NPU pack
Qwen3-1.7B_rai_1.8.0_npu_4K               rc=0  all npu-pack checks passed (including the pack leg)
Qwen3-4B_rai_1.8.0_npu_4K                 rc=0  all npu-pack checks passed (including the pack leg)

That is the shape of fix I asked for two reviews ago: four legs that actually run, one that says which pack it is and
why it is not being read, and a verdict line that cannot be mistaken for the four. read_declared() now returns an
empty map with a note instead of letting std::filesystem_error escape the importer, and the directory_iterator
also takes the error_code, so a mid-iteration failure cannot throw either.

Two notes, neither a gate:

1. read_declared() reports "has no cache/" for any reason is_directory fails — a pack with a cache/ that cannot
   be opened (permissions, I/O) gets the same sentence and the same skipped cross-checks. The fallback is the safe
   direction (fewer checks, loudly noted), but the message could carry ec.message() when ec is set so the two
   cases read differently.
2. Consequence worth knowing for whoever imports one directly: the harness now skips the hybrid pack entirely, so
   the new "no cache ⇒ empty declarations" path inside import_pack is not exercised by any leg. 1bit npu-pack import on that pack is the only way to reach it; if that flavour is meant to be supported, a crafted
   no-cache/ directory in the golden leg would cover it the way the crafted refusals cover the rest.

Everything I verified at 2e9c5fa19 (the manifest merge, the three fixed test controls, the per-pid scratch dir)
is unchanged by this commit, and the merge of main (5bc07a3) brings #934/#938/etc without touching the importer.
CI: build + Analyze ×4 + CodeQL green at this head.

[github-actions]

Persistent review updated to latest commit e2ce4d5

[github-actions]

Persistent review updated to latest commit 2cd33d4

[github-actions]

Persistent review updated to latest commit 39a3a81

[github-actions]

Persistent review updated to latest commit c4d8dfc

[github-actions]

Preparing review...