Skip to content

feat(qwen35): render tool calls in native XML, not legacy Hermes JSON#513

Closed
nwoolmer wants to merge 2 commits into
masterfrom
feat/qwen35-native-xml-toolcalls
Closed

feat(qwen35): render tool calls in native XML, not legacy Hermes JSON#513
nwoolmer wants to merge 2 commits into
masterfrom
feat/qwen35-native-xml-toolcalls

Conversation

@nwoolmer

Copy link
Copy Markdown
Collaborator

Follow-on to #508 (the dropped-outer-brace grammar fix). That PR made the Hermes-JSON grammar correct; this PR stops forcing qwen3.5/3.6 into Hermes JSON at all, since those models are XML-native (<function=NAME><parameter=ARG>). Emitting a JSON example / re-rendering history as JSON fights the model's training and is what induced the drift in the first place.

Changes

  1. Per-model grammar gating (cli/index.ts, registry.json): resolveToolGrammar() picks the tool format from the registry default_tool_format (added "hermes" to the 4 carnice cards) or a hermes/carnice heuristic. Non-Hermes qwen defaults the Hermes JSON grammar OFF → the model emits its native XML; Hermes-format models (carnice) keep it ON. Applied in serve() too.
  2. CLI tools-block example → native XML (Part A): for qwen3_5/qwen3_5_moe with the grammar off, the injected example is <tool_call><function=><parameter=> instead of the Hermes JSON one. Keyed on the live grammar env so the example always matches what the model will actually emit.
  3. Cache-miss history re-render → native XML (Part B): build_cached_history's ChatScaffold miss branch (the HIPFIRE_JINJA_CHAT=0 hand-rolled fallback) gains a ToolCallRender param; grammar-off qwen re-renders a historical tool_call as native XML via render_tool_call_qwen_xml(). Both call sites (LCP planning in plan_prompt_cache + the actual render) read the same env-derived format so their token streams stay byte-consistent.

Note on scope

On the default path qwen3.6 already renders through its bundled chat_template (jinja_active is on by default), which is native XML for both the tools-block and history — so the real forcing signal was the grammar (now off for non-Hermes qwen). Parts A/B additionally harden the HIPFIRE_JINJA_CHAT=0 hand-rolled fallback so it's consistent too.

Testing

  • prompt_frame red→green tests for both XML and Hermes-JSON history render; full hipfire-runtime lib (315) + hipfire-arch-qwen35 grammar (75) green.
  • Live on qwen3.6-27b.mq4: single-turn 5/5 angle-bracket-heavy writes clean (heredoc <<, #include <stdio.h>, <vector>, HTML, a < b); multi-turn (history tool_call + tool result → new call) clean; the HIPFIRE_JINJA_CHAT=0 cache path exercised (eligible=true, Assistant tool_call miss → XML render, lcp=256 forward extension).

🤖 Generated with Claude Code

qwen3.5/3.6 are XML-native (`<function=NAME><parameter=ARG>`); the write
path was still emitting Hermes JSON in three places, fighting the model's
training and (with the grammar on) inducing the dropped-outer-brace bug.

- Per-model grammar gating (cli): resolveToolGrammar() picks the tool
  format from the registry `default_tool_format` (or a hermes/carnice
  heuristic). Non-Hermes qwen defaults grammar OFF -> model emits its
  native XML; Hermes models (carnice) keep grammar ON/JSON. Applied in
  serve() too. registry.json marks the 4 carnice cards hermes.
- CLI tools-block example (Part A): for qwen3_5/qwen3_5_moe with grammar
  off, show the native `<tool_call><function=><parameter=>` example
  instead of the JSON one. Keyed on the live grammar env so the example
  always matches what the model will emit.
- Cache-miss history re-render (Part B): build_cached_history's
  ChatScaffold miss branch (HIPFIRE_JINJA_CHAT=0 fallback) gains a
  ToolCallRender param; grammar-off qwen re-renders a historical
  tool_call as native XML via render_tool_call_qwen_xml(). Both call
  sites (plan_prompt_cache LCP planning + the render) read the same
  env-derived format so their token streams stay byte-consistent.

Note: on the DEFAULT path qwen3.6 renders through its bundled
chat_template (jinja_active), which is already native XML for tools-block
and history; Part A/B harden the HIPFIRE_JINJA_CHAT=0 hand-rolled
fallback. The real forcing signal was the grammar, now off by default
for non-Hermes qwen.

Tests: prompt_frame red->green for XML and JSON history render; full
hipfire-runtime lib (315) + qwen35 grammar (75) green. Validated live on
qwen3.6-27b.mq4: single-turn 5/5 angle-bracket-heavy writes clean,
multi-turn clean, and the jinja=0 cache path exercised (eligible=true,
Assistant tool_call miss -> XML render, lcp=256 forward extension).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown

Claude finished @nwoolmer's task in 3m 4s —— View job


Review complete

  • Gather context (PR diff, changed files)
  • Review cli/index.ts (resolveToolGrammar, tools-block example)
  • Review cli/registry.json (default_tool_format additions)
  • Review crates/hipfire-runtime/examples/daemon.rs
  • Review crates/hipfire-runtime/src/prompt_frame.rs (ToolCallRender, XML render)
  • Post final review

Verdict: Minor — one real gate-coverage gap, rest is nits.

The core logic is clean and consistent: resolveToolGrammar() (cli/index.ts:1431-1443) and qwen_history_tool_render() (daemon.rs:3641-3647) both key off the same "not explicitly 0" test, so tool-call render and grammar enforcement stay in lockstep at both build_cached_history call sites (plan_prompt_cache LCP planning + the actual render). Tests (prompt_frame.rs:1700-1786) correctly assert XML-vs-JSON exclusivity in both directions.

Findings:

  • Gate coverage gapscripts/coherence-gate.sh:44 and scripts/agentic-gate.sh:158 spawn ./target/release/examples/daemon directly, never setting HIPFIRE_QWEN35_GRAMMAR. Since the new native-XML default is only applied by the CLI's applyConfigEnv (cli/index.ts:1456-1458) before it spawns/talks to the daemon, the raw daemon still defaults grammar_enabled = true (Hermes JSON) at daemon.rs:4126/5916/8456 when launched directly. That means the behavior this PR actually ships to users (native XML for non-Hermes qwen3.5/3.6 via hipfire serve) is not exercised by the existing gate scripts — they keep validating the old Hermes-JSON path. The PR's "Live on qwen3.6-27b.mq4" testing presumably went through CLI serve, not through these gates; worth confirming, and worth a follow-up to make the gates model the CLI's env resolution (or explicitly set HIPFIRE_QWEN35_GRAMMAR=0) so a future regression in the new default path gets caught.
  • Minor: render_tool_call_qwen_xml (prompt_frame.rs:384-402) silently emits <function=NAME></function> with no parameters if tc.arguments isn't a JSON object (the HermesJson branch doesn't lose data in that case since it embeds the raw Value). Low risk since tool-call arguments should always parse as an object, but worth a debug_assert or comment if that invariant isn't enforced upstream.
  • Minor (self-healing, noted only for awareness): the XML render doesn't escape parameter values, so a code-writing tool call whose argument literally contains </parameter> or </function> would desync the historical token stream on cache-miss replay — but this is documented as an accepted degrade-to-miss case (daemon.rs:3652-3660, LCP divergence forces a reset), and mirrors what the model's own upstream chat_template does, so not a regression.

cli/registry.json's new default_tool_format fields are additive and the CLI fallback heuristics (regex on model tag / desc) already covered the same carnice cards before this PR, so no behavior-breaking risk if registry/v1.json lags behind before the next registry-gen refresh.

GPU gates still required: scripts/coherence-gate.sh (qwen3.5/3.6 rows) recommended before merge given the tool-call format flip, but per the gap above, run it with HIPFIRE_QWEN35_GRAMMAR=0 explicitly (or via hipfire serve) to actually cover the new default path — as shipped, the plain gate invocation won't. scripts/serve-multiturn-gate.sh is also relevant since Part B changes the prefix-cache render used for cross-turn LCP byte-consistency (plan_prompt_cache + the actual render both read qwen_history_tool_render()) — worth a run to confirm no state-bleed across a multi-turn tool-call session.

@Kaden-Schutt

Copy link
Copy Markdown
Owner

/gate

@Kaden-Schutt

Copy link
Copy Markdown
Owner

/gate

@Kaden-Schutt

Copy link
Copy Markdown
Owner

/gate

@Kaden-Schutt

Copy link
Copy Markdown
Owner

/gate

5 similar comments
@Kaden-Schutt

Copy link
Copy Markdown
Owner

/gate

@Kaden-Schutt

Copy link
Copy Markdown
Owner

/gate

@Kaden-Schutt

Copy link
Copy Markdown
Owner

/gate

@Kaden-Schutt

Copy link
Copy Markdown
Owner

/gate

@Kaden-Schutt

Copy link
Copy Markdown
Owner

/gate

@github-actions

Copy link
Copy Markdown

GPU PR Gate (Tier 3)

arch verdict
gfx1100 REJECT
gfx1151 REJECT
gfx1201 PASS
behavior REJECT

Bill of Debt (blocking):

  • gfx1100 probe_unavailable — required box never produced a probe result; this arch is required per validated_dispatch.json.required_archs and cannot be silently skipped.
  • gfx1151 probe_unavailable — same failure mode as gfx1100; required box unreachable.
  • behavior qwen35-native-xml-toolcall — plain (non-carnice) qwen3.6 model did not emit native XML tool calls (<tool_call>\n<function=write_file>\n<parameter=...>) on the angle-bracket code-writing prompt; this is the PR's own stated regression trigger.
  • behavior qwen35-history-cache-miss-xml-replay — under HIPFIRE_JINJA_CHAT=0, the turn-1 historical tool_call re-rendered on cache-miss did not stay native XML and/or the turn-2 follow-up tool call was not coherent.
  • behavior qwen35-grammar-explicit-override — explicit HIPFIRE_QWEN35_GRAMMAR=1 override did not force Hermes JSON tool-call rendering for the plain qwen3.6 model, indicating applyConfigEnv's === undefined guard may be clobbering a user override.

Actionable next steps:

  1. Restore probe availability on gfx1100/gfx1151 (hipx box) and re-run — these are required archs and cannot pass without a probe result.
  2. Re-verify qwen_history_tool_render() and render_tool_call_qwen_xml actually produce XML-delimited tool calls (not Hermes JSON) for non-carnice qwen3.6 models across both daemon.rs call sites (plan_prompt_cache and the real render), per review.md's LCP byte-consistency note.
  3. Re-check resolveToolGrammar's default_tool_format/regex fallback and applyConfigEnv's === undefined guard for the explicit-override precedence bug flagged by the failing hiptrx behavior test.

This PR is blocked until the above are resolved and all three required archs (gfx1100, gfx1151, gfx1201) plus all three behavior tests pass.

…t the CLI

The native-XML-vs-Hermes decision lived only in the CLI's applyConfigEnv,
which sets HIPFIRE_QWEN35_GRAMMAR before spawning the daemon. Anything that
launches examples/daemon DIRECTLY -- the GPU behavior gate, coherence-gate.sh,
agentic-gate.sh -- left the env unset, and all four daemon sites defaulted
grammar ON (Hermes JSON). So the native-XML path this PR ships (plain
qwen3.5/3.6) was never exercised by the gates, and the GPU behavior gate
REJECTed all three tool-call checks (native-xml-toolcall,
history-cache-miss-xml-replay, grammar-explicit-override).

Move the default into the lib (prompt_frame::qwen35_grammar_on /
qwen35_history_render) and key it on the model path so a daemon-direct launch
resolves correctly: explicit HIPFIRE_QWEN35_GRAMMAR wins ("0" = off
kill-switch, any other set value = on); otherwise carnice (Hermes-native)
=> ON, plain qwen3.5/3.6 (XML-native) => OFF. Wire it at all four grammar /
history-render sites in the daemon (generate, generate_multi, generate_dflash,
plan_prompt_cache). Mirrors the CLI's resolveToolGrammar exactly, so `serve`
and direct-spawn agree -- which also closes the review's gate-coverage gap
(the gates now exercise the shipped native-XML path automatically).

Also addresses the two review nits on render_tool_call_qwen_xml: a debug_assert
that arguments is a JSON object, and a comment documenting the accepted
degrade-to-cache-miss for unescaped parameter values.

Validated: 5 new prompt_frame unit tests (grammar default/override/history);
and end-to-end on gfx1151 via the daemon batch protocol -- plain qwen3.6-27b
emits native <function=write_file> XML with the env unset, and Hermes
{"name":"write_file"} JSON with HIPFIRE_QWEN35_GRAMMAR=1.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nwoolmer

Copy link
Copy Markdown
Collaborator Author

/gate

Kaden-Schutt added a commit that referenced this pull request Jul 16, 2026
Parse Qwen-native XML at the shared runtime boundary so daemon tool_calls events, finish_reason, and assistant-cache fingerprints agree with the Bun serve parser. Resolve model-derived grammar per request instead of persisting the prewarm model's choice in process.env; explicit user overrides still win.

Observed on hiptrx gfx1201 device 3 with qwen3.6-35b-a3b.mq4r: rebased PR #513 emitted a correct XML write_file block, but /v1/chat/completions returned finish_reason=stop. Master and beta returned finish_reason=tool_calls. The serve_harness five-prompt floor remained byte-identical between beta and PR #513.

Tests:
- cargo test -p hipfire-runtime emit_text --locked
- cargo check -p hipfire-runtime --example daemon --locked
- bun test cli/serve_ux_parse.test.ts cli/parse_tool_calls.test.ts
- git diff --check
Kaden-Schutt added a commit that referenced this pull request Jul 16, 2026
Rebased contributor head 25de968 onto codex/beta, then fixed the two validation blockers found on the user-facing Bun serve path: native-XML calls now produce daemon structured events/finish_reason=tool_calls, and grammar selection is resolved per requested model without prewarm-model bleed.

hiptrx gfx1201, ROCR_VISIBLE_DEVICES=3
model: /home/kaden/.hipfire/models/qwen3.6-35b-a3b.mq4r
model md5: 2a2be837d58ef8e57f8cf2b907050770
request md5: 1c4046d4f2119b4e7f05124a510197e0

A/B:
- beta: write_file parsed, finish_reason=tool_calls
- rebased #513: write_file parsed by CLI fallback, finish_reason=stop (rejected)
- reconciled #513: native XML parsed at runtime, finish_reason=tool_calls (pass)
- serve_harness battery: beta and reconciled #513 byte-identical, 0 empty, 0 attractors

Redline staging-base gate:
- stable decode capture: 733 launches, 23 kernels, hash 3318ffca3daf2338
- 23 AQL contracts
- 15-position retained-PM4 shadow parity bit-exact

Local gates:
- cargo test -p hipfire-runtime emit_text --locked
- cargo check -p hipfire-runtime --example daemon --locked
- bun test cli/serve_ux_parse.test.ts cli/parse_tool_calls.test.ts
- git diff --check
Kaden-Schutt added a commit that referenced this pull request Jul 16, 2026
Carry PR #513's per-model default_tool_format through registry/v1 so the production dynamic-registry path does not discard Carnice's Hermes selection. Validate the field in both the Bun loader and registry generator.

Tests: bun test cli/registry_loader.test.ts cli/serve_ux_parse.test.ts cli/parse_tool_calls.test.ts (79 pass)

Copy link
Copy Markdown
Owner

Folded into #534 as a cleanly rebased/cherry-picked delta on beta for v0.3.0, preserving the newer Redline and modular-runtime line. Closing this source PR in favor of the consolidated release PR. Thank you for the contribution.

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.

2 participants