diff --git a/sgl-router/src/routers/grpc/harmony/parser.rs b/sgl-router/src/routers/grpc/harmony/parser.rs index 92add45f8..8f20b8198 100644 --- a/sgl-router/src/routers/grpc/harmony/parser.rs +++ b/sgl-router/src/routers/grpc/harmony/parser.rs @@ -106,7 +106,7 @@ impl HarmonyParserAdapter { pub fn parse_messages( messages: &[openai_harmony::chat::Message], ) -> (Option, Option>, String) { - let mut analysis = None; + let mut analysis: Option = None; let mut commentary: Option> = None; let mut final_text = String::new(); @@ -119,6 +119,60 @@ impl HarmonyParserAdapter { let channel = msg.channel.as_deref().unwrap_or(""); let recipient = msg.recipient.as_deref(); + // IMPORTANT: Check recipient FIRST before channel + // The model sometimes generates tool calls with channel="analysis" + recipient="functions.*" + // instead of channel="commentary" + recipient="functions.*" + // We should trust the recipient field to determine if this is a tool call + if let Some(recipient_str) = recipient { + if recipient_str.starts_with("functions.") { + // This is a tool call, regardless of channel + let function_name = recipient_str.strip_prefix("functions.").unwrap(); + + // Process each content item separately + for content in &msg.content { + if let openai_harmony::chat::Content::Text(tc) = content { + let call_id = format!("call_{}", Uuid::new_v4()); + let tool_call = ToolCall { + id: call_id, + tool_type: "function".to_string(), + function: FunctionCallResponse { + name: function_name.to_string(), + arguments: Some(tc.text.clone()), + }, + }; + + match commentary.as_mut() { + Some(calls) => calls.push(tool_call), + None => commentary = Some(vec![tool_call]), + } + } + } + // Skip further channel processing for this message + continue; + } else if recipient_str.starts_with("python") + || recipient_str.starts_with("browser") + || recipient_str.starts_with("container") + { + // Built-in tools → treat as reasoning + // For Chat API, we add to analysis content + let text = Self::extract_text_from_content(&msg.content); + + if !text.is_empty() { + // Append to analysis (built-in tools are reasoning) + match analysis.as_mut() { + Some(existing) => { + existing.push('\n'); + existing.push_str(&text); + } + None => analysis = Some(text), + } + } + // Skip further channel processing + continue; + } + } + + // Now process by channel (only if not already handled by recipient) match channel { "analysis" => { // Process each content item @@ -130,51 +184,25 @@ impl HarmonyParserAdapter { } } "commentary" => { - // Handle different recipient types - if let Some(recipient_str) = recipient { - if recipient_str.starts_with("functions.") { - let function_name = recipient_str.strip_prefix("functions.").unwrap(); + // If we reach here, recipient was not "functions.*" or built-in tools + // Commentary channel should always have a recipient + // This is likely a model bug - log warning and treat as reasoning + tracing::warn!( + channel = "commentary", + recipient = ?recipient, + "Commentary message without valid recipient, treating as reasoning" + ); - // Process each content item separately - for content in &msg.content { - if let openai_harmony::chat::Content::Text(tc) = content { - let call_id = format!("call_{}", Uuid::new_v4()); - let tool_call = ToolCall { - id: call_id, - tool_type: "function".to_string(), - function: FunctionCallResponse { - name: function_name.to_string(), - arguments: Some(tc.text.clone()), - }, - }; + let text = Self::extract_text_from_content(&msg.content); - match commentary.as_mut() { - Some(calls) => calls.push(tool_call), - None => commentary = Some(vec![tool_call]), - } - } - } - } else if recipient_str.starts_with("python") - || recipient_str.starts_with("browser") - || recipient_str.starts_with("container") - { - // Built-in tools → treat as reasoning - // For Chat API, we add to analysis content - let text = Self::extract_text_from_content(&msg.content); - - if !text.is_empty() { - // Append to analysis (built-in tools are reasoning) - match analysis.as_mut() { - Some(existing) => { - existing.push('\n'); - existing.push_str(&text); - } - None => analysis = Some(text), - } + if !text.is_empty() { + match analysis.as_mut() { + Some(existing) => { + existing.push('\n'); + existing.push_str(&text); } + None => analysis = Some(text), } - // Unknown recipient would raise ValueError - // For now, we silently ignore (can add logging later) } } "final" => { @@ -215,16 +243,9 @@ impl HarmonyParserAdapter { ) -> Result { // Feed all tokens to the parser for &token_id in output_ids { - self.parser.process(token_id).map_err(|e| { - // Log the full output_ids context on error - tracing::error!( - token_id = token_id, - output_ids = ?output_ids, - error = %e, - "Harmony parser failed to process token" - ); - format!("Failed to process token {}: {}", token_id, e) - })?; + self.parser + .process(token_id) + .map_err(|e| format!("Failed to process token {}: {}", token_id, e))?; } // Extract all completed messages from the parser @@ -240,7 +261,7 @@ impl HarmonyParserAdapter { let final_finish_reason = if commentary.is_some() { "tool_calls".to_string() } else { - finish_reason + finish_reason.clone() }; Ok(HarmonyChannelOutput {