From ba50a61a3e7dffdce6eb2f48de8b49d63406a7f0 Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Wed, 22 Apr 2026 14:32:22 +0200 Subject: [PATCH 01/10] WIP --- crates/agent/src/db.rs | 2 +- crates/agent/src/edit_agent/evals.rs | 2 +- .../agent/src/tests/edit_file_thread_test.rs | 5 +- crates/agent/src/tests/mod.rs | 169 ++++++++++++++---- crates/agent/src/thread.rs | 53 +++--- .../src/tools/context_server_registry.rs | 39 +++- .../src/tools/evals/streaming_edit_file.rs | 2 +- crates/anthropic/src/completion.rs | 66 ++++--- crates/google_ai/src/completion.rs | 53 +++--- crates/language_model/src/fake_provider.rs | 8 +- crates/language_model_core/src/request.rs | 139 +++++++++++++- .../language_models/src/provider/bedrock.rs | 30 ++-- .../src/provider/copilot_chat.rs | 97 ++++++---- .../language_models/src/provider/deepseek.rs | 21 ++- .../language_models/src/provider/lmstudio.rs | 34 ++-- .../language_models/src/provider/mistral.rs | 18 +- crates/language_models/src/provider/ollama.rs | 2 +- .../src/provider/open_router.rs | 28 +-- crates/open_ai/src/completion.rs | 63 ++++--- 19 files changed, 605 insertions(+), 226 deletions(-) diff --git a/crates/agent/src/db.rs b/crates/agent/src/db.rs index bde07a040869bf..db7741c2643d46 100644 --- a/crates/agent/src/db.rs +++ b/crates/agent/src/db.rs @@ -261,7 +261,7 @@ impl DbThread { tool_use_id: tool_result.tool_use_id, tool_name: name.into(), is_error: tool_result.is_error, - content: tool_result.content, + content: vec![tool_result.content], output: tool_result.output, }, ); diff --git a/crates/agent/src/edit_agent/evals.rs b/crates/agent/src/edit_agent/evals.rs index ba8b7ed867ea26..bc72d07aba2701 100644 --- a/crates/agent/src/edit_agent/evals.rs +++ b/crates/agent/src/edit_agent/evals.rs @@ -1156,7 +1156,7 @@ fn tool_result( tool_use_id: LanguageModelToolUseId::from(id.into()), tool_name: name.into(), is_error: false, - content: LanguageModelToolResultContent::Text(result.into()), + content: vec![LanguageModelToolResultContent::Text(result.into())], output: None, }) } diff --git a/crates/agent/src/tests/edit_file_thread_test.rs b/crates/agent/src/tests/edit_file_thread_test.rs index b5ce6441e790e0..3efd7753740bc8 100644 --- a/crates/agent/src/tests/edit_file_thread_test.rs +++ b/crates/agent/src/tests/edit_file_thread_test.rs @@ -387,10 +387,7 @@ async fn test_streaming_edit_json_parse_error_does_not_cause_unsaved_changes( "Tool result should succeed, got: {:?}", tool_result ); - let content_text = match &tool_result.content { - language_model::LanguageModelToolResultContent::Text(t) => t.to_string(), - other => panic!("Expected text content, got: {:?}", other), - }; + let content_text = tool_result.text_contents(); assert!( !content_text.contains("file has been modified since you last read it"), "Did not expect a stale last-read error, got: {content_text}" diff --git a/crates/agent/src/tests/mod.rs b/crates/agent/src/tests/mod.rs index 16952e178aff86..eb73a7c1e4e292 100644 --- a/crates/agent/src/tests/mod.rs +++ b/crates/agent/src/tests/mod.rs @@ -637,7 +637,7 @@ async fn test_prompt_caching(cx: &mut TestAppContext) { tool_use_id: "tool_1".into(), tool_name: EchoTool::NAME.into(), is_error: false, - content: "test".into(), + content: vec!["test".into()], output: Some("test".into()), }; assert_eq!( @@ -866,14 +866,14 @@ async fn test_tool_authorization(cx: &mut TestAppContext) { tool_use_id: tool_call_auth_1.tool_call.tool_call_id.0.to_string().into(), tool_name: ToolRequiringPermission::NAME.into(), is_error: false, - content: "Allowed".into(), + content: vec!["Allowed".into()], output: Some("Allowed".into()) }), language_model::MessageContent::ToolResult(LanguageModelToolResult { tool_use_id: tool_call_auth_2.tool_call.tool_call_id.0.to_string().into(), tool_name: ToolRequiringPermission::NAME.into(), is_error: true, - content: "Permission to run tool denied by user".into(), + content: vec!["Permission to run tool denied by user".into()], output: Some("Permission to run tool denied by user".into()) }) ] @@ -912,7 +912,7 @@ async fn test_tool_authorization(cx: &mut TestAppContext) { tool_use_id: tool_call_auth_3.tool_call.tool_call_id.0.to_string().into(), tool_name: ToolRequiringPermission::NAME.into(), is_error: false, - content: "Allowed".into(), + content: vec!["Allowed".into()], output: Some("Allowed".into()) } )] @@ -940,7 +940,7 @@ async fn test_tool_authorization(cx: &mut TestAppContext) { tool_use_id: "tool_id_4".into(), tool_name: ToolRequiringPermission::NAME.into(), is_error: false, - content: "Allowed".into(), + content: vec!["Allowed".into()], output: Some("Allowed".into()) } )] @@ -1562,14 +1562,14 @@ async fn test_mcp_tools(cx: &mut TestAppContext) { tool_use_id: "tool_3".into(), tool_name: "echo".into(), is_error: false, - content: "native".into(), + content: vec!["native".into()], output: Some("native".into()), },), MessageContent::ToolResult(LanguageModelToolResult { tool_use_id: "tool_2".into(), tool_name: "test_server_echo".into(), is_error: false, - content: "mcp".into(), + content: vec!["mcp".into()], output: Some("mcp".into()), },), ] @@ -1578,6 +1578,125 @@ async fn test_mcp_tools(cx: &mut TestAppContext) { events.collect::>().await; } +#[gpui::test] +async fn test_mcp_tool_multi_content_response(cx: &mut TestAppContext) { + let ThreadTest { + model, + thread, + context_server_store, + fs, + .. + } = setup(cx, TestModel::Fake).await; + let fake_model = model.as_fake(); + fake_model.set_supports_images(true); + + fs.insert_file( + paths::settings_file(), + json!({ + "agent": { + "tool_permissions": { "default": "allow" }, + "profiles": { + "test": { + "name": "Test Profile", + "enable_all_context_servers": true, + "tools": {} + }, + } + } + }) + .to_string() + .into_bytes(), + ) + .await; + cx.run_until_parked(); + thread.update(cx, |thread, cx| { + thread.set_profile(AgentProfileId("test".into()), cx) + }); + + let mut mcp_tool_calls = setup_context_server( + "screenshot_server", + vec![context_server::types::Tool { + name: "screenshot".into(), + description: None, + input_schema: json!({"type": "object", "properties": {}}), + output_schema: None, + annotations: None, + }], + &context_server_store, + cx, + ); + + let events = thread.update(cx, |thread, cx| { + thread + .send(UserMessageId::new(), ["Take a screenshot"], cx) + .unwrap() + }); + cx.run_until_parked(); + + let completion = fake_model.pending_completions().pop().unwrap(); + fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse( + LanguageModelToolUse { + id: "tool_1".into(), + name: "screenshot".into(), + raw_input: json!({}).to_string(), + input: json!({}), + is_input_complete: true, + thought_signature: None, + }, + )); + fake_model.end_last_completion_stream(); + cx.run_until_parked(); + let _ = completion; + + let (tool_call_params, tool_call_response) = mcp_tool_calls.next().await.unwrap(); + assert_eq!(tool_call_params.name, "screenshot"); + tool_call_response + .send(context_server::types::CallToolResponse { + content: vec![ + context_server::types::ToolResponseContent::Text { + text: "Screenshot attached".into(), + }, + context_server::types::ToolResponseContent::Image { + data: "aGVsbG8=".into(), + mime_type: "image/png".into(), + }, + ], + is_error: None, + meta: None, + structured_content: None, + }) + .unwrap(); + cx.run_until_parked(); + + // Verify the tool result round-trips back to the model as a multi-part Vec. + let completion = fake_model.pending_completions().pop().unwrap(); + let tool_result = completion + .messages + .last() + .unwrap() + .content + .iter() + .find_map(|c| match c { + MessageContent::ToolResult(r) => Some(r.clone()), + _ => None, + }) + .expect("expected a tool result"); + assert_eq!(tool_result.tool_use_id, "tool_1".into()); + assert_eq!(tool_result.content.len(), 2); + assert_eq!( + tool_result.content[0], + language_model::LanguageModelToolResultContent::Text(Arc::from("Screenshot attached")) + ); + match &tool_result.content[1] { + language_model::LanguageModelToolResultContent::Image(image) => { + assert_eq!(image.source.as_ref(), "aGVsbG8="); + } + other => panic!("expected Image as second part, got: {:?}", other), + } + fake_model.end_last_completion_stream(); + events.collect::>().await; +} + #[gpui::test] async fn test_mcp_tool_result_displayed_when_server_disconnected(cx: &mut TestAppContext) { let ThreadTest { @@ -2106,10 +2225,7 @@ async fn test_terminal_tool_cancellation_captures_output(cx: &mut TestAppContext .get(&tool_use.id) .expect("expected tool result"); - let result_text = match &tool_result.content { - language_model::LanguageModelToolResultContent::Text(text) => text.to_string(), - _ => panic!("expected text content in tool result"), - }; + let result_text = tool_result.text_contents(); // "partial output" comes from FakeTerminalHandle's output field assert!( @@ -2571,10 +2687,7 @@ async fn test_terminal_tool_stopped_via_terminal_card_button(cx: &mut TestAppCon .get(&tool_use.id) .expect("expected tool result"); - let result_text = match &tool_result.content { - language_model::LanguageModelToolResultContent::Text(text) => text.to_string(), - _ => panic!("expected text content in tool result"), - }; + let result_text = tool_result.text_contents(); assert!( result_text.contains("The user stopped this command"), @@ -2666,10 +2779,7 @@ async fn test_terminal_tool_timeout_expires(cx: &mut TestAppContext) { .get(&tool_use.id) .expect("expected tool result"); - let result_text = match &tool_result.content { - language_model::LanguageModelToolResultContent::Text(text) => text.to_string(), - _ => panic!("expected text content in tool result"), - }; + let result_text = tool_result.text_contents(); assert!( result_text.contains("timed out"), @@ -3290,7 +3400,7 @@ async fn test_building_request_with_pending_tools(cx: &mut TestAppContext) { tool_use_id: echo_tool_use.id.clone(), tool_name: echo_tool_use.name, is_error: false, - content: "test".into(), + content: vec!["test".into()], output: Some("test".into()) })], cache: false, @@ -3776,7 +3886,7 @@ async fn test_send_retry_finishes_tool_calls_on_error(cx: &mut TestAppContext) { tool_use_id: tool_use_1.id.clone(), tool_name: tool_use_1.name.clone(), is_error: false, - content: "test".into(), + content: vec!["test".into()], output: Some("test".into()) } )], @@ -3936,8 +4046,10 @@ async fn test_streaming_tool_completes_when_llm_stream_ends_without_final_input( tool_use_id: tool_use.id.clone(), tool_name: tool_use.name, is_error: true, - content: "Failed to receive tool input: tool input was not fully received" - .into(), + content: vec![ + "Failed to receive tool input: tool input was not fully received" + .into(), + ], output: Some( "Failed to receive tool input: tool input was not fully received" .into() @@ -4044,10 +4156,7 @@ async fn test_streaming_tool_json_parse_error_is_forwarded_to_running_tool( let result = tool_results[0]; assert!(result.is_error); - let content_text = match &result.content { - language_model::LanguageModelToolResultContent::Text(text) => text.to_string(), - other => panic!("Expected text content, got {:?}", other), - }; + let content_text = result.text_contents(); assert!( content_text.contains("Saw partial text 'partial' before invalid JSON"), "Expected tool-enriched partial context, got: {content_text}" @@ -6680,7 +6789,7 @@ async fn test_streaming_tool_error_breaks_stream_loop_immediately(cx: &mut TestA tool_use_id: tool_use.id.clone(), tool_name: tool_use.name, is_error: true, - content: "failed".into(), + content: vec!["failed".into()], output: Some("failed".into()), } )], @@ -6791,14 +6900,14 @@ async fn test_streaming_tool_error_waits_for_prior_tools_to_complete(cx: &mut Te tool_use_id: second_tool_use.id.clone(), tool_name: second_tool_use.name, is_error: true, - content: "failed".into(), + content: vec!["failed".into()], output: Some("failed".into()), }), language_model::MessageContent::ToolResult(LanguageModelToolResult { tool_use_id: first_tool_use.id.clone(), tool_name: first_tool_use.name, is_error: false, - content: "hello world".into(), + content: vec!["hello world".into()], output: Some("hello world".into()), }), ], diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index 432c8c74a143e1..497182fbb0f0ac 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -516,12 +516,14 @@ impl AgentMessage { markdown.push_str("**ERROR:**\n"); } - match &tool_result.content { - LanguageModelToolResultContent::Text(text) => { - writeln!(markdown, "{text}\n").ok(); - } - LanguageModelToolResultContent::Image(_) => { - writeln!(markdown, "\n").ok(); + for part in &tool_result.content { + match part { + LanguageModelToolResultContent::Text(text) => { + writeln!(markdown, "{text}\n").ok(); + } + LanguageModelToolResultContent::Image(_) => { + writeln!(markdown, "\n").ok(); + } } } @@ -586,8 +588,8 @@ impl AgentMessage { let mut tool_result = tool_result.clone(); // Surprisingly, the API fails if we return an empty string here. // It thinks we are sending a tool use without a tool result. - if tool_result.content.is_empty() { - tool_result.content = "".into(); + if tool_result.is_content_empty() { + tool_result.content = vec!["".into()]; } user_message .content @@ -2330,7 +2332,7 @@ impl Thread { let Some(tool) = tool else { let content = format!("No tool named {} exists", tool_use.name); return Some(Task::ready(LanguageModelToolResult { - content: LanguageModelToolResultContent::Text(Arc::from(content)), + content: vec![LanguageModelToolResultContent::Text(Arc::from(content))], tool_use_id: tool_use.id, tool_name: tool_use.name, is_error: true, @@ -2416,9 +2418,11 @@ impl Thread { cx.foreground_executor().spawn(async move { let (is_error, output) = match tool_result.await { Ok(mut output) => { - if let LanguageModelToolResultContent::Image(_) = &output.llm_output - && !supports_images - { + let contains_image = output + .llm_output + .iter() + .any(|part| matches!(part, LanguageModelToolResultContent::Image(_))); + if contains_image && !supports_images { output = AgentToolOutput::from_error( "Attempted to read an image, but this model doesn't support it.", ); @@ -2470,7 +2474,7 @@ impl Thread { let Some(tool) = tool else { let content = format!("No tool named {} exists", tool_use.name); return Some(Task::ready(LanguageModelToolResult { - content: LanguageModelToolResultContent::Text(Arc::from(content)), + content: vec![LanguageModelToolResultContent::Text(Arc::from(content))], tool_use_id: tool_use.id, tool_name: tool_use.name, is_error: true, @@ -2741,7 +2745,9 @@ impl Thread { tool_use_id: tool_use.id.clone(), tool_name: tool_use.name.clone(), is_error: true, - content: LanguageModelToolResultContent::Text(TOOL_CANCELED_MESSAGE.into()), + content: vec![LanguageModelToolResultContent::Text( + TOOL_CANCELED_MESSAGE.into(), + )], output: None, }, ); @@ -3390,14 +3396,21 @@ where pub struct Erased(T); pub struct AgentToolOutput { - pub llm_output: LanguageModelToolResultContent, + /// Output formatted for presenting to the model. + /// + /// Typically a single-element `Vec` for built-in tools; the MCP bridge + /// is the only site that naturally emits multiple parts (e.g. text plus + /// an image) in a single result. + pub llm_output: Vec, pub raw_output: serde_json::Value, } impl AgentToolOutput { pub fn from_error(message: impl Into) -> Self { let message = message.into(); - let llm_output = LanguageModelToolResultContent::Text(Arc::from(message.as_str())); + let llm_output = vec![LanguageModelToolResultContent::Text(Arc::from( + message.as_str(), + ))]; Self { raw_output: serde_json::Value::String(message), llm_output, @@ -3482,7 +3495,7 @@ where AgentToolOutput::from_error(format!("Failed to serialize tool output: {e}")) })?; Ok(AgentToolOutput { - llm_output: output.into(), + llm_output: vec![output.into()], raw_output, }) } @@ -3492,7 +3505,7 @@ where serde_json::Value::Null }); Err(AgentToolOutput { - llm_output: error_output.into(), + llm_output: vec![error_output.into()], raw_output, }) } @@ -4410,8 +4423,8 @@ mod tests { assert_eq!(result.tool_use_id, tool_use_id); assert_eq!(result.tool_name, tool_name); assert!(matches!( - result.content, - LanguageModelToolResultContent::Text(_) + result.content.as_slice(), + [LanguageModelToolResultContent::Text(_)] )); thread.update(cx, |thread, _cx| { diff --git a/crates/agent/src/tools/context_server_registry.rs b/crates/agent/src/tools/context_server_registry.rs index df4cc313036b55..ba167b1cc9ef77 100644 --- a/crates/agent/src/tools/context_server_registry.rs +++ b/crates/agent/src/tools/context_server_registry.rs @@ -5,6 +5,7 @@ use collections::{BTreeMap, HashMap}; use context_server::{ContextServerId, client::NotificationSubscription}; use futures::FutureExt as _; use gpui::{App, AppContext, AsyncApp, Context, Entity, EventEmitter, SharedString, Task}; +use language_model::{LanguageModelImage, LanguageModelToolResultContent}; use project::context_server_store::{ContextServerStatus, ContextServerStore}; use std::sync::Arc; use util::ResultExt; @@ -389,14 +390,31 @@ impl AnyAgentTool for ContextServerTool { return Err(AgentToolOutput::from_error(error_message)); } - let mut result = String::new(); + let mut llm_output: Vec = Vec::new(); + let mut concatenated_text = String::new(); + let mut has_non_text = false; for content in response.content { match content { context_server::types::ToolResponseContent::Text { text } => { - result.push_str(&text); + concatenated_text.push_str(&text); + llm_output.push(LanguageModelToolResultContent::Text(text.into())); } - context_server::types::ToolResponseContent::Image { .. } => { - log::warn!("Ignoring image content from tool response"); + context_server::types::ToolResponseContent::Image { data, mime_type } => { + // `LanguageModelImage` is currently PNG-only; drop other + // mime types with the existing warning behavior. + if mime_type == "image/png" { + has_non_text = true; + llm_output.push(LanguageModelToolResultContent::Image( + LanguageModelImage { + source: data.into(), + size: None, + }, + )); + } else { + log::warn!( + "Ignoring image content from tool response with unsupported mime type: {mime_type}" + ); + } } context_server::types::ToolResponseContent::Audio { .. } => { log::warn!("Ignoring audio content from tool response"); @@ -406,9 +424,18 @@ impl AnyAgentTool for ContextServerTool { } } } + // Preserve the pre-refactor `raw_output` shape when the response only + // contained text parts, so existing replays keep deserializing the + // same way. When there are non-text parts too, we fall back to + // serializing each LLM-visible content part. + let raw_output = if has_non_text { + serde_json::to_value(&llm_output).unwrap_or(serde_json::Value::Null) + } else { + serde_json::Value::String(concatenated_text) + }; Ok(AgentToolOutput { - raw_output: result.clone().into(), - llm_output: result.into(), + raw_output, + llm_output, }) }) } diff --git a/crates/agent/src/tools/evals/streaming_edit_file.rs b/crates/agent/src/tools/evals/streaming_edit_file.rs index 0c6290ec098f9c..ec6249bc87806f 100644 --- a/crates/agent/src/tools/evals/streaming_edit_file.rs +++ b/crates/agent/src/tools/evals/streaming_edit_file.rs @@ -666,7 +666,7 @@ fn tool_result( tool_use_id: LanguageModelToolUseId::from(id.into()), tool_name: name.into(), is_error: false, - content: LanguageModelToolResultContent::Text(result.into()), + content: vec![LanguageModelToolResultContent::Text(result.into())], output: None, }) } diff --git a/crates/anthropic/src/completion.rs b/crates/anthropic/src/completion.rs index a6175a4f7c24b3..023eb0976c003c 100644 --- a/crates/anthropic/src/completion.rs +++ b/crates/anthropic/src/completion.rs @@ -70,25 +70,41 @@ fn to_anthropic_content(content: MessageContent) -> Option { input: tool_use.input, cache_control: None, }), - MessageContent::ToolResult(tool_result) => Some(RequestContent::ToolResult { - tool_use_id: tool_result.tool_use_id.to_string(), - is_error: tool_result.is_error, - content: match tool_result.content { - LanguageModelToolResultContent::Text(text) => { + MessageContent::ToolResult(tool_result) => { + // Preserve the existing on-wire shape: a single `Text` part becomes + // `ToolResultContent::Plain`; anything else (multiple parts, or any + // non-text part) becomes `ToolResultContent::Multipart`. + let content = match tool_result.content.as_slice() { + [LanguageModelToolResultContent::Text(text)] => { ToolResultContent::Plain(text.to_string()) } - LanguageModelToolResultContent::Image(image) => { - ToolResultContent::Multipart(vec![ToolResultPart::Image { - source: ImageSource { - source_type: "base64".to_string(), - media_type: "image/png".to_string(), - data: image.source.to_string(), - }, - }]) + _ => { + let parts = tool_result + .content + .into_iter() + .map(|part| match part { + LanguageModelToolResultContent::Text(text) => ToolResultPart::Text { + text: text.to_string(), + }, + LanguageModelToolResultContent::Image(image) => ToolResultPart::Image { + source: ImageSource { + source_type: "base64".to_string(), + media_type: "image/png".to_string(), + data: image.source.to_string(), + }, + }, + }) + .collect(); + ToolResultContent::Multipart(parts) } - }, - cache_control: None, - }), + }; + Some(RequestContent::ToolResult { + tool_use_id: tool_result.tool_use_id.to_string(), + is_error: tool_result.is_error, + content, + cache_control: None, + }) + } } } @@ -207,14 +223,18 @@ pub fn count_anthropic_tokens_with_tiktoken(request: LanguageModelRequest) -> Re MessageContent::ToolUse(_tool_use) => { // TODO: Estimate token usage from tool uses. } - MessageContent::ToolResult(tool_result) => match &tool_result.content { - LanguageModelToolResultContent::Text(text) => { - string_contents.push_str(text); - } - LanguageModelToolResultContent::Image(image) => { - tokens_from_images += image.estimate_tokens(); + MessageContent::ToolResult(tool_result) => { + for part in &tool_result.content { + match part { + LanguageModelToolResultContent::Text(text) => { + string_contents.push_str(text); + } + LanguageModelToolResultContent::Image(image) => { + tokens_from_images += image.estimate_tokens(); + } + } } - }, + } } } diff --git a/crates/google_ai/src/completion.rs b/crates/google_ai/src/completion.rs index 3a15fdaa0187e5..4682d08eeeb515 100644 --- a/crates/google_ai/src/completion.rs +++ b/crates/google_ai/src/completion.rs @@ -70,38 +70,39 @@ pub fn into_google( })] } MessageContent::ToolResult(tool_result) => { - match tool_result.content { - language_model_core::LanguageModelToolResultContent::Text(text) => { - vec![Part::FunctionResponsePart(crate::FunctionResponsePart { - function_response: crate::FunctionResponse { - name: tool_result.tool_name.to_string(), - // The API expects a valid JSON object - response: serde_json::json!({ - "output": text - }), - }, - })] - } - language_model_core::LanguageModelToolResultContent::Image(image) => { - vec![ - Part::FunctionResponsePart(crate::FunctionResponsePart { - function_response: crate::FunctionResponse { - name: tool_result.tool_name.to_string(), - // The API expects a valid JSON object - response: serde_json::json!({ - "output": "Tool responded with an image" - }), - }, - }), - Part::InlineDataPart(InlineDataPart { + let mut text_output = String::new(); + let mut images: Vec = Vec::new(); + for part in tool_result.content { + match part { + language_model_core::LanguageModelToolResultContent::Text(text) => { + text_output.push_str(&text); + } + language_model_core::LanguageModelToolResultContent::Image(image) => { + images.push(InlineDataPart { inline_data: GenerativeContentBlob { mime_type: "image/png".to_string(), data: image.source.to_string(), }, - }), - ] + }); + } } } + let output = if text_output.is_empty() && !images.is_empty() { + "Tool responded with an image".to_string() + } else { + text_output + }; + let mut parts = vec![Part::FunctionResponsePart(crate::FunctionResponsePart { + function_response: crate::FunctionResponse { + name: tool_result.tool_name.to_string(), + // The API expects a valid JSON object + response: serde_json::json!({ + "output": output + }), + }, + })]; + parts.extend(images.into_iter().map(Part::InlineDataPart)); + parts } }) .collect() diff --git a/crates/language_model/src/fake_provider.rs b/crates/language_model/src/fake_provider.rs index cee65c21e575e7..8329543308db97 100644 --- a/crates/language_model/src/fake_provider.rs +++ b/crates/language_model/src/fake_provider.rs @@ -125,6 +125,7 @@ pub struct FakeLanguageModel { forbid_requests: AtomicBool, supports_thinking: AtomicBool, supports_streaming_tools: AtomicBool, + supports_images: AtomicBool, } impl Default for FakeLanguageModel { @@ -138,6 +139,7 @@ impl Default for FakeLanguageModel { forbid_requests: AtomicBool::new(false), supports_thinking: AtomicBool::new(false), supports_streaming_tools: AtomicBool::new(false), + supports_images: AtomicBool::new(false), } } } @@ -174,6 +176,10 @@ impl FakeLanguageModel { self.supports_streaming_tools.store(supports, SeqCst); } + pub fn set_supports_images(&self, supports: bool) { + self.supports_images.store(supports, SeqCst); + } + pub fn pending_completions(&self) -> Vec { self.current_completion_txs .lock() @@ -280,7 +286,7 @@ impl LanguageModel for FakeLanguageModel { } fn supports_images(&self) -> bool { - false + self.supports_images.load(SeqCst) } fn supports_thinking(&self) -> bool { diff --git a/crates/language_model_core/src/request.rs b/crates/language_model_core/src/request.rs index a35f4883389f0a..3c2daab53b3064 100644 --- a/crates/language_model_core/src/request.rs +++ b/crates/language_model_core/src/request.rs @@ -102,12 +102,79 @@ 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 tool output formatted for presenting to the model. + /// + /// Multiple parts allow tools (most notably MCP tools) to return a + /// combination of text and non-text content (e.g. images) in a single + /// result. Built-in tools still typically produce a single-element `Vec`. + #[serde(with = "tool_result_content_vec")] + pub content: Vec, /// The raw tool output, if available, often for debugging or extra state for replay pub output: Option, } +impl LanguageModelToolResult { + /// Concatenates all `Text` parts of the content, ignoring non-text parts. + pub fn text_contents(&self) -> String { + let mut buffer = String::new(); + for part in &self.content { + if let LanguageModelToolResultContent::Text(text) = part { + buffer.push_str(text); + } + } + buffer + } + + /// Returns true when there are no content parts, or every part is empty. + pub fn is_content_empty(&self) -> bool { + self.content.iter().all(|part| part.is_empty()) + } +} + +/// Serde helper that accepts both the legacy single-value shape and the new +/// array shape for `LanguageModelToolResult::content`, and normalizes both to +/// `Vec`. +mod tool_result_content_vec { + use super::LanguageModelToolResultContent; + use serde::{Deserialize, Deserializer, Serialize, Serializer}; + + pub fn serialize( + value: &Vec, + serializer: S, + ) -> Result + where + S: Serializer, + { + value.serialize(serializer) + } + + pub fn deserialize<'de, D>( + deserializer: D, + ) -> Result, D::Error> + where + D: Deserializer<'de>, + { + let value = serde_json::Value::deserialize(deserializer)?; + match value { + serde_json::Value::Array(items) => { + let mut out = Vec::with_capacity(items.len()); + for item in items { + out.push( + serde_json::from_value::(item) + .map_err(serde::de::Error::custom)?, + ); + } + Ok(out) + } + other => { + let single = serde_json::from_value::(other) + .map_err(serde::de::Error::custom)?; + Ok(vec![single]) + } + } + } +} + #[derive(Debug, Clone, Serialize, Eq, PartialEq, Hash)] pub enum LanguageModelToolResultContent { Text(Arc), @@ -236,7 +303,11 @@ impl MessageContent { MessageContent::Text(text) => Some(text.as_str()), MessageContent::Thinking { text, .. } => Some(text.as_str()), MessageContent::RedactedThinking(_) => None, - MessageContent::ToolResult(tool_result) => tool_result.content.to_str(), + MessageContent::ToolResult(tool_result) => { + // Only return the first Text part as a borrowed string; multi-part + // callers should use `LanguageModelToolResult::text_contents` instead. + tool_result.content.iter().find_map(|part| part.to_str()) + } MessageContent::ToolUse(_) | MessageContent::Image(_) => None, } } @@ -245,7 +316,7 @@ impl MessageContent { match self { MessageContent::Text(text) => text.chars().all(|c| c.is_whitespace()), MessageContent::Thinking { text, .. } => text.chars().all(|c| c.is_whitespace()), - MessageContent::ToolResult(tool_result) => tool_result.content.is_empty(), + MessageContent::ToolResult(tool_result) => tool_result.is_content_empty(), MessageContent::RedactedThinking(_) | MessageContent::ToolUse(_) | MessageContent::Image(_) => false, @@ -462,4 +533,64 @@ mod tests { _ => panic!("Expected Image variant"), } } + + #[test] + fn test_language_model_tool_result_content_vec_deserialization() { + // Legacy single-value shape is normalized to a Vec. + let json = serde_json::json!({ + "tool_use_id": "abc", + "tool_name": "echo", + "is_error": false, + "content": "hello", + "output": null, + }); + let result: LanguageModelToolResult = serde_json::from_value(json).unwrap(); + assert_eq!( + result.content, + vec![LanguageModelToolResultContent::Text(Arc::from("hello"))] + ); + + // Legacy wrapped single-value shape also works. + let json = serde_json::json!({ + "tool_use_id": "abc", + "tool_name": "echo", + "is_error": false, + "content": {"type": "text", "text": "hello"}, + "output": null, + }); + let result: LanguageModelToolResult = serde_json::from_value(json).unwrap(); + assert_eq!( + result.content, + vec![LanguageModelToolResultContent::Text(Arc::from("hello"))] + ); + + // New array shape with text + image deserializes into a Vec. + let json = serde_json::json!({ + "tool_use_id": "abc", + "tool_name": "echo", + "is_error": false, + "content": [ + {"type": "text", "text": "foo"}, + {"source": "data", "size": {"width": 1, "height": 2}} + ], + "output": null, + }); + let result: LanguageModelToolResult = serde_json::from_value(json).unwrap(); + assert_eq!(result.content.len(), 2); + assert_eq!( + result.content[0], + LanguageModelToolResultContent::Text(Arc::from("foo")) + ); + match &result.content[1] { + LanguageModelToolResultContent::Image(image) => { + assert_eq!(image.source.as_ref(), "data"); + } + _ => panic!("Expected Image variant"), + } + + // Round-tripping preserves multi-part content. + let roundtripped: LanguageModelToolResult = + serde_json::from_value(serde_json::to_value(&result).unwrap()).unwrap(); + assert_eq!(roundtripped, result); + } } diff --git a/crates/language_models/src/provider/bedrock.rs b/crates/language_models/src/provider/bedrock.rs index 80c758769cd990..77e7525c0cd026 100644 --- a/crates/language_models/src/provider/bedrock.rs +++ b/crates/language_models/src/provider/bedrock.rs @@ -926,9 +926,10 @@ pub fn into_bedrock( } MessageContent::ToolResult(tool_result) => { messages_contain_tool_content = true; - BedrockToolResultBlock::builder() - .tool_use_id(tool_result.tool_use_id.to_string()) - .content(match tool_result.content { + let mut builder = BedrockToolResultBlock::builder() + .tool_use_id(tool_result.tool_use_id.to_string()); + for part in tool_result.content { + let block = match part { LanguageModelToolResultContent::Text(text) => { BedrockToolResultContentBlock::Text(text.to_string()) } @@ -969,7 +970,10 @@ pub fn into_bedrock( } } } - }) + }; + builder = builder.content(block); + } + builder .status({ if tool_result.is_error { BedrockToolResultStatus::Error @@ -1180,14 +1184,18 @@ pub fn get_bedrock_tokens( MessageContent::ToolUse(_tool_use) => { // TODO: Estimate token usage from tool uses. } - MessageContent::ToolResult(tool_result) => match tool_result.content { - LanguageModelToolResultContent::Text(text) => { - string_contents.push_str(&text); - } - LanguageModelToolResultContent::Image(image) => { - tokens_from_images += image.estimate_tokens(); + MessageContent::ToolResult(tool_result) => { + for part in tool_result.content { + match part { + LanguageModelToolResultContent::Text(text) => { + string_contents.push_str(&text); + } + LanguageModelToolResultContent::Image(image) => { + tokens_from_images += image.estimate_tokens(); + } + } } - }, + } } } diff --git a/crates/language_models/src/provider/copilot_chat.rs b/crates/language_models/src/provider/copilot_chat.rs index 0d7d03c8c75421..7fc55cd67b0cc8 100644 --- a/crates/language_models/src/provider/copilot_chat.rs +++ b/crates/language_models/src/provider/copilot_chat.rs @@ -906,23 +906,40 @@ fn into_copilot_chat( Role::User => { for content in &message.content { if let MessageContent::ToolResult(tool_result) = content { - let content = match &tool_result.content { - LanguageModelToolResultContent::Text(text) => text.to_string().into(), - LanguageModelToolResultContent::Image(image) => { - if model.supports_vision() { - ChatMessageContent::Multipart(vec![ChatMessagePart::Image { - image_url: ImageUrl { - url: image.to_base64_url(), - }, - }]) - } else { - debug_panic!( - "This should be caught at {} level", - tool_result.tool_name - ); - "[Tool responded with an image, but this model does not support vision]".to_string().into() + let parts: Vec = tool_result + .content + .iter() + .map(|part| match part { + LanguageModelToolResultContent::Text(text) => { + ChatMessagePart::Text { + text: text.to_string(), + } } + LanguageModelToolResultContent::Image(image) => { + if model.supports_vision() { + ChatMessagePart::Image { + image_url: ImageUrl { + url: image.to_base64_url(), + }, + } + } else { + debug_panic!( + "This should be caught at {} level", + tool_result.tool_name + ); + ChatMessagePart::Text { + text: "[Tool responded with an image, but this model does not support vision]".to_string(), + } + } + } + }) + .collect(); + + let content = match parts.as_slice() { + [ChatMessagePart::Text { text }] => { + ChatMessageContent::Plain(text.clone()) } + _ => ChatMessageContent::Multipart(parts), }; messages.push(ChatMessage::Tool { @@ -1126,27 +1143,39 @@ fn into_copilot_responses( Role::User => { for content in &message.content { if let MessageContent::ToolResult(tool_result) = content { - let output = match &tool_result.content { - LanguageModelToolResultContent::Text(text) => { + let output = match tool_result.content.as_slice() { + [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( - "[Tool responded with an image, but this model does not support vision]".into(), - ) - } + _ => { + let parts = tool_result + .content + .iter() + .map(|part| match part { + LanguageModelToolResultContent::Text(text) => { + responses::ResponseInputContent::InputText { + text: text.to_string(), + } + } + LanguageModelToolResultContent::Image(image) => { + if model.supports_vision() { + 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::ResponseInputContent::InputText { + text: "[Tool responded with an image, but this model does not support vision]".to_string(), + } + } + } + }) + .collect(); + responses::ResponseFunctionOutput::Content(parts) } }; diff --git a/crates/language_models/src/provider/deepseek.rs b/crates/language_models/src/provider/deepseek.rs index f3dccd5cc1a2e1..309f476869f7ff 100644 --- a/crates/language_models/src/provider/deepseek.rs +++ b/crates/language_models/src/provider/deepseek.rs @@ -404,15 +404,20 @@ pub fn into_deepseek( } } MessageContent::ToolResult(tool_result) => { - match &tool_result.content { - LanguageModelToolResultContent::Text(text) => { - messages.push(deepseek::RequestMessage::Tool { - content: text.to_string(), - tool_call_id: tool_result.tool_use_id.to_string(), - }); + // DeepSeek's Chat Completions tool role only accepts a single + // string. Concatenate all text parts; non-text parts are dropped. + let mut text_content = String::new(); + for part in &tool_result.content { + if let LanguageModelToolResultContent::Text(text) = part { + text_content.push_str(text); } - LanguageModelToolResultContent::Image(_) => {} - }; + } + if !text_content.is_empty() { + messages.push(deepseek::RequestMessage::Tool { + content: text_content, + tool_call_id: tool_result.tool_use_id.to_string(), + }); + } } } } diff --git a/crates/language_models/src/provider/lmstudio.rs b/crates/language_models/src/provider/lmstudio.rs index a541da8cd8092d..e02ffe16995ba0 100644 --- a/crates/language_models/src/provider/lmstudio.rs +++ b/crates/language_models/src/provider/lmstudio.rs @@ -380,21 +380,25 @@ impl LmStudioLanguageModel { } } MessageContent::ToolResult(tool_result) => { - let content = match &tool_result.content { - LanguageModelToolResultContent::Text(text) => { - vec![lmstudio::MessagePart::Text { - text: text.to_string(), - }] - } - LanguageModelToolResultContent::Image(image) => { - vec![lmstudio::MessagePart::Image { - image_url: lmstudio::ImageUrl { - url: image.to_base64_url(), - detail: None, - }, - }] - } - }; + let content: Vec = tool_result + .content + .iter() + .map(|part| match part { + LanguageModelToolResultContent::Text(text) => { + lmstudio::MessagePart::Text { + text: text.to_string(), + } + } + LanguageModelToolResultContent::Image(image) => { + lmstudio::MessagePart::Image { + image_url: lmstudio::ImageUrl { + url: image.to_base64_url(), + detail: None, + }, + } + } + }) + .collect(); messages.push(lmstudio::ChatMessage::Tool { content: content.into(), diff --git a/crates/language_models/src/provider/mistral.rs b/crates/language_models/src/provider/mistral.rs index fdb0fb7b3a7f51..5e68f32cbbc646 100644 --- a/crates/language_models/src/provider/mistral.rs +++ b/crates/language_models/src/provider/mistral.rs @@ -416,12 +416,20 @@ pub fn into_mistral( // Tool use is not supported in User messages for Mistral } MessageContent::ToolResult(tool_result) => { - let tool_content = match &tool_result.content { - LanguageModelToolResultContent::Text(text) => text.to_string(), - LanguageModelToolResultContent::Image(_) => { - "[Tool responded with an image, but Zed doesn't support these in Mistral models yet]".to_string() + // Mistral's tool role only accepts a single string. Text parts + // are concatenated; each non-text part contributes a placeholder + // line (matching the original single-element behavior). + let mut tool_content = String::new(); + for part in &tool_result.content { + match part { + LanguageModelToolResultContent::Text(text) => { + tool_content.push_str(text); + } + LanguageModelToolResultContent::Image(_) => { + tool_content.push_str("[Tool responded with an image, but Zed doesn't support these in Mistral models yet]"); + } } - }; + } messages.push(mistral::RequestMessage::Tool { content: tool_content, tool_call_id: tool_result.tool_use_id.to_string(), diff --git a/crates/language_models/src/provider/ollama.rs b/crates/language_models/src/provider/ollama.rs index 49c326683a225b..1ba271b70c207d 100644 --- a/crates/language_models/src/provider/ollama.rs +++ b/crates/language_models/src/provider/ollama.rs @@ -363,7 +363,7 @@ impl OllamaLanguageModel { MessageContent::ToolResult(tool_result) => { messages.push(ChatMessage::Tool { tool_name: tool_result.tool_name.to_string(), - content: tool_result.content.to_str().unwrap_or("").to_string(), + content: tool_result.text_contents(), }) } _ => unreachable!("Only tool result should be extracted"), diff --git a/crates/language_models/src/provider/open_router.rs b/crates/language_models/src/provider/open_router.rs index fba3a6938aecf1..e8f43c0a5a5c20 100644 --- a/crates/language_models/src/provider/open_router.rs +++ b/crates/language_models/src/provider/open_router.rs @@ -473,18 +473,22 @@ pub fn into_open_router( } } MessageContent::ToolResult(tool_result) => { - let content = match &tool_result.content { - LanguageModelToolResultContent::Text(text) => { - vec![open_router::MessagePart::Text { - text: text.to_string(), - }] - } - LanguageModelToolResultContent::Image(image) => { - vec![open_router::MessagePart::Image { - image_url: image.to_base64_url(), - }] - } - }; + let content: Vec = tool_result + .content + .iter() + .map(|part| match part { + LanguageModelToolResultContent::Text(text) => { + open_router::MessagePart::Text { + text: text.to_string(), + } + } + LanguageModelToolResultContent::Image(image) => { + open_router::MessagePart::Image { + image_url: image.to_base64_url(), + } + } + }) + .collect(); messages.push(open_router::RequestMessage::Tool { content: content.into(), diff --git a/crates/open_ai/src/completion.rs b/crates/open_ai/src/completion.rs index 81fa79d35ee134..64517a3fde5b2a 100644 --- a/crates/open_ai/src/completion.rs +++ b/crates/open_ai/src/completion.rs @@ -89,21 +89,21 @@ pub fn into_open_ai( } } MessageContent::ToolResult(tool_result) => { - let content = match &tool_result.content { - LanguageModelToolResultContent::Text(text) => { - vec![MessagePart::Text { + let content: Vec = tool_result + .content + .iter() + .map(|part| match part { + LanguageModelToolResultContent::Text(text) => MessagePart::Text { text: text.to_string(), - }] - } - LanguageModelToolResultContent::Image(image) => { - vec![MessagePart::Image { + }, + LanguageModelToolResultContent::Image(image) => MessagePart::Image { image_url: ImageUrl { url: image.to_base64_url(), detail: None, }, - }] - } - }; + }, + }) + .collect(); messages.push(crate::RequestMessage::Tool { content: content.into(), @@ -255,21 +255,38 @@ fn append_message_to_response_items( } MessageContent::ToolResult(tool_result) => { flush_response_parts(&message.role, index, &mut content_parts, input_items); + // Preserve the existing on-wire shape: a single `Text` part + // becomes `ResponseFunctionCallOutputContent::Text`; anything + // else (multiple parts, or any non-text part) is flattened + // into the `List` variant. + let output = match tool_result.content.as_slice() { + [LanguageModelToolResultContent::Text(text)] => { + ResponseFunctionCallOutputContent::Text(text.to_string()) + } + _ => { + let parts = tool_result + .content + .into_iter() + .map(|part| match part { + LanguageModelToolResultContent::Text(text) => { + ResponseInputContent::Text { + text: text.to_string(), + } + } + LanguageModelToolResultContent::Image(image) => { + ResponseInputContent::Image { + image_url: image.to_base64_url(), + } + } + }) + .collect(); + ResponseFunctionCallOutputContent::List(parts) + } + }; input_items.push(ResponseInputItem::FunctionCallOutput( ResponseFunctionCallOutputItem { call_id: tool_result.tool_use_id.to_string(), - output: match tool_result.content { - LanguageModelToolResultContent::Text(text) => { - ResponseFunctionCallOutputContent::Text(text.to_string()) - } - LanguageModelToolResultContent::Image(image) => { - ResponseFunctionCallOutputContent::List(vec![ - ResponseInputContent::Image { - image_url: image.to_base64_url(), - }, - ]) - } - }, + output, }, )); } @@ -1007,7 +1024,7 @@ mod tests { tool_use_id: tool_call_id, tool_name: Arc::from("get_weather"), is_error: false, - content: LanguageModelToolResultContent::Text(Arc::from("Sunny")), + content: vec![LanguageModelToolResultContent::Text(Arc::from("Sunny"))], output: Some(json!({ "forecast": "Sunny" })), }; let user_image = LanguageModelImage { From 8709efe25555a9233ab585d0dd9411adb891ebf1 Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Wed, 22 Apr 2026 18:14:27 +0200 Subject: [PATCH 02/10] Delete tool-result-multi-content-plan.md --- docs/tool-result-multi-content-plan.md | 315 ------------------------- 1 file changed, 315 deletions(-) delete mode 100644 docs/tool-result-multi-content-plan.md diff --git a/docs/tool-result-multi-content-plan.md b/docs/tool-result-multi-content-plan.md deleted file mode 100644 index ba8d73f77387f0..00000000000000 --- a/docs/tool-result-multi-content-plan.md +++ /dev/null @@ -1,315 +0,0 @@ -# Implementation plan: multi-content tool results - -## Goal - -Change the `content` field on `LanguageModelToolResult` from a single -`LanguageModelToolResultContent` into a `Vec`, -so that a tool call can carry multiple pieces of content (e.g. a text summary -**and** an image). - -The motivating case is MCP: `CallToolResponse.content` is already a `Vec`, -and our bridge currently collapses it to the first text chunk and drops -everything else. - -```zed/crates/agent/src/tools/context_server_registry.rs#L392-L408 -let mut result = String::new(); -for content in response.content { - match content { - context_server::types::ToolResponseContent::Text { text } => { - result.push_str(&text); - } - context_server::types::ToolResponseContent::Image { .. } => { - log::warn!("Ignoring image content from tool response"); - } - context_server::types::ToolResponseContent::Audio { .. } => { - log::warn!("Ignoring audio content from tool response"); - } - context_server::types::ToolResponseContent::Resource { .. } => { - log::warn!("Ignoring resource content from tool response"); - } - } -} -``` - -After this change, MCP tools that return text + image deliver both to the -model (subject to that provider's vision support). Every other part of the -system keeps doing what it does today. - -## Current shape - -```zed/crates/language_model_core/src/request.rs#L100-L116 -#[derive(Debug, Clone, Serialize, Deserialize, Eq, PartialEq, Hash)] -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, -} - -#[derive(Debug, Clone, Serialize, Eq, PartialEq, Hash)] -pub enum LanguageModelToolResultContent { - Text(Arc), - Image(LanguageModelImage), -} -``` - -`LanguageModelToolResultContent` has a hand-rolled `Deserialize` that accepts -several wire shapes (plain string, `{"type":"text","text":...}`, -`{"text":...}`, `{"image":...}`, direct image). The new code must preserve -all of those, plus accept the new array shape on the `content` field. - -## Decisions - -These were agreed on up front. The rest of the plan flows from them. - -- **Audio and Resource MCP variants stay dropped.** No new variants are added - to `LanguageModelToolResultContent`. Audio and Resource parts from MCP - keep hitting the existing `log::warn!` and getting dropped. The Vec change - is strictly about text + image. -- **`AgentTool::Output` stays single-valued.** The existing bound - `type Output: … + Into` is kept as-is. - Built-in tools (`edit_file_tool`, `find_path_tool`, `read_file_tool`, - `spawn_agent_tool`, `streaming_edit_file_tool`, `web_search_tool`) do not - change. The `Thread` layer wraps each tool output as `vec![output.into()]` - when constructing the `LanguageModelToolResult`. Only the MCP bridge - produces multi-part results in this first iteration. -- **On-disk format: permissive deserializer, no version bump.** - `DbThread::VERSION` stays at `"0.3.0"`. The `content` field gets a - `#[serde(with = …)]` helper (or equivalent hand-rolled `Deserialize`) that - accepts both the old single-value shape and the new array shape and - normalizes to `Vec`. No explicit upgrade step, no migration pass. This - matches the pattern already used by - `LanguageModelToolResultContent::Deserialize`. -- **Keep existing per-provider behavior for non-text parts.** No new - placeholder strategy, no new `LanguageModel::can_send_image_in_tool_result` - knob. Each provider does structurally what it does today — we just teach - every call site to iterate the `Vec`. - -## Step 1 — type definition and helpers - -File: `crates/language_model_core/src/request.rs`. - -- Change the field: - - ``` - pub struct LanguageModelToolResult { - … - pub content: Vec, - … - } - ``` - -- Add a permissive deserializer for the `content` field (e.g. a `one_or_many` - module referenced by `#[serde(with = "one_or_many")]`) that accepts either: - - a single value in any of the shapes `LanguageModelToolResultContent` - already accepts, **or** - - a JSON array of those values, - - and normalizes both to `Vec`. - -- Add ergonomic conversions so existing construction sites keep compiling: - - ``` - impl From<&str> for Vec { … } - impl From for Vec { … } - impl From for Vec { … } - impl From for Vec { … } - ``` - -- Add `Vec`-level helpers on `LanguageModelToolResult`: - - ``` - impl LanguageModelToolResult { - /// Concatenates all `Text` parts; ignores non-text parts. - pub fn text_contents(&self) -> String { … } - - /// True when the `Vec` is empty or every part is empty. - pub fn is_content_empty(&self) -> bool { … } - } - ``` - -- Keep `LanguageModelToolResultContent::{to_str, is_empty}` as-is (they still - make sense on a single element). - -- Extend the existing `test_language_model_tool_result_content_deserialization` - test with cases that feed both the old and new shapes through - `serde_json::from_value::`, asserting the - post-deserialize form is always a `Vec`. - -## Step 2 — producer updates - -Every call site that builds a `LanguageModelToolResult` needs -`content: vec![…]` instead of `content: …`. The `From<…> for Vec<…>` impls -added in step 1 mean `content: "foo".into()` continues to compile. - -| File | Call site | Change | -| ----------------------------------------------------- | --------------------------------------------------------- | ------------------------------------ | -| `crates/agent/src/thread.rs` | `Thread::handle_tool_use_event` (unknown tool error) | wrap in `vec![…]` | -| `crates/agent/src/thread.rs` | `Thread::handle_tool_use_json_parse_error_event` | wrap | -| `crates/agent/src/thread.rs` | `Thread::run_tool` (lifts `AgentToolOutput::llm_output`) | wrap | -| `crates/agent/src/thread.rs` | `Thread::flush_pending_message` (`TOOL_CANCELED_MESSAGE`) | wrap | -| `crates/agent/src/thread.rs` | `AgentMessage::to_request` empty-content guard | see note below | -| `crates/agent/src/db.rs` | `DbThread::upgrade_from_agent_1` | `content: vec![tool_result.content]` | -| `crates/agent/src/tools/context_server_registry.rs` | `ContextServerTool::run` | see step 5 | -| `crates/agent/src/edit_agent/evals.rs` | `tool_result` helper | wrap | -| `crates/agent/src/tools/evals/streaming_edit_file.rs` | `tool_result` helper | wrap | -| `crates/agent/src/tests/**` | ~10 constructor sites | `"foo".into()` keeps working | -| `crates/open_ai/src/completion.rs` | `tests::into_open_ai_response_builds_complete_payload` | wrap | - -`AgentMessage::to_request` has this workaround today: - -```zed/crates/agent/src/thread.rs#L585-L591 -if tool_result.content.is_empty() { - tool_result.content = "".into(); -} -``` - -Becomes: if `tool_result.is_content_empty()`, replace with -`vec!["".into()]`. Keep the comment — the -underlying API still rejects empty tool results. - -Note that `legacy_thread::SerializedToolResult` (`content: -LanguageModelToolResultContent`) is **not** updated. It's the pre-`agent_1` -wire format; only the upgrade site in `db.rs` needs to wrap. - -## Step 3 — consumer updates (providers + UI) - -Every site that reads `tool_result.content` currently matches the enum -directly. They become a loop over the `Vec`. Two buckets: - -**Natively multi-part providers.** These already emit `Vec`-shaped -provider-side types; the diff is just "build up the `Vec` in a loop instead -of in a single `match`". - -| File | Provider-side type | -| -------------------------------------------------------------------------------- | ------------------------------------------------------------- | -| `crates/anthropic/src/completion.rs` (`to_anthropic_content`) | `ToolResultContent::Plain` / `Multipart(Vec)` | -| `crates/anthropic/src/completion.rs` (`count_anthropic_tokens_with_tiktoken`) | token summation | -| `crates/open_ai/src/completion.rs` (`append_message_to_response_items`) | `ResponseFunctionCallOutputContent::{Text, List}` | -| `crates/language_models/src/provider/bedrock.rs` (`into_bedrock`) | `BedrockToolResultBlock.content()` (already a list) | -| `crates/language_models/src/provider/bedrock.rs` (`get_bedrock_tokens`) | token summation | -| `crates/language_models/src/provider/copilot_chat.rs` (`into_copilot_chat`) | `ChatMessageContent::Multipart(Vec)` | -| `crates/language_models/src/provider/copilot_chat.rs` (`into_copilot_responses`) | `ResponseFunctionOutput::Content(Vec<…>)` | -| `crates/language_models/src/provider/lmstudio.rs` (`to_lmstudio_request`) | `Vec` | -| `crates/language_models/src/provider/open_router.rs` (`into_open_router`) | `Vec` | - -For Anthropic specifically: when the post-refactor `Vec` has exactly one -`Text` part, keep emitting `ToolResultContent::Plain(String)` so the on-wire -bytes are identical to today for all built-in tools. Fall through to -`Multipart` for `≥2` parts or any non-text part. - -**Text-only tool-message providers.** These can only carry a single string -in a tool message. Text parts are concatenated; non-text parts keep each -provider's current behavior. - -| File | Current non-text behavior | Post-refactor | -| ---------------------------------------------------------------------------- | --------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------- | -| `crates/open_ai/src/completion.rs` (`into_open_ai`, legacy Chat Completions) | "cheats" with `MessagePart` multipart | iterate, same cheat per element | -| `crates/google_ai/src/completion.rs` (`into_google::map_content`) | splits into `FunctionResponsePart` text + extra `InlineDataPart` for images | same split, but concatenate all text parts into the single `{"output": …}` string and emit one `InlineDataPart` per image | -| `crates/language_models/src/provider/deepseek.rs` (`into_deepseek`) | silently drops images | silently drops all non-text parts | -| `crates/language_models/src/provider/mistral.rs` (`into_mistral`) | emits `"[Tool responded with an image, but Zed doesn't support these yet]"` | emit the same placeholder per non-text part | -| `crates/language_models/src/provider/ollama.rs` (`to_ollama_request`) | `tool_result.content.to_str().unwrap_or("")` | use `tool_result.text_contents()` (joins all text parts) | - -Also consumer-side but not a provider: - -- `crates/agent/src/thread.rs::AgentMessage::to_markdown` — iterate and - render each part (each `Text` as a line; each `Image` as ``). - -## Step 4 — test-site pattern matches - -Around seven test assertions pattern-match on a single-value -`tool_result.content`: - -``` -match &tool_result.content { - language_model::LanguageModelToolResultContent::Text(text) => text.to_string(), - _ => panic!("expected text content in tool result"), -}; -``` - -Sites: - -- `crates/agent/src/tests/edit_file_thread_test.rs::test_streaming_edit_json_parse_error_does_not_cause_unsaved_changes` -- `crates/agent/src/tests/mod.rs::test_terminal_tool_cancellation_captures_output` -- `crates/agent/src/tests/mod.rs::test_terminal_tool_stopped_via_terminal_card_button` -- `crates/agent/src/tests/mod.rs::test_terminal_tool_timeout_expires` -- `crates/agent/src/tests/mod.rs::test_streaming_tool_json_parse_error_is_forwarded_to_running_tool` -- `crates/agent/src/thread.rs::tests::test_handle_tool_use_json_parse_error_adds_tool_use_to_content` -- `crates/remote_server/src/remote_editing_tests.rs::test_remote_agent_fs_tool_calls` -- `crates/zed/src/visual_test_runner.rs::run_agent_thread_view_test` - -Preferred form: replace with `tool_result.text_contents()` when the test -only cares about the textual content. Where a test needs to assert a specific -element count/shape, use a slice pattern: - -``` -match tool_result.content.as_slice() { - [language_model::LanguageModelToolResultContent::Text(text)] => text.to_string(), - _ => panic!("expected a single text part in tool result"), -} -``` - -Note that `ReadFileTool`'s `type Output = LanguageModelToolResultContent` -does **not** change (`AgentTool::Output` stays single-valued per Decision 2), -so test helpers like `error_text(content: LanguageModelToolResultContent)` in -`read_file_tool.rs` keep working unchanged. - -## Step 5 — flip the MCP bridge - -File: `crates/agent/src/tools/context_server_registry.rs::ContextServerTool::run`. - -This is the only intentional behavior change. - -- Build a `Vec` from `response.content`. -- Map `ToolResponseContent::Text { text }` → `LanguageModelToolResultContent::Text(text.into())`. -- Map `ToolResponseContent::Image { data, mime_type }` → `LanguageModelToolResultContent::Image(…)`. - - Only `image/png` is natively representable in `LanguageModelImage` today; - for other mime types, fall back to the existing drop-with-warning - behavior for this iteration. -- Keep the existing `log::warn!` for `Audio` and `Resource` per Decision 1. -- `AgentToolOutput.llm_output` stays single-valued (per Decision 2), but the - MCP bridge is special: it doesn't go through the `AgentTool::Output` → - `vec![output.into()]` wrapper. It constructs the `LanguageModelToolResult` - directly, which means it's the one place that naturally emits a multi-part - `Vec`. - - Concretely: the existing `AgentToolOutput { llm_output, raw_output }` - return path is for the "error / single-string summary" case. The - success-with-multiple-parts path needs to reach into `Thread::run_tool`'s - `LanguageModelToolResult` construction and pass through a `Vec`. Cleanest - option is to widen `AgentToolOutput.llm_output` to - `Vec` **only** inside the MCP bridge's - usage — the built-in tools never hit that path because they return - `Self::Output` which still lifts through `into()`. - -## Step 6 — verification - -- `./script/clippy` clean across the touched crates. -- All existing tests pass without behavior changes for built-in tools. -- New tests in `request.rs`: - - Deserializing `{"content": "foo", …}` (old shape) yields - `content: vec![Text("foo")]`. - - Deserializing `{"content": [{"type": "text", "text": "foo"}, {"source": "…"}], …}` - (new shape) yields `content: vec![Text(…), Image(…)]`. - - Round-tripping a thread through serialize/deserialize preserves - multi-part content. -- New test in `tests/mod.rs::test_mcp_tools` (or a sibling): MCP tool - returning `[Text, Image]` shows up as a `LanguageModelToolResult` with two - elements and an Anthropic request round-trips it as `Multipart`. - -## Out of scope for this change - -Captured here so they don't creep in: - -- Adding `Audio` / `Resource` variants to `LanguageModelToolResultContent`. -- Broadening `AgentTool::Output` to allow built-in tools to emit multi-part. - (Do this later if a concrete use case appears.) -- Any `LanguageModel` capability flag for "this provider supports image - content in tool results". -- Changing `acp_thread::ToolCallContent` / `ContentBlock`. Those are already - `Vec`-shaped for UI rendering and are distinct from the model-facing - `LanguageModelToolResultContent`. -- Bumping `DbThread::VERSION` or writing an explicit on-disk migration. From caa8f036c802e917a363adddf40da9675ec7b035 Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Wed, 22 Apr 2026 19:13:40 +0200 Subject: [PATCH 03/10] Cleanup --- crates/agent/src/tests/mod.rs | 127 ++++++++++++++++++ crates/agent/src/thread.rs | 32 ++++- .../src/tools/context_server_registry.rs | 25 ++-- crates/language_model_core/src/request.rs | 46 ++++++- .../language_models/src/provider/deepseek.rs | 34 +++-- .../language_models/src/provider/mistral.rs | 8 +- crates/open_ai/src/completion.rs | 2 +- 7 files changed, 243 insertions(+), 31 deletions(-) diff --git a/crates/agent/src/tests/mod.rs b/crates/agent/src/tests/mod.rs index eb73a7c1e4e292..97e29b663597f9 100644 --- a/crates/agent/src/tests/mod.rs +++ b/crates/agent/src/tests/mod.rs @@ -1697,6 +1697,133 @@ async fn test_mcp_tool_multi_content_response(cx: &mut TestAppContext) { events.collect::>().await; } +#[gpui::test] +async fn test_mcp_tool_multi_content_response_without_image_support(cx: &mut TestAppContext) { + let ThreadTest { + model, + thread, + context_server_store, + fs, + .. + } = setup(cx, TestModel::Fake).await; + let fake_model = model.as_fake(); + // Intentionally leave `supports_images` at its default of false. + + fs.insert_file( + paths::settings_file(), + json!({ + "agent": { + "tool_permissions": { "default": "allow" }, + "profiles": { + "test": { + "name": "Test Profile", + "enable_all_context_servers": true, + "tools": {} + }, + } + } + }) + .to_string() + .into_bytes(), + ) + .await; + cx.run_until_parked(); + thread.update(cx, |thread, cx| { + thread.set_profile(AgentProfileId("test".into()), cx) + }); + + let mut mcp_tool_calls = setup_context_server( + "screenshot_server", + vec![context_server::types::Tool { + name: "screenshot".into(), + description: None, + input_schema: json!({"type": "object", "properties": {}}), + output_schema: None, + annotations: None, + }], + &context_server_store, + cx, + ); + + let events = thread.update(cx, |thread, cx| { + thread + .send(UserMessageId::new(), ["Take a screenshot"], cx) + .unwrap() + }); + cx.run_until_parked(); + + let completion = fake_model.pending_completions().pop().unwrap(); + fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse( + LanguageModelToolUse { + id: "tool_1".into(), + name: "screenshot".into(), + raw_input: json!({}).to_string(), + input: json!({}), + is_input_complete: true, + thought_signature: None, + }, + )); + fake_model.end_last_completion_stream(); + cx.run_until_parked(); + let _ = completion; + + let (tool_call_params, tool_call_response) = mcp_tool_calls.next().await.unwrap(); + assert_eq!(tool_call_params.name, "screenshot"); + tool_call_response + .send(context_server::types::CallToolResponse { + content: vec![ + context_server::types::ToolResponseContent::Text { + text: "Screenshot attached".into(), + }, + context_server::types::ToolResponseContent::Image { + data: "aGVsbG8=".into(), + mime_type: "image/png".into(), + }, + ], + is_error: None, + meta: None, + structured_content: None, + }) + .unwrap(); + cx.run_until_parked(); + + // On a non-vision model, the image part should be replaced with a + // placeholder but the accompanying text should still be preserved rather + // than having the whole tool result replaced with an error. + let completion = fake_model.pending_completions().pop().unwrap(); + let tool_result = completion + .messages + .last() + .unwrap() + .content + .iter() + .find_map(|c| match c { + MessageContent::ToolResult(r) => Some(r.clone()), + _ => None, + }) + .expect("expected a tool result"); + assert!(!tool_result.is_error); + assert_eq!(tool_result.content.len(), 2); + assert_eq!( + tool_result.content[0], + language_model::LanguageModelToolResultContent::Text(Arc::from("Screenshot attached")) + ); + match &tool_result.content[1] { + language_model::LanguageModelToolResultContent::Text(text) => { + assert!( + text.contains("doesn't support images"), + "expected image placeholder text, got: {text}" + ); + } + other => panic!( + "expected a Text placeholder as second part, got: {:?}", + other + ), + } + fake_model.end_last_completion_stream(); + events.collect::>().await; +} + #[gpui::test] async fn test_mcp_tool_result_displayed_when_server_disconnected(cx: &mut TestAppContext) { let ThreadTest { diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index 497182fbb0f0ac..19b203d4106cf9 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -2423,10 +2423,34 @@ impl Thread { .iter() .any(|part| matches!(part, LanguageModelToolResultContent::Image(_))); if contains_image && !supports_images { - output = AgentToolOutput::from_error( - "Attempted to read an image, but this model doesn't support it.", - ); - (true, output) + // Replace each image part with an inline placeholder so + // any accompanying text is still presented to the model. + // If there's nothing else in the output, surface an error + // to match the pre-multi-part behavior for image-only + // tool results. + let placeholder = LanguageModelToolResultContent::Text(Arc::from( + "[Tool responded with an image, but this model doesn't support images]", + )); + let has_non_image = output + .llm_output + .iter() + .any(|part| !matches!(part, LanguageModelToolResultContent::Image(_))); + if has_non_image { + output.llm_output = output + .llm_output + .into_iter() + .map(|part| match part { + LanguageModelToolResultContent::Image(_) => placeholder.clone(), + other => other, + }) + .collect(); + (false, output) + } else { + let output = AgentToolOutput::from_error( + "Attempted to read an image, but this model doesn't support it.", + ); + (true, output) + } } else { (false, output) } diff --git a/crates/agent/src/tools/context_server_registry.rs b/crates/agent/src/tools/context_server_registry.rs index ba167b1cc9ef77..ff19094335094e 100644 --- a/crates/agent/src/tools/context_server_registry.rs +++ b/crates/agent/src/tools/context_server_registry.rs @@ -390,9 +390,9 @@ impl AnyAgentTool for ContextServerTool { return Err(AgentToolOutput::from_error(error_message)); } - let mut llm_output: Vec = Vec::new(); + let mut llm_output = Vec::new(); let mut concatenated_text = String::new(); - let mut has_non_text = false; + let mut image_count: usize = 0; for content in response.content { match content { context_server::types::ToolResponseContent::Text { text } => { @@ -403,7 +403,7 @@ impl AnyAgentTool for ContextServerTool { // `LanguageModelImage` is currently PNG-only; drop other // mime types with the existing warning behavior. if mime_type == "image/png" { - has_non_text = true; + image_count += 1; llm_output.push(LanguageModelToolResultContent::Image( LanguageModelImage { source: data.into(), @@ -424,14 +424,19 @@ impl AnyAgentTool for ContextServerTool { } } } - // Preserve the pre-refactor `raw_output` shape when the response only - // contained text parts, so existing replays keep deserializing the - // same way. When there are non-text parts too, we fall back to - // serializing each LLM-visible content part. - let raw_output = if has_non_text { - serde_json::to_value(&llm_output).unwrap_or(serde_json::Value::Null) - } else { + // `raw_output` is persisted alongside the thread, so avoid embedding + // raw base64 image bytes here (they're already in `llm_output`). + // When the response only contained text parts, preserve the + // pre-refactor shape so existing replays keep deserializing the + // same way. When there are image parts too, record a small summary + // instead of the full content. + let raw_output = if image_count == 0 { serde_json::Value::String(concatenated_text) + } else { + serde_json::json!({ + "text": concatenated_text, + "images": image_count, + }) }; Ok(AgentToolOutput { raw_output, diff --git a/crates/language_model_core/src/request.rs b/crates/language_model_core/src/request.rs index 3c2daab53b3064..02e82e355ade66 100644 --- a/crates/language_model_core/src/request.rs +++ b/crates/language_model_core/src/request.rs @@ -348,8 +348,24 @@ pub struct LanguageModelRequestMessage { impl LanguageModelRequestMessage { pub fn string_contents(&self) -> String { let mut buffer = String::new(); - for string in self.content.iter().filter_map(|content| content.to_str()) { - buffer.push_str(string); + for content in &self.content { + match content { + MessageContent::ToolResult(tool_result) => { + // Walk tool-result parts directly so we don't truncate + // multi-part results to the first `Text` part (which is what + // `MessageContent::to_str` returns for a borrowed string). + for part in &tool_result.content { + if let LanguageModelToolResultContent::Text(text) = part { + buffer.push_str(text); + } + } + } + other => { + if let Some(text) = other.to_str() { + buffer.push_str(text); + } + } + } } buffer } @@ -593,4 +609,30 @@ mod tests { serde_json::from_value(serde_json::to_value(&result).unwrap()).unwrap(); assert_eq!(roundtripped, result); } + + #[test] + fn test_string_contents_includes_all_tool_result_text_parts() { + let tool_result = LanguageModelToolResult { + tool_use_id: LanguageModelToolUseId::from("id".to_string()), + tool_name: Arc::from("tool"), + is_error: false, + content: vec![ + LanguageModelToolResultContent::Text(Arc::from("first ")), + LanguageModelToolResultContent::Image(LanguageModelImage::empty()), + LanguageModelToolResultContent::Text(Arc::from("second")), + ], + output: None, + }; + let message = LanguageModelRequestMessage { + role: Role::User, + content: vec![ + MessageContent::Text("prefix ".to_string()), + MessageContent::ToolResult(tool_result), + MessageContent::Text(" suffix".to_string()), + ], + cache: false, + reasoning_details: None, + }; + assert_eq!(message.string_contents(), "prefix first second suffix"); + } } diff --git a/crates/language_models/src/provider/deepseek.rs b/crates/language_models/src/provider/deepseek.rs index fd614f3c8a1637..2bc62a841806ee 100644 --- a/crates/language_models/src/provider/deepseek.rs +++ b/crates/language_models/src/provider/deepseek.rs @@ -379,19 +379,33 @@ pub fn into_deepseek( } MessageContent::ToolResult(tool_result) => { // DeepSeek's Chat Completions tool role only accepts a single - // string. Concatenate all text parts; non-text parts are dropped. - let mut text_content = String::new(); + // string. Concatenate all text parts with newline separators; + // non-text parts are replaced with a placeholder so the tool + // response is still present (the API rejects assistant + // `tool_calls` that aren't followed by matching tool messages). + let mut text_parts: Vec = Vec::new(); for part in &tool_result.content { - if let LanguageModelToolResultContent::Text(text) = part { - text_content.push_str(text); + match part { + LanguageModelToolResultContent::Text(text) => { + text_parts.push(text.to_string()); + } + LanguageModelToolResultContent::Image(_) => { + text_parts.push( + "[Tool responded with an image, but Zed doesn't support these in DeepSeek models yet]" + .to_string(), + ); + } } } - if !text_content.is_empty() { - messages.push(deepseek::RequestMessage::Tool { - content: text_content, - tool_call_id: tool_result.tool_use_id.to_string(), - }); - } + let content = if text_parts.is_empty() { + "".to_string() + } else { + text_parts.join("\n") + }; + messages.push(deepseek::RequestMessage::Tool { + content, + tool_call_id: tool_result.tool_use_id.to_string(), + }); } } } diff --git a/crates/language_models/src/provider/mistral.rs b/crates/language_models/src/provider/mistral.rs index f98e67dca71018..ab0927417de1d6 100644 --- a/crates/language_models/src/provider/mistral.rs +++ b/crates/language_models/src/provider/mistral.rs @@ -393,19 +393,19 @@ pub fn into_mistral( // Mistral's tool role only accepts a single string. Text parts // are concatenated; each non-text part contributes a placeholder // line (matching the original single-element behavior). - let mut tool_content = String::new(); + let mut text_parts: Vec = Vec::new(); for part in &tool_result.content { match part { LanguageModelToolResultContent::Text(text) => { - tool_content.push_str(text); + text_parts.push(text.to_string()); } LanguageModelToolResultContent::Image(_) => { - tool_content.push_str("[Tool responded with an image, but Zed doesn't support these in Mistral models yet]"); + text_parts.push("[Tool responded with an image, but Zed doesn't support these in Mistral models yet]".to_string()); } } } messages.push(mistral::RequestMessage::Tool { - content: tool_content, + content: text_parts.join("\n"), tool_call_id: tool_result.tool_use_id.to_string(), }); } diff --git a/crates/open_ai/src/completion.rs b/crates/open_ai/src/completion.rs index 068721da0d2b1a..e9c8962c0d4cd8 100644 --- a/crates/open_ai/src/completion.rs +++ b/crates/open_ai/src/completion.rs @@ -1651,7 +1651,7 @@ mod tests { tool_use_id: tool_use_id, tool_name: Arc::from("search"), is_error: false, - content: LanguageModelToolResultContent::Text(Arc::from("result")), + content: vec![LanguageModelToolResultContent::Text(Arc::from("result"))], output: None, }; let request = LanguageModelRequest { From 3786bc39bf2010cbe3f5434f38618b0c0e1a291c Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Fri, 24 Apr 2026 12:56:28 +0200 Subject: [PATCH 04/10] Remove comments --- crates/agent/src/thread.rs | 5 ----- crates/anthropic/src/completion.rs | 3 --- crates/language_model_core/src/request.rs | 5 ----- crates/open_ai/src/completion.rs | 4 ---- 4 files changed, 17 deletions(-) diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index 19b203d4106cf9..7cc407e0b725de 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -3420,11 +3420,6 @@ where pub struct Erased(T); pub struct AgentToolOutput { - /// Output formatted for presenting to the model. - /// - /// Typically a single-element `Vec` for built-in tools; the MCP bridge - /// is the only site that naturally emits multiple parts (e.g. text plus - /// an image) in a single result. pub llm_output: Vec, pub raw_output: serde_json::Value, } diff --git a/crates/anthropic/src/completion.rs b/crates/anthropic/src/completion.rs index f0ec81cfddf3e0..015f4a900bdede 100644 --- a/crates/anthropic/src/completion.rs +++ b/crates/anthropic/src/completion.rs @@ -71,9 +71,6 @@ fn to_anthropic_content(content: MessageContent) -> Option { cache_control: None, }), MessageContent::ToolResult(tool_result) => { - // Preserve the existing on-wire shape: a single `Text` part becomes - // `ToolResultContent::Plain`; anything else (multiple parts, or any - // non-text part) becomes `ToolResultContent::Multipart`. let content = match tool_result.content.as_slice() { [LanguageModelToolResultContent::Text(text)] => { ToolResultContent::Plain(text.to_string()) diff --git a/crates/language_model_core/src/request.rs b/crates/language_model_core/src/request.rs index 02e82e355ade66..db61485819f068 100644 --- a/crates/language_model_core/src/request.rs +++ b/crates/language_model_core/src/request.rs @@ -102,11 +102,6 @@ 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. - /// - /// Multiple parts allow tools (most notably MCP tools) to return a - /// combination of text and non-text content (e.g. images) in a single - /// result. Built-in tools still typically produce a single-element `Vec`. #[serde(with = "tool_result_content_vec")] pub content: Vec, /// The raw tool output, if available, often for debugging or extra state for replay diff --git a/crates/open_ai/src/completion.rs b/crates/open_ai/src/completion.rs index e9c8962c0d4cd8..4abc752c4d5b65 100644 --- a/crates/open_ai/src/completion.rs +++ b/crates/open_ai/src/completion.rs @@ -270,10 +270,6 @@ fn append_message_to_response_items( } MessageContent::ToolResult(tool_result) => { flush_response_parts(&message.role, index, &mut content_parts, input_items); - // Preserve the existing on-wire shape: a single `Text` part - // becomes `ResponseFunctionCallOutputContent::Text`; anything - // else (multiple parts, or any non-text part) is flattened - // into the `List` variant. let output = match tool_result.content.as_slice() { [LanguageModelToolResultContent::Text(text)] => { ResponseFunctionCallOutputContent::Text(text.to_string()) From 727ddf90bcdf042b6f6217e5744a9ab6e7a2fdf8 Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Fri, 24 Apr 2026 12:58:35 +0200 Subject: [PATCH 05/10] Remove MessageContent::to_str --- crates/agent/src/tests/mod.rs | 8 +++++-- crates/language_model_core/src/request.rs | 28 ++++++++--------------- 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/crates/agent/src/tests/mod.rs b/crates/agent/src/tests/mod.rs index 97e29b663597f9..641a2e023df7c2 100644 --- a/crates/agent/src/tests/mod.rs +++ b/crates/agent/src/tests/mod.rs @@ -494,7 +494,9 @@ async fn test_system_prompt(cx: &mut TestAppContext) { assert_eq!(pending_completion.messages[0].role, Role::System); let system_message = &pending_completion.messages[0]; - let system_prompt = system_message.content[0].to_str().unwrap(); + let MessageContent::Text(system_prompt) = &system_message.content[0] else { + panic!("Expected text content"); + }; assert!( system_prompt.contains("test-shell"), "unexpected system message: {:?}", @@ -530,7 +532,9 @@ async fn test_system_prompt_without_tools(cx: &mut TestAppContext) { assert_eq!(pending_completion.messages[0].role, Role::System); let system_message = &pending_completion.messages[0]; - let system_prompt = system_message.content[0].to_str().unwrap(); + let MessageContent::Text(system_prompt) = &system_message.content[0] else { + panic!("Expected text content"); + }; assert!( !system_prompt.contains("## Tool Use"), "unexpected system message: {:?}", diff --git a/crates/language_model_core/src/request.rs b/crates/language_model_core/src/request.rs index db61485819f068..75ae58dd077de9 100644 --- a/crates/language_model_core/src/request.rs +++ b/crates/language_model_core/src/request.rs @@ -293,20 +293,6 @@ pub enum MessageContent { } impl MessageContent { - pub fn to_str(&self) -> Option<&str> { - match self { - MessageContent::Text(text) => Some(text.as_str()), - MessageContent::Thinking { text, .. } => Some(text.as_str()), - MessageContent::RedactedThinking(_) => None, - MessageContent::ToolResult(tool_result) => { - // Only return the first Text part as a borrowed string; multi-part - // callers should use `LanguageModelToolResult::text_contents` instead. - tool_result.content.iter().find_map(|part| part.to_str()) - } - MessageContent::ToolUse(_) | MessageContent::Image(_) => None, - } - } - pub fn is_empty(&self) -> bool { match self { MessageContent::Text(text) => text.chars().all(|c| c.is_whitespace()), @@ -345,6 +331,12 @@ impl LanguageModelRequestMessage { let mut buffer = String::new(); for content in &self.content { match content { + MessageContent::Text(text) => { + buffer.push_str(text); + } + MessageContent::Thinking { text, .. } => { + buffer.push_str(text); + } MessageContent::ToolResult(tool_result) => { // Walk tool-result parts directly so we don't truncate // multi-part results to the first `Text` part (which is what @@ -355,11 +347,9 @@ impl LanguageModelRequestMessage { } } } - other => { - if let Some(text) = other.to_str() { - buffer.push_str(text); - } - } + MessageContent::RedactedThinking(_) + | MessageContent::ToolUse(_) + | MessageContent::Image(_) => {} } } buffer From 197cd24f2a1b6ef2e1525a176d16657a9c649e34 Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Fri, 24 Apr 2026 13:07:15 +0200 Subject: [PATCH 06/10] Cleanup --- crates/language_model_core/src/request.rs | 3 --- crates/language_models/src/provider/deepseek.rs | 10 +--------- crates/language_models/src/provider/mistral.rs | 3 --- 3 files changed, 1 insertion(+), 15 deletions(-) diff --git a/crates/language_model_core/src/request.rs b/crates/language_model_core/src/request.rs index 75ae58dd077de9..f352ce16d227d6 100644 --- a/crates/language_model_core/src/request.rs +++ b/crates/language_model_core/src/request.rs @@ -338,9 +338,6 @@ impl LanguageModelRequestMessage { buffer.push_str(text); } MessageContent::ToolResult(tool_result) => { - // Walk tool-result parts directly so we don't truncate - // multi-part results to the first `Text` part (which is what - // `MessageContent::to_str` returns for a borrowed string). for part in &tool_result.content { if let LanguageModelToolResultContent::Text(text) = part { buffer.push_str(text); diff --git a/crates/language_models/src/provider/deepseek.rs b/crates/language_models/src/provider/deepseek.rs index 2bc62a841806ee..a08cc25c7b5fb0 100644 --- a/crates/language_models/src/provider/deepseek.rs +++ b/crates/language_models/src/provider/deepseek.rs @@ -378,11 +378,6 @@ pub fn into_deepseek( } } MessageContent::ToolResult(tool_result) => { - // DeepSeek's Chat Completions tool role only accepts a single - // string. Concatenate all text parts with newline separators; - // non-text parts are replaced with a placeholder so the tool - // response is still present (the API rejects assistant - // `tool_calls` that aren't followed by matching tool messages). let mut text_parts: Vec = Vec::new(); for part in &tool_result.content { match part { @@ -390,10 +385,7 @@ pub fn into_deepseek( text_parts.push(text.to_string()); } LanguageModelToolResultContent::Image(_) => { - text_parts.push( - "[Tool responded with an image, but Zed doesn't support these in DeepSeek models yet]" - .to_string(), - ); + text_parts.push("[Tool responded with an image]".to_string()); } } } diff --git a/crates/language_models/src/provider/mistral.rs b/crates/language_models/src/provider/mistral.rs index ab0927417de1d6..403d94e9832178 100644 --- a/crates/language_models/src/provider/mistral.rs +++ b/crates/language_models/src/provider/mistral.rs @@ -390,9 +390,6 @@ pub fn into_mistral( // Tool use is not supported in User messages for Mistral } MessageContent::ToolResult(tool_result) => { - // Mistral's tool role only accepts a single string. Text parts - // are concatenated; each non-text part contributes a placeholder - // line (matching the original single-element behavior). let mut text_parts: Vec = Vec::new(); for part in &tool_result.content { match part { From 1eb5ba2dc10e1487a8d52df1235b1b5868981cfd Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Fri, 24 Apr 2026 13:25:38 +0200 Subject: [PATCH 07/10] Update context_server_registry.rs --- crates/agent/src/tools/context_server_registry.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/agent/src/tools/context_server_registry.rs b/crates/agent/src/tools/context_server_registry.rs index ff19094335094e..9547ffd593a495 100644 --- a/crates/agent/src/tools/context_server_registry.rs +++ b/crates/agent/src/tools/context_server_registry.rs @@ -426,10 +426,6 @@ impl AnyAgentTool for ContextServerTool { } // `raw_output` is persisted alongside the thread, so avoid embedding // raw base64 image bytes here (they're already in `llm_output`). - // When the response only contained text parts, preserve the - // pre-refactor shape so existing replays keep deserializing the - // same way. When there are image parts too, record a small summary - // instead of the full content. let raw_output = if image_count == 0 { serde_json::Value::String(concatenated_text) } else { From ad7628520684f895c7123df3bd2430e11ecaa205 Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Mon, 27 Apr 2026 14:05:32 +0200 Subject: [PATCH 08/10] Update context_server_registry.rs --- .../src/tools/context_server_registry.rs | 30 ++----------------- 1 file changed, 3 insertions(+), 27 deletions(-) diff --git a/crates/agent/src/tools/context_server_registry.rs b/crates/agent/src/tools/context_server_registry.rs index 9547ffd593a495..1362307929cdda 100644 --- a/crates/agent/src/tools/context_server_registry.rs +++ b/crates/agent/src/tools/context_server_registry.rs @@ -392,29 +392,14 @@ impl AnyAgentTool for ContextServerTool { let mut llm_output = Vec::new(); let mut concatenated_text = String::new(); - let mut image_count: usize = 0; for content in response.content { match content { context_server::types::ToolResponseContent::Text { text } => { concatenated_text.push_str(&text); llm_output.push(LanguageModelToolResultContent::Text(text.into())); } - context_server::types::ToolResponseContent::Image { data, mime_type } => { - // `LanguageModelImage` is currently PNG-only; drop other - // mime types with the existing warning behavior. - if mime_type == "image/png" { - image_count += 1; - llm_output.push(LanguageModelToolResultContent::Image( - LanguageModelImage { - source: data.into(), - size: None, - }, - )); - } else { - log::warn!( - "Ignoring image content from tool response with unsupported mime type: {mime_type}" - ); - } + context_server::types::ToolResponseContent::Image { .. } => { + log::warn!("Ignoring image content from tool response"); } context_server::types::ToolResponseContent::Audio { .. } => { log::warn!("Ignoring audio content from tool response"); @@ -424,16 +409,7 @@ impl AnyAgentTool for ContextServerTool { } } } - // `raw_output` is persisted alongside the thread, so avoid embedding - // raw base64 image bytes here (they're already in `llm_output`). - let raw_output = if image_count == 0 { - serde_json::Value::String(concatenated_text) - } else { - serde_json::json!({ - "text": concatenated_text, - "images": image_count, - }) - }; + let raw_output = serde_json::Value::String(concatenated_text); Ok(AgentToolOutput { raw_output, llm_output, From bdc7deab19d2e22178f14a4b7fba988aa2f6cf43 Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Mon, 27 Apr 2026 14:13:27 +0200 Subject: [PATCH 09/10] Update context_server_registry.rs --- crates/agent/src/tools/context_server_registry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/agent/src/tools/context_server_registry.rs b/crates/agent/src/tools/context_server_registry.rs index 1362307929cdda..40df1f80d031f3 100644 --- a/crates/agent/src/tools/context_server_registry.rs +++ b/crates/agent/src/tools/context_server_registry.rs @@ -5,7 +5,7 @@ use collections::{BTreeMap, HashMap}; use context_server::{ContextServerId, client::NotificationSubscription}; use futures::FutureExt as _; use gpui::{App, AppContext, AsyncApp, Context, Entity, EventEmitter, SharedString, Task}; -use language_model::{LanguageModelImage, LanguageModelToolResultContent}; +use language_model::LanguageModelToolResultContent; use project::context_server_store::{ContextServerStatus, ContextServerStore}; use std::sync::Arc; use util::ResultExt; From d83098cb3cfb27fa21b71540f008d7edeab3932e Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Mon, 27 Apr 2026 14:23:53 +0200 Subject: [PATCH 10/10] Update mod.rs --- crates/agent/src/tests/mod.rs | 140 ++-------------------------------- 1 file changed, 7 insertions(+), 133 deletions(-) diff --git a/crates/agent/src/tests/mod.rs b/crates/agent/src/tests/mod.rs index 641a2e023df7c2..37c9fb9d806661 100644 --- a/crates/agent/src/tests/mod.rs +++ b/crates/agent/src/tests/mod.rs @@ -1658,12 +1658,15 @@ async fn test_mcp_tool_multi_content_response(cx: &mut TestAppContext) { .send(context_server::types::CallToolResponse { content: vec![ context_server::types::ToolResponseContent::Text { - text: "Screenshot attached".into(), + text: "Some text".into(), }, context_server::types::ToolResponseContent::Image { data: "aGVsbG8=".into(), mime_type: "image/png".into(), }, + context_server::types::ToolResponseContent::Text { + text: "Some more text".into(), + }, ], is_error: None, meta: None, @@ -1689,141 +1692,12 @@ async fn test_mcp_tool_multi_content_response(cx: &mut TestAppContext) { assert_eq!(tool_result.content.len(), 2); assert_eq!( tool_result.content[0], - language_model::LanguageModelToolResultContent::Text(Arc::from("Screenshot attached")) + language_model::LanguageModelToolResultContent::Text(Arc::from("Some text")) ); - match &tool_result.content[1] { - language_model::LanguageModelToolResultContent::Image(image) => { - assert_eq!(image.source.as_ref(), "aGVsbG8="); - } - other => panic!("expected Image as second part, got: {:?}", other), - } - fake_model.end_last_completion_stream(); - events.collect::>().await; -} - -#[gpui::test] -async fn test_mcp_tool_multi_content_response_without_image_support(cx: &mut TestAppContext) { - let ThreadTest { - model, - thread, - context_server_store, - fs, - .. - } = setup(cx, TestModel::Fake).await; - let fake_model = model.as_fake(); - // Intentionally leave `supports_images` at its default of false. - - fs.insert_file( - paths::settings_file(), - json!({ - "agent": { - "tool_permissions": { "default": "allow" }, - "profiles": { - "test": { - "name": "Test Profile", - "enable_all_context_servers": true, - "tools": {} - }, - } - } - }) - .to_string() - .into_bytes(), - ) - .await; - cx.run_until_parked(); - thread.update(cx, |thread, cx| { - thread.set_profile(AgentProfileId("test".into()), cx) - }); - - let mut mcp_tool_calls = setup_context_server( - "screenshot_server", - vec![context_server::types::Tool { - name: "screenshot".into(), - description: None, - input_schema: json!({"type": "object", "properties": {}}), - output_schema: None, - annotations: None, - }], - &context_server_store, - cx, - ); - - let events = thread.update(cx, |thread, cx| { - thread - .send(UserMessageId::new(), ["Take a screenshot"], cx) - .unwrap() - }); - cx.run_until_parked(); - - let completion = fake_model.pending_completions().pop().unwrap(); - fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse( - LanguageModelToolUse { - id: "tool_1".into(), - name: "screenshot".into(), - raw_input: json!({}).to_string(), - input: json!({}), - is_input_complete: true, - thought_signature: None, - }, - )); - fake_model.end_last_completion_stream(); - cx.run_until_parked(); - let _ = completion; - - let (tool_call_params, tool_call_response) = mcp_tool_calls.next().await.unwrap(); - assert_eq!(tool_call_params.name, "screenshot"); - tool_call_response - .send(context_server::types::CallToolResponse { - content: vec![ - context_server::types::ToolResponseContent::Text { - text: "Screenshot attached".into(), - }, - context_server::types::ToolResponseContent::Image { - data: "aGVsbG8=".into(), - mime_type: "image/png".into(), - }, - ], - is_error: None, - meta: None, - structured_content: None, - }) - .unwrap(); - cx.run_until_parked(); - - // On a non-vision model, the image part should be replaced with a - // placeholder but the accompanying text should still be preserved rather - // than having the whole tool result replaced with an error. - let completion = fake_model.pending_completions().pop().unwrap(); - let tool_result = completion - .messages - .last() - .unwrap() - .content - .iter() - .find_map(|c| match c { - MessageContent::ToolResult(r) => Some(r.clone()), - _ => None, - }) - .expect("expected a tool result"); - assert!(!tool_result.is_error); - assert_eq!(tool_result.content.len(), 2); assert_eq!( - tool_result.content[0], - language_model::LanguageModelToolResultContent::Text(Arc::from("Screenshot attached")) + tool_result.content[1], + language_model::LanguageModelToolResultContent::Text(Arc::from("Some more text")) ); - match &tool_result.content[1] { - language_model::LanguageModelToolResultContent::Text(text) => { - assert!( - text.contains("doesn't support images"), - "expected image placeholder text, got: {text}" - ); - } - other => panic!( - "expected a Text placeholder as second part, got: {:?}", - other - ), - } fake_model.end_last_completion_stream(); events.collect::>().await; }