Conversation
…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.
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>
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>
|
Warning Review limit reachedNext included review available in 12 minutes. View limit detailsLimit 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. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (12)
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. Comment |
Adapt the sparse BlockStored context from vllm-project#44488 so consumers can reconstruct omitted logical blocks. Co-authored-by: Librua <smallliu_2001@163.com> Assisted-by: OpenAI Codex <codex@openai.com> Signed-off-by: logprobz <321553542+logprobz@users.noreply.github.com>
Track the last published full-block endpoint per Request and cache group in a WeakKeyDictionary, so later publications report only the unannounced span as skipped context instead of re-announcing blocks consumers already hold. Anchors are validated against the request's current block hashes, since streaming updates truncate requests in place, and are cleared on a successful prefix-cache reset. Full and partial publication share one skipped-context helper; partial events build skipped extra keys at hash_block_size granularity and never advance the anchor. Full replays start at the root and advance the anchor after each published full run. Partial-only replays start at the root even when a saved anchor exists. Assisted-by: LOCAL/glm-5.3-flash-local-lab-nvfp4-dflash2 (initial implementation) Assisted-by: OpenAI Codex (parent orchestration) Assisted-by: GPT-6 Astra (review corrections and validation) Signed-off-by: logprobz <321553542+logprobz@users.noreply.github.com>
|
@coderabbitai review |
|
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>
|
Status: superseded by PR #669 and intentionally closed. PR #669 consolidates the truthful sparse KV-event reporting and replay continuity 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 #645 separately. |
|
@voipmonitor Thanks for consolidating this work into #669. I noticed that the merged commit 86acae9 does not include a Co-authored-by trailer for me (Librua, author of vllm-project#44488), although #645 credits vllm-project#44488 and the skipped-context design was carried into the merged change. Could you please add or otherwise record the missing co-author attribution in the final PR/release record? Thanks. |
1 similar comment
|
@voipmonitor Thanks for consolidating this work into #669. I noticed that the merged commit 86acae9 does not include a Co-authored-by trailer for me (Librua, author of vllm-project#44488), although #645 credits vllm-project#44488 and the skipped-context design was carried into the merged change. Could you please add or otherwise record the missing co-author attribution in the final PR/release record? Thanks. |
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
4 participants
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.
Purpose
Sparse
BlockStoredevents can claim missing blocks or omit the context neededto reconstruct a retained block after a gap. This change emits contiguous runs
of retained blocks and adopts the skipped-context fields from
vllm-project#44488, while fixing full cached-prefix replay in this fork.
This draft is stacked on #643 and must not merge first.
Its event-specific commits are isolated in the
stack comparison.
carries its exact hashes, tokens, extra keys, and immediate logical parent.
skipped_parent_block_hash,skipped_token_ids,and
skipped_extra_keys, allowing consumers to reconstruct omitted context.respects the reconciled hit length, DCP-effective block sizes, and registered
partial aliases. Every full replay starts at root, including partial-only
replay when a saved publication anchor exists.
prefill chunks. Saved hashes are validated before reuse, anchors clear on a
successful cache reset, and bookkeeping never retains Request objects strongly.
partial replay. Published full runs update the anchor; partial events do not
advance the full-block anchor. Partial context uses hash-block granularity.
Stored hashes, ownership checks, and reconciled lengths are preserved.
The event-specific diff changes metadata publication and the public event
schema. It does not change KV tensors, insertion, eviction, or scheduling.
The underlying cache-retention changes belong to #643. The context-anchor
fix preserves the BlockStored schema introduced by the earlier event adaptation.
Compatibility and attribution
Credit @Li-brua (Librua) for vllm-project#44488. Its skipped-context design
is adapted here with co-author attribution in the commit. The fork-specific
replay correction remains necessary because that upstream PR does not repair
full cached-prefix replay. Credit @glaziermag for the report in
vllm-project#44451.
The optional fields follow the fork's existing fields. Tests verify old and
new payloads decode compatibly when the new fields are unset. MsgPack bytes
are not identical: msgspec may select
map16instead offixmapfor thelarger struct. Consumers that compare encoded bytes must account for this.
The DCP-effective replay sizing overlaps with @jperezdealgaba's
vllm-project#50118. The event-kind markers in vllm-project#51699
remain a separate change.
Validation
The original eight acceptance cases failed before implementation. A stale-anchor
case and three parent-review cases bring focused coverage to twelve cases, all
passing. The three parent-review cases also failed before their corrections.
They cover partial-only replay with a saved anchor and full replay followed by
incremental full or partial publication, including preservation of the full-block
anchor after partial events. The parent reran the exact g1 acceptance command
successfully: 12 passed.
Seven regression suites passed 443 tests: prefix caching, the prefix-cache test
directory, Mamba chunk alignment, Mooncake store coordinator, worker and scheduler,
and distributed KV-cache events. Coverage includes sparse gaps, request and group
isolation, reset and stale anchors, replay ownership, partial aliases, skipped
tokens and extra keys, hashing, and legacy decoding.
Repository-pinned changed-file pre-commit hooks, manual mypy-3.12,
git diff --check, and Python compilation passed. Commands and local receiptsare recorded in
/tmp/pr645-context-checks/review-validation.md, with the parentacceptance rerun in
parent-goal.log, regressions inregression-review.log,hooks in
hooks-review-final.log, and mypy inmypy312-review.log.These are CPU tests with the PR source mounted read-only into the pinned LP26
test container (
sha256:da3588b2adf80999c20cfc254a98cbcc8807a8f3ab357ea769279597df66edb5).They are not GPU model qualification or a rebuilt LP26 integration. No additional
test-container runs were made for publication.
The earlier skipped-context adaptation passed 201 prefix-cache and KV-event
tests in the digest-pinned r25 container. That earlier source was also included
in the local LP26 integration based on
voipmonitor/vllm@sha256:89376e9aa49442a90754662ca1bb281bffbeca29bb7393e6e8281506e5ac4804.That integration retains r25's exact Mamba block-ID handoffs and adjusts test
fixtures for r25's scheduler. Its results are separate from this PR's head;
this update does not rebuild or change that integration.
The runtime was untouched. No new model evaluation was run for these changes.
The cache-retention changes in #643 still require GPU qualification, and this
draft must not merge before #643.
AI assistance
GPT-6 Astra implemented and reviewed the initial event adaptation. OpenAI
Codex assisted with the skipped-context follow-up, integration, and validation.
For the context-anchor fix, LOCAL/glm-5.3-flash-local-lab-nvfp4-dflash2
provided the initial implementation, OpenAI Codex handled parent orchestration,
and GPT-6 Astra performed corrections, review, and validation. The draft remains
subject to the submitting human's review.
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.