Repository navigation
[Bugfix] Implicitly close DeepSeek DSML parameters - #54838
Conversation
Signed-off-by: sfeng33 <4florafeng@gmail.com>
dc3ff2d to
99059db
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Manny7717
left a comment
Manny7717
left a comment
There was a problem hiding this comment.
Verified locally on head 99059db (worktree, VLLM_TARGET_DEVICE=cpu, mock-tokenizer parser tests).
Bug is real on base (merge-base cdefd9d): _PARAM_RE in vllm/parser/deepseek_v4.py only terminated a parameter at </｜DSML｜parameter>, so a model output that omits the closer (issue #53227 class) swallowed the NEXT parameter's opening tag into the previous value. Executed probe on base: <param name="location">Paris <param name="date">tomorrow</param> parses as {"location": "Paris <｜DSML｜parameter name=\"date\" string=\"true\">tomorrow"} — date never materializes.
Fix is correct: _PARAM_RE now terminates at either </parameter> or a lookahead for the next <parameter name= (implicit close); _dsml_arg_converter handles the split; DSML_PARAM_START is wired into both deepseek_v4 and deepseek_v32 configs (v3.2 imports the shared converter, so the shared fix applies there too — covered by the new v32 streaming test).
Regression-proven (executed): all 5 new tests FAIL on the base worktree with the exact bug class above (incl. streaming split-tag case leaking <｜DSML｜parameter into arguments) and PASS on head. Full tests/parser/engine 3806 passed; tests/parser + tests/tool_parsers/test_deepseekv4_tool_parser.py 4078 passed / 285 teardown errors — the errors are torch.accelerator CUDA-less fixture noise, byte-identical on base (verified same-file 20e/20p on both).
Lint: ruff pinned in CI (v0.14.0) passes clean on all 4 files; pre-commit CI green.
Edge behavior noted (fine): a literal <parameter name= sequence inside a value would now split — that is the DSML grammar itself, and the malformed-text test confirms non-tag text like </｜DSML｜parameter\n is still preserved literally. No test plan gaps found.
|
/ci run |
|
✅ Triggered Buildkite CI #86755 for commit |
…ross delegation' Superseded by the upstream parser fixes for DSML parameter streaming and implicit close (vllm-project#52865, vllm-project#54838, vllm-project#54686 subset): with those in place the recovered tool call surfaces through the normal delegation path, so the experimental whitespace/tool-call forwarding is unnecessary and regressed kimi_k2 and deepseek_v4 replay whitespace tests. This reverts commit a59cb3a. Signed-off-by: Jeff Solinsky <logicalfinality@proton.me>
Signed-off-by: sfeng33 <4florafeng@gmail.com>
4 participants
Purpose
Partially fix #53227.
Allow a DeepSeek DSML parameter to end implicitly when the next parameter begins. This prevents a missing parameter closer from swallowing the following parameter. The shared converter also applies this behavior to DeepSeek V3.2.
Test Plan
Unit tests
Live server validation
Validated against a live
deepseek-ai/DeepSeek-V4-Flashserver.End-to-end: 8 prompt shapes (parallel calls, 3-param calls, values containing
</XML markup, multi-line values, unicode, numeric/bool params, long values) × {streaming, non-streaming} × {tool_choice: auto,required} — 32 request pairs. No DSML markup in arguments, all arguments valid JSON, all keys within the tool schema.Chunk-boundary correctness: the server only exercises one token split, so 64 raw generations were captured unparsed (via
/tokenize→/v1/completions) and replayed through the parser at splits of 1/2/3/7/17 chars plus randomized splits — 704 replays per corpus — asserting chunk-size invariance and no terminal leakage.</｜DSML｜parameter>dropped, 704 replays