From f072e6e8d0535efbbcc67d14c321f73ee2793d04 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Fri, 8 May 2026 08:26:57 +0300 Subject: [PATCH] codex: keep service tier id out of PR #20978 Remove the config-level service_tier_id plumbing from the slash-command PR so PR #20974 remains the owner of that API. Co-authored-by: Codex --- codex-rs/core/src/config/config_tests.rs | 8 ++- codex-rs/core/src/config/mod.rs | 21 ++++--- codex-rs/tui/src/app/event_dispatch.rs | 31 +++++----- codex-rs/tui/src/app_event.rs | 1 - codex-rs/tui/src/chatwidget.rs | 56 +++++++++---------- .../src/chatwidget/tests/slash_commands.rs | 11 +--- 6 files changed, 60 insertions(+), 68 deletions(-) diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 342e3b9155..bc9af28a67 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -62,6 +62,7 @@ use codex_model_provider_info::LMSTUDIO_OSS_PROVIDER_ID; use codex_model_provider_info::OLLAMA_OSS_PROVIDER_ID; use codex_model_provider_info::WireApi; use codex_models_manager::bundled_models_response; +use codex_protocol::config_types::ServiceTier; use codex_protocol::models::ActivePermissionProfile; use codex_protocol::models::ActivePermissionProfileModification; use codex_protocol::models::ManagedFileSystemPermissions; @@ -7138,7 +7139,7 @@ async fn explicit_null_service_tier_override_sets_fast_default_opt_out() -> std: } #[tokio::test] -async fn explicit_fast_service_tier_override_preserves_request_id() -> std::io::Result<()> { +async fn legacy_fast_service_tier_override_uses_priority_request_value() -> std::io::Result<()> { let fixture = create_test_fixture()?; let config = Config::load_from_base_config_with_overrides( @@ -7152,7 +7153,10 @@ async fn explicit_fast_service_tier_override_preserves_request_id() -> std::io:: ) .await?; - assert_eq!(config.service_tier, Some("fast".to_string())); + assert_eq!( + config.service_tier, + Some(ServiceTier::Fast.request_value().to_string()) + ); Ok(()) } diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 6438b2840c..b2dfbcb2b8 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -2738,15 +2738,20 @@ impl Config { notices.fast_default_opt_out = Some(true); None } - None => config_profile.service_tier.or(cfg.service_tier).and_then( - |service_tier| match service_tier { - ServiceTier::Fast => features - .enabled(Feature::FastMode) - .then(|| ServiceTier::Fast.request_value().to_string()), - ServiceTier::Flex => Some(ServiceTier::Flex.request_value().to_string()), - }, - ), + None => config_profile + .service_tier + .or(cfg.service_tier) + .map(|service_tier| service_tier.request_value().to_string()), }; + let service_tier = service_tier.and_then(|service_tier| { + match ServiceTier::from_request_value(&service_tier) { + Some(ServiceTier::Fast) => features + .enabled(Feature::FastMode) + .then(|| ServiceTier::Fast.request_value().to_string()), + Some(ServiceTier::Flex) => Some(ServiceTier::Flex.request_value().to_string()), + None => Some(service_tier), + } + }); let compact_prompt = compact_prompt.or(cfg.compact_prompt).and_then(|value| { let trimmed = value.trim(); diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index eb36c955a8..670c6f7de3 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1271,31 +1271,26 @@ impl App { } } } - AppEvent::PersistServiceTierSelection { - service_tier, - service_tier_id, - } => { + AppEvent::PersistServiceTierSelection { service_tier } => { self.refresh_status_line(); let profile = self.active_profile.as_deref(); - self.config.service_tier = service_tier_id.clone().or_else(|| { - service_tier.map(|service_tier| service_tier.request_value().to_string()) - }); - let mut edits = - ConfigEditsBuilder::new(&self.config.codex_home).with_profile(profile); - if let Some(service_tier) = service_tier { - edits = edits.set_service_tier(Some(service_tier)); - } else if service_tier_id.is_none() { - edits = edits.set_service_tier(/*service_tier*/ None); + self.config.service_tier = + service_tier.map(|service_tier| service_tier.request_value().to_string()); + let mut edits = ConfigEditsBuilder::new(&self.config.codex_home) + .with_profile(profile) + .set_service_tier(service_tier); + if service_tier.is_none() { self.config.notices.fast_default_opt_out = Some(true); edits = edits.set_fast_default_opt_out(/*opted_out*/ true); } match edits.apply().await { Ok(()) => { - let mut message = if let Some(service_tier_id) = &self.config.service_tier { - format!("Service tier set to {service_tier_id}") - } else { - "Service tier cleared".to_string() - }; + let mut message = + if let Some(service_tier_value) = &self.config.service_tier { + format!("Service tier set to {service_tier_value}") + } else { + "Service tier cleared".to_string() + }; if let Some(profile) = profile { message.push_str(" for "); message.push_str(profile); diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index ee7eb113d9..4ee405f495 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -570,7 +570,6 @@ pub(crate) enum AppEvent { /// Persist the selected service tier to the appropriate config. PersistServiceTierSelection { service_tier: Option, - service_tier_id: Option, }, /// Open the device picker for a realtime microphone or speaker. diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index ae8ce1fff3..aaecf47dae 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1665,6 +1665,11 @@ fn token_usage_info_from_app_server(token_usage: ThreadTokenUsage) -> TokenUsage } } +fn service_tier_from_value(value: &str) -> Option { + ServiceTier::from_request_value(value) + .or_else(|| (value == SPEED_TIER_FAST).then_some(ServiceTier::Fast)) +} + impl ChatWidget { /// Stores or overwrites the cached nickname and role for a collab agent thread. /// @@ -9304,14 +9309,6 @@ impl ChatWidget { self.effective_service_tier = service_tier; } - pub(crate) fn set_service_tier_id(&mut self, service_tier_id: Option) { - self.effective_service_tier = service_tier_id.as_deref().and_then(|service_tier_id| { - ServiceTier::from_request_value(service_tier_id) - .or_else(|| (service_tier_id == SPEED_TIER_FAST).then_some(ServiceTier::Fast)) - }); - self.config.service_tier = service_tier_id; - } - pub(crate) fn current_service_tier(&self) -> Option { self.effective_service_tier } @@ -9320,10 +9317,7 @@ impl ChatWidget { self.config .service_tier .as_deref() - .and_then(|service_tier_id| { - ServiceTier::from_request_value(service_tier_id) - .or_else(|| (service_tier_id == SPEED_TIER_FAST).then_some(ServiceTier::Fast)) - }) + .and_then(service_tier_from_value) } pub(crate) fn fast_default_opt_out(&self) -> Option { @@ -9418,14 +9412,17 @@ impl ChatWidget { fn set_service_tier_selection( &mut self, service_tier: Option, - service_tier_id: Option, + service_tier_value: Option, ) { - if service_tier_id.is_none() && service_tier.is_none() { + if service_tier_value.is_none() && service_tier.is_none() { self.config.notices.fast_default_opt_out = Some(true); } - let next_service_tier_id = service_tier_id + let next_service_tier_value = service_tier_value .or_else(|| service_tier.map(|service_tier| service_tier.request_value().to_string())); - self.set_service_tier_id(next_service_tier_id.clone()); + self.effective_service_tier = next_service_tier_value + .as_deref() + .and_then(service_tier_from_value); + self.config.service_tier = next_service_tier_value.clone(); self.app_event_tx .send(AppEvent::CodexOp(AppCommand::override_turn_context( /*cwd*/ None, @@ -9436,37 +9433,36 @@ impl ChatWidget { /*model*/ None, /*effort*/ None, /*summary*/ None, - Some(next_service_tier_id.clone()), + Some(next_service_tier_value.clone()), /*collaboration_mode*/ None, /*personality*/ None, ))); - self.app_event_tx - .send(AppEvent::PersistServiceTierSelection { - service_tier, - service_tier_id: next_service_tier_id, - }); + if service_tier.is_some() || next_service_tier_value.is_none() { + self.app_event_tx + .send(AppEvent::PersistServiceTierSelection { service_tier }); + } } fn handle_service_tier_slash_command(&mut self, command: ServiceTierCommand) { - let active_service_tier_id_matches_command = self + let active_service_tier_matches_command = self .config .service_tier .as_deref() - .is_some_and(|service_tier_id| service_tier_id == command.id) + .is_some_and(|service_tier_value| service_tier_value == command.id) || self.effective_service_tier.is_some_and(|service_tier| { - ServiceTier::from_request_value(&command.id) == Some(service_tier) + service_tier_from_value(&command.id) == Some(service_tier) || (matches!(service_tier, ServiceTier::Fast) && command.id == SPEED_TIER_FAST) }); - let next_service_tier_id = if active_service_tier_id_matches_command { + let next_service_tier_value = if active_service_tier_matches_command { None } else { Some(command.id) }; self.set_service_tier_selection( - next_service_tier_id + next_service_tier_value .as_deref() - .and_then(ServiceTier::from_request_value), - next_service_tier_id, + .and_then(service_tier_from_value), + next_service_tier_value, ); self.bottom_pane.record_pending_slash_command_history(); } @@ -9477,7 +9473,7 @@ impl ChatWidget { } else { Some(ServiceTier::Fast) }; - self.set_service_tier_selection(next_tier, /*service_tier_id*/ None); + self.set_service_tier_selection(next_tier, /*service_tier_value*/ None); } pub(crate) fn current_model(&self) -> &str { diff --git a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs index e9cfec84b0..7166f492da 100644 --- a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs +++ b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs @@ -1810,7 +1810,7 @@ async fn slash_rollout_handles_missing_path() { } #[tokio::test] -async fn service_tier_slash_command_updates_and_persists_local_service_tier_id() { +async fn service_tier_slash_command_updates_and_persists_local_service_tier() { let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await; set_fast_mode_test_catalog(&mut chat); chat.set_feature_enabled(Feature::FastMode, /*enabled*/ true); @@ -1833,9 +1833,7 @@ async fn service_tier_slash_command_updates_and_persists_local_service_tier_id() event, AppEvent::PersistServiceTierSelection { service_tier: Some(ServiceTier::Fast), - service_tier_id: Some(service_tier_id), } - if service_tier_id == SPEED_TIER_FAST )), "expected service-tier persistence app event; events: {events:?}" ); @@ -1867,9 +1865,7 @@ async fn fast_keybinding_toggle_uses_same_events_as_fast_slash_command() { event, AppEvent::PersistServiceTierSelection { service_tier: Some(ServiceTier::Fast), - service_tier_id: Some(service_tier_id), } - if service_tier_id == ServiceTier::Fast.request_value() )), "expected fast-mode persistence app event; events: {events:?}" ); @@ -1984,10 +1980,7 @@ async fn user_turn_sends_standard_override_after_fast_is_turned_off() { assert!( events.iter().any(|event| matches!( event, - AppEvent::PersistServiceTierSelection { - service_tier: None, - service_tier_id: None, - } + AppEvent::PersistServiceTierSelection { service_tier: None } )), "expected fast-mode opt-out persistence app event; events: {events:?}" );