mirror of
https://github.com/openai/codex.git
synced 2026-10-20 08:19:50 +00:00
Normalize image detail for the receiving model (#44249)
## Why Switching models can send images from conversation history with `detail: original` to a model that does not support it. ## What changed Normalize image details on request copies using the receiving model's capabilities. Downgrade `original` to `high` when unsupported, covering user messages and function and custom tool outputs. Preserve stored image details so switching back to a supporting model retains `original`, and continue stripping details for Responses Lite. ## Testing Expand unit coverage across image detail values and model capabilities. Extend integration coverage for model switches within and between turns, text-only models, and resumed and forked threads, including assertions that persisted tool outputs remain unchanged. GitOrigin-RevId: 2561651ea74f045e00f708ae53771a41dcd49c94
This commit is contained in:
@@ -16,6 +16,7 @@ use app_test_support::format_with_current_shell_display;
|
||||
use app_test_support::write_chatgpt_auth;
|
||||
use app_test_support::write_mock_responses_config_toml_with_chatgpt_base_url;
|
||||
use app_test_support::write_models_cache;
|
||||
use app_test_support::write_models_cache_with_models;
|
||||
use codex_app_server::INPUT_TOO_LARGE_ERROR_CODE;
|
||||
use codex_app_server::INVALID_PARAMS_ERROR_CODE;
|
||||
use codex_app_server_protocol::AdditionalContextEntry;
|
||||
@@ -76,6 +77,7 @@ use codex_core::test_support::all_model_presets;
|
||||
use codex_exec_server::LOCAL_ENVIRONMENT_ID;
|
||||
use codex_features::Feature;
|
||||
use codex_login::AuthCredentialsStoreMode;
|
||||
use codex_models_manager::model_info::model_info_from_slug;
|
||||
use codex_protocol::config_types::CollaborationMode;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::config_types::MultiAgentMode;
|
||||
@@ -141,6 +143,9 @@ async fn run_local_image_turn(detail: Option<ImageDetail>) -> Result<Vec<Value>>
|
||||
|
||||
let codex_home = TempDir::new()?;
|
||||
MockResponsesConfig::new(&server.uri()).write(codex_home.path())?;
|
||||
let mut model = model_info_from_slug("mock-model");
|
||||
model.supports_image_detail_original = true;
|
||||
write_models_cache_with_models(codex_home.path(), vec![model]).await?;
|
||||
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
|
||||
@@ -897,7 +897,7 @@ impl ModelClient {
|
||||
service_tier: Option<String>,
|
||||
responses_metadata: &CodexResponsesMetadata,
|
||||
) -> Result<ResponsesApiRequest> {
|
||||
let mut input = prompt.get_formatted_input_for_request(model_info.use_responses_lite);
|
||||
let mut input = prompt.get_formatted_input_for_request(model_info);
|
||||
let is_openai = self.state.provider.info().is_openai();
|
||||
let (instructions, tools) = if model_info.use_responses_lite {
|
||||
// These prompt-only items are rebuilt on every request. Hash their visible payloads
|
||||
|
||||
@@ -2,8 +2,11 @@ pub use codex_api::ResponseEvent;
|
||||
use codex_protocol::error::Result;
|
||||
use codex_protocol::models::BaseInstructions;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::DEFAULT_IMAGE_DETAIL;
|
||||
use codex_protocol::models::FunctionCallOutputContentItem;
|
||||
use codex_protocol::models::ImageDetail;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_tools::ToolSpec;
|
||||
use futures::Stream;
|
||||
use serde_json::Value;
|
||||
@@ -55,23 +58,21 @@ impl Default for Prompt {
|
||||
impl Prompt {
|
||||
pub(crate) fn get_formatted_input_for_request(
|
||||
&self,
|
||||
use_responses_lite: bool,
|
||||
model_info: &ModelInfo,
|
||||
) -> Vec<ResponseItem> {
|
||||
let mut input = self.input.clone();
|
||||
if use_responses_lite {
|
||||
strip_image_details(&mut input);
|
||||
}
|
||||
normalize_image_details(&mut input, model_info);
|
||||
input
|
||||
}
|
||||
}
|
||||
|
||||
fn strip_image_details(items: &mut [ResponseItem]) {
|
||||
fn normalize_image_details(items: &mut [ResponseItem], model_info: &ModelInfo) {
|
||||
for item in items {
|
||||
match item {
|
||||
ResponseItem::Message { content, .. } => {
|
||||
for content_item in content {
|
||||
if let ContentItem::InputImage { detail, .. } = content_item {
|
||||
*detail = None;
|
||||
normalize_image_detail(detail, model_info);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -82,7 +83,7 @@ fn strip_image_details(items: &mut [ResponseItem]) {
|
||||
if let FunctionCallOutputContentItem::InputImage { detail, .. } =
|
||||
content_item
|
||||
{
|
||||
*detail = None;
|
||||
normalize_image_detail(detail, model_info);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -106,6 +107,14 @@ fn strip_image_details(items: &mut [ResponseItem]) {
|
||||
}
|
||||
}
|
||||
|
||||
fn normalize_image_detail(detail: &mut Option<ImageDetail>, model_info: &ModelInfo) {
|
||||
if model_info.use_responses_lite {
|
||||
*detail = None;
|
||||
} else if *detail == Some(ImageDetail::Original) && !model_info.supports_image_detail_original {
|
||||
*detail = Some(DEFAULT_IMAGE_DETAIL);
|
||||
}
|
||||
}
|
||||
|
||||
pub struct ResponseStream {
|
||||
pub(crate) rx_event: mpsc::Receiver<Result<ResponseEvent>>,
|
||||
/// Signals the mapper task that the consumer stopped polling before the
|
||||
|
||||
@@ -2,6 +2,7 @@ use codex_api::OpenAiVerbosity;
|
||||
use codex_api::ResponsesApiRequest;
|
||||
use codex_api::TextControls;
|
||||
use codex_api::create_text_param_for_request;
|
||||
use codex_models_manager::model_info::model_info_from_slug;
|
||||
use codex_protocol::config_types::ServiceTier;
|
||||
use codex_protocol::models::FunctionCallOutputPayload;
|
||||
use codex_protocol::models::ImageDetail;
|
||||
@@ -15,16 +16,21 @@ fn empty_tools() -> Arc<RawValue> {
|
||||
Arc::from(RawValue::from_string("[]".to_string()).expect("valid tool JSON"))
|
||||
}
|
||||
|
||||
fn prompt_with_image_outputs() -> Prompt {
|
||||
fn prompt_with_image_outputs(detail: Option<ImageDetail>) -> Prompt {
|
||||
Prompt {
|
||||
input: vec![
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentItem::InputImage {
|
||||
image_url: "https://example.com/image.png".to_string(),
|
||||
detail: Some(ImageDetail::Original),
|
||||
}],
|
||||
content: vec![
|
||||
ContentItem::InputText {
|
||||
text: "Describe this image.".to_string(),
|
||||
},
|
||||
ContentItem::InputImage {
|
||||
image_url: "https://example.com/image.png".to_string(),
|
||||
detail,
|
||||
},
|
||||
],
|
||||
phase: None,
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
},
|
||||
@@ -36,7 +42,7 @@ fn prompt_with_image_outputs() -> Prompt {
|
||||
output: FunctionCallOutputPayload::from_content_items(vec![
|
||||
FunctionCallOutputContentItem::InputImage {
|
||||
image_url: "data:image/png;base64,function".to_string(),
|
||||
detail: Some(ImageDetail::High),
|
||||
detail,
|
||||
},
|
||||
]),
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
@@ -48,7 +54,7 @@ fn prompt_with_image_outputs() -> Prompt {
|
||||
output: FunctionCallOutputPayload::from_content_items(vec![
|
||||
FunctionCallOutputContentItem::InputImage {
|
||||
image_url: "data:image/png;base64,custom".to_string(),
|
||||
detail: Some(ImageDetail::Auto),
|
||||
detail,
|
||||
},
|
||||
]),
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
@@ -58,58 +64,40 @@ fn prompt_with_image_outputs() -> Prompt {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn responses_lite_request_copies_strip_image_details() {
|
||||
let prompt = prompt_with_image_outputs();
|
||||
#[test_case::test_case(Some(ImageDetail::Original), Some(ImageDetail::High); "original")]
|
||||
#[test_case::test_case(Some(ImageDetail::High), Some(ImageDetail::High); "high")]
|
||||
#[test_case::test_case(Some(ImageDetail::Auto), Some(ImageDetail::Auto); "auto")]
|
||||
#[test_case::test_case(Some(ImageDetail::Low), Some(ImageDetail::Low); "low")]
|
||||
#[test_case::test_case(None, None; "unspecified")]
|
||||
fn request_copies_project_image_details_for_receiving_model(
|
||||
detail: Option<ImageDetail>,
|
||||
unsupported_detail: Option<ImageDetail>,
|
||||
) {
|
||||
let prompt = prompt_with_image_outputs(detail);
|
||||
let original = prompt.input.clone();
|
||||
|
||||
let stripped = prompt.get_formatted_input_for_request(/*use_responses_lite*/ true);
|
||||
|
||||
let mut model_info = model_info_from_slug("gpt-5.4");
|
||||
model_info.use_responses_lite = false;
|
||||
model_info.supports_image_detail_original = true;
|
||||
assert_eq!(
|
||||
stripped,
|
||||
vec![
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentItem::InputImage {
|
||||
image_url: "https://example.com/image.png".to_string(),
|
||||
detail: None,
|
||||
}],
|
||||
phase: None,
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
},
|
||||
ResponseItem::FunctionCallOutput {
|
||||
id: None,
|
||||
call_id: Some("function-call".to_string()),
|
||||
name: None,
|
||||
namespace: None,
|
||||
output: FunctionCallOutputPayload::from_content_items(vec![
|
||||
FunctionCallOutputContentItem::InputImage {
|
||||
image_url: "data:image/png;base64,function".to_string(),
|
||||
detail: None,
|
||||
},
|
||||
]),
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
},
|
||||
ResponseItem::CustomToolCallOutput {
|
||||
id: None,
|
||||
call_id: "custom-call".to_string(),
|
||||
name: None,
|
||||
output: FunctionCallOutputPayload::from_content_items(vec![
|
||||
FunctionCallOutputContentItem::InputImage {
|
||||
image_url: "data:image/png;base64,custom".to_string(),
|
||||
detail: None,
|
||||
},
|
||||
]),
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
},
|
||||
]
|
||||
);
|
||||
assert_eq!(prompt.input, original);
|
||||
assert_eq!(
|
||||
prompt.get_formatted_input_for_request(/*use_responses_lite*/ false),
|
||||
prompt.get_formatted_input_for_request(&model_info),
|
||||
original
|
||||
);
|
||||
|
||||
model_info.supports_image_detail_original = false;
|
||||
assert_eq!(
|
||||
prompt.get_formatted_input_for_request(&model_info),
|
||||
prompt_with_image_outputs(unsupported_detail).input
|
||||
);
|
||||
|
||||
model_info.use_responses_lite = true;
|
||||
for supports_original in [false, true] {
|
||||
model_info.supports_image_detail_original = supports_original;
|
||||
assert_eq!(
|
||||
prompt.get_formatted_input_for_request(&model_info),
|
||||
prompt_with_image_outputs(/*detail*/ None).input
|
||||
);
|
||||
}
|
||||
assert_eq!(prompt.input, original);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -38,6 +38,7 @@ use codex_protocol::openai_models::ApprovalMessages;
|
||||
use codex_protocol::openai_models::CollaborationModeMessages;
|
||||
use codex_protocol::openai_models::ConfigShellToolType;
|
||||
use codex_protocol::openai_models::ConfirmationPolicies;
|
||||
use codex_protocol::openai_models::InputModality;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_protocol::openai_models::ModelInstructionsVariables;
|
||||
use codex_protocol::openai_models::ModelTokenBudgetConfig;
|
||||
@@ -299,6 +300,11 @@ async fn tool_result_history_keeps_originating_model_across_switch_and_replay()
|
||||
TruncationPolicyConfig::tokens(if model.slug == MODEL_B { 400 } else { 100 });
|
||||
model.supports_image_detail_original = model.slug == MODEL_B;
|
||||
model.use_responses_lite = false;
|
||||
model.input_modalities = if model.slug == MODEL_C {
|
||||
vec![InputModality::Text]
|
||||
} else {
|
||||
vec![InputModality::Text, InputModality::Image]
|
||||
};
|
||||
}
|
||||
config
|
||||
.features
|
||||
@@ -333,7 +339,12 @@ async fn tool_result_history_keeps_originating_model_across_switch_and_replay()
|
||||
ev_function_call("call-b", "diagnostics", "{}"),
|
||||
ev_completed("resp-b"),
|
||||
]),
|
||||
sse_completed("resp-done"),
|
||||
paused_response("resp-before-a", "pause-before-a"),
|
||||
sse_completed("resp-a-again"),
|
||||
sse_completed("resp-next-turn-b"),
|
||||
sse_completed("resp-next-turn-a"),
|
||||
sse_completed("resp-text-only"),
|
||||
sse_completed("resp-images-again"),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
@@ -341,13 +352,14 @@ async fn tool_result_history_keeps_originating_model_across_switch_and_replay()
|
||||
image::DynamicImage::new_rgba8(/*w*/ 2048, /*h*/ 2048)
|
||||
.write_to(&mut png, image::ImageFormat::Png)?;
|
||||
let text = "diagnostic line\n".repeat(500);
|
||||
let content_items = vec![
|
||||
DynamicToolCallOutputContentItem::InputText { text: text.clone() },
|
||||
DynamicToolCallOutputContentItem::InputImage {
|
||||
image_url: data_url_from_bytes("image/png", &png.into_inner()),
|
||||
},
|
||||
];
|
||||
let response = DynamicToolResponse {
|
||||
content_items: vec![
|
||||
DynamicToolCallOutputContentItem::InputText { text: text.clone() },
|
||||
DynamicToolCallOutputContentItem::InputImage {
|
||||
image_url: data_url_from_bytes("image/png", &png.into_inner()),
|
||||
},
|
||||
],
|
||||
content_items,
|
||||
success: true,
|
||||
};
|
||||
test.codex
|
||||
@@ -397,8 +409,9 @@ async fn tool_result_history_keeps_originating_model_across_switch_and_replay()
|
||||
.await,
|
||||
);
|
||||
}
|
||||
wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::TurnComplete(_))
|
||||
let paused_request = wait_for_event_match(&test.codex, |event| match event {
|
||||
EventMsg::RequestUserInput(request) => Some(request.clone()),
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
|
||||
@@ -439,31 +452,104 @@ async fn tool_result_history_keeps_originating_model_across_switch_and_replay()
|
||||
requests[2].function_call_output("call-a")
|
||||
);
|
||||
|
||||
apply_turn_settings(
|
||||
&test.codex,
|
||||
&paused_request.turn_id,
|
||||
TurnSettingsUpdate {
|
||||
model: Some(MODEL_A.to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
answer_paused_turn(&test.codex, &paused_request.turn_id).await?;
|
||||
wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::TurnComplete(_))
|
||||
})
|
||||
.await;
|
||||
|
||||
let outputs = ["call-a", "call-b"].map(|call_id| requests[2].function_call_output(call_id));
|
||||
assert_eq!(outputs[1]["output"][1]["detail"], "original");
|
||||
|
||||
// Project for each receiving model without rewriting the prepared images.
|
||||
for model in [MODEL_B, MODEL_A, MODEL_C, MODEL_B] {
|
||||
test.codex
|
||||
.submit(Op::ThreadSettings {
|
||||
thread_settings: ThreadSettingsOverrides {
|
||||
model: Some(model.to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
test.submit_text_turn("review previous diagnostics").await?;
|
||||
}
|
||||
let requests = responses.requests();
|
||||
let mut outputs_for_a = outputs.clone();
|
||||
outputs_for_a[1]["output"][1]["detail"] = json!("high");
|
||||
for (index, call_id) in ["call-a", "call-b"].iter().enumerate() {
|
||||
for request_index in [3, 5] {
|
||||
assert_eq!(
|
||||
requests[request_index].function_call_output(call_id),
|
||||
outputs_for_a[index]
|
||||
);
|
||||
}
|
||||
for request_index in [4, 7] {
|
||||
assert_eq!(
|
||||
requests[request_index].function_call_output(call_id),
|
||||
outputs[index]
|
||||
);
|
||||
}
|
||||
let text_output = requests[6].function_call_output(call_id);
|
||||
assert_eq!(
|
||||
text_output["output"][1],
|
||||
json!({"type": "input_text", "text": "image content omitted because you do not support image input"})
|
||||
);
|
||||
}
|
||||
assert_eq!(
|
||||
requests[3..]
|
||||
.iter()
|
||||
.map(|request| request.body_json()["model"].clone())
|
||||
.collect::<Vec<_>>(),
|
||||
vec![
|
||||
json!(MODEL_A),
|
||||
json!(MODEL_B),
|
||||
json!(MODEL_A),
|
||||
json!(MODEL_C),
|
||||
json!(MODEL_B)
|
||||
]
|
||||
);
|
||||
assert_eq!(request_turn_id(&requests[3]), turn_id);
|
||||
assert_ne!(request_turn_id(&requests[5]), request_turn_id(&requests[4]));
|
||||
|
||||
// Persistence and raw notifications retain the prepared, untruncated payload in append order.
|
||||
test.codex.shutdown_and_wait().await?;
|
||||
let rollout_path = test.codex.rollout_path().expect("rollout path");
|
||||
let history = codex_rollout::RolloutRecorder::get_rollout_history(&rollout_path).await?;
|
||||
let saved_outputs = history
|
||||
.get_rollout_items()
|
||||
.iter()
|
||||
.filter_map(|item| match item {
|
||||
RolloutItem::ResponseItem(envelope)
|
||||
if matches!(&envelope.item, ResponseItem::FunctionCallOutput { .. }) =>
|
||||
{
|
||||
Some((
|
||||
serde_json::to_value(&envelope.item).expect("saved output"),
|
||||
envelope
|
||||
.metadata
|
||||
.as_ref()
|
||||
.and_then(|metadata| metadata.history_truncation_token_limit),
|
||||
))
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
async fn saved_outputs(path: &std::path::Path) -> Result<Vec<(Value, Option<usize>)>> {
|
||||
let history = codex_rollout::RolloutRecorder::get_rollout_history(path).await?;
|
||||
Ok(history
|
||||
.get_rollout_items()
|
||||
.iter()
|
||||
.filter_map(|item| match item {
|
||||
RolloutItem::ResponseItem(envelope)
|
||||
if matches!(&envelope.item, ResponseItem::FunctionCallOutput { call_id, .. }
|
||||
if matches!(call_id.as_deref(), Some("call-a") | Some("call-b"))) =>
|
||||
{
|
||||
Some((
|
||||
serde_json::to_value(&envelope.item).expect("saved output"),
|
||||
envelope
|
||||
.metadata
|
||||
.as_ref()
|
||||
.and_then(|metadata| metadata.history_truncation_token_limit),
|
||||
))
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.collect())
|
||||
}
|
||||
let recorded_outputs = saved_outputs(&rollout_path).await?;
|
||||
assert_eq!(
|
||||
saved_outputs,
|
||||
recorded_outputs,
|
||||
raw_outputs
|
||||
.clone()
|
||||
.into_iter()
|
||||
.zip([Some(120), Some(480)])
|
||||
.collect::<Vec<_>>()
|
||||
@@ -496,6 +582,14 @@ async fn tool_result_history_keeps_originating_model_across_switch_and_replay()
|
||||
.thread;
|
||||
for thread in [resumed, forked] {
|
||||
let replay = mount_sse_once(&server, sse_completed("resp-replay")).await;
|
||||
thread
|
||||
.submit(Op::ThreadSettings {
|
||||
thread_settings: ThreadSettingsOverrides {
|
||||
model: Some(model.to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
thread
|
||||
.start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text {
|
||||
text: "review saved diagnostics".to_string(),
|
||||
@@ -505,13 +599,19 @@ async fn tool_result_history_keeps_originating_model_across_switch_and_replay()
|
||||
wait_for_event(&thread, |event| matches!(event, EventMsg::TurnComplete(_))).await;
|
||||
let request = replay.single_request();
|
||||
assert_eq!(request.body_json()["model"], model);
|
||||
for call_id in ["call-a", "call-b"] {
|
||||
assert_eq!(
|
||||
request.function_call_output(call_id),
|
||||
requests[2].function_call_output(call_id)
|
||||
);
|
||||
for (index, call_id) in ["call-a", "call-b"].iter().enumerate() {
|
||||
let expected = if model == MODEL_A {
|
||||
&outputs_for_a[index]
|
||||
} else {
|
||||
&outputs[index]
|
||||
};
|
||||
assert_eq!(request.function_call_output(call_id), *expected);
|
||||
}
|
||||
thread.shutdown_and_wait().await?;
|
||||
assert_eq!(
|
||||
saved_outputs(&thread.rollout_path().expect("replayed rollout")).await?,
|
||||
recorded_outputs
|
||||
);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
|
||||
Reference in New Issue
Block a user