Repository navigation
[None][fix] DeepSeek DSML tool parsers: keep string parameter values as written - #21025
Conversation
…as written The DeepSeek V3.2, V4 and V4.1 encoders render a string parameter value verbatim between its tags, and tell the model to write string values as is. The parser stripped the value, so an edit's old_string lost its indentation and a written file lost its last newline. A string value is now taken as written. JSON values are parsed as before. 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 |
|---|---|
Preserve string parameter whitespace tensorrt_llm/serve/tool_parser/deepseekv32_parser.py, tests/unittest/llmapi/apps/test_tool_parsers.py |
String parameters are stored without trimming. Tests check that V3.2, V4, and V4.1 preserve indentation, embedded newlines, and a trailing newline in one-shot and streaming parses. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Possibly related PRs
- NVIDIA/TensorRT-LLM#20873: Both changes define how the DeepSeek DSML parser handles whitespace, distinguishing string content from structural layout.
Merge Risk: 
09c04
-
The change preserves DeepSeek string argument whitespace, with regression coverage across the three affected parsers. No actionable merge risk is established.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly identifies the DeepSeek DSML parser fix and states the key behavior change: preserving string parameter values as written. Description check The description explains the issue, solution, affected parser versions, regression tests, broader validation, and observed impact. It includes the required Description and Test Coverage sections. The … Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. 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 explains the issue, solution, affected parser versions, regression tests, broader validation, and observed impact. It includes the required Description and Test Coverage sections. The checklist is only partially completed, but the missing checks are non-critical for this assessment.
-
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 |
|
PR_Github #80634 [ run ] triggered by Bot. Commit: |
|
PR_Github #80634 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #80688 [ run ] triggered by Bot. Commit: |
|
PR_Github #80688 [ 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
The DeepSeek V3.2, V4 and V4.1 encoders render a string parameter value verbatim between its tags (
<｜DSML｜parameter name="..." string="true">value</｜DSML｜parameter>). The tool instructions they render tell the model to write string values as is. The DSML parser the three share called.strip()on every string value, so:old_stringandnew_stringlost their leading indentation;The client then ran a call other than the one the model wrote. The history it sent back also rendered differently from what the model had generated.
A string value is now taken as written. JSON values (
string="false") are parsed as before;json.loadsalready ignores the whitespace around them.Test Coverage
tests/unittest/llmapi/apps/test_tool_parsers.py(CPU, already inl0_cpu.yml) gainstest_dsml_string_parameter_values_are_kept_as_written.A property-based check backs the example.
tensorrt_llm/tokenizer/deepseek_v41/tokenizer.py). The arguments were string values with whitespace at either end, and JSON values.End to end, DeepSeek-V4.1-Flash served Claude Code coding sessions over
/v1/messages. What the model wrote (--post_processor_hook) was compared with what was delivered.Before, in 8 sessions, all 72 string arguments the model wrote with whitespace at an edge were delivered stripped:
Writecontents lost their final newline;Editcalls lost the indentation ofold_stringandnew_string;Bashcommands lost a trailing space.The other 1,545 string arguments came through verbatim.
After, in 5 sessions, all 1,000 string arguments were delivered as written, 63 of them with edge whitespace.
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
DeepSeekV32Parser._parse_parameters_from_xmlnow returnsstring="true"values without trimming their contents. Non-string parameter parsing still strips whitespace before JSON decoding and fallback. The scope is limited to shared DSML string-value handling. No current review findings or severity counts were supplied.QA Engineer Review
Modified
tests/unittest/llmapi/apps/test_tool_parsers.py. The newtest_dsml_string_parameter_values_are_kept_as_writtentest checks DeepSeek V3.2, V4, and V4.1 in one-shot and streaming modes. It verifies indentation, embedded newlines, and a trailing newline in string values. The test file is listed intests/integration/test_lists/test-db/l0_cpu.yml. No manual-QA list entry was found in the supplied search results. Coverage verdict: sufficient for the stated whitespace-preservation behavior. Test execution results were not independently verified.Per-File QA Perspective
tensorrt_llm/serve/tool_parser/deepseekv32_parser.py— Verify that DSML string values retain leading and trailing whitespace, while non-string values and direct JSON parsing retain their existing behavior.tests/unittest/llmapi/apps/test_tool_parsers.py— Adds one parameterized test across three parser formats and streaming/non-streaming modes. The test file is listed in the CI test database attests/integration/test_lists/test-db/l0_cpu.yml.