mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Report prompt image resizing to the model (#37134)
## What changed - Add the disabled-by-default `image_resize_notice` feature. - When enabled, append a developer message after resized images from user messages or tool outputs. The notice identifies each image and reports its original and prepared dimensions. - Persist notices with new history while avoiding backfilled notices for images processed when a session is resumed. ## Testing - Cover notice placement, image numbering, failed images, user and tool image sources, and resumed-session replay. GitOrigin-RevId: 11dd808b3ad47eac35e9e2f4a4370f85d0122770
This commit is contained in:
committed by
copyberry
parent
2994f545a7
commit
fa5d5ae047
@@ -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"
|
||||
},
|
||||
|
||||
@@ -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)] =
|
||||
&[("<image_resize_notice>", "</image_resize_notice>")];
|
||||
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub(crate) struct HistoryItemGroup<T> {
|
||||
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)
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
@@ -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<ResizedImage>,
|
||||
}
|
||||
|
||||
impl ImageResizeNotice {
|
||||
pub(crate) fn new(source: ImageResizeNoticeSource, resized_images: Vec<ResizedImage>) -> 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) {
|
||||
("<image_resize_notice>", "</image_resize_notice>")
|
||||
}
|
||||
|
||||
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::<Vec<_>>()
|
||||
.join("\n");
|
||||
format!("\n{notices}\n")
|
||||
}
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
@@ -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<ImagePreparationMetadata> {
|
||||
pub(crate) fn prepare_response_items(
|
||||
items: &mut Vec<ResponseItem>,
|
||||
resize_notice_mode: ImageResizeNoticeMode,
|
||||
) -> Vec<ImagePreparationMetadata> {
|
||||
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<ImagePre
|
||||
| ResponseItem::Compaction { .. }
|
||||
| ResponseItem::CompactionTrigger { .. }
|
||||
| ResponseItem::ContextCompaction { .. }
|
||||
| ResponseItem::Other => {}
|
||||
| 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<ImagePreparationMetadata>,
|
||||
) {
|
||||
) -> Vec<ResizedImage> {
|
||||
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<ImagePreparationMetadata>,
|
||||
) {
|
||||
) -> Vec<ResizedImage> {
|
||||
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<ImageDetail>,
|
||||
origin: ImageOrigin<'_>,
|
||||
metadata: &mut Vec<ImagePreparationMetadata>,
|
||||
) -> Result<(), ImagePreparationError> {
|
||||
) -> Result<Option<PreparedImageResize>, 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)]
|
||||
|
||||
@@ -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!(
|
||||
"<image_resize_notice>\n",
|
||||
"Image 3 of 3 in the preceding user message was resized from 2048x2048 to 1600x1600 pixels.\n",
|
||||
"</image_resize_notice>"
|
||||
);
|
||||
|
||||
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!(
|
||||
"<image_resize_notice>\n",
|
||||
"Image 2 of 2 in the preceding tool output was resized from 2048x2048 to 1600x1600 pixels.\n",
|
||||
"</image_resize_notice>"
|
||||
)
|
||||
.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,
|
||||
|
||||
@@ -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<ImagePreparationMetadata>) {
|
||||
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,
|
||||
|
||||
@@ -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::<RolloutLine>)
|
||||
.collect::<serde_json::Result<Vec<_>>>()?;
|
||||
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::<serde_json::Result<Vec<_>>>()?
|
||||
.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::<Vec<_>>();
|
||||
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!(
|
||||
"<image_resize_notice>\n",
|
||||
"Image 1 of 1 in the preceding user message was resized from 2304x864 to 2048x768 pixels.\n",
|
||||
"</image_resize_notice>"
|
||||
);
|
||||
let resize_notices = request
|
||||
.message_input_texts("developer")
|
||||
.into_iter()
|
||||
.filter(|text| text.starts_with("<image_resize_notice>"))
|
||||
.collect::<Vec<_>>();
|
||||
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("<image_resize_notice>"))
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(replayed_notices, vec![expected_notice.to_string()]);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -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<UserInput>, 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<Vec<u8>> {
|
||||
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, "<image_resize_notice>").then_some(index)
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
match resize_notice_expectation {
|
||||
ResizeNoticeExpectation::Disabled => {
|
||||
assert_eq!(resize_notice_indices, Vec::<usize>::new());
|
||||
}
|
||||
ResizeNoticeExpectation::Enabled => {
|
||||
assert_eq!(resize_notice_indices, vec![image_message_index + 1]);
|
||||
assert_developer_text_message(
|
||||
&input[image_message_index + 1],
|
||||
&format!(
|
||||
concat!(
|
||||
"<image_resize_notice>\n",
|
||||
"Image 1 of 1 in the preceding user message was resized from {}x{} to {}x{} pixels.\n",
|
||||
"</image_resize_notice>"
|
||||
),
|
||||
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!(
|
||||
"<image_resize_notice>\n",
|
||||
"Image 1 of 1 in the preceding tool output was resized from 2304x864 to 2048x768 pixels.\n",
|
||||
"</image_resize_notice>"
|
||||
),
|
||||
);
|
||||
let image_url = output_items[0]
|
||||
.get("image_url")
|
||||
|
||||
@@ -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",
|
||||
|
||||
Reference in New Issue
Block a user