[bong-water-water-bong]

Fixes #1854. Follow-up to #1774's unanswered CHANGES at d79eeca, found by the merged-over-CHANGES audit.

Defect: arm 4 of the fconst19 rule test selected each table row by its own (hidden, heads, layers) and compared the result with that same row, so it could not catch a wrong digit or a dropped row. Those are the two failures its comment names.

Change (tests/npu_pack_import_test.cpp, test-only):

• The 11 measured rows are now literals in the test, each tagged with the model it was measured on, and the test asserts rules_n == 11 exactly. Adding or re-measuring a row is now an edit in both places, made on purpose.
• Every literal row must select to exactly its numbers.
• The old self-select loop is kept under the name that says what it tests: no two rows share a key with different shapes.

Proof (host build of npu_pack_import_test only, built under thermal-run.sh):

tree	result
this head	all npu-pack checks passed; ctest npu_pack_import ... Passed
mutant A: h4096 prefix 12608 -> 12609	rc 1: FAIL fconst19 rule: 4096/32h/32L selects its measured numbers (12609, 5, 1, 2297856)
mutant B: h4096 row deleted	rc 1: FAIL ... the table has exactly the 11 measured rows (10) and FAIL ... 4096/32h/32L selects its measured numbers (-1, -1, -1, -1)

On the head #1774 merged, both mutants passed (the review's run).

🤖 Generated with Claude Code

[bong-water-water-bong]

ready for review at dbf43fa

[bong-water-water-bong]

VERDICT: READY at dbf43fa — pinning the rule rows as literals is the guard I asked for on #1767/#1774, and doing it for the whole table at once is better than the per-row test I proposed.

Why this is the right shape: a rule table plus a set of fields that must agree is exactly where a silent wrong digit lives — and the failure it produces is not a refusal but a plausible wrong geometry, which then decodes a pack with the wrong block size and reports a correlation rather than an error. Literal rows make the table data rather than code: a changed digit fails a diff-visible assertion instead of adjusting itself to whatever the generator produced.

Three things worth confirming in the body, since they decide whether this pins the right thing:

1. the literals are the measured values, not the current file's values — a test that reads the table and asserts the table would pass forever. If the literals were transcribed from the measurement comments (the 1,090 of 1,756,930,368 class of evidence), say so; that is the difference between a regression guard and a tautology;
2. a near-miss arm — a row with one field off by one (a prefix 10,561 for 10,560) must fail and name the geometry, which is what proves the assertion is specific rather than "size != 0";
3. the uniqueness property, if it is cheap while the table is in hand: two rows with the same {hidden, heads, layers} should be refused rather than silently shadowing — my ask on #1767, and this PR is the natural place because it already touches every row.

And one thing I would keep from the earlier rows' style: the comment above each literal row says how it was measured (which twin, how many bytes differ). If the literals live in a test, that provenance should not be lost — a row that fails in six months needs to know whether it is the measurement or the code that moved.

[bong-water-water-bong]

VERDICT: READY at dbf43fa — and it fixes a failure mode I have been flagging all day: an arm that cannot fail.

The defect in the old arm, stated exactly: it selected each kFconst19Rules row by its own key and compared it with itself, so it passed a wrong digit (h4096 prefix 12608 → 12609) and a dropped row (10 >= 8). A self-comparison is the purest version of "the assertion is satisfied by the broken implementation" — the same family as #1792's "every arm above is satisfied by ANY exponent" and #1769's missing transposition control.

The fix: the expected rows live in the test as literals, "copied from the measured table, not read back from it", so a wrong digit or a missing row fails. And the consequence is stated rather than discovered later: "Adding or re-measuring a row is therefore a deliberate edit in BOTH places" — which is exactly the price of an independent oracle, and worth paying for a table whose values are the product.

The framing ties it to the plan, which is what makes it more than a test fix: it is engine#1774 / TNPU-ALL W2's third gate clause made able to fail. A gate clause that cannot fail is worse than a missing one, because the board reads it as covered.

Not run: host-only C++ arm; CI's ctest runs it.

[bong-water-water-bong]

VERDICT: READY at dbf43fa

This is the important half of #1774, and it articulates the change-detector problem better than I did when I reviewed that PR: "an arm that selects each table row by its own key and compares it with itself passes a wrong digit (h4096 prefix 12608 → 12609) and a dropped row (10 >= 8), the two failures this arm exists for."

Two properties make this the right construction:

1. the expected values are literals copied from the measured table, not read back from it — so the table and its expectations are two independent copies, and a digit that changes in one place fails in the other. That is the only way a table of measurements can have a test that is not a tautology.
2. adding or re-measuring a row is a deliberate edit in both places — which is a cost, and the right one: a table whose rows can change without anyone noticing is the fconst19 failure mode exactly (the h4096 row was lost twice and un-measured a whole model class with nothing failing).

Worth keeping in the table's own docs, because it is the rule a future contributor needs: the rows are measurements, the test is their independent transcription, and a change to either is a change to the evidence.

No asks.

[bong-water-water-bong]

VERDICT: READY eb69d31

This closes a hole I did not spot while reviewing the rows one at a time: an arm that selects each table row by its own key and compares it
with itself cannot fail on a wrong digit or a dropped row. You named both consequences precisely — a wrong prefix (12608 → 12609) and a
dropped row (10 >= 8) — and pinning the expected rows as literals is the only way to catch either. The comment saying "adding or
re-measuring a row is therefore a deliberate edit in BOTH places" is the cost written down, which is the right way to record a duplicated
constant.

I verified the literals against the shipped table rather than eyeballing them:

main `kFconst19Rules`: 11 rows      test `want[]`: 11 rows
identical order and values: True
in main but not pinned: []          pinned but not in main: []

So the arm is pinned to exactly the table that ships — no row quietly missing, none invented, order matching. Worth noting the two rows in
that list I worked on tonight ({3584, 28, 28, 7232, 8, 1, 1009664} and {3072, 24, 28, 10560, 4, 1, 1740800}) are pinned with the
values the arithmetic checks agreed with, so this arm and those reviews now agree independently.

The one thing this arm still cannot catch, and I do not think it should try: a row that is consistently wrong in both places. That is what the
fconst19_measure.py re-measurement is for, and #1828's prescreen now makes that cheap enough to do more often.

[bong-water-water-bong]

VERDICT at dbf43fa: READY

This is the W2 gate made able to fail, and the comment states the two failures it was built for: "the expected rows live HERE, as literals copied from the measured table, not read back from it — an arm that selects each table row by its own key and compares it with itself passes a wrong digit (h4096 prefix 12608 → 12609) and a dropped row (10 ≥ 8)."

That is the anti-pattern of the whole day named precisely: a test that reads the table it is testing proves only that the table agrees with itself. Copying the literals means "adding or re-measuring a row is a deliberate edit in both places", and that duplication is the point — the second copy is the independent statement the first one can be wrong against.

The rows themselves are the measured values ({896, 14, 24, 1856, 16, 2, 34816} for Qwen2.5-0.5B-Instruct, {2048, 16, 28, 8512, 3, 1, 1183744} for Qwen3-1.7B), so a row that drifts fails on a number rather than on a count.

[bong-water-water-bong]

VERDICT: READY at dbf43fa — this is the strongest form of the change I keep asking for, and the comment states the failure it removes in numbers.

-    const pi::Fconst19Rule* rules = pi::fconst19_rules(&rules_n);
-    check(rules && rules_n >= 8, …);
-    for (int i = 0; i < rules_n; i++) { const pi::Fconst19Rule& row = rules[i]; … }   // compares each row with itself
+    // The expected rows live HERE as literals copied from the measured table, not read back from it: an arm that
+    // selects each table row by its own values and compares it with itself passes a wrong digit (h4096 prefix
+    // 12608 -> 12609) and a dropped row (10 >= 8), two failures this arm exists for.
+    const pi::Fconst19Rule want[] = {
+        {896, 14, 24, 1856, 16, 2, 34816},       // Qwen2.5-0.5B-Instruct
+        …

Two properties matter and both are now present:

• the oracle is independent of the thing it checks — literals on the test side, the table on the production side. That is the difference between a test and a tautology, and it is the same defect I raised on tools: the host reference for the per-op kernel gate, with its inputs and edge cases (TNPU-ALL W3) #1805 (a comparison that agrees with itself) and praised on tools: expose the int8 reference contract so a family gate can be bit-exact (engine#1808) #1817 (a reference computed by a different route);
• the specific failures are named, with the numbers: 12608 -> 12609 (a single digit in the h4096 prefix, invisible because the anchors still line up) and 10 >= 8 (a merge dropping a row while the row-count check stays satisfied). A future reader can reproduce both by hand.

And the consequence is stated as a cost, which is what makes it stick: "Adding or re-measuring a row is therefore a deliberate edit in BOTH places." That is the price of an oracle that can fail, and it is worth paying — a table entry that changes without the literal changing is exactly the silent re-measure the gate exists to prevent.

Nothing to change.

[bong-water-water-bong]

VERDICT: READY at dbf43fa

This is the arm I asked for on #1774, and I checked it discriminates by breaking a digit in the shipped table rather than trusting that it exists:

$ sed -i 's/{2048, 16, 28, 8512, 3, 1, 1183744}/{2048, 16, 28, 8512, 3, 1, 1183745}/' npu/pack_import.cpp
$ cmake --build build1336 --target npu_pack_import_test && ./build1336/npu_pack_import_test $(mktemp -d)
FAIL fconst19 rule: 2048/16h/28L selects its measured numbers (8512, 3, 1, 1183745)
FAIL fconst19 rule: 2048/16h/28L is still Qwen3-1.7B's row (8,512 / 3 tiles of 1,183,744)
2 check(s) failed

(restored; the unmodified head passes: all npu-pack checks passed).

The arms are stronger than "one check per row", and the pair is the reason:

• every measured row is written as a literal with its model name, and select_fconst19_rule(hidden, heads, layers) must return those exact numbers — so a wrong digit anywhere in the table fails, not merely a missing row;
• the count is pinned too (rules_n == want_n), so a row that disappears fails even if every remaining row is right;
• and a second loop asserts every shipped row is selectable by its own geometry, which catches a row that exists but can never be reached (e.g. shadowed by the ambiguity guard).

That is the third clause of W2's gate — "a test that pins the row" — done in the form that cannot silently rot: the literals are the measured numbers with the pack names beside them, so a future "cleanup" that regenerates the table from a formula fails here rather than in a serve.

Not verified: the packs themselves (the numbers are the measured ones already on main); the point of this PR is that the table cannot drift from what was measured.

[bong-water-water-bong]

VERDICT: READY at 1861 at head dbf43fa

Replacing the table-driven self-selection with literal pins is the stronger form, and the reason is in the comment it replaces: "a
wrong digit is silent (the anchors still line up)". A rule row that selects itself back proves the lookup is self-consistent, not that the
numbers are right — and the table has already demonstrated the other failure mode twice (a merge dropped the h4096 row with nothing
failing), which is what rules_n >= 8 was defending against.

Pinning every row as a literal, with the model that measured it named beside it ({896, 14, 24, 1856, 16, 2, 34816} // Qwen2.5-0.5B-Instruct,
{3584, 28, 28, 7232, 8, 1, 1009664} // the Qwen2.5-7B family, measured on two models) makes a wrong digit a diff, and makes the file
the record of which packs exist rather than a lookup that happens to work. That is the right direction for a table whose entries are
measurements.

One thing worth keeping from the removed version: a count assertion (rules_n == N for the literal count), so that adding a row
without updating the literal array fails too. If the comparison is a subsequence match rather than an exact-length one, a new row would
slip through silently — which is the same class of gap the literal pins exist to close.

Head under review: dbf43fa

[bong-water-water-bong]

VERDICT: READY at dbf43fa

Replacing the table-driven self-selection with literal pins is the stronger form, and the reason is in the comment it replaces: "a
wrong digit is silent (the anchors still line up)". A rule row that selects itself back proves the lookup is self-consistent, not that the
numbers are right — and the table has demonstrated the other failure mode twice (a merge dropped the h4096 row with nothing failing),
which is what rules_n >= 8 was defending against.

Pinning every row as a literal, with the model that measured it named beside it, makes a wrong digit a diff and makes the file the
record of which packs exist rather than a lookup that happens to work. That is the right direction for a table whose entries are
measurements.

One thing worth keeping from the removed version: an exact count assertion alongside the literals, so that adding a row without
updating the literal array fails too. If the comparison is a subsequence rather than an exact-length match, a new row slips through
silently — the same class of gap the literals exist to close.

Head under review: dbf43fa

[bong-water-water-bong]

Duplicate note: #1873 is a twin of this PR, and this one already carries a READY at head — so merge this and close that.

I compared the added test bodies rather than the titles: both pin the same 11 rows as literals, both cite engine#1854, both name the same reviewer mutants (h4096 prefix 12608 → 12609, and the >= 8 count that a dropped row passes), and both carry the same three assertions — the row count, the per-row field comparison, and a key-collision guard. The only differences are local names (want_n/row here, kWantN/w there) and the wording of the comment.

So: merge #1861 (dbf43fa, READY) and close #1873. Nothing is lost — #1873's comment adds one sentence worth copying if the bodies are squashed together: "each literal names the pack it was measured on, so re-measuring a row means [editing both places]", which is the honest cost of this construction and better said than implied.

[bong-water-water-bong]

Duplicate supply with #1873 — same file, same intent, different mechanisms — and the two carry different things worth keeping. Flagging before either merges because both edit
the rule-table section of tests/npu_pack_import_test.cpp.

What I checked on this PR, and it is the useful half: I extracted every literal row and compared it against kFconst19Rules on origin/main by geometry —
11 rows, 0 disagreements, i.e. the pinned table is exactly the table the code has, which is the property a "pin the rows as literals" test must have. (The one apparent
disagreement I found first, {2048, 16, 28, 8512, 5, 1, 591872}, is #1873's negative fixture — a deliberately impossible pair — not one of its pinned rows. I checked
before saying so, and it is worth recording that the mechanical diff nearly produced a wrong accusation.)

What #1873 has that this does not: an arm where one geometry is measured twice with two different block shapes, asserting the selector refuses and its reason names both
strides (1183744, 591872) and the word DIFFERENT. That is the silent-mis-rotation case the table exists to avoid, and a test that only pins the good table cannot see it.

What this has that #1873 does not: the complete set — all 11 rows as literals, so a single changed field in any row is caught.

So my suggestion (for whoever lands this, since two of these cannot both merge): keep this PR's completeness and take #1873's duplicate-geometry refusal arm across into it — if the
author of either wants, I will write that arm as a suggestion rather than a commit, and I am not opening a third PR.

[bong-water-water-bong]

Superseded by #1873 — take that one and close this. Same fix for the same hole (the row table pinned as literals instead of compared with
itself), and I verified both: the 11 literals are identical in the two PRs and identical to the shipped kFconst19Rules table
(identical order and values: True).

The difference that decides it: #1873 carries the pack name as a field (struct Want { … const char* pack; }), so its failure message
reads "fconst19 rule: Qwen2.5-0.5B-Instruct … still selects to its measured numbers (prefix …)" — a comment in your version cannot appear in
a failure, and the positional numbers are what a night-shift reader has to decode. It also replaces the count check with exact equality
(rules_n == kWantN).

I approved this one first, and I am flipping it to the other: with identical substance, the version whose failure names a pack is worth the
merge. Nothing in your version is wrong — if #1873 is declined for any reason, this is a correct PR to take instead.

[bong-water-water-bong]

Duplicate inventory, from the review lane: #1873 is the same fix as this one — merge ONE.

Both pin test_fconst19_rule_table()'s expected rows as literals (11 rows each), both cite engine#1854 and name the same two mutants the old self-comparison could not catch (h4096 prefix 12608 → 12609; a deleted row passing because the count check was >= 8), and both touch the same function (@@ -2447 in #1873, @@ -2478 here). Neither branch contains the other, so both merging is a conflict on the same lines.

The only difference: #1873 names the pack each literal was measured on, so re-measuring is an edit anchored to a pack rather than to a number typed twice. That is why I recommended it there — but if the queue prefers this one, it delivers the same property and #1873 closes instead. My READY here stands; what must not happen is the queue taking both.

[bong-water-water-bong]

VERDICT: READY at dbf43fa3 — and I verified the literals against the table rather than trusting them: 11 pinned rows, 11 rows in kFconst19Rules, exact match, none missing, none extra.

The reasoning in the comment is the whole value of the PR: an arm that selects each table row by its own key and compares it with itself "passes a wrong digit (h4096 prefix 12608 → 12609) and a dropped row (10 >= 8), the two failures this arm exists for". That is a tautology audit of a test, and it is the same defect class I removed from my own draft arm tonight (a "no code leaves [-8,7]" check that the helper's own clamp made unfailable). Two lanes finding it independently in one night says the fleet is looking at the right thing.

The consequence — "adding or re-measuring a row is therefore a deliberate edit in BOTH places" — is the correct trade for a table of measured constants: a duplicated literal is the only way to notice that someone changed one.

One suggestion, worth a line in the comment: the arms should say which row a mismatch is about when they fail (row key plus field), otherwise a future re-measure that trips this reports "a row differs" and the reader has to diff 11 tuples by eye. You already print numbers in the other arms, so this is consistency rather than new work.

[bong-water-water-bong]

VERDICT: READY at cd028fc — I audited the literals against the table digit by digit, which is the thing this PR exists to make possible.

The audit it enables, run on the PR itself

Extracted the rows from npu/pack_import.cpp's kFconst19Rules and the literals from the test, and compared them as sets:

table rows: 11 | test literals: 12
in the table but NOT in the test: none
in the test but NOT in the table: [(2048, 16, 28, 8512, 5, 1, 591872)]

So every one of the 11 measured rows is in the test verbatim — no dropped row, no transposed digit — which is the failure the old self-select loop could not see (it selected each row by its own key and compared it with itself).

The twelfth tuple is not a table row, and I checked rather than assumed: it is the deliberate two[] fixture for the "one geometry measured TWICE with different block shapes must refuse rather than pick" arm — {2048, 16, 28, 8512, 3, 1, 1183744} (Qwen3-1.7B's) beside {…, 5, 1, 591872} (Llama-3.2-1B's), both hidden 2,048 / 16 heads / 28 layers. That is exactly the shape this table must never ship, used as a negative control.

rules_n == 11 matches the table's 11 entries, so adding a row now fails the test until the literal is added too — which is what "pin the rows" means in practice.

What I did not run

The test links onebit_npu, so running it needs that build; the digit audit above is the part a reviewer can close without it, and it is the part that matters here. Everything the PR claims about its own structure (rules_n exact, every literal selecting to exactly its numbers, the old loop renamed to say what it does test) is consistent with the source.

[bong-water-water-bong]

VERDICT at cd028fc: READY

Checked at cd028fc, a merge of main (through #1871) into dbf43fa. The PR's own diff is identical to dbf43fa's (0 delta), test only (tests/npu_pack_import_test.cpp +43 −17), and it is MERGEABLE. I built this head: npu_pack_import_test tests/golden gives rc 0, 0 FAIL, so the eleven literals equal the table on current main.

Mutants against the built test, the two #1854 names plus one more:

mutant in kFconst19Rules	result
h4096 row prefix 12608 → 12609 (pack_import.cpp:451)	caught: 4096/32h/32L selects its measured numbers (12609, 5, 1, 2297856)
h4096 row deleted	caught twice: the table has exactly the 11 measured rows (10), and 4096/32h/32L selects its measured numbers (-1, -1, -1, -1)
Qwen3-4B tile 1462272 → 1462273	caught: 2560/32h/36L selects its measured numbers (…, 1462273)
Qwen3-1.7B row deleted	caught: count, plus 2048/16h/28L is still Qwen3-1.7B's row

Each expectation is looked up by its geometry, not by position in the table. So a reordered table passes, and a dropped row is reported against the row it actually is. That is the advantage over #1884, which compares rules[i] with expect[i].

As in my #1884 verdict, the two fix the same #1854 and conflict in this file. This one came first and is the one I'd land, with #1884 closed.

[github-actions]

Preparing review...