diff --git a/codex-rs/core/src/client_common.rs b/codex-rs/core/src/client_common.rs index dda69a7e..0d634165 100644 --- a/codex-rs/core/src/client_common.rs +++ b/codex-rs/core/src/client_common.rs @@ -41,8 +41,9 @@ impl Prompt { .unwrap_or(model.base_instructions.deref()); let mut sections: Vec<&str> = vec![base]; - // When there are no custom instructions, add apply_patch_tool_instructions if either: - // - the model needs special instructions (4.1), or + // When there are no custom instructions, add apply_patch_tool_instructions if: + // - the model needs special instructions (4.1) + // AND // - there is no apply_patch tool present let is_apply_patch_tool_present = self.tools.iter().any(|tool| match tool { OpenAiTool::Function(f) => f.name == "apply_patch", @@ -50,7 +51,8 @@ impl Prompt { _ => false, }); if self.base_instructions_override.is_none() - && (model.needs_special_apply_patch_instructions || !is_apply_patch_tool_present) + && model.needs_special_apply_patch_instructions + && !is_apply_patch_tool_present { sections.push(APPLY_PATCH_TOOL_INSTRUCTIONS); } @@ -174,22 +176,64 @@ impl Stream for ResponseStream { #[cfg(test)] mod tests { use crate::model_family::find_family_for_model; + use pretty_assertions::assert_eq; use super::*; + struct InstructionsTestCase { + pub slug: &'static str, + pub expects_apply_patch_instructions: bool, + } #[test] fn get_full_instructions_no_user_content() { let prompt = Prompt { ..Default::default() }; - let model_family = find_family_for_model("gpt-4.1").expect("known model slug"); + let test_cases = vec![ + InstructionsTestCase { + slug: "gpt-3.5", + expects_apply_patch_instructions: true, + }, + InstructionsTestCase { + slug: "gpt-4.1", + expects_apply_patch_instructions: true, + }, + InstructionsTestCase { + slug: "gpt-4o", + expects_apply_patch_instructions: true, + }, + InstructionsTestCase { + slug: "gpt-5", + expects_apply_patch_instructions: true, + }, + InstructionsTestCase { + slug: "codex-mini-latest", + expects_apply_patch_instructions: true, + }, + InstructionsTestCase { + slug: "gpt-oss:120b", + expects_apply_patch_instructions: false, + }, + InstructionsTestCase { + slug: "swiftfox", + expects_apply_patch_instructions: false, + }, + ]; + for test_case in test_cases { + let model_family = find_family_for_model(test_case.slug).expect("known model slug"); + let expected = if test_case.expects_apply_patch_instructions { + format!( + "{}\n{}", + model_family.clone().base_instructions, + APPLY_PATCH_TOOL_INSTRUCTIONS + ) + } else { + model_family.clone().base_instructions + }; - let expected = format!( - "{}\n{}", - model_family.base_instructions, APPLY_PATCH_TOOL_INSTRUCTIONS - ); - let full = prompt.get_full_instructions(&model_family); - assert_eq!(full, expected); + let full = prompt.get_full_instructions(&model_family); + assert_eq!(full, expected); + } } #[test] diff --git a/codex-rs/core/src/model_family.rs b/codex-rs/core/src/model_family.rs index 93ab947e..d3a5d005 100644 --- a/codex-rs/core/src/model_family.rs +++ b/codex-rs/core/src/model_family.rs @@ -66,23 +66,6 @@ macro_rules! model_family { }}; } -macro_rules! simple_model_family { - ( - $slug:expr, $family:expr - ) => {{ - Some(ModelFamily { - slug: $slug.to_string(), - family: $family.to_string(), - needs_special_apply_patch_instructions: false, - supports_reasoning_summaries: false, - reasoning_summary_format: ReasoningSummaryFormat::None, - uses_local_shell_tool: false, - apply_patch_tool_type: None, - base_instructions: BASE_INSTRUCTIONS.to_string(), - }) - }}; -} - /// Returns a `ModelFamily` for the given model slug, or `None` if the slug /// does not match any known model family. pub fn find_family_for_model(slug: &str) -> Option { @@ -90,17 +73,20 @@ pub fn find_family_for_model(slug: &str) -> Option { model_family!( slug, "o3", supports_reasoning_summaries: true, + needs_special_apply_patch_instructions: true, ) } else if slug.starts_with("o4-mini") { model_family!( slug, "o4-mini", supports_reasoning_summaries: true, + needs_special_apply_patch_instructions: true, ) } else if slug.starts_with("codex-mini-latest") { model_family!( slug, "codex-mini-latest", supports_reasoning_summaries: true, uses_local_shell_tool: true, + needs_special_apply_patch_instructions: true, ) } else if slug.starts_with("gpt-4.1") { model_family!( @@ -110,26 +96,21 @@ pub fn find_family_for_model(slug: &str) -> Option { } else if slug.starts_with("gpt-oss") || slug.starts_with("openai/gpt-oss") { model_family!(slug, "gpt-oss", apply_patch_tool_type: Some(ApplyPatchToolType::Function)) } else if slug.starts_with("gpt-4o") { - simple_model_family!(slug, "gpt-4o") + model_family!(slug, "gpt-4o", needs_special_apply_patch_instructions: true) } else if slug.starts_with("gpt-3.5") { - simple_model_family!(slug, "gpt-3.5") - } else if slug.starts_with("codex-") { + model_family!(slug, "gpt-3.5", needs_special_apply_patch_instructions: true) + } else if slug.starts_with("codex-") || slug.starts_with("swiftfox") { model_family!( slug, slug, supports_reasoning_summaries: true, reasoning_summary_format: ReasoningSummaryFormat::Experimental, + base_instructions: SWIFTFOX_INSTRUCTIONS.to_string(), ) } else if slug.starts_with("gpt-5") { model_family!( slug, "gpt-5", supports_reasoning_summaries: true, - ) - } else if slug.starts_with("swiftfox") { - model_family!( - slug, "swiftfox", - supports_reasoning_summaries: true, - reasoning_summary_format: ReasoningSummaryFormat::Experimental, - base_instructions: SWIFTFOX_INSTRUCTIONS.to_string(), + needs_special_apply_patch_instructions: true, ) } else { None