From 6b8fc20a9a01d6920c75de30001e2eb512bb44e0 Mon Sep 17 00:00:00 2001 From: key4ng Date: Thu, 9 Jul 2026 13:05:05 -0700 Subject: [PATCH 1/3] fix(tool_parser): treat qwen_xml tool-call arg values literally (#1888) The qwen_xml parser HTML-unescaped every tool-call argument value, so any value containing an entity-like substring (&, <, ', ...) was silently mutated on parse. The Qwen3-Coder XML format is not HTML-escaped on render (the chat template emits values via `| tojson | safe` / `| string`), so parsing must not unescape it. vLLM's Qwen3CoderToolParser, Qwen-Agent, and Qwen's chat template all pass argument values through verbatim. Drop the html_unescape call from safe_val and coerce_value (keeping the JSON / schema-type coercion), delete the now-dead html_unescape helper, and invert the unit/integration tests that asserted decoding. Add a golden conformance test asserting entity-containing values round-trip unchanged on both the inference and string-schema paths. Signed-off-by: key4ng --- crates/tool_parser/src/parsers/qwen_xml.rs | 180 ++++++------------ .../tool_parser/tests/tool_parser_qwen_xml.rs | 34 ++-- 2 files changed, 80 insertions(+), 134 deletions(-) diff --git a/crates/tool_parser/src/parsers/qwen_xml.rs b/crates/tool_parser/src/parsers/qwen_xml.rs index 60381619c5..1ddc4643a4 100644 --- a/crates/tool_parser/src/parsers/qwen_xml.rs +++ b/crates/tool_parser/src/parsers/qwen_xml.rs @@ -54,95 +54,26 @@ pub struct QwenXmlParser { xml_param_pattern: Regex, } -/// Decode HTML entities in a string (equivalent to Python's html.unescape) +/// Parse a raw parameter value, similar to Python's `_safe_val`. /// -/// Handles common HTML entities like & < > " ' and numeric entities -fn html_unescape(s: &str) -> String { - let mut result = String::with_capacity(s.len()); - let mut chars = s.chars().peekable(); - - while let Some(c) = chars.next() { - if c == '&' { - let mut entity = String::new(); - let mut consumed_semicolon = false; - while let Some(&next) = chars.peek() { - if next == ';' { - chars.next(); - consumed_semicolon = true; - break; - } - if next.is_alphanumeric() || next == '#' { - // Safe: peek() returned Some, so next() will too - if let Some(ch) = chars.next() { - entity.push(ch); - } - } else { - break; - } - } - - let decoded = match entity.as_str() { - "amp" => "&", - "lt" => "<", - "gt" => ">", - "quot" => "\"", - "apos" => "'", - "nbsp" => "\u{00A0}", - s if s.starts_with('#') => { - let num_str = &s[1..]; - let code_point = if num_str.starts_with('x') || num_str.starts_with('X') { - u32::from_str_radix(&num_str[1..], 16).ok() - } else { - num_str.parse::().ok() - }; - if let Some(cp) = code_point { - if let Some(ch) = char::from_u32(cp) { - result.push(ch); - continue; - } - } - // Invalid numeric entity, reconstruct original - result.push('&'); - result.push_str(&entity); - if consumed_semicolon { - result.push(';'); - } - continue; - } - _ => { - // Unknown entity, reconstruct original - result.push('&'); - result.push_str(&entity); - if consumed_semicolon { - result.push(';'); - } - continue; - } - }; - result.push_str(decoded); - } else { - result.push(c); - } - } - - result -} - -/// Parse a raw parameter value, similar to Python's _safe_val +/// Argument values are treated **literally** — no HTML-entity decoding. The +/// Qwen3-Coder XML tool format is not HTML-escaped on render (the chat template +/// emits values via `| tojson | safe` / `| string`), so parsing must not +/// unescape it. This matches vLLM's `Qwen3CoderToolParser`, Qwen-Agent, and +/// Qwen's chat template, all of which pass argument values through verbatim. /// -/// 1. Decode HTML entities -/// 2. Try to parse as JSON (numbers, booleans, null, objects, arrays) -/// 3. Fall back to string if JSON parsing fails +/// 1. Try to parse as JSON (numbers, booleans, null, objects, arrays) +/// 2. Fall back to string if JSON parsing fails fn safe_val(raw: &str) -> Value { - let unescaped = html_unescape(raw.trim()); + let trimmed = raw.trim(); // Try JSON parsing first - if let Ok(v) = serde_json::from_str::(&unescaped) { + if let Ok(v) = serde_json::from_str::(trimmed) { return v; } // Handle Python-style literals (True, False, None) - match unescaped.as_str() { + match trimmed { "True" => return Value::Bool(true), "False" => return Value::Bool(false), "None" => return Value::Null, @@ -150,14 +81,17 @@ fn safe_val(raw: &str) -> Value { } // Fall back to string - Value::String(unescaped) + Value::String(trimmed.to_string()) } /// Coerce an XML parameter value by its declared schema type, falling back to /// [`safe_val`] inference when the type is unknown. +/// +/// Values are treated literally; see [`safe_val`] for why the format is not +/// HTML-unescaped. fn coerce_value(raw: &str, declared_type: Option<&str>) -> Value { - let decoded = html_unescape(raw.trim()); - helpers::coerce_by_schema_type(&decoded, declared_type).unwrap_or_else(|| safe_val(raw)) + let trimmed = raw.trim(); + helpers::coerce_by_schema_type(trimmed, declared_type).unwrap_or_else(|| safe_val(raw)) } impl QwenXmlParser { @@ -550,40 +484,6 @@ mod tests { use super::*; - #[test] - fn test_html_unescape_basic() { - assert_eq!(html_unescape("&"), "&"); - assert_eq!(html_unescape("<"), "<"); - assert_eq!(html_unescape(">"), ">"); - assert_eq!(html_unescape("""), "\""); - assert_eq!(html_unescape("'"), "'"); - } - - #[test] - fn test_html_unescape_numeric() { - assert_eq!(html_unescape("<"), "<"); - assert_eq!(html_unescape("<"), "<"); - assert_eq!(html_unescape("<"), "<"); - } - - #[test] - fn test_html_unescape_mixed() { - assert_eq!( - html_unescape("Hello & World <tag>"), - "Hello & World " - ); - } - - #[test] - fn test_html_unescape_unknown() { - // Unknown entities with semicolon should be preserved as-is - assert_eq!(html_unescape("&unknown;"), "&unknown;"); - // Unterminated entities should NOT have semicolon added - assert_eq!(html_unescape("&foo bar"), "&foo bar"); - assert_eq!(html_unescape("&"), "&"); - assert_eq!(html_unescape("& "), "& "); - } - #[test] fn test_safe_val_json() { assert_eq!(safe_val("42"), Value::Number(42.into())); @@ -614,13 +514,21 @@ mod tests { assert_eq!(safe_val(" spaces "), Value::String("spaces".to_string())); } + // Values are treated literally: entity-like substrings must NOT be decoded + // (parity with vLLM's Qwen3CoderToolParser, which does no html.unescape). #[test] - fn test_safe_val_html_entities() { - assert_eq!(safe_val("<div>"), Value::String("
".to_string())); + fn test_safe_val_preserves_html_entities() { + assert_eq!( + safe_val("<div>"), + Value::String("<div>".to_string()) + ); assert_eq!( safe_val("Tom & Jerry"), - Value::String("Tom & Jerry".to_string()) + Value::String("Tom & Jerry".to_string()) ); + // Numeric/hex entities are likewise left untouched. + assert_eq!(safe_val("it's"), Value::String("it's".to_string())); + assert_eq!(safe_val("<"), Value::String("<".to_string())); } fn tool_with_props(props: Value) -> Vec { @@ -690,4 +598,36 @@ mod tests { "int param must coerce: {args}" ); } + + // Golden conformance test (regression guard for #1888): a tool argument + // whose value contains HTML entities must round-trip UNCHANGED, matching + // vLLM's `qwen3coder` parser. Covers both the schema-typed `string` path and + // the schema-less inference fallback. + #[tokio::test] + async fn test_arg_values_with_entities_roundtrip_unchanged() { + let literal = "Tom & Jerry <x> it's"; + + // Schema-less inference path (`safe_val`): not valid JSON -> stays a + // literal string with entities intact. + let text = format!( + "\n\n\ + \n{literal}\n\n\ + \n" + ); + let (_, calls) = QwenXmlParser::new().parse_complete(&text).await.unwrap(); + assert_eq!(calls.len(), 1); + let args: Value = serde_json::from_str(&calls[0].function.arguments).unwrap(); + assert_eq!(args["content"], Value::String(literal.to_string())); + + // Schema-typed `string` path (`coerce_value`): same literal value. + let tools = tool_with_props(serde_json::json!({ + "content": {"type": "string"}, + })); + let (_, calls) = QwenXmlParser::new() + .parse_complete_with_tools(&text, &tools) + .await + .unwrap(); + let args: Value = serde_json::from_str(&calls[0].function.arguments).unwrap(); + assert_eq!(args["content"], Value::String(literal.to_string())); + } } diff --git a/crates/tool_parser/tests/tool_parser_qwen_xml.rs b/crates/tool_parser/tests/tool_parser_qwen_xml.rs index 24ffb036f4..a0a121dac6 100644 --- a/crates/tool_parser/tests/tool_parser_qwen_xml.rs +++ b/crates/tool_parser/tests/tool_parser_qwen_xml.rs @@ -855,14 +855,17 @@ async fn test_qwen_xml_empty_parameter_value() { } // ============================================================================ -// HTML Entity and Python Literal Tests +// Literal Value (HTML-entity parity) and Python Literal Tests +// +// Argument values are treated literally — entity-like substrings are NOT +// decoded, matching vLLM's Qwen3CoderToolParser. Regression guard for #1888. // ============================================================================ #[tokio::test] -async fn test_qwen_xml_html_entity_decoding() { +async fn test_qwen_xml_preserves_html_entities() { let parser = QwenXmlParser::new(); - // Test HTML entities in parameter values + // Named HTML entities in parameter values must be preserved verbatim. let input = r" Tom & Jerry @@ -875,16 +878,16 @@ async fn test_qwen_xml_html_entity_decoding() { assert_eq!(tools.len(), 1); let args: serde_json::Value = serde_json::from_str(&tools[0].function.arguments).unwrap(); - assert_eq!(args["ampersand"], "Tom & Jerry"); - assert_eq!(args["comparison"], "5 < 10 && 10 > 5"); - assert_eq!(args["quotes"], "\"Hello\" & 'World'"); + assert_eq!(args["ampersand"], "Tom & Jerry"); + assert_eq!(args["comparison"], "5 < 10 && 10 > 5"); + assert_eq!(args["quotes"], ""Hello" & 'World'"); } #[tokio::test] -async fn test_qwen_xml_html_numeric_entities() { +async fn test_qwen_xml_preserves_numeric_entity_text() { let parser = QwenXmlParser::new(); - // Test numeric HTML entities + // Numeric/hex entity text must be preserved verbatim (not decoded). let input = r" <tag> @@ -896,8 +899,8 @@ async fn test_qwen_xml_html_numeric_entities() { assert_eq!(tools.len(), 1); let args: serde_json::Value = serde_json::from_str(&tools[0].function.arguments).unwrap(); - assert_eq!(args["decimal"], ""); - assert_eq!(args["hex"], ""); + assert_eq!(args["decimal"], "<tag>"); + assert_eq!(args["hex"], "<tag>"); } #[tokio::test] @@ -934,7 +937,7 @@ async fn test_qwen_xml_python_literals() { async fn test_qwen_xml_mixed_html_and_json() { let parser = QwenXmlParser::new(); - // Test HTML entities within JSON structures + // Entity-like text alongside a JSON-valued parameter. let input = r#" price < 100 && rating > 4 @@ -946,8 +949,11 @@ async fn test_qwen_xml_mixed_html_and_json() { assert_eq!(tools.len(), 1); let args: serde_json::Value = serde_json::from_str(&tools[0].function.arguments).unwrap(); - assert_eq!(args["query"], "price < 100 && rating > 4"); - // JSON with HTML entity inside - the entity gets decoded first, then JSON parsed + // Non-JSON value stays a literal string with entities intact. + assert_eq!(args["query"], "price < 100 && rating > 4"); + // JSON value parses as an object; entities inside its strings are preserved + // (JSON has no notion of HTML entities, so they are ordinary characters). assert!(args["config"].is_object()); - assert_eq!(args["config"]["operator"], "&&"); + assert_eq!(args["config"]["operator"], "&&"); + assert_eq!(args["config"]["escape"], true); } From 3319cd7126222aaec08d3b5bca4b9272607ec7e0 Mon Sep 17 00:00:00 2001 From: key4ng Date: Thu, 9 Jul 2026 16:09:37 -0700 Subject: [PATCH 2/3] docs(tool_parser): note qwen_xml literal-value parity with Qwen's official API Reframe the doc/test comments around the primary source: Qwen's own official API (DashScope) returns tool-call argument values with HTML entities intact, verified live on both Qwen3-Coder and Qwen3.5 (the two families SMG maps to the qwen_xml parser). vLLM, SGLang, and Qwen-Agent are cited as corroborating rather than as the canonical reference. No behavior change. Signed-off-by: key4ng --- crates/tool_parser/src/parsers/qwen_xml.rs | 19 +++++++++++-------- .../tool_parser/tests/tool_parser_qwen_xml.rs | 3 ++- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/crates/tool_parser/src/parsers/qwen_xml.rs b/crates/tool_parser/src/parsers/qwen_xml.rs index 1ddc4643a4..9ad6eb2f1f 100644 --- a/crates/tool_parser/src/parsers/qwen_xml.rs +++ b/crates/tool_parser/src/parsers/qwen_xml.rs @@ -56,11 +56,14 @@ pub struct QwenXmlParser { /// Parse a raw parameter value, similar to Python's `_safe_val`. /// -/// Argument values are treated **literally** — no HTML-entity decoding. The -/// Qwen3-Coder XML tool format is not HTML-escaped on render (the chat template -/// emits values via `| tojson | safe` / `| string`), so parsing must not -/// unescape it. This matches vLLM's `Qwen3CoderToolParser`, Qwen-Agent, and -/// Qwen's chat template, all of which pass argument values through verbatim. +/// Argument values are treated **literally** — no HTML-entity decoding. This +/// matches Qwen's own official API (DashScope), verified for both Qwen3-Coder +/// and Qwen3.5: a tool argument whose value contains `&`, `<`, `'` +/// is returned with those entities intact. The Qwen XML tool format is not +/// HTML-escaped on render either (the chat template emits values via +/// `| tojson | safe` / `| string`), so parsing must not unescape it. vLLM's +/// `Qwen3CoderToolParser`, SGLang's `qwen3_coder_detector`, and Qwen-Agent all +/// agree, passing argument values through verbatim. /// /// 1. Try to parse as JSON (numbers, booleans, null, objects, arrays) /// 2. Fall back to string if JSON parsing fails @@ -515,7 +518,7 @@ mod tests { } // Values are treated literally: entity-like substrings must NOT be decoded - // (parity with vLLM's Qwen3CoderToolParser, which does no html.unescape). + // (parity with Qwen's official API, which returns them intact). #[test] fn test_safe_val_preserves_html_entities() { assert_eq!( @@ -601,8 +604,8 @@ mod tests { // Golden conformance test (regression guard for #1888): a tool argument // whose value contains HTML entities must round-trip UNCHANGED, matching - // vLLM's `qwen3coder` parser. Covers both the schema-typed `string` path and - // the schema-less inference fallback. + // Qwen's official API (verified on Qwen3-Coder and Qwen3.5). Covers both the + // schema-typed `string` path and the schema-less inference fallback. #[tokio::test] async fn test_arg_values_with_entities_roundtrip_unchanged() { let literal = "Tom & Jerry <x> it's"; diff --git a/crates/tool_parser/tests/tool_parser_qwen_xml.rs b/crates/tool_parser/tests/tool_parser_qwen_xml.rs index a0a121dac6..564316a31f 100644 --- a/crates/tool_parser/tests/tool_parser_qwen_xml.rs +++ b/crates/tool_parser/tests/tool_parser_qwen_xml.rs @@ -858,7 +858,8 @@ async fn test_qwen_xml_empty_parameter_value() { // Literal Value (HTML-entity parity) and Python Literal Tests // // Argument values are treated literally — entity-like substrings are NOT -// decoded, matching vLLM's Qwen3CoderToolParser. Regression guard for #1888. +// decoded, matching Qwen's official API (verified on Qwen3-Coder and Qwen3.5). +// Regression guard for #1888. // ============================================================================ #[tokio::test] From 900d660c87c81c4d3e97ad27cd7327d060d2ea6e Mon Sep 17 00:00:00 2001 From: key4ng Date: Thu, 9 Jul 2026 19:47:54 -0700 Subject: [PATCH 3/3] test(tool_parser): exercise the schema-typed path in the qwen_xml golden test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The golden conformance test declared in the XML while the tool built by tool_with_props is named "f". param_types_for_function matches on name, so the lookup returned empty and coerce_value fell back to safe_val — the schema-typed `string` branch (coerce_by_schema_type) was never exercised. Match the XML function name to the tool ("f") so both the inference and schema-typed paths are actually covered. Caught in PR review. Signed-off-by: key4ng --- crates/tool_parser/src/parsers/qwen_xml.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/tool_parser/src/parsers/qwen_xml.rs b/crates/tool_parser/src/parsers/qwen_xml.rs index 9ad6eb2f1f..37f0ea2b88 100644 --- a/crates/tool_parser/src/parsers/qwen_xml.rs +++ b/crates/tool_parser/src/parsers/qwen_xml.rs @@ -610,19 +610,22 @@ mod tests { async fn test_arg_values_with_entities_roundtrip_unchanged() { let literal = "Tom & Jerry <x> it's"; - // Schema-less inference path (`safe_val`): not valid JSON -> stays a - // literal string with entities intact. + // Function name matches `tool_with_props` (`f`) so the schema-typed + // branch below actually resolves the `content` param's type. let text = format!( - "\n\n\ + "\n\n\ \n{literal}\n\n\ \n" ); + + // Schema-less inference path (`safe_val`): not valid JSON -> stays a + // literal string with entities intact. let (_, calls) = QwenXmlParser::new().parse_complete(&text).await.unwrap(); assert_eq!(calls.len(), 1); let args: Value = serde_json::from_str(&calls[0].function.arguments).unwrap(); assert_eq!(args["content"], Value::String(literal.to_string())); - // Schema-typed `string` path (`coerce_value`): same literal value. + // Schema-typed `string` path (`coerce_value` -> `coerce_by_schema_type`). let tools = tool_with_props(serde_json::json!({ "content": {"type": "string"}, }));