Skip to content

fix(v4): the raw xs window may hold rows shorter than itself — B=8 per-seq advance generates again [MEASURED] - #116

Closed
heydryft wants to merge 7 commits into
agent/arcspec-perseq-measurefrom
fix/xs-ragged-rows
Closed

heydryft wants to merge 7 commits into
agent/arcspec-perseq-measurefrom
fix/xs-ragged-rows

Conversation

@heydryft

@heydryft heydryft commented Aug 17, 2026 •

Copy link
Copy Markdown
Contributor

The defect

ARC_V4_XS_PER_SEQ=1 ARC_MTP_PER_SEQ_KV=1 at B=8 returned 1 token,
finish_reason: None, on 8/8 requests
. Measured twice on an H200 by #114, at
be29f397:

ERROR mistralrs_core::engine: completion step - Model failed with error:
  xs rolling cache: row 3 holds 9 tokens, fewer than the 11-wide retained window
  it would have to end at

Zero occurrences in the OFF control. B=1 with the flags returns the full 48.

Root cause — and a correction to the diagnosis

xs_rolling.rs:427 refused any row whose token count was below the batched
window's width. That is not the invariant the end-anchored layout needs. This
file's own doc already stated the right one
— base[i] >= tokens[i] - W, i.e.
the row's promised run fits in the window — and the code checked tokens >= W
instead, which is sufficient to keep the offset arithmetic inside usize
rather than necessary for correctness.

So the fix is not the shape #114 ranked first. The per-row extents already
exist
: tokens/base are already Vec<usize>, clone_in_cache already
front-pads a short row into the shared window (v_slack_at_front: true →
pad_slack), and split_row already re-anchors on the way out. Every piece of
option (1) is in the tree. What was wrong is the invariant that gates them, and
the arithmetic that forced the wrong invariant.

The trigger is prompt-length diversity, not generation length: W is sized for
the greediest row, so the moment prompts differ, a shorter row's entire history
is narrower than W and the check fires at step 1.

What changed

Three edits, all in the representation. None relaxes a check — two tighten
one.

  1. set_row_lens and plan_xs_advance check base + W >= tokens. A row
    shorter than the window is legal and front-padded; its leading W - tokens_i
    columns are pad standing for tokens before token 0.
  2. The history-gap guard now bounds reads by the row's own base, not by
    the shared window's physical start. Those differ whenever a neighbour
    retained more, and the columns between them are that neighbour's width —
    older tokens this row never promised, or zero front pad. Reading them is a
    wrong answer nothing downstream catches, so the tighter bound is the one that
    has to hold. It always does, by induction over the three writers of
    (tokens, base): advance clamps retention at tokens - margin, which is
    never past need_start; try_set_len enforces the same predicate directly;
    split_row preserves base exactly once the window is known to fit.
  3. advance dispatches to the scalar fast path on the assumption that path
    actually makes
    — one tokens, one base, and W == tokens - base —
    instead of on token equality alone. Equal lengths with unequal base (a
    sequence that rolled back, or one batched beside a row that retained more)
    took the fast path, which reads from base[0] into a window that physically
    starts earlier, and compressed the wrong tokens for every row. That was
    live and silent.

Flag-off byte-identity is unchanged and now structural: with ARC_V4_XS_PER_SEQ
off, ensure_uniform_batch_cache_lens refuses unequal token counts and
BatchSrc sets v_slack_dim: None, so every batch has one tokens, one
base, and W == tokens - base — exactly the fast path's predicate.

No mask is needed, and that is a property rather than a hope

Offsets are derived per row from that row's own token count:
off = need_start + W - tokens_i, with need_start >= base_i >= 0. So
off >= W - tokens_i is an identity, not a check — the front pad is
unreadable, not merely unread. off + len <= W + t_new follows the same
way.

Interim, and what the general answer is

This is the interim fix. It makes the dense [B, W, …] window express ragged
rows by padding; the pad is still allocated, still copied, and still sized by the
greediest row. Segment tables (ARC_SEGMENTED_KV, #90) are the general
answer
— ragged rows need no padding because nothing has to be rectangular,
and the existing gather kernel already treats a segment as a row, so B seqs ×
S segments flattens to B*S rows with zero .cu changes. That makes segment
tables a prerequisite for per-sequence advance, not an independent workstream.
This PR unblocks the chain; it does not remove the reason to land them.

One property fell out for free and is recorded with a test: nothing now requires
W == max_i (tokens_i - base_i), so W may be constant. tail is rebuilt
by cat + narrow every step at width tokens - base, and that width cycles
through ratio consecutive sizes (tokens climbs by one per token, base jumps
a whole ratio at a group boundary) — the ArcGraph chain measured exactly that
from the outside, 4096 × {18,19,20,21}. Pinning W to span_groups * ratio + margin would make the allocation constant without touching any semantics here.
Nothing in this PR depends on that being done, and the hypothesis that it is what
blocks graph replay has been retracted by that chain as unproven.

Acceptance gate — arc-tools/arcspec_token_identity_b8.sh, unrelaxed

H200 arc-prefill-curve, one process per arm, greedy (temperature 0, fixed
seed), 8 distinct prompts, max_tokens 48. Graded on completion, independently
of text
— which matters, because the engine is separately not batch-invariant
and text comparison at batch is currently unsound.

BEFORE — be29f397, two independent runs [MEASURED]:

  ON-B8   n=8  token len min=1  max=1  mean=1.0   streams with no finish_reason: 8/8
  ON-B1   n=8  token len min=48 max=48 mean=48.0  streams with no finish_reason: 0/8

AFTER — 73a0c72e, this branch [MEASURED]:

  OFF-B1  n=8  token len min=48 max=48 mean=48.0  streams with no finish_reason: 0/8
  OFF-B8  n=8  token len min=18 max=48 mean=44.2  streams with no finish_reason: 0/8
  ON-B1   n=8  token len min=48 max=48 mean=48.0  streams with no finish_reason: 0/8
  ON-B8   n=8  token len min=48 max=48 mean=48.0  streams with no finish_reason: 0/8

No VERDICT[truncation]. xs rolling cache errors: 0 in both arms (the ON
arm previously died on one). per_seq_steps on the MTP[…] marker reports
49–95 across the ON arm, so per-sequence advance ran for real rather than
being granted-but-inert. Engagement asserted per arm: per-sequence KV advance is ON and Ragged batch admission is ON present in ON, absent in OFF, cannot honour it zero.

Provenance and exclusivity:

  • Both arms' servers logged git revision: 73a0c72e32269eb6dc85e003550511f242cb1e9a,
    equal to the ref built. The built binary was asserted to contain the post-fix
    refusal string and not the pre-fix one; the preserved be29f397 binary is
    the negative control for that assert.
  • The runner claims the GPU lock first and only then reads the compute-app
    list, releasing and re-queuing on any foreign pid — it refused to launch five
    times over 315 s while a neighbour held the card. A watchdog sampled the list
    throughout; the run was exclusive end to end, and would have been discarded,
    not salvaged, if it had not been.

The head of this branch is 6e408eb2, one commit past the measured binary. That
commit is additions only, entirely inside doc comments and #[cfg(test)] —
git diff 73a0c72e..6e408eb2 is 63 insertions, 0 deletions, and no production
line is touched; cargo build --release does not compile #[cfg(test)] items at
all. It was rebuilt on the box and the binary asserted to carry the fix, but the
gate was not re-run at it: that would spend a queue slot confirming something
the diff already proves, on a fleet where another chain is waiting.

The remaining VERDICT[uncontrolled] is not this PR's. The OFF control
diverges on 5/8 prompts and OFF-B8 drops to min=18 — batching alone changes
output on this build, with the flags off, identically before and after. That is
the engine batch-invariance defect the batchinv chain owns. This fix neither
causes nor addresses it; ON-B8 is now more complete than OFF-B8, not less.

CPU evidence, deterministic, reproducing the production string verbatim

Two new end-to-end cases in deepseek4.rs run real XsRollingCaches through the
real compressor — no SimSeq, no sim_step — and require each batched row to be
token-identical to the same sequence run at B=1:

a_row_shorter_than_the_batched_window_is_token_identical_csa   (9/22/39/40, W=23)
a_row_shorter_than_the_batched_window_is_token_identical_hca   (129/143/1024/1150, W=143)

Against the pre-fix source, with the tests unchanged, both fail with the exact
error the H200 produced:

Error: xs rolling cache: row 0 holds 9 tokens, fewer than the 23-wide retained window it would have to end at
Error: xs rolling cache: row 0 holds 129 tokens, fewer than the 143-wide retained window it would have to end at

Every fixture that existed used rows long enough (37..43, 1022..1025, 30..35)
that tokens < W could not arise, which is why the whole file stayed green while
the H200 died. Plan-level tests pin the two refusals apart — a window too narrow
(widening fixes it) versus a genuine history gap (widening does not) — and assert
off >= W - tokens_i per row.

One existing fixture was arithmetically unreachable and had to move:
window_geometry_depends_only_on_the_token_residue passed w_phys = 8 for rows
promising a 9-token run. No batch can produce that state — W is the max of the
rows' own widths — and it only passed before because the old guard compared
against tokens rather than against base.

Two things found on the way, not fixed here

  • The empty ownerless gpu.lock has a producer. It appeared at 21:10 on
    arc-prefill-curve, to the second, as the profkill chain's server started, and
    again on its restart — a zero-byte file with no owner tag, which every
    owner-matched release refuses to remove and which therefore deadlocks the box
    for well-behaved chains. It is that chain's lock write, not a stray.
  • front_pad_kv_cache and pad_slack already implement the front-padding this
    fix relies on
    , but the xs path reaches pad_slack through BatchSrc
    rather than through front_pad_kv_cache, which returns early for XsRolling.
    The two spellings of "left-align a ragged cohort" are worth collapsing.

heydryft and others added 7 commits August 17, 2026 16:44
… an MTP one

The batch admission layer assumed one batch has one cache length, in two
places. `scheduler/default_scheduler.rs` partitioned the running set by
`Sequence::cache_bucket_len` and ran exactly ONE bucket per step, so
sequences at different lengths could not batch together at all; and
`engine/mod.rs` issued `CacheInstruction::In` only when the completion-id
list changed, so a stable cohort's front-alignment was never recomputed.

Both now read one `RaggedAdmission`, decided once in `Engine::new` from the
pipeline's own declaration (`CacheManagerMixin::ragged_batch_admission`,
default a refusal carrying a reason). With admission refused the bucket key
and the `pre_op` predicate are the pre-change expressions exactly.

The cost of the old rule is a law, measured by a CPU scheduler simulation:
with B sequences over D distinct cache lengths far enough apart that the
coalescence override is refused, B/D of them run per step. At B=128 over
128 lengths 64 tokens apart that is 1.00, in steady state.

`KvAdvance::PerSequence` is reachable for a V4 target for the first time.
For every other architecture the declaration still refuses, from
`model_masks_ragged_batches` — no other model in this tree threads a
ragged-batch mask into its forward, and that is where the refusal now is.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…TN_BACKEND)

Expert parallelism was priced on the belief that building with NCCL silently
disables flash attention for V4 — `attention/mod.rs`'s `use_nccl() =>
naive_sdpa` gate — which would make an EP=2-vs-EP=1 comparison a comparison of
two different attention kernels rather than of expert parallelism.

Reading the dispatch says otherwise for V4, in three steps: V4 loads
`attn_sink`, so `sdpa_params.sinks` is `Some`; `Sdpa::run_attention` therefore
diverts to `sinks_attn` on its first line, before `can_use_flash` and long
before the `use_nccl()` gate, which lives in `run_attention_noflash`; and flash
was never reachable at head_dim 512 regardless (`FA2_MAX_HEAD_DIM` is 256, FA3
takes {64,128,256}, `flash_sinks_ok` takes {64..256}).

That is a code read, and a $9.22/hr 2xH100 pair is too expensive to book on
one. These two `OnceLock`-gated lines make it an observation: one names the
backend `sinks_attn` chose, the other fires only if a model actually reaches
the `use_nccl()` gate. For V4 the second must never appear — presence and
absence are the assertion, rather than a comment asserting it.

No numerics change: two log lines, each emitted once, off the hot path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…st the grant

`kv_advance()` logs "per-sequence KV advance is ON" once, at init, when the
mode is GRANTED. That line has been this project's proof of a live treatment
arm all evening. It does not prove the mode did anything: `per_seq_advance`
(`mtp_pipeline.rs`) is recomputed every step from `assembled_here && post_op ==
Out && kv_advance() == PerSequence`, and can be false on every step while the
grant stands.

The gap is not hypothetical. A proposed optimisation — gate the batched-cache
re-assembly on `rows_uniform()` — would have made `assembled_here` false on
every steady-state step, so the mode would have been permanently inert. Cost
would have dropped to zero at every batch size, the throughput chart would have
read as a clean win, the init line would still have said ON, and every
engagement check in the harness would have passed. It was withdrawn only
because the semantics were read before it was written.

So the counter is the assertion: `per_seq_steps` on the `MTP[...]` marker says
how many decode steps actually applied per-sequence advance. Granted-but-inert
is now a visibly zero field rather than an absence, which is the same shape of
fix as naming the attention backend.

Appended at the end of the marker so parsers that look up named fields are
unaffected; 68 mtp tests pass unchanged, including both CPU token-identity
tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`arcspec_perseq_ab.sh` — per-sequence KV advance ON vs OFF, `tok_per_step`
differenced out of the engine's cumulative `MTP[agg]` counters across a
per-cell wall-clock fence. Two arms in one binary, plus a drift-control arm
that re-runs the control after the treatment so the comparison is bounded for
time drift.

`arcspec_token_identity_b8.sh` — greedy token identity at B=8 against a
strictly sequential B=1 reference, in both arms. The OFF arm is the control and
is not optional: if batching alone changes output, no ON divergence is
attributable.

Both refuse rather than report when they cannot measure honestly, because every
guard here was earned by something that went wrong on hardware tonight:

* provenance — the running server's baked `git revision:` must equal the ref
  the script built. A sibling harness built `-p arc-cli` (binary `arc`) and
  launched `target/release/mistralrs`, a stale binary from an unrelated build,
  guarded only by `[ -x "$BIN" ]`. For an A/B that is the worst failure
  available: both arms run the same stale code, the ratio is ~1.0, and it reads
  as an honest refutation.
* counter monotonicity — a negative delta means the parse crossed a restart, so
  it reports NA rather than the number it would otherwise produce.
* crash watchdog — a cell that crosses a CUDA fault is `VOID`, never a number.
  Note the shape of `crash_count`: `grep -c` prints `0` AND exits 1, so the
  obvious `|| echo 0` emits "0\n0", the integer test errors, and the guard is
  silently dead while looking installed. Found by testing the guard, not
  reading it.
* owner-matched lock release — a lock is only removed if it carries this
  process's own pid. Tonight I deleted another chain's live GPU lock because I
  saw the file, assumed it was my leak, and did not `cat` it first. The owner
  tag was already there; nobody was checking it.
* completion graded independently of text — a diverging control invalidates
  comparing *what* was generated, not *whether* anything was. The first version
  returned "uncontrolled" for everything and so misclassified a one-token
  failure to generate as an unattributable text difference.

Driver counts streamed SSE deltas, not completed requests: at B>=32 V4 finishes
no request inside a 60s window, and a completion-counting driver reports 0.00
tok/s with zero errors, which reads as a crash and is not one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…apart from text

Two defects found by running these harnesses on a shared box, both of the same
shape: a guard that is safe in isolation and wrong in company.

**The lock could deadlock the box permanently.** Owner-matched release — only
remove a lock carrying your own pid — stops one chain deleting another's lock,
which I did tonight by seeing a file, assuming it was my leak, and not reading
it first. But it cannot self-heal: a lock whose owner died, or one created
empty with no owner tag at all, is unremovable by every well-behaved
participant. That is not hypothetical either — a size-0 lock appeared, its
presumed owner was dead, the H200 sat at 0% for six minutes, and the run
waiting on it would have aborted after 40 minutes having measured nothing.

`lock_is_stale` reclaims, but only on evidence and never on one signal alone:
older than a grace period (so a lock caught mid-write is never stolen), AND
empty or its named pid is dead, AND the GPU is idle by both memory and compute
processes. Tested across all five cases, including the two that would do harm:
a lock owned by a live pid is never taken, and a dead owner with a busy GPU is
never taken.

**The comparator misclassified the failure it exists to catch.** It graded only
text identity, so a diverging control returned "uncontrolled" for everything —
including an arm that returned one token with no `finish_reason` while its own
B=1 reference returned full-length. A diverging control invalidates comparing
WHAT was generated; it does not invalidate comparing WHETHER anything was.
Completion is now graded on its own axis and reported first.

Also worth recording, because `bash -n` did not catch it: an earlier attempt to
lift the lock block between scripts by regex deleted `acquire_lock`'s
definition while leaving its call site. The script parsed clean — `bash -n`
checks syntax, not name resolution — and would have failed at runtime on the
box. Verified here by running the script far enough to acquire and release.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`set_row_lens` refused any row whose token count was below the batched
window's width. That is not the invariant the end-anchored layout needs —
this file's own doc states the right one, `base[i] >= tokens[i] - W` — and
it is violated by every batch of unequal prompts, because the window is
sized for the greediest row and every shorter row is front-padded into it.

Measured consequence, twice on an H200: `ARC_V4_XS_PER_SEQ=1
ARC_MTP_PER_SEQ_KV=1` at B=8 returned one token and `finish_reason: None`
for every request, dying at the first decode step with

    row 3 holds 9 tokens, fewer than the 11-wide retained window

Three changes, all in the representation, none relaxing a check:

* `set_row_lens` and `plan_xs_advance` check `base + W >= tokens` — the
  row's promised run fits the window — instead of `tokens >= W`, which is
  sufficient for the arithmetic rather than necessary for correctness.
* The history-gap guard now bounds reads by the row's OWN `base` rather
  than by the shared window's physical start. The columns between the two
  are a neighbour's width — older tokens this row never promised, or zero
  front pad — and reading them is a silent wrong answer.
* `advance` dispatches to the scalar fast path on the assumption that path
  actually makes (one `tokens`, one `base`, `W == tokens - base`) instead
  of on token equality alone. Equal lengths with unequal `base` took the
  fast path and compressed the wrong tokens for every row.

Offsets are now derived per row from that row's own token count, which
makes `off >= W - tokens_i` an identity: the front pad is unreadable
rather than merely unread, so no mask is needed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The fix above stopped requiring `W == max_i (tokens_i - base_i)` as a side
effect: `plan_xs_advance` and `set_row_lens` take `W` as given and ask only
that every row fit inside it, and every read offset is
`need_start + W - tokens_i`, which tracks `W` exactly. So `W` may be any
width at or above the widest row's run — including a constant one.

Worth recording because this buffer does not have a constant size today.
`tail` is rebuilt by `cat` + `narrow` every step at width `tokens - base`,
and `tokens` climbs by one per token while `base` jumps a whole `ratio` at a
group boundary, so the allocation cycles through `ratio` consecutive sizes.
The ArcGraph chain measured that from the outside — `4096 x {18,19,20,21}`
at `hidden = 4096`, `ratio = 4`. Their hypothesis that it is what stops a
graph replaying is retracted as unproven; the size measurement stands as a
fact about this buffer regardless, and nothing here depends on changing it.

`an_oversized_window_reads_exactly_the_same_absolute_tokens` turns the claim
into a check: widening the window shifts every offset by exactly the
widening and reads the same absolute tokens, with identical groups, dests,
takes, bands and per-row bookkeeping.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
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                189        37703            0        28931         8772
 |- BASH                  70         1618         1189          313          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)                            43427         4663        29455         9309
─────────────────────────────────────────────────────────────────────────────────
 Rust                    662       307804       266560        13893        27351
 |- Markdown             477        22853          471        19669         2713
 (Total)                           330657       267031        33562        30064
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 Total                  1277       451971       330166        73659        48146
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

@heydryft

Copy link
Copy Markdown
Contributor Author

🟢 This PR is parked by stacking, not by content — and it does not have to be

#104 is held on a real question (the clone_in_cache cadence — see my writeup there). #114, this PR and #121 sit above it and inherited the park. For this PR that inheritance is an accident of branch topology.

Its two commits touch only kv_cache/xs_rolling.rs and models/deepseek4.rs. #104's work is scheduler/, engine/mod.rs, pipeline/mtp_pipeline.rs, ragged_admission.rs. Disjoint.

Verified, not assumed — cherry-picked 73a0c72e3 + 6e408eb2e onto origin/master (006657e05) in a scratch worktree:

both applied clean (auto-merge in xs_rolling.rs and deepseek4.rs, no conflicts)
cargo check -p mistralrs-core --tests   -> exit 0    (unpiped; the exit code is the compiler's)
cargo test  -p mistralrs-core --lib xs_rolling -> 13 passed, 0 failed

and the 13 include the one that matters:

a_row_shorter_than_the_batched_window_is_accepted ... ok
a_row_whose_promise_outruns_the_window_is_refused ... ok
a_row_shorter_than_the_window_plans_and_never_reads_the_pad ... ok

i.e. the corrected invariant (base[i] >= tokens[i] - W, the row's promised run fits the window) holds on master, and the old tokens >= W refusal is gone, without any of #104 present.

Why this is worth doing rather than waiting. This is a real defect — ARC_V4_XS_PER_SEQ=1 ARC_MTP_PER_SEQ_KV=1 at B=8 returned 1 token, finish_reason: None, on 8/8 requests, measured twice on an H200 — and the work above it (#121) carries the measured +23.2% window pin and the 4.46× spread-B=8 per-sequence advance. All of that is currently gated behind a question about a different PR's cache-in cadence.

I have NOT rebased or force-pushed anything. Re-parenting this branch would orphan #121's base, and rewriting someone else's stack is not a call I will make unasked. The finding is here so its owner can act on it:

Either way it needs a real CI complete first: this PR currently shows 1 check — the comment bot — because it predates the stacked-PR trigger fix. Any push to it now starts the full 16 lanes.

@heydryft

Copy link
Copy Markdown
Contributor Author

Followup: lifted as #128, per Option B. Your two commits (73a0c72e3 + 6e408eb2e), cherry-picked unchanged onto master 006657e05, opened as a small PR that can gate on its own.

Nothing here was rebased, force-pushed, or deleted. This branch, #114, #121 and #104 are untouched on the remote. Option A (rebase this branch onto master and re-parent #121) was considered and rejected: it rewrites a stack whose owner did not ask for it, and it re-parents #121 as a side effect. This PR closes on its own terms, by you, once #128's content is on master — not by me.

One correction to what I wrote above, and it matters for #121. I said the measured +23.2% window pin was recoverable the same way. It is not, and I checked rather than assuming twice. Two independent blockers:

  1. perf(v4): pin the xs window (+23.2% at uniform B=32) + the ladder that prices per-sequence advance — 4.46x at spread B=8 [MEASURED] #121's first commit 5ccd1a49a calls ragged_bucket_len(&running, **&self.admission**) in default_scheduler.rs. self.admission is feat(scheduler): admit ragged cohorts — the admission layer #103 named, and a general throughput bug #104's ragged-admission state — so that commit cannot exist without feat(scheduler): admit ragged cohorts — the admission layer #103 named, and a general throughput bug #104, and every later perf(v4): pin the xs window (+23.2% at uniform B=32) + the ladder that prices per-sequence advance — 4.46x at spread B=8 [MEASURED] #121 commit sits on top of it. default_scheduler.rs is the file that makes perf(v4): pin the xs window (+23.2% at uniform B=32) + the ladder that prices per-sequence advance — 4.46x at spread B=8 [MEASURED] #121 inseparable.
  2. Even the pin alone (57d330c8b, which touches only kv_cache/mod.rs, xs_rolling.rs, deepseek4.rs — no contended file) does not cherry-pick onto master + this PR: it removes compressor_needs_from(), which your own second commit adds. Zero occurrences survive on perf(v4): pin the xs window (+23.2% at uniform B=32) + the ladder that prices per-sequence advance — 4.46x at spread B=8 [MEASURED] #121's tip. So fix(v4): the raw xs window may hold rows shorter than itself — B=8 per-seq advance generates again [MEASURED] #116 → perf(v4): pin the xs window (+23.2% at uniform B=32) + the ladder that prices per-sequence advance — 4.46x at spread B=8 [MEASURED] #121 is a genuine rebase, not a sequence of independent lifts.

⇒ #116 lifts; #121 does not. The pin stays parked until #104's cadence question resolves, or until you rebase the sequence deliberately.

@heydryft
heydryft force-pushed the agent/arcspec-perseq-measure branch from 612b3a1 to c2e2b16 Compare August 19, 2026 11:09
heydryft added a commit that referenced this pull request Aug 19, 2026
…onfound it has to be read against

is worth anything: MTP was measured at 1.93 tok/step at one user collapsing to
1.06 at 128, and that collapse is what this stack targets.

Two pieces, because the number is unreadable without the second.

1. `scheduler::bucket_telemetry` — a `SCHED[agg]` marker carrying
   buckets_per_step, running_bucket_size and offered_per_step, emitted on the
   SAME log fence as `MTP[agg]`.

   Both schedulers bucket the running set by exact cache length and run one
   bucket per step, preempting the rest. So a cell labelled B=128 can be a
   3-wide step in the engine, and an aggregate number measured inside that is a
   measurement of the scheduler rather than of KV advance. Ragged admission
   exists precisely to collapse those buckets, so the two effects are
   confounded by construction — without these counters "aggregate did not move"
   is unattributable. running_bucket_size is the width that actually ran;
   offered_per_step is the width the harness thinks it asked for.

2. `arc-tools/arcspec_perseq_ladder.sh` — B = 1, 8, 32, 128, both arms, one
   server per arm, counters differenced across each cell's own wall-clock
   fence.

   It reports aggregate tok/s AND tok_per_step AND the decomposition
   (tok_per_batch_step x batch_steps/s, plus ms/step), because tok/step rising
   while aggregate falls has already happened on this chain and either number
   alone is unreadable.

   Prompt lengths are RAGGED by construction — 24..320 words cycled across
   workers — which is the deliberate difference from `arcspec_perseq_ab.sh`'s
   fixed 40. Uniform prompts hide the failure mode this stack addresses: the
   `xs` window defect could not even be reached with equal-length prompts. 320
   keeps a >3x margin to the ~1,055-word serving cliff this branch does not
   carry the fix for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@heydryft

Copy link
Copy Markdown
Contributor Author

Already landed on master — this PR should be closed.

Both of this branch's own commits are byte-identical (by git patch-id) to commits already merged via #128 (fix/xs-window-ragged-rows):

this branch on master (via #128) subject
73a0c72e3 547ad9051 fix(v4): a ragged xs window may hold rows shorter than itself
6e408eb2e 8f9f58ce5 doc(v4): record that the xs window's width is now free, and pin it

git cherry -v origin/master origin/fix/xs-ragged-rows 612b3a17b marks both with - (already upstream). The only diffs between the two patch pairs are hunk offsets and blob hashes.

Nothing was rebased here on purpose: rebasing onto master would replay two commits whose content is already there, plus this branch's inherited base commits from #114/#104.

Note also that its base branch agent/arcspec-perseq-measure (#114) has since been rebased onto master, so this PR's commit list no longer reflects a real stack.

@heydryft

Copy link
Copy Markdown
Contributor Author

Closing: this PR's own two commits are already on master, patch-identical.

One of the five PRs based on another PR's branch (agent/arcspec-perseq-measure, #114), so it ran zero CI lanes and showed only comment. #164 makes that state red rather than blank.

git cherry -v origin/master origin/fix/xs-ragged-rows marks exactly this PR's two commits with -, meaning master already contains an equivalent patch:

- 73a0c72e3  fix(v4): a ragged xs window may hold rows shorter than itself
- 6e408eb2e  doc(v4): record that the xs window's width is now free, and pin it

They landed via #128 (fix/xs-window-ragged-rows — a similarly-named but different branch) as 547ad9051 and 8f9f58ce5.

The commits still marked + on this branch (ab42c4508, be29f3972, e785a5e7d, 4ed7deff0, 612b3a17b) are not this PR's — they come from its stack base and belong to #114 / #104, both of which are separately tracked. Closing here loses nothing.

One further reason not to rebase it: its base branch agent/arcspec-perseq-measure has since been force-pushed during the #114 rebase, so this PR's commit list no longer describes a real stack.

Reopen if any of the above is wrong.

@heydryft heydryft closed this Aug 19, 2026
heydryft added a commit that referenced this pull request Aug 21, 2026
…onfound it has to be read against

is worth anything: MTP was measured at 1.93 tok/step at one user collapsing to
1.06 at 128, and that collapse is what this stack targets.

Two pieces, because the number is unreadable without the second.

1. `scheduler::bucket_telemetry` — a `SCHED[agg]` marker carrying
   buckets_per_step, running_bucket_size and offered_per_step, emitted on the
   SAME log fence as `MTP[agg]`.

   Both schedulers bucket the running set by exact cache length and run one
   bucket per step, preempting the rest. So a cell labelled B=128 can be a
   3-wide step in the engine, and an aggregate number measured inside that is a
   measurement of the scheduler rather than of KV advance. Ragged admission
   exists precisely to collapse those buckets, so the two effects are
   confounded by construction — without these counters "aggregate did not move"
   is unattributable. running_bucket_size is the width that actually ran;
   offered_per_step is the width the harness thinks it asked for.

2. `arc-tools/arcspec_perseq_ladder.sh` — B = 1, 8, 32, 128, both arms, one
   server per arm, counters differenced across each cell's own wall-clock
   fence.

   It reports aggregate tok/s AND tok_per_step AND the decomposition
   (tok_per_batch_step x batch_steps/s, plus ms/step), because tok/step rising
   while aggregate falls has already happened on this chain and either number
   alone is unreadable.

   Prompt lengths are RAGGED by construction — 24..320 words cycled across
   workers — which is the deliberate difference from `arcspec_perseq_ab.sh`'s
   fixed 40. Uniform prompts hide the failure mode this stack addresses: the
   `xs` window defect could not even be reached with equal-length prompts. 320
   keeps a >3x margin to the ~1,055-word serving cliff this branch does not
   carry the fix for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
heydryft added a commit that referenced this pull request Aug 21, 2026
…t prices per-sequence advance — 4.46x at spread B=8 [MEASURED] (#121)

* measure(sched+arcspec): the throughput ladder #116 unlocks, and the confound it has to be read against

is worth anything: MTP was measured at 1.93 tok/step at one user collapsing to
1.06 at 128, and that collapse is what this stack targets.

Two pieces, because the number is unreadable without the second.

1. `scheduler::bucket_telemetry` — a `SCHED[agg]` marker carrying
   buckets_per_step, running_bucket_size and offered_per_step, emitted on the
   SAME log fence as `MTP[agg]`.

   Both schedulers bucket the running set by exact cache length and run one
   bucket per step, preempting the rest. So a cell labelled B=128 can be a
   3-wide step in the engine, and an aggregate number measured inside that is a
   measurement of the scheduler rather than of KV advance. Ragged admission
   exists precisely to collapse those buckets, so the two effects are
   confounded by construction — without these counters "aggregate did not move"
   is unattributable. running_bucket_size is the width that actually ran;
   offered_per_step is the width the harness thinks it asked for.

2. `arc-tools/arcspec_perseq_ladder.sh` — B = 1, 8, 32, 128, both arms, one
   server per arm, counters differenced across each cell's own wall-clock
   fence.

   It reports aggregate tok/s AND tok_per_step AND the decomposition
   (tok_per_batch_step x batch_steps/s, plus ms/step), because tok/step rising
   while aggregate falls has already happened on this chain and either number
   alone is unreadable.

   Prompt lengths are RAGGED by construction — 24..320 words cycled across
   workers — which is the deliberate difference from `arcspec_perseq_ab.sh`'s
   fixed 40. Uniform prompts hide the failure mode this stack addresses: the
   `xs` window defect could not even be reached with equal-length prompts. 320
   keeps a >3x margin to the ~1,055-word serving cliff this branch does not
   carry the fix for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* measure(arcspec): add the uniform arm — the only regime where the batch is whole

The scheduler A/B came back while this was being built and it changes what the
ladder can conclude. The bucketing law is now measured, not hypothesised:

    running bucket size = B / (distinct cache lengths)

holding at 8/8=1 and 32/8=4, with `1 running, 7 waiting` sustained, and B=8 on
spread lengths measuring 7.91 tok/s against B=1's 15.36 — batching is NEGATIVE
on realistic traffic.

So a spread-only ladder measures per-sequence KV advance inside a scheduler
that is running one sequence at a time, and a flat aggregate there is
unreadable: it cannot distinguish "the fix does not pay" from "the batch never
existed". Both regimes are now run, per arm, mean-matched:

  spread  144 24 320 64 260 40 200 96 words (8 distinct lengths)
  uniform 144 words — the spread's mean AND its first element, so B=1 is
          byte-identical between regimes and the only thing that differs at
          B>1 is the spread itself

Uniform is where a whole batch actually forms, so it is the only place this
stack's effect on aggregate throughput is visible without the serialisation
swamping it. Spread is still the regime the stack exists for.

The report now prints the bucketing law's prediction beside the measured
`running_bucket_size` per cell, and refuses to let a spread cell be read as a
batch result when its running bucket is ~1.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* perf(v4): stop reallocating the xs window every decode step — restore the bound the type already documents

`tail` is described at the top of this file as "the raw rows behind `tokens`,
bounded by `span_groups * ratio + margin` and independent of context length".
That is an invariant the type has always claimed. The allocation did not honour
it: `advance` rebuilt the buffer with `cat` + `narrow` every step at width
exactly `tokens - base`, which is not stable — `tokens` climbs by one per token
while `base` jumps a whole `ratio` at a group boundary — so the buffer cycled
through `ratio` consecutive sizes and was reallocated on every decode step, on
every one of V4's 41 compressed layers.

This is not "add a pin". It is restoring a documented invariant that one code
path broke.

THE BOUND, WHICH IS THE REVIEWABLE PART

  keep_from = ((tokens - margin) / ratio + 1 - span_groups) * ratio
  base      = max(keep_from, previous base), capped at tokens

Write m = tokens - margin and q = floor(m / ratio). Then q * ratio > m - ratio,
so keep_from = (q + 1 - span_groups) * ratio > m - span_groups * ratio, giving

  W = tokens - base <= tokens - keep_from < margin + span_groups * ratio

so W <= span_groups * ratio + margin - 1, ALWAYS. Pinning to
span_groups * ratio + margin is provably sufficient and provably never
truncates. It is a function of the layer's geometry, not of context length: 24
for CSA (ratio 4, span 2, margin 16), 144 for HCA. That is the difference
between this and a capacity that turns out to be a million-token context.

Derived twice independently and agreeing, and checked a third way: the steady
band is measured at [capacity - ratio, capacity - 1] = {20..23}.

THE {18..21} vs {20..23} DISCREPANCY, SETTLED RATHER THAN ASSERTED

ArcGraph measured 4096 x {18,19,20,21} from outside the engine; the retention
rule predicts {20,21,22,23}. Both ratio-consecutive, both containing 21, offset
by 2. The pre-committed reconciliation was that theirs is the pre-saturation
ramp (base still at 0 while tokens climbs) and mine the steady state.
`the_window_ramps_then_settles_to_ratio_consecutive_sizes` checks exactly that:
the early band sits below the steady one, the steady one is `ratio` consecutive
sizes at [cap-ratio, cap-1], and no width over 128 steps reaches `cap`. A bound
that is right for the wrong reason is a trap for the next context length.

WHY DEFAULT ON WITHOUT A THROUGHPUT NUMBER FIRST

The measure-then-default-on rule exists because FP8 KV shipped on and changed
VALUES. This changes only the size of an allocation: the compressor is handed a
slice covering the same absolute tokens either way, because every offset is
derived from the row's own token count rather than from the buffer's width.
`pinning_the_window_is_numerically_inert` runs both settings against one stream
for 40 steps and requires exact equality of the compressed rows and both time
bases — and asserts the unpinned widths actually varied, so the equality is not
vacuous. `ARC_V4_XS_PIN_WINDOW=0` restores the resizing buffer, which is also
the A/B arm.

split_row keeps the pinned buffer rather than re-narrowing it: `clone_out_cache`
calls it once per layer per sequence on EVERY engine step, so narrowing there
would undo the pin exactly on the hot path. The resume point does not move.

Two fixtures now run explicitly unpinned, because pinning removes their
discriminator rather than their subject: `ragged_xs_tail_is_refused_by_name_not_panicked`
needs the widths 18/22 that only a resizing buffer produces, and
`splitting_a_batched_row_restores_the_per_sequence_window` needs a row narrower
than the shared window. Both keep testing what they were written for, and both
gained a pinned-side counterpart.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* measure(arcspec): price the window pin in the same session — a third arm

ON vs ON_UNPINNED differ in exactly one thing: whether the compressor's raw
window is reallocated every decode step on all 41 compressed layers, or held at
the bound the type documents. Same per-sequence flags in both, so the pin is
isolated rather than confounded with the KV mechanism, and it is priced in the
session that was already going to queue for the box.

On the width discrepancy the pin rests on: this harness does not need to settle
it. `max_tokens` is 4096 and each cell drives a 45 s steady window, so every
request is thousands of tokens past saturation and the ramp is invisible here
anyway. The settling is measured directly instead, on CPU, over 128 steps, by
`the_window_ramps_then_settles_to_ratio_consecutive_sizes` — the early band sits
below the steady one, the steady one is `ratio` consecutive sizes at
[cap-ratio, cap-1], and no width ever reaches `cap`. That is a stronger
instrument than a throughput leg and it costs no card time.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* instr(v4): name the window mode once per process, and assert the pin A/B is real

D18, applied to the arm I am about to run. `ARC_V4_XS_PIN_WINDOW=0` is the only
thing separating ON from ON_UNPINNED, and if that name were wrong the two arms
would be the same build, produce a 1.000x ratio, and read as "the pin costs
nothing" — the granted-but-inert failure with a throughput number attached.

So the flag names itself once per process on first read ("xs rolling window is
PINNED / RESIZING"), following the same convention as "per-sequence KV advance
is ON", and the ladder summary now requires PINNED in the ON log and RESIZING
in the ON_UNPINNED log before either arm's ratio may be read. If it cannot find
both it prints PIN A/B IS VOID and says the comparison is of one thing with
itself, rather than printing a ratio.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* measure(arcspec): make the pin control provably the control, not the default

The window pin defaults ON, so the TREATMENT is the default and the CONTROL is
the arm carrying `ARC_V4_XS_PIN_WINDOW=0`. That inverts the usual failure: a
typo in the flag name does not disable the feature, it produces a control that
silently ran the treatment and a clean-looking 1.000x ratio.

Three things, all of which had to be right and only one of which was obvious:

* The assertion reads the runtime LOG, never the binary. Both mode strings are
  compiled in unconditionally, so `strings <binary> | grep RESIZING` succeeds in
  every arm and proves nothing. Only the line a process emits says which branch
  it took.
* `RUST_LOG=info` is set once inside `run_arm`, shared by every arm, so a log
  filter cannot suppress the line in one arm while leaving it in another. An
  assertion that configuration can mute is a guard with an undocumented off
  switch.
* The guard now also fails when the control's log contains PINNED, not just
  when it lacks RESIZING — the leak is the thing being tested for, so it is
  checked directly rather than inferred from an absence.

On VOID it refuses to let the ratio be read at all rather than printing it with
a caveat.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(ArcKV): one xs window pin, two triggers; scope the unpinned tests

Integration fix for #121 landing after #181's `pin_tail_width`. Both arrived at
the same idea — hold the retained raw xs window at a constant width instead of
letting it breathe with `tokens % ratio` — for two different reasons, and both
callers are real:

  * #181: CUDA-graph capture, where a moving allocation size is not slow but
    INVALID (a capture-time miss becomes an unstable graph memory node).
    Per-cache trigger, `pin_tail_width()`.
  * #121: serving, where the per-step reallocation is simply waste.
    Process-wide trigger, `ARC_V4_XS_PIN_WINDOW`.

They agreed on the width and disagreed on everything else, so this keeps one
width policy and both triggers:

1. `graph_tail_width()` and `window_capacity()` were the same expression
   (`span_groups * ratio + margin`) written twice. `graph_tail_width` now
   delegates. Two pins that computed "the pinned width" independently could
   drift, and capture and serving would then disagree about a number whose only
   value is that it does not move.

2. `retained_width` is gated on `pin_is_on()` = `self.pin_tail ||
   xs_pin_window_enabled()`, so capture still gets its constant width when the
   env switch is off — which, for a capture-only run, it is.

3. #181's `pinned_base` form is dropped in favour of #121's. Both produce the
   same constant width; they differed in where the constancy came from. #181
   moved `base` earlier so the logical span went constant; #121 leaves `base`
   at the retention point the type documents and widens the physical buffer.
   #121's is the one that composes, because it also moved `win_start` to the
   buffer's PHYSICAL start (`tokens - w_phys`) rather than `base`, which makes
   "buffer wider than the row promised" a representable state. `plan_xs_advance`
   still refuses to read below `base`, so the pin cannot change an answer.

4. Three `clone_in_cache_invariant_tests` assert the geometry of the UNPINNED
   window — exact tail widths `(4, 132)`, and that column 0 holds token `base`.
   Neither survives a pinned buffer, and #121 makes the pin the DEFAULT. They
   now run under `unpinned(..)`, the same `pin_test_override` shape #121 uses
   for its own affected tests. They are not testing a dead path:
   `ARC_V4_XS_PIN_WINDOW=0` is a supported mode and is the control arm of
   #121's own A/B.

NOTE FOR REVIEW, not a defect: #121 flips the serving default — the window is
PINNED unless `ARC_V4_XS_PIN_WINDOW` is set to 0. Its "+23.2% at uniform B=32"
was measured on #121's own branch and is NOT re-measured here; this rebase
changes which code that number describes.

`cargo test -p mistralrs-core --lib`: 701 passed, 0 failed. Warning count back
to master's 23 (the `xs_rolling` import is test-only and now lives in the test
module).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(ArcKV): the xs window pin is OPT-IN, with the experiment that flips it named

#121 shipped `ARC_V4_XS_PIN_WINDOW` default-ON, with `=0` as the control arm.
Retracting the default and keeping the change.

Why: the pin changes what the serving path RETAINS — at V4's HCA geometry the
window is held at 144 columns where the resizing policy keeps as few as 4 at
some residues — and the +23.2% at uniform B=32 that motivated defaulting it on
was measured on #121's own pre-rebase branch, against a tree that no longer
exists. That is an unmeasured default. TCFRAG was an unmeasured default too: it
carried "UNVERIFIED ON HARDWARE — NEVER RUN" in its own header, held 63 GB and
permanently broke a layer through a poisoned `OnceLock` (#209). So: unverified
means default-off, and "unverified" means unmeasured, not new.

The correctness argument #121 made for the pin is SOUND and is kept in place —
it proves the pin changes no answer, which is necessary and not sufficient. The
40-step bit-identity A/B in `deepseek4` still guards it; its doc comment no
longer claims to justify a default.

## Off is a temporary state with an owner, not the finish line

Arc's larger problem is not unbuilt work — it is finished, correct, tested work
left switched off. So this does NOT ship as a parked flag. The flip condition is
written three places, one of them enforced:

* `xs_pin_window_enabled_from`'s doc comment carries it in full, under a
  "FLIP CONDITION" heading, at the gate itself.
* `capability_reachability.rs` — the file titled "the switched-off guard", which
  is where someone goes looking for exactly this — carries it as a registry
  entry, so the gate cannot go dark without CI going red.
* `arcspec_perseq_ladder.sh`'s `ON_PINNED` arm IS the experiment.

The experiment: one binary, uniform B=32, same prompt and seed, `ON` (flag
unset) against `ON_PINNED` (`ARC_V4_XS_PIN_WINDOW=1`). Pass = ON_PINNED faster
on aggregate tok/s with identical generated tokens. On pass the default flips to
ON in the same change that records the number.

## The harness inverted with the default, and that is easy to get silently wrong

The pin used to be the default, so the A/B's TREATMENT was the default arm and
its CONTROL carried the flag. Now it is the other way round. Three things had to
move together, and any one left behind would have produced a clean-looking and
meaningless number:

1. The arm carries `=1` and is renamed `ON_UNPINNED` -> `ON_PINNED`.
2. The report's ratio pairing is swapped, so it stays treatment/control.
   Renaming the arm without this would have inverted every printed ratio.
3. The engagement guard's expectations are swapped: `ON` must log RESIZING,
   `ON_PINNED` must log PINNED, and the leak check now looks for a treatment
   that silently ran the control.

Polarity is `== Some("1")`, split into the pure `xs_pin_window_enabled_from` and
pinned by tests: unset is OFF, and `0`/`false`/`off`/`true`/`on`/`2`/`" 1"` are
all OFF. #212 converted 23 `ARC_*` flags for this reason — `var_os(..).is_some()`
made `ARC_FOO=0` mean ON, which turns an A/B control into a second treatment.

A third test pins the thing most likely to be broken by this retraction:
CUDA-graph capture must still get a constant width with the env flag off. It
does — capture asks per-cache via `pin_tail_width()` and `pin_is_on` honours
that trigger independently. Without that, capture would silently return to a
per-step-varying allocation size, which is not a slow graph but an invalid one.

`cargo test -p mistralrs-core -p mistralrs-quant -p mistralrs-vision`: green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.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