Repository navigation
fix(kv-cache): reconcile XsRolling base — a plain-path batching bug split out of #86 - #93
Merged
Merged
Conversation
Split out of #86 on its own merit. This half needs no MTP to reach and should not be hostage to a superseded speculative-decoding direction; #92 supersedes the rest of #86 (it removes the cohort min-rollback rather than enforcing lockstep around it). The bug: uniform `current_seq_len` is NOT sufficient for `tail` to batch. `tail` is `tokens - base` wide and `base` tracks the sequence's HIGH-WATER length, not its current one -- `set_len` narrows the tail without moving `base`, and `advance` never lowers it. So two sequences at an identical token count can hold tails 4 and 132 wide. That is reachable on ordinary decode: `prefix_cacher` calls `set_len` on every stored layer, so a sequence restored from an entry stored at a greater length holds a narrower tail than one that arrived directly. `clone_in_cache` then batches the two with `slice_set`, which demands an exact dim match -- a second, independent route into the same `shape mismatch on dim 1, ...` failure that #84's `ensure_uniform_batch_cache_lens` closed only half of. The fix, in two pieces: - `XsRollingCache::trim_tail_to` raises `base`, refusing by name if it would cut into rows the next compressed row is built from. `compressor_needs_from` names that boundary once so the retention rule and the trim rule cannot drift apart (`advance` now calls it, with a debug_assert pinning it to the expression it replaced). - `reconcile_xs_bases` trims every member of the batch to the batch's LARGEST base before gathering. Lossless by construction: the member already sitting there proves nothing below it is needed. Both tests are mutation-proven, not merely green. Deleting the `reconcile_xs_bases` call makes `xs_base_divergence_at_equal_lengths_is_reconciled_not_refused` fail with the real `shape mismatch on dim 1, 4 <> 132`; the fixture asserts the lengths AGREE and the widths DISAGREE first, so it cannot silently degenerate into reproducing the ragged-length failure instead. 375 tests pass (373 + these 2), scoped clippy lane green, and the file is rustfmt-clean with no reformat churn in upstream code. 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 140 14489 11930 782 1777 Shell 21 4531 3182 1056 293 Plain Text 4 3801 0 2479 1322 TOML 32 1458 1271 40 147 YAML 3 25 23 2 0 ───────────────────────────────────────────────────────────────────────────────── HTML 3 2500 2461 14 25 |- CSS 1 249 235 6 8 (Total) 2749 2696 20 33 ───────────────────────────────────────────────────────────────────────────────── 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 180 34668 0 26482 8186 |- BASH 63 1477 1111 266 100 |- C 1 10 10 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 64 2043 1710 75 258 |- TOML 6 207 164 0 43 |- YAML 4 38 33 5 0 (Total) 40244 4580 26957 8707 ───────────────────────────────────────────────────────────────────────────────── Rust 652 302564 262171 13459 26934 |- Markdown 467 21514 471 18505 2538 (Total) 324078 262642 31964 29472 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Total 1251 438902 323002 69117 46783 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ |
Contributor
Author
|
Admin-merged without review: |
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>
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.
Split out of #86 on its own merit, per the triage decision. #92 supersedes the rest of #86 — it removes the cohort min-rollback rather than enforcing lockstep around it — but this half needs no MTP to reach and should not be hostage to a superseded speculative-decoding direction.
The bug is on the plain decode path
Uniform
current_seq_lenis not sufficient fortailto batch.tailistokens - basewide, andbasetracks the sequence's high-water length, not its current one:set_lennarrows the tail without movingbase, andadvancenever lowers it. So two sequences at an identical token count can hold tails 4 and 132 wide.That is reachable with no speculative decoding anywhere near it.
prefix_cachercallsset_lenon every stored layer, so a sequence restored from an entry stored at a greater length holds a narrower tail than one that arrived at the same length directly.clone_in_cachethen batches the two withslice_set, which demands an exact dim match.This is a second, independent route into the same
shape mismatch on dim 1, …failure that #84'sensure_uniform_batch_cache_lensclosed only half of — that check compares lengths, and here the lengths agree.The fix
XsRollingCache::trim_tail_toraisesbase, dropping the leading raw rows — and refuses by name if that would cut into rows the next compressed row is built from, rather than silently dropping history.compressor_needs_fromnames that boundary once, so the retention rule and the trim rule cannot drift apart.advancenow calls it, with adebug_assert_eq!pinning it to the expression it replaced.reconcile_xs_basestrims every member of the batch to the batch's largestbasebefore gathering. Lossless by construction: the member already sitting atbase_maxproves nothing below it is needed. The rows dropped only ever shortened how far a rollback could reach, and afterwards every member shares the reach the shared batched cache would have had anyway.The tests have teeth
Not merely green — both were mutation-checked:
reconcile_xs_basescall makesxs_base_divergence_at_equal_lengths_is_reconciled_not_refusedfail with the real error,shape mismatch on dim 1, 4 <> 132. Verified by actually doing it.ensure_uniform_batch_cache_lenswould catch that one and never reach the tensor.trimming_the_retained_window_past_what_the_compressor_needs_is_refusedpins the lossless boundary exactly: trimming tocompressor_needs_from()succeeds, one token past it refuses, and the refusal must name what it would have destroyed.Gates
cargo test -p mistralrs-core --lib→ 375 passed, 0 failed (373 + these 2) · scoped clippy lane green ·cargo checkclean ·kv_cache/mod.rsrustfmt-clean, 183 insertions / 0 deletions, no reformat churn in upstream files.Provenance
Authored by @asmit383's work on #86; extracted, re-verified and mutation-checked during the 2026-08-17 PR triage. Nothing here is new design.