Skip to content

feat(batching): batch-decode RoPE-correct cache infrastructure (A1)#65

Merged
magicnight merged 1 commit into
mainfrom
feat/continuous-batching-a1
Jul 8, 2026
Merged

feat(batching): batch-decode RoPE-correct cache infrastructure (A1)#65
magicnight merged 1 commit into
mainfrom
feat/continuous-batching-a1

Conversation

@magicnight

Copy link
Copy Markdown
Owner

Foundation (step 1) for continuous batching (v0.6 headline). Standalone infrastructure — not wired into any generation path yet; A2 (the BatchScheduler orchestrator) consumes it next.

The problem it solves

mlx-swift 0.31.6's fused RoPE kernel corrupts batched single-token decode ([B,H,1,D], scalar offset, B>1): lanes 1..B-1 come out as uninitialized memory/NaN. Confirmed real (upstream ml-explore/mlx#3494/#3496, fixed in mlx-core 0.32.0 which mlx-swift hasn't vendored — I filed ml-explore/mlx-swift#441). Models read their RoPE offset from cache.ropeOffset, so a cache returning .batch([B]) (per-row array offset) routes onto the correct kernel. Proven end-to-end on gemma-4: cross-row 12–19 → exactly 0.0, bit-identical tokens across a B=4 identical batch, throughput held at 3.6×.

What lands

  • BatchPositionedCacheWrapper — composition over a stock KVCache; forwards every requirement verbatim, overrides ropeOffset to .batch. Deletable once mlx-swift ships the fix.
  • batchPositioned(_:) -> [KVCache]? — a safety gate, not a blind wrap: returns nil if any cache isn't a plain dense KVCacheSimple/RotatingKVCache (CacheList/QuantizedKVCache would fatalError if wrapped), so A2's scheduler refuses to batch an uncoverable model rather than emit garbage.
  • Honest coverage: MLXLLM dense-text models that read cache?.ropeOffset (Gemma/Gemma2/Gemma3Text/Gemma4Text/Qwen2/Qwen3/Llama); the six MLXVLM towers applying RoPE from the scalar cache.offset (Paligemma, Gemma3/Gemma4 VLM, Pixtral, Mistral3, LFM2VL) are not covered and documented as such.
  • Deletion tripwire: the regression test asserts scalar-offset RoPE → NaN while .batch → 0.0; it flips to FAILURE (with a message pointing at #441) once mlx-swift ships the fix, telling us to delete the wrapper.

Review + verification

Adversarially reviewed (approve-with-comments) — the review caught two MEDIUM safety issues (a false 'model-agnostic' coverage claim, and a blind-wrap factory that would fatalError on hybrid/quantized caches); both fixed (honest scope + the [KVCache]? safety gate + 3 refusal tests). No force-unwrap/try!/as!. Independently verified: xcodebuild ** TEST SUCCEEDED **, bare swift test 244 green (0 MLX errors, MLX-gated asserts skip), CLI builds. 7 tests.

Foundation for continuous batching. Fixes the mlx-swift 0.31.6
scalar-offset RoPE batched-decode bug (uninitialized lanes 1..B-1;
upstream ml-explore/mlx-swift#441, fixed in mlx-core 0.32.0 but not
yet vendored) at the cache layer — no model edits.

- BatchPositionedCacheWrapper: composition over a stock KVCache that
  forwards every requirement verbatim but overrides ropeOffset to
  .batch([offset x B]) (per-row array offset), routing decode RoPE onto
  the correct kernel path. Proven end-to-end on gemma-4: cross-row
  12-19 -> 0.0, bit-identical tokens, throughput held at 3.6x.
- batchPositioned([KVCache]) -> [KVCache]?: a SAFETY GATE — returns nil
  if any cache isn't a plain dense KVCacheSimple/RotatingKVCache
  (CacheList/QuantizedKVCache would fatalError when wrapped), so A2
  refuses to batch an uncoverable model rather than emit garbage.
- Honest coverage: MLXLLM dense-text models reading cache?.ropeOffset
  (Gemma/Gemma2/Gemma3Text/Gemma4Text/Qwen2/Qwen3/Llama); the six MLXVLM
  scalar-offset towers are NOT covered and documented.
- Regression test asserts scalar RoPE -> NaN while .batch -> 0.0; a
  deletion tripwire that flips to FAILURE (pointing at #441) once
  mlx-swift ships the fix. Not wired into any generation path yet.

Reviewed (approve-with-comments; both MEDIUM findings fixed). 7 tests
incl. 3 refusal tests; xcodebuild + bare swift test green, CLI builds.
@cursor

cursor Bot commented Jul 8, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@magicnight magicnight merged commit c5ef56a into main Jul 8, 2026
3 checks passed
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