diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 0403479949..fd463f4af1 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -570,6 +570,9 @@ "image_generation": { "type": "boolean" }, + "image_resize_notice": { + "type": "boolean" + }, "imagegenext": { "type": "boolean" }, @@ -5198,6 +5201,9 @@ "image_generation": { "type": "boolean" }, + "image_resize_notice": { + "type": "boolean" + }, "imagegenext": { "type": "boolean" }, diff --git a/codex-rs/core/src/compact_remote_history.rs b/codex-rs/core/src/compact_remote_history.rs index 15706cd69d..39f3aa0212 100644 --- a/codex-rs/core/src/compact_remote_history.rs +++ b/codex-rs/core/src/compact_remote_history.rs @@ -1,12 +1,11 @@ use std::borrow::Borrow; +use crate::context::ContextualUserFragment; +use crate::context::ImageResizeNotice; use crate::context_manager::estimate_item_token_count; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; -const ATTACHED_NOTICE_MARKERS: &[(&str, &str)] = - &[("", "")]; - #[derive(Debug, Clone, PartialEq)] pub(crate) struct HistoryItemGroup { pub(crate) source: T, @@ -51,9 +50,7 @@ fn is_attached_notice(notice: &ResponseItem) -> bool { && matches!( content.as_slice(), [ContentItem::InputText { text }] - if ATTACHED_NOTICE_MARKERS.iter().any(|(opening, closing)| { - text.starts_with(opening) && text.ends_with(closing) - }) + if ImageResizeNotice::matches_text(text) ) ) } diff --git a/codex-rs/core/src/context/image_resize_notice.rs b/codex-rs/core/src/context/image_resize_notice.rs new file mode 100644 index 0000000000..e9248aa1b9 --- /dev/null +++ b/codex-rs/core/src/context/image_resize_notice.rs @@ -0,0 +1,74 @@ +use super::ContextualUserFragment; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ImageResizeNoticeSource { + UserMessage, + ToolOutput, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct ResizedImage { + pub(crate) image_number: usize, + pub(crate) image_count: usize, + pub(crate) source_width: u32, + pub(crate) source_height: u32, + pub(crate) prepared_width: u32, + pub(crate) prepared_height: u32, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct ImageResizeNotice { + source: ImageResizeNoticeSource, + resized_images: Vec, +} + +impl ImageResizeNotice { + pub(crate) fn new(source: ImageResizeNoticeSource, resized_images: Vec) -> Self { + Self { + source, + resized_images, + } + } +} + +impl ContextualUserFragment for ImageResizeNotice { + fn role(&self) -> &'static str { + "developer" + } + + fn requires_separate_message(&self) -> bool { + true + } + + fn markers(&self) -> (&'static str, &'static str) { + Self::type_markers() + } + + fn type_markers() -> (&'static str, &'static str) { + ("", "") + } + + fn body(&self) -> String { + let source = match self.source { + ImageResizeNoticeSource::UserMessage => "user message", + ImageResizeNoticeSource::ToolOutput => "tool output", + }; + let notices = self + .resized_images + .iter() + .map(|image| { + format!( + "Image {} of {} in the preceding {source} was resized from {}x{} to {}x{} pixels.", + image.image_number, + image.image_count, + image.source_width, + image.source_height, + image.prepared_width, + image.prepared_height, + ) + }) + .collect::>() + .join("\n"); + format!("\n{notices}\n") + } +} diff --git a/codex-rs/core/src/context/mod.rs b/codex-rs/core/src/context/mod.rs index 2e69331475..0398d21a1d 100644 --- a/codex-rs/core/src/context/mod.rs +++ b/codex-rs/core/src/context/mod.rs @@ -9,6 +9,7 @@ mod environment_context; mod environments_instructions; mod guardian_followup_review_reminder; mod hook_additional_context; +mod image_resize_notice; mod inter_agent_completion_message; mod inter_agent_message; mod internal_model_context; @@ -48,6 +49,9 @@ pub(crate) use current_time_reminder::CurrentTimeReminder; pub(crate) use environments_instructions::EnvironmentsInstructions; pub(crate) use guardian_followup_review_reminder::GuardianFollowupReviewReminder; pub(crate) use hook_additional_context::HookAdditionalContext; +pub(crate) use image_resize_notice::ImageResizeNotice; +pub(crate) use image_resize_notice::ImageResizeNoticeSource; +pub(crate) use image_resize_notice::ResizedImage; pub(crate) use inter_agent_completion_message::InterAgentCompletionMessage; pub(crate) use inter_agent_message::InterAgentMessage; pub(crate) use inter_agent_message::InterAgentMessageType; diff --git a/codex-rs/core/src/image_preparation.rs b/codex-rs/core/src/image_preparation.rs index 4b19c7e053..36f617b129 100644 --- a/codex-rs/core/src/image_preparation.rs +++ b/codex-rs/core/src/image_preparation.rs @@ -1,3 +1,7 @@ +use crate::context::ContextualUserFragment; +use crate::context::ImageResizeNotice; +use crate::context::ImageResizeNoticeSource; +use crate::context::ResizedImage; use codex_analytics::ImageDetailSetting; use codex_analytics::ImagePreparationMetadata; use codex_protocol::models::ContentItem; @@ -33,6 +37,20 @@ struct ImageOrigin<'a> { item_id: Option<&'a str>, } +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum ImageResizeNoticeMode { + Disabled, + Enabled, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +struct PreparedImageResize { + source_width: u32, + source_height: u32, + prepared_width: u32, + prepared_height: u32, +} + #[derive(Debug, thiserror::Error)] enum ImagePreparationError { #[error("remote image URLs are not supported")] @@ -56,37 +74,51 @@ impl ImagePreparationError { } } -pub(crate) fn prepare_response_items(items: &mut [ResponseItem]) -> Vec { +pub(crate) fn prepare_response_items( + items: &mut Vec, + resize_notice_mode: ImageResizeNoticeMode, +) -> Vec { let mut metadata = Vec::new(); - for item in items { - match item { + let mut prepared_items = Vec::with_capacity(items.len()); + for mut item in std::mem::take(items) { + let resize_notice = match &mut item { ResponseItem::Message { role, content, .. } => { - prepare_message_content( + let resized_images = prepare_message_content( content, ImageOrigin { message_role: Some(role), item_id: None, }, + if role == "user" { + resize_notice_mode + } else { + ImageResizeNoticeMode::Disabled + }, &mut metadata, ); + (!resized_images.is_empty()).then(|| { + ImageResizeNotice::new(ImageResizeNoticeSource::UserMessage, resized_images) + }) } ResponseItem::FunctionCallOutput { call_id, output, .. } | ResponseItem::CustomToolCallOutput { call_id, output, .. - } => { - if let Some(content) = output.content_items_mut() { - prepare_tool_output_content( - content, - ImageOrigin { - message_role: None, - item_id: Some(call_id), - }, - &mut metadata, - ); - } - } + } => output.content_items_mut().and_then(|content| { + let resized_images = prepare_tool_output_content( + content, + ImageOrigin { + message_role: None, + item_id: Some(call_id), + }, + resize_notice_mode, + &mut metadata, + ); + (!resized_images.is_empty()).then(|| { + ImageResizeNotice::new(ImageResizeNoticeSource::ToolOutput, resized_images) + }) + }), ResponseItem::AdditionalTools { .. } | ResponseItem::Reasoning { .. } | ResponseItem::AgentMessage { .. } @@ -100,44 +132,93 @@ pub(crate) fn prepare_response_items(items: &mut [ResponseItem]) -> Vec {} + | ResponseItem::Other => None, + }; + prepared_items.push(item); + if let Some(resize_notice) = resize_notice { + prepared_items.push(ContextualUserFragment::into(resize_notice)); } } + *items = prepared_items; metadata } fn prepare_message_content( items: &mut [ContentItem], origin: ImageOrigin<'_>, + resize_notice_mode: ImageResizeNoticeMode, metadata: &mut Vec, -) { +) -> Vec { + let image_count = items + .iter() + .filter(|item| matches!(item, ContentItem::InputImage { .. })) + .count(); + let mut image_number = 0; + let mut resized_images = Vec::new(); for item in items { - if let ContentItem::InputImage { image_url, detail } = item - && let Err(error) = prepare_image(image_url, *detail, origin, metadata) - { - warn!(%error, "failed to prepare message image"); - *item = ContentItem::InputText { - text: error.placeholder().to_string(), - }; + if let ContentItem::InputImage { image_url, detail } = item { + image_number += 1; + match prepare_image(image_url, *detail, origin, metadata) { + Ok(Some(resize)) if resize_notice_mode == ImageResizeNoticeMode::Enabled => { + resized_images.push(ResizedImage { + image_number, + image_count, + source_width: resize.source_width, + source_height: resize.source_height, + prepared_width: resize.prepared_width, + prepared_height: resize.prepared_height, + }); + } + Ok(_) => {} + Err(error) => { + warn!(%error, "failed to prepare message image"); + *item = ContentItem::InputText { + text: error.placeholder().to_string(), + }; + } + } } } + resized_images } fn prepare_tool_output_content( items: &mut [FunctionCallOutputContentItem], origin: ImageOrigin<'_>, + resize_notice_mode: ImageResizeNoticeMode, metadata: &mut Vec, -) { +) -> Vec { + let image_count = items + .iter() + .filter(|item| matches!(item, FunctionCallOutputContentItem::InputImage { .. })) + .count(); + let mut image_number = 0; + let mut resized_images = Vec::new(); for item in items { - if let FunctionCallOutputContentItem::InputImage { image_url, detail } = item - && let Err(error) = prepare_image(image_url, *detail, origin, metadata) - { - warn!(%error, "failed to prepare tool output image"); - *item = FunctionCallOutputContentItem::InputText { - text: error.placeholder().to_string(), - }; + if let FunctionCallOutputContentItem::InputImage { image_url, detail } = item { + image_number += 1; + match prepare_image(image_url, *detail, origin, metadata) { + Ok(Some(resize)) if resize_notice_mode == ImageResizeNoticeMode::Enabled => { + resized_images.push(ResizedImage { + image_number, + image_count, + source_width: resize.source_width, + source_height: resize.source_height, + prepared_width: resize.prepared_width, + prepared_height: resize.prepared_height, + }); + } + Ok(_) => {} + Err(error) => { + warn!(%error, "failed to prepare tool output image"); + *item = FunctionCallOutputContentItem::InputText { + text: error.placeholder().to_string(), + }; + } + } } } + resized_images } fn is_remote_image_url(image_url: &str) -> bool { @@ -157,12 +238,12 @@ fn prepare_image( detail: Option, origin: ImageOrigin<'_>, metadata: &mut Vec, -) -> Result<(), ImagePreparationError> { +) -> Result, ImagePreparationError> { if is_remote_image_url(image_url) { return Err(ImagePreparationError::RemoteUrlUnsupported); } if !is_data_url(image_url) { - return Ok(()); + return Ok(None); } let (effective_detail, limits) = match detail { @@ -182,8 +263,15 @@ fn prepare_image( prepared_width: image.width, prepared_height: image.height, }); + let resize = ((image.source_width, image.source_height) != (image.width, image.height)) + .then_some(PreparedImageResize { + source_width: image.source_width, + source_height: image.source_height, + prepared_width: image.width, + prepared_height: image.height, + }); *image_url = image.into_data_url(); - Ok(()) + Ok(resize) } #[cfg(test)] diff --git a/codex-rs/core/src/image_preparation_tests.rs b/codex-rs/core/src/image_preparation_tests.rs index 627da58fb8..53fad8ff66 100644 --- a/codex-rs/core/src/image_preparation_tests.rs +++ b/codex-rs/core/src/image_preparation_tests.rs @@ -51,7 +51,7 @@ fn preparation_preserves_small_image_bytes_and_replaces_remote_urls() { internal_chat_message_metadata_passthrough: None, }]; - prepare_response_items(&mut items); + prepare_response_items(&mut items, ImageResizeNoticeMode::Disabled); let ResponseItem::Message { content, .. } = &items[0] else { panic!("expected message"); @@ -105,7 +105,7 @@ fn detail_policies_apply_the_expected_budgets() { internal_chat_message_metadata_passthrough: None, }]; - let metadata = prepare_response_items(&mut items); + let metadata = prepare_response_items(&mut items, ImageResizeNoticeMode::Disabled); let ResponseItem::Message { content, .. } = &items[0] else { panic!("expected message"); @@ -144,7 +144,7 @@ fn preparation_reports_tool_output_item_id() { ]), internal_chat_message_metadata_passthrough: None, }]; - let metadata = prepare_response_items(&mut items); + let metadata = prepare_response_items(&mut items, ImageResizeNoticeMode::Disabled); assert_eq!( metadata, @@ -160,6 +160,136 @@ fn preparation_reports_tool_output_item_id() { ); } +#[test] +fn resize_notices_preserve_original_image_positions_and_skip_failed_images() { + let (large_image_url, _) = png_data_url(/*width*/ 2048, /*height*/ 2048); + let (small_image_url, _) = png_data_url(/*width*/ 64, /*height*/ 32); + let mut items = vec![ + ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ + ContentItem::InputImage { + image_url: small_image_url, + detail: Some(ImageDetail::High), + }, + ContentItem::InputImage { + image_url: "data:image/png;base64,%%%".to_string(), + detail: Some(ImageDetail::High), + }, + ContentItem::InputImage { + image_url: large_image_url.clone(), + detail: Some(ImageDetail::High), + }, + ], + phase: None, + internal_chat_message_metadata_passthrough: None, + }, + ResponseItem::FunctionCallOutput { + id: None, + call_id: "call-image".to_string(), + output: FunctionCallOutputPayload::from_content_items(vec![ + FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,%%%".to_string(), + detail: Some(ImageDetail::High), + }, + FunctionCallOutputContentItem::InputImage { + image_url: large_image_url, + detail: Some(ImageDetail::High), + }, + ]), + internal_chat_message_metadata_passthrough: None, + }, + ]; + + prepare_response_items(&mut items, ImageResizeNoticeMode::Enabled); + let expected_user_notice = concat!( + "\n", + "Image 3 of 3 in the preceding user message was resized from 2048x2048 to 1600x1600 pixels.\n", + "" + ); + + let ResponseItem::Message { content, .. } = &items[0] else { + panic!("expected message"); + }; + let [ + ContentItem::InputImage { + image_url: small_message_image_url, + .. + }, + ContentItem::InputText { + text: failed_message_image, + }, + ContentItem::InputImage { + image_url: resized_message_image_url, + .. + }, + ] = content.as_slice() + else { + panic!("expected unchanged image, failed image placeholder, and resized image"); + }; + assert_eq!( + decoded_image(small_message_image_url).1.dimensions(), + (64, 32) + ); + assert_eq!(failed_message_image, IMAGE_PROCESSING_ERROR_PLACEHOLDER); + assert_eq!( + decoded_image(resized_message_image_url).1.dimensions(), + (1600, 1600) + ); + + assert_eq!( + &items[1], + &ResponseItem::Message { + id: None, + role: "developer".to_string(), + content: vec![ContentItem::InputText { + text: expected_user_notice.to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + } + ); + + let ResponseItem::FunctionCallOutput { output, .. } = &items[2] else { + panic!("expected function call output"); + }; + let [ + FunctionCallOutputContentItem::InputText { + text: failed_tool_image, + }, + FunctionCallOutputContentItem::InputImage { + image_url: resized_tool_image_url, + .. + }, + ] = output.content_items().expect("tool output content items") + else { + panic!("expected failed image placeholder and resized image in the tool output"); + }; + assert_eq!(failed_tool_image, IMAGE_PROCESSING_ERROR_PLACEHOLDER); + assert_eq!( + decoded_image(resized_tool_image_url).1.dimensions(), + (1600, 1600) + ); + assert_eq!( + &items[3], + &ResponseItem::Message { + id: None, + role: "developer".to_string(), + content: vec![ContentItem::InputText { + text: concat!( + "\n", + "Image 2 of 2 in the preceding tool output was resized from 2048x2048 to 1600x1600 pixels.\n", + "" + ) + .to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + } + ); +} + #[test] fn preparation_replaces_only_failed_tool_images_and_preserves_metadata() { let (valid_image_url, _) = png_data_url(/*width*/ 64, /*height*/ 32); @@ -195,7 +325,7 @@ fn preparation_replaces_only_failed_tool_images_and_preserves_metadata() { internal_chat_message_metadata_passthrough: None, }]; - prepare_response_items(&mut items); + prepare_response_items(&mut items, ImageResizeNoticeMode::Disabled); assert_eq!( items, diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index bb0bf3f051..7b02bd51c9 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -31,6 +31,7 @@ use crate::environment_selection::TurnEnvironmentSnapshot; use crate::exec_policy::BANNED_PREFIX_SUGGESTIONS; use crate::exec_policy::ExecPolicyManager; use crate::exec_policy::default_policy_path; +use crate::image_preparation::ImageResizeNoticeMode; use crate::image_preparation::prepare_response_items as prepare_image_response_items; use crate::parse_turn_item; use crate::realtime_conversation::RealtimeConversationManager; @@ -1432,8 +1433,9 @@ impl Session { // Keep the recorded rollout unchanged. Prepare its reconstructed history before // installing it, so legacy media is processed once for this resume or fork and // will be processed again if the rollout is reconstructed in a future session. - // This meets media preparation requirements without modifying persisted rollouts. - prepare_image_response_items(&mut history); + // Never backfill resize notices during replay; only newly recorded items may + // emit them, so the historical model prefix remains unchanged. + let _ = prepare_image_response_items(&mut history, ImageResizeNoticeMode::Disabled); prepare_audio_response_items(&mut history); { let mut state = self.state.lock().await; @@ -2898,13 +2900,25 @@ impl Session { turn_context: &TurnContext, items: &'a [ResponseItem], ) -> (Cow<'a, [ResponseItem]>, Vec) { - let mut items = Cow::Borrowed(items); - let image_preparations = prepare_image_response_items(items.to_mut()); - prepare_audio_response_items(items.to_mut()); + let mut items = items.to_vec(); + let image_preparations = prepare_image_response_items( + &mut items, + if turn_context + .config + .features + .enabled(Feature::ImageResizeNotice) + { + ImageResizeNoticeMode::Enabled + } else { + ImageResizeNoticeMode::Disabled + }, + ); + prepare_audio_response_items(&mut items); // Most response items get their passthrough turn ID at the durable history boundary. - for item in items.to_mut() { + for item in &mut items { item.set_turn_id_if_missing(&turn_context.sub_id); } + let items = Cow::Owned(items); ( Self::assign_missing_response_item_ids(items), image_preparations, diff --git a/codex-rs/core/tests/suite/image_rollout.rs b/codex-rs/core/tests/suite/image_rollout.rs index e74a81ee25..1f937fbdd7 100644 --- a/codex-rs/core/tests/suite/image_rollout.rs +++ b/codex-rs/core/tests/suite/image_rollout.rs @@ -1,6 +1,10 @@ use anyhow::Context; +use base64::Engine; +use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; +use codex_features::Feature; use codex_protocol::models::ContentItem; use codex_protocol::models::DEFAULT_IMAGE_DETAIL; +use codex_protocol::models::ImageDetail; use codex_protocol::models::PermissionProfile; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::AskForApproval; @@ -9,6 +13,7 @@ use codex_protocol::protocol::Op; use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::RolloutLine; use codex_protocol::user_input::UserInput; +use codex_utils_image::data_url_from_bytes; use core_test_support::TempDirExt; use core_test_support::responses; use core_test_support::responses::ev_assistant_message; @@ -24,9 +29,12 @@ use core_test_support::test_codex::local_selections; use core_test_support::test_codex::test_codex; use core_test_support::test_codex::turn_permission_fields; use core_test_support::wait_for_event; +use image::GenericImageView; use image::ImageBuffer; use image::Rgba; use pretty_assertions::assert_eq; +use serde_json::Value; +use std::fs; use std::path::Path; use std::time::Duration; @@ -278,3 +286,184 @@ async fn drag_drop_image_persists_rollout_request_shape() -> anyhow::Result<()> Ok(()) } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn resumed_history_only_emits_resize_notices_for_new_images() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let initial = test_codex().build_with_auto_env(&server).await?; + responses::mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-initial"), + ev_assistant_message("msg-initial", "recorded"), + ev_completed("resp-initial"), + ]), + ) + .await; + initial.submit_turn("historical image").await?; + + let rollout_path = initial + .session_configured + .rollout_path + .clone() + .context("initial rollout path")?; + initial.codex.shutdown_and_wait().await?; + + let image_path = initial.cwd.path().join("large-image.png"); + ImageBuffer::from_pixel( + /*width*/ 2304, + /*height*/ 864, + Rgba([12u8, 34, 56, 255]), + ) + .save(&image_path)?; + let original_image_url = data_url_from_bytes("image/png", &fs::read(&image_path)?); + + let mut rollout_lines = fs::read_to_string(&rollout_path)? + .lines() + .map(serde_json::from_str::) + .collect::>>()?; + let historical_content = rollout_lines + .iter_mut() + .find_map(|line| match &mut line.item { + RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) + if role == "user" + && content.iter().any(|item| { + matches!(item, ContentItem::InputText { text } if text == "historical image") + }) => + { + Some(content) + } + _ => None, + }) + .context("historical user message in rollout")?; + historical_content.insert( + /*index*/ 0, + ContentItem::InputImage { + image_url: original_image_url.clone(), + detail: Some(ImageDetail::High), + }, + ); + let rollout = rollout_lines + .iter() + .map(serde_json::to_string) + .collect::>>()? + .join("\n"); + fs::write(&rollout_path, format!("{rollout}\n"))?; + + let mut resume_builder = test_codex().with_config(|config| { + let _ = config.features.enable(Feature::ImageResizeNotice); + }); + let resumed = resume_builder + .resume(&server, initial.home.clone(), rollout_path.clone()) + .await?; + let resumed_mock = responses::mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-resumed"), + ev_assistant_message("msg-resumed", "done"), + ev_completed("resp-resumed"), + ]), + ) + .await; + resumed + .codex + .submit(Op::UserInput { + items: vec![UserInput::Image { + image_url: original_image_url, + detail: Some(ImageDetail::High), + }], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: Default::default(), + }) + .await?; + wait_for_event(&resumed.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + let request = resumed_mock.single_request(); + let input = request.input(); + let image_message_indices = input + .iter() + .enumerate() + .filter_map(|(index, item)| { + (item.get("type").and_then(Value::as_str) == Some("message") + && item + .get("content") + .and_then(Value::as_array) + .is_some_and(|content| { + content.iter().any(|item| { + item.get("type").and_then(Value::as_str) == Some("input_image") + }) + })) + .then_some(index) + }) + .collect::>(); + assert_eq!(image_message_indices.len(), 2); + + let historical_image_url = input[image_message_indices[0]] + .get("content") + .and_then(Value::as_array) + .and_then(|content| { + content + .iter() + .find(|item| item.get("type").and_then(Value::as_str) == Some("input_image")) + }) + .and_then(|item| item.get("image_url")) + .and_then(Value::as_str) + .context("historical image URL in resumed request")?; + let (_, encoded_image) = historical_image_url + .split_once(',') + .context("historical image data URL")?; + let historical_image = image::load_from_memory(&BASE64_STANDARD.decode(encoded_image)?)?; + assert_eq!(historical_image.dimensions(), (2048, 768)); + + let expected_notice = concat!( + "\n", + "Image 1 of 1 in the preceding user message was resized from 2304x864 to 2048x768 pixels.\n", + "" + ); + let resize_notices = request + .message_input_texts("developer") + .into_iter() + .filter(|text| text.starts_with("")) + .collect::>(); + assert_eq!(resize_notices, vec![expected_notice.to_string()]); + assert_eq!( + input[image_message_indices[1] + 1] + .get("content") + .and_then(Value::as_array) + .and_then(|content| content.first()) + .and_then(|item| item.get("text")) + .and_then(Value::as_str), + Some(expected_notice) + ); + + resumed.codex.shutdown_and_wait().await?; + let replayed = resume_builder + .resume(&server, resumed.home.clone(), rollout_path) + .await?; + let replayed_mock = responses::mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-replayed"), + ev_assistant_message("msg-replayed", "done"), + ev_completed("resp-replayed"), + ]), + ) + .await; + replayed.submit_turn("preserve recorded notices").await?; + let replayed_notices = replayed_mock + .single_request() + .message_input_texts("developer") + .into_iter() + .filter(|text| text.starts_with("")) + .collect::>(); + assert_eq!(replayed_notices, vec![expected_notice.to_string()]); + + Ok(()) +} diff --git a/codex-rs/core/tests/suite/view_image.rs b/codex-rs/core/tests/suite/view_image.rs index d63e361698..8f03468036 100644 --- a/codex-rs/core/tests/suite/view_image.rs +++ b/codex-rs/core/tests/suite/view_image.rs @@ -7,6 +7,7 @@ use codex_exec_server::CreateDirectoryOptions; use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_exec_server::REMOTE_ENVIRONMENT_ID; use codex_exec_server::RemoveOptions; +use codex_features::Feature; use codex_login::CodexAuth; use codex_protocol::config_types::ReasoningSummary; use codex_protocol::models::PermissionProfile; @@ -66,6 +67,12 @@ use wiremock::MockServer; const VIEW_IMAGE_TURN_COMPLETE_TIMEOUT: Duration = Duration::from_secs(30); +#[derive(Clone, Copy)] +enum ResizeNoticeExpectation { + Disabled, + Enabled, +} + fn disabled_user_turn(test: &TestCodex, items: Vec, model: String) -> Op { let (sandbox_policy, permission_profile) = turn_permission_fields(PermissionProfile::Disabled, test.config.cwd.as_path()); @@ -118,6 +125,29 @@ fn find_image_message(body: &Value) -> Option<&Value> { image_messages(body).into_iter().next() } +fn message_has_text_with_prefix(item: &Value, prefix: &str) -> bool { + item.get("content") + .and_then(Value::as_array) + .is_some_and(|content| { + content.iter().any(|span| { + span.get("text") + .and_then(Value::as_str) + .is_some_and(|text| text.starts_with(prefix)) + }) + }) +} + +fn assert_developer_text_message(item: &Value, expected_text: &str) { + assert_eq!(item.get("role").and_then(Value::as_str), Some("developer")); + assert_eq!( + item.get("content").and_then(Value::as_array), + Some(&vec![json!({ + "type": "input_text", + "text": expected_text, + })]) + ); +} + fn png_bytes(width: u32, height: u32, rgba: [u8; 4]) -> anyhow::Result> { let image = ImageBuffer::from_pixel(width, height, Rgba(rgba)); let mut cursor = Cursor::new(Vec::new()); @@ -174,10 +204,17 @@ async fn write_workspace_png( async fn assert_user_turn_local_image_resizes_to( original_dimensions: (u32, u32), expected_dimensions: (u32, u32), + resize_notice_expectation: ResizeNoticeExpectation, ) -> anyhow::Result<()> { let server = start_mock_server().await; - let mut builder = test_codex(); + let builder = test_codex(); + let mut builder = match resize_notice_expectation { + ResizeNoticeExpectation::Disabled => builder, + ResizeNoticeExpectation::Enabled => builder.with_config(|config| { + let _ = config.features.enable(Feature::ImageResizeNotice); + }), + }; let test = builder.build_with_auto_env(&server).await?; let TestCodex { codex, @@ -220,8 +257,42 @@ async fn assert_user_turn_local_image_resizes_to( .await; let body = mock.single_request().body_json(); + let input = body + .get("input") + .and_then(Value::as_array) + .context("request input")?; let image_message = find_image_message(&body).context("pending input image message not included in request")?; + let image_message_index = input + .iter() + .position(|item| std::ptr::eq(item, image_message)) + .context("image message index")?; + let resize_notice_indices = input + .iter() + .enumerate() + .filter_map(|(index, item)| { + message_has_text_with_prefix(item, "").then_some(index) + }) + .collect::>(); + match resize_notice_expectation { + ResizeNoticeExpectation::Disabled => { + assert_eq!(resize_notice_indices, Vec::::new()); + } + ResizeNoticeExpectation::Enabled => { + assert_eq!(resize_notice_indices, vec![image_message_index + 1]); + assert_developer_text_message( + &input[image_message_index + 1], + &format!( + concat!( + "\n", + "Image 1 of 1 in the preceding user message was resized from {}x{} to {}x{} pixels.\n", + "" + ), + original_width, original_height, expected_dimensions.0, expected_dimensions.1 + ), + ); + } + } let image_url = image_message .get("content") .and_then(Value::as_array) @@ -255,21 +326,36 @@ async fn assert_user_turn_local_image_resizes_to( async fn user_turn_with_local_image_attaches_image() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); - assert_user_turn_local_image_resizes_to((2304, 864), (2048, 768)).await + assert_user_turn_local_image_resizes_to( + (2304, 864), + (2048, 768), + ResizeNoticeExpectation::Disabled, + ) + .await } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn user_turn_with_vertical_local_image_resizes_to_square_bounds() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); - assert_user_turn_local_image_resizes_to((1024, 4096), (512, 2048)).await + assert_user_turn_local_image_resizes_to( + (1024, 4096), + (512, 2048), + ResizeNoticeExpectation::Disabled, + ) + .await } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn user_turn_local_image_applies_patch_budget() -> anyhow::Result<()> { +async fn user_turn_local_image_applies_patch_budget_and_reports_resize() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); - assert_user_turn_local_image_resizes_to((2048, 2048), (1600, 1600)).await + assert_user_turn_local_image_resizes_to( + (2048, 2048), + (1600, 1600), + ResizeNoticeExpectation::Enabled, + ) + .await } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -277,7 +363,9 @@ async fn view_image_tool_attaches_local_image() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; - let mut builder = test_codex(); + let mut builder = test_codex().with_config(|config| { + let _ = config.features.enable(Feature::ImageResizeNotice); + }); let test = builder.build_with_auto_env(&server).await?; let TestCodex { codex, @@ -394,12 +482,31 @@ async fn view_image_tool_attaches_local_image() -> anyhow::Result<()> { assert_eq!( output_items.len(), 1, - "view_image should return only the image content item (no tag/label text)" + "view_image tool output should remain unchanged apart from image preparation" ); assert_eq!( output_items[0].get("type").and_then(Value::as_str), Some("input_image"), - "view_image should return only an input_image content item" + "view_image should return only its input_image content item" + ); + let input = body + .get("input") + .and_then(Value::as_array) + .expect("request input"); + let function_output_index = input + .iter() + .position(|item| { + item.get("type").and_then(Value::as_str) == Some("function_call_output") + && item.get("call_id").and_then(Value::as_str) == Some(call_id) + }) + .expect("function call output index"); + assert_developer_text_message( + &input[function_output_index + 1], + concat!( + "\n", + "Image 1 of 1 in the preceding tool output was resized from 2304x864 to 2048x768 pixels.\n", + "" + ), ); let image_url = output_items[0] .get("image_url") diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index 180eb87b21..aaaa5b5a3a 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -222,6 +222,8 @@ pub enum Feature { ExternalMigration, /// Enable extension-backed image generation. ImageGeneration, + /// Tell the model when a prompt image was resized and include its dimensions. + ImageResizeNotice, /// Removed compatibility flag for always-on centralized image preparation. ResizeAllImages, /// Removed compatibility flag for always-on response item IDs. @@ -1268,6 +1270,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::Stable, default_enabled: true, }, + FeatureSpec { + id: Feature::ImageResizeNotice, + key: "image_resize_notice", + stage: Stage::UnderDevelopment, + default_enabled: false, + }, FeatureSpec { id: Feature::ResizeAllImages, key: "resize_all_images",