From 24ba480bd3d7b1a177af4169904dbc8fc168bd3d Mon Sep 17 00:00:00 2001 From: key4ng Date: Wed, 24 Jun 2026 10:04:58 -0700 Subject: [PATCH 1/7] fix(tool_parser): coerce XML tool-call args by declared schema type qwen_xml and glm4_moe blindly JSON-parsed XML parameter values, coercing string-typed args ("4", "true", "[60,30]", "{...}") to int/bool/array/object. BFCL JavaScript schemas declare many params as string, so the coerced type was wrong and the call was rejected (simple_javascript: qwen -32, glm -40 vs vLLM, which keeps them as strings). Coerce by the function's declared schema (helpers::param_types_for_function + coerce_by_schema_type): a `string` param stays a string, typed params are parsed, and unknown types fall back to the previous inference. minimax_m2 already did this. The gateway already passes tools via parse_complete_with_tools; both parsers now honor it in the complete and streaming paths. Signed-off-by: key4ng --- crates/tool_parser/src/parsers/glm4_moe.rs | 181 +++++++++++++++------ crates/tool_parser/src/parsers/qwen_xml.rs | 144 ++++++++++++---- 2 files changed, 249 insertions(+), 76 deletions(-) diff --git a/crates/tool_parser/src/parsers/glm4_moe.rs b/crates/tool_parser/src/parsers/glm4_moe.rs index 2eec81c03e..719f00c787 100644 --- a/crates/tool_parser/src/parsers/glm4_moe.rs +++ b/crates/tool_parser/src/parsers/glm4_moe.rs @@ -1,3 +1,5 @@ +use std::collections::HashMap; + use async_trait::async_trait; use openai_protocol::common::Tool; use regex::Regex; @@ -89,37 +91,23 @@ impl Glm4MoeParser { Self::new(r"(?s)\s*([^<\s]+)\s*(.*?)") } - /// Parse arguments from key-value pairs - fn parse_arguments(&self, args_text: &str) -> serde_json::Map { + /// Parse arguments from key-value pairs, coerced by the function's declared + /// schema. A `string` parameter keeps a numeric/bool/array-looking value as a + /// string (matches vLLM); unknown types fall back to [`infer_value`]. + fn parse_arguments( + &self, + args_text: &str, + param_types: &HashMap, + ) -> serde_json::Map { let mut arguments = serde_json::Map::new(); for capture in self.arg_extractor.captures_iter(args_text) { let key = capture.get(1).map_or("", |m| m.as_str()).trim(); let value_str = capture.get(2).map_or("", |m| m.as_str()).trim(); - // Try to parse the value as JSON first, fallback to string - let value = if let Ok(json_val) = serde_json::from_str::(value_str) { - json_val - } else { - // Try parsing as Python literal (similar to Python's ast.literal_eval) - if value_str == "true" || value_str == "True" { - Value::Bool(true) - } else if value_str == "false" || value_str == "False" { - Value::Bool(false) - } else if value_str == "null" || value_str == "None" { - Value::Null - } else if let Ok(num) = value_str.parse::() { - Value::Number(num.into()) - } else if let Ok(num) = value_str.parse::() { - if let Some(n) = serde_json::Number::from_f64(num) { - Value::Number(n) - } else { - Value::String(value_str.to_string()) - } - } else { - Value::String(value_str.to_string()) - } - }; + let value = + helpers::coerce_by_schema_type(value_str, param_types.get(key).map(String::as_str)) + .unwrap_or_else(|| infer_value(value_str)); arguments.insert(key.to_string(), value); } @@ -128,7 +116,7 @@ impl Glm4MoeParser { } /// Parse a single tool call block - fn parse_tool_call(&self, block: &str) -> ParserResult> { + fn parse_tool_call(&self, block: &str, tools: &[Tool]) -> ParserResult> { if let Some(captures) = self.func_detail_extractor.captures(block) { // Get function name let func_name = captures.get(1).map_or("", |m| m.as_str()).trim(); @@ -136,8 +124,9 @@ impl Glm4MoeParser { // Get arguments text let args_text = captures.get(2).map_or("", |m| m.as_str()); - // Parse arguments - let arguments = self.parse_arguments(args_text); + // Parse arguments, coerced by this function's declared schema. + let param_types = helpers::param_types_for_function(tools, func_name); + let arguments = self.parse_arguments(args_text, ¶m_types); let arguments_str = serde_json::to_string(&arguments) .map_err(|e| ParserError::ParsingFailed(e.to_string()))?; @@ -154,12 +143,12 @@ impl Glm4MoeParser { } /// Parse all tool calls from text (shared logic for complete and incremental parsing) - fn parse_tool_calls_from_text(&self, text: &str) -> Vec { - let mut tools = Vec::new(); + fn parse_tool_calls_from_text(&self, text: &str, tools: &[Tool]) -> Vec { + let mut parsed = Vec::new(); for mat in self.tool_call_extractor.find_iter(text) { - match self.parse_tool_call(mat.as_str()) { - Ok(Some(tool)) => tools.push(tool), + match self.parse_tool_call(mat.as_str(), tools) { + Ok(Some(tool)) => parsed.push(tool), Ok(None) => continue, Err(e) => { tracing::debug!("Failed to parse tool call: {}", e); @@ -168,20 +157,17 @@ impl Glm4MoeParser { } } - tools + parsed } } -impl Default for Glm4MoeParser { - fn default() -> Self { - Self::glm45() - } -} - -#[async_trait] -impl ToolParser for Glm4MoeParser { - async fn parse_complete(&self, text: &str) -> ParserResult<(String, Vec)> { - // Check if text contains GLM-4 MoE format +impl Glm4MoeParser { + /// Shared non-streaming parse, schema-aware when `tools` are provided. + fn parse_complete_inner( + &self, + text: &str, + tools: &[Tool], + ) -> ParserResult<(String, Vec)> { if !self.has_tool_markers(text) { return Ok((text.to_string(), vec![])); } @@ -193,15 +179,58 @@ impl ToolParser for Glm4MoeParser { .ok_or_else(|| ParserError::ParsingFailed("tool call marker not found".to_string()))?; let normal_text = text[..idx].to_string(); - // Parse all tool calls using shared helper - let tools = self.parse_tool_calls_from_text(text); + let parsed = self.parse_tool_calls_from_text(text, tools); // If no tools were successfully parsed despite having markers, return entire text as fallback - if tools.is_empty() { + if parsed.is_empty() { return Ok((text.to_string(), vec![])); } - Ok((normal_text, tools)) + Ok((normal_text, parsed)) + } +} + +/// Infer a JSON value from raw text when the schema type is unknown: JSON +/// (numbers/bools/null/objects/arrays), then Python-style literals, then string. +fn infer_value(value_str: &str) -> Value { + if let Ok(json_val) = serde_json::from_str::(value_str) { + return json_val; + } + match value_str { + "true" | "True" => Value::Bool(true), + "false" | "False" => Value::Bool(false), + "null" | "None" => Value::Null, + _ => { + if let Ok(num) = value_str.parse::() { + Value::Number(num.into()) + } else if let Ok(num) = value_str.parse::() { + serde_json::Number::from_f64(num) + .map_or_else(|| Value::String(value_str.to_string()), Value::Number) + } else { + Value::String(value_str.to_string()) + } + } + } +} + +impl Default for Glm4MoeParser { + fn default() -> Self { + Self::glm45() + } +} + +#[async_trait] +impl ToolParser for Glm4MoeParser { + async fn parse_complete(&self, text: &str) -> ParserResult<(String, Vec)> { + self.parse_complete_inner(text, &[]) + } + + async fn parse_complete_with_tools( + &self, + text: &str, + tools: &[Tool], + ) -> ParserResult<(String, Vec)> { + self.parse_complete_inner(text, tools) } async fn parse_incremental( @@ -250,7 +279,7 @@ impl ToolParser for Glm4MoeParser { // Parse the complete block using shared helper let block_end = end_pos + self.eot_token.len(); - let parsed_tools = self.parse_tool_calls_from_text(¤t_text[..block_end]); + let parsed_tools = self.parse_tool_calls_from_text(¤t_text[..block_end], tools); // Extract normal text before tool calls let idx = current_text.find(self.bot_token); @@ -346,3 +375,59 @@ impl ToolParser for Glm4MoeParser { self.streamed_args_for_tool.clear(); } } + +#[cfg(test)] +mod tests { + use openai_protocol::common::Function; + + use super::*; + + fn tool_with_props(props: Value) -> Vec { + vec![Tool { + tool_type: "function".to_string(), + function: Function { + name: "f".to_string(), + description: None, + parameters: serde_json::json!({"type": "object", "properties": props}), + strict: None, + }, + }] + } + + // String-typed params keep numeric/bool/array-looking values as strings (the + // JavaScript-category bug); non-string params still coerce. + #[tokio::test] + async fn test_schema_aware_coercion_keeps_strings() { + let tools = tool_with_props(serde_json::json!({ + "limit": {"type": "string"}, + "flag": {"type": "string"}, + "coords": {"type": "string"}, + "count": {"type": "integer"}, + })); + let text = "f\n\ + limit\n4\n\ + flag\ntrue\n\ + coords\n[60,30]\n\ + count\n5\n\ + "; + let (_, calls) = Glm4MoeParser::glm45() + .parse_complete_with_tools(text, &tools) + .await + .unwrap(); + assert_eq!(calls.len(), 1); + let args: Value = serde_json::from_str(&calls[0].function.arguments).unwrap(); + assert_eq!(args["limit"], Value::String("4".to_string())); + assert_eq!(args["flag"], Value::String("true".to_string())); + assert_eq!(args["coords"], Value::String("[60,30]".to_string())); + assert_eq!(args["count"], Value::Number(5.into())); + } + + // Without a schema (no tools), behavior is unchanged: blind inference. + #[tokio::test] + async fn test_no_schema_infers_type() { + let text = "f\ncount\n5\n"; + let (_, calls) = Glm4MoeParser::glm45().parse_complete(text).await.unwrap(); + let args: Value = serde_json::from_str(&calls[0].function.arguments).unwrap(); + assert_eq!(args["count"], Value::Number(5.into())); + } +} diff --git a/crates/tool_parser/src/parsers/qwen_xml.rs b/crates/tool_parser/src/parsers/qwen_xml.rs index e5e4e458d0..774ce52992 100644 --- a/crates/tool_parser/src/parsers/qwen_xml.rs +++ b/crates/tool_parser/src/parsers/qwen_xml.rs @@ -153,6 +153,16 @@ fn safe_val(raw: &str) -> Value { Value::String(unescaped) } +/// Coerce a raw XML parameter value by its declared JSON-schema type: a declared +/// `string` is kept verbatim (decoded), typed values are parsed, and an unknown +/// type falls back to [`safe_val`]'s inference. Mirrors `minimax_m2`/vLLM so a +/// numeric- or bool-looking value for a `string` parameter stays a string +/// instead of being coerced to a number/bool/array/object. +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)) +} + impl QwenXmlParser { /// Create a new Qwen XML parser #[expect( @@ -188,7 +198,7 @@ impl QwenXmlParser { } /// Parse XML format tool call: value - fn parse_xml_format(&self, content: &str) -> ParserResult> { + fn parse_xml_format(&self, content: &str, tools: &[Tool]) -> ParserResult> { let function_captures = self .xml_function_pattern .captures(content) @@ -205,13 +215,16 @@ impl QwenXmlParser { return Ok(None); } + // Coerce by the function's declared schema so e.g. a `string` parameter + // whose value looks numeric/bool/array stays a string (matches vLLM). + let param_types = helpers::param_types_for_function(tools, &function_name); let mut parameters = serde_json::Map::new(); for cap in self.xml_param_pattern.captures_iter(content) { if let (Some(key_match), Some(value_match)) = (cap.get(1), cap.get(2)) { let key = key_match.as_str().trim().to_string(); let value = value_match.as_str(); - let json_value = safe_val(value); + let json_value = coerce_value(value, param_types.get(&key).map(String::as_str)); parameters.insert(key, json_value); } } @@ -229,16 +242,19 @@ impl QwenXmlParser { /// Parse and stream complete parameters from buffer /// Returns tool call items to emit (similar to Python's _parse_and_stream_parameters) - fn parse_and_stream_parameters(&mut self) -> Vec { + fn parse_and_stream_parameters(&mut self, tools: &[Tool]) -> Vec { let mut calls: Vec = vec![]; + // Coerce by the current function's declared schema (see parse_xml_format). + let param_types = helpers::param_types_for_function(tools, &self.current_function_name); + // Find all complete parameter patterns in buffer let mut new_params = serde_json::Map::new(); for cap in self.xml_param_pattern.captures_iter(&self.buffer) { if let (Some(key_match), Some(value_match)) = (cap.get(1), cap.get(2)) { let key = key_match.as_str().trim().to_string(); let value = value_match.as_str(); - let json_value = safe_val(value); + let json_value = coerce_value(value, param_types.get(&key).map(String::as_str)); new_params.insert(key, json_value); } } @@ -305,24 +321,12 @@ impl QwenXmlParser { calls } - /// Reset streaming state for next tool call - fn reset_streaming_state(&mut self) { - self.in_tool_call = false; - self.current_tool_name_sent = false; - self.current_function_name.clear(); - self.current_parameters.clear(); - } -} - -impl Default for QwenXmlParser { - fn default() -> Self { - Self::new() - } -} - -#[async_trait] -impl ToolParser for QwenXmlParser { - async fn parse_complete(&self, text: &str) -> ParserResult<(String, Vec)> { + /// Shared non-streaming parse, schema-aware when `tools` are provided. + fn parse_complete_inner( + &self, + text: &str, + tools: &[Tool], + ) -> ParserResult<(String, Vec)> { // Check if text contains Qwen XML format if !self.has_tool_markers(text) { return Ok((text.to_string(), vec![])); @@ -336,13 +340,13 @@ impl ToolParser for QwenXmlParser { let normal_text = text[..idx].to_string(); // Extract tool calls - let mut tools = Vec::new(); + let mut parsed = Vec::new(); for captures in self.extractor.captures_iter(text) { if let Some(content_str) = captures.get(1) { let content = content_str.as_str().trim(); - match self.parse_xml_format(content) { - Ok(Some(tool)) => tools.push(tool), + match self.parse_xml_format(content, tools) { + Ok(Some(tool)) => parsed.push(tool), Ok(None) => continue, Err(e) => { tracing::warn!("Failed to parse XML tool call: {:?}", e); @@ -353,11 +357,40 @@ impl ToolParser for QwenXmlParser { } // If no tools were successfully parsed despite having markers, return entire text - if tools.is_empty() { + if parsed.is_empty() { return Ok((text.to_string(), vec![])); } - Ok((normal_text, tools)) + Ok((normal_text, parsed)) + } + + /// Reset streaming state for next tool call + fn reset_streaming_state(&mut self) { + self.in_tool_call = false; + self.current_tool_name_sent = false; + self.current_function_name.clear(); + self.current_parameters.clear(); + } +} + +impl Default for QwenXmlParser { + fn default() -> Self { + Self::new() + } +} + +#[async_trait] +impl ToolParser for QwenXmlParser { + async fn parse_complete(&self, text: &str) -> ParserResult<(String, Vec)> { + self.parse_complete_inner(text, &[]) + } + + async fn parse_complete_with_tools( + &self, + text: &str, + tools: &[Tool], + ) -> ParserResult<(String, Vec)> { + self.parse_complete_inner(text, tools) } async fn parse_incremental( @@ -459,7 +492,7 @@ impl ToolParser for QwenXmlParser { // Parse parameters (only complete ones) if self.current_tool_name_sent { - let param_calls = self.parse_and_stream_parameters(); + let param_calls = self.parse_and_stream_parameters(tools); calls.extend(param_calls); // Check if tool call is complete @@ -520,6 +553,8 @@ impl ToolParser for QwenXmlParser { #[cfg(test)] mod tests { + use openai_protocol::common::Function; + use super::*; #[test] @@ -594,4 +629,57 @@ mod tests { Value::String("Tom & Jerry".to_string()) ); } + + fn tool_with_props(props: Value) -> Vec { + vec![Tool { + tool_type: "function".to_string(), + function: Function { + name: "f".to_string(), + description: None, + parameters: serde_json::json!({"type": "object", "properties": props}), + strict: None, + }, + }] + } + + // String-typed params keep numeric/bool/array/object-looking values as + // strings (the JavaScript-category bug); non-string params still coerce. + #[tokio::test] + async fn test_schema_aware_coercion_keeps_strings() { + let tools = tool_with_props(serde_json::json!({ + "limit": {"type": "string"}, + "flag": {"type": "string"}, + "coords": {"type": "string"}, + "cfg": {"type": "string"}, + "count": {"type": "integer"}, + })); + let text = "\n\n\ + 4\n\ + true\n\ + [60,30]\n\ + {\"a\": 1}\n\ + 5\n\ + \n"; + let (_, calls) = QwenXmlParser::new() + .parse_complete_with_tools(text, &tools) + .await + .unwrap(); + assert_eq!(calls.len(), 1); + let args: Value = serde_json::from_str(&calls[0].function.arguments).unwrap(); + assert_eq!(args["limit"], Value::String("4".to_string())); + assert_eq!(args["flag"], Value::String("true".to_string())); + assert_eq!(args["coords"], Value::String("[60,30]".to_string())); + assert_eq!(args["cfg"], Value::String("{\"a\": 1}".to_string())); + assert_eq!(args["count"], Value::Number(5.into())); + } + + // Without a schema (no tools), behavior is unchanged: blind inference. + #[tokio::test] + async fn test_no_schema_infers_type() { + let text = + "\n\n5\n\n"; + let (_, calls) = QwenXmlParser::new().parse_complete(text).await.unwrap(); + let args: Value = serde_json::from_str(&calls[0].function.arguments).unwrap(); + assert_eq!(args["count"], Value::Number(5.into())); + } } From d0693df7ecf772e7bf2db072bd219230050bf9a2 Mon Sep 17 00:00:00 2001 From: key4ng Date: Wed, 24 Jun 2026 16:59:35 -0700 Subject: [PATCH 2/7] test(tool_parser): cover schema-aware coercion in the streaming path Add parse_incremental tests for qwen_xml and glm4_moe asserting string-typed params stay strings (and non-string ones coerce) during streaming, not just parse_complete_with_tools (CodeRabbit). Signed-off-by: key4ng --- crates/tool_parser/src/parsers/glm4_moe.rs | 20 ++++++++++++++++ crates/tool_parser/src/parsers/qwen_xml.rs | 27 ++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/crates/tool_parser/src/parsers/glm4_moe.rs b/crates/tool_parser/src/parsers/glm4_moe.rs index 719f00c787..400d7bbb66 100644 --- a/crates/tool_parser/src/parsers/glm4_moe.rs +++ b/crates/tool_parser/src/parsers/glm4_moe.rs @@ -430,4 +430,24 @@ mod tests { let args: Value = serde_json::from_str(&calls[0].function.arguments).unwrap(); assert_eq!(args["count"], Value::Number(5.into())); } + + // The incremental path forwards tools, so it coerces by schema too. + #[tokio::test] + async fn test_streaming_schema_aware_coercion() { + let tools = tool_with_props(serde_json::json!({ + "limit": {"type": "string"}, + "count": {"type": "integer"}, + })); + let text = "f\n\ + limit\n4\n\ + count\n5\n\ + "; + let result = Glm4MoeParser::glm45() + .parse_incremental(text, &tools) + .await + .unwrap(); + let args: Value = serde_json::from_str(&result.calls[0].parameters).unwrap(); + assert_eq!(args["limit"], Value::String("4".to_string())); + assert_eq!(args["count"], Value::Number(5.into())); + } } diff --git a/crates/tool_parser/src/parsers/qwen_xml.rs b/crates/tool_parser/src/parsers/qwen_xml.rs index 774ce52992..2f6758bc4d 100644 --- a/crates/tool_parser/src/parsers/qwen_xml.rs +++ b/crates/tool_parser/src/parsers/qwen_xml.rs @@ -682,4 +682,31 @@ mod tests { let args: Value = serde_json::from_str(&calls[0].function.arguments).unwrap(); assert_eq!(args["count"], Value::Number(5.into())); } + + // The streaming path coerces by schema too: concatenated parameter fragments + // keep a string-typed value quoted while a non-string one is parsed. + #[tokio::test] + async fn test_streaming_schema_aware_coercion() { + let tools = tool_with_props(serde_json::json!({ + "limit": {"type": "string"}, + "count": {"type": "integer"}, + })); + let text = "\n\n\ + 4\n\ + 5\n\ + \n"; + let result = QwenXmlParser::new() + .parse_incremental(text, &tools) + .await + .unwrap(); + let args: String = result.calls.iter().map(|c| c.parameters.as_str()).collect(); + assert!( + args.contains(r#""limit": "4""#), + "string param must stay string: {args}" + ); + assert!( + args.contains(r#""count": 5"#), + "int param must coerce: {args}" + ); + } } From deb8645b8ca771b2021899ce954fbe24d8ac7e2b Mon Sep 17 00:00:00 2001 From: key4ng Date: Wed, 24 Jun 2026 17:05:31 -0700 Subject: [PATCH 3/7] fix(tool_parser): unwrap JSON string literals for string-typed args MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit coerce_by_schema_type kept a string param verbatim, so a model emitting a JSON string literal ("4") for a string param produced '"4"' (extra quotes) — a regression vs the old safe_val path. Unwrap a JSON string literal to its content while still keeping bare 4/true/[60,30] as strings. Fixes the qwen_xml/glm4_moe string fast-path and keeps minimax_m2 (same helper) consistent. Signed-off-by: key4ng --- crates/tool_parser/src/parsers/helpers.rs | 47 +++++++++++++++++++++-- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/crates/tool_parser/src/parsers/helpers.rs b/crates/tool_parser/src/parsers/helpers.rs index 81a4463232..7e0fca39ac 100644 --- a/crates/tool_parser/src/parsers/helpers.rs +++ b/crates/tool_parser/src/parsers/helpers.rs @@ -33,12 +33,16 @@ pub fn param_types_for_function(tools: &[Tool], func_name: &str) -> HashMap) -> Option { match declared_type? { - "string" => Some(Value::String(text.to_string())), + "string" => Some(Value::String( + serde_json::from_str::(text).unwrap_or_else(|_| text.to_string()), + )), "integer" => text .trim() .parse::() @@ -622,4 +626,39 @@ mod tests { &serde_json::json!({"key": "value"}) ); } + + #[test] + fn test_coerce_by_schema_type() { + use serde_json::json; + // string: bare numeric/bool/array-looking text stays a string... + assert_eq!(coerce_by_schema_type("4", Some("string")), Some(json!("4"))); + assert_eq!( + coerce_by_schema_type("true", Some("string")), + Some(json!("true")) + ); + assert_eq!( + coerce_by_schema_type("[60,30]", Some("string")), + Some(json!("[60,30]")) + ); + // ...but a JSON string literal is unwrapped to its content. + assert_eq!( + coerce_by_schema_type("\"4\"", Some("string")), + Some(json!("4")) + ); + + // typed values are parsed + assert_eq!(coerce_by_schema_type("4", Some("integer")), Some(json!(4))); + assert_eq!( + coerce_by_schema_type("true", Some("boolean")), + Some(json!(true)) + ); + assert_eq!( + coerce_by_schema_type("[60,30]", Some("array")), + Some(json!([60, 30])) + ); + + // unknown type / unparseable -> None (caller falls back to inference) + assert_eq!(coerce_by_schema_type("4", None), None); + assert_eq!(coerce_by_schema_type("abc", Some("integer")), None); + } } From c26fa182d6c3693f3efe54b2d7e66e999dfbe179 Mon Sep 17 00:00:00 2001 From: key4ng Date: Wed, 24 Jun 2026 17:14:50 -0700 Subject: [PATCH 4/7] style(tool_parser): reword test comment to satisfy codespell 'unparseable' -> plain wording; no code change. Signed-off-by: key4ng --- crates/tool_parser/src/parsers/helpers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/tool_parser/src/parsers/helpers.rs b/crates/tool_parser/src/parsers/helpers.rs index 7e0fca39ac..ed288d3f97 100644 --- a/crates/tool_parser/src/parsers/helpers.rs +++ b/crates/tool_parser/src/parsers/helpers.rs @@ -657,7 +657,7 @@ mod tests { Some(json!([60, 30])) ); - // unknown type / unparseable -> None (caller falls back to inference) + // unknown type / value that fails to parse -> None (caller infers) assert_eq!(coerce_by_schema_type("4", None), None); assert_eq!(coerce_by_schema_type("abc", Some("integer")), None); } From 622575e5e291e1bc62bc28b7bde4e22afccf65e9 Mon Sep 17 00:00:00 2001 From: key4ng Date: Thu, 25 Jun 2026 09:37:54 -0700 Subject: [PATCH 5/7] fix(tool_parser): resolve nullable/union schema types for coercion param_types_for_function only read a scalar `type`, so an optional string declared as {"type": ["string", "null"]} or via anyOf/oneOf produced no entry and fell back to inference (coercing e.g. "4" to a number). Resolve such schemas to their non-null type so optional strings stay strings. Shared helper, so qwen_xml/glm4_moe/minimax_m2 all benefit. (No BFCL schema uses unions today, so scores are unchanged; this is real-world robustness.) Signed-off-by: key4ng --- crates/tool_parser/src/parsers/helpers.rs | 55 ++++++++++++++++++++++- 1 file changed, 53 insertions(+), 2 deletions(-) diff --git a/crates/tool_parser/src/parsers/helpers.rs b/crates/tool_parser/src/parsers/helpers.rs index ed288d3f97..bd82c8bcc5 100644 --- a/crates/tool_parser/src/parsers/helpers.rs +++ b/crates/tool_parser/src/parsers/helpers.rs @@ -9,10 +9,33 @@ use crate::{ types::{StreamingParseResult, ToolCallItem}, }; +/// Resolve a property schema to a single representative JSON-schema type, +/// looking through the common nullable/union spellings so an optional `string` +/// is still treated as `string`: +/// - scalar: `{"type": "string"}` +/// - nullable array: `{"type": ["string", "null"]}` (first non-`null`) +/// - union: `{"anyOf"|"oneOf": [{"type": "string"}, {"type": "null"}]}` +fn schema_type(schema: &Value) -> Option<&str> { + if let Some(t) = schema.get("type").and_then(Value::as_str) { + return Some(t); + } + if let Some(arr) = schema.get("type").and_then(Value::as_array) { + return arr.iter().filter_map(Value::as_str).find(|t| *t != "null"); + } + ["anyOf", "oneOf"].iter().find_map(|key| { + schema + .get(key) + .and_then(Value::as_array)? + .iter() + .filter_map(schema_type) + .find(|t| *t != "null") + }) +} + /// `param_name -> declared JSON-schema type` for the named function (empty if the /// function or its `properties` are absent). Lets XML-style parsers coerce by the /// declared type instead of guessing from text (e.g. keep a numeric-looking -/// `string` as a string). +/// `string` as a string). Nullable/union schemas resolve to their non-`null` type. pub fn param_types_for_function(tools: &[Tool], func_name: &str) -> HashMap { let mut types = HashMap::new(); let Some(tool) = tools.iter().find(|t| t.function.name == func_name) else { @@ -25,7 +48,7 @@ pub fn param_types_for_function(tools: &[Tool], func_name: &str) -> HashMap Date: Thu, 25 Jun 2026 10:00:48 -0700 Subject: [PATCH 6/7] refactor(tool_parser): trim non-essential tests and comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep the essential coverage — coerce_by_schema_type + param_types_for_function unit tests, and one end-to-end coercion test per XML parser — and drop the redundant no-schema/streaming variants (same helper, covered elsewhere). Trim verbose doc/inline comments. Signed-off-by: key4ng --- crates/tool_parser/src/parsers/glm4_moe.rs | 38 ++-------------- crates/tool_parser/src/parsers/helpers.rs | 24 +++------- crates/tool_parser/src/parsers/qwen_xml.rs | 51 ++-------------------- 3 files changed, 13 insertions(+), 100 deletions(-) diff --git a/crates/tool_parser/src/parsers/glm4_moe.rs b/crates/tool_parser/src/parsers/glm4_moe.rs index 400d7bbb66..2bffc4bb13 100644 --- a/crates/tool_parser/src/parsers/glm4_moe.rs +++ b/crates/tool_parser/src/parsers/glm4_moe.rs @@ -91,9 +91,8 @@ impl Glm4MoeParser { Self::new(r"(?s)\s*([^<\s]+)\s*(.*?)") } - /// Parse arguments from key-value pairs, coerced by the function's declared - /// schema. A `string` parameter keeps a numeric/bool/array-looking value as a - /// string (matches vLLM); unknown types fall back to [`infer_value`]. + /// Parse arguments, coercing each value by its declared schema type and + /// falling back to [`infer_value`] when the type is unknown. fn parse_arguments( &self, args_text: &str, @@ -124,7 +123,6 @@ impl Glm4MoeParser { // Get arguments text let args_text = captures.get(2).map_or("", |m| m.as_str()); - // Parse arguments, coerced by this function's declared schema. let param_types = helpers::param_types_for_function(tools, func_name); let arguments = self.parse_arguments(args_text, ¶m_types); @@ -394,8 +392,7 @@ mod tests { }] } - // String-typed params keep numeric/bool/array-looking values as strings (the - // JavaScript-category bug); non-string params still coerce. + // String-typed params stay strings even when they look numeric/bool/array. #[tokio::test] async fn test_schema_aware_coercion_keeps_strings() { let tools = tool_with_props(serde_json::json!({ @@ -421,33 +418,4 @@ mod tests { assert_eq!(args["coords"], Value::String("[60,30]".to_string())); assert_eq!(args["count"], Value::Number(5.into())); } - - // Without a schema (no tools), behavior is unchanged: blind inference. - #[tokio::test] - async fn test_no_schema_infers_type() { - let text = "f\ncount\n5\n"; - let (_, calls) = Glm4MoeParser::glm45().parse_complete(text).await.unwrap(); - let args: Value = serde_json::from_str(&calls[0].function.arguments).unwrap(); - assert_eq!(args["count"], Value::Number(5.into())); - } - - // The incremental path forwards tools, so it coerces by schema too. - #[tokio::test] - async fn test_streaming_schema_aware_coercion() { - let tools = tool_with_props(serde_json::json!({ - "limit": {"type": "string"}, - "count": {"type": "integer"}, - })); - let text = "f\n\ - limit\n4\n\ - count\n5\n\ - "; - let result = Glm4MoeParser::glm45() - .parse_incremental(text, &tools) - .await - .unwrap(); - let args: Value = serde_json::from_str(&result.calls[0].parameters).unwrap(); - assert_eq!(args["limit"], Value::String("4".to_string())); - assert_eq!(args["count"], Value::Number(5.into())); - } } diff --git a/crates/tool_parser/src/parsers/helpers.rs b/crates/tool_parser/src/parsers/helpers.rs index bd82c8bcc5..821155a01c 100644 --- a/crates/tool_parser/src/parsers/helpers.rs +++ b/crates/tool_parser/src/parsers/helpers.rs @@ -9,12 +9,9 @@ use crate::{ types::{StreamingParseResult, ToolCallItem}, }; -/// Resolve a property schema to a single representative JSON-schema type, -/// looking through the common nullable/union spellings so an optional `string` -/// is still treated as `string`: -/// - scalar: `{"type": "string"}` -/// - nullable array: `{"type": ["string", "null"]}` (first non-`null`) -/// - union: `{"anyOf"|"oneOf": [{"type": "string"}, {"type": "null"}]}` +/// Representative JSON-schema type for a property, resolving the common nullable/ +/// union spellings (`["string","null"]`, `anyOf`/`oneOf`) to their non-`null` +/// type so an optional string still resolves to `string`. fn schema_type(schema: &Value) -> Option<&str> { if let Some(t) = schema.get("type").and_then(Value::as_str) { return Some(t); @@ -57,10 +54,9 @@ pub fn param_types_for_function(tools: &[Tool], func_name: &str) -> HashMap) -> Option { match declared_type? { "string" => Some(Value::String( @@ -653,23 +649,17 @@ mod tests { #[test] fn test_coerce_by_schema_type() { use serde_json::json; - // string: bare numeric/bool/array-looking text stays a string... + // string: bare text stays a string; a JSON string literal is unwrapped. assert_eq!(coerce_by_schema_type("4", Some("string")), Some(json!("4"))); - assert_eq!( - coerce_by_schema_type("true", Some("string")), - Some(json!("true")) - ); assert_eq!( coerce_by_schema_type("[60,30]", Some("string")), Some(json!("[60,30]")) ); - // ...but a JSON string literal is unwrapped to its content. assert_eq!( coerce_by_schema_type("\"4\"", Some("string")), Some(json!("4")) ); - // typed values are parsed assert_eq!(coerce_by_schema_type("4", Some("integer")), Some(json!(4))); assert_eq!( coerce_by_schema_type("true", Some("boolean")), diff --git a/crates/tool_parser/src/parsers/qwen_xml.rs b/crates/tool_parser/src/parsers/qwen_xml.rs index 2f6758bc4d..dac5a0688f 100644 --- a/crates/tool_parser/src/parsers/qwen_xml.rs +++ b/crates/tool_parser/src/parsers/qwen_xml.rs @@ -153,11 +153,8 @@ fn safe_val(raw: &str) -> Value { Value::String(unescaped) } -/// Coerce a raw XML parameter value by its declared JSON-schema type: a declared -/// `string` is kept verbatim (decoded), typed values are parsed, and an unknown -/// type falls back to [`safe_val`]'s inference. Mirrors `minimax_m2`/vLLM so a -/// numeric- or bool-looking value for a `string` parameter stays a string -/// instead of being coerced to a number/bool/array/object. +/// Coerce an XML parameter value by its declared schema type, falling back to +/// [`safe_val`] inference when the type is unknown. 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)) @@ -215,8 +212,6 @@ impl QwenXmlParser { return Ok(None); } - // Coerce by the function's declared schema so e.g. a `string` parameter - // whose value looks numeric/bool/array stays a string (matches vLLM). let param_types = helpers::param_types_for_function(tools, &function_name); let mut parameters = serde_json::Map::new(); @@ -244,8 +239,6 @@ impl QwenXmlParser { /// Returns tool call items to emit (similar to Python's _parse_and_stream_parameters) fn parse_and_stream_parameters(&mut self, tools: &[Tool]) -> Vec { let mut calls: Vec = vec![]; - - // Coerce by the current function's declared schema (see parse_xml_format). let param_types = helpers::param_types_for_function(tools, &self.current_function_name); // Find all complete parameter patterns in buffer @@ -642,8 +635,7 @@ mod tests { }] } - // String-typed params keep numeric/bool/array/object-looking values as - // strings (the JavaScript-category bug); non-string params still coerce. + // String-typed params stay strings even when they look numeric/bool/array/object. #[tokio::test] async fn test_schema_aware_coercion_keeps_strings() { let tools = tool_with_props(serde_json::json!({ @@ -672,41 +664,4 @@ mod tests { assert_eq!(args["cfg"], Value::String("{\"a\": 1}".to_string())); assert_eq!(args["count"], Value::Number(5.into())); } - - // Without a schema (no tools), behavior is unchanged: blind inference. - #[tokio::test] - async fn test_no_schema_infers_type() { - let text = - "\n\n5\n\n"; - let (_, calls) = QwenXmlParser::new().parse_complete(text).await.unwrap(); - let args: Value = serde_json::from_str(&calls[0].function.arguments).unwrap(); - assert_eq!(args["count"], Value::Number(5.into())); - } - - // The streaming path coerces by schema too: concatenated parameter fragments - // keep a string-typed value quoted while a non-string one is parsed. - #[tokio::test] - async fn test_streaming_schema_aware_coercion() { - let tools = tool_with_props(serde_json::json!({ - "limit": {"type": "string"}, - "count": {"type": "integer"}, - })); - let text = "\n\n\ - 4\n\ - 5\n\ - \n"; - let result = QwenXmlParser::new() - .parse_incremental(text, &tools) - .await - .unwrap(); - let args: String = result.calls.iter().map(|c| c.parameters.as_str()).collect(); - assert!( - args.contains(r#""limit": "4""#), - "string param must stay string: {args}" - ); - assert!( - args.contains(r#""count": 5"#), - "int param must coerce: {args}" - ); - } } From 503ec8a1d4e5cf0f79ff2f6f65b363460f00a746 Mon Sep 17 00:00:00 2001 From: key4ng Date: Thu, 25 Jun 2026 10:21:22 -0700 Subject: [PATCH 7/7] =?UTF-8?q?fix(tool=5Fparser):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20drop=20union=20resolver,=20keep=20precision,=20rest?= =?UTF-8?q?ore=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Revert the nullable/union schema_type resolver back to a scalar `type` lookup. It collapsed real multi-type unions (["string","object"]) to one branch and swallowed bare null for ["string","null"]; unions/nullable now fall back to inference (same as before this PR, no BFCL impact). - coerce_by_schema_type `number` parses JSON instead of f64 so large integers keep precision (e.g. 9007199254740993). - Restore the streaming-path coercion tests (distinct code path) and the boolean-as-string helper assertion. Signed-off-by: key4ng --- crates/tool_parser/src/parsers/glm4_moe.rs | 20 ++++++ crates/tool_parser/src/parsers/helpers.rs | 74 ++++++---------------- crates/tool_parser/src/parsers/qwen_xml.rs | 26 ++++++++ 3 files changed, 64 insertions(+), 56 deletions(-) diff --git a/crates/tool_parser/src/parsers/glm4_moe.rs b/crates/tool_parser/src/parsers/glm4_moe.rs index 2bffc4bb13..de86b3854f 100644 --- a/crates/tool_parser/src/parsers/glm4_moe.rs +++ b/crates/tool_parser/src/parsers/glm4_moe.rs @@ -418,4 +418,24 @@ mod tests { assert_eq!(args["coords"], Value::String("[60,30]".to_string())); assert_eq!(args["count"], Value::Number(5.into())); } + + // The streaming path threads `tools` separately, so cover it too. + #[tokio::test] + async fn test_streaming_schema_aware_coercion() { + let tools = tool_with_props(serde_json::json!({ + "limit": {"type": "string"}, + "count": {"type": "integer"}, + })); + let text = "f\n\ + limit\n4\n\ + count\n5\n\ + "; + let result = Glm4MoeParser::glm45() + .parse_incremental(text, &tools) + .await + .unwrap(); + let args: Value = serde_json::from_str(&result.calls[0].parameters).unwrap(); + assert_eq!(args["limit"], Value::String("4".to_string())); + assert_eq!(args["count"], Value::Number(5.into())); + } } diff --git a/crates/tool_parser/src/parsers/helpers.rs b/crates/tool_parser/src/parsers/helpers.rs index 821155a01c..9f232c6667 100644 --- a/crates/tool_parser/src/parsers/helpers.rs +++ b/crates/tool_parser/src/parsers/helpers.rs @@ -9,30 +9,11 @@ use crate::{ types::{StreamingParseResult, ToolCallItem}, }; -/// Representative JSON-schema type for a property, resolving the common nullable/ -/// union spellings (`["string","null"]`, `anyOf`/`oneOf`) to their non-`null` -/// type so an optional string still resolves to `string`. -fn schema_type(schema: &Value) -> Option<&str> { - if let Some(t) = schema.get("type").and_then(Value::as_str) { - return Some(t); - } - if let Some(arr) = schema.get("type").and_then(Value::as_array) { - return arr.iter().filter_map(Value::as_str).find(|t| *t != "null"); - } - ["anyOf", "oneOf"].iter().find_map(|key| { - schema - .get(key) - .and_then(Value::as_array)? - .iter() - .filter_map(schema_type) - .find(|t| *t != "null") - }) -} - /// `param_name -> declared JSON-schema type` for the named function (empty if the /// function or its `properties` are absent). Lets XML-style parsers coerce by the /// declared type instead of guessing from text (e.g. keep a numeric-looking -/// `string` as a string). Nullable/union schemas resolve to their non-`null` type. +/// `string` as a string). Only scalar `type` is read; unions/nullable schemas +/// have no single type, so they fall back to the caller's inference. pub fn param_types_for_function(tools: &[Tool], func_name: &str) -> HashMap { let mut types = HashMap::new(); let Some(tool) = tools.iter().find(|t| t.function.name == func_name) else { @@ -45,7 +26,7 @@ pub fn param_types_for_function(tools: &[Tool], func_name: &str) -> HashMap) -> Option< .parse::() .ok() .map(|n| Value::Number(n.into())), - "number" => text - .trim() - .parse::() + // Parse as JSON so large integers keep their precision (a plain f64 parse + // would round e.g. 9007199254740993). + "number" => serde_json::from_str::(text.trim()) .ok() - .and_then(serde_json::Number::from_f64) - .map(Value::Number), + .filter(Value::is_number), "boolean" => match text.trim() { "true" | "True" => Some(Value::Bool(true)), "false" | "False" => Some(Value::Bool(false)), @@ -649,8 +629,13 @@ mod tests { #[test] fn test_coerce_by_schema_type() { use serde_json::json; - // string: bare text stays a string; a JSON string literal is unwrapped. + // string: numeric/bool/array-looking text stays a string; a JSON string + // literal is unwrapped. assert_eq!(coerce_by_schema_type("4", Some("string")), Some(json!("4"))); + assert_eq!( + coerce_by_schema_type("true", Some("string")), + Some(json!("true")) + ); assert_eq!( coerce_by_schema_type("[60,30]", Some("string")), Some(json!("[60,30]")) @@ -669,37 +654,14 @@ mod tests { coerce_by_schema_type("[60,30]", Some("array")), Some(json!([60, 30])) ); + // number keeps integer precision (no f64 rounding). + assert_eq!( + coerce_by_schema_type("9007199254740993", Some("number")), + Some(json!(9007199254740993i64)) + ); // unknown type / value that fails to parse -> None (caller infers) assert_eq!(coerce_by_schema_type("4", None), None); assert_eq!(coerce_by_schema_type("abc", Some("integer")), None); } - - #[test] - fn test_param_types_nullable_and_union() { - use openai_protocol::common::Function; - let tools = vec![Tool { - tool_type: "function".to_string(), - function: Function { - name: "f".to_string(), - description: None, - parameters: serde_json::json!({ - "type": "object", - "properties": { - "a": {"type": "string"}, - "b": {"type": ["string", "null"]}, - "c": {"anyOf": [{"type": "string"}, {"type": "null"}]}, - "d": {"type": ["null", "integer"]}, - } - }), - strict: None, - }, - }]; - let types = param_types_for_function(&tools, "f"); - // Optional/nullable strings still resolve to "string" (not dropped). - assert_eq!(types.get("a").map(String::as_str), Some("string")); - assert_eq!(types.get("b").map(String::as_str), Some("string")); - assert_eq!(types.get("c").map(String::as_str), Some("string")); - assert_eq!(types.get("d").map(String::as_str), Some("integer")); - } } diff --git a/crates/tool_parser/src/parsers/qwen_xml.rs b/crates/tool_parser/src/parsers/qwen_xml.rs index dac5a0688f..60381619c5 100644 --- a/crates/tool_parser/src/parsers/qwen_xml.rs +++ b/crates/tool_parser/src/parsers/qwen_xml.rs @@ -664,4 +664,30 @@ mod tests { assert_eq!(args["cfg"], Value::String("{\"a\": 1}".to_string())); assert_eq!(args["count"], Value::Number(5.into())); } + + // The streaming path threads `tools` separately, so cover it too. + #[tokio::test] + async fn test_streaming_schema_aware_coercion() { + let tools = tool_with_props(serde_json::json!({ + "limit": {"type": "string"}, + "count": {"type": "integer"}, + })); + let text = "\n\n\ + 4\n\ + 5\n\ + \n"; + let result = QwenXmlParser::new() + .parse_incremental(text, &tools) + .await + .unwrap(); + let args: String = result.calls.iter().map(|c| c.parameters.as_str()).collect(); + assert!( + args.contains(r#""limit": "4""#), + "string param must stay string: {args}" + ); + assert!( + args.contains(r#""count": 5"#), + "int param must coerce: {args}" + ); + } }