Skip to content

chat : add qwen3 specialized parser - #26252

Merged
aldehir merged 6 commits into
ggml-org:masterfrom
aldehir:qwen3-specialized-parser
Aug 2, 2026
Merged

chat : add qwen3 specialized parser#26252
aldehir merged 6 commits into
ggml-org:masterfrom
aldehir:qwen3-specialized-parser

Conversation

@aldehir

@aldehir aldehir commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Overview

Supersedes #24202, #26244

Qwen 3 models may occasionally emit a <tool_call> after <think>\n, usually without any reasoning which leads me to believe this is a valid tool call and not a reasoning trace.

This PR supports both </think> and <tool_call> as reasoning end sequences. It also adds support for older models that sometimes omit <tool_call>. Although I rather not support these outdated models, it seems there is community desire for support.

Additionally, we bring back argument permutation support, previously tried by @pwilkin, but optimized to minimize the number of active stacks in the grammar sampler at any one point in time. We have seen Qwen 3 try really hard to order arguments differently, to the point we had to impose harsher grammar constraints. Rather than fight it, we might as well allow it.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: Yes, predominately written with Opus 5.

@bartdeboer

Copy link
Copy Markdown
Contributor

Tested PR #26252 at commit 4bc63c9.

  • Qwen3.5-specific test-chat suite passed.
  • Full test-chat suite passed.
  • CUDA deployment with Qwen3.5-9B passed.
  • Verified tool calls before </think>, multiple calls, and reordered arguments.
  • A tool loop completed successfully without errors.

Image:
ghcr.io/bartdeboer/llama-cpp:server-cuda-qwen3-parser-pr26252-4bc63c901

Digest:
sha256:fd3eb704adcf2e3780443ec00c6eb0de08c9e614d0bb0039d298c7f5b9b3be6b

@pwilkin

pwilkin commented Jul 29, 2026

Copy link
Copy Markdown
Member

/bot review

@ggml-gh-bot

ggml-gh-bot Bot commented Jul 29, 2026

Copy link
Copy Markdown
Automated code review

andtool_call_start. The PR supports both as reasoning-end sequences and supports models omitting tool_call_start`.

Let me reconsider finding 1 with concrete token names from the PR description:

  • reasoning start tag = <think_open>
  • reasoning end tags = {"<think_close>", "<tool_call_start>"}
  • A tool call = <tool_call_start>\n + <function=...> ... or (omitted) just <function=...>.

The reasoning parser: optional(<think_open> + space + reasoning(until_one_of({"<think_close>", "<tool_call_start>"})) + (literal("<think_close>") | peek(literal("<tool_call_start>")))).

If the model:

  1. emits <think_open>, reasons, then emits <function=...> DIRECTLY (omitting <tool_call_start>), then until_one_of would NOT stop at <function=, swallowing the tool body. So reasoning grabs everything, tool_calls gets nothing, content empty → either fails or misparses.

Let me write the review accordingly, using the concrete tokens from the PR.

Now let me finalize the review messageStatic review of PR #26252 (commit 4bc63c9). Reviewed common/chat-peg-parser.{h,cpp}, the new common_chat_params_init_qwen3_coder in common/chat.cpp, and the test additions, and traced the PEG mapper plus the production reasoning-budget / grammar-suppression path (common/sampling.cpp, common/reasoning-budget.cpp).

The permute helper is well built: the bitmask remaining uniquely maps to a subset (bit i <-> parsers[i]), memoization via std::map<uint32_t,...> is keyed before re-emitting rule(...), and the new tests cover orderings, the single/empty/fallback edges, and the 2^n-1 rule bound. AI disclosure is present. A few things to address before merge.

Correctness - please confirm with a test

(point 1) Omitting the tool-call start token while a reasoning block is open is not handled and is untested. In the new parser the reasoning block is terminated with p.until_one_of({ "", "" }) while content is terminated with p.until_one_of({ "", "<function=" }). The two lists differ on ", so reasoning does not stop at ". If a model opens reasoning ( + "\n" + msg.reasoning_content with no following "...embedded <function=foo>...\n in the reasoning text, the until_one_of would consume it into reasoning and the tool call would be lost/misparsed. The closest existing tests all start the model output with the opening tool-call token, so the omission+open-thinking combination is genuinely untested.

Either it and the grammar stays inactive for the whole call, or document that omitted opening tokens are only expected when no reasoning block is open.

Will slow the review

(point 3) Worst-case grammar size. COMMON_CHAT_MAX_PERMUTE = 6 emits up to 2^6 - 1 = 63 distinct tool-<name>-args-<mask> rules per tool, on top of per-arg schema rules. For a server exposing many tools with several required params each, the grammar gets sizeable. A one-line comment near the cap explaining the rule-explosion / stack-depth tradeoff (the PR mentions minimizing active stacks) would let future readers judge whether to touch it.

Nits

(point 4) permute shifts 1u << parsers.size() with an unsigned int (common/chat-peg-parser.cpp). Safe at the cap of 6, but UB if COMMON_CHAT_MAX_PERMUTE is ever raised to >= 32. A static_assert(COMMON_CHAT_MAX_PERMUTE < 32) or a clamp in the permuting branch only guards it for free.

(point 5) permute relies on the caller supplying a unique rule_prefix (here "tool-" + name + "-args", safe because tool names are unique). The header doc only says "Matches every parser exactly once, in any order"; add that rule_prefix must be unique within the builder so a future caller doesn't redefine rules via rule(...).

(point 6) Optional args use p.zero_or_more(p.choice(optional_args)), which allows the same optional to repeat, producing duplicate keys in the arguments JSON (last-wins on parse). If the intent is "each optional at most once", consider permuting optionals too (bounded by the same cap) or noting the behavior. Low impact, models rarely repeat.

(point 7) The TOOL delimiter "<|im_start|>user\n<|im_start|>l_response>" (common/chat.cpp:1140) is non-obvious; it matches the sibling common_chat_params_init_minicpm5, so it isn't wrong, but one short comment would spare the next reader template archaeology.

`

Reproduced. Now let me write the final review cleanly with the actual token namesStatic review of PR #26252 (commit 4bc63c9). Reviewed common/chat-peg-parser.{h,cpp}, the new common_chat_params_init_qwen3_coder in common/chat.cpp, and the test additions, and traced the PEG mapper plus the production reasoning-budget / grammar-suppression path (common/sampling.cpp, common/reasoning-budget.cpp).

The permute helper is well built: the bitmask remaining maps uniquely to a subset (bit i <-> parsers[i]), memoization is consulted before re-emitting rule(...), and the single/empty/fallback edges plus the 2^n - 1 rule bound are all tested. AI disclosure is present. A few things to address before merge.

Correctness - please confirm with a test

(point 1) Omitting the tool-call start token while a reasoning block is open is not handled and is untested. In the new parser the open-reasoning block is p.until_one_of({ "", "" }), but content uses p.until_one_of({ "", "<function=" }). The two lists disagree on ". If a model opens reasoning ("|peek(p.literal(""` is the natural close, so doing so does not change the forced-end token).

Will slow the review

(point 3) Worst-case grammar size. COMMON_CHAT_MAX_PERMUTE = 6 emits up to 2^6 - 1 = 63 distinct tool-<name>-args-<mask> rules per tool, on top of per-arg schema rules. For a server exposing many tools with several required params each, the grammar gets sizeable. A short comment near the cap explaining the rule-explosion vs. stack-depth tradeoff (the PR text mentions minimizing active stacks) would help future readers judge whether to touch it.

Nits

(point 4) permute shifts 1u << parsers.size() with an unsigned int (common/chat-peg-parser.cpp). Safe at the current cap of 6, but UB if COMMON_CHAT_MAX_PERMUTE is ever raised to >= 32. A static_assert(COMMON_CHAT_MAX_PERMUTE < 32) (or a clamp in the permuting branch only) guards it for free.

(point 5) permute relies on the caller supplying a unique rule_prefix (here "tool-" + name + "-args", safe because tool names are unique). The header doc only says "Matches every parser exactly once, in any order"; add that rule_prefix must be unique within the builder so a future caller doesn't redefine rules via rule(...).

(point 6) Optional args use p.zero_or_more(p.choice(optional_args)), which lets the same optional repeat, producing duplicate keys in the arguments JSON string (last-wins on parse). If the intent is "each optional at most once", consider permuting optionals too (bounded by the same cap) or noting the behavior. Low impact - models rarely repeat.

(point 7) The TOOL delimiter "<|im_start|>user\n<|im_start|>user" (common/chat.cpp:1140) is non-obvious. It matches the sibling common_chat_params_init_minicpm5, so it isn't wrong, but one short comment would spare the next reader template archaeology.

This review was generated automatically by pi coding agent using zai-org/GLM-5.2. It may contain mistakes. Maintainers make the final call.

@pwilkin

pwilkin commented Jul 29, 2026

Copy link
Copy Markdown
Member

@aldehir bot went crazy but I think one point is valid, i.e. the in-thinking tool call only accepts the full syntax (<tool_call>...) while the out-of-thinking admits also <function= as the starter.

@aldehir

aldehir commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@aldehir bot went crazy but I think one point is valid, i.e. the in-thinking tool call only accepts the full syntax (<tool_call>...) while the out-of-thinking admits also <function= as the starter.

That was intentional to support Qwen3-Coder which sometimes doesn't emit the tool call tag, and it's not a thinking model.

@pwilkin

pwilkin commented Jul 29, 2026

Copy link
Copy Markdown
Member

Oh :) maybe add a comment for this then.

@aldehir

aldehir commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@pwilkin Done.

@pwilkin pwilkin added the merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. label Aug 1, 2026
@aldehir
aldehir merged commit f5919bf into ggml-org:master Aug 2, 2026
23 of 26 checks passed
kashif pushed a commit to kashif/llama.cpp that referenced this pull request Aug 2, 2026
* Add tagged thinking tool parser

* chat : refactor and add permute helper

* cont : add support for <tool_call> omission

* cont : update tool delimiters

* cont : add comment for qwen3-coder

* cont : fix trigger pattern for <function

---------

Co-authored-by: Bart de Boer <bart.deboer@gmail.com>
TheTom pushed a commit to TheTom/llama-cpp-turboquant that referenced this pull request Aug 3, 2026
* Add tagged thinking tool parser

* chat : refactor and add permute helper

* cont : add support for <tool_call> omission

* cont : update tool delimiters

* cont : add comment for qwen3-coder

* cont : fix trigger pattern for <function

---------

Co-authored-by: Bart de Boer <bart.deboer@gmail.com>
belarusian pushed a commit to belarusian/llama.cpp that referenced this pull request Aug 4, 2026
* Add tagged thinking tool parser

* chat : refactor and add permute helper

* cont : add support for <tool_call> omission

* cont : update tool delimiters

* cont : add comment for qwen3-coder

* cont : fix trigger pattern for <function

---------

Co-authored-by: Bart de Boer <bart.deboer@gmail.com>
smalinin pushed a commit to smalinin/llama.cpp that referenced this pull request Aug 4, 2026
* Add tagged thinking tool parser

* chat : refactor and add permute helper

* cont : add support for <tool_call> omission

* cont : update tool delimiters

* cont : add comment for qwen3-coder

* cont : fix trigger pattern for <function

---------

Co-authored-by: Bart de Boer <bart.deboer@gmail.com>
satindergrewal pushed a commit to satindergrewal/llama.cpp that referenced this pull request Aug 11, 2026
* Add tagged thinking tool parser

* chat : refactor and add permute helper

* cont : add support for <tool_call> omission

* cont : update tool delimiters

* cont : add comment for qwen3-coder

* cont : fix trigger pattern for <function

---------

Co-authored-by: Bart de Boer <bart.deboer@gmail.com>
satindergrewal pushed a commit to satindergrewal/llama.cpp that referenced this pull request Aug 12, 2026
* Add tagged thinking tool parser

* chat : refactor and add permute helper

* cont : add support for <tool_call> omission

* cont : update tool delimiters

* cont : add comment for qwen3-coder

* cont : fix trigger pattern for <function

---------

Co-authored-by: Bart de Boer <bart.deboer@gmail.com>
cubetitled-ui pushed a commit to cubetitled-ui/llama.cpp that referenced this pull request Aug 14, 2026
* Add tagged thinking tool parser

* chat : refactor and add permute helper

* cont : add support for <tool_call> omission

* cont : update tool delimiters

* cont : add comment for qwen3-coder

* cont : fix trigger pattern for <function

---------

Co-authored-by: Bart de Boer <bart.deboer@gmail.com>
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