Repository navigation
Conversation
…r does (#1531) With tool_choice required, llama.cpp (cd959bae9, common/parsers/qwen3-coder.cpp) builds a non-lazy grammar that takes an optional thought, then any text up to the first call opening, then the calls. For the Qwen3-Coder template, which has no thought, the first call may also leave out `<tool_call>` and start at a declared tool's `<function=name>`. llamadart's required grammar started with `<tool_call>`, so Qwen3.5 and StepFun3.5-Flash could not think first and every reply had to open with the call. - `buildXmlToolCallGrammar` takes `textBeforeCalls` (text without a call opening before the calls) and `bareFirstCall` (optional first scope start; declared `<function=name>` openings end the text). - `ToolCallGrammarUtils.excludingRules` now excludes several delimiters (Aho-Corasick); its output for one delimiter is unchanged. - For the Qwen3-Coder template the lazy grammar also triggers at `<function=name>`, and the parse and stream (mark `bareFirstCallOf`) take a bare first call, as llama.cpp's parser does. llama.cpp's own grammar also admits a `<tool_call>` split between its thought and content rules after a thought; llamadart rejects those replies, which llama.cpp's parser does not take as calls either. Refs #1874
They read the vendored templates from disk, which Chrome cannot.
This was referenced Nov 10, 2026
…as \x5E
- Drop the `<function=name>` lazy triggers: with ToolChoice.auto, a reply that
writes a declared tool's tag as text made the native sampler fail ("Unexpected
empty grammar stack") or forced a call. llama.cpp has that trigger; llamadart
diverges on purpose. The required grammar, parse and stream still take a bare
first call.
- `excludingRules` escaped `^` as `\^`, which llama.cpp's GBNF parser rejects;
a tool name holding `^` failed grammar init.
- Docs: a required call may now follow a thought or text, so a reply that
spends maxTokens first ends with `length` and no call.
- Pin that Nemotron-3-Nano keeps its own required grammar.
Refs #1531
With ToolChoice.auto, a reply that writes a declared tool's `<function=name>` as text lost everything from that tag on, as llama.cpp's parse does. The parse, stream and grammar now take a call without `<tool_call>` only for a required call; otherwise the tag is text, as before #1531. This diverges from llama.cpp on purpose. - The fixture records the bare-call cases under two Qwen3-Coder required entries (`tool_choice` per entry) and adds `longer-bare-function-in-text` (`<function=get_weathers>`) everywhere. - The real-model auto test now checks the answer after the tag is kept. Refs #1531
19 tasks done
leehack
added a commit
that referenced
this pull request
Nov 10, 2026
leehack
added a commit
that referenced
this pull request
Nov 10, 2026
Owner
Author
|
Landed on main via batch integration PR #1895 (rebase merge, main |
9 of 10 tasks
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
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.
Summary
Refs #1531. Refs #1874 (0.19.4 plan, lane 4c).
Stacked on #1885 (base
fix/1531-qwen3coder-seed-parityat47a86a526, itself on #1882). Review only the commits after47a86a526, and retarget once #1885 merges.With
ToolChoice.required, Qwen3-Coder XML grammars (Qwen3-Coder, Qwen3.5, StepFun3.5-Flash) now take what llama.cppcd959bae9takes (common/parsers/qwen3-coder.cpplines 165–194):parallelToolCalls, one or more). Before, it started with<tool_call>, so Qwen3.5 and StepFun3.5-Flash could not think before a required call.<tool_call>. That template has no<think>, so llama.cpp also accepts a first call that starts at a declared tool's<function=name>, and ends the content there. For a required call, the grammar, the final parse and the stream now do the same. Later calls still need<tool_call>, and templates with a thought still treat a bare<function=…>as text.<function=name>stays text. This differs from llama.cpp on purpose, in the lazy grammar, the parse and the stream. llama.cpp'sautogrammar triggers at that tag, and its parse ends the content there. A reply that writes the tag as text then fails in the sampler (Unexpected empty grammar stack after accepting piece), is forced into a call, or loses the rest of its answer. llamadart keeps the whole answer, as before.maxTokensfirst ends withlengthand no call. The docs, the skill and the changelog say so.excludingRulesescapes^as\x5E. llama.cpp's GBNF parser rejects\^, so a tool name holding^failed grammar init.Root cause.
buildXmlToolCallGrammarhad one root for every tool choice,tool-call-scope(ortool-call-scope+), which the engine turns non-lazy forrequired. llama.cpp's required root isoptional thought, until(call openings), calls, with<tool_call>\noptional on the first call when the template has no thought.One intentional difference. After a thought, llama.cpp's thought rule and content rule are separate
untilrules whose states all accept, so a<tool_call>split across them passes: its grammar takes<tool_call>before a call, and with parallel calls off a second call. Its parser then drops what follows the first call. llamadart's exclusion rules already refuse to end text where a delimiter could start inside it, so these replies are rejected. This is recorded intest/support/qwen3_coder_required_calls.dart.Production-readiness scope
<tool_call>returns that call.<tool_call>replies above: llama.cpp's grammar takes them, llamadart's does not.auto(ornone), a bare<function=name>is text, not a call start (see above).<). llama.cpp's body is schema-typed and newline-exact. Unchanged here.<tool_call>is reasoning in llama.cpp and content in llamadart (pre-existing, from Parse Qwen3-Coder XML replies around their tool calls as llama.cpp does (#1531) #1885's thought handling). The two fixture cases that show it are left out and noted inqwen3_coder_xml_upstream.md.</think>). To be reported on #1874.Completeness checklist
tool-calling.md,template-engine-internals.md,skills/llamadart-tool-calling/SKILL.md. No public API change.)PR type guidance
test/support/qwen3_coder_required_calls.dart): one list of accepted and rejected replies per template config, built from llama.cpp's own required grammars. It runs through the DartGbnfRecognizerintest/integration/core/template/qwen3_coder_required_grammar_test.dart(CI, VM) and through llama.cpp'stest-gbnf-validatorinspecialized_tool_grammar_validation_e2e_test.dart(local-only). The configs are Qwen3-Coder, Qwen3.5-0.8B (thinking on and off) and StepFun3.5-Flash (on and off), each with parallel off and on. The cases cover: a call; text then a call; a thought then text then a call;</think>twice;<function=unknown>in text; parallel calls; a bare first call (Qwen3-Coder only); and as rejections, no call, text after the calls, an unknown tool, a bare second call, a second call without parallel, and a bare call where llama.cpp takes it as text.qwen3_coder_xml_upstream.jsongains 6 cases per config, recorded with the samecd959bae9probe:bare-call,text-then-bare-call,bare-call-then-call,call-then-bare-call,unknown-bare-function-in-text,longer-bare-function-in-text. The probe first reproduced all 180 existing records (0 mismatches). The integration test checks each case's parse, partial parse and every stream chunking.llama_cpp_chat_template_backend_e2e_test.dart(local-only) loads Qwen3.5-0.8B-Q4_K_M with its own template. It makes a required call with thinking on and off. The grammar must compile in the native sampler and givetool_calls, and with thinking on the reply must have reasoning first.ToolCallGrammarUtils.excludingRulesnow handles several delimiters. A brute-force test checks it on all strings up to length 5 for four delimiter sets. Its output for one delimiter is byte-identical to the old_excludingRules(checked on 11 delimiters).Test Plan
dart run tool/prepare_workspace.dartdart format --output=none --set-exit-if-changed .dart analyze, plusdart run tool/testing/check_platform_boundaries.dartdart test -p vm -j 1 --exclude-tags local-only(isolatedTMPDIR, atb75aa2e91): 12,782 passed, 11 skipped, and 6 failed. All 6 arewindows_registry_qualification_test.dart, which needspwshand this host has nonedart test -p chrome --exclude-tags local-only(atb75aa2e91): 4,667 passed, 1 skippedcoverage-lib: run by the dispatched CI below (ci.ymlcollects coverage on manual runs)b75aa2e91:specialized_tool_grammar_validation_e2e_test.dartwith thev0.9.0(cd959bae9)test-gbnf-validator: 81 passed.llama_cpp_chat_template_backend_e2e_test.darton Qwen3.5-0.8B-Q4_K_M, CPU: 4 passed./tool/testing/run_template_parity_suites.sh(atb75aa2e91): detection 74, diagnostic 103, unit template 1,875, upstream parser 10, GBNF acceptance 81, all passedMatrix Evidence
root-vmpwsh-only failuresroot-chrometemplate-paritystructured-output-adversarialcd959bae9grammars andcommon_chat_sessionoracle; Qwen3.5-0.8B-Q4_K_Mhigh-risk-exact-head-independent-qacoverage-libci.ymlrunGrammar differential. The 10 configs' llama.cpp required grammars came from the
cd959bae9probe. Each reply ran throughtest-gbnf-validatorwith the generation prompt prefilled, as llama.cpp's sampler does (common/sampling.cpp). llamadart's grammar ran on the same reply.<tool_call>after a thought, accepted)Failing first. The new and updated tests were run against base
47a86a526lib. 19 fail:required_grammar_startbare-call,text-then-bare-callandbare-call-then-callfixture casesThe real-model test fails under mutant M1 (the old root): the reply has no thought before the call.
Parse sweep against llama.cpp. The corpus is all fixture outputs plus 8 bare-tag prose replies, 32 in all. It runs on 20 configs: each template, thinking setting and parallel setting, with
autoandrequired. That is 640 records, compared with thecd959bae9common_chat_sessionfinished parse.47a86a52655c69df04b75aa2e91Every changed record is intended:
requiredrecords change.requiredhas no valid call, so llama.cpp fails the reply and llamadart returns content (pre-existing, noted above). The required grammar does not let the model write these replies.55c69df04: only the 18 Qwen3-Coderautorecords holding a bare<function=get_weather>change. They go back to base behaviour: the tag and what follows are content.Real-model regressions. Both use Qwen3.5-0.8B-Q4_K_M on CPU, in
llama_cpp_chat_template_backend_e2e_test.dart:ToolChoice.autowith the Qwen3-Coder template and the prompt "explain what the literal text<function=get_weather>means". The reply must have no call, no error, and the answer after the tag.55c69df04it fails: the content is only<think>\n.<function=name>trigger restored, llama.cpp's sampler throwsUnexpected empty grammar stack after accepting piece.Mutations: 18 of 18 killed at
b75aa2e91.excludingRulesexcludingRules<function=name>lazy trigger in auto (llama.cpp's trigger)<tool_call>withThoughtMarksOfparseRendereddrops the mark_parsedrops the flag^escaped as\^(also fails the validator:unknown escape at \^])excludingRules>(audit A12,<function=get_weathers>)M13 is the mark copied by the stream's forced-open copy. It survived at
03f40f9cebecause that copy can never run, so the copy was removed. M16 (the lazy grammar takes a bare first call) is now the same mutant as M19.Audit follow-ups.
55c69df04fixed the three findings from the audit at95d4986c9: the auto trigger, therequiredbudget docs and the^escape. It also pinned A15.b75aa2e91follows the re-audit at55c69df04. It takes a bare first call only for a required call (maintainer decision), and pins A12.Review Notes
95d4986c9PASS with three findings, fixed in55c69df04. Re-audit at55c69df04PASS with one change, made inb75aa2e91. A re-audit ofb75aa2e91is pending.b75aa2e91, dispatched ci.yml run 42796454762 (success, coverage gate included).High-risk regression review
backendRuntime,structuredOutput)95d4986c9and re-audit at55c69df04: PASS, with findings fixed in55c69df04andb75aa2e91. Re-audit ofb75aa2e91pendingb75aa2e91/fix/1531-qwen3coder-seed-parity47a86a526cd959bae9grammars and parses cover the vendored Qwen3-Coder, Qwen3.5 and StepFun3.5-Flash templates.