diff --git a/crates/tool_parser/src/parsers/glm4_moe.rs b/crates/tool_parser/src/parsers/glm4_moe.rs index 2eec81c03e..de86b3854f 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,22 @@ 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, 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, + 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 +115,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 +123,8 @@ 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); + 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 +141,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 +155,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 +177,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 +277,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 +373,69 @@ 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 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!({ + "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())); + } + + // 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 81a4463232..9f232c6667 100644 --- a/crates/tool_parser/src/parsers/helpers.rs +++ b/crates/tool_parser/src/parsers/helpers.rs @@ -12,7 +12,8 @@ use crate::{ /// `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). 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 { @@ -33,23 +34,25 @@ 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::() .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)), @@ -622,4 +625,43 @@ mod tests { &serde_json::json!({"key": "value"}) ); } + + #[test] + fn test_coerce_by_schema_type() { + use serde_json::json; + // 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]")) + ); + assert_eq!( + coerce_by_schema_type("\"4\"", Some("string")), + Some(json!("4")) + ); + + 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])) + ); + // 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); + } } diff --git a/crates/tool_parser/src/parsers/qwen_xml.rs b/crates/tool_parser/src/parsers/qwen_xml.rs index e5e4e458d0..60381619c5 100644 --- a/crates/tool_parser/src/parsers/qwen_xml.rs +++ b/crates/tool_parser/src/parsers/qwen_xml.rs @@ -153,6 +153,13 @@ fn safe_val(raw: &str) -> Value { Value::String(unescaped) } +/// 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)) +} + impl QwenXmlParser { /// Create a new Qwen XML parser #[expect( @@ -188,7 +195,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 +212,14 @@ impl QwenXmlParser { return Ok(None); } + 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,8 +237,9 @@ 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![]; + 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(); @@ -238,7 +247,7 @@ impl QwenXmlParser { 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 +314,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 +333,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 +350,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 +485,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 +546,8 @@ impl ToolParser for QwenXmlParser { #[cfg(test)] mod tests { + use openai_protocol::common::Function; + use super::*; #[test] @@ -594,4 +622,72 @@ 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 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!({ + "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())); + } + + // 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}" + ); + } }