fix(qwen35-spec): parse tool calls whenever the request carries tools, independent of the grammar switch - #727
Closed
alpineQ wants to merge 1 commit into
Closed
Conversation
…, independent of the grammar switch
16 tasks
6 tasks
Collaborator
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
On the speculative (DFlash/MTP) route, tool definitions reached the emitter only when the grammar matcher was armed, so with grammar off the tool-call parser never ran and a well-formed Qwen XML call was streamed to the client as plain text with
finish_reason: stopand nocalls; tools now always reach the emitter andSpecEmitCtx.grammarcarries the matcher switch separately.What was wrong
generate_dflash(crates/hipfire-generate/src/qwen.rs) built the emit request withQwen35Emit::newderives bothtool_protocol_enabled(arms the XML/JSON tool-call parser) and the grammarToolSchemalist fromctx.tools. Coupling them meant that wheneverqwen35_grammar_enabledsaid no, the parser was disabled too. Measured with an agent harness againstqwen3.8-27b(read_filetool, 33k context):with the model's
text_previewholding a correct<tool_call><function=read_file>…</function></tool_call>every time. For an agent client this is worse than an error: chatter comes back where a call was expected.What changes
SpecEmitCtx/SpecEmitRequestgaingrammar: bool.Qwen35Emitbuilds the grammarToolSchemalist only whengrammaris set; the tool-call parser followstoolsalone.generate_dflashalways forwardstools;grammarcarries the arch grammar switch. Other constructors (dense.rs, daemon slots, tests) passgrammar: true, i.e. unchanged behaviour.[qwen35-spec] …) instead of silently latchingrouter_malformed.Which surface(s) does this touch?
crates/hipfire-generate(qwen.rs,dense.rs,common.rs), runtimespec.rs/emit_text.rs, daemonslots.rshipfire-arch-qwen35(spec_emit.rs)crates/hipfire-quantizeTest plan
./scripts/no-gpu-ci.sh: Rust check + no-GPU unit tests + env/docs drift check pass; the Python stage reports 838 passed / 6 failed intests/test_mq4c_repack.py, and those 6 fail identically on untouchedmasterin this environment (NixOS: no/bin/bash, and the test module resolves a differentmq4c_repackthan the script) — not touched by this changecargo build --releaseclean (--workspace --all-targets --locked)cargo test --lib --workspacepasses;hipfire-generateintegration tests (qwen_dflash_semantic_terminal_tests,ds4_malformed_terminal_tests) pass with the new fieldserve_harness.pybattery + chain onqwen3.6:27b(registry fixture) — JSON attachedread_filetool through the daemon JSONL (qwen3.8-27b.mq4+.mtpsidecar, MTP K=3, greedy, thinking off): turns 0 and 2 return a parsedcallsentry (previouslycalls: 0with the XML intext), the tool-result turn continues from the prefix cache. Turn 2 cold-prefills — the MTP drafter has no checkpoint ring on master, unrelated to this change.It defines amainfunction that prints the string "hello" to standard output.⏎It defineslocal serve_harness JSON
Hardware validation request (optional)
{ "routes": [ {"mode": "battery", "tag": "qwen3.6:27b"}, {"mode": "chain", "tag": "qwen3.6:27b"} ], "claim": "tool-free serve on the speculative route is unchanged; a request carrying tools now yields parsed calls on that route regardless of the grammar switch." }Related: #220 (tools calling), #646 Track A (tool parity on the slot path).
Architecture-trait change?
No.