serve: stream GLM reasoning through reasoning_content on the OpenAI endpoint (#597 item 4) - #607
Merged
Merged
Conversation
…ndpoint (#597 item 4) On /v1/chat/completions, GLM's reasoning was not separated from the answer: with tools it was silently dropped (parse_tool_calls strips everything before </think>), without tools the raw reasoning AND the literal </think> were streamed as ordinary `content`. Only the Anthropic endpoint and Inkling split it. (My earlier note on #597 that non-streaming was already handled was wrong — it was handled only for Anthropic/Inkling, not the OpenAI GLM path.) Route GLM reasoning to `reasoning_content` on both the streaming and non-streaming OpenAI chat paths, reusing the existing cross-chunk ThinkingStreamSplit: - streaming, no tools: reasoning -> reasoning_content deltas, answer -> content deltas - streaming, tools: a think splitter runs BEFORE the tool-call buffer, so reasoning goes to reasoning_content and never contaminates parse_tool_calls' input - non-streaming: split_thinking_reply populates message.reasoning_content and a clean message.content - <think>/</think> markers are never exposed to the client ThinkingStreamSplit gains initial_thinking (start in answer mode when thinking is off, so a marker-free answer isn't filed as reasoning) and a close() alias for the streaming path; enable_thinking is plumbed from chat_completion into generation(). Tests: 5 unit cases on the splitter (single-chunk, close-tag split across chunks, stray open marker, thinking-off all-answer, missing close tag) + 5 end-to-end HTTP cases (streaming split, cross-chunk </think>, thinking-off, reasoning-out-of-tool-args, non-streaming split). Full server suite 78/78. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 25, 2026
NeuralNotwerk
pushed a commit
to NeuralNotwerk/colibri
that referenced
this pull request
Jul 25, 2026
…en 200 stream (JustVugg#597 item 6) A streaming chat request committed its HTTP 200 (SSE headers) before the engine tokenised the prompt and checked it against CTX. An over-long prompt was only then found -- ERROR CONTEXT_EXCEEDED -- far too late to send the 400 the client needs, so the client saw a broken/empty stream instead of a clean context_length_exceeded. (Non-streaming was already correct: it sends its 200 only after generation.) Add an acceptance frame to the engine<->gateway protocol (mkelcb's design in JustVugg#597): - c/colibri.c mux_submit(): after all cheap validation (encode, non-empty, fits CTX) and BEFORE prefill, emit `ACCEPT <id> <ntok>`. Every early failure already returns first with an ERROR, so a request yields exactly one of ACCEPT or an early ERROR. - Engine._dispatch_stdout(): parse ACCEPT -> ("accept", {...}) without popping pending. - Engine.generate(): new on_accept callback; commit exactly once on the first of ACCEPT / DATA / DONE. DATA/DONE before an ACCEPT still commit (an older engine with no ACCEPT frame keeps working); a second ACCEPT is a protocol error. - APIHandler.generation() streaming: defer the SSE 200 + keepalive into start_stream(), fired via on_accept. An ERROR that arrives before ACCEPT now propagates as an APIError with nothing committed, so do_POST returns a normal JSON 400. Tests: 4 Engine-level cases over a fake engine process (ACCEPT-before-DATA fires on_accept first; ERROR-before-ACCEPT raises 400 and never commits; DATA-before-ACCEPT still commits for an old engine; duplicate ACCEPT is a protocol error) + 1 end-to-end HTTP case (streaming context-exceeded returns a clean 400 with context_length_exceeded, not a 200 stream). Full server suite 83/83; the C engine builds clean. Completes the JustVugg#597 series (items 1/2/4/5 already on dev via JustVugg#535/JustVugg#606/JustVugg#607). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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 #597 item 4 for the OpenAI
/v1/chat/completionsendpoint.The bug (worse than my earlier triage said)
On the OpenAI endpoint, GLM's reasoning was never separated from the answer:
parse_tool_callsstrips everything before</think>);</think>were streamed as ordinarycontent.Only the Anthropic endpoint (
split_thinking_reply) and Inkling (InklingStreamSplit) handled it. My earlier comment on #597 that non-streaming was "already handled" was wrong — it was handled only for Anthropic/Inkling, not the OpenAI GLM path. This PR corrects that.The fix
Route GLM reasoning to
reasoning_contenton both OpenAI chat paths, reusing the existing cross-chunkThinkingStreamSplit:reasoning_contentdeltas, answer →contentdeltas.reasoning_contentand never reachesparse_tool_calls' input (@mkelcb's exact warning).split_thinking_replyfillsmessage.reasoning_contentand a cleanmessage.content.<think>/</think>markers are never exposed.ThinkingStreamSplitgainsinitial_thinking(start in answer mode when thinking is off, so a marker-free answer isn't misfiled as reasoning) and aclose()alias;enable_thinkingis plumbed fromchat_completionintogeneration().Tests (the point of this PR)
5 unit cases on the splitter — single-chunk, close-tag split across chunks (
abc</thi+nk>def), stray open marker, thinking-off is all-answer, missing close tag surfaces reasoning.5 end-to-end HTTP cases — streaming split, cross-chunk
</think>, thinking-off all-content, reasoning-stays-out-of-tool-args and out of visible answer, non-streaming split.Full server suite 78/78.
Part of the #597 series (items 2 + 5 landed in #606). Remaining: item 3 (retest), item 6 (ACCEPT-frame context ordering — native protocol, separate PR).