From 5593f82866f490735fe1b3d28f1fedba6acad7ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Mon, 5 Oct 2026 07:40:28 +0200 Subject: [PATCH 1/5] fix(llm): send tool result images in Responses function_call_output Both Responses providers sent only the text of a tool result, so screenshots never reached the model. function_call_output.output now becomes a list of input_text/input_image when the result carries images (verified live against the ChatGPT subscription WebSocket endpoint). The WebSocket provider also keeps an image's media type instead of labelling every image as PNG. --- crates/llm/src/openai_responses.rs | 80 +++++++++++++++++++++--- crates/llm/src/openai_responses_ws.rs | 88 +++++++++++++++++++++++++-- 2 files changed, 154 insertions(+), 14 deletions(-) diff --git a/crates/llm/src/openai_responses.rs b/crates/llm/src/openai_responses.rs index d06feb84..48579afe 100644 --- a/crates/llm/src/openai_responses.rs +++ b/crates/llm/src/openai_responses.rs @@ -351,7 +351,7 @@ enum ResponseInputItem { }, FunctionCallOutput { call_id: String, - output: String, + output: FunctionCallOutput, }, Reasoning { id: String, @@ -378,6 +378,34 @@ impl PromptCacheBreakpoint { } } +/// `output` of a `function_call_output` item: plain text, or a list of +/// input content when the tool result carries images (screenshots), which a +/// string cannot hold. +#[derive(Debug, Serialize)] +#[serde(untagged)] +enum FunctionCallOutput { + Text(String), + Content(Vec), +} + +impl From<&ToolResultContent> for FunctionCallOutput { + fn from(content: &ToolResultContent) -> Self { + if !content.has_images() { + return Self::Text(content.text_content().to_string()); + } + let mut items = vec![ResponseContentItem::input_text( + content.text_content().to_string(), + )]; + items.extend(content.images().iter().map(|image| { + ResponseContentItem::input_image(format!( + "data:{};base64,{}", + image.media_type, image.base64_data + )) + })); + Self::Content(items) + } +} + /// Content item within messages #[derive(Debug, Serialize)] #[serde(tag = "type", rename_all = "snake_case")] @@ -1730,7 +1758,7 @@ fn convert_structured_message( pending.flush(result); result.push(ResponseInputItem::FunctionCallOutput { call_id: tool_use_id.clone(), - output: content.text_content().to_string(), + output: content.into(), }); } ContentBlock::RedactedThinking { @@ -1845,15 +1873,47 @@ mod tests { ])]; let converted = client.convert_messages_with_cache(messages, false); - assert_eq!(converted.len(), 1); + assert_eq!( + serde_json::to_value(&converted).unwrap(), + serde_json::json!([{ + "type": "function_call_output", + "call_id": "test_id", + "output": "Tool output", + }]) + ); + } - match &converted[0] { - ResponseInputItem::FunctionCallOutput { call_id, output } => { - assert_eq!(call_id, "test_id"); - assert_eq!(output, "Tool output"); - } - _ => panic!("Expected FunctionCallOutput"), - } + #[test] + fn test_tool_result_images_travel_in_function_call_output() { + let client = OpenAIResponsesClient::new( + "test_key".to_string(), + "gpt-5".to_string(), + "https://api.openai.com/v1".to_string(), + ); + + let messages = vec![Message::new_user_content(vec![ + ContentBlock::new_tool_result_with_images( + "call_1", + "Screenshot taken", + vec![crate::types::ToolResultImage { + media_type: "image/jpeg".to_string(), + base64_data: "QUJD".to_string(), + }], + ), + ])]; + + let converted = client.convert_messages_with_cache(messages, false); + assert_eq!( + serde_json::to_value(&converted).unwrap(), + serde_json::json!([{ + "type": "function_call_output", + "call_id": "call_1", + "output": [ + {"type": "input_text", "text": "Screenshot taken"}, + {"type": "input_image", "image_url": "data:image/jpeg;base64,QUJD"}, + ], + }]) + ); } #[test] diff --git a/crates/llm/src/openai_responses_ws.rs b/crates/llm/src/openai_responses_ws.rs index 2feadacc..00bd881f 100644 --- a/crates/llm/src/openai_responses_ws.rs +++ b/crates/llm/src/openai_responses_ws.rs @@ -149,7 +149,7 @@ enum WsInputItem { }, FunctionCallOutput { call_id: String, - output: String, + output: FunctionCallOutput, }, Reasoning { id: String, @@ -158,6 +158,34 @@ enum WsInputItem { }, } +/// `output` of a `function_call_output` item: plain text, or a list of +/// input content when the tool result carries images (screenshots), which a +/// string cannot hold. +#[derive(Debug, Clone, Serialize)] +#[serde(untagged)] +enum FunctionCallOutput { + Text(String), + Content(Vec), +} + +impl From<&ToolResultContent> for FunctionCallOutput { + fn from(content: &ToolResultContent) -> Self { + if !content.has_images() { + return Self::Text(content.text_content().to_string()); + } + let mut items = vec![WsContentItem::input_text( + content.text_content().to_string(), + )]; + items.extend(content.images().iter().map(|image| { + WsContentItem::input_image(format!( + "data:{};base64,{}", + image.media_type, image.base64_data + )) + })); + Self::Content(items) + } +} + /// Content item within messages. #[derive(Debug, Clone, Serialize)] #[serde(tag = "type", rename_all = "snake_case")] @@ -1548,12 +1576,14 @@ fn convert_structured_message( pending.push(WsContentItem::input_text(text.clone()), None, items); } } - ContentBlock::Image { data, .. } => { + ContentBlock::Image { + media_type, data, .. + } => { // `input_image` is user content; assistant images (none of // the Responses providers produce any) are not round-tripped. if !is_assistant { pending.push( - WsContentItem::input_image(format!("data:image/png;base64,{}", data)), + WsContentItem::input_image(format!("data:{media_type};base64,{data}")), None, items, ); @@ -1605,7 +1635,7 @@ fn convert_structured_message( pending.flush(items); items.push(WsInputItem::FunctionCallOutput { call_id: tool_use_id.clone(), - output: content.text_content().to_string(), + output: content.into(), }); } } @@ -2209,6 +2239,56 @@ mod tests { assert!(json.get("phase").is_none()); } + #[test] + fn test_ws_tool_result_images_travel_in_function_call_output() { + let blocks = vec![ + ContentBlock::new_tool_result("call_0", "Clicked"), + ContentBlock::new_tool_result_with_images( + "call_1", + "Screenshot taken", + vec![crate::types::ToolResultImage { + media_type: "image/jpeg".to_string(), + base64_data: "QUJD".to_string(), + }], + ), + ]; + let mut items = Vec::new(); + convert_structured_message(&MessageRole::User, &blocks, &mut items); + + assert_eq!( + serde_json::to_value(&items).unwrap(), + serde_json::json!([ + {"type": "function_call_output", "call_id": "call_0", "output": "Clicked"}, + { + "type": "function_call_output", + "call_id": "call_1", + "output": [ + {"type": "input_text", "text": "Screenshot taken"}, + {"type": "input_image", "image_url": "data:image/jpeg;base64,QUJD"}, + ], + }, + ]) + ); + } + + #[test] + fn test_ws_user_image_keeps_its_media_type() { + let blocks = vec![ContentBlock::Image { + media_type: "image/jpeg".to_string(), + data: "QUJD".to_string(), + start_time: None, + end_time: None, + }]; + let mut items = Vec::new(); + convert_structured_message(&MessageRole::User, &blocks, &mut items); + + let json = serde_json::to_value(&items).unwrap(); + assert_eq!( + json[0]["content"][0]["image_url"], + "data:image/jpeg;base64,QUJD" + ); + } + fn ws_items_with_breakpoint(items: &[WsInputItem]) -> Vec { items .iter() From b654dbcd9d0191dbae199803cfc7b0e2b54114a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Mon, 5 Oct 2026 07:45:29 +0200 Subject: [PATCH 2/5] fix(llm): opt Responses function tools out of strict mode Without an explicit strict flag the Responses API normalizes tool schemas into strict mode, which makes every property required. GPT models then filled optional parameters with placeholders (tab_id "", duration 0, even coordinate [0, 0] next to a ref) that the tools cannot tell from real values. Both Responses providers now send strict: false through one shared helper, so every request (including side requests) keeps an identical tool prefix. --- crates/llm/src/openai_responses.rs | 57 +++++++++++++++++++++------ crates/llm/src/openai_responses_ws.rs | 20 +++------- 2 files changed, 50 insertions(+), 27 deletions(-) diff --git a/crates/llm/src/openai_responses.rs b/crates/llm/src/openai_responses.rs index 48579afe..1c2a0851 100644 --- a/crates/llm/src/openai_responses.rs +++ b/crates/llm/src/openai_responses.rs @@ -310,6 +310,22 @@ impl ModelCapabilities { } } +/// Render a tool definition as a Responses API function tool. +/// +/// Without an explicit `strict`, the Responses API normalizes the schema into +/// strict mode, which makes every property required: the model then fills +/// optional parameters with placeholders (`""`, `0`, `[0, 0]`) that the tools +/// cannot tell from real values. Opting out lets it omit them. +pub(crate) fn function_tool(tool: ToolDefinition) -> serde_json::Value { + serde_json::json!({ + "type": "function", + "name": tool.name, + "description": tool.description, + "parameters": tool.parameters, + "strict": false, + }) +} + /// Place an explicit cache breakpoint on the last breakpoint-eligible content /// block within `items[..end]`. /// @@ -1567,19 +1583,9 @@ impl LLMProvider for OpenAIResponsesClient { ); } - let tools = request.tools.map(|tools| { - tools - .into_iter() - .map(|tool| { - serde_json::json!({ - "type": "function", - "name": tool.name, - "description": tool.description, - "parameters": tool.parameters - }) - }) - .collect() - }); + let tools = request + .tools + .map(|tools| tools.into_iter().map(function_tool).collect()); // Configure for stateless mode with encrypted reasoning let store = false; @@ -1883,6 +1889,31 @@ mod tests { ); } + #[test] + fn test_function_tools_opt_out_of_strict_mode() { + let tool = function_tool(ToolDefinition { + name: "click".to_string(), + description: "Click".to_string(), + parameters: serde_json::json!({ + "type": "object", + "properties": {"ref": {"type": "string"}}, + }), + }); + assert_eq!( + tool, + serde_json::json!({ + "type": "function", + "name": "click", + "description": "Click", + "parameters": { + "type": "object", + "properties": {"ref": {"type": "string"}}, + }, + "strict": false, + }) + ); + } + #[test] fn test_tool_result_images_travel_in_function_call_output() { let client = OpenAIResponsesClient::new( diff --git a/crates/llm/src/openai_responses_ws.rs b/crates/llm/src/openai_responses_ws.rs index 00bd881f..befa5728 100644 --- a/crates/llm/src/openai_responses_ws.rs +++ b/crates/llm/src/openai_responses_ws.rs @@ -66,7 +66,9 @@ use tokio_tungstenite::{ use tracing::{debug, info, warn}; // Re-export types shared with the HTTP provider -use crate::openai_responses::{PromptCacheBreakpoint, Verbosity, model_supports_explicit_cache}; +use crate::openai_responses::{ + PromptCacheBreakpoint, Verbosity, function_tool, model_supports_explicit_cache, +}; // ============================================================================ // Request / Response types (WebSocket-specific envelope) @@ -973,19 +975,9 @@ impl OpenAIResponsesWsClient { None }; - let tools = request.tools.map(|tools| { - tools - .into_iter() - .map(|tool| { - serde_json::json!({ - "type": "function", - "name": tool.name, - "description": tool.description, - "parameters": tool.parameters, - }) - }) - .collect() - }); + let tools = request + .tools + .map(|tools| tools.into_iter().map(function_tool).collect()); let reasoning = if capabilities.supports_reasoning { Some(ReasoningConfig { From 067564d7050643e71fd56c93d00a4732d4d0a486 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Mon, 5 Oct 2026 08:00:03 +0200 Subject: [PATCH 3/5] fix(tools): treat empty placeholders in optional parameters as absent Models fill optional parameters with "", [] or null instead of omitting them: a tab_id "" then fails as an unknown tab, a coordinate [] as a malformed one. The schema coercion pass now drops such values from properties the schema does not list as required; 0, false and {} stay, since they are as likely to be meant. Browser batch steps go through the same coercion against the inner tool's schema, so a step behaves like the direct call. --- .../src/tools/impls/browser/batch.rs | 92 ++++++++++++--- crates/tools_core/src/coerce.rs | 108 +++++++++++++++++- 2 files changed, 177 insertions(+), 23 deletions(-) diff --git a/crates/code_assistant_core/src/tools/impls/browser/batch.rs b/crates/code_assistant_core/src/tools/impls/browser/batch.rs index e5fd8fff..e31e81c9 100644 --- a/crates/code_assistant_core/src/tools/impls/browser/batch.rs +++ b/crates/code_assistant_core/src/tools/impls/browser/batch.rs @@ -126,27 +126,68 @@ fn step_label(step: &BatchStep) -> String { } } +/// Parse a step's input the way a direct call of `tool` would: coerced +/// against that tool's schema first, so empty placeholders and lone scalars +/// behave the same inside a batch. +fn parse_step(step: &BatchStep, tool: T) -> Result +where + T::Input: DeserializeOwned, +{ + let mut input = step.input.clone(); + tools_core::coerce::coerce_to_schema(&mut input, &tool.spec().parameters_schema); + serde_json::from_value(input) + .map_err(|e| anyhow::anyhow!("invalid input for {}: {e}", step.name)) +} + async fn run_step(manager: &BrowserSessionManager, step: &BatchStep) -> Result { - fn parse(step: &BatchStep) -> Result { - serde_json::from_value(step.input.clone()) - .map_err(|e| anyhow::anyhow!("invalid input for {}: {e}", step.name)) - } let name = step.name.trim_start_matches("browser_"); Ok(match name { - "navigate" => page::navigate(manager, &parse(step)?).await, - "read_page" => page::read_page(manager, &parse(step)?).await, - "find" => page::find(manager, &parse(step)?).await, - "get_page_text" => page::get_page_text(manager, &parse(step)?).await, - "form_input" => page::form_input(manager, &parse(step)?).await, - "javascript" => page::javascript(manager, &parse(step)?).await, - "computer" => computer::computer(manager, &parse(step)?).await, - "read_console_messages" => devtools::read_console(manager, &parse(step)?).await, - "read_network_requests" => devtools::read_network(manager, &parse(step)?).await, - "resize_window" => tabs::resize_window(manager, &parse(step)?).await, - "tabs_context" => tabs::tabs_context(manager, &parse(step)?).await, - "tabs_create" => tabs::tabs_create(manager, &parse(step)?).await, - "tabs_select" => tabs::tabs_select(manager, &parse(step)?).await, - "tabs_close" => tabs::tabs_close(manager, &parse(step)?).await, + "navigate" => page::navigate(manager, &parse_step(step, page::BrowserNavigateTool)?).await, + "read_page" => { + page::read_page(manager, &parse_step(step, page::BrowserReadPageTool)?).await + } + "find" => page::find(manager, &parse_step(step, page::BrowserFindTool)?).await, + "get_page_text" => { + page::get_page_text(manager, &parse_step(step, page::BrowserGetPageTextTool)?).await + } + "form_input" => { + page::form_input(manager, &parse_step(step, page::BrowserFormInputTool)?).await + } + "javascript" => { + page::javascript(manager, &parse_step(step, page::BrowserJavascriptTool)?).await + } + "computer" => { + computer::computer(manager, &parse_step(step, computer::BrowserComputerTool)?).await + } + "read_console_messages" => { + devtools::read_console( + manager, + &parse_step(step, devtools::BrowserReadConsoleTool)?, + ) + .await + } + "read_network_requests" => { + devtools::read_network( + manager, + &parse_step(step, devtools::BrowserReadNetworkTool)?, + ) + .await + } + "resize_window" => { + tabs::resize_window(manager, &parse_step(step, tabs::BrowserResizeWindowTool)?).await + } + "tabs_context" => { + tabs::tabs_context(manager, &parse_step(step, tabs::BrowserTabsContextTool)?).await + } + "tabs_create" => { + tabs::tabs_create(manager, &parse_step(step, tabs::BrowserTabsCreateTool)?).await + } + "tabs_select" => { + tabs::tabs_select(manager, &parse_step(step, tabs::BrowserTabsSelectTool)?).await + } + "tabs_close" => { + tabs::tabs_close(manager, &parse_step(step, tabs::BrowserTabsCloseTool)?).await + } other => anyhow::bail!("browser_{other} cannot run in a batch"), }) } @@ -165,6 +206,21 @@ mod tests { } } + #[test] + fn a_step_drops_empty_placeholders_like_a_direct_call() -> Result<()> { + let input = parse_step( + &step( + "browser_computer", + json!({"action": "left_click", "ref": "ref_7", "coordinate": [], "tab_id": ""}), + ), + computer::BrowserComputerTool, + )?; + assert_eq!(input.r#ref.as_deref(), Some("ref_7")); + assert!(input.coordinate.is_none()); + assert!(input.target.tab_id.is_none()); + Ok(()) + } + #[tokio::test] async fn steps_run_in_order_and_stop_at_the_first_error() -> Result<()> { let page = data_url( diff --git a/crates/tools_core/src/coerce.rs b/crates/tools_core/src/coerce.rs index f244d824..ac4eed4a 100644 --- a/crates/tools_core/src/coerce.rs +++ b/crates/tools_core/src/coerce.rs @@ -8,9 +8,15 @@ //! schema and wraps such scalars in a one-element array, so the common mistake //! parses on the first try. //! -//! The coercion is deliberately conservative: it only ever *adds* an array -//! wrapper around a value the schema says should be an array, and it never -//! discards or reinterprets data. Because [`crate::dyn_tool::DynTool::invoke`] +//! Models also fill optional parameters with empty placeholders (`""`, `[]`, +//! `null`) instead of omitting them — a tab id `""` then fails as an unknown +//! tab, a coordinate `[]` as a malformed one. Such values are dropped from +//! properties the schema does not list as `required`, so the tool sees the +//! parameter as absent. +//! +//! Beyond that the coercion is deliberately conservative: it only ever *adds* +//! an array wrapper around a value the schema says should be an array, and it +//! never reinterprets data. Because [`crate::dyn_tool::DynTool::invoke`] //! re-serializes the parsed input back into `params`, a coerced value also //! becomes the canonical form recorded in the conversation history. @@ -52,8 +58,12 @@ pub fn coerce_to_schema(value: &mut Value, schema: &Value) { schema.get("properties").and_then(Value::as_object), value.as_object_mut(), ) { + let required = schema.get("required").and_then(Value::as_array); + let is_required = |key: &str| required.is_some_and(|r| r.iter().any(|k| k == key)); for (key, prop_schema) in props { - if let Some(field) = map.get_mut(key) { + if !is_required(key) && map.get(key).is_some_and(is_empty_placeholder) { + map.remove(key); + } else if let Some(field) = map.get_mut(key) { coerce_to_schema(field, prop_schema); } } @@ -63,6 +73,20 @@ pub fn coerce_to_schema(value: &mut Value, schema: &Value) { } } +/// Values a model emits for an optional parameter it means to leave out: +/// some providers (strict function calling) force every property to be +/// present, and models fill the unused ones with `""`, `[]` or `null`. +/// Numbers, booleans and objects are left alone, since `0`, `false` and `{}` +/// are as likely to be meant. +fn is_empty_placeholder(value: &Value) -> bool { + match value { + Value::Null => true, + Value::String(s) => s.is_empty(), + Value::Array(a) => a.is_empty(), + _ => false, + } +} + enum SchemaType { Array, Object, @@ -201,7 +225,8 @@ mod tests { fn does_not_wrap_null() { let schema = json!({ "type": "object", - "properties": {"paths": {"type": "array", "items": {"type": "string"}}} + "properties": {"paths": {"type": "array", "items": {"type": "string"}}}, + "required": ["paths"] }); let out = coerced(json!({"paths": null}), &schema); assert_eq!(out, json!({"paths": null})); @@ -289,6 +314,79 @@ mod tests { assert_eq!(out, json!({"value": "x"})); } + #[test] + fn drops_empty_placeholders_in_optional_fields() { + let schema = json!({ + "type": "object", + "properties": { + "action": {"type": "string"}, + "tab_id": {"type": "string"}, + "coordinate": {"type": "array", "items": {"type": "integer"}}, + "ref": {"type": "string"}, + }, + "required": ["action"] + }); + let out = coerced( + json!({"action": "left_click", "tab_id": "", "coordinate": [], "ref": null}), + &schema, + ); + assert_eq!(out, json!({"action": "left_click"})); + } + + #[test] + fn keeps_empty_values_in_required_fields() { + let schema = json!({ + "type": "object", + "properties": { + "old_text": {"type": "string"}, + "new_text": {"type": "string"}, + }, + "required": ["old_text", "new_text"] + }); + let out = coerced(json!({"old_text": "x", "new_text": ""}), &schema); + assert_eq!(out, json!({"old_text": "x", "new_text": ""})); + } + + #[test] + fn keeps_meaningful_falsy_values_in_optional_fields() { + let schema = json!({ + "type": "object", + "properties": { + "duration": {"type": "number"}, + "shift": {"type": "boolean"}, + "options": {"type": "object"}, + } + }); + let value = json!({"duration": 0, "shift": false, "options": {}}); + assert_eq!(coerced(value.clone(), &schema), value); + } + + #[test] + fn drops_empty_placeholders_in_nested_objects() { + let schema = json!({ + "type": "object", + "properties": { + "actions": { + "type": "array", + "items": { + "type": "object", + "properties": { + "name": {"type": "string"}, + "tab_id": {"type": "string"}, + }, + "required": ["name"] + } + } + }, + "required": ["actions"] + }); + let out = coerced( + json!({"actions": [{"name": "screenshot", "tab_id": ""}]}), + &schema, + ); + assert_eq!(out, json!({"actions": [{"name": "screenshot"}]})); + } + #[test] fn no_schema_is_noop() { let out = coerced(json!({"paths": "x"}), &json!(true)); From b87217d51ace1afd68ad64672fd6cc6411d9f3a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Mon, 5 Oct 2026 08:44:53 +0200 Subject: [PATCH 4/5] fix(llm): send tool result images as Gemini function response parts The Vertex/Gemini provider sent only the text of a tool result, so the model never saw screenshots and made up their content. Images now travel as functionResponse.parts with inlineData (multimodal function responses), verified live with Gemini 3.5 Flash. --- crates/llm/src/vertex.rs | 52 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/crates/llm/src/vertex.rs b/crates/llm/src/vertex.rs index 5ddd04a2..6f9f488c 100644 --- a/crates/llm/src/vertex.rs +++ b/crates/llm/src/vertex.rs @@ -258,6 +258,16 @@ struct VertexPartialArg { struct VertexFunctionResponse { name: String, response: serde_json::Value, + /// Media of a multimodal function response (screenshots), which the JSON + /// `response` cannot carry. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + parts: Vec, +} + +#[derive(Debug, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +struct VertexFunctionResponsePart { + inline_data: VertexInlineData, } /// Rate limit information extracted from response headers @@ -955,8 +965,17 @@ impl VertexClient { .unwrap_or(tool_use_id) .to_string() }), - // Wrap content in a proper JSON object (text only) response: json!({ "result": content.text_content() }), + parts: content + .images() + .iter() + .map(|image| VertexFunctionResponsePart { + inline_data: VertexInlineData { + mime_type: image.media_type.clone(), + data: image.base64_data.clone(), + }, + }) + .collect(), }), }), _ => None, @@ -1817,6 +1836,37 @@ mod tests { ); } + #[test] + fn tool_result_images_travel_as_function_response_parts() { + use std::collections::HashMap; + + let mut tool_names = HashMap::new(); + tool_names.insert("tool-1-1".to_string(), "browser_computer".to_string()); + let message = Message::new_user_content(vec![ + ContentBlock::new_tool_result("tool-1-0", "Clicked"), + ContentBlock::new_tool_result_with_images( + "tool-1-1", + "Screenshot taken", + vec![crate::types::ToolResultImage { + media_type: "image/jpeg".to_string(), + base64_data: "QUJD".to_string(), + }], + ), + ]); + let converted = VertexClient::convert_message(&message, &tool_names); + let value = serde_json::to_value(&converted).unwrap(); + + assert!(value["parts"][0]["functionResponse"].get("parts").is_none()); + assert_eq!( + value["parts"][1]["functionResponse"], + json!({ + "name": "browser_computer", + "response": {"result": "Screenshot taken"}, + "parts": [{"inlineData": {"mimeType": "image/jpeg", "data": "QUJD"}}], + }) + ); + } + #[test] fn empty_text_parts_are_dropped() { // A model turn often carries an empty text block next to a function From f3b551bd24a3fb3648544fe10a7207bfe9d32c3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Mon, 5 Oct 2026 08:44:53 +0200 Subject: [PATCH 5/5] fix(llm): attach tool result images to the Ollama tool message Ollama messages carry images on any role; tool messages were sent with images: None, so screenshots were dropped. Verified live with a local vision model (qwen3.8:27b). --- crates/llm/src/ollama.rs | 45 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/crates/llm/src/ollama.rs b/crates/llm/src/ollama.rs index bd6420b7..1c77f7ae 100644 --- a/crates/llm/src/ollama.rs +++ b/crates/llm/src/ollama.rs @@ -188,12 +188,19 @@ impl OllamaClient { current_images.clear(); } - // ToolResult as separate "tool" message (text only) + // ToolResult as separate "tool" message; images (screenshots) + // ride along like on a user message messages.push(OllamaMessage { role: "tool".to_string(), content: content.text_content().to_string(), thinking: "".to_string(), - images: None, + images: content.has_images().then(|| { + content + .images() + .iter() + .map(|image| image.base64_data.clone()) + .collect() + }), tool_calls: None, }); } @@ -530,3 +537,37 @@ impl LLMProvider for OllamaClient { Ok(response) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn tool_result_images_travel_on_the_tool_message() { + let messages = OllamaClient::convert_user_message(&[ + ContentBlock::new_tool_result("call_0", "Clicked"), + ContentBlock::new_tool_result_with_images( + "call_1", + "Screenshot taken", + vec![crate::types::ToolResultImage { + media_type: "image/png".to_string(), + base64_data: "QUJD".to_string(), + }], + ), + ]); + + assert_eq!( + serde_json::to_value(&messages).unwrap(), + serde_json::json!([ + {"role": "tool", "content": "Clicked", "thinking": "", "tool_calls": null}, + { + "role": "tool", + "content": "Screenshot taken", + "thinking": "", + "images": ["QUJD"], + "tool_calls": null, + }, + ]) + ); + } +}