[zw-g]

Dev Engineer Review

DeepSeek-V4 and V4.1 now attach request-level tools to a copied leading system message. They preserve the caller’s dictionary and retain synthetic-message behavior when no leading system message exists. No public API declarations changed.

QA Engineer Review

Modified tests cover tool attachment, system-message ordering, input immutability, and prevention of an extra system block. Reported results are 48 passed, 42 skipped; skipped tests require a local DeepSeek checkpoint. Coverage is sufficient. No test-list changes were reported.

Per-File QA Perspective

• tensorrt_llm/tokenizer/deepseek_v4/tokenizer.py: Verify leading system handling and synthetic-message fallback.
• tensorrt_llm/tokenizer/deepseek_v41/tokenizer.py: Verify the equivalent V4.1 behavior.
• tests/unittest/llmapi/test_deepseek_v4_tokenizer.py: Covers request-level tools, output ordering, and input immutability. No test-list entry was reported.
• tests/unittest/llmapi/test_deepseek_v41_tokenizer.py: Covers tool attachment, single-system-message rendering, and input immutability. No test-list entry was reported.

Description

Fixes #17021.

DeepseekV4Tokenizer.apply_chat_template handles the OpenAI tools argument with an unconditional messages.insert(0, {"role": "system", "tools": tools}). The renderer's system branch emits content and then "\n\n" + <tool block>, so when the request already has a system message:

• the synthetic message renders first as an empty prompt followed by the tool block (a stray \n\n right after BOS);
• the caller's system prompt renders second, after the tool block and glued to its trailing newline.

The DeepSeek reference format attaches the tools to the existing system message (the vendor fixtures do the same: messages[0]["tools"] = case["tools"]). This affects every /v1/chat/completions request that carries both a system message and tools, which is the normal serving path (async_apply_chat_template(tools=...) -> tokenizer.apply_chat_template(messages, tools=tools)). Measured with a system prompt + one tool + one user turn:

before: <BOS>\n\n## Tools\n...invoke tool calls.\nSYSTEM_PROMPT_HERE<｜User｜>Q<｜Assistant｜></think>
after:  <BOS>SYSTEM_PROMPT_HERE\n\n## Tools\n...invoke tool calls.\n<｜User｜>Q<｜Assistant｜></think>

DeepseekV41Tokenizer.apply_chat_template (added after the issue was filed) has the same insert and renders two <｜System｜> blocks with the tool block first:

before: <BOS><｜System｜>\n\n## Tools\n...invoke tool calls.\n<｜System｜>SYSTEM_PROMPT_HERE<｜User｜>Q<｜Assistant｜></think>
after:  <BOS><｜System｜>SYSTEM_PROMPT_HERE\n\n## Tools\n...invoke tool calls.\n<｜User｜>Q<｜Assistant｜></think>

The change, the same in both files: when messages[0] is a system message, the request tools are attached to a copy of it ({**messages[0], "tools": tools}, so the caller's dict is not modified); otherwise the synthetic system message is inserted as before. This is the fix suggested in the issue, and it matches how vLLM and SGLang attach request tools for these models. The closed #17036 took the same approach for V4 only.

Behaviour notes:

• Requests without a system message, or starting with a developer message, render byte-identically to before.
• Only messages[0] is considered, as in the issue's suggested fix and the vendor fixtures.
• If messages[0] already carries its own tools and tools= is also passed, the request tools take its place; previously two tool blocks were rendered. The OpenAI server does not attach message-level tools for these models.
• deepseek_v32/tokenizer.py inserts a synthetic system message the same way, but it is a separate model and encoder, so it is left out of this PR.

Test Coverage

New regression tests, which fail on main and pass with this change:

• tests/unittest/llmapi/test_deepseek_v4_tokenizer.py::test_deepseek_v4_chat_template_attaches_request_tools_to_leading_system_message
• tests/unittest/llmapi/test_deepseek_v41_tokenizer.py::test_tools_argument_is_attached_to_an_existing_leading_system_message

Each one asserts that passing tools= with a leading system message renders the same prompt as attaching those tools to that system message directly. It also checks the start of the prompt (V4) or that there is exactly one <｜System｜> (V4.1), and that the caller's message dict is not mutated. The existing tests for the no-system-message path pass unchanged.

Run locally on a CPU-only machine:

$ python -m pytest -q tests/unittest/llmapi/test_deepseek_v4_tokenizer.py tests/unittest/llmapi/test_deepseek_v41_tokenizer.py -p no:cacheprovider --durations-min=10
48 passed, 42 skipped, 1 warning in 0.38s          # this branch
2 failed, 46 passed, 42 skipped, 1 warning in 0.56s # same tests against main's tokenizer.py files
FAILED tests/unittest/llmapi/test_deepseek_v4_tokenizer.py::test_deepseek_v4_chat_template_attaches_request_tools_to_leading_system_message
FAILED tests/unittest/llmapi/test_deepseek_v41_tokenizer.py::test_tools_argument_is_attached_to_an_existing_leading_system_message

The 42 skips are the checkpoint differential / vendor golden-fixture tests, which need a local DeepSeek checkpoint. pre-commit run --files on the four changed files passes.

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]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: NVIDIA/TensorRT-LLM/.coderabbit.yaml
• Review profile: CHILL
• Plan: Enterprise
• Run ID: 505a066c-5d4f-41ea-ac98-fbc8094342f8

📥 Commits

Reviewing files that changed from the base of the PR and between 2d12ed6 and 6d5e696.

📒 Files selected for processing: 4

• tensorrt_llm/tokenizer/deepseek_v4/tokenizer.py
• tensorrt_llm/tokenizer/deepseek_v41/tokenizer.py
• tests/unittest/llmapi/test_deepseek_v41_tokenizer.py
• tests/unittest/llmapi/test_deepseek_v4_tokenizer.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

Both DeepSeek-V4 tokenizer variants now attach request-level tools to an existing leading system message. If no such message exists, they retain the insertion behavior. Tests cover rendered output and input immutability.

Changes

Request tool placement

Layer / File(s)	Summary
Merge request tools into the leading system message tensorrt_llm/tokenizer/deepseek_v4/tokenizer.py, tensorrt_llm/tokenizer/deepseek_v41/tokenizer.py, tests/unittest/llmapi/test_deepseek_v4_tokenizer.py, tests/unittest/llmapi/test_deepseek_v41_tokenizer.py	When a conversation starts with a system message, both tokenizer variants add request-level tools to that message. Otherwise, they insert a system message as before. Tests verify rendered output and that the original message is not mutated.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: qijune, ziyixiong-nv

Merge Risk: ⚪ Minimal · up to 6d5e6

No actionable issue is established for the tool-placement change; it is mergeable after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly identifies issue #17021, the fix type, and the main change: attaching request tools to the leading system message for DeepSeek V4 and V4.1.
Description check		The description explains the issue, implementation, behavior changes, scope, regression tests, test results, and checklist status. It provides sufficient context for review.
Linked Issues check		The diff implements the active requirements in #17021 for both DeepseekV4Tokenizer and DeepseekV41Tokenizer. When messages[0] has role system, each tokenizer copies that message and replaces i…
Out of Scope Changes check		The diff changes only the two requested tokenizer paths and adds their regression tests. It does not modify the separate deepseek_v32 tokenizer or unrelated serving behavior. The test changes direct…

Full details: Linked Issues check

Explanation

The diff implements the active requirements in #17021 for both DeepseekV4Tokenizer and DeepseekV41Tokenizer. When messages[0] has role system, each tokenizer copies that message and replaces its tools value. Otherwise, each tokenizer preserves synthetic system-message insertion. The added tests verify rendered equivalence, one V4.1 system block, and no mutation of the caller message. The existing no-system behavior remains in the insertion branch.

Full details: Out of Scope Changes check

Explanation

The diff changes only the two requested tokenizer paths and adds their regression tests. It does not modify the separate deepseek_v32 tokenizer or unrelated serving behavior. The test changes directly verify the requested tools and leading-system-message behavior.

• Fix all pre-merge checks with AI

✨ 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.}

[zw-g]

Closing: #21051 (merged) fixes #17021. It attaches the request tools to the leading system message for DeepSeek-V3.2, V4 and V4.1, and also renders V4.1 developer messages as system messages. This PR's two regression tests pass on current main.