mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Support file references in image edit requests (#47956)
## Why Editing recent conversation images failed when the selected images included a file reference from a message or tool output. ## What changed Use `ImageReference` in `ImageEditRequest` to accept both `image_url` and `file_id` inputs. Pass file references from conversation history through to image edit requests while preserving image selection order. ## Testing Update tests to cover file references from messages and tool outputs, and serialization of mixed inline and file references in image edit requests. GitOrigin-RevId: 2764bf36a81a62ddd25cf9df27346a434a10c449
This commit is contained in:
committed by
copyberry
parent
40e13e6281
commit
51d66fdd3b
@@ -122,13 +122,13 @@ mod tests {
|
||||
use crate::images::ImageBackground;
|
||||
use crate::images::ImageData;
|
||||
use crate::images::ImageQuality;
|
||||
use crate::images::ImageUrl;
|
||||
use crate::provider::RetryConfig;
|
||||
use codex_client::Request;
|
||||
use codex_client::RequestBody;
|
||||
use codex_client::Response;
|
||||
use codex_client::StreamResponse;
|
||||
use codex_client::TransportError;
|
||||
use codex_protocol::models::ImageReference;
|
||||
use http::StatusCode;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
@@ -339,9 +339,14 @@ mod tests {
|
||||
let (response, imagegen_request_id) = client
|
||||
.edit(
|
||||
&ImageEditRequest {
|
||||
images: vec![ImageUrl {
|
||||
image_url: "data:image/png;base64,Zm9v".to_string(),
|
||||
}],
|
||||
images: vec![
|
||||
ImageReference::Inline {
|
||||
image_url: "data:image/png;base64,Zm9v".to_string(),
|
||||
},
|
||||
ImageReference::File {
|
||||
file_id: "file-image".to_string(),
|
||||
},
|
||||
],
|
||||
prompt: "add a red hat".to_string(),
|
||||
background: None,
|
||||
model: "gpt-image-1.5".to_string(),
|
||||
@@ -362,7 +367,10 @@ mod tests {
|
||||
assert_eq!(
|
||||
request.body.as_ref().and_then(RequestBody::json),
|
||||
Some(&json!({
|
||||
"images": [{"image_url": "data:image/png;base64,Zm9v"}],
|
||||
"images": [
|
||||
{"image_url": "data:image/png;base64,Zm9v"},
|
||||
{"file_id": "file-image"},
|
||||
],
|
||||
"prompt": "add a red hat",
|
||||
"model": "gpt-image-1.5",
|
||||
}))
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use codex_protocol::models::ImageReference;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
|
||||
@@ -17,7 +18,7 @@ pub struct ImageGenerationRequest {
|
||||
|
||||
#[derive(Debug, Clone, Serialize, PartialEq)]
|
||||
pub struct ImageEditRequest {
|
||||
pub images: Vec<ImageUrl>,
|
||||
pub images: Vec<ImageReference>,
|
||||
pub prompt: String,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub background: Option<ImageBackground>,
|
||||
@@ -30,11 +31,6 @@ pub struct ImageEditRequest {
|
||||
pub size: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
|
||||
pub struct ImageUrl {
|
||||
pub image_url: String,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)]
|
||||
#[serde(rename_all = "lowercase")]
|
||||
pub enum ImageBackground {
|
||||
|
||||
@@ -83,7 +83,6 @@ pub use crate::images::ImageEditRequest;
|
||||
pub use crate::images::ImageGenerationRequest;
|
||||
pub use crate::images::ImageQuality;
|
||||
pub use crate::images::ImageResponse;
|
||||
pub use crate::images::ImageUrl;
|
||||
pub use crate::provider::Provider;
|
||||
pub use crate::provider::RetryConfig;
|
||||
pub use crate::provider::is_azure_responses_provider;
|
||||
|
||||
@@ -2,7 +2,6 @@ use codex_api::ImageBackground;
|
||||
use codex_api::ImageEditRequest;
|
||||
use codex_api::ImageGenerationRequest;
|
||||
use codex_api::ImageQuality;
|
||||
use codex_api::ImageUrl;
|
||||
use codex_extension_api::ToolOutput;
|
||||
use codex_extension_api::ToolPayload;
|
||||
use codex_extension_api::ToolSpec;
|
||||
@@ -168,10 +167,9 @@ async fn recent_image_fallback_selects_newest_images_in_chronological_order() {
|
||||
);
|
||||
}
|
||||
|
||||
/// A file-backed image in the requested window must not cause an older inline image to be edited.
|
||||
#[tokio::test]
|
||||
async fn recent_image_fallback_rejects_file_backed_image_in_requested_window() {
|
||||
let error = request_for_call_args(
|
||||
async fn recent_image_fallback_passes_file_backed_image_to_edit_request() {
|
||||
let request = request_for_call_args(
|
||||
&ImagegenArgs {
|
||||
prompt: "change the lighting".to_string(),
|
||||
transparent_background: false,
|
||||
@@ -196,19 +194,28 @@ async fn recent_image_fallback_rejects_file_backed_image_in_requested_window() {
|
||||
&[],
|
||||
)
|
||||
.await
|
||||
.expect_err("a selected file-backed image should fail");
|
||||
.expect("a selected file-backed image should be passed through");
|
||||
|
||||
assert_eq!(
|
||||
error.to_string(),
|
||||
"requested the last 1 conversation images, but that window includes a file-backed image \
|
||||
that cannot be used for editing"
|
||||
request,
|
||||
ImageRequest::Edit(ImageEditRequest {
|
||||
images: vec![ImageReference::File {
|
||||
file_id: "newer-file-backed-image".to_string(),
|
||||
}],
|
||||
prompt: "change the lighting".to_string(),
|
||||
background: Some(ImageBackground::Opaque),
|
||||
model: "gpt-image-2".to_string(),
|
||||
n: None,
|
||||
quality: Some(ImageQuality::Auto),
|
||||
size: Some("auto".to_string()),
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
/// Tool-output file references must count toward the window instead of exposing an older image.
|
||||
#[tokio::test]
|
||||
async fn recent_image_fallback_rejects_file_backed_tool_output_in_requested_window() {
|
||||
let error = request_for_call_args(
|
||||
async fn recent_image_fallback_passes_file_backed_tool_output_to_edit_request() {
|
||||
let request = request_for_call_args(
|
||||
&ImagegenArgs {
|
||||
prompt: "change the lighting".to_string(),
|
||||
transparent_background: false,
|
||||
@@ -242,12 +249,21 @@ async fn recent_image_fallback_rejects_file_backed_tool_output_in_requested_wind
|
||||
&[],
|
||||
)
|
||||
.await
|
||||
.expect_err("a selected file-backed tool output should fail");
|
||||
.expect("a selected file-backed tool output should be passed through");
|
||||
|
||||
assert_eq!(
|
||||
error.to_string(),
|
||||
"requested the last 1 conversation images, but that window includes a file-backed image \
|
||||
that cannot be used for editing"
|
||||
request,
|
||||
ImageRequest::Edit(ImageEditRequest {
|
||||
images: vec![ImageReference::File {
|
||||
file_id: "newer-file-backed-image".to_string(),
|
||||
}],
|
||||
prompt: "change the lighting".to_string(),
|
||||
background: Some(ImageBackground::Opaque),
|
||||
model: "gpt-image-2".to_string(),
|
||||
n: None,
|
||||
quality: Some(ImageQuality::Auto),
|
||||
size: Some("auto".to_string()),
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
@@ -432,7 +448,7 @@ fn expected_edit_request(prompt: &str, images: &[&str]) -> ImageEditRequest {
|
||||
ImageEditRequest {
|
||||
images: images
|
||||
.iter()
|
||||
.map(|image| ImageUrl {
|
||||
.map(|image| ImageReference::Inline {
|
||||
image_url: format!("data:image/png;base64,{image}"),
|
||||
})
|
||||
.collect(),
|
||||
|
||||
@@ -6,7 +6,6 @@ use codex_api::ImageBackground;
|
||||
use codex_api::ImageEditRequest;
|
||||
use codex_api::ImageGenerationRequest;
|
||||
use codex_api::ImageQuality;
|
||||
use codex_api::ImageUrl;
|
||||
use codex_exec_server::CreateDirectoryOptions;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_extension_api::ExtensionTurnItem;
|
||||
@@ -500,16 +499,14 @@ async fn request_for_call_args(
|
||||
fn recent_images(
|
||||
history: &[ResponseItem],
|
||||
count: usize,
|
||||
) -> Result<Vec<ImageUrl>, FunctionCallError> {
|
||||
) -> Result<Vec<ImageReference>, FunctionCallError> {
|
||||
let mut images = Vec::with_capacity(count);
|
||||
'history: for item in history.iter().rev() {
|
||||
let mut image_urls = Vec::new();
|
||||
let mut image_references = Vec::new();
|
||||
match item {
|
||||
ResponseItem::Message { content, .. } => {
|
||||
image_urls.extend(content.iter().rev().filter_map(|item| match item {
|
||||
ContentItem::InputImage { image, .. } => {
|
||||
Some(inline_image_url(image).map(str::to_owned))
|
||||
}
|
||||
image_references.extend(content.iter().rev().filter_map(|item| match item {
|
||||
ContentItem::InputImage { image, .. } => Some(image.clone()),
|
||||
ContentItem::InputText { .. }
|
||||
| ContentItem::InputAudio { .. }
|
||||
| ContentItem::OutputText { .. } => None,
|
||||
@@ -517,12 +514,12 @@ fn recent_images(
|
||||
}
|
||||
ResponseItem::FunctionCallOutput { output, .. }
|
||||
| ResponseItem::CustomToolCallOutput { output, .. } => {
|
||||
image_urls.extend(
|
||||
output_images(output).map(|image| inline_image_url(image).map(str::to_owned)),
|
||||
);
|
||||
image_references.extend(output_images(output).cloned());
|
||||
}
|
||||
ResponseItem::ImageGenerationCall { result, .. } if !result.is_empty() => {
|
||||
image_urls.push(Some(format!("data:image/png;base64,{result}")));
|
||||
image_references.push(ImageReference::Inline {
|
||||
image_url: format!("data:image/png;base64,{result}"),
|
||||
});
|
||||
}
|
||||
ResponseItem::AdditionalTools { .. }
|
||||
| ResponseItem::Reasoning { .. }
|
||||
@@ -540,8 +537,8 @@ fn recent_images(
|
||||
| ResponseItem::ContextCompaction { .. }
|
||||
| ResponseItem::Other => {}
|
||||
}
|
||||
for image_url in image_urls {
|
||||
images.push(image_url.map(|image_url| ImageUrl { image_url }));
|
||||
for image in image_references {
|
||||
images.push(image);
|
||||
if images.len() == count {
|
||||
break 'history;
|
||||
}
|
||||
@@ -553,18 +550,8 @@ fn recent_images(
|
||||
images.len()
|
||||
)));
|
||||
}
|
||||
let mut inline_images = Vec::with_capacity(count);
|
||||
for image in images {
|
||||
let Some(image) = image else {
|
||||
return Err(FunctionCallError::RespondToModel(format!(
|
||||
"requested the last {count} conversation images, but that window includes a \
|
||||
file-backed image that cannot be used for editing"
|
||||
)));
|
||||
};
|
||||
inline_images.push(image);
|
||||
}
|
||||
inline_images.reverse();
|
||||
Ok(inline_images)
|
||||
images.reverse();
|
||||
Ok(images)
|
||||
}
|
||||
|
||||
/// Extracts image references from a tool output in newest-first order.
|
||||
@@ -582,18 +569,10 @@ fn output_images(output: &FunctionCallOutputPayload) -> impl Iterator<Item = &Im
|
||||
})
|
||||
}
|
||||
|
||||
fn inline_image_url(image: &ImageReference) -> Option<&str> {
|
||||
match image {
|
||||
ImageReference::Inline { image_url } => Some(image_url),
|
||||
// TODO(kc) Image generation and the Images API only accept inline image URLs.
|
||||
ImageReference::File { .. } => None,
|
||||
}
|
||||
}
|
||||
|
||||
async fn image_url(
|
||||
path: &AbsolutePathBuf,
|
||||
environment: &ToolEnvironment<'_>,
|
||||
) -> Result<ImageUrl, FunctionCallError> {
|
||||
) -> Result<ImageReference, FunctionCallError> {
|
||||
let path_uri = PathUri::from_abs_path(path);
|
||||
let sandbox = environment.file_system_sandbox_context.clone();
|
||||
let bytes = environment
|
||||
@@ -614,7 +593,7 @@ async fn image_url(
|
||||
))
|
||||
},
|
||||
)?;
|
||||
Ok(ImageUrl {
|
||||
Ok(ImageReference::Inline {
|
||||
image_url: image.into_data_url(),
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user