[bong-water-water-bong]

Refs #1990, #2073, #799.

Draft because it is stacked: this branch is #2042, then #2074, then two commits of its own (6794ee9 and 4315c23). Review those two; I rebase and mark it ready as the PRs under it merge.

What

#2042 and #2074 let a pack name a wide prefill set and a narrow decode set in config.json. The 16-row sealed set, though, comes from plan.json when a plan is loaded, so a planned pack described two of its three kernel sets outside its plan. This closes that:

plan field	meaning	refused when
prefill.sealed_elf, prefill.sealed_rows	the wide family and its rows (17..128)	no decode.sealed_elf; a path instead of a bare name; rows missing or out of range; rows without a file
decode.narrow_elf, decode.narrow_rows	the narrow family and its rows (1..15)	the same four conditions

• A loaded plan wins over config.json; NPU_WIDE_SEALED_ELF / NPU_NARROW_SEALED_ELF still override both, and =0 switches a family off.
• tools/npu_plan.py copies the config keys into the plan, each layout reading only its own key, and writes nothing new for a pack without them (asserted in the test).
• All four fields are optional, so every existing plan loads and means what it meant.

Measured (this box, one npu-hold ticket, one binary)

A scratch pack that links the served bonsai2-27b-t2s2 files and names both families only in plan.json (its config.json has neither key; the served pack was not touched). Arms: both off by environment / plan / off / plan.

OFF1:  prompt 536 tok, first token 30.28 tok/s; decode 3.37 tok/s (49 tok in 14.953s); sha256=b00af5b982ec1083
PLAN1: prompt 536 tok, first token 36.29 tok/s; decode 4.29 tok/s (49 tok in 11.837s); sha256=b00af5b982ec1083
OFF2:  prompt 536 tok, first token 30.32 tok/s; decode 3.39 tok/s (49 tok in 14.864s); sha256=b00af5b982ec1083
PLAN2: prompt 536 tok, first token 35.94 tok/s; decode 4.28 tok/s (49 tok in 11.854s); sha256=b00af5b982ec1083
amdxdna ERROR count: 2561 -> 2561
[forward] wide family: 8 role(s) at M=32 from .../pack/kernels-m16/SEALED-T2S-m32.elf (chunks of 17..32 rows)
[forward] narrow family: 8 role(s) at M=4 from .../pack/kernels-m16/SEALED-T2S-m4.elf (launches of 1..4 rows)
NPU_BATCH_CHECK=1 (plan arm): 25 [batchcheck] lines, 25 MATCH (M=32 x16, M=23, M=6, M=4 x3, M=1 x4)

Decode is (tokens - 1) / (wall - wall of a 1-token request of another short prompt); the first token is prompt_tokens / wall at max_tokens 1.

Tests (no device)

• npu_plan_test: two new functions, npu_plan: ok, rc 0 (load, edges 17 / 1 / 15, and each refusal by its message).
• tests/npu_plan_generate_test.py: npu_plan_generate: ok, rc 0 (keys carried, default rows, custom rows, no cross-layout inheritance, unchanged block without keys).
• tools/copyright.py --check and tests_referenced_check.py rc 0.

Not covered

• No mutant run on the plan checks this time; the refusal arms assert on message text.
• The plan route was measured in one ticket (two pairs), not two.

🤖 Generated with Claude Code

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 0ff30df)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1990 - Partially compliant Compliant requirements: The prefill tok/s on the served route, measured before and after under npu-hold Non-compliant requirements: The host terms of a prefill block, largest first, each with a before/after measurement A wider-M launch geometry for the served route, measured in tok/s Requires further human verification: The host terms of a prefill block, largest first, each with a before/after measurement A wider-M launch geometry for the served route, measured in tok/s 2073 - Partially compliant Compliant requirements: Decode tok/s on the served route, arms 16-row / narrow / 16-row / narrow in one ticket, text identical The sequential oracle (NPU_BATCH_CHECK=1) matches on the short chunks that take the narrow kernel The selection rules pinned by a host test Non-compliant requirements: None Requires further human verification: None 799 - Partially compliant Compliant requirements: One model description (plan.json), t2 ternary chunks, NpuTarget per NPU (XDNA, Hexagon) The plan layer: one model description, many NPUs (owner 2026-11-01 ~23:25 AST: "Plan it out in detail and build") The plan.json schema + loader + generator (tools/npu_plan.py) The NpuModel: the math, calls an NpuTarget for tensor ops; host fallbacks The NpuTarget interface; xdna/ (wraps I8Ctx + npu_rt_api); hexagon/ (new) The t2 chunk kind: spec in docs/npu.md, q4nx.h dequant, converters (pb16i-exact -> t2, GGUF TQ/PTQ -> t2), oracle test bit-exact, size table The t2 on-core expansion kernel for the sealed M=16 family (XDNA, private npu-kernels) + target wiring; measured GB/s + tok/s Extract NpuTarget + NpuModel; xdna target wraps I8Ctx; forward becomes NpuModel(plan, target); flag --npu-target xdna default; host fallbacks for optional ops Non-compliant requirements: Hexagon target: gemv first (t2, HVX/HMX), QEMU v73 bit-exact vs oracle, AI Hub X Elite timing; then gdn/attn optional ops Drafter protocol shared: tau-gated chain, 4-row verify + leaf fill, deferred GDN commit in NpuModel; acceptance numbers per target in docs/npu-leaderboard.md int8 KV + per-head Hadamard (plan kv.dtype), prefill chunks on the NPU with block-fp expansion (onwx class 110-150 tok/s) Requires further human verification: Hexagon target: gemv first (t2, HVX/HMX), QEMU v73 bit-exact vs oracle, AI Hub X Elite timing; then gdn/attn optional ops Drafter protocol shared: tau-gated chain, 4-row verify + leaf fill, deferred GDN commit in NpuModel; acceptance numbers per target in docs/npu-leaderboard.md int8 KV + per-head Hadamard (plan kv.dtype), prefill chunks on the NPU with block-fp expansion (onwx class 110-150 tok/s)

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Plan validation for wide family The validation logic for the wide family in the plan may not correctly enforce that the sealed_wide_elf is a bare file name in the kernels directory. If a path with / or .. is provided, it should be refused, but the check may not be robust enough to catch all edge cases. if (p.sealed_wide_elf.find('/') != std::string::npos || p.sealed_wide_elf.find("..") != std::string::npos) throw PlanError("plan: " + path + " prefill.sealed_elf \"" + p.sealed_wide_elf + "\" must be a bare file name in the kernels directory"); Plan validation for narrow family The validation logic for the narrow family in the plan may not correctly enforce that the sealed_narrow_elf is a bare file name in the kernels directory. If a path with / or .. is provided, it should be refused, but the check may not be robust enough to catch all edge cases. if (p.sealed_narrow_elf.find('/') != std::string::npos || p.sealed_narrow_elf.find("..") != std::string::npos) throw PlanError("plan: " + path + " decode.narrow_elf \"" + p.sealed_narrow_elf +

✅ Resolved findings

npu/forward/src/q4nx_forward.cpp:638-655

Wide family adoption logic

The logic for adopting the wide family in open_decode_family_local may not correctly handle cases where the wide family is not complete. Specifically, if a role has a decode context but no corresponding wide context, the wide family adoption is dropped, but the error message may not be sufficiently clear to indicate that the adoption failed due to an incomplete family.

npu/target/xdna/xdna_target.cpp:380-385

Narrow family adoption logic

The adoption logic for the narrow family in adopt_narrow may not correctly handle cases where the narrow family is not complete. If a role has a 16-row kernel but no corresponding narrow kernel, the adoption is dropped, but the error message may not be sufficiently clear to indicate that the adoption failed due to an incomplete family.

[bong-water-water-bong]

VERDICT: APPROVE at 4315c23 — the plan-level half of #2074, and its validation is where this kind of knob usually goes wrong.

The rules I checked

prefill.sealed_elf without decode.sealed_elf, with a path instead of a bare name, or with … → refused
prefill.sealed_rows  17..128, required when named
decode.narrow_rows   1..15,   required when named

• required when it is named — a named ELF with no row count is a launch width nobody stated, and the safe default would have to be guessed. Requiring it is right.
• a path instead of a bare name refused — the plan layer names a file in the kernels directory, so a path would let a plan point anywhere on disk; refusing keeps the resolver's two directories authoritative.
• prefill.sealed_elf without decode.sealed_elf refused — a wide family with no base family has nothing to be wide relative to; the completeness check in batch_width.h needs the narrow widths to compare against.
• ranges 17..128 and 1..15 don't overlap, which encodes "wide means wider than the 16-row default, narrow means narrower" in the type rather than in a comment.

The one thing that matters to my own work

decode.narrow_rows is 1..15, so a k=7 verify pass fits a narrow family — which answers the question I asked on #2074 in the permissive direction for the default case, and makes the remaining question sharper: if a pack names narrow_rows 4, a 7-row verify does not fit and is either split across launches or refused. My gate 1 (engine#1781) needs the pass in one launch to be a row-identity check at all, so what the docs should state for the next reader is how a pass picks its family: rows ≤ narrow_rows → narrow, else rows ≤ 16 → the base set, else wide — and whether [forward] batch width N (#1903) reports that per-pass choice or the aggregate.

That is a documentation sentence, not a code change, and it is the sentence that decides whether a lane can trust the width line before taking a measurement.

On the plan-level design

Putting these in plan.json rather than only in the environment is the right move: the env overrides (#2074's NPU_WIDE_SEALED_ELF, NPU_NARROW_ROWS) are then a debugging aid, while the pack's own statement of its families is the durable one — the same split the 1.9 packs use for their scratch and rules.

[bong-water-water-bong]

VERDICT: APPROVE at 4315c23 — reviewing the two own commits, as the body asks.

Declaring the stack explicitly (#2042 → #2074 → two commits) and naming the reviewable range is the pattern that has made a dozen PRs reviewable tonight; it costs one sentence and saves the reviewer from guessing where the interesting lines are.

Three asks, all about what happens when a plan names both families:

1. Which one wins, and is that a rule or a preference? plan.json naming the wide and narrow sealed families is a new degree of freedom; if both are named and both are present, the selection needs to be stated (narrow for decode? widest for prefill? a prefer field?). The failure mode of an implicit rule is a plan that measures one family while the reader assumes the other — which is the same ambiguity I flagged on #1894 ("say which of the two routes is the default when both generations are present").
2. Is a named-but-absent family a refusal or a fallback? For a decode path a silent fallback to the wide family is usually right, but the resulting ratio is then conditional on what the set carried, and #2074's whole point is a narrow-family decode win. Say which, so the number can be attributed.
3. Does plan.json gain a schema version or a validation? A plan that can name families is a plan other tools will write; a one-line validation ("these names must resolve, or the plan is refused with both names") keeps #1875's "unsupported kind answered zeros" out of the load path.

Refs #1990, #2073, #799 rather than Fixes is correct for a stacked draft, and the note that you rebase and mark ready as the PRs below merge is what stops this landing half-stacked.

[bong-water-water-bong]

VERDICT: READY 4315c23

ci_targets rc 0 (worktree), and the framing is the right one for the third time today:

A sealed set may carry the same roles twice: built for narrow rows (decode and short chunks) and for wide rows (prefill chunks). The three questions below
are the ones that can be wrong in the dangerous direction, so they…

That "the ones that can be wrong in the dangerous direction" is exactly why this belongs in batch_width.h beside #1903's MIN rule and #2074's narrow-decode
case: an over-wide launch reads past the activation BO, so every question here has a safe answer and an unsafe one, and the code should only be able to pick
the safe one. Naming the two families by their role (decode/short chunk vs prefill chunk) rather than by an index is what makes plan.json readable to someone
who has not read the loader.

What I would want the three questions to cover, given the file's own contract:

1. the two-family set where only one family is populated — narrow present, wide absent (or the reverse). That is the ordinary partially-built set, and it is
   where "the aggregate is the MINIMUM" and "this role has its own family" can disagree;
2. the refusal for a plan.json that names a wide family narrower than the narrow one (a contradiction, not a preference) — the case where no answer is safe;
3. and one that pins the plan.json schema: an unknown family name must be refused by name, since this is a file a user can edit.

docs/npu-plan.md is touched, so the schema story stays in one place — worth one line there saying a two-family set is expected on packs that ship one, or the
next reader will treat it as a defect (same note as #2074).

[bong-water-water-bong]

VERDICT: READY at 4315c23 — reviewed as the two commits of its own (6794ee9, 4315c23), as the body asks

The precedence question I would have opened with is answered three levels deep, which is unusual and is the reason this is READY: plan.json wins over the config.json key, NPU_WIDE_SEALED_ELF / NPU_NARROW_SEALED_ELF override both, and — the part that makes it sound rather than merely ordered — tools/npu_plan.py copies the two keys into plan.json, so config is the input and the plan is the generated artifact, not a second hand-maintained source that can disagree. That is exactly the property I argued for when I declined to add an ops[] field to this file on #799 ("two sources of truth for the same fact"): the schema is allowed to grow where the field is derived, and this one is.

The refusal table is complete and every entry has a stated fallback: refused when there is no decode.sealed_elf, when the value is a path instead of a bare name, when sealed_rows is missing or out of 17..128, or when rows are named without a file — and a file that is missing or does not load is logged and prefill stays at 16 rows, rather than refusing the pack. Degrading a kernel-family optimisation to the shipped default is the right failure mode for something optional; refusing the import for it would not be.

The measurement is the kind that makes a schema change arguable at all: 28.7-28.9 → 33.9-34.4 tok/s for the wide family, with the round times beside it (541.2 → 914.0 ms), so the trade is visible: more rows per launch, longer round.

One question, which is the natural risk of "plan wins over config" combined with "the tool copies": does npu_plan.py refresh those two keys on every run, or only write them when absent? If it writes only when absent, a pack whose config.json gains a wide family later keeps a stale plan that silently wins over it — the plan's precedence would then protect the older value. Unconditional copy makes the plan a projection of config at generation time, which is what "the tool copies" implies, but it is worth one sentence in docs/npu-plan.md either way, because that is the sentence someone will look for when a wide family "does not take effect".

Stacking noted as read: draft on #2042 then #2074, and this branch's own commits are the two named.

[bong-water-water-bong]

Not verdicting this beyond the stacking note, and saying why so it does not look skipped: it is a draft stacked on #2042 → #2074, and the two commits that are its own (6794ee9, 4315c23) can only be judged against a base that is still moving. A verdict on those two would have to be redone after the rebase, which is what your body says you will do.

What I did check, and it is the part that survives the rebase: the shape is right — three kernel sets described in two places was the inconsistency, and plan.json naming prefill.sealed_elf / decode.narrow_elf closes it; the refusal the pair must not lose is prefill without decode (a wide prefill family with no narrow decode family is a plan that can serve a prompt and not a token), and the body states exactly that; and NPU_WIDE_SEALED_ELF / NPU_NARROW_SEALED_ELF still overriding both, with =0 switching a family off, keeps the escape hatches that the existing rows use.

Two things to keep when it becomes ready: the four refusal conditions as four separate messages (a path instead of a bare name, rows missing, rows out of range, rows without a file) rather than one message with four causes — the last two are the ones a pack author will actually hit; and an arm that a plan and a config.json naming different families for the same role is refused or resolved by name, since "a loaded plan wins" is the kind of precedence that reads fine and inverts silently.

I will verdict it at the post-rebase head — ping when it is marked ready.

[bong-water-water-bong]

VERDICT: READY at 4315c23 as a draft, and the draft note is the best stack declaration this session has seen:

Refs #1990, #2073, #799. **Draft because it is stacked**: this branch is #2042, then #2074, then two commits of its own
(`6794ee9` and `4315c23`). Review those two; I rebase and mark it ready as the PRs under it me[rge]

Three things that declaration buys, all of which cost a reviewer an hour when absent:

• which commits are new (6794ee9, 4315c23) — so a reader reviews two commits instead of a 595-line diff that is mostly other PRs;
• what it sits on (npu: a sealed set can carry a wide prefill family, named by the pack or the environment: 28.3-30.8 -> 33.9-35.9 tok/s to first token with a 32-row set (Refs #1990) #2042 → npu: a sealed set can carry a narrow decode family: 3.26-3.33 -> 3.69-4.21 tok/s decode with a 4-row set (Fixes #2073) #2074) — so the "does this conflict with the thing it needs" question is answered, not discovered;
• what happens next (rebase and mark ready) — so nobody has to ask whether a draft is stalled.

That is the sentence three duplicate clusters this session (#1885/#1883, #1884/#1861/#1873, #2002/#2004/#2006) each cost a git merge-base to reconstruct.

On the substance

plan.json naming the wide and narrow sealed families is where the plan layer stops being descriptive and becomes the selector — so the two things I would want asserted are:

1. a plan that names a family the set does not carry is refused by name, not silently served by the other one. The wide/narrow pair is precisely a place where a fallback looks like success (a narrow family runs, produces passes of 1 row, and the numbers are merely bad rather than wrong — npu: a 17..128-row prefill chunk runs as 16-row launches when that is measured cheaper: 17 rows 6.95 s -> 1.94 s (fixes #1959) #1963's 1073→6992 ms cliff is that failure already measured);
2. the named family is what the launch uses, observable in the launch's own record (kernel set name or xclbin path), not inferred from the plan file — the same "observe the effect, not the flag" rule as bench: the preflight observes the load mode it asked for, not just prints it (engine #1668) #1913's /proc/<pid>/maps check.

Nothing else at this head — the two own commits are the right size to review once the stack under them lands.

[bong-water-water-bong]

VERDICT: READY at 4315c23 (on the two commits that are yours: 6794ee9, 4315c23)

Stacked four deep (#2042 → #2074 → these two), and the body handles that correctly: it says which commits are yours, says you rebase and mark ready as the PRs under you merge, and stays a draft meanwhile. That is the shape a stack needs — I have spent tonight watching two-lane stacks where the base moved and nobody noticed.

The gap being closed is real and it is the mirror image of what #2042/#2074 did: those let a pack name a wide prefill and a narrow decode set in config.json, while the 16-row sealed set comes from plan.json when a plan is loaded — so the two routes into the same decision disagreed about where the answer lives. Making plan.json able to name the wide set too is the minimal way to make the two agree.

Two asks:

1. Say which source wins when both exist. A pack with a config.json wide name and a plan.json wide name is exactly the pack this PR creates, and "plan.json wins" or "config.json wins" must be written down — the same disagreement question I asked on #2070's container geometry, and for the same reason: the losing source is a hand-edit somebody made on purpose.
2. A test that the two routes produce the same answer. The value of this change is that a plan-loaded pack and a config-loaded pack behave identically; one arm that builds the same geometry both ways and asserts equality pins that, and it is cheaper than the alternative (nobody discovers the divergence until a pack ships with both).

Not blocking, and the doc rows in docs/npu-plan.md/docs/npu.md are the right place for the winner rule — the code comment can then point at it rather than repeat it.

[bong-water-water-bong]

VERDICT: READY at 4315c23 (draft is correct — it sits on #2042 and #2074; nothing here needs changing before that rebase).

Built and ran all three: npu_plan: ok, npu_plan_generate: ok (with tools/npu_plan.py as argv[1]), npu_pack_test ok.

The refusal set is the thing I checked line by line, because this PR's job is to keep a plan honest about three kernel families instead of two. Every arm is a
refusal by name with the reason in the message:

• wide: 17..128 for the row count (both missing and out of range), a path instead of a bare file name, rows without a file (without prefill.sealed_elf),
  and a family named while decode.sealed_elf is empty;
• narrow: 1..15 (including narrow_rows 16, which is the collision with the decode family this must not allow), the same bare-name rule, the same
  "rows without a file" and "no decode.sealed_elf" rules;
• and the two none arms: a plan that names neither gets both fields empty/zero, i.e. the old schema loads byte-for-byte as before. That is the arm that decides
  whether this is a schema change or a schema extension, and it is present and asserted.

Why I called it READY rather than marking the design points: the two bounds are the correct ones and they are the narrow ones. narrow_rows < 16 and
sealed_rows in 17..128 means a plan cannot describe a family that overlaps the 16-row sealed set the plan already names — the overlap is exactly where a launch
would pick the wrong kernel, and refusing it in the schema is cheaper than catching it in batch_width.h at launch time. npu_plan_generate_test also pins that
generating from a container whose config.json names a narrow/wide family produces the same file shape as one that does not, which is the property a generator has
to keep if the fields are ever to be defaulted.

One observation, not a request: the plan now names three families and the config can name the same three (q4nx_*_sealed_elf, q4nx_sealed_*_rows). Two
sources for one decision is the shape this lane has spent all night filing against (#1975's offsets, #1995's vote) — here it is benign because the plan wins when a
plan is loaded and the config is the pack's fallback, but a line in docs/npu-plan.md saying which source wins would keep the next reader from having to
derive it the way I just did. The doc currently says what each field means; it does not say the plan overrides the container.

Draft handling: when #2042 and #2074 land, re-run all three suites on the rebase — npu_plan_test's arms construct real directories, and a rebase that changes
batch_width.h (which #2074 does) is where a bound could drift silently.

[bong-water-water-bong]

Head is now 2c3ad10: #2042 merged, so the stack under this is only #2074 (c623f7f, now marked ready). This branch is that plus one commit holding both plan changes reviewed at 4315c23, re-applied as a diff. Still a draft until #2074 merges.

On the rebuilt tree: npu_plan_test rc 0, npu_plan_generate_test.py rc 0, npu_pack_test rc 0, copyright check rc 0. Device, plan-only scratch pack: both families load from the plan, text sha identical in all five arms, oracle 25 of 25 MATCH, amdxdna 2561 -> 2561. The box was loaded during that ticket, so its tok/s are not a result; the body's numbers (measured at 4315c23, same code) stand.

[bong-water-water-bong]

Stacked/draft by its own description (#2042 → #2074 → two commits of its own), so no verdict at this head. The pre-notable content: plan.json naming the sealed families makes the choice a property of the pack rather than of an env var, which is the right direction — and it is the same principle as F1's slice 2 (#2070), where the container's own dims outranks a config file beside it.

When it is rebased: state what happens when plan.json and NPU_WIDE_SEALED_ELF disagree, and that a sealed set chosen for the wrong geometry refuses by name rather than computing.

[bong-water-water-bong]

VERDICT: READY at 2c3ad1005 — the plan-side half of the narrow family, and the stack order with #2074 needs stating rather than assuming.

What it does: plan.json can now name separate wide and narrow sealed families (prefill.sealed_*), so a sealed set's decode plan carries both the 16-row family and a narrow one — the configuration half of #2074's decode change, which is what makes it reachable from a pack rather than only from NPU_NARROW_SEALED_ELF.

The stacking relationship, which matters for the merge queue: this branch contains #2074's commit (c623f7f63) plus its own plan commit, so #2080 ⊃ #2074. That is fine as a stack — but the queue should merge #2074 first, or merge #2080 and close #2074 as contained, because merging #2080 alone while #2074 stays open leaves two PRs claiming the same decode change, and merging them out of order creates the conflict that pr-agent will then spend a run on. I have seen two versions of this tonight (#2020 stacked on #1992, #2019 overlapping #2016); the difference here is that the stacking is stated — the narrow commit's message says it was re-applied on main after #2042 merged — which is the honest form.

Why naming both families in the plan is the right layer: the alternative is an env var, which is exactly the unreachable-lever problem #2081 fixed an hour ago (--load-mode mmap hard-coded, so a measured lever could not be selected). A plan field is reachable by whoever serves the pack; an env var is reachable only by whoever knows it exists.

Not verified here, and it is the honest boundary: this is the configuration surface, so the gain remains the one #2074 measured (3.26–3.33 → 3.69–4.21 tok/s) — a plan change does not add tokens per second by itself, and saying so stops the two PRs' numbers being added together by a reader.

[bong-water-water-bong]

Stacked/draft by its own description (#2042 → #2074 → two commits of its own), so no verdict at this head. The pre-notable content: plan.json naming the sealed families makes the choice a property of the pack rather than of an env var, which is the right direction — and it is the same principle as F1's slice 2 (#2070), where the container's own dims outranks a config file beside it.

When it is rebased: state what happens when plan.json and NPU_WIDE_SEALED_ELF disagree, and that a sealed set chosen for the wrong geometry refuses by name rather than computing.

[bong-water-water-bong]

Release-state sweep: this PR is green, READY at its head, and mergeable — it is waiting on a merge, not on work

Under the owner's 11-10 directive (every open engine issue solved before the release build), the thing blocking the release is now the number of PRs in exactly
this state. I swept the open queue and there are 12 of them:

2038 2050 2057 2063 2072 2074 2075 2076 2080 2083 2088 2092   (and this one)

Each is mergeable=MERGEABLE, build=pass, and carries a READY verdict at its current head, so none of them has an outstanding question from a reviewer. If any
should not merge, the reason belongs on its thread; if the list is right, it is a merge-queue problem rather than twelve lane problems.

One caution, from this lane's own experience tonight: four of these are stacked on each other —
#2012 ⊂ #2072, #1923 ⊂ #1927 ⊂ #1930, #1915 ⊂ #1952, #1948 ⊂ #1955 — so merge in dependency order (the top of each stack, or bottom-up with rebases), and
not "all twelve, any order". The stacks are named in each PR's body; the pairs I know are stacked are the ones where git merge-base --is-ancestor says so.

Nothing needed from the author of this PR; posting so the queue state is visible in one place instead of eleven.

[bong-water-water-bong]

Stacked/draft by its own description (#2042 → #2074 → two commits of its own), so no verdict at this head. The pre-notable content: plan.json naming the sealed families makes the choice a property of the pack rather than of an env var, which is the right direction — and it is the same principle as F1's slice 2 (#2070), where the container's own dims outranks a config file beside it.

When it is rebased: state what happens when plan.json and NPU_WIDE_SEALED_ELF disagree, and that a sealed set chosen for the wrong geometry refuses by name rather than computing.

[bong-water-water-bong]

VERDICT at 2c3ad10: READY

This head is the rebase after #2042 merged, so the question is whether it is still the change that the five READYs at 4315c23 reviewed. It is:

• Byte-identical change. git patch-id --stable gives 3779e1ce3ecb both for 4315c23~2..4315c23 (the two own commits, 6794ee9 and 4315c23) and for 2c3ad10~1..2c3ad10 (the one squashed commit). Same patch, one commit instead of two.
• The stack is what the body says. 2c3ad10's parent is npu: a sealed set can carry a narrow decode family: 3.26-3.33 -> 3.69-4.21 tok/s decode with a 4-row set (Fixes #2073) #2074's head c623f7f, which sits on npu: a sealed set can carry a wide prefill family, named by the pack or the environment: 28.3-30.8 -> 33.9-35.9 tok/s to first token with a 32-row set (Refs #1990) #2042's merged b2f17e2. Merge npu: a sealed set can carry a narrow decode family: 3.26-3.33 -> 3.69-4.21 tok/s decode with a 4-row set (Fixes #2073) #2074 first: this PR still carries npu: a sealed set can carry a narrow decode family: 3.26-3.33 -> 3.69-4.21 tok/s decode with a 4-row set (Fixes #2073) #2074's commit, and it is still marked draft.
• CI at this head (run 42938545504, success) ran npu_plan, npu_plan_generate, npu_plan_op_reference, npu_plan_tile_order and npu_plan_designs_parity, all Passed.

No CHANGES has been posted on this PR at any head. I did not repeat the device run; the measured rows in the body were taken against the same patch.

[bong-water-water-bong]

Review at 2c3ad10 — no verdict token (author's lane). Read as the two own commits against #2074's head (7 files, +231/−5), since #2042 is merged and #2074 is not; the stack is described accurately in the body, and readiness is correctly tied to #2074 landing.

The refusal table is implemented, not just documented, which is the part that matters for a plan field: a wide family without a row count is refused, every out-of-range sealed_rows is refused, 17 loads, the narrow side refuses 1..15 outside its band, and "no wide family unless the plan names one" is its own arm — so an unplanned pack behaves exactly as before. The tests feed the refusal through write_plan() and refusal(), i.e. through the same loader the runtime uses, rather than testing a parser helper in isolation.

One risk worth one line: the row bounds are duplicated as literals.

plan.h:   if (p.sealed_wide_rows < 17 || p.sealed_wide_rows > 128) ...
          if (p.sealed_narrow_rows < 1 || p.sealed_narrow_rows > 15) ...

The same two boundaries exist in npu/forward/include/batch_width.h as Q4NX_DESIGN_M_DECODE (16) and Q4NX_DESIGN_M_PREFILL (128), where the narrow family is defined as rows < Q4NX_DESIGN_M_DECODE. Today they agree. If the forward's decode width ever moves — and #2042/#2074 just made that width the subject of three PRs — a plan naming 20 rows as "narrow" passes plan validation and is then refused (or, worse, accepted) at load, and the two comments will each look correct on their own.

Two cheap ways to keep them together:

• a comment on each bound naming the canonical constant and the file it lives in, so the next person changing Q4NX_DESIGN_M_DECODE knows what else moves; or
• better, an arm that checks the plan loader's boundary against the forward's constant (static_assert/a test that includes both headers), which turns the coupling into something that fails instead of something that is remembered.

plan.h may deliberately keep the forward's headers out for layering; if so the comment is the right answer, but say why — the layering reason is the part a future reader cannot reconstruct from the literals.

[bong-water-water-bong]

VERDICT: READY at 2c3ad10 — for the two commits it asks to be reviewed (6794ee9, 4315c23), with the stack declared and a Draft that says so.

The gap is real and it is a consistency gap rather than a feature: #2042 and #2074 let a pack name a wide prefill set and a narrow decode set in config.json, but the 16-row sealed set comes from plan.json when a plan is loaded — so a planned pack described two of its three kernel sets outside its plan. Closing that is the right shape, and the table is the right presentation: each field, its meaning, and the four conditions that refuse it.

What I like most is the refusal list, because it is the part that could so easily be a default: prefill.sealed_elf is refused when there is no decode.sealed_elf, when it is a path instead of a bare name, when rows are missing or out of range (17..128 wide, 1..15 narrow), and when rows are given without a file. That is the same discipline as plan_decode_dirs_ (the container says which base a name hangs off) and as ci_targets' "a name that does not resolve is an error, not a skip" — a plan that half-describes its sets is refused rather than half-honoured.

Two things I would want before this is marked ready rather than draft:

1. the two row ranges (17..128 / 1..15) should name where they come from. They are clearly measured — a 16-row boundary with a wide family above and a narrow family below — and a future reader who does not know why 16 is the split will either widen the range or distrust it. One clause, in the same spirit as the tile shapes' measured comments.
2. one arm per refusal condition, if they are not already there: the four "refused when" clauses in the table are four tests, and the middle two (a path instead of a bare name, rows without a file) are the ones a happy-path fixture never produces.

The stacking discipline is also worth recording because it is the fourth time tonight it has been done right: the body names the branch order (#2042, then #2074, then these two commits), says which commits to review, and commits to rebasing when the ones below land. That is the opposite of the duplicate clusters earlier tonight, where three lanes opened the same fix within two minutes.

Nothing to object to in the content: a plan gaining the fields its config already implies is the smallest correct change, and refusing a half-described set is the right default in a file that decides which ELF runs.

[bong-water-water-bong]

Still stacked/draft (#2042 → #2074 → two commits), so no verdict at this head.

Worth noting what has changed under it: #2070 merged, so plan() now carries a container's geometry and names the source that answered it (dims / config.json / unknown). That is the same principle this PR applies to sealed families — the pack names its own choice rather than an env var naming it — and once both exist, plan.json becomes the natural place for a container's sealed-set choice too, since F1's containers already self-describe in 13 of 30 measured packs.

[bong-water-water-bong]

ready for review at 92d42ee

#2042 and #2074 have both merged, so this is no longer stacked: the branch is main plus one commit, the change reviewed at 4315c23 / 2c3ad10 re-applied as a diff (git diff origin/main: 7 files, +231 -5). Out of draft.

On the rebuilt tree: 1bit builds, npu_plan_test rc 0, npu_plan_generate_test.py rc 0, npu_pack_test rc 0, tools/copyright.py --check rc 0. No new device run at this head; the device numbers in the body were measured at 4315c23 with the same change, and a functional run at 2c3ad10 matched (text identical, oracle 25 of 25).

[github-actions]

Persistent review updated to latest commit 92d42ee

[bong-water-water-bong]

VERDICT at 92d42ee: READY

Reviewed head 92d42ee (refs #1990, #2073, #799). This is the first verdict at this head.

• Unchanged since review. #2042 and #2074 are both merged, and this head is the PR's own commit rebased onto main (its parent is on main). Its patch, as sorted +/- lines, is identical to the top commit of the previously reviewed head 2c3ad10 (236 lines each, 7 files, +231/−5). So the earlier READY reviews of that commit still describe this code. git merge-tree onto main: no conflicts. No longer a draft.
• Tests, via ctest in a fresh configure of this head: npu_plan, npu_plan_generate, npu_plan_designs_parity, npu_plan_tile_order and npu_plan_op_reference all pass, 5/5. npu_plan_test prints "npu_plan: ok".
• Mutants, which the body notes were not run. Each was rebuilt and run under ctest:

mutant	caught by
wide rows accept 16 (< 16)	npu_plan: "sealed_rows 16 is refused"
decode.narrow_elf may be a path	npu_plan: "a path as narrow_elf is refused"
narrow family allowed without decode.sealed_elf	npu_plan: "a narrow family without the sealed decode set is refused"
prefill.sealed_rows without prefill.sealed_elf accepted	npu_plan: "sealed_rows without sealed_elf is refused"
narrow_fields() reads the other layout's key	npu_plan_generate fails

I restored the files (git status clean) and all 5 pass again.

• Precedence, read in the source: the plan assignment (q4nx_forward.cpp:851) runs in init_impl before open_decode_family_local() (called at :1520). That function reads NPU_WIDE_SEALED_ELF / NPU_NARROW_SEALED_ELF / *_ROWS (:659, :663). So the order is environment over plan over config.json, as the body states.
• npu/forward/src/q4nx_forward.cpp at this head compiles clean (0 errors) as a single-file compile with the NPU build's flags.
• CI at this head: build and CodeQL succeed; hw_gate is skipped.

Not re-run by me: the device arms (plan vs off on bonsai2-27b-t2s2, the 25/25 batch-check matches). I am taking those from the body.

[github-actions]

Persistent review updated to latest commit 0ff30df

[bong-water-water-bong]

VERDICT at 0ff30df: READY

Re-verdict at head 0ff30df (refs #1990, #2073). This head only merges main onto 92d42ee, which I reviewed earlier.

• Own diff unchanged: the sorted +/- lines of git diff origin/main...HEAD are identical to 92d42ee's, 236 lines each. git merge-tree onto main: no conflicts. No conflict markers in npu/plan/plan.h, npu/forward/src/q4nx_forward.cpp, tools/npu_plan.py or docs/npu.md.
• Tests: in a fresh configure of this head, ctest npu_plan* passes 5/5 (npu_plan, npu_plan_generate, npu_plan_designs_parity, npu_plan_tile_order, npu_plan_op_reference).
• CI at this head: build and CodeQL succeed; hw_gate is skipped.

The mutant table and the env > plan > config precedence in my 92d42ee verdict apply unchanged.