Skip to content

[Bugfix] Flag every KV cache group holding a separately prefixed drafter's layers as a draft group - #56026

Open
Windless84 wants to merge 1 commit into
vllm-project:mainfrom
Windless84:fix/drafter-prefix-eagle-groups
Open

Windless84 wants to merge 1 commit into
vllm-project:mainfrom
Windless84:fix/drafter-prefix-eagle-groups

Conversation

@Windless84

Copy link
Copy Markdown

Purpose

Follow-up to #52047, complementary to #55390.

On Qwen3.8-Flash-Next (Qwen4ExpForConditionalGeneration: GDN + QSA + MTP, packed grouping path) no KV cache group is ever identified as the drafter's, so the coordinator falls back to flagging every group, the four Mamba groups included, as a draft group. Effects on a live server:

  • kv_cache_utils.py:1893 warning on every start.
  • Zero cross-request prefix-cache hits: a replay of 8 prompts with shared prefixes (34K/41K/45K chained, 140K then 250K sharing the 140K) queried 816,343 tokens and hit 0.

Why the existing rules miss it:

  1. Rule 1 (non_causal_multi_token_decode) is DSpark-only.
  2. Rule 2 (last registered layer) is gated on model_type == "deepseek_v4". [Bugfix] Annotate MTP draft KV cache groups positionally on the hybrid grouping path #55390 generalizes the gate to method == "mtp" but, as its docstring notes, flags only the group holding the very last layer. The Qwen4Exp MTP block registers three caches under mtp. (QSA attention KV, compressed-key MLAAttentionSpec, ring CircularBufferSpec) that land in two packed groups, so the group holding the drafter's attention KV can still be left unflagged.

Fix

Rule 3 in _annotate_eagle_groups: when no group was flagged, and the last registered layer's top-level module prefix differs from the first layer's (a separately built drafter registered after the target), flag every group holding a layer under that prefix. Drafters that reuse the target's prefix (EAGLE heads under model.) are not detected and keep the conservative all-groups fallback, as before.

_warn_if_unannotated_eagle_mamba now keys on use_eagle_block_drop(), which is what KVCacheManager actually receives; with disable_eagle_block_drop no group is a draft group and the warning was spurious.

If #55390 lands first this rebases on top of it cleanly (rule 3 only runs when rules 1 and 2 flagged nothing).

Test Plan

pytest tests/v1/core/test_kv_cache_utils.py -q

New: test_annotate_eagle_groups_flags_separately_prefixed_drafter_layers (draft layer under mtp. flags its group and leaves the Mamba group alone; same-prefix layout flags nothing; no block drop configured leaves everything untouched).

Test Result

106 passed on main (d290609), CPU.

Live, Qwen3.8-Flash-Next NVFP4 on one RTX PRO 6000, MTP 3, 262,144 context, 240 blocks of 1,600 tokens (3x12 GDN, 1 PLE, 13 QSA rings, 13 QSA main+compressed):

draft groups 1893 warning replay hits
before none flagged (all 6 treated as draft) yes 0 of 816,343 queried tokens
after groups 4 and 5 (the two holding mtp. layers) no 41K after 34K: 33,600 cached, 0.88 s (was 4.0 s); 250K after 140K: 139,200 cached, 11.5 s (was 25.6 s)

Footprint unchanged (140K prompt = 113 of 240 blocks before and after). Note that the first request extending a prefix still misses with the trailing-block drop on, because the align-mode Mamba checkpoint sits on the last full block while the attention hit is offered one block lower; that is #52244's write-side fix, and the numbers above were taken with disable_eagle_block_drop as a stand-in.


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.

https://claude.ai/code/session_01LqPLAsuNFXRF7yoojyna8j

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
🔒 Security Review Completed 2026-09-09T07:50:50.375034Z 506a4bd PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment /ci run for upstream CI or /amd-ci run for AMD CI only whenever CI signals are needed.

Once the PR is approved or has the ready label, the PR author can also use the corresponding /ci run, /ci retry, and /ci cancel commands, or their /amd-ci variants. New commits do not start upstream CI automatically.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

Agent Guidelines

IMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban.

🚀

…l-groups draft fallback only when the block drop is on

Qwen4ExpMTP registers its attention layers under "mtp." after the target's
"model." layers, but no group carried a spec marker and the DeepseekV4
positional fallback is model-gated, so _annotate_eagle_groups flagged nothing,
the coordinator treated every KV group as a draft group and Mamba groups
could never serve a prefix hit (kv_cache_utils.py:1893 warning; zero hits
across a full replay). Flag every group holding a layer under the drafter's
distinct top-level prefix; drafters sharing the target's prefix keep the old
conservative fallback. The warning now keys on use_eagle_block_drop(), which
is what KVCacheManager actually receives.

Claude-Session: https://claude.ai/code/session_01LqPLAsuNFXRF7yoojyna8j
Signed-off-by: Windless84 <15976122+Windless84@users.noreply.github.com>
@rufftruffles

Copy link
Copy Markdown

Tested exact head 4a365dee4aed8ae3135213b4cf6ce04f6d82e756 live on one RTX PRO 6000 using the ae71862c current-main nightly, Qwen3.8-27B target, DFlash2 K=7, 262K native model with the existing 1M YaRN test configuration, and sparse retention 0.

The patch applied cleanly. Its annotation behavior check passed: the Mamba group remained unflagged, the separately prefixed drafter group was flagged, same-prefix layouts remained unflagged, and block-drop-off left the groups untouched. The live cannot identify DFlash's draft KV cache group warning also disappeared.

Fresh-server 50,011-token cold/replay results:

Mode Accuracy Cold TTFT Replay TTFT Cumulative queries Cumulative hits
default block drop 6/6, 6/6 11.456 s 10.435 s 100,022 0
disable_eagle_block_drop 6/6, 6/6 10.543 s 1.200 s 100,022 46,144

This verifies #56026 fixes the DFlash group-annotation failure. The default-drop miss is a separate write-side issue; disabling the drop recovers reuse immediately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working kv-cache-manager

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants