[II] Require decode state for FULL CUDA graph dispatch - #298
Conversation
|
Warning Review limit reached
Next review available in: 52 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 6602a45d26e9e55c40c258b24b38995da5471dcc and b8d92cb. 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughChangesThe change adds prefill-aware uniform decode classification. Uniform decode dispatch
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GPUModelRunner
participant InputBatch
participant Speculator
GPUModelRunner->>GPUModelRunner: Gather batch request state
GPUModelRunner->>InputBatch: Pass prefill metadata
InputBatch-->>Speculator: Expose has_prefill
Speculator->>Speculator: Compute uniform decode token count
Speculator-->>GPUModelRunner: Select CUDA graph dispatch
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
vllm/v1/worker/utils.py (1)
604-619: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse Google-style docstrings for the new helper APIs.
Add applicable
Args:andReturns:sections.
vllm/v1/worker/utils.py#L604-L619: Document arguments and return values for both classifiers.vllm/v1/worker/gpu/cudagraph_utils.py#L105-L112: Document arguments and return value forget_uniform_token_count.vllm/v1/worker/gpu/model_runner.py#L1336-L1344: Document arguments and tuple return values forgather_batch_req_state.vllm/v1/worker/gpu/input_batch.py#L115-L118: Add aReturns:section forhas_prefill.tests/v1/worker/test_gpu_batch_ordering.py#L50-L79: Document helper arguments and return values.As per coding guidelines, Python code must use Google-style docstrings with
Args:/Returns:/Raises:sections.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vllm/v1/worker/utils.py` around lines 604 - 619, Update the Google-style docstrings for is_uniform_query_len and get_uniform_decode_token_count in vllm/v1/worker/utils.py, get_uniform_token_count in vllm/v1/worker/gpu/cudagraph_utils.py, gather_batch_req_state in vllm/v1/worker/gpu/model_runner.py, has_prefill in vllm/v1/worker/gpu/input_batch.py, and the helpers in tests/v1/worker/test_gpu_batch_ordering.py. Add Args: entries for each parameter and Returns: entries describing each return value, including gather_batch_req_state’s tuple components; the input_batch.py site requires only Returns: if arguments are already documented.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@vllm/v1/worker/utils.py`:
- Around line 604-619: Update the Google-style docstrings for
is_uniform_query_len and get_uniform_decode_token_count in
vllm/v1/worker/utils.py, get_uniform_token_count in
vllm/v1/worker/gpu/cudagraph_utils.py, gather_batch_req_state in
vllm/v1/worker/gpu/model_runner.py, has_prefill in
vllm/v1/worker/gpu/input_batch.py, and the helpers in
tests/v1/worker/test_gpu_batch_ordering.py. Add Args: entries for each parameter
and Returns: entries describing each return value, including
gather_batch_req_state’s tuple components; the input_batch.py site requires only
Returns: if arguments are already documented.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2304580e-066a-4da1-86c1-b430f087e11c
📥 Commits
Reviewing files that changed from the base of the PR and between ce5f50f and 6602a45d26e9e55c40c258b24b38995da5471dcc.
📒 Files selected for processing (9)
tests/v1/spec_decode/test_dynamic_sd_cug.pytests/v1/worker/test_gpu_batch_ordering.pytests/v1/worker/test_gpu_input_batch_v2.pyvllm/v1/worker/gpu/cudagraph_utils.pyvllm/v1/worker/gpu/input_batch.pyvllm/v1/worker/gpu/model_runner.pyvllm/v1/worker/gpu/spec_decode/autoregressive/speculator.pyvllm/v1/worker/gpu/spec_decode/multi_module_mtp/speculator.pyvllm/v1/worker/utils.py
Classify scheduled batches with per-request prefill state before selecting a FULL decode graph. A prompt chunk can contain the same K+1 token count as speculative decode, so token shape alone is not a valid decode invariant. Dummy capture batches remain shape-classified because they are uniform by construction. Uniform all-decode batches retain FULL graph replay. Mixed prefill/decode batches and prompt-only batches use the non-FULL dispatch path. The target runner, autoregressive speculator, and multi-module MTP speculator share the state-aware predicate. Validation: 50 focused tests passed across CUDA graph dispatch, request ordering, V2 input batches, autoregressive speculation, multi-module MTP, and acceptance control. The exact Infernal Invocation integration stack passed 36 focused tests and accepted this commit after all twelve release PR heads without a merge conflict. Ruff, formatting, compileall, and staged diff checks passed. DeepSeek-V4-Flash-0731 with TP2, DSpark K5, concurrency four, and FULL CUDA graphs produced one corrupt response in each of two independent 160-request control runs. The state-aware build completed 160/160 correctly with no response-integrity findings or runtime errors. Concurrent 150,003-token and 300,128-token strict tool-call requests passed with 40 GiB native KV offload enabled, and the server remained healthy. Ported from vLLM commit 0a94d85.
6602a45 to
b8d92cb
Compare
Release-composition qualificationStatus: qualified. Commit Conditions: DeepSeek-V4-Flash-0731 revision
Artifacts:
|
2 participants
Behavior
FULL CUDA graph replay is selected only when every scheduled request is decoding and every request has the same query length. Prompt-only batches and mixed prompt/decode batches use the non-FULL dispatch path even when their token-count shape matches speculative decode.
Dummy capture batches retain shape-only classification because the capture input is uniform by construction. Uniform decode remains eligible for FULL replay in the target runner, autoregressive speculator, and multi-module MTP speculator.
Root cause
Model Runner V2 classified uniform decode from token shape alone. A prompt chunk can contain the same
K+1tokens per request as speculative decode, allowing a decode CUDA graph to replay over prompt state.DeepSeek-V4-Flash-0731 with DSpark K5 exposed the collision at six tokens per request. The failure returned syntactically valid API responses containing repeated, raw-token, or multilingual output; the engine did not necessarily raise an exception.
This change ports the state-aware classification from vLLM commit
0a94d85a, merged through vLLM #51865.Compatibility
Validation
voipmonitor/vllm:infernal-invocation-vllm6c50b0a-b12x1584743-fi1ac6942-cu133-torch213-20260812-r3: two independent TP2/DSpark-K5/C4 runs each produced one corrupt response, at request 159 and request 155 respectively.-0.50%).Summary by CodeRabbit
Bug Fixes
Tests