From 99e1d33bd197d4ce6e11ed397be1c2d690e40054 Mon Sep 17 00:00:00 2001 From: jimmyfraiture2 Date: Mon, 15 Sep 2025 00:25:43 +0100 Subject: [PATCH] feat: update model save (#3589) Edit model save to save by default as global or on the profile depending on the session --- codex-rs/tui/src/app.rs | 153 +++++++-------------------------- codex-rs/tui/src/app_event.rs | 6 ++ codex-rs/tui/src/chatwidget.rs | 6 +- 3 files changed, 44 insertions(+), 121 deletions(-) diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index d4a24d69..72865991 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -42,8 +42,6 @@ pub(crate) struct App { /// Config is stored here so we can recreate ChatWidgets as needed. pub(crate) config: Config, pub(crate) active_profile: Option, - model_saved_to_profile: bool, - model_saved_to_global: bool, pub(crate) file_search: FileSearchManager, @@ -128,8 +126,6 @@ impl App { chat_widget, config, active_profile, - model_saved_to_profile: false, - model_saved_to_global: false, file_search, enhanced_keys_supported, transcript_cells: Vec::new(), @@ -304,9 +300,38 @@ impl App { if let Some(family) = find_family_for_model(&model) { self.config.model_family = family; } - self.model_saved_to_profile = false; - self.model_saved_to_global = false; - self.show_model_save_hint(); + } + AppEvent::PersistModelSelection { model, effort } => { + let profile = self.active_profile.as_deref(); + match persist_model_selection(&self.config.codex_home, profile, &model, effort) + .await + { + Ok(()) => { + if let Some(profile) = profile { + self.chat_widget.add_info_message( + format!("Model changed to {model} for {profile} profile"), + None, + ); + } else { + self.chat_widget + .add_info_message(format!("Model changed to {model}"), None); + } + } + Err(err) => { + tracing::error!( + error = %err, + "failed to persist model selection" + ); + if let Some(profile) = profile { + self.chat_widget.add_error_message(format!( + "Failed to save model for profile `{profile}`: {err}" + )); + } else { + self.chat_widget + .add_error_message(format!("Failed to save default model: {err}")); + } + } + } } AppEvent::UpdateAskForApprovalPolicy(policy) => { self.chat_widget.set_approval_policy(policy); @@ -322,107 +347,9 @@ impl App { self.chat_widget.token_usage() } - fn show_model_save_hint(&mut self) { - let model = self.config.model.clone(); - if self.active_profile.is_some() { - self.chat_widget.add_info_message( - format!("Model changed to {model} for the current session"), - Some("(ctrl+s to set as profile default)".to_string()), - ); - } else { - self.chat_widget.add_info_message( - format!("Model changed to {model} for the current session"), - Some("(ctrl+s to set as default)".to_string()), - ); - } - } - fn on_update_reasoning_effort(&mut self, effort: Option) { - let changed = self.config.model_reasoning_effort != effort; self.chat_widget.set_reasoning_effort(effort); self.config.model_reasoning_effort = effort; - if changed { - let show_hint = self.model_saved_to_profile || self.model_saved_to_global; - self.model_saved_to_profile = false; - self.model_saved_to_global = false; - if show_hint { - self.show_model_save_hint(); - } - } - } - - async fn persist_model_shortcut(&mut self) { - enum SaveScope<'a> { - Profile(&'a str), - Global, - AlreadySaved, - } - - let scope = if let Some(profile) = self - .active_profile - .as_deref() - .filter(|_| !self.model_saved_to_profile) - { - SaveScope::Profile(profile) - } else if !self.model_saved_to_global { - SaveScope::Global - } else { - SaveScope::AlreadySaved - }; - - let model = self.config.model.clone(); - let effort = self.config.model_reasoning_effort; - let codex_home = self.config.codex_home.clone(); - - match scope { - SaveScope::Profile(profile) => { - match persist_model_selection(&codex_home, Some(profile), &model, effort).await { - Ok(()) => { - self.model_saved_to_profile = true; - self.chat_widget.add_info_message( - format!("Profile model changed to {model} for all sessions"), - Some("(view global config in config.toml)".to_string()), - ); - } - Err(err) => { - tracing::error!( - error = %err, - "failed to persist model selection via shortcut" - ); - self.chat_widget.add_error_message(format!( - "Failed to save model preference for profile `{profile}`: {err}" - )); - } - } - } - SaveScope::Global => { - match persist_model_selection(&codex_home, None, &model, effort).await { - Ok(()) => { - self.model_saved_to_global = true; - self.chat_widget.add_info_message( - format!("Default model changed to {model} for all sessions"), - Some("(view global config in config.toml)".to_string()), - ) - } - Err(err) => { - tracing::error!( - error = %err, - "failed to persist global model selection via shortcut" - ); - self.chat_widget.add_error_message(format!( - "Failed to save global model preference: {err}" - )); - } - } - } - SaveScope::AlreadySaved => { - self.chat_widget.add_info_message( - "Model preference already saved globally; no further action needed." - .to_string(), - None, - ); - } - } } async fn handle_key_event(&mut self, tui: &mut tui::Tui, key_event: KeyEvent) { @@ -438,14 +365,6 @@ impl App { self.overlay = Some(Overlay::new_transcript(self.transcript_cells.clone())); tui.frame_requester().schedule_frame(); } - KeyEvent { - code: KeyCode::Char('s'), - modifiers: crossterm::event::KeyModifiers::CONTROL, - kind: KeyEventKind::Press, - .. - } => { - self.persist_model_shortcut().await; - } // Esc primes/advances backtracking only in normal (not working) mode // with an empty composer. In any other state, forward Esc so the // active UI (e.g. status indicator, modals, popups) handles it. @@ -519,8 +438,6 @@ mod tests { chat_widget, config, active_profile: None, - model_saved_to_profile: false, - model_saved_to_global: false, file_search, transcript_cells: Vec::new(), overlay: None, @@ -533,10 +450,8 @@ mod tests { } #[test] - fn update_reasoning_effort_updates_config_and_resets_flags() { + fn update_reasoning_effort_updates_config() { let mut app = make_test_app(); - app.model_saved_to_profile = true; - app.model_saved_to_global = true; app.config.model_reasoning_effort = Some(ReasoningEffortConfig::Medium); app.chat_widget .set_reasoning_effort(Some(ReasoningEffortConfig::Medium)); @@ -551,7 +466,5 @@ mod tests { app.chat_widget.config_ref().model_reasoning_effort, Some(ReasoningEffortConfig::High) ); - assert!(!app.model_saved_to_profile); - assert!(!app.model_saved_to_global); } } diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index ec067792..41992cdd 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -51,6 +51,12 @@ pub(crate) enum AppEvent { /// Update the current model slug in the running app and widget. UpdateModel(String), + /// Persist the selected model and reasoning effort to the appropriate config. + PersistModelSelection { + model: String, + effort: Option, + }, + /// Update the current approval policy in the running app and widget. UpdateAskForApprovalPolicy(AskForApproval), diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index b53f87e6..008cc777 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1192,6 +1192,10 @@ impl ChatWidget { })); tx.send(AppEvent::UpdateModel(model_slug.clone())); tx.send(AppEvent::UpdateReasoningEffort(effort)); + tx.send(AppEvent::PersistModelSelection { + model: model_slug.clone(), + effort, + }); tracing::info!( "New model: {}, New effort: {}, Current model: {}, Current effort: {}", model_slug.clone(), @@ -1215,7 +1219,7 @@ impl ChatWidget { self.bottom_pane.show_selection_view( "Select model and reasoning level".to_string(), Some("Switch between OpenAI models for this and future Codex CLI session".to_string()), - Some("Press Enter to confirm, Esc to go back, Ctrl+S to save".to_string()), + Some("Press Enter to confirm or Esc to go back".to_string()), items, ); }