[None][fix] Handle None attention_dp_relax in ADP router sort key

[Wanli-Jiang]

Summary by CodeRabbit

• Bug Fixes

  • Fixed a routing issue that caused errors when scheduling requests with default parameters, improving system reliability.

• Tests

  • Added regression tests to verify request routing handles default parameter configurations correctly.

Description

Test Coverage

PR Checklist

Please review the following before submitting your PR:

• PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.

• PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.

• Test cases are provided for new code paths (see test instructions)

• If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.

• Any new dependencies have been scanned for license and vulnerabilities

• CODEOWNERS updated if ownership changes

• Documentation updated as needed

• Update tava architecture diagram if there is a significant design change in PR.

• The reviewers assigned automatically/manually are appropriate for the PR.

• Please check this after reviewing the above items as appropriate for this PR.

GitHub Bot Help

To see a list of available CI bot commands, please comment /bot help.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR refactors attention distribution policy (ADP) router sorting logic to safely handle attention_dp_relax=None by introducing a centralized normalization helper. SchedulingParams.attention_dp_relax field is changed from bool to Optional[bool], and both DefaultADPRouter and KVCacheAwareADPRouter replace inline logic with a unified _get_attention_dp_relax static method that treats None as True.

Changes

ADP Router Optional Attention Relax Handling

Layer / File(s)	Summary
SchedulingParams Optional field contract tensorrt_llm/scheduling_params.py	attention_dp_relax field type updated from bool to Optional[bool], with documentation clarifying None defaults to True.
Centralized boolean normalization helper tensorrt_llm/_torch/pyexecutor/scheduler/adp_router.py	New ADPRouter._get_attention_dp_relax() static method normalizes missing or None values to True to ensure sort keys remain boolean-safe.
Router integration and regression tests tensorrt_llm/_torch/pyexecutor/scheduler/adp_router.py, tests/unittest/_torch/executor/test_adp_router.py	Both DefaultADPRouter.route_requests() and KVCacheAwareADPRouter.route_requests() use the new normalization helper. Tests verify both routers successfully route requests with attention_dp_relax=None without raising TypeError.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• NVIDIA/TensorRT-LLM#14276: Both PRs modify adp_router.py's DefaultADPRouter.route_requests() and KVCacheAwareADPRouter.route_requests() to ensure attention_dp_relax=None is treated as the non-strict/relaxed sort key (and add corresponding regression tests).

Suggested reviewers

• joyang-nv
• mikeiovine
• Tabrizian

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description only contains the template placeholder sections without any actual implementation details, test coverage information, or completed checklist items.	Fill in the Description section explaining the issue and solution, list the relevant tests in Test Coverage, and complete the PR Checklist with appropriate checkmarks.
Docstring Coverage	⚠️ Warning	Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main fix: handling None values for attention_dp_relax in the ADP router sort key, which matches the changeset.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

🧹 Nitpick comments (2)

tensorrt_llm/_torch/pyexecutor/scheduler/adp_router.py (1)

160-163: ⚡ Quick win

Update stale type wording in the helper comment.

The note says the field is annotated bool, but this PR updates it to Optional[bool].

✏️ Suggested edit

-        # treated as relaxed (True). The field is annotated bool but callers
+        # treated as relaxed (True). The field is annotated Optional[bool], and callers

🤖 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 `@tensorrt_llm/_torch/pyexecutor/scheduler/adp_router.py` around lines 160 -
163, Update the helper comment in adp_router.py to reflect that
py_scheduling_params.attention_dp_relax is now Optional[bool] (not plain bool)
and clarify that both missing py_scheduling_params or attention_dp_relax=None
are treated as relaxed (True) to avoid TypeError when calling sorted();
reference the py_scheduling_params and attention_dp_relax symbols and adjust the
sentence "The field is annotated bool" to "The field is annotated
Optional[bool]" (or equivalent) so the comment matches the code behavior.

tests/unittest/_torch/executor/test_adp_router.py (1)

207-219: QA list update is unnecessary for this PR scope.

These are unit regression tests under tests/unittest/..., so no tests/integration/test_lists/qa/ entry update is required.

As per coding guidelines, "If the PR only touches unittest/ or narrow unit scope, say explicitly whether QA list updates are unnecessary or optional."

Also applies to: 960-973

🤖 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 `@tests/unittest/_torch/executor/test_adp_router.py` around lines 207 - 219,
This PR only touches unit tests
(tests/unittest/_torch/executor/test_adp_router.py) so explicitly state in the
PR description that QA list updates are unnecessary for this change; update the
PR body to add a short line like "QA list update unnecessary — only unit tests
changed" and, if you want local clarity, add a one-line comment above the
test_attention_dp_relax_none_is_sortable test noting that QA list updates are
not required for unit-scope changes.

🤖 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 `@tensorrt_llm/_torch/pyexecutor/scheduler/adp_router.py`:
- Around line 160-163: Update the helper comment in adp_router.py to reflect
that py_scheduling_params.attention_dp_relax is now Optional[bool] (not plain
bool) and clarify that both missing py_scheduling_params or
attention_dp_relax=None are treated as relaxed (True) to avoid TypeError when
calling sorted(); reference the py_scheduling_params and attention_dp_relax
symbols and adjust the sentence "The field is annotated bool" to "The field is
annotated Optional[bool]" (or equivalent) so the comment matches the code
behavior.

In `@tests/unittest/_torch/executor/test_adp_router.py`:
- Around line 207-219: This PR only touches unit tests
(tests/unittest/_torch/executor/test_adp_router.py) so explicitly state in the
PR description that QA list updates are unnecessary for this change; update the
PR body to add a short line like "QA list update unnecessary — only unit tests
changed" and, if you want local clarity, add a one-line comment above the
test_attention_dp_relax_none_is_sortable test noting that QA list updates are
not required for unit-scope changes.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: bf4a92b7-f4c1-4b38-9be7-af8b605bd9fc

📥 Commits

Reviewing files that changed from the base of the PR and between aac0d65 and 10bbf48.

📒 Files selected for processing (3)

• tensorrt_llm/_torch/pyexecutor/scheduler/adp_router.py
• tensorrt_llm/scheduling_params.py
• tests/unittest/_torch/executor/test_adp_router.py

[Wanli-Jiang]

/bot run --disable-fail-fast

[tensorrt-cicd]

PR_Github #49373 [ run ] triggered by Bot. Commit: 52aab1a Link to invocation

[nv-guomingz]

I also encountered this issue #14267

[Wanli-Jiang]

I also encountered this issue #14267

good, let me close my pr.

[Wanli-Jiang]

same PR from #14267

[tensorrt-cicd]

PR_Github #49373 [ run ] completed with state SUCCESS. Commit: 52aab1a
/LLM/main/L0_MergeRequest_PR pipeline #39026 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