Skip to content

feat(native-decode): commit recurrent GDN state to accepted prefix in speculative decode - #1598

Merged
justinchuby merged 2 commits into
mainfrom
squad/mtp-recurrent-state-commit
Aug 20, 2026
Merged

justinchuby merged 2 commits into
mainfrom
squad/mtp-recurrent-state-commit

Conversation

@justinchuby

Copy link
Copy Markdown
Owner

Summary

Unblocks MTP self-speculative decode for the dense-hybrid Qwen3.8-27B (48/64 layers are Gated-DeltaNet recurrent + conv1d state). Previously, speculative rewind of a recurrent state to a non-zero length was unsupported: native_decode prefix-sliced the attention KV but skipped the destructive GDN SSM + conv1d rolling caches (backend.rs: "speculative rewind of a recurrent state is unsupported"; cuda.rs: fixed states only re-zeroed at target_len==0). Any partially-accepted speculative step on a hybrid recurrent model would then run a stale recurrent state → silent corruption.

Mechanism (approach A — snapshot + re-advance, matching vLLM)

vLLM does not rewind GDN state on rejected drafts. Instead: snapshot the pre-draft recurrent/conv state, run the verify window, then advance the committed state by exactly num_accepted. Our native verify runs the k-token window as a single Scan forward, so per-position intermediate states aren't exposed — approach (B) would need invasive execution changes. Approach (A) restores the pre-draft snapshot and re-runs only the accepted tokens (≤k, cheap, deterministic); the attention KV keeps its existing prefix-slice rewind.

Changes

  • Primitive on NativeDecodeSession (native_decode/mod.rs): snapshot_recurrent_state / restore_recurrent_state / commit_recurrent_state_to_accepted. Recurrent/conv bindings are found via the existing structural detectors — is_recurrent_state_shape over declared present→past pairs (host path) and fixed_state_binding_range (CUDA path). No hardcoded layer/dim numbers.
  • CUDA (native_decode/cuda.rs): snapshot_fixed_states / restore_fixed_states (device→host read / host→device write, no shape change → CUDA-graph capture-safe).
  • Wiring (speculative/mod.rs): snapshot before run_target_verification; after the accept decision, commit the recurrent state and square the paged length bookkeeping so the ordinary KV prefix-slice rewind becomes a no-op. Gated on a native runner that has_recurrent_state() via new DecodeState::native_recurrent_runner_mut.
  • Refreshed the two stale "unsupported" comments to point at the new primitive.

Correctness / inertness

  • Unit-test oracle native_recurrent_commit_matches_accepted_prefix_replay: a synthetic hybrid decoder whose recurrent/conv states mutate deterministically per token; asserts snapshot → run k drafts → commit(j) is byte-identical to snapshot → run only j tokens for j ∈ {0, 1, k}.
  • Greedy / dense byte-identity: the snapshot machinery only engages for a native runner with recurrent state; greedy and non-recurrent speculative paths never call it, and rewind_inner still skips recurrent as before.
  • 563 engine lib tests pass under --features native-backend (incl. existing speculative-matches-greedy); CUDA-enabled build (native-backend,cuda,cuda-13000) checks clean.

Validation status

Validated by unit test. End-to-end MTP is pending the env-blocked mobius MTP artifact. The recurrent-commit primitive is reachable today for prompt-lookup / draft-model speculation over native recurrent targets; the MTP-specific path additionally needs the pre-existing runner hidden-output limitation (next_session_token_logits_and_hidden bails on has_runner()) lifted, which is out of scope here.

⚠️ This task was flagged as "needs review" — please have a squad member review the speculative-loop wiring before merging (E2E path is artifact-blocked this session).

Working as Gaff (CUDA/Rust engine specialist).

justinchuby and others added 2 commits August 20, 2026 17:26
… speculative decode

Speculative rewind of a Gated-DeltaNet recurrent (SSM) + conv1d state to a
non-zero length was unsupported: native_decode rewind prefix-sliced attention KV
but skipped the destructive rolling caches, leaving them stale after any
partially-accepted speculative step on a hybrid recurrent model.

Add a snapshot + accepted-token re-advance primitive (vLLM's no-rollback rule)
on NativeDecodeSession: snapshot_recurrent_state / restore_recurrent_state /
commit_recurrent_state_to_accepted. Recurrent/conv bindings are identified via
the existing structural detectors (is_recurrent_state_shape on the host past;
fixed_state_binding_range on CUDA) with no hardcoded layer/dim numbers. Commit
restores the pre-draft snapshot and re-runs exactly the accepted tokens; the
CUDA path snapshots device->host and restores host->device with no shape change
(CUDA-graph capture-safe).

Wire into the speculative loop: snapshot before the verify window, commit after
the accept decision, and square the paged length bookkeeping so the existing KV
prefix-slice rewind is a no-op. Inert unless the target is a native runner that
has_recurrent_state(), so greedy and dense/non-recurrent paths are byte-identical.

Unit-test oracle (native_recurrent_commit_matches_accepted_prefix_replay) proves
snapshot->run k drafts->commit(j) is byte-identical to snapshot->run only j
tokens for j in {0,1,k} using a synthetic mutating hybrid decoder.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ixture

The mtp_speculative_generation_matches_plain_greedy oracle hardcoded the MTP
head path as mtp/model.onnx, but only the git-friendly model.onnx.textproto is
checked in (the binary head is Mobius-generated and cannot be rebuilt
in-sandbox). The ignored E2E therefore failed to load the head. Resolve the head
to whichever of .onnx / .onnx.textproto exists (Session::new already converts
textproto), so the baseline target -> MTP head -> verify/accept equivalence is
runnable in-sandbox. Confirms the speculative verify/accept loop that the new
recurrent-state commit wiring plugs into.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@justinchuby

Copy link
Copy Markdown
Owner Author

Oracle validation (follow-up on reachable in-repo MTP oracles)

Per the reviewer intel, I evaluated the three suggested oracles. Summary of what I ran and the fixture gaps found:

1. Baseline E2E MTP verify/accept loop — ✅ PASS (now runnable in-sandbox).
tests/mtp_full.rs::mtp_speculative_generation_matches_plain_greedy (--ignored) asserts MTP spec-decode output == plain-greedy on tests/fixtures/tiny-mtp-full. It was failing to even load because it hardcoded mtp/model.onnx while only the git-friendly mtp/model.onnx.textproto is checked in (the binary head is Mobius-generated, not rebuildable in-sandbox). Fixed the test to resolve either extension (Session::new already converts textproto) — the E2E now passes, confirming the verify/accept loop that the new recurrent-commit wiring plugs into. Commit ffb05c4f3.

2. Hybrid target-recurrent equivalence — fixture gap, not constructible at this base.

  • tiny-mtp-full's target is pure-attention (past/present.0.key|value only, no recurrent/conv bindings) — it never exercises the recurrent commit path.
  • tiny-hc-mtp is only an MTP-head graph (mtp_hidden + mtp_state outputs); its mtp_state is a head-level hidden-cache carrier, not a target's native recurrent decode state. It is a tracked but orphan fixture — no loader/test references it, and crates/onnx-genai-ort/tests/mtp_session.rs only loads tiny-qwen35-mtp. It cannot drive a full-engine generate() loop (no embedding/lm_head/metadata) and does not carry genuine GDN SSM + conv1d target state.
  • The target-recurrent MTP path is additionally gated by the pre-existing next_session_token_logits_and_hidden → has_runner() bail (out of scope here).

Conclusion — oracles used:

  • Primitive correctness: synthetic byte-identity unit test native_recurrent_commit_matches_accepted_prefix_replay (j ∈ {0,1,k}) — the exact equivalence oracle for the snapshot+commit-by-accepted primitive.
  • Surrounding loop: tiny-mtp-full E2E spec-vs-greedy equivalence — now runnable and passing.

A true end-to-end hybrid (GDN+GQA) mtp-spec-vs-greedy equivalence needs a full-engine hybrid MTP fixture (recurrent target + head + embedding/lm_head/metadata) that does not exist at origin/main 461309ca3, plus lifting the has_runner() hidden-output limitation. Both are tracked as the E2E follow-up in the decision note.

@justinchuby
justinchuby merged commit 3041c4c into main Aug 20, 2026
6 of 14 checks passed
@justinchuby
justinchuby deleted the squad/mtp-recurrent-state-commit branch August 20, 2026 17:35
@codecov

codecov Bot commented Aug 20, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.79%. Comparing base (461309c) to head (ffb05c4).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #1598       +/-   ##
===========================================
- Coverage   82.60%   80.79%    -1.81%     
===========================================
  Files          12      380      +368     
  Lines        5472   175074   +169602     
  Branches     5472   175074   +169602     
===========================================
+ Hits         4520   141451   +136931     
- Misses        759    28727    +27968     
- Partials      193     4896     +4703     
Flag Coverage Δ
cli-ort-linux 82.60% <ø> (ø)
cli-ort-windows 82.19% <ø> (?)
offline 80.73% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 370 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

justinchuby added a commit that referenced this pull request Aug 20, 2026
…site

Two gates are currently red on pristine main (0da1fe0), independently of
this branch:

* `Rust quality` runs `cargo fmt --all -- --check`, and #1598 landed six
  files unformatted (decode/mod.rs, decode/state.rs, native_decode/{cuda,
  mod,tests}.rs).
* `Fast` runs clippy with -D warnings, and #1576 added one more
  chunks_exact site in simd_activations.rs's ARM sweep test.

Both are fixed here so the branch can prove the gates green.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby pushed a commit that referenced this pull request Aug 20, 2026
…ral allow

`main` does not pass its own gates on a current stable toolchain. Three
independent causes, all found by running the CI matrix locally:

1. #1598 landed unformatted. `cargo fmt --all --check` reports six diffs across
   `decode/mod.rs`, `decode/state.rs` and `native_decode/{cuda,mod,tests}.rs`.
   Fixed by running `cargo fmt --all`; no logic change.

2. rustc/clippy 1.98.0 (2026-08-18) turned on `clippy::manual_slice_fill` and
   `clippy::needless_late_init` findings that this tree trips:
   - `native_decode/cuda.rs` (two `for row_len in &mut self.row_lens` loops from
     #1598) now use `self.row_lens.fill(..)`. Note clippy's own suggestion text
     (`&mut self.row_lens.fill(..)`) does not compile; the `&mut` is spurious.
   - `kernels/moe.rs`'s SwiGLU split rewrites the deferred `let linear_part; let
     gate_part;` pair as a single `let (gate_part, linear_part) = if ..`, which
     is what the lint asks for and reads better besides.
   Both are behaviour-preserving.

3. The same release added `clippy::chunks_exact_to_as_chunks`, warn-by-default.
   This repo pins no toolchain, so every `-D warnings` gate (Linux offline,
   quality lane, native-backend, aarch64, `check_cross_compile.sh`) goes red as
   soon as a runner picks up the new stable — which this host already has.

   That lint fires at 201 call sites across 12 crates, so it is not a local
   defect, and it is not uniformly auto-fixable: in `onnx-runtime-ir::read_vec_le`
   the chunk size is `T::BYTE_SIZE`, an associated const of a type parameter, and
   the suggested `as_chunks::<T::BYTE_SIZE>()` fails to compile — "generic
   parameters may not be used in const operations" (checked against rustc 1.98.0
   directly rather than assumed).

   So it is allowed in the existing `[workspace.lints.clippy]` table that all 52
   crates already opt into, keeping a toolchain bump from gating unrelated work.
   Migrating the sites that genuinely benefit — `as_chunks` yields `&[T; N]`,
   dropping fallible `try_into` and helping codegen — belongs with each crate's
   owner, not in one 201-site emergency rewrite. Cases 2's two lints are fixed
   rather than allowed precisely because they are small and local.

Verified locally: fmt; offline build; ep-cpu tests (default, `--no-default-features`,
`--features mlas`, `--all-features`); clippy `-D warnings` over the Linux-offline
set, the full quality-lane set plus `onnx-genai-cli`, `onnx-genai-engine
--features native-backend`, `onnx-runtime-ep-cuda --features cuda`, and
`--target aarch64-unknown-linux-gnu`; the MLAS-free artifact gate;
`check_cross_compile.sh`; and all eight repo lint scripts. 19/20 — the sole
failure is `--target aarch64-pc-windows-msvc`, which needs Windows SDK headers
this container lacks (`fatal error: 'stdlib.h' file not found`) and fails
identically on unmodified `main`.

Pre-existing and deliberately not widened into this repair: `clippy::approx_constant`
denies in `onnx-runtime-ep-cuda`'s lib *tests* (`optimizer.rs:4726`). CI runs that
crate's clippy without `--all-targets`, so no gate sees it today.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby added a commit that referenced this pull request Aug 20, 2026
…ral allow (#1604)

## `main` is red on stable 1.98.0

I ran the CI matrix locally against unmodified `origin/main`
(`3041c4c0f`) before starting my own work and found it failing 6/20
steps. Three independent causes:

**1. #1598 landed unformatted.** `cargo fmt --all --check` reports six
diffs across `decode/mod.rs`, `decode/state.rs`,
`native_decode/{cuda,mod,tests}.rs`. Fixed with `cargo fmt --all`.

**2. Two new 1.98.0 clippy findings, fixed at the source:**
- `manual_slice_fill` — two `for row_len in &mut self.row_lens` loops in
`native_decode/cuda.rs` (also from #1598) become
`self.row_lens.fill(..)`. Clippy's own suggestion text (`&mut
self.row_lens.fill(..)`) does not compile; the `&mut` is spurious.
- `needless_late_init` — `kernels/moe.rs`'s SwiGLU split rewrites `let
linear_part; let gate_part;` + `if/else` as `let (gate_part,
linear_part) = if ..`.

Both behaviour-preserving; `cargo test -p onnx-runtime-ep-cpu` covers
the MoE path.

**3. `chunks_exact_to_as_chunks` — allowed centrally, not rewritten.**

rustc/clippy 1.98.0 (built 2026-08-18) added this warn-by-default lint.
The repo pins no toolchain, so **every** `-D warnings` gate — Linux
offline, quality lane, native-backend, aarch64, and
`check_cross_compile.sh` — goes red the moment a runner picks up the new
stable. This host already has it.

Two reasons not to mass-rewrite:

- It fires at **201 call sites across 12 crates** (123 in `ep-cuda`, 46
in `ep-cpu`). That is a toolchain event, not a defect, and rewriting
other owners' hot paths under time pressure is how subtle bugs land.
- It is **not uniformly auto-fixable**. In
`onnx-runtime-ir::read_vec_le` the chunk size is `T::BYTE_SIZE`, an
associated const of a type parameter, and clippy's suggested
`as_chunks::<T::BYTE_SIZE>()` does not compile:

  ```
  error: generic parameters may not be used in const operations
    |     bytes.as_chunks::<{ T::BYTE_SIZE }>()...
    = note: type parameters may not be used in const expressions
  ```

  I compiled that against rustc 1.98.0 to check rather than assuming it.

So it goes in the existing `[workspace.lints.clippy]` table that all 52
crates already opt into, with the reasoning recorded inline. Migrating
the sites that genuinely benefit (`as_chunks` yields `&[T; N]`, dropping
fallible `try_into` and helping codegen) is real work worth doing
per-crate by each owner. The two small lints in §2 are fixed rather than
allowed precisely because they *are* small and local — the allow is for
the 201-site case only.

## Validation

Local CI matrix, **19/20**:

| | step | result |
|---|---|---|
| A | `cargo fmt --all --check` | PASS |
| B | offline build (24 crates) | PASS |
| C | `cargo test -p onnx-runtime-ep-cpu` | PASS |
| D | clippy Linux-offline set `-D warnings` | PASS |
| E | `cargo test -p onnx-runtime-ep-cpu --features mlas` | PASS |
| F | clippy `onnx-genai-engine --features native-backend` | PASS |
| G | clippy `--target aarch64-unknown-linux-gnu` | PASS |
| H | `cargo check --target aarch64-pc-windows-msvc` | **FAIL —
environmental** |
| I | ep-cpu `--no-default-features` tests | PASS |
| J | ep-cpu `--all-features` check | PASS |
| K | `default_artifacts_are_mlas_free` | PASS |
| L | `scripts/check_cross_compile.sh` | PASS |
| S | all 8 repo lint scripts | PASS |

Plus, beyond the script: clippy `-D warnings` over the **full
quality-lane package set + `onnx-genai-cli`**, and `onnx-runtime-ep-cuda
--features cuda` (the exact CI invocation). Both clean.

Step H fails with `onnxruntime_c_api.h:34:10: fatal error: 'stdlib.h'
file not found` — bindgen needs Windows SDK headers this container does
not have. It fails identically on unmodified `main`, so it is not a
regression from this PR.

## Not fixed here

`clippy::approx_constant` denies in `onnx-runtime-ep-cuda`'s lib
**tests** (`optimizer.rs:4726`). CI runs that crate's clippy without
`--all-targets`, so no gate sees it today — but `cargo clippy
--workspace --all-targets` does fail on it. Left for that crate's owner
rather than widened into a repair PR.

## Why I'm filing this

I hit these while re-validating my own branch after merging latest
`main`, and confirmed against a clean `origin/main` checkout that they
are not mine. Unblocking `main` first, then rebasing my stack onto it.

Co-authored-by: Roy <roy@squad.local>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby added a commit that referenced this pull request Aug 20, 2026
… 1.98, reformat main (#1603)

## Both required checks are currently red on pristine `main`

This is not caused by any one branch. On `0da1fe0bd`, with a clean
checkout:

* **`Fast (Linux x86_64)`** — Rust **1.98.0** was released on
**2026-08-18** and
added `clippy::chunks_exact_to_as_chunks` to the default set. CI
installs
unpinned `stable` (`rustup toolchain install stable`, ci.yml:126), so
the lint
arrived on its own. The clippy gate runs `-D warnings`, and there were
**70
  hits across 28 files**, so every PR in the repo now fails this check.
* **`Rust quality`** — `cargo fmt --all -- --check` fails: #1598 landed
six files
unformatted (`decode/mod.rs`, `decode/state.rs`,
`native_decode/{cuda,mod,tests}.rs`).

I found this while trying to get `Fast` green on an unrelated PR of
mine. It
blocks everyone, so it is fixed here.

## Why migrate rather than pin or allow

`docs/architecture/DESIGN.md:818` states the policy explicitly:

| MSRV | Latest stable | No old Rust version compat needed |

Pinning the toolchain would contradict that, and a blanket
`allow(chunks_exact_to_as_chunks)` would suppress a lint that is
pointing at
real code quality. So the code is migrated.

## What the migration actually does

`as_chunks::<N>()` yields `&[T; N]` where `chunks_exact(N)` yielded
`&[T]`. That
has a pleasant consequence: the fallible conversions call sites had to
perform
are not merely redundant afterwards, **they no longer type-check**. So
most of
this diff is deleting them:

```rust
-.map(|bytes| f32::from_le_bytes(bytes.try_into().unwrap()))
+.map(|bytes| f32::from_le_bytes(*bytes))
```

Each one removed a `panic!` branch that could never fire.
`onnx-runtime-comm`'s
reductions, `onnx-runtime-ep-cpu`'s quantization/qlinear paths,
`onnx-runtime-shape-inference`, `onnx-std` and the session tests all
shed one.

Where the old code used `ChunksExact::remainder()`, `as_chunks` returns
the
remainder as `.1`, so the iterator no longer has to be threaded through
`by_ref()` and then re-consulted:

```rust
-let mut weight_chunks = weight.chunks_exact(LANES);
-let mut activation_chunks = activation.chunks_exact(LANES);
-for (w, a) in weight_chunks.by_ref().zip(activation_chunks.by_ref()) { ... }
-for (w, a) in weight_chunks.remainder().iter().zip(activation_chunks.remainder())
+let (weight_chunks, weight_tail) = weight.as_chunks::<LANES>();
+let (activation_chunks, activation_tail) = activation.as_chunks::<LANES>();
+for (w, a) in weight_chunks.iter().zip(activation_chunks) { ... }
+for (w, a) in weight_tail.iter().zip(activation_tail)
```

(`dot_u8_f32` in `matmul_nbits.rs`, and `PipelineCacheKey::absorb` in
`onnx-genai-engine`, which is a hash — the chunk sequence and the
remainder
handling are unchanged, so the digest is unchanged.)

**One genuine false positive is annotated, not changed.**
`onnx-runtime-ir`'s
`read_vec_le` chunks by `T::BYTE_SIZE`, an associated const of a
*generic
parameter*, which is not permitted as a const generic argument on stable
—
clippy's own suggestion does not compile there. It carries an `#[allow]`
with a
`reason`.

## Two hazards worth flagging, since this was partly mechanical

1. **Clippy's suggestion for `chunks_exact_mut` is
`as_chunks_mut::<N>().0.iter()`,
which silently drops the mutability the call site needs.** Applying it
verbatim
would not compile in `onnx-runtime-comm::reduction` (the loop body calls
   `copy_from_slice`). Every mutable site uses `.iter_mut()` here.
2. **One pass is never enough.** A `chunks_exact` inside the same
expression as
another is masked until the first is fixed, so the migration was driven
to a
fixed point and the gate re-run from a forced-fresh analysis (clippy
caches
diagnostics, and an unchanged tree reports nothing on a second run —
which
   will happily look like success over a dirty tree).

## Local validation

The **exact** CI clippy command (all 30 crates, `--all-targets -- -D
warnings`)
is clean, as is the same gate over `onnx-genai-cli`,
`onnx-genai-engine`,
`onnx-runtime-ep-plugin` and `onnx-runtime-ep-cpu-plugin`. `cargo fmt
--all --check`
is clean.

Tests, on `stable` 1.98.0:

| suite | result |
| --- | --- |
| `onnx-runtime-ep-cpu --lib` | **1551 passed, 0 failed** |
| `onnx-runtime-ep-plugin --lib` | 310 passed |
| `onnx-runtime-ep-cpu-plugin --test plugin_ort_e2e --include-ignored` |
**57 passed** (ORT conformance) |
| `onnx-runtime-session` | all suites ok |
| `onnx-genai-engine --lib` | 433 passed |
| `onnx-runtime-comm`, `-ir`, `-shape-inference`, `-eager`, `-capi`,
`onnx-std`, `onnx-genai-preprocess` | all ok |
| aarch64 (`aarch64-unknown-linux-gnu` under qemu) `onnx-runtime-ep-cpu
--lib` | see below |

The kernel crates are where the numeric risk is, and 1551 kernel tests
plus the
57-case ORT conformance run cover it.

## Note on scope

This touches crates outside my usual area (`onnx-runtime-comm`,
`onnx-std`,
`onnx-genai-engine`, `onnx-runtime-session`,
`onnx-runtime-shape-inference`,
`onnx-runtime-capi`, `onnx-genai-preprocess`). That is not
empire-building — the
gate is all-or-nothing, so `Fast` cannot go green for anyone until every
hit is
addressed. The changes there are mechanical and each is covered by that
crate's
own tests.

---

## Independent review found a BLOCKER, now fixed

Reviewed adversarially (Opus, independent context). It found a real
defect I had
missed, and it is the interesting kind:

> The PR migrated every hit that appears under the **default** feature
set, but
> `native_decode`/`native_component` are `#[cfg(feature =
"native-backend")]` and
> are never compiled by the default clippy lanes. `Rust quality` has a
dedicated
> step for exactly that (`ci.yml:416`), so ten more sites kept that gate
**red**.

That step exists because of a previous incident — its comment says
"without this
step a refactor can leave it uncompilable while every other job still
passes".
It caught me the same way. Fixed in `83d7bc9ee`, along with a **second**
new-1.98
lint the same step exposes, `clippy::manual_slice_fill`, in
`native_decode/cuda.rs`
(clippy's suggestion there, `&mut self.row_lens.fill(..)`, is malformed
— the
borrow is not wanted, `self.row_lens.fill(..)` is).

Lesson recorded: for a lint migration, "clean under default features" is
not the
gate. Every feature-gated lane has to be run.

The review verified and found **no** behavioural regression in the rest,
in
particular the two sites I was most concerned about:

* **`PipelineCacheKey::absorb` (a cache-key hash)** — same 8-byte words
in the
same order, same zero-padded tail, so the digest is unchanged and cache
keys
  stay stable.
* **`reduction.rs`** — mutability preserved (`iter_mut`, not clippy's
suggested
  `iter`); confirmed by running the crate's own falsifiers, including
`distributed_all_reduce_matches_single_device_bitwise`, which a
non-mutating
  iterator would turn into a no-op.

It also confirmed `dot_u8_f32`'s tail arithmetic, that every dropped
`try_into()`
was a genuine `&[T; N]` → `[T; N]`, that the `read_vec_le` `#[allow]`
justification
is true, that the reformatting commit is formatting-only, and that the
ARM sweep
test's `covered` is unchanged.

## Full local validation

Gates, all on `stable` 1.98.0 (installed locally to match CI — my
default was
1.97.1, which is precisely why I could not see this lint at first):

| gate | result |
| --- | --- |
| `Fast` clippy (30 crates, `--all-targets -- -D warnings`) | clean |
| `Rust quality` clippy (same set) | clean |
| clippy `-p onnx-genai-engine --features native-backend` | clean |
| clippy `onnx-genai-cli`, `onnx-runtime-ep-plugin`,
`onnx-runtime-ep-cpu-plugin` | clean |
| `cargo fmt --all -- --check` | clean |
| `scripts/check_cross_compile.sh` (x86_64 + real aarch64 pass) | pass |
| the nine `Rust quality` python gates (publish order, dispatch
manifest/reachability, feature-gate coverage, env vars, …) | 9/9 pass |

Tests:

| suite | result |
| --- | --- |
| `onnx-runtime-ep-cpu --lib` | **1551 passed, 0 failed** |
| same, **aarch64** under qemu | **1455 passed, 0 failed** |
| `onnx-runtime-ep-cpu --features mlas kernels::moe::` | 19 passed |
| `onnx-runtime-ep-cpu --features mlas kernels::qlinear_matmul::` | 30
passed |
| `onnx-runtime-ep-cpu --features mlas` registry config | 1 passed |
| `onnx-genai-engine --lib --features native-backend` | 567 passed |
| `onnx-genai-engine --lib` | 433 passed |
| `onnx-runtime-ep-cpu-plugin --test plugin_ort_e2e --include-ignored` |
**57 passed** (ORT conformance) |
| `onnx-runtime-ep-plugin --lib` | 310 passed |
| `onnx-runtime-session`, `-comm`, `-ir`, `-shape-inference`, `-eager`,
`-capi`, `onnx-std`, `onnx-genai-preprocess` | all ok |

**Disclosure of missing scope:** `onnx-genai-engine`'s *integration*
test binaries
(`iterative_pipeline_e2e`, `vlm_pipeline_e2e`, …) could not be linked
locally —
the build host's disk is shared and repeatedly hit 100%, and `ld` died
with a bus
error. The library tests for that crate ran (567 with `native-backend`,
433
without) and cover the changed code; CI will link the rest. No local red
anywhere.

---

## Why `main` looks green when it is not

Worth recording, because it misled me for a while and will mislead the
next
person: `Fast` is **passing on `main`** right now, on rustc 1.98.0, with
108
`chunks_exact` sites still in the linted crates.

It is passing because its clippy step never *checked* those crates. The
step
compiled 244 crates from a restored cache, and `onnx-runtime-ir`,
`onnx-runtime-ep-cpu` and `onnx-runtime-comm` are not among them — they
were
already fresh, and **cargo does not re-emit diagnostics for a crate it
did not
rebuild**. The lint is a default-`warn` lint promoted by `-D warnings`,
so a
cached crate is indistinguishable from a clean one.

The moment a crate is genuinely re-checked, it fires. That is exactly
what
happened on #1587, whose log shows the error arriving on the line after
`Checking onnx-runtime-ir`. So the situation is not "main is fine and
this PR is
cleanup" — it is that **main is one cache eviction away from a repo-wide
outage**,
and in the meantime every PR that touches one of these crates goes red
for a
reason that has nothing to do with its own change.

Both my own PRs died this way before I understood it.

---

## Second blocker found while landing this: a bf16 route assertion (now
fixed on main)

`Fast` on this PR then failed in a place my diff does not touch
(`matmul.rs`,
zero lines changed):

```
BFloat16 m=2: prefill did not take the fused widen-pack GEBP
  left: 0
 right: 1
```

`half_prefill_gebp_agrees_with_the_blocked_half_gemm_and_is_the_route`
asserted
that bf16 prefill takes the fused widen-pack GEBP. On any host with
**AVX-512
BF16** it does not: `half_gemm_tile` hands bf16 to the native
microkernel and
returns before the GEBP is reached, so the route counter is legitimately
0. The
f16 arm has no such interception, which is why only the bf16 rows
failed.

My CPU has no AVX-512 at all, so it passed locally and failed on runners
that
have the *faster* kernel -- deterministic per runner, indistinguishable
from
flake across a heterogeneous pool. Confirmed rather than argued: forcing
the
bf16 early return on my host reproduced the CI panic **verbatim**,
including
`m=2` and `left: 0`.

I fixed it here, and then a fix for the same defect landed on `main`
while this
PR was in the queue. **Main's is better** -- it derives the expectation
from the
very predicates `half_gemm_tile` dispatches on, instead of restating the
hardware assumption, and it treats the no-AVX2 host as a legitimate `(0,
0)`
rather than a skip. So I took `main`'s version wholesale and dropped
mine; this
PR now carries no change to `matmul.rs` at all. Recording the diagnosis
here
anyway, since the reproduction is the part that was expensive.

Re-validated after taking main's version: ep-cpu **1555 passed** on
x86_64,
**1455 passed, 0 failed** on aarch64 under qemu, ORT conformance **57
passed**,
engine `--features native-backend` **568 passed**; fmt and every clippy
gate
still clean.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby added a commit that referenced this pull request Aug 21, 2026
… driver (MTP E2E foundation) (#1633)

## Summary

Lands the **recurrent-state correctness fix** in the native-backend
speculative driver — the piece that finally exercises the #1598
GDN-state commit primitive on the driver that actually runs the real
dense-hybrid Qwen3.8-27B — and corrects the campaign's architectural
model for where MTP plugs in on the native backend.

Working as **Gaff** (CUDA/Rust engine specialist). Foundation for MTP
self-speculative decode on hybrid GDN+GQA models.

## Architectural correction (important for the campaign)

The native backend does **not** reach `generate_speculative_loop`
(`speculative/mod.rs`); that loop is ORT-session-only, and
`DecodeRunner::Native` is never constructed in production (dead
scaffolding). The real native speculative path is
**`NativeSpeculativeDriver`** (`native_speculative.rs`), dispatched from
`generate_native_cold_with_callback` via `native_speculation_plan`. This
is where MTP and the #1598 recurrent-commit primitive belong — not the
three loop locations originally hypothesized.

## What changed

- The driver's accept path called `rewind(base + accepted)`, which only
prefix-slices attention KV and left a hybrid decoder's destructive
Gated-DeltaNet recurrent (SSM) + conv1d state stranded at `base + K`
after each verify window — silently corrupting every subsequent token on
hybrid models under speculation.
- Now the driver **snapshots** the recurrent/conv state at the committed
boundary before `decode_verify`, and on accept **commits** it to exactly
the accepted prefix via the #1598 `commit_recurrent_state_to_accepted`
primitive (snapshot restore + accepted-token re-advance, which also
performs the KV rewind).
- Detected generically via `has_recurrent_state()` — **no hardcoded
dims/layers**. Pure-attention decoders keep the plain `rewind` and are
byte-identical to before, so the greedy (non-speculative) path is fully
inert.

## Tests

- New
`native_verify_then_recurrent_commit_matches_accepted_prefix_replay`:
drives the exact `snapshot -> decode_verify(K) -> commit(j)` sequence
for `j in {0,1,k}` on a synthetic hybrid decoder and asserts the
committed recurrent state is **byte-identical** to a fresh `base ++
draft[..j]` replay. Includes a **negative control** proving a plain
`rewind` strands the state (the fix is load-bearing, not cosmetic).
- Full native-backend lib suite: **569 passed / 0 failed / 1 ignored**
(`cargo test -p onnx-genai-engine --no-default-features --features
native-backend --lib`). 568 baseline + 1 new.

## MTP E2E enablement (Gaps 1 & 2) — deliberately NOT in this PR

⚠️ **This PR lands the solid correctness foundation only.** The two
remaining MTP-enablement gaps are real and localized but **cannot be
compiled or validated in the CI/dev sandbox** (no CUDA toolkit: `which
nvcc` → not found; the 17 GB artifact is not reachable), so per campaign
policy they are **not half-wired** here:

1. **Gap 1 (native MTP proposer + load wiring):**
`from_native_model_directory` (`engine/load.rs`) hard-bails on metadata
speculation and hard-sets `mtp: None`. Needs a `NativeProposer::Mtp`
variant (mirroring `SharedKv`, using `onnx-genai-ort::MtpDecodeSession`
+ `LinearEmbedder`/lm_head + the target's `last_hidden()` seed) plus
`reject_native_request_speculation` / `native_speculation_plan`
(`engine/decode_backend.rs`) and the `speculative_mode` injection in
`generate_native_cold_with_callback` extended for
`SpeculativeMode::Mtp`. `mtp_config_from_metadata` already exists.
2. **Gap 2 (aux hidden-seed shape rigidity — the true architectural
blocker):** the MTP seed is the target's `hidden_states.63`, an
auxiliary graph output. `persistent_output_shape`
(`native_decode/cuda.rs`) collapses its symbolic query-seq axis to 1 —
correct for the m=1 captured decode step, but the resulting `[1,1,5120]`
binding cannot hold the eager prefill shape `[1,m,5120]` (the
`dispatch.rs` rejection Sebastian observed). The fix must materialize
the aux hidden output per-step to a host buffer sized to the actual seq
during m>1 eager forwards (mirroring the logits FIX-1 padded binding)
while keeping the seq=1 persistent binding for captured decode. This is
**CUDA-graph-capture-sensitive executor plumbing that must be validated
on the GPU against the 62.56 tok/s baseline**; Gap 1's proposer is
useless until Gap 2 delivers a valid hidden seed.

**No tok/s number is reported** — it cannot be honestly measured without
the GPU + artifact, and I will not fabricate a speedup. Recommend
re-scoping Gaps 1 & 2 to an H200 (ordinal 5) session with the CUDA
toolchain and artifact mounted. The recurrent-commit correctness fix
here is the enabling foundation.

Validated by unit test (byte-identity oracle + negative control);
end-to-end MTP-on-hybrid pending the GPU session above.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant