mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Preserve image detail in app-server inputs (#20693)
## Summary - Add optional image detail to user image inputs across core, app-server v2, thread history/event mapping, and the generated app-server schemas/types. - Preserve requested detail when serializing Responses image inputs: omitted detail stays on the existing `high` default, while explicit `original` keeps local images on the original-resolution path. - Support `high`/`original` consistently for tool image outputs, including MCP `codex/imageDetail`, code-mode image helpers, and `view_image`.
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
use crate::mcp::CallToolResult;
|
||||
use crate::memory_citation::MemoryCitation;
|
||||
use crate::models::ContentItem;
|
||||
use crate::models::ImageDetail;
|
||||
use crate::models::MessagePhase;
|
||||
use crate::models::ResponseItem;
|
||||
use crate::models::WebSearchAction;
|
||||
@@ -243,7 +244,9 @@ impl UserMessageItem {
|
||||
EventMsg::UserMessage(UserMessageEvent {
|
||||
message: self.message(),
|
||||
images: Some(self.image_urls()),
|
||||
image_details: self.image_details(),
|
||||
local_images: self.local_image_paths(),
|
||||
local_image_details: self.local_image_details(),
|
||||
text_elements: self.text_elements(),
|
||||
})
|
||||
}
|
||||
@@ -290,21 +293,54 @@ impl UserMessageItem {
|
||||
self.content
|
||||
.iter()
|
||||
.filter_map(|c| match c {
|
||||
UserInput::Image { image_url } => Some(image_url.clone()),
|
||||
UserInput::Image { image_url, .. } => Some(image_url.clone()),
|
||||
_ => None,
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
pub fn image_details(&self) -> Vec<Option<ImageDetail>> {
|
||||
trim_trailing_default_image_details(
|
||||
self.content
|
||||
.iter()
|
||||
.filter_map(|c| match c {
|
||||
UserInput::Image { detail, .. } => Some(*detail),
|
||||
_ => None,
|
||||
})
|
||||
.collect(),
|
||||
)
|
||||
}
|
||||
|
||||
pub fn local_image_paths(&self) -> Vec<std::path::PathBuf> {
|
||||
self.content
|
||||
.iter()
|
||||
.filter_map(|c| match c {
|
||||
UserInput::LocalImage { path } => Some(path.clone()),
|
||||
UserInput::LocalImage { path, .. } => Some(path.clone()),
|
||||
_ => None,
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
pub fn local_image_details(&self) -> Vec<Option<ImageDetail>> {
|
||||
trim_trailing_default_image_details(
|
||||
self.content
|
||||
.iter()
|
||||
.filter_map(|c| match c {
|
||||
UserInput::LocalImage { detail, .. } => Some(*detail),
|
||||
_ => None,
|
||||
})
|
||||
.collect(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
fn trim_trailing_default_image_details(
|
||||
mut details: Vec<Option<ImageDetail>>,
|
||||
) -> Vec<Option<ImageDetail>> {
|
||||
while matches!(details.last(), Some(None)) {
|
||||
details.pop();
|
||||
}
|
||||
details
|
||||
}
|
||||
|
||||
impl HookPromptItem {
|
||||
|
||||
@@ -721,8 +721,6 @@ pub enum ContentItem {
|
||||
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "lowercase")]
|
||||
pub enum ImageDetail {
|
||||
Auto,
|
||||
Low,
|
||||
High,
|
||||
Original,
|
||||
}
|
||||
@@ -1064,8 +1062,13 @@ pub fn local_image_content_items_with_label_number(
|
||||
path: &std::path::Path,
|
||||
file_bytes: Vec<u8>,
|
||||
label_number: Option<usize>,
|
||||
mode: PromptImageMode,
|
||||
detail: ImageDetail,
|
||||
) -> Vec<ContentItem> {
|
||||
let mode = match detail {
|
||||
ImageDetail::Original => PromptImageMode::Original,
|
||||
ImageDetail::High => PromptImageMode::ResizeToFit,
|
||||
};
|
||||
|
||||
match load_for_prompt_bytes(path, file_bytes, mode) {
|
||||
Ok(image) => {
|
||||
let mut items = Vec::with_capacity(3);
|
||||
@@ -1076,7 +1079,7 @@ pub fn local_image_content_items_with_label_number(
|
||||
}
|
||||
items.push(ContentItem::InputImage {
|
||||
image_url: image.into_data_url(),
|
||||
detail: Some(DEFAULT_IMAGE_DETAIL),
|
||||
detail: Some(detail),
|
||||
});
|
||||
if label_number.is_some() {
|
||||
items.push(ContentItem::InputText {
|
||||
@@ -1221,29 +1224,31 @@ impl From<Vec<UserInput>> for ResponseInputItem {
|
||||
.into_iter()
|
||||
.flat_map(|c| match c {
|
||||
UserInput::Text { text, .. } => vec![ContentItem::InputText { text }],
|
||||
UserInput::Image { image_url } => {
|
||||
UserInput::Image { image_url, detail } => {
|
||||
image_index += 1;
|
||||
let detail = detail.unwrap_or(DEFAULT_IMAGE_DETAIL);
|
||||
vec![
|
||||
ContentItem::InputText {
|
||||
text: image_open_tag_text(),
|
||||
},
|
||||
ContentItem::InputImage {
|
||||
image_url,
|
||||
detail: Some(DEFAULT_IMAGE_DETAIL),
|
||||
detail: Some(detail),
|
||||
},
|
||||
ContentItem::InputText {
|
||||
text: image_close_tag_text(),
|
||||
},
|
||||
]
|
||||
}
|
||||
UserInput::LocalImage { path } => {
|
||||
UserInput::LocalImage { path, detail } => {
|
||||
image_index += 1;
|
||||
let detail = detail.unwrap_or(DEFAULT_IMAGE_DETAIL);
|
||||
match std::fs::read(&path) {
|
||||
Ok(file_bytes) => local_image_content_items_with_label_number(
|
||||
&path,
|
||||
file_bytes,
|
||||
Some(image_index),
|
||||
PromptImageMode::ResizeToFit,
|
||||
detail,
|
||||
),
|
||||
Err(err) => vec![local_image_error_placeholder(&path, err)],
|
||||
}
|
||||
@@ -1587,8 +1592,6 @@ fn convert_mcp_content_to_items(
|
||||
.and_then(|meta| meta.get(CODEX_IMAGE_DETAIL_META_KEY))
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.and_then(|detail| match detail {
|
||||
"auto" => Some(ImageDetail::Auto),
|
||||
"low" => Some(ImageDetail::Low),
|
||||
"high" => Some(ImageDetail::High),
|
||||
"original" => Some(ImageDetail::Original),
|
||||
_ => None,
|
||||
@@ -1633,6 +1636,14 @@ mod tests {
|
||||
use std::path::PathBuf;
|
||||
use tempfile::tempdir;
|
||||
|
||||
// A tiny valid PNG (1x1) so image conversion tests don't depend on cross-crate
|
||||
// file paths, which break under Bazel sandboxing.
|
||||
const TINY_PNG_BYTES: &[u8] = &[
|
||||
137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82, 0, 0, 0, 1, 0, 0, 0, 1, 8, 6,
|
||||
0, 0, 0, 31, 21, 196, 137, 0, 0, 0, 11, 73, 68, 65, 84, 120, 156, 99, 96, 0, 2, 0, 0, 5, 0,
|
||||
1, 122, 94, 171, 63, 0, 0, 0, 0, 73, 69, 78, 68, 174, 66, 96, 130,
|
||||
];
|
||||
|
||||
#[test]
|
||||
fn response_input_message_conversion_preserves_phase() {
|
||||
let item = ResponseItem::from(ResponseInputItem::Message {
|
||||
@@ -2545,6 +2556,7 @@ mod tests {
|
||||
|
||||
let item = ResponseInputItem::from(vec![UserInput::Image {
|
||||
image_url: image_url.clone(),
|
||||
detail: None,
|
||||
}]);
|
||||
|
||||
match item {
|
||||
@@ -2569,6 +2581,31 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn image_user_input_preserves_requested_detail() -> Result<()> {
|
||||
let image_url = "data:image/png;base64,abc".to_string();
|
||||
|
||||
let item = ResponseInputItem::from(vec![UserInput::Image {
|
||||
image_url: image_url.clone(),
|
||||
detail: Some(ImageDetail::Original),
|
||||
}]);
|
||||
|
||||
match item {
|
||||
ResponseInputItem::Message { content, .. } => {
|
||||
assert_eq!(
|
||||
content.get(1),
|
||||
Some(&ContentItem::InputImage {
|
||||
image_url,
|
||||
detail: Some(ImageDetail::Original),
|
||||
})
|
||||
);
|
||||
}
|
||||
other => panic!("expected message response but got {other:?}"),
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_search_call_roundtrips() -> Result<()> {
|
||||
let parsed: ResponseItem = serde_json::from_str(
|
||||
@@ -2737,20 +2774,17 @@ mod tests {
|
||||
let image_url = "data:image/png;base64,abc".to_string();
|
||||
let dir = tempdir()?;
|
||||
let local_path = dir.path().join("local.png");
|
||||
// A tiny valid PNG (1x1) so this test doesn't depend on cross-crate file paths, which
|
||||
// break under Bazel sandboxing.
|
||||
const TINY_PNG_BYTES: &[u8] = &[
|
||||
137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82, 0, 0, 0, 1, 0, 0, 0, 1,
|
||||
8, 6, 0, 0, 0, 31, 21, 196, 137, 0, 0, 0, 11, 73, 68, 65, 84, 120, 156, 99, 96, 0, 2,
|
||||
0, 0, 5, 0, 1, 122, 94, 171, 63, 0, 0, 0, 0, 73, 69, 78, 68, 174, 66, 96, 130,
|
||||
];
|
||||
std::fs::write(&local_path, TINY_PNG_BYTES)?;
|
||||
|
||||
let item = ResponseInputItem::from(vec![
|
||||
UserInput::Image {
|
||||
image_url: image_url.clone(),
|
||||
detail: None,
|
||||
},
|
||||
UserInput::LocalImage {
|
||||
path: local_path,
|
||||
detail: None,
|
||||
},
|
||||
UserInput::LocalImage { path: local_path },
|
||||
]);
|
||||
|
||||
match item {
|
||||
@@ -2797,6 +2831,33 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn local_image_user_input_preserves_requested_detail() -> Result<()> {
|
||||
let dir = tempdir()?;
|
||||
let local_path = dir.path().join("local.png");
|
||||
std::fs::write(&local_path, TINY_PNG_BYTES)?;
|
||||
|
||||
let item = ResponseInputItem::from(vec![UserInput::LocalImage {
|
||||
path: local_path,
|
||||
detail: Some(ImageDetail::Original),
|
||||
}]);
|
||||
|
||||
match item {
|
||||
ResponseInputItem::Message { content, .. } => {
|
||||
assert!(matches!(
|
||||
content.get(1),
|
||||
Some(ContentItem::InputImage {
|
||||
detail: Some(ImageDetail::Original),
|
||||
..
|
||||
})
|
||||
));
|
||||
}
|
||||
other => panic!("expected message response but got {other:?}"),
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn local_image_read_error_adds_placeholder() -> Result<()> {
|
||||
let dir = tempdir()?;
|
||||
@@ -2804,6 +2865,7 @@ mod tests {
|
||||
|
||||
let item = ResponseInputItem::from(vec![UserInput::LocalImage {
|
||||
path: missing_path.clone(),
|
||||
detail: None,
|
||||
}]);
|
||||
|
||||
match item {
|
||||
@@ -2838,6 +2900,7 @@ mod tests {
|
||||
|
||||
let item = ResponseInputItem::from(vec![UserInput::LocalImage {
|
||||
path: json_path.clone(),
|
||||
detail: None,
|
||||
}]);
|
||||
|
||||
match item {
|
||||
@@ -2875,6 +2938,7 @@ mod tests {
|
||||
|
||||
let item = ResponseInputItem::from(vec![UserInput::LocalImage {
|
||||
path: svg_path.clone(),
|
||||
detail: None,
|
||||
}]);
|
||||
|
||||
match item {
|
||||
|
||||
@@ -34,6 +34,7 @@ use crate::memory_citation::MemoryCitation;
|
||||
use crate::models::ActivePermissionProfile;
|
||||
use crate::models::BaseInstructions;
|
||||
use crate::models::ContentItem;
|
||||
use crate::models::ImageDetail;
|
||||
use crate::models::MessagePhase;
|
||||
use crate::models::PermissionProfile;
|
||||
use crate::models::ResponseInputItem;
|
||||
@@ -2225,7 +2226,7 @@ pub struct AgentMessageEvent {
|
||||
pub memory_citation: Option<MemoryCitation>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
|
||||
#[derive(Debug, Clone, Default, Deserialize, Serialize, JsonSchema, TS)]
|
||||
pub struct UserMessageEvent {
|
||||
pub message: String,
|
||||
/// Image URLs sourced from `UserInput::Image`. These are safe
|
||||
@@ -2233,11 +2234,19 @@ pub struct UserMessageEvent {
|
||||
/// the model.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub images: Option<Vec<String>>,
|
||||
/// Detail hints for `images`, indexed in parallel. Missing entries imply
|
||||
/// default image detail behavior.
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
pub image_details: Vec<Option<ImageDetail>>,
|
||||
/// Local file paths sourced from `UserInput::LocalImage`. These are kept so
|
||||
/// the UI can reattach images when editing history, and should not be sent
|
||||
/// to the model or treated as API-ready URLs.
|
||||
#[serde(default)]
|
||||
pub local_images: Vec<std::path::PathBuf>,
|
||||
/// Detail hints for `local_images`, indexed in parallel. Missing entries
|
||||
/// imply default image detail behavior.
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
pub local_image_details: Vec<Option<ImageDetail>>,
|
||||
/// UI-defined spans within `message` used to render or persist special elements.
|
||||
#[serde(default)]
|
||||
pub text_elements: Vec<crate::user_input::TextElement>,
|
||||
@@ -5133,6 +5142,7 @@ mod tests {
|
||||
images: None,
|
||||
local_images: Vec::new(),
|
||||
text_elements: Vec::new(),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let json_event = serde_json::to_value(event)?;
|
||||
@@ -5148,6 +5158,62 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn user_message_event_deserializes_without_image_detail_fields() -> Result<()> {
|
||||
let event: UserMessageEvent = serde_json::from_value(json!({
|
||||
"message": "hello",
|
||||
"images": ["https://example.com/image.png"],
|
||||
"local_images": ["/tmp/local.png"],
|
||||
"text_elements": [],
|
||||
}))?;
|
||||
|
||||
assert_eq!(event.message, "hello");
|
||||
assert_eq!(
|
||||
event.images,
|
||||
Some(vec!["https://example.com/image.png".to_string()])
|
||||
);
|
||||
assert_eq!(event.image_details, Vec::<Option<ImageDetail>>::new());
|
||||
assert_eq!(event.local_images, vec![PathBuf::from("/tmp/local.png")]);
|
||||
assert_eq!(event.local_image_details, Vec::<Option<ImageDetail>>::new());
|
||||
assert_eq!(event.text_elements, Vec::new());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn user_message_item_legacy_event_preserves_image_details() {
|
||||
let local_path = PathBuf::from("/tmp/local.png");
|
||||
let item = UserMessageItem::new(&[
|
||||
crate::user_input::UserInput::Image {
|
||||
image_url: "https://example.com/first.png".to_string(),
|
||||
detail: Some(ImageDetail::Original),
|
||||
},
|
||||
crate::user_input::UserInput::Image {
|
||||
image_url: "https://example.com/second.png".to_string(),
|
||||
detail: None,
|
||||
},
|
||||
crate::user_input::UserInput::LocalImage {
|
||||
path: local_path.clone(),
|
||||
detail: Some(ImageDetail::Original),
|
||||
},
|
||||
]);
|
||||
|
||||
let EventMsg::UserMessage(event) = item.as_legacy_event() else {
|
||||
panic!("expected user message event");
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
event.images,
|
||||
Some(vec![
|
||||
"https://example.com/first.png".to_string(),
|
||||
"https://example.com/second.png".to_string(),
|
||||
])
|
||||
);
|
||||
assert_eq!(event.image_details, vec![Some(ImageDetail::Original)]);
|
||||
assert_eq!(event.local_images, vec![local_path]);
|
||||
assert_eq!(event.local_image_details, vec![Some(ImageDetail::Original)]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn turn_aborted_event_deserializes_without_turn_id() -> Result<()> {
|
||||
let event: EventMsg = serde_json::from_value(json!({
|
||||
|
||||
@@ -3,6 +3,8 @@ use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use ts_rs::TS;
|
||||
|
||||
use crate::models::ImageDetail;
|
||||
|
||||
/// Conservative cap so one user message cannot monopolize a large context window.
|
||||
pub const MAX_USER_INPUT_TEXT_CHARS: usize = 1 << 20;
|
||||
|
||||
@@ -21,11 +23,21 @@ pub enum UserInput {
|
||||
text_elements: Vec<TextElement>,
|
||||
},
|
||||
/// Pre‑encoded data: URI image.
|
||||
Image { image_url: String },
|
||||
Image {
|
||||
image_url: String,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
detail: Option<ImageDetail>,
|
||||
},
|
||||
|
||||
/// Local image path provided by the user. This will be converted to an
|
||||
/// `Image` variant (base64 data URL) during request serialization.
|
||||
LocalImage { path: std::path::PathBuf },
|
||||
LocalImage {
|
||||
path: std::path::PathBuf,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
detail: Option<ImageDetail>,
|
||||
},
|
||||
|
||||
/// Skill selected by the user (name + path to SKILL.md).
|
||||
Skill {
|
||||
|
||||
Reference in New Issue
Block a user