diff --git a/CHANGELOG.md b/CHANGELOG.md index 086b73c12..e5ca54d38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1232,6 +1232,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Hugging Face MCP calls (`mcp_call`, `mcp_list_tools`) and their results are + no longer marked `dynamic`, as in `@ai-sdk/huggingface` 2.0.62. Without a + local tool of that name, `generate_text` now returns the call marked invalid + (`NoSuchTool`), as the AI SDK's `parseToolCall` does. +- A Hugging Face `function_call` or `mcp_call` output item with a non-empty + `output` string now also yields a tool-result holding that string, as + `@ai-sdk/huggingface` 2.0.62 does (`if (part.output)`). `function_call` + outputs were dropped in `do_generate`, and an empty `output` gave an MCP + tool-result. +- Hugging Face streams read in-stream errors as `@ai-sdk/huggingface` 2.0.62 + does: an `error` event (message on the event or under `error`) or a + `response.failed` event (`response.error`) with a string message is an + error part, and the finish reason becomes `error` with the code, else the + event type, as `raw`. A malformed chunk also makes the finish reason + `error`. Before, only a top-level `error` field was read, so `error` events + carrying the message on the event and `response.failed` events passed + unnoticed, and the finish reason stayed `other`. - Groq's `usage.raw` keeps a `null` usage field as `null`, as `@ai-sdk/groq` 4.0.52 does (`convertGroqUsage` returns the parsed object); it used to drop the field. Applies to generate and stream. diff --git a/aimux-providers/src/huggingface/responses.rs b/aimux-providers/src/huggingface/responses.rs index d965c10bc..baba665c8 100644 --- a/aimux-providers/src/huggingface/responses.rs +++ b/aimux-providers/src/huggingface/responses.rs @@ -91,37 +91,73 @@ fn huggingface_successful_response_handler() -> aimux_provider_utils::ResponseHa }) } -fn huggingface_stream_error( - event: &Value, - url: &str, - request_body_values: Value, - response_headers: HashMap, -) -> Option { - let error = event.get("error").filter(|value| !value.is_null())?; - let message = error - .as_str() - .or_else(|| error.get("message").and_then(Value::as_str)) - .unwrap_or("Hugging Face stream failed"); - let status_code = error - .get("status") - .or_else(|| error.get("code")) - .and_then(Value::as_u64) - .and_then(|status| u16::try_from(status).ok()) - .filter(|status| (400..=599).contains(status)); - let provider_code = error - .get("code") - .or_else(|| error.get("type")) - .and_then(Value::as_str) - .map(str::to_owned); - Some(aimux_provider_utils::stream_error_api_call( - message, - provider_code, - status_code, - event, - url, - request_body_values, - response_headers, - )) +/// An in-stream error event, read like upstream +/// `createHuggingFaceResponsesStreamError`: only `error` and `response.failed` +/// events, with a string message. +struct HuggingFaceStreamError<'a> { + message: &'a str, + event_type: &'a str, + code: Option, + details: &'a Value, +} + +impl<'a> HuggingFaceStreamError<'a> { + fn parse(event: &'a Value) -> Option { + let event_type = event.get("type").and_then(Value::as_str)?; + let details = match event_type { + "response.failed" => event.get("response")?.get("error")?, + "error" => event + .get("error") + .filter(|error| error.is_object()) + .unwrap_or(event), + _ => return None, + }; + let message = details.get("message").and_then(Value::as_str)?; + let code = details.get("code").and_then(|code| match code { + Value::String(code) => Some(code.clone()), + Value::Number(code) => Some(code.to_string()), + _ => None, + }); + Some(Self { + message, + event_type, + code, + details, + }) + } + + /// The finish reason's `raw`: the code, else the event type. + fn raw_finish_reason(&self) -> String { + self.code + .clone() + .unwrap_or_else(|| self.event_type.to_string()) + } + + fn into_error( + self, + event: &Value, + url: &str, + request_body_values: Value, + response_headers: HashMap, + ) -> AiMuxError { + let status_code = self + .details + .get("status") + .or_else(|| self.details.get("code")) + .and_then(Value::as_u64) + .and_then(|status| u16::try_from(status).ok()) + .filter(|status| (400..=599).contains(status)); + let provider_code = Some(self.raw_finish_reason()); + aimux_provider_utils::stream_error_api_call( + self.message, + provider_code, + status_code, + event, + url, + request_body_values, + response_headers, + ) + } } // ───────────────────────────────────────────────────────────────────────────── @@ -234,10 +270,9 @@ impl LanguageModel for HuggingFaceResponsesModel { first_event => first_event, }; if let Some(Ok(event)) = first_event.as_ref() - && let Some(error) = - huggingface_stream_error(event, &endpoint, body.clone(), response_headers.clone()) + && let Some(error) = HuggingFaceStreamError::parse(event) { - return Err(error); + return Err(error.into_error(event, &endpoint, body.clone(), response_headers.clone())); } let stream = pipe_through( futures::stream::iter(first_event).chain(sse_stream), @@ -312,12 +347,17 @@ impl HuggingFaceResponsesStream { } = *self; match event { Ok(parsed) => { - if let Some(error) = huggingface_stream_error( - &parsed, - &self.ctx.url, - self.ctx.request_body.clone(), - self.ctx.response_headers.clone(), - ) { + if let Some(error) = HuggingFaceStreamError::parse(&parsed) { + *finish_reason = FinishReason { + unified: FinishReasonUnified::Error, + raw: Some(error.raw_finish_reason()), + }; + let error = error.into_error( + &parsed, + &self.ctx.url, + self.ctx.request_body.clone(), + self.ctx.response_headers.clone(), + ); controller.enqueue(StreamPart::Error { error }); self.finished = true; return; @@ -446,9 +486,7 @@ impl HuggingFaceResponsesStream { provider_metadata: None, })); - if let Some(output) = - item.get("output").and_then(|v| v.as_str()) - { + if let Some(output) = non_empty_output(item) { controller.enqueue(StreamPart::ToolResult(ToolResult { tool_call_id: call_id, tool_name: name.clone(), @@ -534,6 +572,10 @@ impl HuggingFaceResponsesStream { } } Err(error) => { + *finish_reason = FinishReason { + unified: FinishReasonUnified::Error, + raw: None, + }; if error.is_recoverable_stream_error() { controller.enqueue_error(error); } else { @@ -1008,6 +1050,18 @@ fn build_generate_content(response: &Value) -> Result, AiMu dynamic: None, provider_metadata: None, })); + + if let Some(output) = non_empty_output(part) { + content.push(GenerateContent::ToolResult(ToolResult { + tool_call_id: call_id.to_string(), + tool_name: name.to_string(), + result: Value::String(output.to_string()), + is_error: None, + preliminary: None, + dynamic: None, + provider_metadata: None, + })); + } } "mcp_call" => { @@ -1023,18 +1077,18 @@ fn build_generate_content(response: &Value) -> Result, AiMu tool_name: name.to_string(), input, provider_executed: Some(true), - dynamic: Some(true), + dynamic: None, provider_metadata: None, })); - if let Some(output) = part.get("output").filter(|v| !v.is_null()) { + if let Some(output) = non_empty_output(part) { content.push(GenerateContent::ToolResult(ToolResult { tool_call_id: id.to_string(), tool_name: name.to_string(), - result: output.clone(), + result: Value::String(output.to_string()), is_error: None, preliminary: None, - dynamic: Some(true), + dynamic: None, provider_metadata: None, })); } @@ -1048,7 +1102,7 @@ fn build_generate_content(response: &Value) -> Result, AiMu tool_name: "list_tools".to_string(), input: json!({ "server_label": server_label }).to_string(), provider_executed: Some(true), - dynamic: Some(true), + dynamic: None, provider_metadata: None, })); @@ -1059,7 +1113,7 @@ fn build_generate_content(response: &Value) -> Result, AiMu result: json!({ "tools": tools }), is_error: None, preliminary: None, - dynamic: Some(true), + dynamic: None, provider_metadata: None, })); } @@ -1072,6 +1126,14 @@ fn build_generate_content(response: &Value) -> Result, AiMu Ok(content) } +/// `part.output` when JavaScript reads it as true (`if (part.output)`): a +/// non-empty string. +fn non_empty_output(part: &Value) -> Option<&str> { + part.get("output") + .and_then(Value::as_str) + .filter(|output| !output.is_empty()) +} + /// Parse the finish reason from a Responses API JSON body. fn parse_finish_reason(response: &Value) -> FinishReason { let incomplete_reason = response diff --git a/aimux-providers/tests/huggingface_responses_test.rs b/aimux-providers/tests/huggingface_responses_test.rs index c06505769..01d5adf49 100644 --- a/aimux-providers/tests/huggingface_responses_test.rs +++ b/aimux-providers/tests/huggingface_responses_test.rs @@ -575,14 +575,16 @@ async fn should_handle_mcp_tools_with_annotations() { other => panic!("expected Source at [4], got {other:?}"), } + // Like upstream, the MCP call is not dynamic: without a local `search` + // tool, Core returns it marked invalid (parseToolCall's NoSuchToolError). let result = generate_text(&model, "Hello", GenerateTextOptions::default()) .await - .expect("dynamic MCP call should pass Core validation without local tools"); + .expect("an unknown tool is an invalid call, not an error"); let call = *result.tool_calls().first().expect("MCP tool call"); assert_eq!(call.tool_name, "search"); assert_eq!(call.provider_executed, Some(true)); assert_eq!(call.dynamic, Some(true)); - assert_eq!(call.invalid, None); + assert_eq!(call.invalid, Some(true)); assert_eq!(call.input, json!({ "query": "San Francisco tech events" })); } @@ -1144,9 +1146,8 @@ async fn should_handle_function_call_tool_responses() { .await .expect("should succeed"); - // content: tool-call, text — a client-executed `function_call` carries no - // output, so there is no paired ToolResult. - assert_eq!(result.content.len(), 2); + // content: tool-call, tool-result (the item has an output), text. + assert_eq!(result.content.len(), 3); match &result.content[0] { GenerateContent::ToolCall(RawToolCall { tool_call_id, @@ -1161,10 +1162,26 @@ async fn should_handle_function_call_tool_responses() { other => panic!("expected ToolCall at [0], got {other:?}"), } match &result.content[1] { + GenerateContent::ToolResult(ToolResult { + tool_call_id, + tool_name, + result, + .. + }) => { + assert_eq!(tool_call_id, "call_123"); + assert_eq!(tool_name, "getWeather"); + assert_eq!( + result, + &Value::String(r#"{"temperature": "72°F", "condition": "sunny"}"#.into()) + ); + } + other => panic!("expected ToolResult at [1], got {other:?}"), + } + match &result.content[2] { GenerateContent::Text { text, .. } => { assert_eq!(text, "The weather in New York is 72°F and sunny."); } - other => panic!("expected Text at [1], got {other:?}"), + other => panic!("expected Text at [2], got {other:?}"), } } @@ -1971,3 +1988,115 @@ async fn supports_http_image_urls() { assert!(urls.0["image/*"][0].is_match("http://example.com/a.png")); assert!(!urls.0["image/*"][0].is_match("ftp://example.com/a.png")); } + +/// aimux's own: an `mcp_call` whose `output` is an empty string gives only the +/// tool-call; upstream pushes the tool-result only `if (part.output)` +/// (huggingface-responses-language-model.ts:273). +#[tokio::test] +async fn mcp_call_with_empty_output_has_no_tool_result() { + let server = MockServer::start().await; + let mut body = basic_response_body(); + body["output"] = json!([{ + "id": "mcp_empty", + "type": "mcp_call", + "server_label": "web_search", + "name": "search", + "arguments": "{}", + "output": "" + }]); + mock_json(&server, body).await; + + let result = make_provider(&server) + .responses("deepseek-ai/DeepSeek-V3-0324") + .do_generate(&default_options(test_prompt())) + .await + .expect("should succeed"); + + assert_eq!(result.content.len(), 1); + assert!(matches!(result.content[0], GenerateContent::ToolCall(_))); +} + +/// aimux's own: an `error` or `response.failed` event after the first one is +/// an error part, and the finish reason becomes error with the code (else the +/// event type) as raw (huggingface-responses-language-model.ts:406-419, +/// :562-590). Upstream's cases send the error as the first event, which aimux +/// returns from `do_stream` instead (registered SSE pre-read). +#[tokio::test] +async fn reads_mid_stream_error_events() { + let created = r#"{"type":"response.created","response":{"id":"resp_test","object":"response","created_at":1741269019,"status":"in_progress","model":"deepseek-ai/DeepSeek-V3-0324"},"sequence_number":0}"#; + for (event, message, raw) in [ + ( + r#"{"type":"response.failed","response":{"error":{"code":"429","message":"Rate limit reached"}},"sequence_number":1}"#, + "Rate limit reached", + "429", + ), + ( + r#"{"type":"error","message":"Service unavailable","param":null,"sequence_number":1}"#, + "Service unavailable", + "error", + ), + ] { + let server = MockServer::start().await; + mock_sse(&server, sse_body(&[sse_event(created), sse_event(event)])).await; + + let result = make_provider(&server) + .responses("deepseek-ai/DeepSeek-V3-0324") + .do_stream(&default_options(test_prompt())) + .await + .expect("the first event is not an error"); + let parts = collect_stream(result).await; + + let error = parts + .iter() + .find_map(|part| match part { + StreamPart::Error { error } => Some(error), + _ => None, + }) + .expect("an error part"); + match error { + AiMuxError::ApiCall(error) => { + assert_eq!(error.message, message); + assert_eq!(error.provider_code.as_deref(), Some(raw)); + } + other => panic!("expected ApiCall, got {other:?}"), + } + match parts.last() { + Some(StreamPart::Finish { finish_reason, .. }) => { + assert_eq!(finish_reason.unified, FinishReasonUnified::Error); + assert_eq!(finish_reason.raw.as_deref(), Some(raw)); + } + other => panic!("expected Finish last, got {other:?}"), + } + } +} + +/// aimux's own: a chunk that fails to parse is an error part and, when no +/// later `response.completed` sets it again, the finish reason is error with +/// no raw value (huggingface-responses-language-model.ts:396-404). +#[tokio::test] +async fn malformed_chunk_ends_with_an_error_finish() { + let server = MockServer::start().await; + let chunks = sse_body(&[ + sse_event( + r#"{"type":"response.output_item.added","output_index":0,"item":{"id":"msg_test","type":"message","role":"assistant"},"sequence_number":1}"#, + ), + "data:invalid json}\n\n".to_string(), + ]); + mock_sse(&server, chunks).await; + + let result = make_provider(&server) + .responses("deepseek-ai/DeepSeek-V3-0324") + .do_stream(&default_options(test_prompt())) + .await + .expect("should succeed"); + + let outcomes: Vec<_> = result.stream.collect().await; + assert!(outcomes.iter().any(Result::is_err), "a parse error item"); + match outcomes.last() { + Some(Ok(StreamPart::Finish { finish_reason, .. })) => { + assert_eq!(finish_reason.unified, FinishReasonUnified::Error); + assert_eq!(finish_reason.raw, None); + } + other => panic!("expected Finish last, got {other:?}"), + } +}