Repository navigation
feat(mtp): per-row cache_len in the fused step — the pipeline blocker #100 named - #102
Merged
Merged
Conversation
#100 named `step_full` keyed the whole fused step on one scalar `cache_len` (the BATCHED buffer's width, which left-alignment makes `max_j L_j`). All four of PR #100's defects close here: * `uncached[i]` no longer saturates to 0 — `plan_step_window` takes each row's own length, so every ragged tail stays inside the window; * `toks[cache_lens[i]..]` is in range by the same invariant, closing the panic site the saturation was hiding; * `seqlen_offsets` routes per row through `inputs_processor` — which is what feeds `dsv4_attention`'s `row_q0`; * the per-sequence commit strips the dead prefix (`drop_dead_prefix`, the inverse of `front_pad_kv_cache`) so a row's recorded length is its own. Flag-off byte-identity is structural: `resolve_row_cache_lens` and `resolve_row_offsets` return `None` for a uniform batch, and `None` runs the pre-change scalar code verbatim. `KvAdvance::PerSequence` is still refused. The refusal moved to the MTP DRAFT chain, which keys a whole group on one absolute position. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Code Metrics Report━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Language Files Lines Code Comments Blanks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ C Header 5 305 210 52 43 CSS 2 1181 1036 34 111 CUDA 72 24328 17592 4018 2718 Dockerfile 1 39 22 8 9 JavaScript 16 3546 2676 482 388 Jinja2 7 694 656 5 33 JSON 74 4600 4597 0 3 Makefile 1 6 5 0 1 Metal Shading Lan| 33 12224 9431 1142 1651 PowerShell 1 300 227 30 43 Python 143 14830 12217 797 1816 Shell 23 5756 3964 1412 380 Plain Text 4 3801 0 2479 1322 TOML 33 1485 1292 43 150 YAML 3 25 23 2 0 ───────────────────────────────────────────────────────────────────────────────── HTML 4 2687 2604 43 40 |- CSS 2 543 479 37 27 |- JavaScript 1 1233 1215 12 6 (Total) 4463 4298 92 73 ───────────────────────────────────────────────────────────────────────────────── Jupyter Notebooks 4 122 83 23 16 |- Markdown 1 60 30 22 8 |- Python 1 122 113 1 8 (Total) 304 226 46 32 ───────────────────────────────────────────────────────────────────────────────── Markdown 190 37932 0 29111 8821 |- BASH 71 1623 1192 315 116 |- C 2 12 12 0 0 |- CUDA 2 84 56 16 12 |- JSON 18 708 708 0 0 |- PowerShell 1 1 1 0 0 |- Python 23 1008 787 113 108 |- Rust 65 2048 1713 77 258 |- TOML 6 207 164 0 43 |- YAML 4 38 33 5 0 (Total) 43661 4666 29637 9358 ───────────────────────────────────────────────────────────────────────────────── Rust 662 309912 268369 14069 27474 |- Markdown 477 23482 471 20234 2777 (Total) 333394 268840 34303 30251 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Total 1278 454942 331978 74582 48382 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ |
This was referenced Aug 17, 2026
heydryft
added a commit
that referenced
this pull request
Aug 17, 2026
…ort it PR #93 landed `XsRollingCache::trim_tail_to(new_base: usize)` and `reconcile_xs_bases` on master while this branch was making `base`/`tokens` per-row `Vec<usize>`. The 7 resulting errors are semantic, not textual: `base` is the same identifier naming two different quantities. - On master `tail` is `tokens - base` wide, so `base` IS the physical left edge of the buffer. Divergent `base` at equal `tokens` gives physical widths 4 vs 132 and the `slice_set` mismatch #93 exists to fix, reachable on plain decode through the prefix cacher. - Here `tail` is `[B, W, hidden]` and end-anchored, with `base[i] >= tokens[i] - W`. `base` is a logical resume point decoupled from the buffer; `BatchSrc::of` sets `v_slack_dim: Some(1)` with `v_slack_at_front: true`, so the widths already agree. So the reconciliation is required on one path and a regression on the other. Trimming every row to `base_max` raises the shortest-reach row's rollback floor to the batch's -- a per-row quantity replaced by a batch-wide scalar, the same shape as the cohort min-rollback #92 removed and the padding traps #102/#103/#104 each caught. This is the last layer still holding one. - `reconcile_xs_bases` takes `xs_per_seq` and returns early when set. Flag off it runs verbatim; #93's two mutation-proven tests pass unchanged (they run flag-off), with only a private-field read swapped for the `resumable_from()` accessor, which is `base[0]` for a single-row cache. - `trim_tail_to` REFUSES a multi-row cache rather than reading `base[0]`. A scalar `new_base` cannot describe a trim of rows at different resume points, and proceeding returns a right-shaped tensor -- exactly what the caller checks -- so it names the reason instead (D18). `per_row_xs_bases_survive_a_batch_round_trip_and_are_not_flattened` is the test that discriminates: flag on, two rows at divergent `base` and equal `tokens` must batch AND each keep its own `base` through `split_row`. Both mutations were run rather than assumed -- trimming the tensors is caught by the width assertion, but reconciling only the logical `base` passes every shape check and fails solely on the per-row read. 8/8 mutations caught. One survivor on the first pass (`trim_tail_to` narrowing from the wrong end) was real: no test read tail CONTENT, because every fixture fed zeros. `trimming_the_retained_window_drops_the_oldest_rows_not_the_newest` feeds a ramp and closes it. 430 lib tests pass, 14 synthetic, workspace check clean, scoped clippy exits 0, zero new clippy diagnostics in the touched files, and rustfmt drift is 6 before and 6 after -- no upstream reformat churn. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Admin-merged without review (no second reviewer). Bypass covered the review requirement only — the required Merged without |
heydryft
added a commit
that referenced
this pull request
Aug 18, 2026
Master moved 10+ commits since 120f348, including #100 (v4 ragged mask wire) and #102 (per-row cache_len in the fused MTP step) — adjacent territory. Merged now rather than letting the divergence be discovered later. Clean auto-merge; the only overlapping file is `kv_cache/mod.rs`. 466/466 green on the merged tree (441 mine + master's new tests). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
heydryft
added a commit
that referenced
this pull request
Aug 18, 2026
feat(mtp): per-row positions in the MTP draft chain — the blocker #102 named
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #100 (
feat/v4-ragged-mask-wire), which is stacked on #95, which is stacked on #92. Review those first.The blocker, in one sentence
#92 named two blockers; #95 removed the first, #100 removed the second and named this one. This removes it.
cache_supports_per_sequence_advance,model_masks_ragged_batchesand this step's own arithmetic all pass for a V4 target now.🔑 The per-row vector already existed —
clone_in_cachewas throwing it awayfront_align_batchreturnslead_pad[i], the dead prefix it left ahead of rowi.MtpSpeculativePipeline::clone_in_cachebound it to_lead_padand dropped it. Oncefront_pad_singlehas run, the slot's owncurrent_seq_lenis the batch width for every row — so that discarded vector is the only surviving record of who is actually where, and discarding it is what forced every downstream quantity onto one shared scalar.resolve_row_cache_lenshands it straight back:cache_lens[i] = cache_len - lead_pad[i]. Nothing is plumbed, noHashMap<seq_id, …>appears, no signature changes through the engine. Same shape as #100's discovery thatseqlen_offsets[i]was already the per-rowq0.The four defects, and how each closes
uncached[i] = toks_i.len() - cache_lensaturates to 0 for every row but the longest ⇒window_okfails on every ragged stepplan_step_windowtakes each row's own length; every tail is 1, which is the B=1 invariant at every batch sizetoks[cache_len..]is a panic site oncecache_len > toks_i.len()toks[cache_lens[i]..], andoknow implies the slice is in range (u_i >= 1⟺cache_lens[i] < tok_lens[i])prefill_window = Some((w, cache_len))collapsesseqlen_offsetsto one scalar (last_n_context_len.1), so therow_q0#100 built is fed uniform valuesinputs_processor— see belowset_len(cache_len + c_i)counts the dead prefix inside the length, so it accumulates linearlydrop_dead_prefixstrips it first; the recorded length iscache_lens[i] + c_i, the row's own🔑 Defect 4: the code was wrong, not the doc
front_pad_single's doc claims the dead prefix tracks asqrt(steps)random walk. That claim is a property of the code, not of the batch, and it only holds if the prefix is taken back out on the way to the per-sequence caches.It was not.
front_pad_singlesetscurrent_seq_len = target_len(it has to — that is where the one shared append offset comes from) andclone_out_cachecopies that length verbatim into every row. Recording it as the sequence's own makes the nextfront_align_batchpad relative to already-padded lengths, so each step adds anothermax_j c_j - c_icolumns.Worse than "it grows": the rows re-converge onto one length. Every row's recorded length ends up advancing at the fastest row's rate — the cohort barrier this whole workstream exists to remove, reappearing as padding.
the_dead_prefix_does_not_accumulate_across_stepsruns the loop at 6 and 12 steps and pins the inflation at 10 and 22 columns; with the strip it is 0 at both.So the fix is
drop_dead_prefix, the inverse offront_pad_kv_cache— a ~25-line symmetric primitive that restores the invariant everything else already assumes (front_pad_single's ownnarrow(dim, 0, live)requires the live run to start at column 0). No new field onSingleCache, andlead == 0— every B=1 request, every uniform batch — touches no tensor at all.🔑 Defect 3:
chunk_offset_tokswas the flattening, one layer downget_prompt_inputdidlet offset = input_seqs[0].token_offset();— a per-sequence value collapsed to row 0 — andmake_prompt_chunkthen used it forseqlen_offsets,position_ids,seqlens_kand the paged slot mappings. Every one of those wants per-row.So the trait signature is untouched (
last_n_context_len: Option<(usize, usize)>stays; ~15 vision processors are not churned).make_prompt_chunkgainsrow_offsets: Option<&[usize]>which shadowschunk_offset_toksinside the per-sequence loop, and the value reaches it onSequencenext toprefill_prompt_toks— the field whose doc already says "only meant for internal speculative decoding usage".reset_prefill_toksclears both, so an offset can never outlive the window it describes.Why "flag off is byte-identical" is structural
Two dispatch functions, both returning
Nonefor a uniform batch, andNoneruns the pre-change code verbatim:resolve_row_cache_lens—Noneunless some row carries a lead and the mode is authorized;resolve_row_offsets—Noneunless some sequence carries its own offset.No fourth flag. The gate is
ARC_MTP_PER_SEQ_KV, coordinated with #92 and #95 exactly as #100 did it:front_align_batchis the only producer of a ragged dense batch and it runs only underKvAdvance::PerSequence.The draft grouping key moved from
uto(u, chain_start). On a batch that shares a width the chain start is a function ofualone, so theBTreeMappartitions and iterates identically — but keying onualone under raggedness would not be slow, it would be silent: a row whose seed is not at the group's chain start is skipped, so it would stop drafting with no signal but a fallingtok_per_step.window_ok's deferral is no longer reachable for a padded batch — it refuses instead. #100 was right that the deferral lands in the target's own decode, which builds no mask att_q == 1(layers_masker.rs:285), so handing it a left-aligned batch would attend the dead prefix as real keys. Deferring is only safe for a batch that is not padded.📊 Measured — CPU unit tests and plan arithmetic, not hardware (D14)
cargo test -p mistralrs-core --lib→ 423 passed, 0 failed (was 407 on #100's tip) ·--test synthetic_load_smoke→ 13 passed ·cargo check --workspace --testsgreen · scoped clippy lane green ·mistralrs-coreclippy introduces zero new diagnostics (diffed before/after on the touched files) · zero rustfmt drift, checked like-for-like.Mutation runs — 17 mutations, all 17 caught
the_dead_prefix_does_not_accumulate_across_stepscomputed the post-strip live length itself (target - lead) instead of reading it back from the slot — so it asserted my arithmetic against my arithmetic and passed withdrop_dead_prefixstubbed to a no-op. It now readsslot.current_seq_len()back and asserts the live run's content is at column 0, which is what makes the no-op, the length-preserving and the narrow-from-0 mutations all fail. This is the same trap in a fourth disguise (#95's sharedcompcolumn, #98's non-binding margin, #100's invisible uniform dispatch): the fixture could not reach the condition the code was written for.🔴 The honest gap —
PerSequenceis still refused, and the refusal moved AGAINThis does not move the B=128 number. With the target's time base now per-row end to end, a fourth blocker is visible that none of #92, #95 or #100 could see: the MTP draft chain is a second, independent time base, and it is still keyed on one scalar. Four sites:
MtpHiddenCapture::store(seqlen_offsets.first(), ..)(deepseek4.rs:3985) records row 0's absolute position for the whole capture, soextend_draft_kv_row'soffdescribes only the leading row;propose_chain_batched(.., start_pos, ..)takes ONEstart_posand handsstart_pos + itoMtpBlock::forward_step, whoseposis both the RoPE position and the draft-KV slot;batch_draft_cachesrefuses rows whosecurrent_seq_lendiffers — under per-sequence advance that is all of them;u_i ≡ 1and every row at its own length that is one group per sequence:Bsingle-row MTP-block forwards per chain step instead of<= depth + 1batched ones.the_draft_group_partition_collapses_when_the_rows_carry_their_own_lengthscounts it: 8 rows on a shared width → 4 groups; 8 rows at their own lengths → 8.Correct and
B× slower is not a mode worth granting, and granting it would hide the real fix — a per-rowposinMtpBlock::forward_step(a positions vector for RoPE plus a per-row draft-KV slot) and a per-row capture offset. That is a model change indeepseek4, which is why it is not here. It is stated asdraft_chain_carries_per_sequence_positions()so the next change flips exactly one thing.I am not claiming this is the last blocker. Four agents have now made that claim and been wrong; the useful output of each wave has been naming precisely where the refusal moved.
🔴 The GPU ask
Nothing changes a default, so this is a no-regression check on the box already serving V4. I did not run it (D15 — I never call
runcrate).The one number: decode tok/s in B as a fraction of A, at B=1 and B=128; the claim is
B/A == 1.00at both, because the refusal means B still takes the cohort path. Andgrep "cannot honour it" /tmp/b.txtmust now cite the draft chain and must contain none ofXsRolling, the ragged mask,cache_lenorwindow_ok— if it names any of them, this wave did not land.Noticed, not shipped
drop_dead_prefixnarrows-and-contiguates, then the nextfront_align_batchallocates and shifts again. GivingSingleCachealive_startwould fold both into one — but it touchescurrent_data/append/k()/v()on the hot path, so it is its own change. B=1 pays neither copy (both are no-ops atlead == 0).drop_front_singleshrinkscapacity_seq_lenby the lead, so a stripped slot hitsCACHE_GROW_SIZEgrowth marginally sooner than an unstripped one. Bounded and correct, but a capacity that only ever ratchets down is worth a look.kv_advance's doc still links[Self::target_masks_ragged_batches], which does not exist — the function is the freemodel_masks_ragged_batches. Fixed on the one line I touched; two other references remain.