Repository navigation
[None][fix] Responses: a reasoning object turns thinking on; DeepSeek-V4.1 takes OpenAI effort levels - #21053
JunyiXu-nv wants to merge 1 commit into
Conversation
…-V4.1 takes OpenAI effort levels
A Responses request asks a reasoning model to reason by sending a
`reasoning` object; `effort` is optional and defaults to the model's own.
The server enabled thinking only when `effort` was set, so a client that
sends `reasoning` without an effort (the Codex CLI's `{"context":
"all_turns"}`) got no thinking from hybrid models such as DeepSeek-V4.1.
`effort: "none"` was passed on as a reasoning effort of "none" with
thinking on.
A `reasoning` object now turns thinking on at the model's default effort,
`effort: "none"` turns it off, and any other effort is passed on as
before. Keys the caller sets in `chat_template_kwargs` still win.
DeepSeek-V4.1's chat template takes a numeric budget (1-100) or its own
names low/high/max; OpenAI's levels minimal, medium and xhigh made it
raise. apply_chat_template now places OpenAI's levels on the budget
(minimal 25, low 50, medium 62, high 75, xhigh 87, max 100); the renderer
itself stays strict.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Responses reasoning kwargs tensorrt_llm/serve/responses_utils.py, tests/unittest/llmapi/apps/test_chat_template_kwargs_guard.py |
The helper handles absent reasoning, effort values, and caller-provided kwargs. Tests cover these cases. |
DeepSeek V4.1 effort mapping tensorrt_llm/tokenizer/deepseek_v41/tokenizer.py, tests/unittest/llmapi/test_deepseek_v41_tokenizer.py |
The tokenizer maps six named reasoning-effort values to numeric budgets before rendering. Tests verify the mapped values. |
Priority:
Estimated code review effort: 2 (Simple) | ~12 minutes
Change: Bug fix
Possibly related PRs
- NVIDIA/TensorRT-LLM#21049: Both changes update reasoning-effort handling in Responses or chat serving.
Suggested reviewers: qijune, ziyixiong-nv
Merge Risk: 
df419
-
The reviewed reasoning behavior is ready to merge after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title follows the required [None][fix] format and clearly summarizes both related changes: Responses reasoning handling and DeepSeek-V4.1 effort mapping. Description check The description includes the required Description and Test Coverage sections, explains the problem and solution, lists relevant tests, and records reported test results. The PR Checklist is only parti… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description includes the required Description and Test Coverage sections, explains the problem and solution, lists relevant tests, and records reported test results. The PR Checklist is only partially completed, but the substantive description is complete and relevant.
-
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
|
/bot run --disable-fail-fast |
1 similar comment
|
/bot run --disable-fail-fast |
|
PR_Github #80741 [ run ] triggered by Bot. Commit: |
|
PR_Github #80741 [ run ] completed with state
|
|
Automatically added "ci: full pre-merge approved" because this PR has satisfied the required GitHub review approvals. Unresolved review conversations and other required checks remain independent merge requirements. |
Description
A Responses request asks a reasoning model to reason by sending a
reasoningobject. Itseffortis optional; without one, the model reasons at its default effort.On main,
reasoning_chat_template_kwargsturned thinking on only whenreasoning.effortwas set. So a client that sendsreasoningwithout an effort got no thinking from hybrid models that think only on request, such as DeepSeek-V4.1. The Codex CLI is such a client: it can send"reasoning": {"context": "all_turns"}.effort: "none"was passed on to the template as a reasoning effort of"none", with thinking on.Now:
reasoningobject turns thinking on, at the model's default effort.effort: "none"turns thinking off.chat_template_kwargsstill win.reasoningis unchanged.DeepSeek-V4.1's chat template takes a numeric budget from 1 to 100, or its own names
low,highandmax. OpenAI's levelsminimal,mediumandxhighmade it raise, so a Responses request at the defaultmediumeffort failed.DeepseekV41Tokenizer.apply_chat_templatenow places OpenAI's levels on the budget:The names the checkpoint already defines keep their budgets, and the others fall between them in order. The renderer,
encode_messages, stays strict.Test Coverage
tests/unittest/llmapi/apps/test_chat_template_kwargs_guard.py(CPU, inl0_cpu.yml):test_responses_reasoning_object_sets_thinkingcovers no reasoning object, a reasoning object without an effort,mediumandnone;test_responses_caller_thinking_wins_over_the_reasoning_objectcovers a caller's ownthinkingsetting.tests/unittest/llmapi/test_deepseek_v41_tokenizer.py:test_apply_chat_template_places_openai_effort_levels_on_the_budgetcovers each of the six levels. The existingtest_out_of_range_effort_is_rejectedstill holds for the renderer.On a build of current main, both files pass: 129 tests, 1 skipped (the existing vendor-fixture case that needs image input).
PR Checklist
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.🤖 Generated with Claude Code
Dev Engineer Review
reasoning_chat_template_kwargsnow enablesthinkingby default when a reasoning object is present, including wheneffortis absent or empty. It disables thinking by default foreffort: "none"and preserves caller-provided values throughsetdefault. Requests without a reasoning object remain unchanged.apply_chat_templatemaps DeepSeek-V4.1 effort names to numeric budgets:minimal25,low50,medium62,high75,xhigh87, andmax100. Other values pass through. The PR states that the renderer remains strict; this change is scoped to the wrapper mapping.QA Engineer Review
The PR changes two test files. The reasoning kwargs tests cover absent reasoning, context-only reasoning,
mediumandnoneeffort, caller override, and whetherthinkingwas injected. The tokenizer test covers all six effort mappings when thinking is enabled.The CI list includes
test_chat_template_kwargs_guard.pyintests/integration/test_lists/test-db/l0_cpu.yml. The search returned no listing fortest_deepseek_v41_tokenizer.py. No test-list changes were reported. Coverage verdict: sufficient for the described behavior. The PR reports 129 tests passed and one vendor-fixture test skipped because it requires image input; these results were not independently verified here.Per-File QA Perspective
tensorrt_llm/serve/responses_utils.py: Verify that a reasoning object enables default thinking when effort is absent or empty, that"none"disables it, and that caller-provided kwargs take precedence.tensorrt_llm/tokenizer/deepseek_v41/tokenizer.py: Verify thatapply_chat_templatemaps the six named effort levels to the intended budgets and passes other values through.tests/unittest/llmapi/apps/test_chat_template_kwargs_guard.py: Covers reasoning-object defaults and caller overrides. Listed intests/integration/test_lists/test-db/l0_cpu.yml.tests/unittest/llmapi/test_deepseek_v41_tokenizer.py: Covers all six effort-to-budget mappings with thinking enabled. No CI or manual-QA list entry was found in the returned search.