Skip to content

[BugFix] Preserve sparse hybrid replay boundaries under EAGLE/MTP - #643

Closed
logprobz wants to merge 6 commits into
local-inference-lab:dev/jovian-judgementfrom
logprobz:fix/hybrid-sparse-retention-boundaries
Closed

logprobz wants to merge 6 commits into
local-inference-lab:dev/jovian-judgementfrom
logprobz:fix/hybrid-sparse-retention-boundaries

Conversation

@logprobz

@logprobz logprobz commented Sep 4, 2026

Copy link
Copy Markdown

Purpose

This draft builds directly on #556. It preserves both
original commits unchanged, so @tobymao remains their author. Thank you to Toby
Mao for finding and implementing the original Mamba sparse-retention fix.

The follow-up implements findings from
logprobz's review comment
and two independent Claude Opus 5 reviews:

  • Retain both positions an EAGLE/MTP attention lookup can make reachable: the
    aligned boundary and its predecessor. A manager capability now identifies
    lookups that actually drop a proof block, including full attention, MLA, and
    SWA, while excluding Mamba and unsupported local-attention managers. This
    refines the bool(eagle_group_ids) predicate introduced by [BugFix] Mamba sparse retention keeps a state below each boundary under EAGLE/MTP #556.
  • Pass the coordinator's _cache_hit_alignment_tokens into sparse-retention
    masks. For Mamba, retain both the fine lookup position and a
    scheduler-aligned state-materialization fallback, so a mathematically valid
    fine boundary does not displace the only state the scheduler produced.
  • Apply the same boundary and alignment rules to Mooncake store masks. The
    connector now shares the engine's sparse-retention input resolver instead of
    offloading states that local lookup cannot consume. It also mirrors the
    engine's compatibility gate, so mixed SWA and Mamba layouts fall back to
    block-aligned hits instead of asserting during store.

The PR targets dev/jovian-judgement, so its diff contains Toby's two original
commits from #556 followed by four logprobz follow-up commits.

The BlockStored sparse-event concern from the linked review remains out of
scope because it changes connector event semantics. This PR does not file a
separate issue; that work remains tracked in the review comment pending an
independent issue or PR.

Related work

Test Plan

The pytest runs use the pinned r22 container's isolated /opt/venv after adding
the tblib test dependency, which the image does not include:

/opt/venv/bin/python -m pytest -q -p no:cacheprovider \
  tests/v1/core/test_prefix_caching.py \
  tests/v1/core/prefix_cache \
  tests/v1/core/test_mamba_align_chunk_split.py \
  tests/v1/kv_connector/unit/test_mooncake_store_coordinator.py \
  tests/v1/kv_connector/unit/test_mooncake_store_worker.py \
  tests/v1/kv_connector/unit/test_mooncake_store_scheduler.py

The lint runs use an isolated uv environment and the repository's pinned hook
configuration:

pre-commit run --files \
  tests/v1/core/test_prefix_caching.py \
  tests/v1/kv_connector/unit/test_mooncake_store_coordinator.py \
  vllm/v1/core/kv_cache_coordinator.py \
  vllm/v1/core/single_type_kv_cache_manager.py \
  vllm/distributed/kv_transfer/kv_connector/v1/mooncake/store/coordinator.py

pre-commit run mypy-3.12 --hook-stage manual --files \
  tests/v1/core/test_prefix_caching.py \
  tests/v1/kv_connector/unit/test_mooncake_store_coordinator.py \
  vllm/v1/core/kv_cache_coordinator.py \
  vllm/v1/core/single_type_kv_cache_manager.py \
  vllm/distributed/kv_transfer/kv_connector/v1/mooncake/store/coordinator.py

Container image:
voipmonitor/vllm@sha256:284784e685aa0377f1cf63a312a364fc884b02beb98949d6886624edbddb3806.

Test Result

Baseline on the unchanged #556 head, with the first two follow-up tests
overlaid:

  • SWA replay returned 0 cached tokens instead of the reachable 64-token prefix.
  • Sparse retention received 64-token scheduler alignment instead of the
    32-token fine-hit alignment.
  • Result: 2 failed, 95 deselected.

The Opus review added focused coverage against the prior PR head. Four cases
failed before the review fixes:

  • SWA-driven hybrid replay returned 0 instead of 224 tokens for its Mamba group.
  • Pure SWA under EAGLE returned 0 instead of 224 tokens on replay.
  • Mooncake omitted the Mamba and SWA states at the EAGLE-reachable predecessor.
  • Mooncake retained Mamba block 5 with coarse alignment instead of block 6 with
    fine-hit alignment.

A closure review then found that Mooncake enabled fine hits for a mixed SWA and
Mamba layout that the engine correctly rejects. The new regression test failed
before the compatibility fix on assert not coord.enable_partial_hash_hits.
Without that guard, store_mask reached the SWA alignment assertion.

A GPT-6 Astra review found that fine alignment could retain an unmaterialized
Mamba state while removing its usable scheduler-aligned fallback. In its
480-token reproduction, identical replay improved from 0 to 384 tokens after
retaining both positions. Engine and Mooncake regression tests cover this case.

Current head:

  • 382 tests passed across the six suites in the pytest command.
  • All focused EAGLE, SWA, Mamba, and Mooncake regressions passed, including
    three SWA annotation routes and the mixed-layout compatibility fallback.
  • The complete changed-file hook set passed, including Ruff, typos, mypy 3.10,
    SPDX, import, configuration, and forbidden-API checks.
  • The manual CI-style mypy 3.12 hook passed.
  • git diff --check passed.

No GPU model evaluation was run for the follow-up commits. This PR remains a
draft because the repository requires a serving evaluation and human review
before it is ready. logprobz must review every changed line, run the relevant
tests, and add the GLM-5.3-Flash replay results before marking it ready.

AI assistance

OpenAI Codex assisted with implementation, tests, and this description. Two
Claude Opus 5 agents independently reviewed the implementation and the PR's
guideline compliance. The original review posted by logprobz on #556 credits
Claude Fable 5.1. GPT-6 Astra independently reviewed the current PR and found
the fine-hit materialization regression described above.

The first Codex follow-up commit was already published without attribution and
sign-off trailers. It remains unamended to preserve published history without a
force-push. All three later follow-up commits include
Assisted-by: OpenAI Codex and Signed-off-by: logprobz trailers.


Essential Elements of an Effective PR Description Checklist
  • The purpose and distinction from related PRs are described above.
  • Test commands and current results are included.
  • GLM-5.3-Flash model evaluation is pending while this remains a draft.
  • No documentation update is required for this internal cache correction.

BEFORE SUBMITTING, PLEASE READ https://docs.vllm.ai/en/latest/contributing

Superseded

Superseded by #669, now merged on dev/jovian-judgement. The coordinated port preserves endpoint checkpoint guards and includes the cache, scheduler, connector and event repairs from this earlier branch. The combined release passed full MTP3 and DFlash2 serving qualification and was promoted, with zero preemptions and all LP26 5% performance gates satisfied. Closing this older proposal to avoid duplicate integration.

tobymao and others added 3 commits August 31, 2026 22:59
…er EAGLE/MTP

The full-attention EAGLE lookup drops one block below what it matched, so
until a request decodes past the block boundary after its prompt, the only
candidate the coordinator can offer a Mamba group is one block below the
replay boundary. Latest-only retention (prefix_cache_retention_interval=0,
the default) kept exactly the boundary state, leaving every retained state
one block above every reachable candidate: the reconciled prefix-cache hit
was always zero on hybrid models running MTP/EAGLE spec decode.

Measured live on GLM-5.3-Flash (MTP k=3, TP=4): 0 hits across 16,897
queries; per-group lookups on an identical resent 8,901-token prompt
returned (6912, 6912, 6912) for the Mamba groups against 4608 for the
eagle-dropped full-attention group, reconciling to 0.

Keep the state one block below each reachable boundary as well when the
group runs under EAGLE. The extra state only materializes where a block
boundary coincides with an aligned prefill chunk end (the running-state
block), which is how the align-mode scheduler chunks prompts.
…group's drop

Two defects found reviewing the first cut, both caught by new tests:

1. The back-off was hard-coded as one Mamba block, but the drop the
   full-attention finder applies is min(alignment_tokens, its block_size)
   followed by a re-floor to the alignment -- which lands exactly one
   ALIGNMENT unit below the boundary either way. When a group's block size
   differs from the alignment (their LCM), one block is the wrong step: the
   retained state sits at an offset Mamba's own finder rejects, so the hit
   stays 0 and the extra block is dead weight. Now computed in tokens via
   reachable_hit_positions(), which is exact for alignment >, == and < the
   block size.

2. Retention keyed off the group's OWN use_eagle bit, which is 'this group
   holds draft layers'. The right predicate is 'some group's lookup shortens
   the candidate offered to me', since the coordinator reconciles all groups
   to one hit length. It worked only via the coordinator's flag-all fallback
   (no annotator exists for glm5_next); the day one lands, the zero-hit bug
   returns silently. The coordinator now sets lookup_drops_eagle_block on
   every manager from bool(self.eagle_group_ids).

Tests: parametrize the MTP test over the three annotation routes (fallback,
full-only, both) and give it real speculative blocks + lookahead; add a
differing-block-size test pinning the alignment-unit back-off. Both fail on
the previous cut (full_only: 'mamba hash 1 should be cached'; backoff:
'reachable state missing; cached=[5]'). 95/95 in test_prefix_caching.py.
Retain the SWA predecessor reached after a full-attention EAGLE drop and pass the coordinator cache-hit alignment into sparse retention masks.
@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 12 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: ca604b37-ed9d-49b0-abc2-aecc438be364

📥 Commits

Reviewing files that changed from the base of the PR and between a84f907 and 1279e4f.

📒 Files selected for processing (5)
  • tests/v1/core/test_prefix_caching.py
  • tests/v1/kv_connector/unit/test_mooncake_store_coordinator.py
  • vllm/distributed/kv_transfer/kv_connector/v1/mooncake/store/coordinator.py
  • vllm/v1/core/kv_cache_coordinator.py
  • vllm/v1/core/single_type_kv_cache_manager.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Use manager capabilities to identify EAGLE lookups that can lower the
shared boundary. Reuse the engine retention inputs and fine hit alignment
for Mooncake store masks.

Assisted-by: OpenAI Codex
Signed-off-by: logprobz <321553542+logprobz@users.noreply.github.com>
Disable fine hits when an incompatible sparse manager requires block-aligned
lookups. Use the manager capability for EAGLE margins and cover the exact SWA
boundary where the fallback is required.

Assisted-by: OpenAI Codex
Signed-off-by: logprobz <321553542+logprobz@users.noreply.github.com>
Assisted-by: OpenAI Codex
Signed-off-by: logprobz <321553542+logprobz@users.noreply.github.com>
@voipmonitor

Copy link
Copy Markdown

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

lukealonso pushed a commit that referenced this pull request Sep 6, 2026
…points (#669)

* Preserve aligned cache reuse alongside request boundary checkpoints

Port the qualified hybrid retention, fine-hit, event, exact external-state,
and aligned-budget changes onto dev/jovian-judgement. Keep endpoint bundles
private and preserve their allocation, reader, and invalidation lifecycle.

Retain the behavioral contributions from PRs #557, #643, #645, #646,
#655, #656, #657, and #663. Resolve shared helpers once so fine and coarse
replay boundaries are not expanded twice.

Validation: 910 cache, scheduler, parser, event, connector, and endpoint
regressions pass. The three literal LP26 cache regressions also pass.

Co-authored-by: Jason Cook <jasonc@maxlyn.com>
Co-authored-by: Martin Vit <martin@voipmonitor.org>
Co-authored-by: Yifan Qiao <yifanqiao@inferact.ai>
Co-authored-by: haic0 <149741444+haic0@users.noreply.github.com>
Co-authored-by: tobymao <toby.mao@gmail.com>
Signed-off-by: logprobz <321553542+logprobz@users.noreply.github.com>

* Clean up cache regression fixtures for pre-commit checks

Signed-off-by: logprobz <321553542+logprobz@users.noreply.github.com>

---------

Signed-off-by: logprobz <321553542+logprobz@users.noreply.github.com>
Co-authored-by: logprobz <321553542+logprobz@users.noreply.github.com>
Co-authored-by: Jason Cook <jasonc@maxlyn.com>
Co-authored-by: Martin Vit <martin@voipmonitor.org>
Co-authored-by: Yifan Qiao <yifanqiao@inferact.ai>
Co-authored-by: haic0 <149741444+haic0@users.noreply.github.com>
Co-authored-by: tobymao <toby.mao@gmail.com>
@logprobz logprobz closed this Sep 6, 2026
@voipmonitor

Copy link
Copy Markdown

Status: superseded by PR #669 and intentionally closed.

PR #669 consolidates the sparse hybrid replay-boundary behavior from this pull request, preserves the contributing authorship in commit trailers, and was merged into dev/jovian-judgement as 86acae9. Do not merge or cherry-pick PR #643 separately.

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.

3 participants