Defer view_image processing to history insertion (#37902)

## What changed

- Pass image bytes from `view_image` through unchanged for both direct and code-mode calls, leaving decoding and resizing to the shared history-insertion path.
- Represent invalid image data with the existing image-omission placeholder instead of returning a tool error.

## Testing

- Update the `view_image` integration test to cover placeholder output for invalid image data.

GitOrigin-RevId: 62863defd4815efb8a7725712fd583ad81db3fbb
This commit is contained in:
rka-oai
2026-08-11 00:17:36 +00:00
committed by copyberry
parent ab3b4d26d4
commit 260261ed8f
2 changed files with 11 additions and 132 deletions
+5 -125
View File
@@ -12,7 +12,6 @@ use serde::Deserialize;
use crate::function_tool::FunctionCallError;
use crate::original_image_detail::can_request_original_image_detail;
use crate::tools::context::ToolCallSource;
use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolOutput;
use crate::tools::context::ToolPayload;
@@ -25,8 +24,6 @@ use crate::tools::registry::CoreToolRuntime;
use crate::tools::registry::ToolExecutor;
use codex_tools::ToolName;
use codex_tools::ToolSpec;
use image::ImageFormat;
use std::io::Cursor;
pub struct ViewImageHandler {
options: ViewImageToolOptions,
@@ -52,10 +49,6 @@ impl ViewImageHandler {
const VIEW_IMAGE_UNSUPPORTED_MESSAGE: &str =
"view_image is not allowed because you do not support image inputs";
const VIEW_IMAGE_INVALID_MESSAGE: &str =
"unable to process image: invalid or unsupported image data";
const VIEW_IMAGE_ENCODING_ERROR_MESSAGE: &str =
"unable to process image: could not encode validated image";
#[derive(Deserialize)]
struct ViewImageArgs {
@@ -111,7 +104,6 @@ impl ViewImageHandler {
step_context,
payload,
call_id,
source,
..
} = invocation;
@@ -181,9 +173,6 @@ impl ViewImageHandler {
"unable to read image at `{model_visible_path}`: {error}"
))
})?;
let decoded_image = image::load_from_memory(&file_bytes).map_err(|_| {
FunctionCallError::RespondToModel(VIEW_IMAGE_INVALID_MESSAGE.to_string())
})?;
let can_request_original_detail = can_request_original_image_detail(&turn.model_info);
let use_original_detail = self.options.unified_image_budget
@@ -194,21 +183,8 @@ impl ViewImageHandler {
DEFAULT_IMAGE_DETAIL
};
let image_url = if matches!(source, ToolCallSource::CodeMode { .. }) {
// Code mode must receive only image pixels, while keeping the original
// dimensions for centralized resizing and resize notices.
let mut sanitized_bytes = Vec::new();
decoded_image
.write_to(&mut Cursor::new(&mut sanitized_bytes), ImageFormat::Png)
.map_err(|_| {
FunctionCallError::RespondToModel(VIEW_IMAGE_ENCODING_ERROR_MESSAGE.to_string())
})?;
data_url_from_bytes("image/png", &sanitized_bytes)
} else {
// Direct tool calls retain image metadata for centralized preparation.
data_url_from_bytes("application/octet-stream", &file_bytes)
};
drop(decoded_image);
// The history insertion path owns image decoding and resizing.
let image_url = data_url_from_bytes("application/octet-stream", &file_bytes);
let item = TurnItem::ImageView(ImageViewItem {
id: call_id,
@@ -279,14 +255,13 @@ mod tests {
use crate::session::step_context::StepContext;
use crate::session::tests::make_session_and_context;
use crate::session::turn_context::TurnEnvironment;
use crate::tools::context::ToolCallSource;
use crate::tools::context::ToolInvocation;
use crate::turn_diff_tracker::TurnDiffTracker;
use codex_protocol::models::PermissionProfile;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use core_test_support::TempDirExt;
use image::ImageBuffer;
use image::Rgba;
use pretty_assertions::assert_eq;
use serde_json::json;
use std::sync::Arc;
@@ -309,15 +284,6 @@ mod tests {
));
}
fn tiny_png() -> Vec<u8> {
let image = ImageBuffer::from_pixel(1, 1, Rgba([255u8, 0, 0, 255]));
let mut bytes = Vec::new();
image
.write_to(&mut Cursor::new(&mut bytes), ImageFormat::Png)
.expect("encode test image");
bytes
}
#[test]
fn log_preview_omits_image_data() {
let output = ViewImageOutput {
@@ -358,7 +324,7 @@ mod tests {
replace_primary_environment_cwd(&mut turn, image_cwd.clone());
let image_path = image_cwd.join("image.png");
std::fs::write(image_path.as_path(), tiny_png()).expect("write test image");
std::fs::write(image_path.as_path(), b"not a real image").expect("write test image");
Arc::make_mut(&mut turn.config)
.permissions
.set_permission_profile(PermissionProfile::Disabled)
@@ -434,7 +400,7 @@ mod tests {
replace_primary_environment_cwd(&mut turn, image_cwd.clone());
let image_path = image_cwd.join("image.png");
std::fs::write(image_path.as_path(), tiny_png()).expect("write test image");
std::fs::write(image_path.as_path(), b"not a real image").expect("write test image");
let TurnEnvironmentState::Ready(environment) = &mut turn.environments.environments[0]
else {
panic!("primary environment should be ready");
@@ -461,90 +427,4 @@ mod tests {
result.expect("explicit high detail should be accepted");
}
#[tokio::test(flavor = "multi_thread")]
async fn handle_returns_png_image_for_code_mode() {
let (session, mut turn) = make_session_and_context().await;
let image_dir = tempfile::tempdir().expect("create image temp dir");
let image_cwd = image_dir.abs();
replace_primary_environment_cwd(&mut turn, image_cwd.clone());
let image_path = image_cwd.join("image.png");
std::fs::write(image_path.as_path(), tiny_png()).expect("write test image");
let TurnEnvironmentState::Ready(environment) = &mut turn.environments.environments[0]
else {
panic!("primary environment should be ready");
};
environment.config.permission_profile =
PermissionProfileSnapshot::legacy(PermissionProfile::Disabled);
let turn = Arc::new(turn);
let output = ViewImageHandler::default()
.handle(ToolInvocation {
session: Arc::new(session),
step_context: StepContext::for_test(Arc::clone(&turn)),
turn,
cancellation_token: tokio_util::sync::CancellationToken::new(),
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
call_id: "call-view-image".to_string(),
tool_name: codex_tools::ToolName::plain("view_image"),
source: ToolCallSource::CodeMode {
cell_id: "cell-1".to_string(),
runtime_tool_call_id: "tool-1".to_string(),
},
payload: ToolPayload::Function {
arguments: json!({ "path": "image.png" }).to_string(),
},
})
.await
.expect("code mode image should be accepted");
let result = output.code_mode_result(&ToolPayload::Function {
arguments: "{}".to_string(),
});
let image_url = result["image_url"]
.as_str()
.expect("code mode image URL should be a string");
assert!(image_url.starts_with("data:image/png;base64,"));
}
#[tokio::test(flavor = "multi_thread")]
async fn handle_rejects_invalid_image_before_returning_output() {
let (session, mut turn) = make_session_and_context().await;
let image_dir = tempfile::tempdir().expect("create image temp dir");
let image_cwd = image_dir.abs();
replace_primary_environment_cwd(&mut turn, image_cwd.clone());
let image_path = image_cwd.join("not-an-image.txt");
std::fs::write(image_path.as_path(), b"arbitrary file contents")
.expect("write invalid image");
let TurnEnvironmentState::Ready(environment) = &mut turn.environments.environments[0]
else {
panic!("primary environment should be ready");
};
environment.config.permission_profile =
PermissionProfileSnapshot::legacy(PermissionProfile::Disabled);
let turn = Arc::new(turn);
let result = ViewImageHandler::default()
.handle(ToolInvocation {
session: Arc::new(session),
step_context: StepContext::for_test(Arc::clone(&turn)),
turn,
cancellation_token: tokio_util::sync::CancellationToken::new(),
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
call_id: "call-view-image".to_string(),
tool_name: codex_tools::ToolName::plain("view_image"),
source: ToolCallSource::Direct,
payload: ToolPayload::Function {
arguments: json!({ "path": "not-an-image.txt" }).to_string(),
},
})
.await;
let Err(FunctionCallError::RespondToModel(message)) = result else {
panic!("expected invalid image error");
};
assert_eq!(message, VIEW_IMAGE_INVALID_MESSAGE);
}
}
+6 -7
View File
@@ -1443,7 +1443,7 @@ async fn view_image_tool_errors_when_path_is_directory() -> anyhow::Result<()> {
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn view_image_tool_rejects_invalid_image_before_tool_output() -> anyhow::Result<()> {
async fn view_image_tool_turns_invalid_image_into_placeholder() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
@@ -1496,13 +1496,12 @@ async fn view_image_tool_rejects_invalid_image_before_tool_output() -> anyhow::R
.await;
let request = second_mock.single_request();
let output_text = request
.function_call_output_content_and_success(call_id)
.and_then(|(content, _)| content)
.context("invalid view_image error text present")?;
assert_eq!(
output_text,
"unable to process image: invalid or unsupported image data"
request.function_call_output(call_id).get("output"),
Some(&serde_json::json!([{
"type": "input_text",
"text": "image content omitted because it could not be processed"
}]))
);
Ok(())
}