Skip to content

chat : stop qwen3-coder reasoning and content before template separators - #30557

Merged
ggerganov merged 4 commits into
ggml-org:masterfrom
7konigsberg:fix-qwen3-whitespace
Oct 28, 2026
Merged

ggerganov merged 4 commits into
ggml-org:masterfrom
7konigsberg:fix-qwen3-whitespace

Conversation

@7konigsberg

@7konigsberg 7konigsberg commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Overview

Addresses the server side of #30357 :)

The Qwen3-Coder parser (also used by Qwen3.5, Nemotron Nano 3 and StepFun-3.5-Flash) now stops reasoning before the newline before </think>, and content before the one or two newlines before <tool_call>. The template adds these back on the next turn, so the parsed message re-renders to what the model generated. This uses the parser diff from aldehir's comment. No whitespace stripping, and the shared mapper is untouched.

This does not change extra whitespace right after <think> or </think> (cases B and C in the issue). The grammar consumes that through space(), so it cannot be returned, as I saw discussed in the thread.

Additional information

The session tests in test-chat check the exact reasoning and content strings, including content before a tool call fed in small chunks. They fail without the parser change.

I also checked Qwen3.5-4B with llama-server built from master and this branch, using greedy decoding and the same prompts, with and without tools, streamed and non-streamed. The text, tool calls, finish reasons and token counts match. The differences are the newline before </think> and the newlines before <tool_call>, which the template adds back.

One issue I noticed while testing: with tools in the request, if content ends with a newline and no tool call, that newline is now lost. The until rule holds it as a partial match at the end of input. I left this unresolved here because fixing it appears to need the core parser to know when it is doing the final parse, which would affect every format.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: YES. I used Claude Code to explore the parser code, write the tests and run the comparison. I reviewed and understand every line, and I own the design.

@7konigsberg
7konigsberg requested review from a team, ggerganov and pwilkin as code owners October 14, 2026 23:41
@github-actions github-actions Bot added the testing Everything test related label Oct 14, 2026
@aldehir

aldehir commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

I rather handle this in the parser itself:

diff --git a/common/parsers/qwen3-coder.cpp b/common/parsers/qwen3-coder.cpp
index abed16974..7cb9cce90 100644
--- a/common/parsers/qwen3-coder.cpp
+++ b/common/parsers/qwen3-coder.cpp
@@ -76,9 +76,10 @@ common_chat_params common_chat_params_init_qwen3_coder(const common_chat_templat
 
         auto reasoning = p.eps();
         if (supports_reasoning && extract_reasoning) {
+            // The template strips the newline before </think>, so leave it out of the reasoning
             reasoning = p.optional("<think>" + p.space() +
-                                   p.reasoning(p.until_one_of({ "</think>", "<tool_call>" })) +
-                                   (p.literal("</think>") | p.peek(p.literal("<tool_call>"))));
+                                   p.reasoning(p.until_one_of({ "\n</think>", "</think>", "<tool_call>" })) +
+                                   (p.literal("\n</think>") | p.literal("</think>") | p.peek(p.literal("<tool_call>"))));
         }
 
         // Response format parser
@@ -166,8 +167,15 @@ common_chat_params common_chat_params_init_qwen3_coder(const common_chat_templat
             auto calls      = inputs.parallel_tool_calls ? tool_call_first + p.zero_or_more(tool_call) : tool_call_first;
             auto tool_calls = p.trigger_rule("tool-call-root", p.repeat(calls, min_calls, 1));
 
+            // The templates separate content from the tool calls with one or two newlines,
+            // stop the content before them
+            std::vector<std::string> content_ends;
+            for (const auto & start : tool_call_starts) {
+                content_ends.insert(content_ends.end(), { "\n\n" + start, "\n" + start, start });
+            }
+
             return generation_prompt +
-                   (reasoning << p.content(p.until_one_of(tool_call_starts)) << tool_calls);
+                   (reasoning << p.content(p.until_one_of(content_ends)) << tool_calls);
         }
 
         // Content only parser

@pwilkin

pwilkin commented Oct 15, 2026

Copy link
Copy Markdown
Member

Your daily reminder that :\n\n is a token in Qwen and trimming it before a tool call will cause the model to slowly degrade its tool calling ability. So please, no whitespace stripping, this has been beaten to death already (the parser approach @aldehir mentioned is fine, but I'm generally against changing the parser code to account for bad / degraded model output since once it happens, further output will likely be even more degraded).

@7konigsberg 7konigsberg changed the title chat : strip trailing whitespace from parsed reasoning and content chat : stop qwen3-coder reasoning and content before template separators Oct 15, 2026
@7konigsberg

Copy link
Copy Markdown
Contributor Author

Thanks both. I switched to the parser change from aldehir's comment and dropped the mapper trim, so there is no whitespace stripping.

The session tests in test-chat check the exact strings and fail without the parser change. I also checked Qwen3.5-4B against master. Same text, tool calls, finish reasons and token counts. The separators before </think> and <tool_call> are left out of the parsed fields and added back by the template.

I did notice one issue while testing: with tools in the request, content ending in a newline without a tool call now loses that newline. The until rule treats it as a partial match at the end of input. I left that unresolved because fixing it looks like it needs the core parser to distinguish the final parse, which affects every format and felt outside the scope of this change.

@aldehir aldehir self-assigned this Oct 18, 2026
@aldehir

aldehir commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

I'll need to review all of Qwen's chat templates and ensure the assistant responses are properly reconstructed if we choose to do this.

I did notice one issue while testing: with tools in the request, content ending in a newline without a tool call now loses that newline. The until rule treats it as a partial match at the end of input.

Yeah, I can follow up to address this.

@7konigsberg

Copy link
Copy Markdown
Contributor Author

Thanks for picking up the trailing newline one.

So, I ran the reconstruction test on the parsed output for the four templates in models/templates that use this parser, plus the current HF Qwen3-Coder, Qwen3-Coder-Next, Qwen3.5, Qwen3.6, Qwen3.8 and Qwen-AgentWorld templates. The only thing that goes from OK to FAIL is an extra trailing newline case I added, on the Qwen3-Coder templates.

What do we think?

Details

The reconstruction test renders tc.expect, which is compared with the parse after trimming, so whitespace in the parsed fields doesn't reach it. I changed it in test_peg_parser to re-render the parsed fields:

reconstruction_msg.content           = parsed_msg.content;
reconstruction_msg.reasoning_content = parsed_msg.reasoning_content;

and ran test-chat --template <path> --force-reconstruction-test on master at ff5a233 and with this PR merged on top. I went through HF as well and I swapped the file in models/templates and ran the matching block.

Template master OK/FAIL branch OK/FAIL
Qwen3-Coder 18/5 18/5
Qwen3-Coder-30B-A3B-Instruct (HF, also 480B) 18/5 18/5
Qwen3-Coder-Next (HF) 19/4 19/4
Qwen3.5-4B (same file as the 9B to 397B models) 13/12 13/12
Qwen3.5-0.8B (HF, also 2B) 13/12 13/12
Qwen3.6-35B-A3B (HF, also 27B) 13/12 13/12
Qwen3.8-27B (HF, also Flash-Next) 12/13 12/13
Qwen3.8-2.4T-A95B (HF) 4/13 4/13
Qwen-AgentWorld-35B-A3B (HF) 13/12 13/12
NVIDIA-Nemotron-3-Nano-30B-A3B 2/8 5/5
StepFun3.5-Flash 10/3 10/3

Qwen3.8-2.4T raises when thinking is disabled, so I skipped those 8 cases for it. Nemotron passes 3 more on the branch because master keeps the newline before </think> in the reasoning and that template adds its own without trimming. Apart from those three, the failures are the same cases on both sides: continuations, inputs that don't follow the template's own separators, and re-serialized arguments. The Qwen3.5, Qwen3.6, Qwen3.8 and AgentWorld templates trim content and reasoning, and StepFun goes through trim_all_content, so the change doesn't show up in those rows.

The trailing newline case isn't in the counts. It's an extra case I added locally at the top of each block and ran with the change above (this is the Qwen3-Coder one):

tst.test("Hello, world!\nWhat's up?\n").tools({ special_function_tool }).expect(message_assist).run();

It goes from OK to FAIL only on the Qwen3-Coder templates, which don't trim plain content. On the other templates the content gets trimmed, so it fails on both sides.

Comment thread tests/test-chat.cpp Outdated
Comment on lines 8067 to 8101
for (size_t i = 0; i < thinking.size(); i += 3) {
session.feed(common_chat_input(thinking.substr(i, 3)));
}
assert_msg_equals(simple_assist_msg("", "I'm\nthinking\n"), session.msg());
assert_msg_equals(simple_assist_msg("", "I'm\nthinking"), session.msg());

for (size_t i = thinking.size(); i < output.size(); i += 3) {
session.feed(common_chat_input(output.substr(i, 3)));
}
assert_msg_equals(simple_assist_msg("", "I'm\nthinking\n", "special_function", "{\"arg1\":1}"), session.finish());
assert_msg_equals(simple_assist_msg("", "I'm\nthinking", "special_function", "{\"arg1\":1}"), session.finish());
}

// content before a tool call stops before the separator the template re-adds
{
const std::string output_with_content =
"I'm\nthinking\n</think>\n\n"
"Let me check.\n\n"
"<tool_call>\n"
"<function=special_function>\n"
"<parameter=arg1>\n1\n</parameter>\n"
"</function>\n"
"</tool_call>";

common_chat_session session(tmpls.get(), nullptr, inputs);

const std::string up_to_call = "I'm\nthinking\n</think>\n\nLet me check.\n\n";
for (size_t i = 0; i < up_to_call.size(); i += 3) {
session.feed(common_chat_input(up_to_call.substr(i, 3)));
}
assert_msg_equals(simple_assist_msg("Let me check.", "I'm\nthinking"), session.msg());

for (size_t i = up_to_call.size(); i < output_with_content.size(); i += 3) {
session.feed(common_chat_input(output_with_content.substr(i, 3)));
}
assert_msg_equals(simple_assist_msg("Let me check.", "I'm\nthinking", "special_function", "{\"arg1\":1}"), session.finish());
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the chat session tests, they're not intended to test the Qwen parser.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@aldehir aldehir left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay. I pushed a fix for the partial delimiter match at the end of a complete message. I checked every Qwen template since Qwen-Coder and the reconstruction matches exactly with these changes. I think it did even before, with the exception of a single Qwen 3.5 template.

@aldehir aldehir added the merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. label Oct 27, 2026
@ggerganov
ggerganov merged commit a71c00f into ggml-org:master Oct 28, 2026
16 checks passed
yzhou51 pushed a commit to yzhou51/llama.cpp that referenced this pull request Oct 30, 2026
…ors (ggml-org#30557)

* chat : strip trailing whitespace from parsed reasoning and content

Assisted-by: Claude Fable 5.1

* chat : stop qwen3-coder reasoning and content before template separators

Assisted-by: Claude

* tests : remove the Qwen parser case from the chat session tests

Assisted-by: Claude

* common : handle partial prefix until match at end of generation

---------

Co-authored-by: Zeshawn Zahid <zzshawnn@gmail.com>
Co-authored-by: Alde Rojas <hello@alde.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants