Preserve streaming preamble before DeepSeek DSML tool calls for all marker forms - #33813
Closed
hassellof wants to merge 1 commit into
Closed
Preserve streaming preamble before DeepSeek DSML tool calls for all marker forms#33813hassellof wants to merge 1 commit into
hassellof wants to merge 1 commit into
Conversation
5 tasks
…r forms DeepSeekV32Detector.parse_streaming_increment returns normal_text="" on the tool-call branch, so assistant prose that arrives in the same streaming delta as the DSML opener is silently dropped. sgl-project#31786 fixes this for bot_token, but DeepSeek-V4 frequently opens a tool-call section with a bare "<|DSML|invoke" (no enclosing tool_calls wrapper), and with speculative decoding a single delta commonly carries both the tail of the preamble and a partial tag. Split the buffered text at the earliest DSML marker of any form (or at a trailing partial tag prefix) before entering the invoke-parsing loop, emit the prefix as normal_text, and keep only the DSML section in the buffer. Guarded to the first tool call, matching sgl-project#31786. Regression test sweeps every chunk-split point for both the wrapped and bare-invoke openers, for both DeepSeekV32Detector and DeepSeekV4Detector, and pins the invariant that total streamed normal_text equals detect_and_parse's normal_text for the same input. Measured on DeepSeek-V4-Flash-0731 (TP=4 + DP attention, speculative decoding, streaming): 4/8 responses with a forced preamble lost it before this change, 0/8 after; non-streaming control 8/8 intact in both builds.
hassellof
force-pushed
the
fix/dsv4-streaming-preamble
branch
from
August 6, 2026 04:44
aac2dbe to
8a92d57
Compare
5 tasks
5 tasks
Collaborator
|
Covered by #34458 (5899674). Preamble preservation now handles the bare You are credited as a co-author on that commit. If I misread what this PR does and part of it is still missing, please rebase on main and reopen — happy to take it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
DeepSeekV32Detector.parse_streaming_increment(inherited byDeepSeekV4Detector) returnsnormal_text=""on the tool-call branch, so any assistant prose that arrives in the same streaming delta as the DSML tool-call opener is silently dropped. The visible message ends mid-sentence, or is empty when the whole preamble lands in that delta. Non-streamingdetect_and_parsealready preserves that prefix, so the two paths disagree.This is the same defect #31786 identifies, and that PR's fix is correct — this change builds on it rather than replacing it. The difference: #31786 anchors the split on
self.bot_token(<|DSML|tool_calls>), but DeepSeek-V4 frequently opens a tool call with a bare<|DSML|invokeand no enclosing wrapper. When that happensfind(bot_token)returns -1, no split occurs, and the preamble is still lost. If it is easier to fold these hunks into #31786, that works just as well — the two are not in competition.Speculative decoding makes this common rather than rare: multi-token deltas mean "prose and opening tag in the same delta" is the normal case, not a boundary condition.
Modifications
_dsml_section_start(): returns the index where the (possibly partial) DSML section begins, searching every marker form (bot_token,<|DSML|invoke,<|DSML|,|DSML|) and treating a trailing partial tag prefix (<,<|,</,</|) as the boundary so a chunk that ends mid-tag holds back only the prefix.parse_streaming_increment(): on entering the tool-call branch, split off any preceding prose, keep only the tag portion buffered, and return the prose asnormal_text. Guarded to the first tool call (current_tool_id == -1) so text between consecutive invokes is not re-emitted.Tool-call parsing itself is unchanged.
Accuracy Tests
Measured on 4× RTX PRO 6000 Blackwell (sm_120), TP=4 + DP attention, DeepSeek-V4-Flash-0731, DSPARK speculative decoding,
tool_call_parser=deepseekv4, thinking enabled. Prompt engineered to force a preamble before the tool call, 8 paired runs, streaming vs non-streaming on the same server:Tool-call arguments remained valid JSON in every run and no DSML markup leaked into content. The serving environment those runs were measured in (image build, patch set, flags) is published at https://github.com/ombori/deepseek-v4-flash-0731-sglang-4x-rtx-pro-6000 if anyone wants to reproduce the streaming behaviour on SM120.
New unit test at
test/registered/unit/function_call/test_deepseekv32_detector.py, run against bothDeepSeekV4DetectorandDeepSeekV32Detector:<|DSML|invokein one delta;<in prose that is not a tag (must not be swallowed, with and without a following tool call);normal_textequals non-streamingdetect_and_parsenormal_text, and streamed calls equal non-streaming calls (names + parsed arguments), at every split point.The suite fails on unpatched
main(9 of 10 test methods, 1144 subtest failures) and passes patched (20/20 tests, 1146 subtests). It is hermetic — pure Python, no GPU, no model.CI States
Latest PR Test (Base): ❌ Run #31072114031
Latest PR Test (Extra): ❌ Run #31072113886