Repository navigation
feat(pipeline): drive the decoder via a stateful PipelineDecoderComponent trait (native multi-component inc2a) - #478
Conversation
…#384) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nent trait (native multi-component inc2a) Inc1 (#450) routed the stateless every_step components through the backend-neutral ComponentSession trait. The decoder cannot reuse that seam: it is stateful — its KV cache grows across steps and, for the native backend, lives device-resident — so a stateless host round-trip would drop KV continuity and re-stage the whole cache every step. Inc2a introduces the stateful counterpart, PipelineDecoderComponent: the flat autoregressive decode loop calls step() once per token and the implementation retains its own per-step outputs, so the loop never touches a concrete tensor type. OrtPipelineDecoder is the ONNX Runtime implementation, behaviour-identical to the previous inline run_decode_step_with_extra / mirror / extract path. A native decoder keeping KV device-resident is the follow-up (Inc2b); see .squad/decisions/inbox/mary-pipeline-inc2-design.md. This is a pure refactor: no native decoder yet, ORT token output unchanged. PipelineDecodeLoopBackend now holds `decoder: Box<dyn PipelineDecoderComponent>` instead of a concrete `&Session` + `&mut DecodeState`, and next_logits() drives it entirely through trait methods. The now-redundant one-line extract_next_token_logits_with_io wrapper is removed in favour of the slice-based extract_next_token_logits_from_outputs (no per-step clone). Proof the ORT path is unchanged: - new unit test pipeline::decoder_component::tests:: ort_decoder_component_matches_inline_step_path asserts the trait wrapper's logits equal the inline helper path bit-for-bit across a 3-token prefill + two decode steps on tiny-multiaxis-state-decoder. - flat-AR e2e token-id goldens unchanged: gemma4_vlm (embedding every_step + decoder -> [0,5,6,7]), vlm_multibinding (2), multimodal_reuse (14), optional_modality (8), pipeline_executor (1). - Inc1 native every_step parity still green. Refs #384. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #478 +/- ##
==========================================
+ Coverage 80.56% 81.52% +0.95%
==========================================
Files 314 314
Lines 122637 122637
Branches 122637 122637
==========================================
+ Hits 98804 99975 +1171
+ Misses 19814 18637 -1177
- Partials 4019 4025 +6
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
VERDICT: APPROVE Independent review of PR #478 — Inc2a stateful This is a genuine pure refactor. Verified with evidence against 1. Behavior-identical guarantee — REAL, not tautologicalThe equivalence test
2. Stateful seam correctness — CONFIRMED
3. Removed wrapper / slice extraction — NO off-by-one
4. Verify
Clean stateful seam, no functional change, all evidence green. Approving. |
✅ Benchmarks — No RegressionComparison of criterion micro-benchmarks: PR head vs merge-base, measured on the same runner in the same job (base first → PR second).
Visual flags: Host infoWhat this cannot catch
|
… (native multi-component inc2b) Inc2a (#478) introduced the stateful PipelineDecoderComponent trait and drove the ORT decoder through it. Inc2b adds the NATIVE counterpart: NativePipelineDecoder wraps NativeDecodeSession and keeps its KV cache session-resident across every step() call, so the expensive KV state never round-trips through the host pipeline pool. The same flat autoregressive decode loop drives either backend through the trait with no forked code path (the DRY principle of inc1/inc2a, now stateful). Per-step seam: each step the every_step embedding component publishes inputs_embeds into the host pool as an ort::Value; the native decoder receives it as one small routed input (one token's embedding, converted value -> ComponentTensor -> native Tensor, reusing the inc1 value seam), while the KV cache stays inside the native session. Native selection is gated behind ONNX_GENAI_PIPELINE_NATIVE_DECODER (names the decoder component or a truthy token), mirroring inc1's ONNX_GENAI_PIPELINE_NATIVE_STEP_COMPONENTS. Unset keeps the ORT decoder (default, unchanged). Requesting it without --features native-backend is a clear error. The native decoder runs the non-paged, fresh-decode path (paged_enabled=false, reused=0): its KV is session-resident and not exposed as host present tensors, so paged present-KV mirroring and cross-request reuse are deferred to inc3 — this changes cross-request KV reuse only, never the tokens produced within a generation. What changed: - pipeline/decoder_component.rs: NativePipelineDecoder impl of the trait (step converts extras + calls decode_with_step_inputs; next_token_logits returns the retained final row; mirror_last_present_kv is unsupported and never reached on the non-paged native path). cfg(native-backend). - native_decode/load.rs: thread the pipeline ModelIoSpec through a new pub(crate) load_with_io so an inputs_embeds decoder (no token input) binds sequence source / KV pairs from metadata instead of guessing. - native_component.rs: pub(crate) component_tensor_to_native_tensor seam. - pipeline/mod.rs: native_decoder_selected() flag + build_native_pipeline_decoder(). - flat_autoregressive.rs: select native vs ORT decoder; force non-paged fresh path when native. Split: Inc2b-i (NativeDecodeSession::decode_with_step_inputs + resident KV) already existed in tree and is proven by the native_decode tests; this PR is Inc2b-ii (the adapter + wiring + parity proof). See .squad/decisions/inbox/mary-pipeline-inc2b-design.md. Token-parity proof (native vs ORT, exact ids): - new test native_pipeline_decoder_parity::native_pipeline_decoder_matches_ort_token_ids: ORT decoder -> [0,5,6,7]; native device-KV decoder -> [0,5,6,7] (identical), on tiny-gemma4-vlm (embedding every_step + inputs_embeds decoder). - ORT-path goldens unchanged: gemma4_vlm, vlm_multibinding (2), multimodal_reuse (14), optional_modality (8), pipeline_executor (1). - inc1 native every_step parity still green. - lib: 283 passed (default), 333 passed (native-backend). - clippy clean: default / native-backend / cuda / cuda,native-backend. Refs #384. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… (native multi-component inc2b) (#479) ## What & why Increment 2b of the native multi-component pipeline work (refs #384). Inc2a (#478, now merged) introduced the stateful `PipelineDecoderComponent` trait and drove the **ORT** decoder through it. **Inc2b adds the native counterpart**: a `NativePipelineDecoder` that wraps `NativeDecodeSession` and keeps its KV cache **session-resident across every `step()` call**, so the expensive KV state never round-trips through the host pipeline pool. The same flat autoregressive decode loop drives either backend through the trait with **no forked code path** — the DRY principle of inc1/inc2a, now stateful. > 📌 Originally stacked on #478; **#478 has merged**, so this branch was rebased onto `origin/main` and now targets `main` directly (only the two Inc2b commits remain). ## The per-step seam (why this is the right design) Each step the every_step embedding component publishes `inputs_embeds` into the host pool as an `ort::Value` (routed edge `embedding.inputs_embeds → decoder.inputs_embeds`). The native decoder receives it as **one small routed input** — one token's embedding, `[1,1,hidden]`, converted `value → ComponentTensor → native Tensor` (reusing the inc1 value-type seam). **The KV cache stays inside the native session** and is never uploaded/downloaded from the pipeline pool. That is exactly why the decoder needs a *stateful* seam and cannot reuse the inc1 stateless `ComponentSession` round-trip. ## What already existed vs. what this PR adds - **Inc2b-i (already in tree):** `NativeDecodeSession::decode_with_step_inputs` already accepts routed / `inputs_embeds` per-step inputs and owns KV across steps (`NativeStepInputSource::InputsEmbeds`), proven by the `native_decode` tests. No new kernel work. - **Inc2b-ii (THIS PR):** the `NativePipelineDecoder` adapter + flat-AR wiring + env selection + **token-parity proof**. ## Changes - `pipeline/decoder_component.rs`: `NativePipelineDecoder` impl of the trait (`step` converts extras + calls `decode_with_step_inputs`; `next_token_logits` returns the retained final logits row; `mirror_last_present_kv` is unsupported and never reached on the non-paged native path). `cfg(native-backend)`. - `native_decode/load.rs`: thread the pipeline `ModelIoSpec` through a new `pub(crate) load_with_io`, so an `inputs_embeds` decoder (no token input) binds its sequence source / KV pairs from **metadata** instead of guessing from tensor shapes. - `native_component.rs`: `pub(crate) component_tensor_to_native_tensor` conversion seam. - `pipeline/mod.rs`: `native_decoder_selected()` env flag + `build_native_pipeline_decoder()`. - `flat_autoregressive.rs`: select native vs ORT decoder; force the non-paged, fresh-decode path when native. ## Selection & paging - Flag: `ONNX_GENAI_PIPELINE_NATIVE_DECODER` (names the decoder component or a truthy token), mirroring inc1's `ONNX_GENAI_PIPELINE_NATIVE_STEP_COMPONENTS`. Unset ⇒ ORT decoder (default, unchanged). Requesting it without `--features native-backend` is a clear error. - **Paging deferred to inc3:** native selection runs the non-paged path (`paged_enabled=false`, `reused=0`) because the native KV is session-resident and not exposed as host present tensors. Paging is a *cross-request KV-reuse cache* and does **not** change the tokens produced within a generation, so ORT (paged) vs native (non-paged) token IDs still match. Native present-KV exposure + paged mirroring + vision cross-KV are inc3. ## Token-parity proof (native vs ORT, exact IDs) - **New test** `native_pipeline_decoder_parity::native_pipeline_decoder_matches_ort_token_ids`: ORT decoder → `[0,5,6,7]`; native device-KV decoder → `[0,5,6,7]` (**identical**), on `tiny-gemma4-vlm` (embedding every_step + `inputs_embeds` decoder). - ORT-path goldens **unchanged**: gemma4_vlm (1), vlm_multibinding (2), multimodal_reuse (14), optional_modality (8), pipeline_executor (1). - inc1 native every_step parity still green. ## Verification - `cargo test -p onnx-genai-engine --lib`: **283 passed** (default), **333 passed** (native-backend), 0 failed. - `cargo fmt --all --check`: clean. - clippy clean (no warnings): **default / native-backend / cuda / cuda,native-backend** (cfg-correct imports; all native decoder code behind `cfg(feature = "native-backend")`). Design note: `.squad/decisions/inbox/mary-pipeline-inc2b-design.md`.⚠️ Draft — do not merge. Stops at the largest PROVEN slice (Inc2b-ii, CPU device-KV text decoder with green token parity). Inc3 (native present-KV exposure + paged reuse + vision cross-KV + CUDA target) deferred. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…CUDA-hybrid merge logs (#483) Scribe round 4 state consolidation (squad-internal, no code). Merges 4 design-note inbox drops into decisions/archive, distils 2 standing directives, logs #477/#478/#479/#480 merges, appends histories. decisions.md 25.4KB→28.5KB (older 07-29 entries flagged for next-round distillation). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…s native multi-component decode, #82/#384/35B-A3B) (#546) ## Summary The task was to introduce a backend-neutral ownership seam so the pipeline decode loop can drive **either** ORT **or** native component sessions per step, then route the existing ORT path through it byte-identically (increment-1). **On inspection, that seam already fully exists on `origin/main`** — it landed across the inc1→inc3c chain (#450, #478, #479, #485, #487, #533) and was hardened by #543. `PipelineDecodeLoopBackend` owns **no** ORT `Session`/decode-state; it holds only `Box<dyn PipelineDecoderComponent>` (stateful decoder seam) and `Vec<(_, Box<dyn ComponentSession>)>` (stateless every_step seam), and both ORT and native backends are driven through one decode loop via runtime env selection. So increment-1 here is the piece the chain had **not** locked: a parity test proving the seam's **keystone** end-state — *every declared component running natively at once* (native every_step embedding **+** native device-KV decoder in the same loop), the exact shape a large multi-component package (up to the 35B-A3B 3-component package) decodes through. ## What changed (test-only, zero production change) - **`crates/onnx-genai-engine/tests/native_full_pipeline_parity.rs`** — drives the `tiny-gemma4-vlm` composite with both `ONNX_GENAI_PIPELINE_NATIVE_STEP_COMPONENTS=embedding` **and** `ONNX_GENAI_PIPELINE_NATIVE_DECODER=decoder`, asserting the fully-native run is token-identical to the ORT baseline `[0, 5, 6, 7]`. - **`crates/onnx-genai-engine/Cargo.toml`** — registers the test (`required-features = ["native-backend"]`, CPU-only). - **`.squad/decisions/inbox/mary-pipeline-native-ownership.md`** — full assessment (ownership map), design affirmation, and the deferred next increment. Prior increments proved each slice in isolation: inc1 (native embedding + ORT decoder), inc2b (ORT embedding + native decoder). Nothing exercised **both** natively at once until now. ## ORT byte-identical proof No production source is touched, so the ORT decode path is byte-identical to `origin/main` by construction. Empirically the ORT baseline `[0,5,6,7]` and the fully-native run `[0,5,6,7]` match exactly. ## Tests - `native_full_pipeline_parity` — **pass** (new) - `native_step_component_parity`, `native_pipeline_decoder_parity` — **pass** - 343 engine lib unit tests — **pass**, 1 ignored - `cargo fmt --all --check` — clean CUDA-gated native tests were not run in this CPU environment (unchanged by this PR). ## Deferred to the next increment (native wiring completion) The one genuine remaining hard limitation the code itself flags (`decoder_component.rs:244-260`): `NativePipelineDecoder::mirror_last_present_kv` bails — the native decoder keeps KV session-resident and does not expose host present tensors, so native selection runs the non-paged, fresh-decode path with no cross-request KV reuse. Wiring native present-KV exposure + paged mirroring is higher blast radius and is intentionally **not** bundled here. Refs #82, #384. Working as Mary (native-decode / pipeline engineer). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
What & why
Increment 2a of the native multi-component pipeline work (refs #384). Inc1 (#450, merged) routed the stateless
every_stepcomponents through the backend-neutralComponentSessiontrait. The decoder cannot reuse that seam: it is stateful — its KV cache grows across steps and, for the native backend, lives device-resident — so a stateless host round-trip would drop KV continuity and re-stage the whole cache every step, destroying decode throughput.This PR introduces the stateful counterpart and lands it as a pure, provably-behavior-identical refactor (no native decoder yet), de-risking the native wiring (Inc2b).
The stateful decoder seam
trait PipelineDecoderComponent(pipeline/decoder_component.rs): the flat autoregressive decode loop callsstep()once per token; the impl retains its own per-step outputs, so the loop readsnext_token_logits()/mirror_last_present_kv()without ever handling a concrete tensor type. Same DRY principle as Inc1, but stateful (KV stays inside the backend).OrtPipelineDecoderis the ONNX Runtime impl, behaviour-identical to the previous inlinerun_decode_step_with_extra→ mirror → extract path.PipelineDecodeLoopBackendnow holdsdecoder: Box<dyn PipelineDecoderComponent>instead of a concrete&Session+&mut DecodeState;next_logits()drives it entirely through trait methods.extract_next_token_logits_with_iowrapper in favour of the slice-basedextract_next_token_logits_from_outputs(no per-step clone of retained KV outputs); test callers updated.Inc2a / Inc2b split
.step(), ORT token output UNCHANGED. Pure refactor.NativeDecodeSessionkeeping device-resident KV across steps + native-decoder-in-pipeline token parity. Requires extendingNativeDecodeSession::stepto accept routed/inputs_embedsper-step inputs and expose present-KV — substantial, its own increment.Design note:
.squad/decisions/inbox/mary-pipeline-inc2-design.md(committed 6209729).Proof the ORT path is unchanged
pipeline::decoder_component::tests::ort_decoder_component_matches_inline_step_path: asserts the trait wrapper's logits equal the inline helper path bit-for-bit across a 3-token prefill + two decode steps ontiny-multiaxis-state-decoder.gemma4_vlm(embedding every_step + decoder →[0,5,6,7]),vlm_multibinding(2),multimodal_reuse(14),optional_modality(8),pipeline_executor(1).every_stepparity still green.Verification
cargo test -p onnx-genai-engine --lib: 283 passed, 0 failed, 1 ignored.native_step_component_parity(native-backend): 1 passed.cargo fmt --all --check: clean.