Repository navigation
fix(server): keep the visible answer of a thinking model on the MLX chat path - #736
Conversation
…hat path mlx_lm returns a thinking model's reasoning in its own reasoning field and the answer in content, without the closing think tag. The chat sanitizer started that content inside the prompt-seeded thinking block, never saw the close, and returned empty content; with include_reasoning the answer was appended to reasoning_content instead. A choice now starts outside the thinking block when the child returned reasoning in its own field and content does not carry it: a stream delta whose reasoning arrives before any content, or a whole message whose content never closes the block. Content that still carries a raw block is stripped as before.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: preserving visible answers from thinking models on the MLX chat path.
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…es or the choice stops When the prompt seeds an open thinking block and the child returns reasoning in its own field, the content of that choice is held instead of being sent through a stripper that starts outside the block. A close marker in the held content means the text before it is reasoning, which stays hidden as before. A choice that finishes with stop or tool_calls without one releases the held content as the answer. Any other finish, such as length, keeps it hidden. This makes a buffered message and the same content streamed agree, and keeps reasoning private when a child leaves the end of the block in content after a reasoning-only delta, or is cut off before it.
|
Review findings on The rule changed. When the prompt seeds The table runs each review input through
Each finding:
Risks: the literal Validation on |
1 participant
Summary
On Apple Silicon,
mlx_lm.server(0.31.3 and 0.32.0 checked) returns a thinking model's reasoning in its ownreasoningfield. The answer comes back incontent, without the</think>marker.Qwen/Qwen3.5-4B:thinkingseeds<think>in the prompt, so_sanitize_chat_payloadstarted itsThinkingBlockStripperinside the thinking block. It never saw the closing marker, so it returnedcontent: "". Withinclude_reasoning: true, the answer was appended toreasoning_contentinstead.Fix. The prompt can seed an open thinking block (
initial_inside_thinking). When it does, and a choice's first chunk that carries content or reasoning also has a non-emptyreasoningorreasoning_contentfield, SIE holds that choice's content instead of stripping it at once. Then:</think>, the text before it is reasoning. It stays hidden, as before this change, and the rest is stripped as usual.stoportool_callsand the content never closed the block, the held content is the answer. It is released and stripped as content that starts outside a block.length, keeps the held content hidden, as before this change. At that point SIE cannot tell a cut-off answer from cut-off reasoning.The same rule runs for a whole message and for a stream, so both return the same content for the same output. A child that never sends a reasoning field is handled exactly as before.
Where it applies.
initial_inside_thinkingis true on two paths:SGLang 0.5.10.post1 fills
reasoning_contentonly when it runs with a reasoning parser (if self.reasoning_parser and request.separate_reasoning:inserving_chat.py). On the second path the child therefore never sends a reasoning field, and its output is unchanged. The CUDA path with a reasoning parser and remote chat start outside the block and are not affected.Cost. In a stream, the answer of such a response is no longer sent token by token. It arrives in the chunk that carries
finish_reason, because until then SIE cannot tell it from reasoning left in content. Reasoning still streams as it arrives wheninclude_reasoningis true. Tool calls are not held.include_reasoning. The contract from #728 is unchanged. Reasoning is returned only when the request asks for it.Known limits, all unchanged from before this change:
reasoningfield. That response looks like a raw-tag response whose block never closes, so its content stays hidden. Withinclude_reasoning: true, the answer is returned asreasoning_content.<think>, so their first such chunk is reasoning.</think>next to split reasoning is cut to the text after the marker.Tests
Two table tests drive the real
/v1/chat/completionsroute and the real sanitizer through a mock child transport. Each case runs with and withoutinclude_reasoning. It checkscontent,reasoning_content, and that the hidden text is absent from the response when reasoning was not requested.test_mlx_thinking_chat_keeps_the_answer_beside_split_reasoning(whole message):contentreasoningreasoningreasoning_contentreasoning_contentraw_tagsreasoning</think>answersplit_and_raw_closereasoning, and content that still closes the blocksplit_then_raw_openreasoning, and contentready<think>one wordreadysplit_cut_offreasoning, and contentstill thinking""no_reasoning_field""test_mlx_thinking_chat_stream_keeps_the_answer_beside_split_reasoning(stream):contentreasoningreasoningdeltas, then content deltasreasoning_content_after_rolereasoning_contentdeltas, then contentraw_tagssplit_then_raw_close...</think>answersplit_then_raw_close_across_deltas</think>split across two deltassplit_and_answer_in_one_deltareasoningand the answeranswer_before_tool_callreasoning, content, then a tool callsplit_cut_offreasoning, then content""no_reasoning_field""Mutation checks, each run against the whole
test_openai_local.pymodule (178 tests). The counts are failing test ids, and each case runs twice.75c83cf9)split_cut_off(both tests),split_then_raw_close,split_then_raw_close_across_deltas,split_and_answer_in_one_deltasplit_cut_off(both tests)stopanswer_before_tool_call</think>in the current delta onlysplit_then_raw_close_across_deltasandtest_cuda_chat_stream_sanitizes_split_reasoning_and_closes_upstreamsplit_then_raw_openreasoning_content_after_roleValidation:
mise run lintis clean.mise run typecheckis clean.mise run test -- packages/sie_server/tests/api: 1249 passed, 4 skipped.tools/ci/check_public_tree.pypassed.Live turn on Apple Silicon
Setup:
mlx-lm0.32.0.mise run serve -- -b sglangservingQwen/Qwen3.5-4B:thinking(mlx-community/Qwen3.5-4B-4bit).SIEClient.chat_completionsandSIEClient.stream_chat_completions.Before (base
0e3bdead):include_reasoningcontentreasoning_content""""""...eady\n\n\nready""...eady\n\n\nreadyAfter (head
f34ff02d):include_reasoningcontentreasoning_content"\n\nready""\n\nready""\n\nready"...se:** ready\n"\n\nready"...se:** ready\nIn both streams, the answer arrived in the last of 140 choice chunks, the one that carries
finish_reason: stop. No earlier chunk carried content.Summary by CodeRabbit