Skip to content

fix(glm): classify long DCP profile rows as prefill - #822

Merged
lukealonso merged 2 commits into
dev/karmic-krakenfrom
fix/karmic-glm-dcp-profile-kda
Sep 22, 2026
Merged

lukealonso merged 2 commits into
dev/karmic-krakenfrom
fix/karmic-glm-dcp-profile-kda

Conversation

@voipmonitor

@voipmonitor voipmonitor commented Sep 21, 2026

Copy link
Copy Markdown

Result

A GLM-5.3-Flash TP4/DCP4/MTP3 server with a 4096-token scheduler budget starts successfully instead of sending a 4096-token memory-profile prefill into the 128-token B12X KDA decode kernel. The GDN builder and its B12X mixed worklist classify a row as speculative decode only when its query length equals one target token plus the declared draft count. Valid one-token and MTP decode rows keep their existing route.

Cause and compatibility

During profile_glm_dcp_attention, a dummy 4096-token prefill can carry a zero-draft marker. A nonnegative marker alone is not evidence that the row is decode. The former condition classified this prefill as speculative decode and raised b12x KDA capacity exceeded: tokens=4096/128. The fix does not alter checkpoint content, model weights, sampling, or real decode geometry. An explicit out-of-capacity speculative row remains an error.

Validation

The published beta image reproduced the failure on four RTX PRO 6000 Blackwell Max-Q GPUs, TP4/DCP4, MTP3, max-num-batched-tokens 4096, max-num-seqs 32, with LMCache disabled. A diagnostic trace at the builder showed query_start=[0,4096], drafts=[0]. With the patched source bound into the same image, the server passed memory profiling, allocated 37.1 GiB KV cache per rank, captured CUDA graphs, completed JIT warmup, passed /health, and answered a temperature-1 chat request with READY. This is a startup and short-response evaluation, not a throughput or long-context claim.

The full metadata test file passed: /opt/venv/bin/python -m pytest -q -p no:cacheprovider tests/v1/attention/test_gdn_metadata_builder.py (57 passed) in the published CUDA image with the changed files bound over installed sources. ruff check, ruff format --check, and git diff --check passed locally.

Duplicate check and review

A search of open LIL vLLM PRs found no PR for this long-prefill/zero-draft KDA classification. #821 addresses recurrent-state reservation and external checkpoint recovery; it does not fix this startup crash. AI assistance was used. A human maintainer must review the changed lines and validation before merge.

Summary by CodeRabbit

  • Bug Fixes
    • Corrected speculative decoding classification for requests with zero draft tokens.
    • Long prefills with zero-draft profile markers now remain on the prefill path instead of entering the bounded decode kernel.
    • Valid multi-token prediction decode requests continue to use the optimized decoding path.
    • Added coverage for mixed prefill and decode metadata scenarios.

Signed-off-by: Martin Vit <martin@voipmonitor.org>
(cherry picked from commit 65f73469a324f54f43b305590081dc3fd83daf0e)
@coderabbitai

coderabbitai Bot commented Sep 21, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: a6ef8eee-442c-4548-8a1b-09ff0834ec73

📥 Commits

Reviewing files that changed from the base of the PR and between 9e5d179 and e36edd0.

📒 Files selected for processing (4)
  • .lil/changes/vllm-822.json
  • tests/v1/attention/test_gdn_metadata_builder.py
  • vllm/v1/attention/backends/b12x_gdn_metadata.py
  • vllm/v1/attention/backends/gdn_attn.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The change tightens speculative KDA classification to require query spans equal draft tokens plus one target token. Tests cover zero-draft prefills and decodes, and release metadata records the correction.

Changes

GDN metadata classification

Layer / File(s) Summary
Tighten speculative-row classification
vllm/v1/attention/backends/b12x_gdn_metadata.py, vllm/v1/attention/backends/gdn_attn.py
Speculative rows now require an exact match between query span and draft count plus one target token.
Validate zero-draft classification
tests/v1/attention/test_gdn_metadata_builder.py, .lil/changes/vllm-822.json
Tests cover long-prefill markers, single-token zero-draft decodes, and mixed B12X metadata. Release metadata records the correction.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: lukealonso

Merge Risk: ⚪ Minimal · up to e36ed

The revised classification preserves long prefills and retains the capacity guard for explicitly supplied oversized speculative metadata. No actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: classifying long GLM DCP profile rows as prefill.
Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

Signed-off-by: Martin Vit <martin@voipmonitor.org>
@lukealonso
lukealonso merged commit 2b8d9d0 into dev/karmic-kraken Sep 22, 2026
6 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.

2 participants