[Fix] Route text before the opening think token by position, not by index 0 - #35085
Open
tancheng33 wants to merge 1 commit into
Open
tancheng33 wants to merge 1 commit into
tancheng33 wants to merge 1 commit into
Conversation
…ndex 0 BaseReasoningFormatDetector enters a reasoning block on `think_start_token in text` (anywhere) but strips the marker with `startswith` (index 0 only), so a single leading space or newline leaves the literal marker inside reasoning_content. The streaming path has the mirror-image gap: it recognises a partial opening marker only when the whole buffer is a prefix of it, so a chunk carrying content plus "<" clears the buffer and the rest of the marker arrives with nothing to attach to -- the whole reasoning block then reaches the client as raw content. Strip the marker wherever it first appears and split off what preceded it: content, or reasoning under force_reasoning where the chat template already opened the block. Hold back a trailing partial opening marker in the not-in-reasoning branch, the same way the reasoning branch holds back the closing one. Closes the exception pinned by sgl-project#34458 as test_text_before_think_token_is_ chunk_dependent; that test becomes an invariance test.
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
Fixes #35083.
BaseReasoningFormatDetectorenters a reasoning block onthink_start_token in text(anywhere) but strips the marker withstartswith(index 0 only).Anything in front of the opening marker — a single space or newline is enough —
falls into the gap, and the streaming path has the mirror-image gap: it only
recognises a partial opening marker when the whole buffer is a prefix of it.
Same string, four answers on
main(--reasoning-parser qwen3,"Sure.<think>checking</think>Done."):reasoning_contentcontent'Sure.<think>checking''Done.''checking''Sure.Done.'<think>'''Sure.<think>checking</think>Done.''Sure.checking''Done.'Row 3 streams the whole reasoning block, markers included, to the client as
assistant content. It needs a chunk carrying content plus a strict prefix of the
marker, which a multi-token decode step produces:
--stream-interval > 1,sampling_params.stream_interval, speculative decoding, or the detokenizerreleasing several tokens after an incomplete UTF-8 sequence. Rows 1, 2 and 4 need
no such condition.
#34458 already named this case and pinned it:
This PR closes that exception instead of pinning it.
Modifications
python/sglang/srt/parser/reasoning_parser.py_split_at_think_start()(new): removes the opening marker wherever it firstappears and returns the text left of it separately. Under
force_reasoningtheblock was already opened by the chat template, so the lead stays reasoning and
only the echoed marker is dropped; otherwise the lead is content. Both match
what the streaming path already did when the marker landed whole at a chunk
start.
_detect_and_parse_impl(): uses it, and prepends the lead tonormal_textonevery return path (truncated reasoning,
tool_start_tokeninterruption, the</think>split, and thecontinue_final_messagebranch)._parse_streaming_increment_impl(): the strip splits the lead out instead ofreplace(..., 1)swallowing it; the not-in-reasoning branch now holds back atrailing partial opening marker with
_ends_with_partial_token, the same waythe in-reasoning branch holds back the closing one. The holdback is skipped
once
stripped_think_startis set, so content after the block is not delayed,and
finish()already flushes a held-back slice that never became a marker.Detectors that override both methods (
gpt-oss,kimi_k3,muse,inkling,apertus2509,cohere_command4) are untouched.Note: #34600 also edits
_parse_streaming_increment_impl(the same two regions).Whichever lands first, I am happy to rebase this one on top of it.
Accuracy Tests
A differential harness over all 26
ReasoningParser.DetectorMapentries(12 samples x {chunk sizes 1,2,3,7,13, six random splits, whole} x
stream_reasoningon/off, comparing streamed +finish()againstdetect_and_parse): 21 model types diverged on this shape before, 0 after.The two that still diverge (
gpt-oss,kimi_k3) are separate defects in theirown overrides, reported separately and out of scope here.
Tests, in
test/registered/unit/parser/test_reasoning_parser.py:test_text_before_think_token_is_content— replacestest_text_before_think_token_is_chunk_dependent; the four splits collapse to("r", "leadtail")across all chunk sizes and one-shot.test_single_leading_newline_before_think_token— the common shape; guards theone-shot marker leak.
test_forced_reasoning_keeps_lead_text_as_reasoning— the opposite routingunder
force_reasoning, so a future "simplification" cannot collapse the two.test_content_ending_in_start_token_prefix_survives— the new holdback must notswallow content that merely ends in a marker prefix and never completes.
Verified fail-before / pass-after: on the pre-fix source the first three fail
(16 failed / 1 passed with the four selected); with the fix
test/registered/unit/parser/is344 passed, 165 subtests, andtest/registered/unit/function_call/ + parser/is808 passed.test/registered/unit/entrypoints/openaihas 8 pre-existing failures intest_serving_transcription.py— identical with and without this change.Benchmarking and Profiling
Not applicable — string handling on the parse path only. The new holdback defers
at most
len(think_start_token) - 1characters of content, and only before thefirst reasoning block.
Checklist
CI States
Latest PR Test (Base): ❌ Run #31992374032
Latest PR Test (Extra): ❌ Run #31992373703