diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index d5c35e3d74..04a00215f1 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -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) -> Result> 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()) diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 786831d1c4..e3dff930d7 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -897,7 +897,7 @@ impl ModelClient { service_tier: Option, responses_metadata: &CodexResponsesMetadata, ) -> Result { - 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 diff --git a/codex-rs/core/src/client_common.rs b/codex-rs/core/src/client_common.rs index 3d5be635ef..96cd7eab01 100644 --- a/codex-rs/core/src/client_common.rs +++ b/codex-rs/core/src/client_common.rs @@ -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 { 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, 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>, /// Signals the mapper task that the consumer stopped polling before the diff --git a/codex-rs/core/src/client_common_tests.rs b/codex-rs/core/src/client_common_tests.rs index 1c8625af56..e8b030e6a1 100644 --- a/codex-rs/core/src/client_common_tests.rs +++ b/codex-rs/core/src/client_common_tests.rs @@ -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 { Arc::from(RawValue::from_string("[]".to_string()).expect("valid tool JSON")) } -fn prompt_with_image_outputs() -> Prompt { +fn prompt_with_image_outputs(detail: Option) -> 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, + unsupported_detail: Option, +) { + 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] diff --git a/codex-rs/core/tests/suite/step_settings.rs b/codex-rs/core/tests/suite/step_settings.rs index 76c6083673..2ff51519b1 100644 --- a/codex-rs/core/tests/suite/step_settings.rs +++ b/codex-rs/core/tests/suite/step_settings.rs @@ -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![ + 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::>(); + async fn saved_outputs(path: &std::path::Path) -> Result)>> { + 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::>() @@ -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(())