Skip to content

fix: parse reasoning when the chat template pre-opens <think> (#108) - #115

Merged
solderzzc merged 3 commits into
mainfrom
claude/issue-108-thinking-preopened
Aug 6, 2026
Merged

fix: parse reasoning when the chat template pre-opens <think> (#108)#115
solderzzc merged 3 commits into
mainfrom
claude/issue-108-thinking-preopened

Conversation

@solderzzc

@solderzzc solderzzc commented Aug 5, 2026

Copy link
Copy Markdown
Member

Fixes #108.

Root cause

Qwen3/3.5/3.6 chat templates append <think>\n to the generation prompt when enable_thinking is true. From the cached Qwen3.6-35B-A3B tokenizer_config.json:

{%- if add_generation_prompt %}
    {{- '<|im_start|>assistant\n' }}
    {%- if enable_thinking is defined and enable_thinking is false %}
        {{- '<think>\n\n</think>\n\n' }}
    {%- else %}
        {{- '<think>\n' }}          {# prompt ends inside the block #}
    {%- endif %}

The model therefore emits reasoning with no opening tag of its own, terminated by </think>. But ThinkingStateTracker always started in .responding (Server.swift:1695) and extractThinkingBlock required both tags (Server.swift:2137). So the whole reasoning section was misfiled as content, with a stray </think> inside it.

That is exactly the reported symptom — "the client doesn't get the beginning <think> tag" — and it explains the follow-on behaviour with tool calls: the replayed assistant turn starts mid-thought, so the model learns to omit its own first line.

The reporter noting that vllm-mlx behaves identically fits: it's the template, not MLX.

Changes

  • Detect the pre-opened block at runtime — decode the rendered prompt tail (32 tokens) and compare the last opening tag against the last closing tag. This is deliberately not keyed to a model family: the same Qwen line differs by version (cached Qwen3-1.7B does not pre-open, Qwen3.6 does), so hard-coding would break on the next release.
  • ThinkingStateTracker(startInThinking:) — starts the parser inside the block; extractThinkingBlock(from:alreadyOpen:) does the same for the non-streaming path.
  • Drain the tracker at end of stream — a second, independent defect: a response whose final characters look like the start of a tag stayed in the buffer and was dropped. The stream loop had no flush at finish.
  • Tag spellings are now shared constants, and matching picks the earliest match instead of preferring <thinking> found later in the buffer.

Tests

New tests/SwiftLMTests/ThinkingPreOpenedTests.swift — 18 tests over prompt detection (both template branches, prior closed blocks in history, Gemma channel markers), streaming with a pre-opened block (including a closing tag split across chunks), the end-of-stream flush, and non-streaming extraction. testExtractPreOpenedBlockWithoutFlagIsUnchanged pins the old broken output so the difference is explicit.

Verified red: with startInThinking ignored and flush() stubbed to return empty, 9 of the 18 fail.

End-to-end, against mlx-community/Qwen3.5-4B-4bit — a real, unmodified model whose chat_template.jinja carries the same pre-opening form as Qwen3.6:

{%- if enable_thinking is defined and enable_thinking is false %}
    {{- '<think>\n\n</think>\n\n' }}
{%- else %}
    {{- '<think>\n' }}
{%- endif %}
case result
--thinking, non-streaming detection fires; reasoning → reasoning_content (461 chars), content = "Tokyo", no leaked tags
--thinking, streaming 148 chunks; reasoning → reasoning_content deltas, content = "\n\n54", no leaked tags
no --thinking (control) template emits an already-closed block, detection stays off, reasoning_content = null, content = "Tokyo"

Also confirmed on mlx-community/Qwen3.5-0.8B-MLX-4bit that detection fires and reasoning is routed correctly, though that model loops without ever emitting </think>, so it never reaches the content phase.

Full suite: 114 tests across 9 suites, 0 failures. PromptCacheTests aborts under swift test with Failed to load the default metallib — pre-existing at HEAD and environmental, so suites were run individually.

Note on scope

Streaming does not trim the newline that follows </think>, so content begins with \n\n (visible in the table above). That is pre-existing behaviour shared with the self-opened path — the non-streaming path does trim — and this PR leaves it alone rather than widening scope. Worth a follow-up if the inconsistency matters.

🤖 Generated with Claude Code

Qwen3/3.5/3.6 chat templates append `<think>\n` to the generation prompt
when enable_thinking is true:

    {%- if add_generation_prompt %}
        {{- '<|im_start|>assistant\n' }}
        {%- if enable_thinking is defined and enable_thinking is false %}
            {{- '<think>\n\n</think>\n\n' }}
        {%- else %}
            {{- '<think>\n' }}          <-- prompt ends inside the block

So the model emits reasoning with no opening tag of its own, terminated by
`</think>`. ThinkingStateTracker always started in .responding and
extractThinkingBlock required both tags, so the entire reasoning section was
misfiled as response content with a stray `</think>` inside it. The client
never received an opening tag, and replaying that assistant turn taught the
model to omit its own first line — the truncation in the report.

- Decode the rendered prompt tail and detect an unclosed opening tag, then
  start the parser inside the thinking block. Detection is at runtime rather
  than keyed to a model family: the same Qwen line differs by version
  (Qwen3-1.7B does not pre-open, Qwen3.6 does).
- extractThinkingBlock gains `alreadyOpen` for the non-streaming path.
- Drain the tracker at end of stream. A response whose final characters look
  like the start of a tag was held in the buffer and dropped entirely.
- Tag lists are now shared constants, and tag matching picks the earliest
  match rather than preferring `<thinking>` found later in the buffer.

Verified end-to-end against a local model with a Qwen3.6-style template:
reasoning lands in reasoning_content, content is clean, no leaked tags, in
both streaming and non-streaming mode. A model whose template does not
pre-open is unaffected — detection stays off and output is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@solderzzc

Copy link
Copy Markdown
Member Author

Update: the tool-call flow from the report is now reproduced and verified, not just inferred.

Same model, same flags, two release binaries differing only in Server.swift. A two-round exchange mirroring a real client: round 1 asks a question with a tool available, round 2 replays the assistant turn plus the tool result.

Before (main):

[round1] reasoning_content: None
[round1] content: 'The user is asking for the weather in Paris and wants me to use the tool. I have access to…'
[round1] tool_calls: ['get_weather({"city":"Paris"})']
[round1] LEAKED TAGS IN CONTENT: ['</think>']
[round2] content: 'The tool returned the weather information for Paris. The temperature is 18°C and…'
[round2] LEAKED TAGS IN CONTENT: ['</think>']

After (this branch):

[round1] reasoning_content: 226 chars
[round1] content: None
[round1] tool_calls: ['get_weather({"city":"Paris"})']
[round1] LEAKED TAGS IN CONTENT: none
[round2] reasoning_content: 121 chars
[round2] content: 'The weather in Paris is currently 18°C with light rain.'
[round2] LEAKED TAGS IN CONTENT: none

This is exactly @coolaj86's description: the client receives the model's private reasoning as content with a stray </think> in it, and that malformed turn is what gets replayed into history on the next round. Note the tool call itself succeeds in both — matching the report that tool calls "work" while the surrounding text is wrong.

On the reporter's exact model. I downloaded mlx-community/Qwen3.6-27B-OptiQ-4bit to test against it directly, and could not: it emits garbage on current main regardless of the thinking flag.

--thinking:                    " (' tml_htmlnsub\x00\n\n#\n\n# &г.� c#厉 code"
enable_thinking=false:         "},-idthlaf�(aren系_1!"

The same binary produces coherent output on Qwen3.5-4B-4bit, so this is specific to that model rather than the build. Its config.json carries 498 per-layer quantization overrides (optiq_mixed_precision, 4.77 achieved bpw, embed_tokens at 8-bit over a 4-bit default) and the architecture uses linear_attn blocks. mlx-swift-lm does implement per-layer quantization with a language_model. prefix fallback, so the cause needs its own investigation — I have not identified it, and it is unrelated to this PR. Filing separately unless someone already knows the answer.

So the verification above stands on Qwen3.5-4B-4bit, whose chat_template.jinja pre-opens <think> in exactly the same form as the reporter's Qwen3.6 model.

solderzzc and others added 2 commits August 6, 2026 13:43
- Detection now requires the opening tag to be the last thing in the prompt.
  Scanning the whole tail meant any `<think>` in the conversation itself fired
  it — a user asking "explain the <think> tag" had their entire answer routed
  into reasoning_content with content left empty. Templates that pre-open
  always end `…assistant\n<think>\n`, so the constraint costs nothing.
- extractThinkingBlock with alreadyOpen now only honours an opening tag at
  index 0. A tag the model mentions inside its reasoning previously sent
  control down the both-tags path, which discarded everything before it —
  text that appeared in neither reasoning_content nor content. This also makes
  the non-streaming path agree with the streaming tracker, which stays in the
  thinking phase until a closing tag.
- The stop-sequence exit path now emits what the tracker actually produced for
  the current chunk and drains it, instead of re-processing a slice of
  fullText. The old arithmetic assumed everything before the chunk had been
  emitted, which is false while the tracker holds a partial tag, so that text
  was dropped — the same truncation this PR set out to fix, on a path it did
  not cover.

Three tests added: a tag mentioned in the prompt is not a pre-open, trailing
whitespace still is, and leading reasoning survives a tag mentioned mid-block.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review follow-up on 849745b. That commit fixed the dropped-held-text bug on
the stop path but introduced a worse one: it fed the whole chunk to the
tracker and emitted the result verbatim, so the client received the stop
string itself and anything after it — with a multi-token stop like "\nUser:"
that leaks an entire hallucinated turn. The three sibling stop-handling sites
in this file all trim; this one silently stopped.

The stop check now runs before the tracker is fed, and only the slice of the
chunk that survives the trim enters the state machine. This keeps both
properties: held partial-tag text is still drained via flush(), and the stop
sequence never reaches the client. The unused `trimmedFull` binding the
review flagged is gone because the value is used again.

Also made lastRange's tie rule explicit (longest tag wins at the same start
position) instead of depending on openTags array order; a reorder would have
silently broken <thinking>-style pre-open detection.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@solderzzc
solderzzc merged commit 42bde9a into main Aug 6, 2026
@solderzzc
solderzzc deleted the claude/issue-108-thinking-preopened branch August 6, 2026 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Beginning of output truncated when --thinking

1 participant