diff --git a/src/providers/codex/translate/request.rs b/src/providers/codex/translate/request.rs index 97fe55b5..ab6226ec 100644 --- a/src/providers/codex/translate/request.rs +++ b/src/providers/codex/translate/request.rs @@ -308,6 +308,14 @@ fn reasoning_summary_requested(summary: Option<&str>) -> bool { !matches!(summary, Some("off" | "none")) } +/// Whether the resolved effort asks the upstream for reasoning output. +/// `Effort::None` still names the effort so the wire request overrides the +/// upstream default, but it must not request a summary or encrypted +/// continuation content, which are reasoning artifacts. +fn reasoning_requested(effort: Option<&Effort>) -> bool { + effort.is_some_and(|effort| *effort != Effort::None) +} + // --------------------------------------------------------------------------- // Compaction fast path // --------------------------------------------------------------------------- @@ -352,10 +360,24 @@ pub(crate) fn is_compact_messages_request(request: &MessagesRequest) -> bool { /// native Claude Code compacts without extended thinking, so burning /// medium/high reasoning on a 200k-token summary only adds latency. The cap /// never raises effort — a request already below it is left alone. +/// +/// A request naming no effort at all also takes the cap. Left unset it would +/// run at the upstream default, which is the effort level the cap exists to +/// avoid. fn compact_effort_cap() -> Option { compact_effort_cap_from(std::env::var("CCP_COMPACT_EFFORT").ok().as_deref()) } +/// Applies the cap to a request's resolved effort. A missing effort takes +/// the cap; an explicit effort at or below it is preserved. +fn apply_compact_effort_cap(resolved: Option, cap: Option) -> Option { + match (resolved, cap) { + (resolved, None) => resolved, + (Some(effort), Some(cap)) if effort <= cap => Some(effort), + (_, Some(cap)) => Some(cap), + } +} + fn compact_effort_cap_from(raw: Option<&str>) -> Option { match raw { None | Some("") => Some(Effort::Low), @@ -559,15 +581,12 @@ fn translate_request_inner( } else { codex_effort }; - if apply_codex_config - && is_compact - && let Some(cap) = compact_effort_cap() - && resolved_effort.as_ref().is_some_and(|e| *e > cap) - { - resolved_effort = Some(cap); + if apply_codex_config && is_compact { + resolved_effort = apply_compact_effort_cap(resolved_effort, compact_effort_cap()); } + let wants_reasoning = reasoning_requested(resolved_effort.as_ref()); if resolved_effort.is_some() || opts.use_responses_lite { - let summary = if resolved_effort.is_some() + let summary = if wants_reasoning && (!apply_codex_config || reasoning_summary_requested(config::codex_reasoning_summary().as_deref())) { @@ -581,7 +600,7 @@ fn translate_request_inner( context: opts.use_responses_lite.then_some("all_turns".to_string()), }); } - if resolved_effort.is_some() { + if wants_reasoning { out.include = Some(vec!["reasoning.encrypted_content".to_string()]); } @@ -1892,6 +1911,54 @@ mod tests { )); } + #[test] + fn compact_effort_cap_defaults_a_missing_effort() { + // No effort named: take the cap instead of the upstream default. + assert!(matches!( + apply_compact_effort_cap(None, Some(Effort::Low)), + Some(Effort::Low) + )); + assert!(matches!( + apply_compact_effort_cap(None, Some(Effort::None)), + Some(Effort::None) + )); + // An explicit effort at or below the cap survives. + assert!(matches!( + apply_compact_effort_cap(Some(Effort::None), Some(Effort::Low)), + Some(Effort::None) + )); + assert!(matches!( + apply_compact_effort_cap(Some(Effort::Low), Some(Effort::Low)), + Some(Effort::Low) + )); + // Above the cap is lowered. + assert!(matches!( + apply_compact_effort_cap(Some(Effort::High), Some(Effort::Low)), + Some(Effort::Low) + )); + // Cap disabled: the request is left exactly as it asked. + assert!(apply_compact_effort_cap(None, None).is_none()); + assert!(matches!( + apply_compact_effort_cap(Some(Effort::High), None), + Some(Effort::High) + )); + } + + #[test] + fn only_a_non_none_effort_requests_reasoning_artifacts() { + assert!(!reasoning_requested(None)); + assert!(!reasoning_requested(Some(&Effort::None))); + for effort in [ + Effort::Low, + Effort::Medium, + Effort::High, + Effort::Xhigh, + Effort::Max, + ] { + assert!(reasoning_requested(Some(&effort))); + } + } + #[test] fn compact_request_downgrades_effort_to_cap() { let req: MessagesRequest = serde_json::from_value(json!({ diff --git a/tests/codex_compact_effort.rs b/tests/codex_compact_effort.rs new file mode 100644 index 00000000..2a7439d3 --- /dev/null +++ b/tests/codex_compact_effort.rs @@ -0,0 +1,187 @@ +//! Full-translation regression coverage for the compaction effort cap. +//! +//! These tests exercise the serialized Codex request body, not just the +//! helper value table, because the cap interacts with effort overrides and +//! with which reasoning artifacts get requested. Every case mutates process +//! environment variables, so it holds the shared `ENV_LOCK` and restores the +//! previous environment through `EnvGuard`. The test binary runs in its own +//! process, keeping this environment manipulation away from the unit tests. + +use std::ffi::{OsStr, OsString}; +use std::path::Path; +use std::sync::{Mutex, OnceLock}; + +use claude_code_proxy::MessagesRequest; +use claude_code_proxy::providers::codex::translate::request::{ + TranslateOptions, translate_request, +}; +use serde_json::{Value, json}; + +static ENV_LOCK: OnceLock> = OnceLock::new(); + +fn env_lock() -> std::sync::MutexGuard<'static, ()> { + ENV_LOCK + .get_or_init(|| Mutex::new(())) + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) +} + +struct EnvGuard { + key: &'static str, + previous: Option, +} + +impl EnvGuard { + fn set(key: &'static str, value: impl AsRef) -> Self { + let previous = std::env::var_os(key); + unsafe { + std::env::set_var(key, value); + } + Self { key, previous } + } + + fn unset(key: &'static str) -> Self { + let previous = std::env::var_os(key); + unsafe { + std::env::remove_var(key); + } + Self { key, previous } + } +} + +impl Drop for EnvGuard { + fn drop(&mut self) { + unsafe { + match self.previous.take() { + Some(value) => std::env::set_var(self.key, value), + None => std::env::remove_var(self.key), + } + } + } +} + +/// Clears the environment knobs that feed effort resolution and reasoning +/// summary selection, then points the config dir at an empty directory so a +/// real user config cannot leak into the test. +fn isolated_environment(config_dir: &Path) -> Vec { + let mut guards = vec![ + EnvGuard::unset("CCP_CODEX_EFFORT"), + EnvGuard::unset("CCP_CODEX_REASONING_SUMMARY"), + EnvGuard::unset("CCP_COMPACT_EFFORT"), + ]; + guards.push(EnvGuard::set("CCP_CONFIG_DIR", config_dir)); + guards +} + +fn compact_request() -> MessagesRequest { + serde_json::from_value(json!({ + "model": "gpt-5.5", + "messages": [{"role": "user", "content": "summarize"}], + "system": "You are a helpful AI assistant tasked with summarizing conversations." + })) + .unwrap() +} + +fn translate_compact() -> Value { + let opts = TranslateOptions { + session_id: None, + service_tier: None, + model: "gpt-5.5".to_string(), + use_responses_lite: false, + }; + let out = translate_request(&compact_request(), opts).unwrap(); + serde_json::to_value(out).unwrap() +} + +#[test] +fn omitted_effort_compact_uses_default_low_cap() { + let _guard = env_lock(); + let config = tempfile::TempDir::new().unwrap(); + let _env = isolated_environment(config.path()); + + let wire = translate_compact(); + + assert_eq!( + wire["reasoning"], + json!({"effort": "low", "summary": "auto"}) + ); + assert_eq!(wire["include"], json!(["reasoning.encrypted_content"])); +} + +#[test] +fn compact_cap_none_overrides_default_without_reasoning_artifacts() { + let _guard = env_lock(); + let config = tempfile::TempDir::new().unwrap(); + let mut env = isolated_environment(config.path()); + env.push(EnvGuard::set("CCP_COMPACT_EFFORT", "none")); + + let wire = translate_compact(); + + // `none` must reach the wire to displace the upstream default, but it + // must not ask for a summary or encrypted continuation content. + assert_eq!(wire["reasoning"], json!({"effort": "none"})); + assert!(wire.get("include").is_none(), "unexpected include: {wire}"); +} + +#[test] +fn compact_cap_off_leaves_omitted_effort_unset() { + let _guard = env_lock(); + let config = tempfile::TempDir::new().unwrap(); + let mut env = isolated_environment(config.path()); + env.push(EnvGuard::set("CCP_COMPACT_EFFORT", "off")); + + let wire = translate_compact(); + + assert!( + wire.get("reasoning").is_none(), + "unexpected reasoning: {wire}" + ); + assert!(wire.get("include").is_none(), "unexpected include: {wire}"); +} + +#[test] +fn global_high_is_lowered_by_default_compact_cap() { + let _guard = env_lock(); + let config = tempfile::TempDir::new().unwrap(); + let mut env = isolated_environment(config.path()); + env.push(EnvGuard::set("CCP_CODEX_EFFORT", "high")); + + let wire = translate_compact(); + + assert_eq!( + wire["reasoning"], + json!({"effort": "low", "summary": "auto"}) + ); + assert_eq!(wire["include"], json!(["reasoning.encrypted_content"])); +} + +#[test] +fn global_none_survives_compact_cap_without_reasoning_artifacts() { + let _guard = env_lock(); + let config = tempfile::TempDir::new().unwrap(); + let mut env = isolated_environment(config.path()); + env.push(EnvGuard::set("CCP_CODEX_EFFORT", "none")); + + let wire = translate_compact(); + + // `none` is at or below the cap, so it is preserved rather than raised. + assert_eq!(wire["reasoning"], json!({"effort": "none"})); + assert!(wire.get("include").is_none(), "unexpected include: {wire}"); +} + +#[test] +fn global_low_is_preserved_under_higher_compact_cap() { + let _guard = env_lock(); + let config = tempfile::TempDir::new().unwrap(); + let mut env = isolated_environment(config.path()); + env.push(EnvGuard::set("CCP_CODEX_EFFORT", "low")); + env.push(EnvGuard::set("CCP_COMPACT_EFFORT", "medium")); + + let wire = translate_compact(); + + assert_eq!( + wire["reasoning"], + json!({"effort": "low", "summary": "auto"}) + ); + assert_eq!(wire["include"], json!(["reasoning.encrypted_content"])); +}