From 38b7c76198ec5b57334eec77f58291c821fc7d2e Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 17 Mar 2026 23:14:51 +0100 Subject: [PATCH] open_ai: Fix tool output for /responses endpoint (#51789) We were sending the raw tool debug output as JSON to the model rather than whatever the tool intended as content for the model. Which meant we were sending unneeded information to the model, which matters in the edit tool case. Release Notes: - N/A --- crates/language_model/src/request.rs | 2 + .../src/provider/copilot_chat.rs | 46 +++++++------------ .../language_models/src/provider/open_ai.rs | 31 ++++--------- 3 files changed, 29 insertions(+), 50 deletions(-) diff --git a/crates/language_model/src/request.rs b/crates/language_model/src/request.rs index 9be3002deae758ee99432842a31e3b90754ada0f..3e8408f8334a52c2c807f991535cbbbd79800c4b 100644 --- a/crates/language_model/src/request.rs +++ b/crates/language_model/src/request.rs @@ -234,7 +234,9 @@ pub struct LanguageModelToolResult { pub tool_use_id: LanguageModelToolUseId, pub tool_name: Arc, pub is_error: bool, + /// The tool output formatted for presenting to the model pub content: LanguageModelToolResultContent, + /// The raw tool output, if available, often for debugging or extra state for replay pub output: Option, } diff --git a/crates/language_models/src/provider/copilot_chat.rs b/crates/language_models/src/provider/copilot_chat.rs index 47d1b316a581c8013843940ecb3e55ed29bc4500..286eb872795642be47dfd46f16e561dcd53f93dc 100644 --- a/crates/language_models/src/provider/copilot_chat.rs +++ b/crates/language_models/src/provider/copilot_chat.rs @@ -1111,38 +1111,26 @@ fn into_copilot_responses( Role::User => { for content in &message.content { if let MessageContent::ToolResult(tool_result) = content { - let output = if let Some(out) = &tool_result.output { - match out { - serde_json::Value::String(s) => { - responses::ResponseFunctionOutput::Text(s.clone()) - } - serde_json::Value::Null => { - responses::ResponseFunctionOutput::Text(String::new()) - } - other => responses::ResponseFunctionOutput::Text(other.to_string()), + let output = match &tool_result.content { + LanguageModelToolResultContent::Text(text) => { + responses::ResponseFunctionOutput::Text(text.to_string()) } - } else { - match &tool_result.content { - LanguageModelToolResultContent::Text(text) => { - responses::ResponseFunctionOutput::Text(text.to_string()) - } - LanguageModelToolResultContent::Image(image) => { - if model.supports_vision() { - responses::ResponseFunctionOutput::Content(vec![ - responses::ResponseInputContent::InputImage { - image_url: Some(image.to_base64_url()), - detail: Default::default(), - }, - ]) - } else { - debug_panic!( - "This should be caught at {} level", - tool_result.tool_name - ); - responses::ResponseFunctionOutput::Text( + LanguageModelToolResultContent::Image(image) => { + if model.supports_vision() { + responses::ResponseFunctionOutput::Content(vec![ + responses::ResponseInputContent::InputImage { + image_url: Some(image.to_base64_url()), + detail: Default::default(), + }, + ]) + } else { + debug_panic!( + "This should be caught at {} level", + tool_result.tool_name + ); + responses::ResponseFunctionOutput::Text( "[Tool responded with an image, but this model does not support vision]".into(), ) - } } } }; diff --git a/crates/language_models/src/provider/open_ai.rs b/crates/language_models/src/provider/open_ai.rs index e033be8ee234156bc2452c11fad438be70a7f143..784db4d9de55d8c51348c833ce3773de8237db6e 100644 --- a/crates/language_models/src/provider/open_ai.rs +++ b/crates/language_models/src/provider/open_ai.rs @@ -9,9 +9,8 @@ use language_model::{ LanguageModelCompletionEvent, LanguageModelId, LanguageModelImage, LanguageModelName, LanguageModelProvider, LanguageModelProviderId, LanguageModelProviderName, LanguageModelProviderState, LanguageModelRequest, LanguageModelRequestMessage, - LanguageModelToolChoice, LanguageModelToolResult, LanguageModelToolResultContent, - LanguageModelToolUse, LanguageModelToolUseId, MessageContent, RateLimiter, Role, StopReason, - TokenUsage, env_var, + LanguageModelToolChoice, LanguageModelToolResultContent, LanguageModelToolUse, + LanguageModelToolUseId, MessageContent, RateLimiter, Role, StopReason, TokenUsage, env_var, }; use menu; use open_ai::responses::{ @@ -647,7 +646,10 @@ fn append_message_to_response_items( input_items.push(ResponseInputItem::FunctionCallOutput( ResponseFunctionCallOutputItem { call_id: tool_result.tool_use_id.to_string(), - output: tool_result_output(&tool_result), + output: match tool_result.content { + LanguageModelToolResultContent::Text(text) => text.to_string(), + LanguageModelToolResultContent::Image(image) => image.to_base64_url(), + }, }, )); } @@ -715,21 +717,6 @@ fn flush_response_parts( parts.clear(); } -fn tool_result_output(result: &LanguageModelToolResult) -> String { - if let Some(output) = &result.output { - match output { - serde_json::Value::String(text) => text.clone(), - serde_json::Value::Null => String::new(), - _ => output.to_string(), - } - } else { - match &result.content { - LanguageModelToolResultContent::Text(text) => text.to_string(), - LanguageModelToolResultContent::Image(image) => image.to_base64_url(), - } - } -} - fn add_message_content_part( new_part: open_ai::MessagePart, role: Role, @@ -1446,7 +1433,9 @@ impl Render for ConfigurationView { mod tests { use futures::{StreamExt, executor::block_on}; use gpui::TestAppContext; - use language_model::{LanguageModelRequestMessage, LanguageModelRequestTool}; + use language_model::{ + LanguageModelRequestMessage, LanguageModelRequestTool, LanguageModelToolResult, + }; use open_ai::responses::{ ReasoningSummaryPart, ResponseFunctionToolCall, ResponseOutputItem, ResponseOutputMessage, ResponseReasoningItem, ResponseStatusDetails, ResponseSummary, ResponseUsage, @@ -1692,7 +1681,7 @@ mod tests { { "type": "function_call_output", "call_id": "call-42", - "output": "{\"forecast\":\"Sunny\"}" + "output": "Sunny" } ], "stream": true,