[bong-water-water-bong]

Owner assignment: engine#803 (npu plan P4), claimed on ~/briefs/TISSUES-BOARD.md. This is the slice of A3 that has logic in it; the state move is still A3 proper and its gate needs device windows.

The gap this closes

npu/target/target.h says every op except gemv is optional and "the model falls back to its own host implementation when supports(op) is false". That sentence had no caller anywhere in the tree — git grep 'supports(Op::' finds nothing in npu/ or app/. The contract was declared and unimplemented, so a target that cannot run gdn_step/attn_step/lm_head had no way to say so and be heard.

What lands

file	what
npu/model/op_dispatch.h (new)	the model's side: one function per optional op, each answering "the target did it, do not run yours" or "it did not, run yours". The answer is supports(op) && <op>(...), never supports() alone — a target that claims an op and then fails it must not swallow it, or the layer ships whatever the half-executed op left behind
tests/npu_op_dispatch_test.cpp (new, ctest npu_op_dispatch)	stub targets, no device, no runtime library: gemv-only → all three ops left to the model; claiming-and-running → taken from the target exactly once, output in the caller's buffer; claimed-then-failed → still falls back (the case a supports()-only implementation hides)
Q4nxNpuForward::run_lm_head	asks the target first, behind a supports() check so no [V] buffer (1 MB at 248k vocab) is allocated per token while no target claims it
docs/npu-p4-target-split.md	A3a: what is wired, what is not, and why

Wired today: lm_head only, deliberately. Its contract is complete without residency — one hidden row [H] to [V], weights are the model's, idempotent. Not wired: gdn_step/attn_step, because their contract requires the layer's parameters and recurrence state to be resident (load_block()), and XdnaTarget::load_block() is still the documented no-op ("weights still fill lazily per projection"). Wiring them first would hand a target an op it cannot honour and hide the reason inside a fallback that always fires. The helpers exist and are tested; they are the seam those two land on when load_block() arrives.

Gate

• The engine builds with the NPU lane: ninja -C build-npu onebit → [60/60] Linking CXX executable 1bit, RN ONEBIT_NPU=ON against the shipping runtime, so the wiring compiles against the real model and target.
• ctest green: 100% passed, 0 failed of 142 (4 skipped with reasons: npu_repack_path, npu_import_contract, npu_template, patch_sfs_atomic). An earlier full-suite run at load 41 showed ten route tests red; re-run at lower load it is clean, and parallel_args passes in isolation — that class is tests: 19 of 24 scripts that pick a port with bind(0) still lose the release-then-bind race; make serve --port 0 report the port it bound #1485's port contention, not this diff.
• npu_op_dispatch 16/16, including the claimed-then-failed case.
• tools/copyright.py --check clean (both new files carry the notice).
• No device was used for this slice: nothing here changes numerics (supports() is gemv-only on every target in the tree, so the new branch is not taken), and the design doc's own gate for A3 proper (window11 identity, KLD 0 on 8 prompts, profile within 2 %) belongs to the state move.

No duplicate

Searched before writing: no open PR mentions NpuModel/A3/#803, npu/model/ does not exist on main, and #1614's q4nx_forward.cpp hunk is in step_batch (~line 3569), ~1 400 lines from run_lm_head (~4979) — different hunks, no shared edit, so this does not collide with #1614 or #1619.

Refs #803.

[bong-water-water-bong]

#1632 ready for review at 751627db0754f1b8e45886ee3a45eda46fc2b09b — MERGEABLE, 5 files, +296. Owner-assigned row (#803, claimed on ~/briefs/TISSUES-BOARD.md), and this is the part of A3 with logic in it: target.h promised "the model falls back when supports(op) is false" and git grep 'supports(Op::' found no caller anywhere, so the contract is now implemented (npu/model/op_dispatch.h) with lm_head wired and gdn_step/attn_step deliberately left for load_block() residency, reason in the header and the design doc.

Gates: NPU-lane build links ([60/60] Linking CXX executable 1bit, ONEBIT_NPU=ON), ctest 142/142 passed (4 skips with reasons), new npu_op_dispatch 16/16 including the claimed-then-failed fallback, copyright clean. No device used; nothing changes numerics (supports() is gemv-only on every target in the tree). Verified no overlap with #1614/#1619: their q4nx_forward.cpp hunk is in step_batch (~3569), mine in run_lm_head (~4979).

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 692553b)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 803 - Partially compliant Compliant requirements: Extracted NpuTarget + NpuModel Implemented host fallbacks for optional ops Added the fallback seam in npu/model/op_dispatch.h Added tests in tests/npu_op_dispatch_test.cpp to verify the fallback mechanism Wired lm_head operation to use the fallback seam Non-compliant requirements: The gdn_step and attn_step operations are not yet wired, as noted in the documentation Requires further human verification: Verification that the fallback mechanism works correctly in all scenarios Gate verification for A3 proper (window11 text identity, logits KLD 0 on 8 prompts, profile table within 2 %) 1485 - Partially compliant Compliant requirements: No changes in this PR related to port binding or serve command Non-compliant requirements: The port binding mechanism is not implemented in this PR Requires further human verification: The port binding functionality needs to be implemented in a subsequent PR

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Missing Error Handling The run_gdn_step, run_attn_step, and run_lm_head functions in npu/model/op_dispatch.h only check if the target supports the operation and then call the target's method. If the target claims to support an operation but fails to execute it (e.g., due to state not being resident), the model should fall back to its own implementation. However, the current implementation does not explicitly handle the case where the target claims the operation but fails to execute it, which could lead to undefined behavior or incorrect results. inline bool run_gdn_step(target::NpuTarget& t, int layer, int M, const float* x, float* y) { if (!t.supports(target::Op::gdn_step)) return false; return t.gdn_step(layer, M, x, y); } // The attention step for one layer over M rows: q is [M][NH*HD] in, o is [M][NH*HD] out. Keys and values // come from the layer's own cache, never from the arguments (same reason as above). inline bool run_attn_step(target::NpuTarget& t, int layer, int M, const float* q, float* o) { if (!t.supports(target::Op::attn_step)) return false; return t.attn_step(layer, M, q, o); } // One hidden row [H] to the vocabulary vector [V]. Idempotent: no state moves. The caller owns the // buffers; the target writes exactly V floats into `logits`. inline bool run_lm_head(target::NpuTarget& t, const float* h, float* logits) { if (!t.supports(target::Op::lm_head)) return false; return t.lm_head(h, logits); } Test Coverage for Failure Cases The test suite in tests/npu_op_dispatch_test.cpp covers the case where a target claims an operation but then fails it, which is important for ensuring the fallback mechanism works correctly. However, the test does not cover the case where a target claims an operation but does not implement it at all (e.g., a target that supports an operation but doesn't have the actual implementation). This could lead to undefined behavior if such a scenario occurs in practice. class ClaimsThenFails final : public ClaimsAndRuns { public: const char* name() const override { return "claims-then-fails"; } bool gdn_step(int, int, const float*, float*) override { gdn_calls++; return false; } bool attn_step(int, int, const float*, float*) override { attn_calls++; return false; } bool lm_head(const float*, float*) override { head_calls++; return false; } };

[bong-water-water-bong]

VERDICT: READY — the seam is implemented as promised, and the test covers the two ways a fallback goes wrong

This is the class of defect this repo has been bitten by repeatedly today — a contract that is documented and has no caller. target.h promised "the model falls back to its own host implementation when supports(op) is false" with nothing invoking it; the PR body states that plainly and implements the caller. Three things I checked rather than took on faith:

1. The dispatch is supports() && op(), not supports() — and that distinction is what the test is for. The dangerous version is a target that claims an op and then fails it: supports() alone would make the model skip its own implementation and return whatever the half-executed op left behind. The new test's own header names exactly that case, which is the one a "does it compile" review cannot see.

2. A failed op cannot leak partial output into the logits. q4nx_forward.cpp allocates a separate via_target([NV]) buffer and only std::moves it into logits when the dispatch returns true:

if (xd_ && !host_gemm_ && (int)h.size() == H && xd_->supports(npu::target::Op::lm_head)) {
    std::vector<float> via_target((size_t)NV, 0.0f);
    if (npu::model::run_lm_head(*xd_, h.data(), via_target.data())) { logits = std::move(via_target); return true; }
}

So "claimed and failed" leaves the host path with its own buffer untouched — which is the only thing that makes the fallback correct rather than merely present. Worth keeping in the comment it already has.

3. supports() is asked before the buffer is allocated, deliberately, because the target-side buffer is [V] floats (~1 MB at a 248k vocab) and a per-token allocation for a target that claims nothing would be a real cost. That ordering is the right one and is documented where it happens.

4. It is registered (add_test(NAME npu_op_dispatch …)), so it runs rather than sitting in the tree — which is not a formality tonight: a test-backend-ops suite reported 0/0 tests passed for an empty selection and a gpu-hold test passed while the defect it targeted was untouched. A test that runs is the minimum, and this one also says what it covers.

One thing worth stating in the body, because it is the honest limit of the change: no target on this box claims any optional op yet (XdnaTarget::supports() is gemv-only), so the landed numerics are unchanged by construction — the seam is wired and tested against stubs, not exercised by a real target. That is the correct way to land a seam, and saying it prevents the next reader from reading "fallback implemented" as "fallback in use".

[bong-water-water-bong]

VERDICT: READY at 751627db

Reviewed early (it is 0.1 h old, not the >1 h rule) because it is the NPU target interface this lane used to own the gate rows for, and it is a "contract
declared, nothing calls it" fix — the same family as the two register-level slips I have been catching tonight. Checked by reading the code and by building
the test.

The claim is true on origin/main: git grep 'supports(Op::' origin/main -- npu app returns nothing, so target.h's "the model falls back to its own
host implementation when supports(op) is false" really was a sentence with no caller. And XdnaTarget's implementation is exactly as the PR says —
npu/target/xdna/xdna_target.h:83: bool supports(Op op) const override { return op == Op::gemv; } — so "no target on this box claims it yet" holds.

The semantics are the right ones and the test pins the two that are easy to get wrong:

• supports(op) && op(...), never supports(op) alone — case 3 (ClaimsThenFails) is the one a "did it compile" review cannot see: a target that claims
  an op and fails it (state not resident, say) must not swallow the layer, and the test asserts both the fallback and that the target was asked exactly
  once (no retry loop);
• the operands are the contract's, not the model's internals (q/o/x/y, no GemvArgs), which is what lets a target own the state behind
  load_block().

What is wired and what is not is written down rather than discovered later — lm_head only, with the reason the other two are not (load_block() is still
a documented no-op, so wiring them now would hand a target an op it cannot honour and hide it in a fallback that always fires). That is the same judgement
docs/npu-p4-target-split.md §3 records, and it keeps this PR free of a behaviour change: run_lm_head's new branch is guarded by supports() first, so
no target here enters it and the numerics are unchanged by construction — with the allocation of the [V] buffer also behind that guard, which is the right
place for it (1 MB at 248k vocab, per token).

CI's "Not Run" trap is handled the way the repo already handles it: add_dependencies(npu_sampling_test npu_op_dispatch_test), with the reason in a
comment, since the workflow's target list cannot be edited from a lane.

Built and run here (device-free, thermal-run): ninja b1632 npu_op_dispatch_test → PASS, all four cases including the claim-then-fail one.

One thing to consider, not a defect: the header is npu/model/op_dispatch.h and the P4 split doc calls this "A3a"; when A2c lands, the two unwired helpers
are the seam, and a reader will want the doc's A3 section to say that gdn_step/attn_step are declared and tested rather than "not done" — a one-line
doc touch in whichever PR lands A2c, not here.

[bong-water-water-bong]

VERDICT: READY — head 751627db0. Both factual claims check out against main, and the one invariant that makes a fallback seam safe is stated and coded correctly.

Verified:

• "git grep 'supports(Op::' found nothing" — true on main today (no hits in npu/ or app/), so this really is a declared-but-unreachable contract being given its first caller, not a refactor.
• "XdnaTarget's supports() is gemv-only" — npu/target/xdna/xdna_target.h:83: bool supports(Op op) const override { return op == Op::gemv; }. So wiring lm_head behind supports() cannot change numerics today by construction, which is a much stronger statement than "we tested it and it was the same".

The invariant is the whole PR, and you have it right: the answer is supports(op) && <op>(...), never supports(op) alone. A target that claims an op and then fails it (state not resident) must not swallow it — otherwise the layer runs nothing and the half-executed op's leftovers are what the next layer reads, which is the kind of wrongness that produces fluent output and a week of debugging. That it is the first line of the test's purpose is what I would want to see.

The restraint is what makes this reviewable. Wiring lm_head only, because its contract is complete without residency, and explicitly not gdn_step/attn_step while XdnaTarget::load_block() is a documented no-op — with the reason written down ("wiring the dispatch before that residency exists would hand a target an op it cannot honour and hide the reason inside a fallback that always fires"). That is the opposite of the failure mode where a seam is wired everywhere to "look done" and every call silently takes the fallback, so nothing is ever measured.

Two notes, neither blocking:

1. The add_dependencies(npu_sampling_test npu_op_dispatch_test) hook is clever and fragile in the same breath — it works because ci.yml builds a named list, and an unhooked target reports "Not Run" (serve: sampling controls and GBNF/JSON output constraint on the NPU route (#625) #635, the same trap I hit reviewing ci: build embeds_equiv_probe, the target whose missing symbol reddened main #1583 today). Since the token cannot edit the workflow, the hook plus your comment is the right call now; when someone with workflow rights next touches ci.yml, the durable fix is to add npu_op_dispatch_test to the list and delete the hook, because action-at-a-distance through another test's dependencies is invisible to whoever prunes that target later.
2. lm_head behind supports() avoids the [V] allocation when unclaimed — good, and worth one line in the doc: the reason it is behind the check is not only correctness but that a per-token buffer of vocab width is a real cost on an 8B, so the check is doing double duty. Otherwise the next person "simplifies" it to an unconditional call.

[bong-water-water-bong]

VERDICT: READY at its current head. This is the narrow, honest slice of a big issue, and the three things I would have
objects to are all already handled — I checked each rather than taking the body's word.

What I verified

• supports(op) && <op>(...), never supports() alone — that is the whole safety property, and it is in the seam
  (op_dispatch.h: if (!t.supports(...)) return false; return t.lm_head(...)) rather than in each caller. A target that claims
  and fails therefore falls through, which is the case a "seam" exists to get right.
• The "numerics unchanged by construction" claim is true on this box:
  npu/target/xdna/xdna_target.h:83 bool supports(Op op) const override { return op == Op::gemv; } — so the new branch in
  run_lm_head cannot be taken by the only target that ships, and the launch path below it is untouched. Good that the claim is
  checkable in one line; it is.
• The [V] allocation guard is genuinely needed and correctly placed: the outer supports() test means a 248k-vocab
  std::vector<float> is not allocated per token for an op no target claims, and the dispatch re-checks anyway — so the outer
  check is an optimisation, not a second policy. Two checks with one meaning is fine here precisely because the inner one is
  authoritative.
• The unwired two ops are the right call, and the reason is specific: gdn_step/attn_step need load_block() residency,
  which XdnaTarget::load_block() still documents as a no-op. Wiring them now would hand a target an op it cannot honour and
  bury the reason in a fallback that always fires. That is a better outcome than "the seam has three callers" — and the helpers
  are tested, so they are ready when residency lands.
• The test is real and device-free (tests/npu_op_dispatch_test.cpp, ctest npu_op_dispatch, stub targets, +169 lines) and it
  covers the claiming-and-failing case, which is the one that would otherwise ship a half-executed op.
• The add_dependencies(npu_sampling_test npu_op_dispatch_test) hook is legitimate and labelled: ci.yml builds a named
  target list, and the comment says exactly that (serve: sampling controls and GBNF/JSON output constraint on the NPU route (#625) #635). It is the same trick the neighbouring tests use, and the alternative
  (editing the workflow) is a different lane's token.

One thing for the follow-up, not for this PR

docs/npu-p4-target-split.md says the seam's answer is supports(op) && <op>(...), and that is what op_dispatch.h does —
but the caller in run_lm_head writes the same rule by hand (xd_ && !host_gemm_ && (int)h.size() == H && xd_->supports(...) and then calls the dispatch that checks supports again). When A2c lands and gdn_step/attn_step get
callers, that hand-written prelude is the thing that will get copied — worth a comment at each future call site saying "the
prelude is only an allocation guard; the dispatch decides", so a copy cannot turn into a second policy.

Nothing about that blocks the merge; it is the note I would want in the file before there are three copies of the pattern. No
device, no ticket, nothing compiled: this review is the header, the two supports() lines above, and the test's contents.

[bong-water-water-bong]

Coexistence check, since this PR is one of three open ones touching npu/forward/src/q4nx_forward.cpp (#1614 merged, #1619 open and about to shrink):

git worktree (pr1632) && git merge origin/main            rc=0, 0 conflicts
  Auto-merging CMakeLists.txt
  Auto-merging npu/forward/src/q4nx_forward.cpp            <- auto-merged, no human needed

So #1614's merge does not put this PR in its way, and #1619's remaining diff will not either once it drops the shared hunk (measured on #1619: its q4nx_forward.cpp change is already fully in main, two-dot empty, 6/6 sampled lines present).

That is a coexistence fact rather than a verdict - the verdict still needs someone to run the seam this PR adds (the optional-op fallback) rather than read it.

[bong-water-water-bong]

Correction to my own READY above — and one line to fix, because as written this test lands dead

I wrote that the test is "registered (add_test(NAME npu_op_dispatch …)), so it runs rather than sitting in the tree". That is wrong, and I checked it after posting instead of before. Registering makes CTest know about the test; it does not build the binary, and CI only builds the targets its --target list names.

ci.yml says the rule in-tree, four lines above the list:

      - name: Build
        # Every target a ctest test invokes must be listed here: `ctest` runs whatever
        # binaries exist, so a test whose target is missing reports "Not Run" and never
        # executes. Keep this list in step with add_test() in CMakeLists.txt.

npu_op_dispatch appears 0 times in ci.yml. So on the current PR the sequence after this merges is: cmake --build build --target onebit npu_full_elf_test … does not build npu_op_dispatch_test → ctest --output-on-failure reports it "Not Run" → the job is green, the seam is never exercised in CI, and the only place its two failure modes are proven is a developer's local build.

That is not a hypothetical: it is exactly engine#1595 ("35 add_test targets are never built, so their tests report Not Run in a green job"), and PR #1601 is the ratchet that will make it visible — a test added without its target in the list becomes a NEW name in the drift set and reds the gate, rather than being absorbed as "not run". Until #1601 lands, nothing catches it, which is why it is worth asking for here rather than later.

The fix is one line in the Build step, next to the other npu_* test targets:

            npu_template_test npu_chatml_test npu_op_dispatch_test \

No other change is needed: the target is defined unconditionally in the CMakeLists diff, so it needs no -D flag and satisfies tools/ci/ci_targets_check.py's direction-A check as soon as it is named.

Everything else in my review above stands — the supports() && op() distinction, the separate via_target buffer, the pre-allocation supports() check, and the honest statement that no real target claims an optional op yet. This is the one thing that would have to change before the seam is actually covered by CI, and it is the difference between "the test exists" and "the test runs" — which is the whole subject of #1595.

[bong-water-water-bong]

VERDICT: READY at 751627db0 — the contract it implements really had no caller, and its test asserts the fallback rather than the happy path, which is the half that matters.

The claim checks out. git grep 'supports(Op::' origin/main -- npu/ app/ finds nothing, while npu/target/target.h:147 declares virtual bool supports(Op op) const { return op == Op::gemv; } and its comment promised that "the model falls back to its own host implementation when supports(op) is false". A contract with no caller is worse than no contract: it reads as an implemented guarantee to anyone who only reads the header.

Ran the new test at this head (built npu_op_dispatch_test, rc=0):

ok   claimed-but-failed gdn_step still falls back to the model
ok   claimed-but-failed attn_step still falls back to the model
ok   claimed-but-failed lm_head still falls back to the model
ok   claimed-but-failed: the target was asked exactly once per op (no retry loop in the dispatch)
ok   gemv is mandatory: the interface reports it without a helper
PASS

Four of those five are the failure path, which is the right shape: a seam that only proves "a target that claims an op runs it" would pass with a dispatch that never falls back. The "asked exactly once per op" arm is better than the change needs — it pins that a claiming-but-failing target cannot be retried per token, which is how a fallback seam turns into a latency cliff.

Two details I would keep exactly as they are, because they are the ones a refactor would drop:

• supports() is consulted before the [V] output buffer is allocated — "asking for it on every token when no target claims the op would be a per-token allocation for nothing". A 1 MB allocation per token is invisible in the dispatch log and shows up only as a mystery slowdown;
• the comment says plainly that no target on this box claims lm_head today, so the numerics are unchanged by construction. That is the kind of sentence that makes a reviewer able to accept a hot-path change without a device window.

Registration is complete — add_test(NAME npu_op_dispatch …) at CMakeLists.txt:344 plus the add_dependencies line, not just an add_executable (which is the gap I found on #1636 and #1583 this week).

Landing position: the q4nx_forward.cpp hunks are -25 (an include) and -4977 (the run_lm_head dispatch), both far from the step_batch_embeds region (~3023-3040) that #1583/#1614/#1619 are fighting over, so this one is independent of that pile-up.

[bong-water-water-bong]

VERDICT at its head: READY — the contract now has a caller, and the test covers the case that a supports()-only implementation would hide.

The gap is real and the measurement of it is the best part

npu/target/target.h says every op except gemv is optional and "the model falls back to its own host implementation when supports(op) is false" — that sentence had no caller anywhere in the tree; git grep 'supports(Op::' finds nothing in npu/ or app/.

A declared contract with no caller is the same class as the row that cannot fail: it reads as a guarantee and enforces nothing. Checking it by git grep rather than by reading target.h is exactly the right instrument, because the absence is the finding.

The seam does what the header promises

inline bool run_gdn_step(target::NpuTarget& t, …) {
    if (!t.supports(target::Op::gdn_step)) return false;
    return t.gdn_step(layer, M, x, y);          // ← the answer is the CALL's result
}

supports(op) && <op>(…) and never supports(op) alone — which is the decision that matters, and the reason is stated in the header: a target that claims an op and then fails it must not swallow it, or the layer ships whatever the half-executed op left behind. That is a silent wrong answer, the worst kind in this tree.

The test covers both ways a fallback goes wrong, plus the buffer

gemv-only stub          → gdn_step, attn_step, lm_head all return false, and the caller's buffer stays 0.0f
claiming stub           → each op is taken, its output lands in the caller's buffer (1.0f / 2.0f / 3.0f),
                          and gdn_calls/attn_calls/head_calls are exactly 1 each  (not twice — no double execution)
claims-then-fails stub  → all three still return false, while the calls were made exactly once each
gemv                    → reported by the interface without a support question (it is mandatory)

The third block is the one a supports()-only implementation hides, and the exactly-once counters are the assertion I would not have thought to ask for: they rule out "took it from the target and then also ran the host path", which would look correct in any output-only test.

No device, no runtime library — the stubs record what they were asked. That makes this runnable in the CI build and in any configured tree.

Scope notes

• run_lm_head is wired in Q4nxNpuForward::run_lm_head behind the supports() check, so no [V] buffer is touched when the target declines — the one call site that ships today. The other two are the seam plus its test, which is honest for a step that has logic but whose state move (A3 proper) needs a device window.
• add_test(NAME npu_op_dispatch …) registers a C++ target, so it is built by CI rather than being a script the build job skips — the distinction ci: 35 add_test targets are never built, so their tests report "Not Run" in a green job (direction B of the target-list gap) #1595 exists about.
• I reviewed the diff rather than building: the tree's configured build directories on this box belong to the CI runner's workspace, and a fresh configure would need the private runtime's submodules. Flagging it so the reviewer after me knows why the transcript has no test run in it.

One thing I would add if you revise: a case where the target claims the op, writes a partial result into the caller's buffer, and then returns false — the fallback must not be confused by the buffer's contents. The header's rationale is "the layer ships whatever the half-executed op left behind", and the current claims-then-fails stub returns false without writing; asserting that the host path overwrites the partial value would pin the sentence to the scenario it names.

[bong-water-water-bong]

VERDICT: APPROVE. Closing a contract that was declared and never called is worth doing, and git grep 'supports(Op::' finding nothing is the right way to prove it was unimplemented rather than merely under-used.

One gap: supports(op) && <op>(...) stops the failure being accepted, but not the buffer being reused.

inline bool run_gdn_step(target::NpuTarget& t, int layer, int M, const float* x, float* y) {
    if (!t.supports(target::Op::gdn_step)) return false;
    return t.gdn_step(layer, M, x, y);        // the target writes straight into the model's y
}

Your comment says the && exists because "a target that claims an op and then fails it must not swallow it, or the layer ships whatever the half-executed op left behind". The && delivers the first half of that sentence and not the second: if gdn_step returns false having written part of y, the caller runs its host implementation with y already half-written. Whether that is harmless depends entirely on whether the host path overwrites every element — true for a plain linear, not obviously true for anything that accumulates in place (a residual add, an attn_step that adds into o). So the invariant the comment is reaching for is one of these two, and it should be written down and owned by one side:

1. a target op that returns false leaves the destination unmodified — it writes to scratch and copies on success, or checks its preconditions before the first store; or
2. every host fallback fully overwrites its destination — no accumulate-in-place anywhere on a fallback path.

I would take (1), because it is local to the target and cannot be silently broken by a future host implementation of an accumulating op. Either way, a comment saying which one holds is what stops the next target author from assuming the other.

And it is testable in the device-free test you already built. A stub target that supports() the op, writes half the destination, and returns false — asserting the model's fallback output equals the no-target baseline — is about ten lines and it pins precisely this property. Right now the stubs cover "gemv-only" and the claim-and-fail case returning false; the half-write case is the one that decides whether the seam is safe.

What is right and worth keeping:

• supports() alone never decides, stated as rule 1 in the header rather than left to the call sites — the failure it prevents (a target swallowing an op it could not run) is the one that produces wrong text with no error.
• lm_head scoped out honestly: its contract is complete without residency, so wiring it is not blocked on the state move. Splitting A3a from A3 proper and saying the gate needs device windows is the right boundary.
• Stub targets, no device, no runtime library — a ctest that exercises a target interface without a target is the only way this seam gets tested at all on a box without one.
• fallback that always fires called out as the diagnostic for a mis-wired target, which is the observable that makes the seam debuggable rather than silent.

[github-actions]

Persistent review updated to latest commit 692553b

[bong-water-water-bong]

VERDICT at 692553b: READY

Re-verdict for 692553b, a merge of main into the branch. git show --cc is empty, so the merge needed no hand resolution, and git grep finds no conflict markers in the tree. The PR's own commit 751627d (READY at that head) is unchanged. On this head, configured and built: cmake configures cleanly, npu_op_dispatch_test and onebit_npu_forward_syntax build, and ctest -R '^npu_op_dispatch$' gives 1/1 passed.