Skip to content

feat(core): XML tool-argument grammar constraint for Qwen3-Coder (#383) - #385

Merged
pekkah merged 4 commits into
masterfrom
feat/383-qwen-coder-xml-tool-grammar
Jun 24, 2026
Merged

feat(core): XML tool-argument grammar constraint for Qwen3-Coder (#383)#385
pekkah merged 4 commits into
masterfrom
feat/383-qwen-coder-xml-tool-grammar

Conversation

@pekkah

@pekkah pekkah commented Jun 24, 2026

Copy link
Copy Markdown
Owner

Closes #383.

Schema/grammar-constrained tool-call argument decoding (#374) now covers every JSON wire format — Gemma 4 and Qwen/Llama/DeepSeek (#376). Qwen3-Coder was the one remaining family: it emits arguments as XML, not JSON, and QwenCoderToolCallAdapter had no BuildArgumentConstraint override, so Coder tool args generated unconstrained.

<tool_call><function=get_weather><parameter=location>Paris</parameter></function></tool_call>

What this does

A byte-level pushdown-automaton sibling of GemmaToolArgumentConstraint / JsonToolArgumentConstraint (QwenCoderToolArgumentConstraint) that walks the Coder XML skeleton instead of JSON braces:

  • Arms on the <tool_call> special token (leak-proof gate), byte-scans the preamble for <function=NAME>, and engages at the > closing the function tag — before the first <parameter=> — so a merged ></function> can't drop required parameters (the XML analogue of the merged-{} early-engage trick).
  • Constrains <parameter=KEY> to declared keys, enforces required-once, and forbids </function> until every required parameter is emitted; once all declared keys are used, only the close tag remains.
  • Constrains each value region by type where checkable: numbers, enums, booleans, null are bare-text constrained; typed strings / arrays / objects / untyped values are free content (Tool-arg grammar: constrain partially-typed tools (Any/open-object values) (#374 follow-up) #378 FreeValue) terminated by </parameter> — so partially-typed Coder tools stay constrained on their typed/required parts.
  • Reuses the shared ToolSchemaCompiler (CompiledObject/CompiledNode).

Architecture routing (the part GPU verification surfaced)

The real Qwen3-Coder-30B-A3B-Instruct GGUF reports general.architecture = qwen3moeidentical to non-Coder Qwen3-MoE — and is distinguishable only by its chat template. So it resolves to QwenToolCallAdapter, whose constraint was JSON-only and never engaged on Coder's XML. Architecture alone can't tell the two apart.

Fix: QwenToolCallAdapter.BuildArgumentConstraint now overlays the JSON (#376) and Coder XML (#383) constraints with a new CompositeToolArgumentConstraint. Both arm on the shared <tool_call> token and diverge on the first body byte ({ vs <function=), so whichever matches the model's actual output engages and the other stays inert — no chat-template threading needed, and the JSON path is byte-identical to before. A GGUF that does report qwen3coder keeps the dedicated XML-only adapter.

Guarantees (enforced by the repo)

  • Default-off byte-identical to unconstrained decoding; allocation-free per-token hot path (reused mask/scratch buffers); NativeAOT-compatible; InvariantGlobalization (Ordinal-only).
  • Clean dotnet build (0 warnings, TreatWarningsAsErrors + trim/AOT analyzers).

GPU A/B (Qwen3-Coder-30B-A3B-Instruct-Q4_K_M, CUDA hybrid --cpu-moe)

Adversarial tool: temperature_scale is a required enum ["celsius","fahrenheit"]; the prompt asks for an out-of-enum unit:

What's the temperature in Tokyo right now? I want the answer in Kelvin.

Unconstrained (--tools) — emits a schema-violating value:

get_weather({"location":"Tokyo","temperature_scale":"kelvin"})

Constrained (--tools --tool-grammar) — forced into the declared enum:

get_weather({"location":"Tokyo","temperature_scale":"celsius"})

Tests

  • FakeCoderTokenizer (Coder vocab with merged structural pieces, à la FakeJsonTokenizer) + 16 model-free mock conformance tests (early-engage required-param block, foreign-key rejection, bare enum/number/boolean, free strings, partially-typed free values, multi-param required-once, all-params-emitted, re-arm across functions, </tool_call> disarm, default-off no-<tool_call>).
  • A model-gated conformance test over the real Qwen3-Coder GGUF (asserts the same invariants over the real BPE — including the merged >\n / =get tokens the model actually emits).
  • Updated the model-gated Qwen conformance test: XML output now engages via the composite (was the documented Tool-arg grammar: Qwen/Llama JSON-syntax constraints (#374 follow-up) #376 limitation); JSON still engages unchanged.
  • 237/237 Tests.Core pass.

🤖 Generated with Claude Code

pekkah and others added 3 commits June 24, 2026 12:57
Qwen3-Coder emits tool-call arguments as XML
(<function=NAME><parameter=KEY>VALUE</parameter></function>), the one wire
format not covered by the JSON (#376) or Gemma (#374) argument-grammar
constraints. QwenCoderToolCallAdapter had no BuildArgumentConstraint override,
so its arguments decoded unconstrained.

Add QwenCoderToolArgumentConstraint, a byte-level pushdown-automaton sibling of
GemmaToolArgumentConstraint / JsonToolArgumentConstraint that walks the Coder XML
skeleton instead of JSON braces:

- Arms on the <tool_call> special token (leak-proof gate), byte-scans the
  preamble for <function=NAME>, and engages at the '>' closing the function tag —
  BEFORE the first <parameter=> — so a merged "></function>" can't drop required
  parameters (the XML analogue of the merged-{} early-engage trick).
- Constrains <parameter=KEY> to declared keys, enforces required-once, and forbids
  </function> until every required parameter is emitted. Once all declared keys are
  used, only the close tag remains.
- Constrains each value region by type where checkable: bare numbers, enums,
  booleans, and null; typed strings / arrays / objects / untyped values are free
  content (issue #378 FreeValue) terminated by </parameter>, so partially-typed
  Coder tools stay constrained on their typed/required parts.
- Reuses the shared ToolSchemaCompiler (CompiledObject/CompiledNode).

Default-off byte-identical to unconstrained decoding; allocation-free per-token hot
path (reused mask/scratch buffers); NativeAOT-compatible; Ordinal-only string ops.

Wire via QwenCoderToolCallAdapter.BuildArgumentConstraint; refresh the now-stale
"Gemma 4 today" CLI/server help text to list every supported family.

Tests: FakeCoderTokenizer (Coder vocab with merged structural pieces) + 16
model-free mock conformance tests, plus a model-gated conformance test over the
real Qwen3-Coder-30B-A3B GGUF.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LKvjcWw1gH5HyXAwsK3quP
…#383)

The real Qwen3-Coder-30B-A3B GGUF reports general.architecture="qwen3moe"
(identical to non-Coder Qwen3-MoE) and is distinguishable only by its chat
template, so it resolves to QwenToolCallAdapter, whose argument-grammar constraint
was JSON-only (#376) and never engaged on Coder's XML output — the XML constraint
added in the prior commit was wired to the "qwen3coder" architecture key the real
model never reports.

Fix: QwenToolCallAdapter.BuildArgumentConstraint now overlays the JSON and
Qwen3-Coder XML constraints with a new CompositeToolArgumentConstraint. Both arm on
the shared <tool_call> token and diverge on the first body byte ({ vs <function=),
so whichever matches the model's actual output engages and the other stays inert —
no chat-template threading needed, and the JSON path is byte-identical to before.

GPU A/B verified on Qwen3-Coder-30B-A3B (CUDA hybrid, --cpu-moe), enum
[celsius,fahrenheit] with a "give it in Kelvin" prompt:
  unconstrained  → temperature_scale="kelvin"   (out-of-enum, schema violation)
  --tool-grammar → temperature_scale="celsius"  (forced into the declared set)

Also: reword the PushValue full-stack doc comment to match the code (review nit);
update the model-gated Qwen conformance test — XML output now engages via the
composite (was the documented #376 limitation), JSON still engages unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LKvjcWw1gH5HyXAwsK3quP
…ispatch (#383)

The composite's JSON-vs-XML format dispatch was previously exercised only by the
model-gated conformance tests, so a CI runner without the GGUFs didn't cover it.
Add two model-free tests (FakeJsonTokenizer): Coder XML output engages the XML sub
and enforces the required parameter; JSON output engages the JSON sub unchanged and
Reset returns the whole composite to pass-through. Review follow-up.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LKvjcWw1gH5HyXAwsK3quP

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for Qwen3-Coder's XML tool-call format by implementing QwenCoderToolArgumentConstraint and a CompositeToolArgumentConstraint to overlay JSON and XML constraints. It also adds comprehensive unit tests and updates documentation. The review feedback highlights several critical issues in the new constraint implementation: potential bit collisions and wrap-around when handling schemas with more than 64 parameters or enum values due to ulong bitmask limitations, an issue where _toolCallCloseId defaults to 0 instead of -1 on lookup failure, and a bug in FeedRawBytes where valid merged tokens with trailing characters after are incorrectly rejected.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +199 to +205
if (node.Literals is not null)
{
parent.State = FParamClose;
ref var f = ref _stack[_depth++];
f = default; f.Kind = FK.Lit; f.State = LStart; f.Node = node; f.Cand = AllBits(node.Literals.Length);
return true;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The constraint uses a ulong bitmask (f.Cand) to track candidate literals. If a schema defines an enum with more than 64 values, 1UL << i will wrap around due to i % 64 behavior in C#, causing bit collisions. This can lead to incorrect masking where invalid enum values are accepted or valid ones are rejected. Safely fall back to treating the parameter as a free value (FK.Free) if the enum has more than 64 values.

        if (node.Literals is not null && node.Literals.Length <= 64)
        {
            parent.State = FParamClose;
            ref var f = ref _stack[_depth++];
            f = default; f.Kind = FK.Lit; f.State = LStart; f.Node = node; f.Cand = AllBits(node.Literals.Length);
            return true;
        }

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Already guarded upstream, so this path is unreachable: ToolSchemaNode caps EnumValues at Count <= 64 (ToolSchema.cs:56) — a larger enum degrades to null (no enum) — and booleans/null are 2/1 literals. So CompiledNode.Literals.Length is always ≤ 64 by the time it reaches PushValue. This is the same AllBits(...) + 1UL << i pattern the JSON (JsonToolArgumentConstraint) and Gemma siblings use without an extra check, relying on the same upstream cap. Adding a redundant <= 64 here would diverge from the siblings for no reachable benefit, so leaving as-is.

Comment on lines +88 to +89
_toolCallCloseId = -1;
_ = vocab.TryGetSpecialToken(QwenCoderToolCallAdapter.ArmCloseMarker, out _toolCallCloseId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The default value of -1 assigned to _toolCallCloseId is immediately overwritten by the out parameter of TryGetSpecialToken, even if the token is not found (which typically defaults to 0). If 0 is a valid token ID in the vocabulary, this can lead to accidental disarming of the constraint when token 0 is emitted. It is safer to check the return value of TryGetSpecialToken and explicitly set it to -1 on failure.

        if (!vocab.TryGetSpecialToken(QwenCoderToolCallAdapter.ArmCloseMarker, out _toolCallCloseId))
        {
            _toolCallCloseId = -1;
        }

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Good catch — fixed in b67fbf2. Now set from the return value so a missing </tool_call> stays -1 and never matches token id 0 (pad/unk).

Comment on lines +94 to +99
foreach (var t in tools)
{
if (t.Arguments.Open) continue; // unconstrained body — skip
var compiled = ToolSchemaCompiler.TryCompileObject(t.Arguments);
if (compiled is not null) _tools[t.Name] = compiled;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Similar to the enum issue, if a tool schema defines more than 64 parameters, the ulong bitmasks used for tracking emitted parameters (f.Emitted) and candidate keys (f.Cand) will suffer from bit collisions. Skip compiling constraints for tools with more than 64 parameters to avoid undefined behavior.

            foreach (var t in tools)
            {
                if (t.Arguments.Open) continue;                 // unconstrained body — skip
                var compiled = ToolSchemaCompiler.TryCompileObject(t.Arguments);
                if (compiled is not null && compiled.Count <= 64) _tools[t.Name] = compiled;
            }

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Already guarded upstream: ToolSchemaObject forces Open = true when Properties.Count > 64 (ToolSchema.cs:85), and ToolSchemaCompiler.TryCompileObject returns null for an open / >64-property object (ToolSchemaCompiler.cs:74). So a CompiledObject reaching _tools always has Count <= 64 — the f.Emitted / f.Cand bitmasks can never collide. Same upstream cap the JSON/Gemma siblings rely on; the redundant compiled.Count <= 64 check would diverge from them with no reachable effect.

int i = 0;
while (i < bytes.Length)
{
if (_depth == 0) return false; // closed the function but bytes remain

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

In FeedRawBytes, if a token contains </function> followed by trailing characters (such as a newline \n or space), matching </function> will set _depth to 0. The next iteration of the loop will see _depth == 0 while bytes still remain, returning false. This causes Accept to treat the token as rejected, disabling the constraint and printing an "accept-divergence" warning, even though the token was completely valid. Since any bytes after </function> are outside the tool call and thus unconstrained, return true instead of false when _depth == 0 to gracefully allow trailing bytes in merged tokens.

            if (_depth == 0) return true;                  // closed the function; trailing bytes are unconstrained

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This does not manifest under the constraint, and the behavior matches the JSON/Gemma siblics intentionally. A token that completes </function> and carries trailing bytes (e.g. the merged >\n the model would otherwise emit at the close) is masked out: SimulateToken runs this same FeedRawBytes, hits _depth == 0 with bytes remaining, returns false, and the token is set to -inf. So the model is forced to emit the close token ending exactly at > (then the newline separately), and Accept never sees trailing-after-close → no accept-divergence warning. Verified on a real GPU run of Qwen3-Coder-30B-A3B (CUDA hybrid, --cpu-moe): the constrained generation is clean (no warnings), producing the correctly-constrained call. JsonToolArgumentConstraint/GemmaToolArgumentConstraint return false here for the identical reason (their close byte } / <tool_call|> with trailing is likewise masked out), so keeping this consistent. Returning true would also silently swallow a second <function=…> merged into the same token, so false is the safer default.

… review)

Gemini review: `_toolCallCloseId = -1` was immediately overwritten by
`TryGetSpecialToken`'s out-param, which is set to 0 (not left at -1) on a lookup
miss. If a vocabulary lacks </tool_call>, the disarm check would then match token
id 0 (pad/unk) and spuriously disarm the constraint. Set it from the return value
so a missing marker stays -1 and never matches a real token.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LKvjcWw1gH5HyXAwsK3quP
@pekkah
pekkah merged commit 2c27223 into master Jun 24, 2026
1 check passed
@pekkah
pekkah deleted the feat/383-qwen-coder-xml-tool-grammar branch June 24, 2026 12:05
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.

Tool-arg grammar: Qwen3-Coder XML <function=…> argument constraint (#374/#376 follow-up)

1 participant