Skip to content

Parse Qwen3-Coder XML replies around their tool calls as llama.cpp does (#1531) - #1885

Closed
leehack wants to merge 2 commits into
fix/1865-thinking-tags-textfrom
fix/1531-qwen3coder-seed-parity
Closed

leehack wants to merge 2 commits into
fix/1865-thinking-tags-textfrom
fix/1531-qwen3coder-seed-parity

Conversation

@leehack

@leehack leehack commented Nov 10, 2026 •

Copy link
Copy Markdown
Owner

Summary

Refs #1531. Refs #1874 (0.19.4 plan, lane 4).

Stacked on #1882 (base fix/1865-thinking-tags-text at a6ea0316b). Review only the commits after a6ea0316b, and retarget to main once #1882 merges.

Qwen3-Coder XML replies (Qwen3-Coder, Qwen3.5, StepFun3.5-Flash) now parse and stream as llama.cpp cd959bae9 parses them (common/parsers/qwen3-coder.cpp):

  • Text after the calls is dropped. The content ends at the first <tool_call>. The calls are those of the valid scopes after it, and anything after them is dropped: trailing text, a second <function=> in one scope, an empty or invalid scope, or an unknown tool. Before, that text stayed as content, and an invalid first scope kept the whole reply as content.
  • One call without parallelToolCalls. Without a grammar, llama.cpp takes only the first call when parallel tool calls are off. llamadart now does the same.
  • A truncated last scope still counts. A finished reply whose last valid call is missing its </tool_call>, or has only part of it, still returns that call.
  • The Qwen3-Coder template has no reasoning. It has no <think>, so llama.cpp takes no thought from its replies, and a stray </think> stays as content (lane 3's 128 Plan.</think> rows).
  • StepFun3.5-Flash with thinking off now ends its thought at </think> or <tool_call>. llama.cpp keeps the thought open in its prompt (<think>\n); llamadart closes it, as it does for Seed-OSS and Cohere2 MoE. Before, the stream held the whole reply. Those three formats now share one registry trait (reopensThoughtClosedForThinkingOff) in place of two OR checks.

Contract change for review: before, an invalid Qwen call stayed in the reply as content (#531's malformed-call tests). Now it is dropped, as llama.cpp drops it. The affected tests are updated to llama.cpp's results.

Root cause. parseXmlToolCalls treated any text it could not parse as a call as content, and it kept every call. llama.cpp's PEG parser does neither: it stops at the end of the last valid call, takes min..1 call groups, and has no end().

Production-readiness scope

  • User-facing scope: Qwen3-Coder, Qwen3.5 and StepFun3.5-Flash tool-call replies give the content, reasoning and calls llama.cpp gives, in the final parse and in every stream chunking.

  • Supported platforms/paths: All. The change is in shared Dart template parsing (native llama.cpp and WebGPU bridge).

  • Unsupported or intentionally unavailable paths: Each item below still differs from llama.cpp on purpose. They are recorded as left out in test/fixtures/qwen3_coder_xml_upstream.md.

    • A reply cut inside a call: llama.cpp emits unterminated JSON arguments; llamadart emits no call.
    • Strict newline framing (<tool_call><function=, </function></tool_call>): llamadart still reads these forms.
    • A partial parse of an unclosed scope.
    • Under required, a reply with no valid call: llama.cpp fails it; llamadart returns content.
  • Out of scope / follow-ups:

    • A bare <function=NAME> call without <tool_call> for the Qwen3-Coder template.
    • The Seed-OSS leniencies in #1542: llama.cpp fails those replies, so llamadart needs a policy decision first.
    • The Qwen3.5/StepFun required + forced-open grammar.

    These are reported on #1874.

Completeness checklist

  • Declared scope is fully implemented, or explicitly reduced above.
  • Unsupported platform/option combinations fail loudly with actionable diagnostics or are clearly documented as unavailable. (No new unsupported path.)
  • Public API docs, README/website docs, examples, support matrices, and changelog entries are updated where relevant. (Both changelogs, chat-template-and-parsing.md, template-engine-internals.md, and two skills/ SKILL.md files. No public API change.)
  • Regression coverage covers the original issue plus key negative/version-skew paths where relevant.
  • Public capability changes update the validation coverage registry with a pack or a reasoned, issue-linked exemption; inventory checks pass (or N/A with scope). N/A: parser parity fix, no capability change.
  • Security/privacy review completed: no secrets, bearer tokens, signed URLs, or raw secret-bearing paths leak through logs, cache keys, metadata, errors, or snapshots.
  • Follow-up work that is useful but not required for this PR is tracked in GitHub Issues, or explicitly marked "None" above.

PR type guidance

  • Bugfix PR: The root cause and the fix are above.
    • New upstream fixture. test/fixtures/qwen3_coder_xml_upstream.json records 18 cases on 5 template configs: Qwen3-Coder (thinking on), and Qwen3.5-0.8B and StepFun3.5-Flash (thinking on and off), each with parallel off and on. Every case has llama.cpp's finished parse, and its partial parse where that is meaningful. The cases cover:
      • text after a call, and two calls with text after them
      • two functions in one scope
      • an empty first scope
      • an unknown tool between calls
      • a later scope with no end, part of an end, or cut in its name
      • a scope tag in text
      • </think> followed by a JSON or XML call
      • a thought followed by text and a call
    • New test. test/integration/core/engine/qwen3_coder_xml_upstream_test.dart (180 tests) checks the parse against llama.cpp. It checks the stream against the parse for the whole reply, byte by byte, in 3-character chunks and split in two at every point. 125 of its tests fail on base.
    • Unit tests. The endsAtLastCall group in xml_tool_call_format_test.dart and the single-call mark in template_internal_metadata_test.dart.

Test Plan

  • dart run tool/prepare_workspace.dart
  • dart format --output=none --set-exit-if-changed .
  • dart analyze, plus dart run tool/testing/check_platform_boundaries.dart
  • dart test -p vm -j 1 --exclude-tags local-only (isolated TMPDIR): 12,658 passed, 55 skipped, and 6 failed. All 6 are windows_registry_qualification_test.dart, which needs pwsh and this host has none
  • dart test -p chrome --exclude-tags local-only: 4,665 passed, 1 skipped
  • coverage-lib: run by the dispatched CI below (ci.yml collects coverage on manual runs)
  • ./tool/testing/run_template_parity_suites.sh: detection 74, diagnostic 103, unit template 1,873, upstream parser 10, GBNF acceptance 79, all passed

Matrix Evidence

Matrix row Scope covered Platform / model / backend Result Evidence / notes
root-vm native-safe unit/integration macOS arm64 VM PASS 12,658 passed, 55 skipped, and 6 failed. All 6 are windows_registry_qualification_test.dart, which needs pwsh and this host has none
root-chrome browser-compatible tests Chrome PASS 4,665 passed, 1 skipped
template-parity detection, render and parse parity; GBNF acceptance macOS arm64 PASS detection 74, diagnostic 103, unit template 1,873, upstream parser 10, GBNF acceptance 79
structured-output-adversarial parse, partial and every-chunking stream against llama.cpp; auto/required/none tool choice; thinking on/off; parallel on/off llama.cpp cd959bae9 common_chat_session oracle PASS See the sweep tables below. 0 records regress.
high-risk-exact-head-independent-qa independent blocking review of the exact head — PENDING Required before mark-ready
coverage-lib lib/ line coverage Linux CI PASS Dispatched ci.yml run
Other rows — — N/A No runtime, backend, grammar, release or LiteRT-LM change

Lane sweep. It covers 6 XML templates (Qwen3-Coder, Qwen3.5-0.8B and 4B, StepFun3.5-Flash, Seed-OSS and Nemotron-3-Nano), 48,264 records in all, thinking on and off, and 6 tool modes (no tools, none, auto, required, and auto and required with parallel). The corpus includes the #1539/#1542 shapes. Each record is compared with llama.cpp cd959bae9:

Measure Base a6ea0316b Head Regressions
Finished parse = llama.cpp 13,430 19,608 0
Partial parse = llama.cpp 13,710 19,888 0
Every chunking = parse 23,800 23,884 0

Broad corpora (lane 3's, all formats; finished parse = llama.cpp). Only the three Qwen3-Coder XML templates change.

Corpus Base Head Regressions
bare 59,640 60,604 0
full 66,990 67,658 0
native 3,286 3,572 0
vbare 18,012 18,100 0
vfull 20,152 20,256 0

Mutations: 19 of 20 killed.

Area Mutants
Handler Q1–Q4
Call parser X1–X6
Engine E1–E3
Stream S1, S3
Registry R1–R3 (Qwen, Seed-OSS, Cohere2 MoE)
Thought marks T1

S2 (no single-call mark on the stream's partial parse) survived as equivalent, because partial calls only decide whether the reply has calls. That plumbing was removed (47a86a526).

Review Notes

  • Independent review status: pending (a fresh auditor session, at the exact head).
  • CI status / head SHA: 47a86a526, dispatched ci.yml run 42745372367 (success, coverage gate included).

High-risk regression review

  • Classification: high-risk (backendRuntime, structuredOutput)
  • Implementation task: lane 4 of #1874
  • Independent blocking QA task: pending
  • Exact head / current base: 47a86a526 / fix/1865-thinking-tags-text a6ea0316b
  • Production-branch deletion, bypass, or miswire proof: the 19 killed mutants above, covering the handler, parser, engine, stream and registry call sites
  • Affected-family real-model/artifact evidence: primary llama.cpp cd959bae9 emissions on the vendored Qwen3-Coder, Qwen3.5 and StepFun3.5-Flash templates (durable fixture plus sweeps). No model weights were needed: the change is in parsing only.
  • Explicit unavailable-family or other N/A evidence: N/A
  • Known PR-caused P1 regressions: 0
  • Unresolved review threads: 0

Partial tool calls only decide whether a reply has calls, so the mark
changed nothing there.
@leehack

leehack commented Nov 10, 2026

Copy link
Copy Markdown
Owner Author

Landed on main via batch integration PR #1895 (rebase merge, main ba4e0e1b; tree identical to the audited integration head 5ffd1224).

@leehack leehack closed this Nov 10, 2026
@leehack
leehack deleted the fix/1531-qwen3coder-seed-parity branch November 10, 2026 13:16
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.

1 participant