[None][refactor] BREAKING remove conversation ID from disaggregated params

[jiaganc]

Dev Engineer Review

• Removed conversation_id from LLM API and OpenAI serving DisaggregatedParams.
• Updated conversation ID resolution to use ConversationParams.conversation_id as the sole request-body source.
• Removed obsolete conversion and forwarding logic.
• This is an API-breaking change for callers that set DisaggregatedParams.conversation_id.
• No configuration files or test-list files changed.
• The changes are consistent across dataclasses, protocol conversion, routing, and disaggregated services.
• Pre-commit checks, Python compilation, and whitespace validation passed.
• Full pytest was not run because the remote-container workflow was required.

QA Engineer Review

• Modified coordinator routing tests to use ConversationParams.conversation_id.
• Modified internal authentication tests and renamed test_unprotected_disagg_fields_do_not_invalidate_internal_auth_header to test_conversation_params_do_not_invalidate_internal_auth_header.
• Removed obsolete conversation_id conversion assertions and the conversation ID round-trip test.
• Modified server tests to validate conversation_params.conversation_id.
• Added a regression test for _get_ctx_request serialization of conversation_params.conversation_id.
• No corresponding tests/integration/test_lists/ coverage entries were updated.
• Verdict: needs follow-up because full pytest and CI test-list coverage were not verified.

Description

Remove conversation_id from both the LLM API and OpenAI serving DisaggregatedParams models. Conversation-aware routing and context/generation requests now carry the identifier only through ConversationParams.conversation_id, eliminating the duplicate source of truth and the associated conversion and forwarding logic.

This is an API-breaking change for callers that still construct DisaggregatedParams with conversation_id; those callers must migrate to ConversationParams.

Upgrade Compatibility

This change is not safe for mixed-version disaggregated deployments. Drain traffic and upgrade the disaggregated coordinator and context servers together, then upgrade generation servers. An older context server may send disaggregated_params.conversation_id, which a newer generation server rejects because unknown protocol fields are forbidden. Roll back in reverse order.

Test Coverage

• Updated disaggregated coordinator, internal-auth, protocol-conversion, server, and service tests to use ConversationParams.
• pre-commit run --files <changed files>
• python3 -m py_compile <changed Python files>
• git diff --check upstream/main...HEAD

Full pytest was not run locally because the TensorRT-LLM development workflow requires tests to run in a remote-host container.

PR Checklist

• PR description clearly explains what and why.
• PR follows TRT-LLM coding guidelines to the best of my knowledge.
• Existing functional tests were updated for the canonical conversation-parameter path.
• API change is identified as breaking in the title and description.
• No new dependencies or ownership changes.
• No architecture diagram update is required.

[jiaganc]

/bot run

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 9961f0e2-1807-45f7-a0ad-35b86548f4c5

📥 Commits

Reviewing files that changed from the base of the PR and between 8398196 and 688403e.

📒 Files selected for processing (9)

• tensorrt_llm/disaggregated_params.py
• tensorrt_llm/serve/conversation_id.py
• tensorrt_llm/serve/openai_disagg_service.py
• tensorrt_llm/serve/openai_protocol.py
• tests/unittest/disaggregated/test_coordinator_worker.py
• tests/unittest/disaggregated/test_disagg_internal_auth.py
• tests/unittest/disaggregated/test_disaggregated_params.py
• tests/unittest/disaggregated/test_openai_disagg_server.py
• tests/unittest/disaggregated/test_openai_disagg_service.py

💤 Files with no reviewable changes (2)

• tests/unittest/disaggregated/test_openai_disagg_server.py
• tensorrt_llm/serve/openai_disagg_service.py

---

Walkthrough

The change removes conversation_id from DisaggregatedParams. Conversation IDs now use ConversationParams during resolution, disaggregation, routing, authentication, and wire-preservation tests.

Changes

Conversation ID ownership

Layer / File(s)	Summary
Conversation parameter contract and resolution tensorrt_llm/disaggregated_params.py, tensorrt_llm/serve/conversation_id.py, tensorrt_llm/serve/openai_protocol.py	DisaggregatedParams no longer stores or serializes conversation_id. Resolution reads request.conversation_params.conversation_id and documents body precedence over headers.
Disaggregation request propagation tensorrt_llm/serve/openai_disagg_service.py	Context and generation requests no longer copy conversation IDs into disaggregation parameters.
Routing and wire-path test updates tests/unittest/disaggregated/*	Tests use ConversationParams for routing, authentication, and wire round trips. Service tests verify conversation ID preservation. Assertions for removed disaggregation fields are deleted.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score: 🔵 Low · up to 68840

This change removes conversation_id from the disaggregated parameter APIs and requires callers to migrate to ConversationParams. No concrete runtime defect is identified, but merge should proceed with explicit owner approval for the breaking API change.

Suggested reviewers: juney-nvidia

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	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.
Title check	✅ Passed	The title clearly identifies the breaking refactor that removes conversation IDs from disaggregated parameters.
Description check	✅ Passed	The description explains the change, breaking API impact, upgrade order, test coverage, and checklist status.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[tensorrt-cicd]

PR_Github #65807 [ run ] triggered by Bot. Commit: 688403e Link to invocation

[tensorrt-cicd]

PR_Github #65807 [ run ] completed with state FAILURE. Commit: 688403e
/LLM/main/L0_MergeRequest_PR pipeline #53511 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

• Please check the failed tests and fix your PR
• If you cannot view the failures, ask the CI triggerer to share details
• Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

[Shixiaowei02]

Since OpenAIBaseModel is extra=forbid, an old context server still sending conversation_id would be rejected outright by an upgraded generation server. Worth noting the upgrade order in the description.

[jiaganc]

Thanks @Shixiaowei02 for reviewing. Added a section in the PR description about upgrade order.

[jiaganc]

/bot run

[tensorrt-cicd]

PR_Github #65917 [ run ] triggered by Bot. Commit: 15638c9 Link to invocation

[tensorrt-cicd]

PR_Github #65917 [ run ] completed with state SUCCESS. Commit: 15638c9
/LLM/main/L0_MergeRequest_PR pipeline #53608 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

• Please check the failed tests and fix your PR
• If you cannot view the failures, ask the CI triggerer to share details
• Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

[Shixiaowei02]

LGTM. The PR needs another approval from @reasonsolo before merging. Thanks for the effort.