From fa71afc8a701a88cc8c92b94240f7a23e31b3568 Mon Sep 17 00:00:00 2001 From: Benjamin Carlsson Date: Tue, 20 Oct 2026 15:01:47 +0000 Subject: [PATCH] Rank `$` mentions by recent selection frequency (#55596) ## What changed Prefer frequently selected entries in the TUI's `$` picker within exact, prefix, and fuzzy match tiers, keeping stronger match tiers ahead of usage preferences. Track the last 50 confirmed selections and persist them in `mention_usage.json` under `CODEX_HOME` when history persistence is enabled. Preserve in-memory preferences across compatible chat widget replacements, including when history persistence is disabled. Serialize background writes, merge updates under a bounded file lock, and flush pending writes on shutdown. ## Testing Add coverage for picker ordering, preference handoff and persistence, history-disabled widget replacement, selection expiry, malformed history, and lock contention. GitOrigin-RevId: 7efa8c1c365e23000fcda10dbbc2a7e90f4e777c --- codex-rs/tui/src/app/session_lifecycle.rs | 7 +- codex-rs/tui/src/app/startup.rs | 1 + .../tui/src/app/tests/new_session_tests.rs | 67 ++++++ ...s_mention_usage_with_history_disabled.snap | 16 ++ codex-rs/tui/src/bottom_pane/chat_composer.rs | 45 ++++- codex-rs/tui/src/bottom_pane/mention_usage.rs | 191 ++++++++++++++++++ .../src/bottom_pane/mention_usage_tests.rs | 88 ++++++++ codex-rs/tui/src/bottom_pane/mod.rs | 14 ++ codex-rs/tui/src/bottom_pane/skill_popup.rs | 91 ++++++--- ...onfirmed_selections_within_match_tier.snap | 13 ++ codex-rs/tui/src/chatwidget/constructor.rs | 4 + codex-rs/tui/src/chatwidget/input_restore.rs | 5 + 12 files changed, 516 insertions(+), 26 deletions(-) create mode 100644 codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__session_lifecycle_requests__new_session_tests__widget_replacement_preserves_mention_usage_with_history_disabled.snap create mode 100644 codex-rs/tui/src/bottom_pane/mention_usage.rs create mode 100644 codex-rs/tui/src/bottom_pane/mention_usage_tests.rs create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__skill_popup_prefers_confirmed_selections_within_match_tier.snap diff --git a/codex-rs/tui/src/app/session_lifecycle.rs b/codex-rs/tui/src/app/session_lifecycle.rs index 1f1fedaeb6..811d97176f 100644 --- a/codex-rs/tui/src/app/session_lifecycle.rs +++ b/codex-rs/tui/src/app/session_lifecycle.rs @@ -480,8 +480,8 @@ impl App { Ok(live_attached) } - /// Replaces the chat widget, carrying the editor yank and re-seeding collab metadata from the - /// navigation cache. + /// Replaces the chat widget, carrying composer preferences and the editor yank, and + /// re-seeding collab metadata from the navigation cache. /// /// Thread switches reconstruct the `ChatWidget`, which loses the `collab_agent_metadata` map. /// This helper copies every known nickname/role from `AgentNavigationState` into the @@ -519,6 +519,9 @@ impl App { )); chat_widget.inherit_backend_banner_state(&mut self.chat_widget); chat_widget.inherit_security_setup(&mut self.chat_widget); + chat_widget + .mention_usage_mut() + .inherit(self.chat_widget.mention_usage_mut()); for (thread_id, entry) in self.agent_navigation.ordered_threads() { chat_widget.set_collab_agent_metadata( thread_id, diff --git a/codex-rs/tui/src/app/startup.rs b/codex-rs/tui/src/app/startup.rs index 260dc2913d..85e4dc1fff 100644 --- a/codex-rs/tui/src/app/startup.rs +++ b/codex-rs/tui/src/app/startup.rs @@ -1598,6 +1598,7 @@ See the Codex keymap documentation for supported actions and examples." } } }; + app.chat_widget.mention_usage_mut().flush().await; if let Err(err) = backends.shutdown().await { tracing::warn!(error = %err, "failed to shut down embedded app server"); } diff --git a/codex-rs/tui/src/app/tests/new_session_tests.rs b/codex-rs/tui/src/app/tests/new_session_tests.rs index 6e567ecdc5..1571bef205 100644 --- a/codex-rs/tui/src/app/tests/new_session_tests.rs +++ b/codex-rs/tui/src/app/tests/new_session_tests.rs @@ -1,6 +1,10 @@ //! Replacement starts use server defaults and preserve the current task on failure. use super::*; +use codex_app_server_protocol::SkillMetadata; +use codex_app_server_protocol::SkillScope; +use codex_app_server_protocol::SkillsListEntry; +use codex_config::types::HistoryPersistence; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyModifiers; @@ -130,6 +134,69 @@ async fn new_session_preserves_vim_line_yank() -> Result<()> { Ok(()) } +#[tokio::test] +async fn widget_replacement_preserves_mention_usage_with_history_disabled() -> Result<()> { + let (mut app, _events, _ops) = make_test_app_with_channels().await; + app.local_settings.history.persistence = HistoryPersistence::None; + let mut tui = crate::tui::test_support::make_test_tui()?; + let skills = SkillsListResponse { + data: vec![SkillsListEntry { + cwd: app.config.cwd.to_path_buf(), + skills: ["statsig", "status"] + .into_iter() + .map(|name| SkillMetadata { + name: name.to_owned(), + description: String::new(), + short_description: None, + interface: None, + dependencies: None, + path: app + .config + .cwd + .to_path_buf() + .join(name) + .join("SKILL.md") + .abs() + .into(), + scope: SkillScope::User, + enabled: true, + plugin_id: None, + }) + .collect(), + errors: Vec::new(), + }], + }; + let init = app.chatwidget_init_for_forked_or_resumed_thread( + &mut tui, + app.config.clone(), + /*initial_user_message*/ None, + ); + app.replace_chat_widget(ChatWidget::new_with_app_event(init)); + app.chat_widget.set_skills_from_response(&skills); + app.chat_widget.insert_str("$status"); + app.chat_widget.handle_key_event(KeyCode::Enter.into()); + + let init = app.chatwidget_init_for_forked_or_resumed_thread( + &mut tui, + app.config.clone(), + /*initial_user_message*/ None, + ); + app.replace_chat_widget(ChatWidget::new_with_app_event(init)); + app.chat_widget.set_skills_from_response(&skills); + app.chat_widget.insert_str("$stat"); + insta::assert_snapshot!(normalize_snapshot_paths(render_bottom_popup( + &app.chat_widget, + /*width*/ 80 + ))); + assert!( + !app.local_settings + .codex_home + .join("mention_usage.json") + .exists() + ); + Ok(()) +} + #[tokio::test] async fn replacement_uses_server_defaults_and_preserves_explicit_launch_settings() -> Result<()> { for (managed_effort, explicit, expected_model, expected_effort) in [ diff --git a/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__session_lifecycle_requests__new_session_tests__widget_replacement_preserves_mention_usage_with_history_disabled.snap b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__session_lifecycle_requests__new_session_tests__widget_replacement_preserves_mention_usage_with_history_disabled.snap new file mode 100644 index 0000000000..6a47bf2b72 --- /dev/null +++ b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__session_lifecycle_requests__new_session_tests__widget_replacement_preserves_mention_usage_with_history_disabled.snap @@ -0,0 +1,16 @@ +--- +source: tui/src/app/tests/new_session_tests.rs +expression: "normalize_snapshot_paths(render_bottom_popup(&app.chat_widget, 80))" +--- + >_ OpenAI Codex (v0.0.0) + /tmp/project + + +› status [Skill] + statsig [Skill] + + enter insert · esc close + +› $stat + + GPT-5.6-Sol default · /tmp/project diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 57d5b49ac0..320bcc7b8e 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -33,6 +33,7 @@ //! By default, `@` lists plugins, filesystem entries, and skills. Skills are hidden when their //! owning plugin is listed. `$` lists individual skills and apps, but not plugins. //! Disabling `mentions_v2` restores file-only `@` search and adds plugins back to `$`. +//! Confirmed `$` picker selections inform future ordering within each match tier. //! //! # Key Event Routing //! @@ -352,6 +353,7 @@ use super::footer::single_line_footer_layout; use super::footer::status_line_right_indicator_line; use super::footer::toggle_shortcut_mode; use super::footer::uses_passive_footer_status_layout; +use super::mention_usage::MentionUsage; use super::mentions_v2::MentionV2Popup; use super::mentions_v2::MentionV2Selection; use super::paste_burst::CharDecision; @@ -614,6 +616,7 @@ impl ChatComposerConfig { pub(crate) struct ChatComposer { draft: DraftState, popups: PopupState, + pub(super) mention_usage: MentionUsage, app_event_tx: AppEventSender, history: ChatComposerHistory, agents_navigation_enabled: bool, @@ -745,6 +748,7 @@ impl ChatComposer { let mut this = Self { draft: DraftState::new(), popups: PopupState::default(), + mention_usage: MentionUsage::default(), app_event_tx, history: ChatComposerHistory::new(), agents_navigation_enabled: false, @@ -2287,6 +2291,9 @@ impl ChatComposer { && let Some(target) = self.current_mention_target() { self.insert_selected_mention(target.range, &insert_text, path.as_deref()); + if let Some(path) = path { + self.mention_usage.record(&path); + } } self.popups.active = ActivePopup::None; } @@ -4113,7 +4120,8 @@ impl ChatComposer { popup.set_mentions(mentions); } _ => { - let mut popup = SkillPopup::new(mentions); + let mut popup = + SkillPopup::new(mentions).with_frequencies(self.mention_usage.frequencies()); popup.set_query(&query); self.popups.active = ActivePopup::Skill(popup); } @@ -7166,6 +7174,41 @@ mod tests { ); } + #[test] + fn skill_popup_prefers_confirmed_selections_within_match_tier() { + snapshot_composer_state( + "skill_popup_prefers_confirmed_selections_within_match_tier", + /*enhanced_keys_supported*/ false, + |composer| { + composer.set_skill_mentions(Some(vec![ + test_skill_metadata("statsig"), + test_skill_metadata("status"), + test_skill_metadata("hello-statsig"), + ])); + for name in ["hello-statsig", "hello-statsig", "status"] { + let text = format!("${name}"); + composer.set_text_content(text.clone(), Vec::new(), Vec::new()); + composer.set_current_cursor(text.len()); + composer.sync_popups(); + let _ = composer + .handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + } + composer.set_text_content("$stat".to_owned(), Vec::new(), Vec::new()); + composer.set_current_cursor("$stat".len()); + composer.sync_popups(); + let ActivePopup::Skill(popup) = &composer.popups.active else { + panic!("expected skill popup"); + }; + assert_eq!( + popup + .selected_mention() + .map(|item| item.insert_text.as_str()), + Some("$status") + ); + }, + ); + } + #[test] fn skill_popup_targets_unbound_mention_right_of_adjacent_bound_mention_snapshot() { snapshot_composer_state( diff --git a/codex-rs/tui/src/bottom_pane/mention_usage.rs b/codex-rs/tui/src/bottom_pane/mention_usage.rs new file mode 100644 index 0000000000..3ba298bce4 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/mention_usage.rs @@ -0,0 +1,191 @@ +//! Locally remembers a bounded window of confirmed `$` picker selections. +//! +//! The active composer updates its window immediately and carries it across compatible widget +//! replacements. Background workers serialize writes, and a bounded file-lock wait merges +//! selections made by separate TUI processes without indefinitely delaying shutdown. +//! Malformed history is treated as empty and replaced by the next persisted selection. + +use std::collections::HashMap; +use std::collections::VecDeque; +use std::fs::File; +use std::fs::OpenOptions; +use std::io; +use std::path::Path; +use std::path::PathBuf; +use std::time::Duration; +use std::time::Instant; + +use codex_config::types::HistoryPersistence; +use codex_utils_path::write_atomically; +use tokio::sync::mpsc; +use tokio::task::JoinHandle; + +const HISTORY_LIMIT: usize = 50; +const MAX_FILE_BYTES: u64 = 1024 * 1024; +const FILE_NAME: &str = "mention_usage.json"; +const LOCK_TIMEOUT: Duration = Duration::from_millis(100); +const LOCK_RETRY_INTERVAL: Duration = Duration::from_millis(10); +const FLUSH_TIMEOUT: Duration = Duration::from_secs(1); + +#[derive(PartialEq)] +struct Storage { + path: PathBuf, + persistence: HistoryPersistence, +} + +#[derive(Default)] +pub(crate) struct MentionUsage { + recent: VecDeque, + storage: Option, + writer: Option>, + workers: Vec>, +} + +impl MentionUsage { + pub(super) fn load(codex_home: &Path, persistence: HistoryPersistence) -> Self { + let path = codex_home.join(FILE_NAME); + let recent = match persistence { + HistoryPersistence::SaveAll => read_recent(&path).unwrap_or_else(|error| { + tracing::debug!(%error, "could not load mention preferences"); + VecDeque::new() + }), + HistoryPersistence::None => VecDeque::new(), + }; + Self { + recent, + storage: Some(Storage { path, persistence }), + ..Self::default() + } + } + + pub(crate) fn inherit(&mut self, previous: &mut Self) { + let mut retired = std::mem::take(previous); + if self.storage == retired.storage { + std::mem::swap(self, &mut retired); + } + // Close the retired queue, but keep its worker reachable for orderly shutdown. + drop(retired.writer.take()); + self.workers.append(&mut retired.workers); + self.workers.retain(|worker| !worker.is_finished()); + } + + pub(crate) async fn flush(&mut self) { + drop(self.writer.take()); + let mut workers = std::mem::take(&mut self.workers); + let drain = async { + for worker in &mut workers { + if let Err(error) = worker.await { + tracing::debug!(%error, "mention preference writer stopped"); + } + } + }; + if tokio::time::timeout(FLUSH_TIMEOUT, drain).await.is_err() { + for worker in workers { + worker.abort(); + } + tracing::debug!("timed out saving mention preferences"); + } + } + + pub(super) fn frequencies(&self) -> HashMap { + let mut counts = HashMap::new(); + for identity in &self.recent { + *counts.entry(identity.clone()).or_default() += 1; + } + counts + } + + pub(super) fn record(&mut self, identity: &str) { + append_recent(&mut self.recent, identity.to_owned()); + let Some(Storage { + path: file_path, + persistence: HistoryPersistence::SaveAll, + }) = self.storage.as_ref() + else { + return; + }; + let Ok(runtime) = tokio::runtime::Handle::try_current() else { + return; + }; + let writer = self.writer.get_or_insert_with(|| { + let (sender, mut receiver) = mpsc::channel::(HISTORY_LIMIT); + let file_path = file_path.clone(); + self.workers.push(runtime.spawn(async move { + while let Some(identity) = receiver.recv().await { + let file_path = file_path.clone(); + match tokio::task::spawn_blocking(move || { + persist_selection(&file_path, identity) + }) + .await + { + Ok(Ok(())) => {} + Ok(Err(error)) => { + tracing::debug!(%error, "could not save mention preferences"); + } + Err(error) => { + tracing::debug!(%error, "mention preference writer stopped"); + } + } + } + })); + sender + }); + if writer.try_send(identity.to_owned()).is_err() { + tracing::debug!("could not queue mention preference update"); + } + } +} + +fn append_recent(recent: &mut VecDeque, identity: String) { + if recent.len() == HISTORY_LIMIT { + recent.pop_front(); + } + recent.push_back(identity); +} + +fn read_recent(path: &Path) -> io::Result> { + let file = match File::open(path) { + Ok(file) => file, + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(VecDeque::new()), + Err(error) => return Err(error), + }; + if file.metadata()?.len() > MAX_FILE_BYTES { + return Err(io::Error::other("mention preferences exceed size limit")); + } + let mut recent: VecDeque = match serde_json::from_reader(file) { + Ok(recent) => recent, + Err(error) if error.is_io() => return Err(error.into()), + Err(_) => VecDeque::new(), + }; + recent.drain(..recent.len().saturating_sub(HISTORY_LIMIT)); + Ok(recent) +} + +fn persist_selection(path: &Path, identity: String) -> io::Result<()> { + let lock_file = OpenOptions::new() + .create(true) + .write(true) + .truncate(false) + .open(path.with_extension("lock"))?; + let deadline = Instant::now() + LOCK_TIMEOUT; + loop { + match lock_file.try_lock() { + Ok(()) => break, + Err(std::fs::TryLockError::WouldBlock) if Instant::now() < deadline => { + std::thread::sleep(LOCK_RETRY_INTERVAL); + } + Err(error) => return Err(error.into()), + } + } + let mut recent = read_recent(path)?; + append_recent(&mut recent, identity); + let contents = serde_json::to_string(&recent)?; + if contents.len() as u64 > MAX_FILE_BYTES { + return Err(io::Error::other("mention preferences exceed size limit")); + } + write_atomically(path, &contents) +} + +#[cfg(test)] +#[path = "mention_usage_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/bottom_pane/mention_usage_tests.rs b/codex-rs/tui/src/bottom_pane/mention_usage_tests.rs new file mode 100644 index 0000000000..5e17c2fa63 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/mention_usage_tests.rs @@ -0,0 +1,88 @@ +use super::*; +use pretty_assertions::assert_eq; + +#[tokio::test] +async fn selection_frequency_persists_after_handoff_and_flush() { + let home = tempfile::tempdir().expect("temporary home"); + let mut usage = MentionUsage::load(home.path(), HistoryPersistence::SaveAll); + usage.record("skill://status"); + usage.record("skill://status"); + let mut replacement = MentionUsage::load(home.path(), HistoryPersistence::SaveAll); + replacement.inherit(&mut usage); + replacement.flush().await; + + assert_eq!( + MentionUsage::load(home.path(), HistoryPersistence::SaveAll).frequencies(), + HashMap::from([("skill://status".to_owned(), 2)]) + ); +} + +#[tokio::test] +async fn changing_storage_keeps_usage_separate_and_flushes_prior_writes() { + let first_home = tempfile::tempdir().expect("first temporary home"); + let second_home = tempfile::tempdir().expect("second temporary home"); + let mut first = MentionUsage::load(first_home.path(), HistoryPersistence::SaveAll); + first.record("skill://first"); + let mut second = MentionUsage::load(second_home.path(), HistoryPersistence::SaveAll); + second.inherit(&mut first); + second.record("skill://second"); + second.flush().await; + + assert_eq!( + ( + MentionUsage::load(first_home.path(), HistoryPersistence::SaveAll).frequencies(), + MentionUsage::load(second_home.path(), HistoryPersistence::SaveAll).frequencies(), + ), + ( + HashMap::from([("skill://first".to_owned(), 1)]), + HashMap::from([("skill://second".to_owned(), 1)]), + ) + ); +} + +#[test] +fn old_selections_expire() { + let mut usage = MentionUsage::default(); + usage.record("skill://status"); + for _ in 0..HISTORY_LIMIT { + usage.record("skill://other"); + } + assert_eq!( + usage.frequencies(), + HashMap::from([("skill://other".to_owned(), HISTORY_LIMIT)]) + ); +} + +#[test] +fn selection_replaces_malformed_history() { + let home = tempfile::tempdir().expect("temporary home"); + let path = home.path().join(FILE_NAME); + std::fs::write(&path, "[\"truncated").expect("write malformed history"); + + persist_selection(&path, "skill://status".to_owned()).expect("persist selection"); + + assert_eq!( + MentionUsage::load(home.path(), HistoryPersistence::SaveAll).frequencies(), + HashMap::from([("skill://status".to_owned(), 1)]) + ); +} + +#[test] +fn contended_lock_does_not_prevent_later_selections() { + let home = tempfile::tempdir().expect("temporary home"); + let path = home.path().join(FILE_NAME); + let lock = File::create(path.with_extension("lock")).expect("create lock file"); + lock.lock().expect("hold lock"); + assert_eq!( + persist_selection(&path, "skill://blocked".to_owned()) + .expect_err("lock contention") + .kind(), + io::ErrorKind::WouldBlock + ); + drop(lock); + persist_selection(&path, "skill://status".to_owned()).expect("persist selection"); + assert_eq!( + MentionUsage::load(home.path(), HistoryPersistence::SaveAll).frequencies(), + HashMap::from([("skill://status".to_owned(), 1)]) + ); +} diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index ca77db8794..83341eedb4 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -154,6 +154,8 @@ mod file_search_popup; mod footer; mod list_selection_view; mod memories_settings_view; +mod mention_usage; +pub(crate) use mention_usage::MentionUsage; mod mentions_v2; mod picker_presets; mod picker_rows; @@ -352,6 +354,18 @@ pub(crate) struct BottomPaneParams { } impl BottomPane { + pub(crate) fn set_mention_usage_home( + &mut self, + codex_home: &std::path::Path, + persistence: codex_config::types::HistoryPersistence, + ) { + self.composer.mention_usage = MentionUsage::load(codex_home, persistence); + } + + pub(crate) fn mention_usage_mut(&mut self) -> &mut MentionUsage { + &mut self.composer.mention_usage + } + pub fn new(params: BottomPaneParams) -> Self { Self::new_with_composer_config(params, ChatComposerConfig::default()) } diff --git a/codex-rs/tui/src/bottom_pane/skill_popup.rs b/codex-rs/tui/src/bottom_pane/skill_popup.rs index 0d12d10b5e..702a0b111f 100644 --- a/codex-rs/tui/src/bottom_pane/skill_popup.rs +++ b/codex-rs/tui/src/bottom_pane/skill_popup.rs @@ -7,6 +7,8 @@ use ratatui::style::Stylize; use ratatui::text::Line; use ratatui::widgets::Widget; use ratatui::widgets::WidgetRef; +use std::cmp::Reverse; +use std::collections::HashMap; use super::picker_rows::render_rows_single_line; use super::popup_consts::MAX_POPUP_ROWS; @@ -35,6 +37,13 @@ pub(crate) struct MentionMatch { score: i32, } +#[derive(Clone, Copy, Eq, Ord, PartialEq, PartialOrd)] +enum MatchTier { + Exact, + Prefix, + Fuzzy, +} + impl MentionItem { /// Matches the display name first, then falls back to the best distinct search term. pub(crate) fn fuzzy_match_query(&self, query: &str) -> Option { @@ -55,6 +64,24 @@ impl MentionItem { }) } } + + fn match_tier(&self, lowercase_query: &str) -> MatchTier { + let display_name = self.display_name.to_lowercase(); + let insert_name = self + .insert_text + .strip_prefix('$') + .unwrap_or(&self.insert_text) + .to_lowercase(); + if display_name == lowercase_query || insert_name == lowercase_query { + MatchTier::Exact + } else if display_name.starts_with(lowercase_query) + || insert_name.starts_with(lowercase_query) + { + MatchTier::Prefix + } else { + MatchTier::Fuzzy + } + } } const MENTION_NAME_TRUNCATE_LEN: usize = 28; @@ -62,6 +89,7 @@ const MENTION_NAME_TRUNCATE_LEN: usize = 28; pub(crate) struct SkillPopup { query: String, mentions: Vec, + frequencies: HashMap, state: ScrollState, } @@ -70,10 +98,16 @@ impl SkillPopup { Self { query: String::new(), mentions, + frequencies: HashMap::new(), state: ScrollState::new(), } } + pub(crate) fn with_frequencies(mut self, frequencies: HashMap) -> Self { + self.frequencies = frequencies; + self + } + pub(crate) fn set_mentions(&mut self, mentions: Vec) { self.mentions = mentions; self.clamp_selection(); @@ -158,6 +192,7 @@ impl SkillPopup { fn filtered(&self) -> Vec<(usize, Option>, i32)> { let filter = self.query.trim(); + let lowercase_filter = filter.to_lowercase(); let mut out: Vec<(usize, Option>, i32)> = Vec::new(); for (idx, mention) in self.mentions.iter().enumerate() { @@ -171,26 +206,27 @@ impl SkillPopup { } } - out.sort_by(|a, b| { - if filter.is_empty() { - self.mentions[a.0] - .sort_rank - .cmp(&self.mentions[b.0].sort_rank) + out.sort_by_cached_key(|(idx, indices, score)| { + let mention = &self.mentions[*idx]; + let tier = if filter.is_empty() { + MatchTier::Fuzzy } else { - a.1.is_none() - .cmp(&b.1.is_none()) - .then_with(|| a.2.cmp(&b.2)) - .then_with(|| { - self.mentions[a.0] - .sort_rank - .cmp(&self.mentions[b.0].sort_rank) - }) - } - .then_with(|| { - let an = self.mentions[a.0].display_name.as_str(); - let bn = self.mentions[b.0].display_name.as_str(); - an.cmp(bn) - }) + mention.match_tier(&lowercase_filter) + }; + let frequency = mention + .path + .as_ref() + .and_then(|path| self.frequencies.get(path)) + .copied() + .unwrap_or_default(); + ( + tier, + Reverse(frequency), + indices.is_none(), + *score, + mention.sort_rank, + &mention.display_name, + ) }); out @@ -410,9 +446,16 @@ mod tests { } #[test] - fn query_match_score_sorts_before_plugin_rank_bias() { + fn match_tier_precedes_preference_and_plugin_rank() { let mut popup = SkillPopup::new(vec![ - plugin_mention_item("GitHub", &["github", "pull requests", "pr"]), + MentionItem { + path: Some("plugin://github".to_owned()), + ..plugin_mention_item("GitHub", &["github", "pull requests", "pr"]) + }, + MentionItem { + insert_text: "$pr".to_owned(), + ..named_mention_item("Review and triage", &["pr"]) + }, named_mention_item("pr-review-triage", &["pr-review-triage"]), named_mention_item("prd", &["prd"]), named_mention_item("Plugin Creator", &["plugin-creator", "Plugin Creator"]), @@ -421,7 +464,8 @@ mod tests { &["logging-best-practices", "Logging Best Practices"], ), named_mention_item("PR Babysitter", &["babysit-pr", "PR Babysitter"]), - ]); + ]) + .with_frequencies(HashMap::from([("plugin://github".to_owned(), 50)])); popup.set_query("pr"); let filtered_items: Vec<(String, Option)> = popup @@ -438,15 +482,16 @@ mod tests { assert_eq!( filtered_items, vec![ + ("Review and triage".to_string(), Some("[Skill]".to_string())), ("PR Babysitter".to_string(), Some("[Skill]".to_string())), ("pr-review-triage".to_string(), Some("[Skill]".to_string())), ("prd".to_string(), Some("[Skill]".to_string())), + ("GitHub".to_string(), Some("[Plugin]".to_string())), ("Plugin Creator".to_string(), Some("[Skill]".to_string())), ( "Logging Best Practices".to_string(), Some("[Skill]".to_string()) ), - ("GitHub".to_string(), Some("[Plugin]".to_string())), ] ); } diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__skill_popup_prefers_confirmed_selections_within_match_tier.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__skill_popup_prefers_confirmed_selections_within_match_tier.snap new file mode 100644 index 0000000000..b5e91a0777 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__skill_popup_prefers_confirmed_selections_within_match_tier.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/bottom_pane/chat_composer.rs +expression: terminal.backend() +--- +" " +"› status [Skill] Example skill used in tests. " +" statsig [Skill] Example skill used in tests. " +"↓ " +" enter insert · esc close " +" " +"› $stat " +" " +" 100% context left " diff --git a/codex-rs/tui/src/chatwidget/constructor.rs b/codex-rs/tui/src/chatwidget/constructor.rs index 8ef50403dd..34ee1ec53d 100644 --- a/codex-rs/tui/src/chatwidget/constructor.rs +++ b/codex-rs/tui/src/chatwidget/constructor.rs @@ -298,6 +298,10 @@ impl ChatWidget { test_codex_home: None, }; + widget.bottom_pane.set_mention_usage_home( + widget.local_settings.codex_home.as_path(), + widget.local_settings.history.persistence, + ); widget.prefetch_rate_limits(); if let Some(keymap) = runtime_keymap { widget.bottom_pane.set_keymap_bindings(&keymap); diff --git a/codex-rs/tui/src/chatwidget/input_restore.rs b/codex-rs/tui/src/chatwidget/input_restore.rs index 063fb7d919..bc4a9c1be3 100644 --- a/codex-rs/tui/src/chatwidget/input_restore.rs +++ b/codex-rs/tui/src/chatwidget/input_restore.rs @@ -6,11 +6,16 @@ use codex_protocol::models::local_image_label_text; use crate::bottom_pane::ComposerDraftSnapshot; use crate::bottom_pane::KillBufferSnapshot; +use crate::bottom_pane::MentionUsage; use super::user_messages::remap_colliding_paste_placeholders; use super::*; impl ChatWidget { + pub(crate) fn mention_usage_mut(&mut self) -> &mut MentionUsage { + self.bottom_pane.mention_usage_mut() + } + pub(crate) fn take_kill_buffer_snapshot(&mut self) -> KillBufferSnapshot { self.bottom_pane.take_kill_buffer_snapshot() }