Skip to content

fix(trtllm): handle rc22 disaggregated request changes - #12203

Merged
krishung5 merged 9 commits into
mainfrom
krish/fix-terminal-prefill-response
Jul 30, 2026
Merged

fix(trtllm): handle rc22 disaggregated request changes#12203
krishung5 merged 9 commits into
mainfrom
krish/fix-terminal-prefill-response

Conversation

@krishung5

@krishung5 krishung5 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Overview:

This PR fixes two compatibility gaps encountered when running Dynamo
disaggregated serving with TensorRT-LLM rc22:

  1. rc22 changed global disaggregation request IDs from a single 10-bit
    machine_id to a (node_id, process_id) pair. Passing the existing Dynamo
    machine ID as node_id can exceed the rc22 8-bit range.
  2. A CTX request can finish during its one-token prefill step, for example by
    sampling EOS or a stop token. That terminal response legitimately has no
    KV-cache handoff IDs, but Dynamo currently dispatches GEN anyway and
    eventually fails with ctx_request_id is None.
    Example error:
[07/24/2026-01:28:15] [TRT-LLM] [E] [_torch][RANK 7]
  Error in event loop: ctx_request_id is None for task unique_rid=145494718845048

  Traceback (most recent call last):
    ...
    File ".../pyexecutor/py_executor.py", line 5997, in _recv_disagg_gen_cache
      self.kv_cache_transceiver.request_and_receive_async(req)
    File ".../disaggregation/native/transfer.py", line 1497, in _build_recv_req_info
      assert task._params.ctx_request_id is not None, (
  AssertionError: ctx_request_id is None for task unique_rid=145494718845048

Closes DIS-2530

Details:

Request-ID compatibility

  • Detect the installed TRT-LLM request-ID API signature.
  • Preserve rc21 behavior by passing the existing Dynamo machine ID unchanged.
  • For rc22, losslessly split the Dynamo 10-bit worker slot with
    divmod(machine_id, 64) into the new 8-bit node ID and 6-bit process ID
    fields.
  • Add tests for the rc21 and rc22 paths, range validation, and uniqueness
    across all 1,021 Dynamo machine-ID slots.

Terminal prefill responses

  • Distinguish terminal CTX responses from responses that still require a GEN
    handoff.
  • Continue to GEN for FinishReason::Length and an absent finish reason,
    which corresponds to not_finished.
  • Return EOS, stop, cancellation, and error responses directly from CTX
    without dispatching GEN.
  • Strip internal disaggregated parameters before returning the terminal
    response.
  • Add regression tests for terminal, length-limited, and unfinished prefill
    responses.

The terminal behavior matches the native TensorRT-LLM disaggregated
orchestrator fix for NVBug 6245861.

Validation:

  • cargo fmt --all -- --check
  • Pre-commit checks passed for all five changed files.
  • PROTOC=/home/krish/.local/protoc/bin/protoc cargo test -p dynamo-llm kv_router::prefill_router --lib — 20 passed.
  • The compatibility helper was smoke-tested against both rc21 and rc22
    function signatures.
  • Bia B300 AgentPerf validation with both fixes: approximately 2.49M completed
    frontend requests across 13 post-fix runs, zero ctx_request_id is None
    assertions, and one observed terminal CTX response returned successfully
    without a GEN dispatch.

cargo clippy was not available in the local Rust toolchain; CI should run it.

Where should the reviewer start?

  • components/src/dynamo/trtllm/utils/disagg_utils.py: cross-version
    request-ID adapter.
  • lib/llm/src/kv_router/prefill_router/admission.rs: terminal
    classification.
  • lib/llm/src/kv_router/prefill_router/mod.rs: direct terminal response path
    before decode routing.

Related Issues

🚫 This PR is NOT linked to an issue:

  • Confirmed — no related issue

Summary by CodeRabbit

  • Bug Fixes

    • Improved compatibility for disaggregated request IDs across TRT-LLM API versions.
    • Correctly handles completed one-token prefill responses without unnecessary decode routing.
    • Strengthened validation of KV-cache handoff metadata, returning clear errors for missing or invalid request identifiers.
  • Tests

    • Added coverage for request ID compatibility, boundary validation, terminal prefill outcomes, and invalid handoff metadata.

Signed-off-by: krishung5 <krish@nvidia.com>
@github-actions github-actions Bot added fix router Relates to routing, KV-aware routing, etc. backend::trtllm Relates to the trtllm backend labels Jul 27, 2026
@krishung5 krishung5 changed the title fix(kv-router): return terminal prefill responses directly fix(trtllm): handle rc22 disaggregated request changes Jul 27, 2026
@datadog-official

datadog-official Bot commented Jul 27, 2026

Copy link
Copy Markdown

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 44.48% (-1.37%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 6314461 | Docs | Datadog PR Page | Give us feedback!

Signed-off-by: krishung5 <krish@nvidia.com>
Signed-off-by: krishung5 <krish@nvidia.com>
@krishung5
krishung5 marked this pull request as ready for review July 27, 2026 17:31
@krishung5
krishung5 requested review from a team as code owners July 27, 2026 17:31

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

Open in Devin Review

Comment thread lib/llm/src/kv_router/prefill_router/mod.rs
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The change adds TRT-LLM request ID compatibility across API versions and updates prefill routing to classify terminal responses, validate handoff metadata, and bypass decode routing for terminal outputs.

Changes

Disaggregation request ID compatibility

Layer / File(s) Summary
Compatible request ID generation
components/src/dynamo/trtllm/utils/disagg_utils.py, components/src/dynamo/trtllm/tests/test_trtllm_disagg_request_id.py
Adds machine ID validation, process-aware ID mapping, legacy fallback handling, and unit tests for both API paths and invalid IDs.
Request handler integration
components/src/dynamo/trtllm/request_handlers/handler_base.py
Uses the compatibility helper when creating or backfilling PREFILL disaggregation request IDs.

Terminal prefill routing

Layer / File(s) Summary
Prefill completion classification and validation
lib/llm/src/kv_router/prefill_router/admission.rs
Classifies non-length finish reasons as terminal and rejects missing or null TRT-LLM context request IDs for handoff paths, with corresponding tests.
Terminal response normalization and routing
lib/llm/src/kv_router/prefill_router/mod.rs
Adds terminal completion handling, removes disaggregation parameters, returns a single-item stream, and prevents terminal outcomes from reaching decode routing.

Estimated code review effort: 4 (Complex) | ~45 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check ✅ Passed The title accurately covers the rc22 request-ID compatibility change, though it omits the terminal prefill fix.
Description check ✅ Passed The description matches the required template sections and includes overview, details, reviewer start points, and issue status.

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@lib/llm/src/kv_router/prefill_router/admission.rs`:
- Around line 113-128: Move the terminal-output classification around
first_output ahead of the non-bootstrap stream drain in the prefill admission
flow. When the first frame is terminal, return PrefillCompletion::Terminal
immediately while background-draining remaining frames using the existing
bootstrap behavior, so delayed frames and trailing Annotated::from_error values
cannot alter the result. Add a regression test covering a terminal first frame
followed by a trailing error.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b3c53733-32d1-445e-ae9a-59ebed423729

📥 Commits

Reviewing files that changed from the base of the PR and between 47222ed and fc8b649.

📒 Files selected for processing (5)
  • components/src/dynamo/trtllm/request_handlers/handler_base.py
  • components/src/dynamo/trtllm/tests/test_trtllm_disagg_request_id.py
  • components/src/dynamo/trtllm/utils/disagg_utils.py
  • lib/llm/src/kv_router/prefill_router/admission.rs
  • lib/llm/src/kv_router/prefill_router/mod.rs

Comment thread lib/llm/src/kv_router/prefill_router/admission.rs
Signed-off-by: krishung5 <krish@nvidia.com>
@krishung5
krishung5 enabled auto-merge (squash) July 27, 2026 23:57
@krishung5
krishung5 merged commit bde20c9 into main Jul 30, 2026
247 of 252 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend::trtllm Relates to the trtllm backend fix router Relates to routing, KV-aware routing, etc. size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants