diff --git a/CHANGELOG.md b/CHANGELOG.md index dd7dc91..45d68b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ All notable changes to this project are documented here. The format follows ### Fixed +- **Tool-result images reach Codex/Kimi/Grok.** User-message images were already translated, but + Anthropic `tool_result` content with base64 `image` blocks was dropped (`[image omitted]` on + Codex/Grok, text-only flatten on Kimi). Base64 tool images now emit Responses + `input_image` parts (Codex/Grok) or chat-completions `image_url` parts (Kimi); pure-text + tool results stay a string. Remote-URL tool images remain textual placeholders. + - **Codex `-fast` models request priority service tier.** Incoming ids like `gpt-5.4-mini-fast` already stripped to the base model, but the Responses body never set `service_tier`. Codex upstream requests now send `"service_tier": "priority"` when the client asked for `-fast` diff --git a/crates/llmtrim-cli/src/reroute/codex.rs b/crates/llmtrim-cli/src/reroute/codex.rs index c2cba97..2cb9666 100644 --- a/crates/llmtrim-cli/src/reroute/codex.rs +++ b/crates/llmtrim-cli/src/reroute/codex.rs @@ -152,28 +152,123 @@ fn image_url(block: &Value) -> Option { } } -/// Render a `tool_result` block's content into the Responses `function_call_output.output` string. -fn tool_result_output(block: &Value) -> String { - let body = match block.get("content") { - Some(Value::String(s)) => s.clone(), - Some(Value::Array(parts)) => parts - .iter() - .map(|p| match p.get("type").and_then(Value::as_str) { - Some("image") => "[image omitted]".to_string(), - _ => p - .get("text") - .and_then(Value::as_str) - .unwrap_or_default() - .to_string(), - }) - .collect::>() - .join("\n"), - _ => String::new(), +/// Tool-result images: base64 becomes a data URL; remote URLs stay textual placeholders +/// (Responses accepts remote user-message images more readily than tool images). +fn tool_result_image_url(block: &Value) -> Result { + let Some(source) = block.get("source") else { + return Err("[image omitted]".into()); }; - if block.get("is_error").and_then(Value::as_bool) == Some(true) { - format!("[tool execution error]\n{body}") - } else { - body + match source.get("type").and_then(Value::as_str) { + Some("base64") => { + let media = source.get("media_type").and_then(Value::as_str); + let data = source.get("data").and_then(Value::as_str); + match (media, data) { + (Some(media), Some(data)) if !data.is_empty() => { + Ok(format!("data:{media};base64,{data}")) + } + _ => Err("[image omitted]".into()), + } + } + Some("url") if source.get("url").and_then(Value::as_str).is_some() => { + Err("[image omitted: url]".into()) + } + _ => Err("[image omitted]".into()), + } +} + +/// Render a `tool_result` block into Responses `function_call_output.output`. +/// +/// Pure text stays a string for wire compatibility with existing clients/tests. When the content +/// array carries base64 images, output becomes a content-parts array mixing `input_text` and +/// `input_image` (matching the Codex Responses API). +fn tool_result_output(block: &Value) -> Value { + let is_error = block.get("is_error").and_then(Value::as_bool) == Some(true); + + match block.get("content") { + Some(Value::String(s)) => { + let body = if is_error { + format!("[tool execution error]\n{s}") + } else { + s.clone() + }; + Value::String(body) + } + Some(Value::Array(parts)) => { + let mut out: Vec = Vec::new(); + let mut has_image = false; + for p in parts { + match p.get("type").and_then(Value::as_str) { + Some("image") => match tool_result_image_url(p) { + Ok(url) => { + has_image = true; + out.push(json!({ + "type": "input_image", + "image_url": url, + })); + } + Err(placeholder) => { + out.push(json!({ + "type": "input_text", + "text": placeholder, + })); + } + }, + _ => { + let text = p.get("text").and_then(Value::as_str).unwrap_or_default(); + out.push(json!({ + "type": "input_text", + "text": text, + })); + } + } + } + + if !has_image { + let body = out + .iter() + .filter_map(|p| p.get("text").and_then(Value::as_str)) + .collect::>() + .join("\n"); + let body = if is_error { + format!("[tool execution error]\n{body}") + } else { + body + }; + return Value::String(body); + } + + if is_error { + out.insert( + 0, + json!({ "type": "input_text", "text": "[tool execution error]" }), + ); + } + Value::Array(out) + } + _ => { + if is_error { + Value::String("[tool execution error]\n".into()) + } else { + Value::String(String::new()) + } + } + } +} + +/// Append a note onto a tool-result `output` that may be a string or a content-parts array. +fn append_tool_result_note(output: &mut Value, note: &str) { + match output { + Value::String(s) => { + s.push_str("\n\n"); + s.push_str(note); + } + Value::Array(parts) => { + parts.push(json!({ + "type": "input_text", + "text": format!("\n\n{note}"), + })); + } + _ => {} } } @@ -265,8 +360,8 @@ fn build_input(anthropic: &Value) -> Vec { if let Some(rw) = crate::reroute::read_rewrite::read_offset_rewrite(call_id) { - output.push_str("\n\n"); - output.push_str( + append_tool_result_note( + &mut output, &crate::reroute::read_rewrite::read_offset_rewrite_note(&rw), ); } @@ -1546,6 +1641,125 @@ mod tests { assert_eq!(body["input"][0]["output"], "[tool execution error]\nboom"); } + #[test] + fn tool_result_base64_image_becomes_content_parts() { + let body = build_request_body( + &json!({ + "messages": [{ + "role": "user", + "content": [{ + "type": "tool_result", + "tool_use_id": "tu_img", + "content": [ + {"type": "text", "text": "before"}, + {"type": "image", "source": { + "type": "base64", + "media_type": "image/png", + "data": "AAAA" + }}, + {"type": "text", "text": "after"} + ] + }] + }] + }), + "gpt-5.5", + None, + ) + .expect("build"); + assert_eq!( + body["input"][0]["output"], + json!([ + {"type": "input_text", "text": "before"}, + {"type": "input_image", "image_url": "data:image/png;base64,AAAA"}, + {"type": "input_text", "text": "after"} + ]) + ); + } + + #[test] + fn tool_result_text_only_stays_string() { + let body = build_request_body( + &json!({ + "messages": [{ + "role": "user", + "content": [{ + "type": "tool_result", + "tool_use_id": "tu_text", + "content": [ + {"type": "text", "text": "first"}, + {"type": "text", "text": "second"} + ] + }] + }] + }), + "gpt-5.5", + None, + ) + .expect("build"); + assert_eq!(body["input"][0]["output"], "first\nsecond"); + } + + #[test] + fn tool_result_error_prefix_precedes_image() { + let body = build_request_body( + &json!({ + "messages": [{ + "role": "user", + "content": [{ + "type": "tool_result", + "tool_use_id": "tu_err_img", + "is_error": true, + "content": [{ + "type": "image", + "source": { + "type": "base64", + "media_type": "image/png", + "data": "BBBB" + } + }] + }] + }] + }), + "gpt-5.5", + None, + ) + .expect("build"); + assert_eq!( + body["input"][0]["output"], + json!([ + {"type": "input_text", "text": "[tool execution error]"}, + {"type": "input_image", "image_url": "data:image/png;base64,BBBB"} + ]) + ); + } + + #[test] + fn tool_result_url_image_becomes_placeholder() { + let body = build_request_body( + &json!({ + "messages": [{ + "role": "user", + "content": [{ + "type": "tool_result", + "tool_use_id": "tu_url", + "content": [{ + "type": "image", + "source": { + "type": "url", + "url": "https://example.invalid/a.png" + } + }] + }] + }] + }), + "gpt-5.5", + None, + ) + .expect("build"); + // No real image part → string wire form with the CCP-style placeholder. + assert_eq!(body["input"][0]["output"], "[image omitted: url]"); + } + #[test] fn tools_map_and_web_search_translated() { let body = build_request_body( diff --git a/crates/llmtrim-cli/src/reroute/grok.rs b/crates/llmtrim-cli/src/reroute/grok.rs index 12d4a87..ea908af 100644 --- a/crates/llmtrim-cli/src/reroute/grok.rs +++ b/crates/llmtrim-cli/src/reroute/grok.rs @@ -161,27 +161,102 @@ fn content_blocks(content: &Value) -> Vec { } } -fn tool_result_output(block: &Value) -> String { - let body = match block.get("content") { - Some(Value::String(s)) => s.clone(), - Some(Value::Array(parts)) => parts - .iter() - .map(|p| match p.get("type").and_then(Value::as_str) { - Some("image") => "[image omitted]".to_string(), - _ => p - .get("text") - .and_then(Value::as_str) - .unwrap_or_default() - .to_string(), - }) - .collect::>() - .join("\n"), - _ => String::new(), +/// Tool-result images: base64 becomes a data URL; remote URLs stay textual placeholders. +fn tool_result_image_url(block: &Value) -> Result { + let Some(source) = block.get("source") else { + return Err("[image omitted]".into()); }; - if block.get("is_error").and_then(Value::as_bool) == Some(true) { - format!("[tool execution error]\n{body}") - } else { - body + match source.get("type").and_then(Value::as_str) { + Some("base64") => { + let media = source.get("media_type").and_then(Value::as_str); + let data = source.get("data").and_then(Value::as_str); + match (media, data) { + (Some(media), Some(data)) if !data.is_empty() => { + Ok(format!("data:{media};base64,{data}")) + } + _ => Err("[image omitted]".into()), + } + } + Some("url") if source.get("url").and_then(Value::as_str).is_some() => { + Err("[image omitted: url]".into()) + } + _ => Err("[image omitted]".into()), + } +} + +/// Render a `tool_result` into Responses `function_call_output.output`. +/// Pure text stays a string; base64 images become a content-parts array. +fn tool_result_output(block: &Value) -> Value { + let is_error = block.get("is_error").and_then(Value::as_bool) == Some(true); + + match block.get("content") { + Some(Value::String(s)) => { + let body = if is_error { + format!("[tool execution error]\n{s}") + } else { + s.clone() + }; + Value::String(body) + } + Some(Value::Array(parts)) => { + let mut out: Vec = Vec::new(); + let mut has_image = false; + for p in parts { + match p.get("type").and_then(Value::as_str) { + Some("image") => match tool_result_image_url(p) { + Ok(url) => { + has_image = true; + out.push(json!({ + "type": "input_image", + "image_url": url, + })); + } + Err(placeholder) => { + out.push(json!({ + "type": "input_text", + "text": placeholder, + })); + } + }, + _ => { + let text = p.get("text").and_then(Value::as_str).unwrap_or_default(); + out.push(json!({ + "type": "input_text", + "text": text, + })); + } + } + } + + if !has_image { + let body = out + .iter() + .filter_map(|p| p.get("text").and_then(Value::as_str)) + .collect::>() + .join("\n"); + let body = if is_error { + format!("[tool execution error]\n{body}") + } else { + body + }; + return Value::String(body); + } + + if is_error { + out.insert( + 0, + json!({ "type": "input_text", "text": "[tool execution error]" }), + ); + } + Value::Array(out) + } + _ => { + if is_error { + Value::String("[tool execution error]\n".into()) + } else { + Value::String(String::new()) + } + } } } @@ -1548,4 +1623,94 @@ mod tests { let text = serde_json::to_string(input).unwrap(); assert!(text.contains("[image omitted]"), "{text}"); } + + #[test] + fn tool_result_base64_image_becomes_content_parts() { + let body = build_request_body( + &json!({ + "messages": [{ + "role": "user", + "content": [{ + "type": "tool_result", + "tool_use_id": "call_img", + "content": [ + {"type": "text", "text": "shot"}, + {"type": "image", "source": { + "type": "base64", + "media_type": "image/png", + "data": "xx" + }} + ] + }] + }] + }), + "grok-4.5", + None, + ) + .expect("build"); + assert_eq!( + body["input"][0]["output"], + json!([ + {"type": "input_text", "text": "shot"}, + {"type": "input_image", "image_url": "data:image/png;base64,xx"} + ]) + ); + } + + #[test] + fn tool_result_text_only_stays_string() { + let body = build_request_body( + &json!({ + "messages": [{ + "role": "user", + "content": [{ + "type": "tool_result", + "tool_use_id": "call_t", + "content": [ + {"type": "text", "text": "a"}, + {"type": "text", "text": "b"} + ] + }] + }] + }), + "grok-4.5", + None, + ) + .expect("build"); + assert_eq!(body["input"][0]["output"], "a\nb"); + } + + #[test] + fn tool_result_error_prefix_with_image() { + let body = build_request_body( + &json!({ + "messages": [{ + "role": "user", + "content": [{ + "type": "tool_result", + "tool_use_id": "call_e", + "is_error": true, + "content": [{ + "type": "image", + "source": { + "type": "base64", + "media_type": "image/jpeg", + "data": "yy" + } + }] + }] + }] + }), + "grok-4.5", + None, + ) + .expect("build"); + assert_eq!( + body["input"][0]["output"], + json!([ + {"type": "input_text", "text": "[tool execution error]"}, + {"type": "input_image", "image_url": "data:image/jpeg;base64,yy"} + ]) + ); + } } diff --git a/crates/llmtrim-cli/src/reroute/kimi.rs b/crates/llmtrim-cli/src/reroute/kimi.rs index ebaee8c..4718515 100644 --- a/crates/llmtrim-cli/src/reroute/kimi.rs +++ b/crates/llmtrim-cli/src/reroute/kimi.rs @@ -271,23 +271,85 @@ fn build_tool_result(block: &Value) -> Value { .get("is_error") .and_then(Value::as_bool) .unwrap_or(false); - let mut content = tool_result_text(block.get("content")); - if is_error { - content = format!("[tool execution error]\n{content}"); - } + let content = tool_result_content(block.get("content"), is_error); json!({"role": "tool", "tool_call_id": tool_call_id, "content": content}) } -/// A `tool_result` content is a string or an array of blocks; flatten to text. -fn tool_result_text(content: Option<&Value>) -> String { +/// A `tool_result` content is a string or an array of blocks. +/// Pure text stays a string; when images are present, content becomes a parts array +/// (`text` + `image_url`) so multimodal tool results reach Kimi. +fn tool_result_content(content: Option<&Value>, is_error: bool) -> Value { match content { - Some(Value::String(s)) => s.clone(), - Some(Value::Array(blocks)) => blocks - .iter() - .filter_map(|b| b.get("text").and_then(Value::as_str)) - .collect::>() - .join("\n"), - _ => String::new(), + Some(Value::String(s)) => { + let body = if is_error { + format!("[tool execution error]\n{s}") + } else { + s.clone() + }; + Value::String(body) + } + Some(Value::Array(blocks)) => { + let mut parts: Vec = Vec::new(); + let mut has_image = false; + for b in blocks { + match b.get("type").and_then(Value::as_str) { + Some("text") => { + let text = b.get("text").and_then(Value::as_str).unwrap_or(""); + parts.push(json!({"type": "text", "text": text})); + } + Some("image") => { + if let Some(url) = image_url(b.get("source")) { + has_image = true; + parts.push(json!({ + "type": "image_url", + "image_url": {"url": url}, + })); + } else { + parts.push(json!({ + "type": "text", + "text": "[image omitted]", + })); + } + } + Some(other) => { + parts.push(json!({ + "type": "text", + "text": format!("[unsupported content block omitted: {other}]"), + })); + } + None => {} + } + } + + if !has_image { + let body = parts + .iter() + .filter_map(|p| p.get("text").and_then(Value::as_str)) + .collect::>() + .join("\n"); + let body = if is_error { + format!("[tool execution error]\n{body}") + } else { + body + }; + return Value::String(body); + } + + if is_error { + parts.insert( + 0, + json!({"type": "text", "text": "[tool execution error]\n"}), + ); + } + Value::Array(parts) + } + _ => { + if is_error { + Value::String("[tool execution error]\n".into()) + } else { + Value::String(String::new()) + } + } } } @@ -720,6 +782,75 @@ mod tests { assert_eq!(content[1]["image_url"]["url"], "data:image/png;base64,AAAA"); } + #[test] + fn tool_result_base64_image_becomes_parts_array() { + let out = body(json!({ + "max_tokens": 100, + "messages": [{"role": "user", "content": [{ + "type": "tool_result", + "tool_use_id": "toolu_1", + "content": [ + {"type": "text", "text": "caption"}, + {"type": "image", "source": { + "type": "base64", + "media_type": "image/png", + "data": "abc" + }}, + ] + }]}], + })); + let tool = &out["messages"][0]; + assert_eq!(tool["role"], "tool"); + assert_eq!(tool["tool_call_id"], "toolu_1"); + let content = tool["content"].as_array().expect("parts array"); + assert_eq!(content.len(), 2); + assert_eq!(content[0]["type"], "text"); + assert_eq!(content[0]["text"], "caption"); + assert_eq!(content[1]["type"], "image_url"); + assert_eq!(content[1]["image_url"]["url"], "data:image/png;base64,abc"); + } + + #[test] + fn tool_result_text_only_stays_string() { + let out = body(json!({ + "max_tokens": 100, + "messages": [{"role": "user", "content": [{ + "type": "tool_result", + "tool_use_id": "toolu_t", + "content": [ + {"type": "text", "text": "first"}, + {"type": "text", "text": "second"}, + ] + }]}], + })); + assert_eq!(out["messages"][0]["content"], "first\nsecond"); + } + + #[test] + fn tool_result_error_prefix_with_image() { + let out = body(json!({ + "max_tokens": 100, + "messages": [{"role": "user", "content": [{ + "type": "tool_result", + "tool_use_id": "toolu_e", + "is_error": true, + "content": [{ + "type": "image", + "source": { + "type": "base64", + "media_type": "image/png", + "data": "ZZ" + } + }] + }]}], + })); + let content = out["messages"][0]["content"].as_array().expect("parts"); + assert_eq!(content[0]["type"], "text"); + assert_eq!(content[0]["text"], "[tool execution error]\n"); + assert_eq!(content[1]["type"], "image_url"); + assert_eq!(content[1]["image_url"]["url"], "data:image/png;base64,ZZ"); + } + #[test] fn tools_mapping_and_web_search_stripped() { let out = body(json!({