Skip to content

fix(tool_parser): treat qwen_xml tool-call arg values literally #1899

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
key4ng merged 3 commits into from
Jul 10, 2026
Merged

fix(tool_parser): treat qwen_xml tool-call arg values literally #1899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6b8fc20
fix(tool_parser): treat qwen_xml tool-call arg values literally (#1888)
key4ng Jul 9, 2026
3319cd7
docs(tool_parser): note qwen_xml literal-value parity with Qwen's off…
key4ng Jul 9, 2026
900d660
test(tool_parser): exercise the schema-typed path in the qwen_xml gol…
key4ng Jul 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
186 changes: 66 additions & 120 deletions crates/tool_parser/src/parsers/qwen_xml.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,110 +54,47 @@ 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::<u32>().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. This
/// matches Qwen's own official API (DashScope), verified for both Qwen3-Coder
/// and Qwen3.5: a tool argument whose value contains `&amp;`, `&lt;`, `&#39;`
/// 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. 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::<Value>(&unescaped) {
if let Ok(v) = serde_json::from_str::<Value>(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,
_ => {}
}

// 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 {
Expand Down Expand Up @@ -550,40 +487,6 @@ mod tests {

use super::*;

#[test]
fn test_html_unescape_basic() {
assert_eq!(html_unescape("&amp;"), "&");
assert_eq!(html_unescape("&lt;"), "<");
assert_eq!(html_unescape("&gt;"), ">");
assert_eq!(html_unescape("&quot;"), "\"");
assert_eq!(html_unescape("&apos;"), "'");
}

#[test]
fn test_html_unescape_numeric() {
assert_eq!(html_unescape("&#60;"), "<");
assert_eq!(html_unescape("&#x3C;"), "<");
assert_eq!(html_unescape("&#x3c;"), "<");
}

#[test]
fn test_html_unescape_mixed() {
assert_eq!(
html_unescape("Hello &amp; World &lt;tag&gt;"),
"Hello & World <tag>"
);
}

#[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()));
Expand Down Expand Up @@ -614,13 +517,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 Qwen's official API, which returns them intact).
#[test]
fn test_safe_val_html_entities() {
assert_eq!(safe_val("&lt;div&gt;"), Value::String("<div>".to_string()));
fn test_safe_val_preserves_html_entities() {
assert_eq!(
safe_val("&lt;div&gt;"),
Value::String("&lt;div&gt;".to_string())
);
assert_eq!(
safe_val("Tom &amp; Jerry"),
Value::String("Tom & Jerry".to_string())
Value::String("Tom &amp; Jerry".to_string())
);
// Numeric/hex entities are likewise left untouched.
assert_eq!(safe_val("it&#39;s"), Value::String("it&#39;s".to_string()));
assert_eq!(safe_val("&#x3C;"), Value::String("&#x3C;".to_string()));
}

fn tool_with_props(props: Value) -> Vec<Tool> {
Expand Down Expand Up @@ -690,4 +601,39 @@ 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
// 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 = "<a>Tom &amp; Jerry</a> &lt;x&gt; it&#39;s";

// Function name matches `tool_with_props` (`f`) so the schema-typed
// branch below actually resolves the `content` param's type.
let text = format!(
"<tool_call>\n<function=f>\n\
<parameter=content>\n{literal}\n</parameter>\n\
</function>\n</tool_call>"
);
Comment on lines +615 to +619

@gemini-code-assist gemini-code-assist Bot Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

In this test, the XML input uses <function=write_file>, but the tools list is created using tool_with_props, which defines a function named "f".

Because of this mismatch, helpers::param_types_for_function(tools, "write_file") returns an empty map, meaning the schema-typed path (coerce_value) does not find the "content" parameter's declared type ("string") and silently falls back to the schema-less safe_val path. As a result, the schema-typed string path is not actually being tested here.

Changing the function name in the XML template to "f" ensures that the schema-typed path is correctly matched and exercised.

        let text = format!(
            "<tool_call>\n<function=f>\n\\\n             <parameter=content>\n{literal}\n</parameter>\n\\\n             </function>\n</tool_call>"
        );

@key4ng key4ng Jul 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch — you're right. With the tool named f but the XML declaring <function=write_file>, param_types_for_function returned an empty map, so coerce_value got declared_type=None and fell through to safe_val. Both halves of the test were exercising the inference path; coerce_by_schema_type's string branch was never hit.

Fixed in 900d660 by matching the XML function name to the tool (<function=f>), so the schema-typed path is now genuinely exercised. Thanks!


// 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` -> `coerce_by_schema_type`).
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()));
}
}
35 changes: 21 additions & 14 deletions crates/tool_parser/tests/tool_parser_qwen_xml.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -855,14 +855,18 @@ 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 Qwen's official API (verified on Qwen3-Coder and Qwen3.5).
// 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"<tool_call>
<function=process>
<parameter=ampersand>Tom &amp; Jerry</parameter>
Expand All @@ -875,16 +879,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 &amp; Jerry");
assert_eq!(args["comparison"], "5 &lt; 10 &amp;&amp; 10 &gt; 5");
assert_eq!(args["quotes"], "&quot;Hello&quot; &amp; &apos;World&apos;");
}

#[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"<tool_call>
<function=process>
<parameter=decimal>&#60;tag&#62;</parameter>
Expand All @@ -896,8 +900,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"], "<tag>");
assert_eq!(args["hex"], "<tag>");
assert_eq!(args["decimal"], "&#60;tag&#62;");
assert_eq!(args["hex"], "&#x3C;tag&#x3E;");
}

#[tokio::test]
Expand Down Expand Up @@ -934,7 +938,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#"<tool_call>
<function=search>
<parameter=query>price &lt; 100 &amp;&amp; rating &gt; 4</parameter>
Expand All @@ -946,8 +950,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 &lt; 100 &amp;&amp; rating &gt; 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"], "&amp;&amp;");
assert_eq!(args["config"]["escape"], true);
}
Loading