From d9dd2418eead7a877dc4a55be151d5393701538e Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Mon, 19 Jan 2026 11:42:34 -0800 Subject: [PATCH] Switch slash popup to prefix filtering --- codex-rs/tui/src/bottom_pane/command_popup.rs | 60 ++++++++------ ..._chat_composer__tests__slash_popup_mo.snap | 4 +- .../tui2/src/bottom_pane/command_popup.rs | 50 ++++++++---- ..._chat_composer__tests__slash_popup_mo.snap | 4 +- codex-rs/utils/cargo-bin/src/lib.rs | 79 +++++++++++++++++-- 5 files changed, 146 insertions(+), 51 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/command_popup.rs b/codex-rs/tui/src/bottom_pane/command_popup.rs index 83afdca30c..82c490e9a4 100644 --- a/codex-rs/tui/src/bottom_pane/command_popup.rs +++ b/codex-rs/tui/src/bottom_pane/command_popup.rs @@ -10,7 +10,6 @@ use crate::render::Insets; use crate::render::RectExt; use crate::slash_command::SlashCommand; use crate::slash_command::built_in_slash_commands; -use codex_common::fuzzy_match::fuzzy_match; use codex_protocol::custom_prompts::CustomPrompt; use codex_protocol::custom_prompts::PROMPTS_CMD_PREFIX; use std::collections::HashSet; @@ -22,6 +21,10 @@ fn windows_degraded_sandbox_active() -> bool { && !codex_core::is_windows_elevated_sandbox_enabled() } +fn prefix_match_indices(prefix_len: usize) -> Vec { + (0..prefix_len).collect() +} + /// A selectable item in the popup: either a built-in command or a user prompt. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum CommandItem { @@ -119,11 +122,14 @@ impl CommandPopup { measure_rows_height(&rows, &self.state, MAX_POPUP_ROWS, width) } - /// Compute fuzzy-filtered matches over built-in commands and user prompts, - /// paired with optional highlight indices and score. Preserves the original + /// Compute prefix-filtered matches over built-in commands and user prompts, + /// paired with optional highlight indices. Preserves the original /// presentation order for built-ins and prompts. fn filtered(&self) -> Vec<(CommandItem, Option>, i32)> { let filter = self.command_filter.trim(); + let filter_lower = filter.to_ascii_lowercase(); + let filter_len = filter.chars().count(); + let prompts_prefix_len = PROMPTS_CMD_PREFIX.len() + 1; let mut out: Vec<(CommandItem, Option>, i32)> = Vec::new(); if filter.is_empty() { // Built-ins first, in presentation order. @@ -138,17 +144,32 @@ impl CommandPopup { } for (_, cmd) in self.builtins.iter() { - if let Some((indices, score)) = fuzzy_match(cmd.command(), filter) { - out.push((CommandItem::Builtin(*cmd), Some(indices), score)); + if cmd.command().starts_with(&filter_lower) { + out.push(( + CommandItem::Builtin(*cmd), + Some(prefix_match_indices(filter_len)), + 0, + )); } } - // Support both search styles: - // - Typing "name" should surface "/prompts:name" results. - // - Typing "prompts:name" should also work. for (idx, p) in self.prompts.iter().enumerate() { let display = format!("{PROMPTS_CMD_PREFIX}:{}", p.name); - if let Some((indices, score)) = fuzzy_match(&display, filter) { - out.push((CommandItem::UserPrompt(idx), Some(indices), score)); + let display_lower = display.to_ascii_lowercase(); + if display_lower.starts_with(&filter_lower) { + out.push(( + CommandItem::UserPrompt(idx), + Some(prefix_match_indices(filter_len)), + 0, + )); + continue; + } + + // Support "name" prefix search for "/prompts:name" results. + if p.name.to_ascii_lowercase().starts_with(&filter_lower) { + let indices = (0..filter_len) + .map(|i| prompts_prefix_len + i) + .collect::>(); + out.push((CommandItem::UserPrompt(idx), Some(indices), 0)); } } out @@ -298,17 +319,7 @@ mod tests { CommandItem::UserPrompt(_) => None, }) .collect(); - assert_eq!( - cmds, - vec![ - "model", - "experimental", - "resume", - "compact", - "mention", - "mcp" - ] - ); + assert_eq!(cmds, vec!["model", "mention", "mcp"]); } #[test] @@ -404,7 +415,7 @@ mod tests { } #[test] - fn fuzzy_filter_matches_subsequence_for_ac() { + fn prefix_filter_excludes_non_prefix_matches() { let mut popup = CommandPopup::new(Vec::new(), CommandPopupFlags::default()); popup.on_composer_text_change("/ac".to_string()); @@ -416,10 +427,7 @@ mod tests { CommandItem::UserPrompt(_) => None, }) .collect(); - assert!( - cmds.contains(&"compact") && cmds.contains(&"feedback"), - "expected fuzzy search for '/ac' to include compact and feedback, got {cmds:?}" - ); + assert_eq!(cmds, Vec::<&str>::new()); } #[test] diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__slash_popup_mo.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__slash_popup_mo.snap index 661e82e3ad..2d5b29038a 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__slash_popup_mo.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__slash_popup_mo.snap @@ -5,5 +5,5 @@ expression: terminal.backend() " " "› /mo " " " -" /model choose what model and reasoning effort to use " -" /mention mention a file " +" " +" /model choose what model and reasoning effort to use " diff --git a/codex-rs/tui2/src/bottom_pane/command_popup.rs b/codex-rs/tui2/src/bottom_pane/command_popup.rs index 000ce2e4ea..593baa82e4 100644 --- a/codex-rs/tui2/src/bottom_pane/command_popup.rs +++ b/codex-rs/tui2/src/bottom_pane/command_popup.rs @@ -10,7 +10,6 @@ use crate::render::Insets; use crate::render::RectExt; use crate::slash_command::SlashCommand; use crate::slash_command::built_in_slash_commands; -use codex_common::fuzzy_match::fuzzy_match; use codex_protocol::custom_prompts::CustomPrompt; use codex_protocol::custom_prompts::PROMPTS_CMD_PREFIX; use std::collections::HashSet; @@ -22,6 +21,10 @@ fn windows_degraded_sandbox_active() -> bool { && !codex_core::is_windows_elevated_sandbox_enabled() } +fn prefix_match_indices(prefix_len: usize) -> Vec { + (0..prefix_len).collect() +} + /// A selectable item in the popup: either a built-in command or a user prompt. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum CommandItem { @@ -119,11 +122,14 @@ impl CommandPopup { measure_rows_height(&rows, &self.state, MAX_POPUP_ROWS, width) } - /// Compute fuzzy-filtered matches over built-in commands and user prompts, - /// paired with optional highlight indices and score. Preserves the original + /// Compute prefix-filtered matches over built-in commands and user prompts, + /// paired with optional highlight indices. Preserves the original /// presentation order for built-ins and prompts. fn filtered(&self) -> Vec<(CommandItem, Option>, i32)> { let filter = self.command_filter.trim(); + let filter_lower = filter.to_ascii_lowercase(); + let filter_len = filter.chars().count(); + let prompts_prefix_len = PROMPTS_CMD_PREFIX.len() + 1; let mut out: Vec<(CommandItem, Option>, i32)> = Vec::new(); if filter.is_empty() { // Built-ins first, in presentation order. @@ -138,17 +144,32 @@ impl CommandPopup { } for (_, cmd) in self.builtins.iter() { - if let Some((indices, score)) = fuzzy_match(cmd.command(), filter) { - out.push((CommandItem::Builtin(*cmd), Some(indices), score)); + if cmd.command().starts_with(&filter_lower) { + out.push(( + CommandItem::Builtin(*cmd), + Some(prefix_match_indices(filter_len)), + 0, + )); } } - // Support both search styles: - // - Typing "name" should surface "/prompts:name" results. - // - Typing "prompts:name" should also work. for (idx, p) in self.prompts.iter().enumerate() { let display = format!("{PROMPTS_CMD_PREFIX}:{}", p.name); - if let Some((indices, score)) = fuzzy_match(&display, filter) { - out.push((CommandItem::UserPrompt(idx), Some(indices), score)); + let display_lower = display.to_ascii_lowercase(); + if display_lower.starts_with(&filter_lower) { + out.push(( + CommandItem::UserPrompt(idx), + Some(prefix_match_indices(filter_len)), + 0, + )); + continue; + } + + // Support "name" prefix search for "/prompts:name" results. + if p.name.to_ascii_lowercase().starts_with(&filter_lower) { + let indices = (0..filter_len) + .map(|i| prompts_prefix_len + i) + .collect::>(); + out.push((CommandItem::UserPrompt(idx), Some(indices), 0)); } } out @@ -297,7 +318,7 @@ mod tests { CommandItem::UserPrompt(_) => None, }) .collect(); - assert_eq!(cmds, vec!["model", "resume", "compact", "mention", "mcp"]); + assert_eq!(cmds, vec!["model", "mention", "mcp"]); } #[test] @@ -393,7 +414,7 @@ mod tests { } #[test] - fn fuzzy_filter_matches_subsequence_for_ac() { + fn prefix_filter_excludes_non_prefix_matches() { let mut popup = CommandPopup::new(Vec::new(), CommandPopupFlags::default()); popup.on_composer_text_change("/ac".to_string()); @@ -405,10 +426,7 @@ mod tests { CommandItem::UserPrompt(_) => None, }) .collect(); - assert!( - cmds.contains(&"compact") && cmds.contains(&"feedback"), - "expected fuzzy search for '/ac' to include compact and feedback, got {cmds:?}" - ); + assert_eq!(cmds, Vec::<&str>::new()); } #[test] diff --git a/codex-rs/tui2/src/bottom_pane/snapshots/codex_tui2__bottom_pane__chat_composer__tests__slash_popup_mo.snap b/codex-rs/tui2/src/bottom_pane/snapshots/codex_tui2__bottom_pane__chat_composer__tests__slash_popup_mo.snap index dc66d149e4..d497826853 100644 --- a/codex-rs/tui2/src/bottom_pane/snapshots/codex_tui2__bottom_pane__chat_composer__tests__slash_popup_mo.snap +++ b/codex-rs/tui2/src/bottom_pane/snapshots/codex_tui2__bottom_pane__chat_composer__tests__slash_popup_mo.snap @@ -5,5 +5,5 @@ expression: terminal.backend() " " "› /mo " " " -" /model choose what model and reasoning effort to use " -" /mention mention a file " +" " +" /model choose what model and reasoning effort to use " diff --git a/codex-rs/utils/cargo-bin/src/lib.rs b/codex-rs/utils/cargo-bin/src/lib.rs index 40fa40c62f..e1a861c78c 100644 --- a/codex-rs/utils/cargo-bin/src/lib.rs +++ b/codex-rs/utils/cargo-bin/src/lib.rs @@ -51,14 +51,83 @@ pub fn cargo_bin(name: &str) -> Result { }) } } - Err(err) => Err(CargoBinError::NotFound { - name: name.to_owned(), - env_keys, - fallback: format!("assert_cmd fallback failed: {err}"), - }), + Err(err) => { + let mut fallback = format!("assert_cmd fallback failed: {err}"); + match resolve_bin_from_workspace_target_dir(name) { + Ok(Some(bin)) => return Ok(bin), + Ok(None) => {} + Err(workspace_err) => { + fallback = format!("{fallback}; workspace fallback failed: {workspace_err}"); + } + } + + Err(CargoBinError::NotFound { + name: name.to_owned(), + env_keys, + fallback, + }) + } } } +fn resolve_bin_from_workspace_target_dir(name: &str) -> Result, String> { + let profile_dir = cargo_profile_dir_from_current_exe()?; + let Some(profile_dir) = profile_dir else { + return Ok(None); + }; + + let bin_path = profile_dir.join(format!("{name}{}", std::env::consts::EXE_SUFFIX)); + if bin_path.exists() { + return Ok(Some(bin_path)); + } + + let workspace_root = profile_dir + .parent() + .and_then(|target_dir| target_dir.parent()) + .ok_or_else(|| { + format!( + "could not derive workspace root from profile dir {}", + profile_dir.display() + ) + })?; + + let status = std::process::Command::new("cargo") + .current_dir(workspace_root) + .args(["build", "--bin", name]) + .status() + .map_err(|err| format!("failed to run cargo build --bin {name}: {err}"))?; + + if !status.success() { + return Err(format!("cargo build --bin {name} exited with {status}")); + } + + if bin_path.exists() { + Ok(Some(bin_path)) + } else { + Err(format!( + "cargo build succeeded but {} does not exist", + bin_path.display() + )) + } +} + +fn cargo_profile_dir_from_current_exe() -> Result, String> { + let exe = + std::env::current_exe().map_err(|err| format!("failed to read current exe: {err}"))?; + + // Cargo integration tests typically live under: + // /target//deps/ + // The workspace bin lives at: + // /target// + for ancestor in exe.ancestors() { + if ancestor.file_name().is_some_and(|name| name == "deps") { + return Ok(ancestor.parent().map(PathBuf::from)); + } + } + + Ok(None) +} + fn cargo_bin_env_keys(name: &str) -> Vec { let mut keys = Vec::with_capacity(2); keys.push(format!("CARGO_BIN_EXE_{name}"));