From 1fdcac28fcf2866c3d2d0e6dc2eb27a53f9edd17 Mon Sep 17 00:00:00 2001 From: Rob Ellis <325508215+bhncat@users.noreply.github.com> Date: Sun, 6 Sep 2026 02:04:42 -0400 Subject: [PATCH] fix(qwen35): parse XML tool calls when grammar is off Qwen3.8 (and other XML-native Qwen3.5/3.6 cards) default HIPFIRE_QWEN35_GRAMMAR off. The DFlash path treated that as "do not pass tools into the emitter," which also disabled ToolOutputRouter. The model still emitted native XML, but the HTTP API returned it as assistant content with finish_reason=stop and no message.tool_calls. OpenAI clients such as Hermes never executed the tools. Keep grammar off for those cards, but still forward tools so the parser runs. Constrained grammar stays a separate flag (enable_grammar). --- crates/hipfire-arch-qwen35/src/spec_emit.rs | 55 +++++++++++++++++++ crates/hipfire-daemon/src/slots.rs | 1 + crates/hipfire-generate/src/common.rs | 5 +- crates/hipfire-generate/src/dense.rs | 1 + crates/hipfire-generate/src/qwen.rs | 14 ++--- .../tests/ds4_malformed_terminal_tests.rs | 1 + .../qwen_dflash_semantic_terminal_tests.rs | 5 ++ crates/hipfire-runtime/src/spec.rs | 10 +++- 8 files changed, 81 insertions(+), 11 deletions(-) diff --git a/crates/hipfire-arch-qwen35/src/spec_emit.rs b/crates/hipfire-arch-qwen35/src/spec_emit.rs index 4a37ebb46..d3ad98bb3 100644 --- a/crates/hipfire-arch-qwen35/src/spec_emit.rs +++ b/crates/hipfire-arch-qwen35/src/spec_emit.rs @@ -94,6 +94,7 @@ impl<'a> Qwen35Emit<'a> { let tool_protocol_enabled = ctx.tools.is_some(); let tool_schemas: Vec = ctx .tools + .filter(|_| ctx.enable_grammar) .map(|arr| { arr.iter() .filter_map(|t| { @@ -613,6 +614,7 @@ mod tests { eos: 9, im_end: Some(1), tools: Some(&[]), + enable_grammar: true, stop: Vec::new(), max_think: 0, max_tokens: 256, @@ -709,6 +711,57 @@ mod tests { assert_eq!(calls[0].name, "get_weather"); } + #[test] + fn xml_tool_call_held_when_tools_present_and_grammar_off() { + // Qwen3.5/3.8 XML-native: grammar stays off, but tools must still + // enable ToolOutputRouter or `` leaks as assistant text. + let tok = test_tokenizer(); + let tools = [serde_json::json!({ + "type": "function", + "function": { + "name": "get_time", + "parameters": {"type": "object", "properties": {}} + } + })]; + let mut emit = Qwen35Emit::from_ctx(SpecEmitCtx { + tokenizer: &tok, + eos: 9, + im_end: Some(1), + tools: Some(&tools), + enable_grammar: false, + stop: Vec::new(), + max_think: 0, + max_tokens: 256, + assistant_prefix: AssistantPrefix::Plain, + think_mode: hipfire_runtime::prompt_frame::ThinkMode::NonThink, + decoded_vocab: None, + }); + let text = "\n\n\n"; + let ids = tok.encode(text); + assert!(!ids.is_empty()); + let mut stream = Vec::new(); + let mut first = true; + for id in &ids { + let outcome = if first { + first = false; + emit.begin(*id) + } else { + emit.observe(*id) + }; + stream.extend(outcome.events); + if outcome.stop.is_some() { + break; + } + } + let finish = emit.finish(); + assert!(!tokens_text(&stream).contains("")); + assert_eq!(finish.finish_reason, "tool_calls"); + assert_eq!(finish.tool_calls, 1); + let calls = held_calls(&finish); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].name, "get_time"); + } + #[test] fn multiple_calls_and_surrounding_prose() { let body = format!( @@ -837,6 +890,7 @@ mod tests { eos: 9, im_end: Some(1), tools: None, + enable_grammar: false, stop: vec![first_text.clone()], max_think: 0, max_tokens: 256, @@ -865,6 +919,7 @@ mod tests { eos: 9, im_end: Some(1), tools: None, + enable_grammar: false, stop: vec!["STOP".to_string()], max_think: 0, max_tokens: 256, diff --git a/crates/hipfire-daemon/src/slots.rs b/crates/hipfire-daemon/src/slots.rs index 9daa7f6ef..6b57fb45b 100644 --- a/crates/hipfire-daemon/src/slots.rs +++ b/crates/hipfire-daemon/src/slots.rs @@ -640,6 +640,7 @@ impl SlotBackend { eos: self.tokenizer.eos_id, im_end: self.tokenizer.special_token_id("<|im_end|>"), tools: None, + enable_grammar: false, stop: Vec::new(), max_think: 0, max_tokens, diff --git a/crates/hipfire-generate/src/common.rs b/crates/hipfire-generate/src/common.rs index 79e1ec92c..e7fa18700 100644 --- a/crates/hipfire-generate/src/common.rs +++ b/crates/hipfire-generate/src/common.rs @@ -1253,8 +1253,11 @@ pub fn fail_closed_epilogue_after_sync( /// + the slot's eos + the tokenizer and calls `carrier.make_spec_emitter`. pub struct SpecEmitRequest { pub im_end: Option, - /// Raw tool definitions (OpenAI-shape JSON); `None`/empty ⇒ no tool grammar. + /// Raw tool definitions (OpenAI-shape JSON); `None` ⇒ no tool-call parser. pub tools: Option>, + /// Constrained tool grammar. False on XML-native Qwen3.5/3.8 (parser still + /// runs when `tools` is `Some`). + pub enable_grammar: bool, pub stop: Vec, pub max_think: usize, pub assistant_prefix: hipfire_runtime::prompt_frame::AssistantPrefix, diff --git a/crates/hipfire-generate/src/dense.rs b/crates/hipfire-generate/src/dense.rs index 96f35c92a..f766e2c77 100644 --- a/crates/hipfire-generate/src/dense.rs +++ b/crates/hipfire-generate/src/dense.rs @@ -501,6 +501,7 @@ pub fn generate_deepseek4_spec( SpecEmitRequest { im_end: None, tools: tools.map(|t| t.to_vec()), + enable_grammar: tools.is_some(), stop: Vec::new(), max_think: 0, assistant_prefix: hipfire_runtime::prompt_frame::AssistantPrefix::Plain, diff --git a/crates/hipfire-generate/src/qwen.rs b/crates/hipfire-generate/src/qwen.rs index f91daf1ae..da832ddba 100644 --- a/crates/hipfire-generate/src/qwen.rs +++ b/crates/hipfire-generate/src/qwen.rs @@ -2274,17 +2274,15 @@ pub fn generate_dflash( // qwen35 enforces tool-call grammar POST-acceptance inside the emitter // (`Qwen35Emit::observe`); the emitter now extracts its own `ToolSchema` // list from the raw tool JSON inside `make_spec_emitter`. This wrapper only - // honors the `HIPFIRE_QWEN35_GRAMMAR=0` kill-switch by withholding `tools` - // (⇒ empty schema ⇒ grammar inactive). + // honors the `HIPFIRE_QWEN35_GRAMMAR=0` kill-switch by setting + // `enable_grammar=false` (empty schema ⇒ matcher inactive). Tools still + // reach SpecEmit so ToolOutputRouter parses native XML; withholding them + // used to leak `` as assistant content (Hermes never executed). let grammar_enabled = hipfire_runtime::prompt_frame::qwen35_grammar_on( hipfire_config::developer_var("HIPFIRE_QWEN35_GRAMMAR").ok().as_deref(), &m.model_path, ); - let emit_tools: Option> = if grammar_enabled { - tools.map(|t| t.to_vec()) - } else { - None - }; + let emit_tools: Option> = tools.map(|t| t.to_vec()); // The decode core (slot guard, prefill, accept-window loop, bake, finish) is // the arch-generic `generate_spec`. This wrapper owns the qwen35/llama-specific @@ -2337,6 +2335,7 @@ pub fn generate_dflash( SpecEmitRequest { im_end: im_end_token, tools: emit_tools, + enable_grammar: grammar_enabled, stop: stop.to_vec(), max_think: max_think_tokens, assistant_prefix: spec_assistant_prefix(started_in_think), @@ -3100,6 +3099,7 @@ pub fn generate_spec( eos: slot.eos_token(), im_end: emit_req.im_end, tools: emit_req.tools.as_deref(), + enable_grammar: emit_req.enable_grammar, stop: emit_req.stop, max_think: emit_req.max_think, max_tokens, diff --git a/crates/hipfire-generate/tests/ds4_malformed_terminal_tests.rs b/crates/hipfire-generate/tests/ds4_malformed_terminal_tests.rs index 6c6133e86..3bc0d57dd 100644 --- a/crates/hipfire-generate/tests/ds4_malformed_terminal_tests.rs +++ b/crates/hipfire-generate/tests/ds4_malformed_terminal_tests.rs @@ -671,6 +671,7 @@ use hipfire_engine::terminal::{set_active_attempt_id, ClientTerminalDecision}; eos: 7, im_end: None, tools: None, + enable_grammar: false, stop: Vec::new(), max_think: 0, max_tokens: 16, diff --git a/crates/hipfire-generate/tests/qwen_dflash_semantic_terminal_tests.rs b/crates/hipfire-generate/tests/qwen_dflash_semantic_terminal_tests.rs index 38a8e59d8..d58bd5337 100644 --- a/crates/hipfire-generate/tests/qwen_dflash_semantic_terminal_tests.rs +++ b/crates/hipfire-generate/tests/qwen_dflash_semantic_terminal_tests.rs @@ -140,6 +140,7 @@ use hipfire_runtime::emit_text::extract_tool_calls_from_text; eos: 9, im_end: Some(1), tools: Some(&[]), + enable_grammar: true, stop: Vec::new(), max_think: 0, max_tokens: 256, @@ -820,6 +821,7 @@ use hipfire_runtime::emit_text::extract_tool_calls_from_text; eos: 9, im_end: Some(1), tools: None, + enable_grammar: false, stop: Vec::new(), max_think: 1, max_tokens: 256, @@ -2418,6 +2420,7 @@ use hipfire_runtime::emit_text::extract_tool_calls_from_text; eos: 9, im_end: Some(1), tools: None, + enable_grammar: false, stop: vec![first_text.clone()], max_think: 0, max_tokens: 256, @@ -2519,6 +2522,7 @@ use hipfire_runtime::emit_text::extract_tool_calls_from_text; eos: 9, im_end: Some(1), tools: None, + enable_grammar: false, stop: vec![stop_text.clone()], max_think: 0, max_tokens: 256, @@ -2699,6 +2703,7 @@ use hipfire_runtime::emit_text::extract_tool_calls_from_text; eos: 9, im_end: Some(1), tools: None, + enable_grammar: false, stop: vec![first_text.clone()], max_think: 0, max_tokens: 1, diff --git a/crates/hipfire-runtime/src/spec.rs b/crates/hipfire-runtime/src/spec.rs index 1499f231a..d05285e52 100644 --- a/crates/hipfire-runtime/src/spec.rs +++ b/crates/hipfire-runtime/src/spec.rs @@ -1392,10 +1392,14 @@ pub struct SpecEmitCtx<'a> { pub eos: u32, /// Secondary terminator (e.g. `<|im_end|>`), if the arch uses one. pub im_end: Option, - /// Raw tool definitions from the request (OpenAI-shape JSON). Each carrier - /// extracts its own grammar `ToolSchema` from these; `None`/empty ⇒ no - /// tool-call grammar. + /// Raw tool definitions from the request (OpenAI-shape JSON). `Some` enables + /// the tool-call *parser* (XML or JSON) even when constrained grammar is off. + /// `None` ⇒ tool-looking text is ordinary assistant content. pub tools: Option<&'a [serde_json::Value]>, + /// Constrained tool-call grammar. Independent of [`Self::tools`]: Qwen3.5/3.8 + /// XML-native cards keep this false (default `qwen35_grammar_on`) so the + /// matcher does not force Hermes-JSON, but still parse `` XML. + pub enable_grammar: bool, /// User stop sequences matched against the decoded suffix. pub stop: Vec, /// `max_think_tokens` budget (0 ⇒ no think force-close).