fix: split the think block in non-streaming responses - #740
Open
Javinator9889 wants to merge 2 commits into
Open
Javinator9889 wants to merge 2 commits into
Javinator9889 wants to merge 2 commits into
Conversation
Only some models separate reasoning in parse_nstream_content (gpt-oss, nanbeige); the rest leave the block inline in content. The split otherwise happens only in the streaming parser, so any buffered response -- which is what tool_choice=required and named produce -- reached the client as literal <think> tags instead of reasoning_content. Observed on qwen3.5:9b through OpenCode, which renders the tags rather than a reasoning block. Split it once in build_nstream_response so every model reports reasoning the same way. No-op when the model already filled reasoning_content. Handles the generation prompt having opened the block, where only the closing tag appears in the generated text. Measured on qwen3.5:9b: tool_choice=required goes from a literal <think> in content to reasoning_content with clean content; plain non-streaming likewise. Offline ctest passes, tool-policy suite unchanged at 7/8 (the one failure is the pre-existing local include_usage deviation), usage contract passes.
When generation hits the token limit inside the think block the closing </think> never arrives, so the split could not fire and the partial thought was delivered as content. The client then shows "Thinking Process: 1. **Analyze the Request:** The user wants me to ..." as the reply, which reads like the prompt being echoed back. Measured on qwen3.5:9b at 81k context over 30 turns: 14 turns produced a wrong answer and every one of them had finish_reason "length" with empty reasoning_content. Not one turn that finished normally was wrong. The same conversation at 97k in a single turn answers perfectly, so this is not context-length degradation -- it is truncation surfacing as content. Route the unclosed remainder to reasoning_content and leave content empty, since there is no answer yet. The streaming path already handles this: its parser stays in REASONING mode when the block never closes.
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.
Fixes #739.
build_nstream_responsetakesreasoning_contentfromparse_nstream_content,which only gemma4_12b, gemma4e, gpt_oss, lfm2, llama3, nanbeige and qwen3
populate. For qwen3_5vl, qwen3_6_moe, qwen3vl and qwen3_5_omni the think block
stays in
contentand reaches the client as raw<think>tags.Two commits:
build_nstream_response, so every model reportsreasoning the same way. No-op when the model already populated
reasoning_content. Handles the generation prompt having opened the block,where only the closing tag appears in the generated text.
finish_reasonlength— toreasoning_contentwith emptycontent, since there is noanswer yet. Previously the partial thought was delivered as the answer.
The streaming path is untouched: its parser stays in
REASONINGwhen the blocknever closes, so it already behaves correctly.
Verified on
qwen3.5:9b, same two requests as the issue:reasoning_contentabsent,content='<think>\nThinking Process:...'reasoning_contentset, no tags incontentreasoning_contentset,contentempty