From 2ce1579916f6f6f8002360be67a3f8590bd12132 Mon Sep 17 00:00:00 2001 From: Azeem Shaik Date: Sun, 23 Aug 2026 22:35:25 +0530 Subject: [PATCH 01/13] refactor completions into a backend based architecture - Add a CompletionBackend trait: one `complete(request) -> result` method, so each backend decides for itself whether a request is its own. - Add matcher.rs, a pure ranking function shared by every backend. Ranking now follows the typed pattern rather than the order the scan happened to produce. - Fold the file-path scanner behind the trait: one flat workspace index in place of per-directory scans, dropping DirEntry, refilter and dir_part. --- crates/alan/src/core/completion.rs | 809 ---------------------- crates/alan/src/core/completion/mod.rs | 294 ++++++++ crates/alan/src/core/completion/paths.rs | 390 +++++++++++ crates/alan/src/core/matcher.rs | 112 +++ crates/alan/src/core/mod.rs | 5 +- crates/alan/src/views/components/popup.rs | 74 +- crates/alan/src/views/mod.rs | 172 ++--- 7 files changed, 871 insertions(+), 985 deletions(-) delete mode 100644 crates/alan/src/core/completion.rs create mode 100644 crates/alan/src/core/completion/mod.rs create mode 100644 crates/alan/src/core/completion/paths.rs create mode 100644 crates/alan/src/core/matcher.rs diff --git a/crates/alan/src/core/completion.rs b/crates/alan/src/core/completion.rs deleted file mode 100644 index 60102fe..0000000 --- a/crates/alan/src/core/completion.rs +++ /dev/null @@ -1,809 +0,0 @@ -//! File-path completion for the prompt editor. -//! -//! Typing `@` in the editor opens a completion popup listing files and -//! folders. A bare token (`@popup`) recursively searches the project and -//! matches anywhere in relative paths; a token with a slash (`@src/fo`) -//! lists `src/` filtered by the `fo` prefix. Directory scans run on the -//! blocking thread pool and deliver results through a channel drained by -//! [`CompletionController::poll`]. - -use super::Poll; -use std::io; -use std::path::{Component, Path, PathBuf}; -use std::sync::Arc; -use std::sync::atomic::{AtomicU64, Ordering}; -use tokio::sync::watch::{self, Receiver, Sender}; - -/// Maximum candidates retained from one scan for later in-memory filtering. -const CANDIDATE_LIMIT: usize = 5_000; -/// Maximum candidates displayed after filtering. -const SCAN_LIMIT: usize = 250; -/// Maximum filesystem entries visited by one scan. -const VISIT_LIMIT: usize = 20_000; -/// Maximum recursive depth for a bare-token scan. -const MAX_SCAN_DEPTH: usize = 32; -/// Directories excluded from scans regardless of prefix. -const SKIPPED_DIRS: &[&str] = &[".git", "target", "node_modules"]; - -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct DirEntry { - /// Path relative to the scanned root, e.g. `crates/agent/src/lib.rs`. - pub path: String, - pub is_dir: bool, -} - -impl DirEntry { - /// Final path segment, for prefix filtering. - fn file_name(&self) -> &str { - self.path.rsplit('/').next().unwrap_or(&self.path) - } -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum CompletionStatus { - Loading, - Ready, - Error(String), -} - -/// Filtered completion candidates shown in the popup. -#[derive(Debug, Clone)] -pub struct CompletionState { - pub items: Vec, - pub selected: usize, - pub status: CompletionStatus, -} - -impl Default for CompletionState { - fn default() -> Self { - Self { - items: Vec::new(), - selected: 0, - status: CompletionStatus::Loading, - } - } -} - -/// Result of accepting the highlighted completion. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct Accepted { - /// Text replacing the `@token`, including the typed directory segments. - pub replacement: String, - /// Directories keep the popup open so the user can drill deeper. - pub is_dir: bool, -} - -type ScanResults = io::Result>; - -/// Token shared with a scan task. Bumping it asks the traversal to bail early. -/// `Arc` is the cheapest cancellation primitive available on the -/// blocking thread pool: no `JoinHandle` polling, no runtime blocking. -type ScanCancel = Arc; - -pub struct CompletionController { - state: CompletionState, - open: bool, - /// Workspace root used to resolve typed relative directories. - root: PathBuf, - /// Raw scan results for the current directory. - entries: Vec, - /// Directory segment of the active token as typed ("" for the project root). - dir_part: String, - prefix: String, - /// Staleness stamp bumped on every new scan request. - generation: u64, - /// Directory of the most recent scan request, for deduplication. - last_scan_dir: Option, - /// Latest scan result. Stale results are drained before applying the newest. - tx: Sender>, - rx: Receiver>, - /// Epoch token shared with a scan task so it can bail early. - cancel_epoch: ScanCancel, -} - -impl CompletionController { - pub fn new() -> Self { - let root = std::env::current_dir() - .ok() - .and_then(|path| path.canonicalize().ok()) - .unwrap_or_else(|| PathBuf::from(".")); - Self::with_root(root) - } - - pub(crate) fn with_root(root: PathBuf) -> Self { - let (tx, rx) = watch::channel(None); - Self { - state: CompletionState::default(), - open: false, - root, - entries: Vec::new(), - dir_part: String::new(), - prefix: String::new(), - generation: 0, - last_scan_dir: None, - tx, - rx, - cancel_epoch: Arc::new(AtomicU64::new(1)), - } - } - - /// Track the token currently typed after `@`. - /// - /// A bare token recursively searches the workspace; a token containing - /// `/` lists that directory. The scan is independent of the filename - /// prefix so later keystrokes can refilter the same result safely. - pub fn update(&mut self, token: &str) { - let (dir_part, prefix) = match token.rsplit_once('/') { - Some((dir, prefix)) => (dir, prefix), - None => ("", token), - }; - let scan_key = if dir_part.is_empty() { - PathBuf::from(".") - } else { - PathBuf::from(dir_part) - }; - let Some(scan_dir) = self.resolve_relative_dir(dir_part) else { - self.cancel_scan(); - self.open = true; - self.entries.clear(); - self.state = CompletionState { - status: CompletionStatus::Error("path is outside the workspace".into()), - ..CompletionState::default() - }; - self.dir_part = dir_part.to_owned(); - self.prefix = prefix.to_owned(); - self.last_scan_dir = None; - return; - }; - - self.prefix = prefix.to_owned(); - self.dir_part = dir_part.to_owned(); - if self.last_scan_dir.as_ref() != Some(&scan_key) { - self.generation += 1; - self.last_scan_dir = Some(scan_key.clone()); - self.entries.clear(); - self.state = CompletionState { - status: CompletionStatus::Loading, - ..CompletionState::default() - }; - self.spawn_scan(scan_dir, scan_key, self.generation, dir_part.is_empty()); - } - self.open = true; - self.refilter(); - } - - pub fn poll(&mut self) -> Poll { - if !self.rx.has_changed().unwrap_or(false) { - return Poll::Idle; - } - let results = { - let result = self.rx.borrow_and_update(); - let Some((generation, key, results)) = result.as_ref() else { - return Poll::Idle; - }; - if *generation != self.generation || self.last_scan_dir.as_ref() != Some(key) { - return Poll::Idle; - } - match results { - Ok(entries) => Ok(entries.clone()), - Err(error) => Err(io::Error::new(error.kind(), error.to_string())), - } - }; - self.state.status = match results { - Ok(entries) => { - self.entries = entries; - self.refilter(); - CompletionStatus::Ready - } - Err(error) => { - self.entries.clear(); - self.state.items.clear(); - CompletionStatus::Error(completion_error(&error)) - } - }; - Poll::Changed - } - - /// Invalidate any in-flight traversal. The blocking task observes this - /// epoch and exits cooperatively at its next filesystem boundary. - fn cancel_scan(&mut self) { - self.cancel_epoch.fetch_add(1, Ordering::Release); - } - - fn resolve_relative_dir(&self, dir_part: &str) -> Option { - let relative = Path::new(dir_part); - if relative.is_absolute() { - return None; - } - let mut normalized = PathBuf::new(); - for component in relative.components() { - match component { - Component::CurDir => {} - Component::Normal(segment) => normalized.push(segment), - Component::ParentDir => { - if !normalized.pop() { - return None; - } - } - Component::RootDir | Component::Prefix(_) => return None, - } - } - - let candidate = self.root.join(normalized); - let mut existing = candidate.as_path(); - loop { - if let Ok(canonical) = existing.canonicalize() { - return canonical.starts_with(&self.root).then_some(candidate); - } - existing = existing.parent()?; - } - } - - pub fn move_selection(&mut self, delta: isize) { - if self.state.items.is_empty() { - return; - } - let max = self.state.items.len() - 1; - let next = (self.state.selected as isize + delta).clamp(0, max as isize); - self.state.selected = next as usize; - } - - /// Accept the highlighted entry. Files close the popup; directories keep - /// it open so the next token update drills into them. - pub fn accept(&mut self) -> Option { - if !matches!(self.state.status, CompletionStatus::Ready) { - return None; - } - let entry = self.state.items.get(self.state.selected)?.clone(); - - let mut replacement = String::new(); - if !self.dir_part.is_empty() { - replacement.push_str(&self.dir_part); - replacement.push('/'); - } - replacement.push_str(&entry.path); - if entry.is_dir { - replacement.push('/'); - } else { - self.dismiss(); - } - - Some(Accepted { - replacement, - is_dir: entry.is_dir, - }) - } - - pub fn dismiss(&mut self) { - self.open = false; - self.entries.clear(); - self.state = CompletionState::default(); - self.dir_part.clear(); - self.prefix.clear(); - self.last_scan_dir = None; - self.cancel_scan(); - } - - pub fn is_open(&self) -> bool { - self.open - } - - pub fn has_items(&self) -> bool { - !self.state.items.is_empty() - } - - pub fn state(&self) -> Option<&CompletionState> { - self.open.then_some(&self.state) - } - - fn spawn_scan(&mut self, dir: PathBuf, key: PathBuf, generation: u64, recursive: bool) { - self.cancel_scan(); - let my_epoch = self.cancel_epoch.load(Ordering::Acquire); - let cancel = self.cancel_epoch.clone(); - let tx = self.tx.clone(); - if let Ok(handle) = tokio::runtime::Handle::try_current() { - handle.spawn_blocking(move || { - let results = scan_dir_with(&dir, recursive, &cancel, my_epoch); - let _ = tx.send(Some((generation, key, results))); - }); - } else { - let results = scan_dir_with(&dir, recursive, &cancel, my_epoch); - let _ = tx.send(Some((generation, key, results))); - } - } - - fn refilter(&mut self) { - let prefix = self.prefix.to_lowercase(); - self.state.items = self - .entries - .iter() - .filter(|entry| matches_prefix(&entry.path, entry.file_name(), &prefix)) - .take(SCAN_LIMIT) - .cloned() - .collect(); - if self.state.selected >= self.state.items.len() { - self.state.selected = self.state.items.len().saturating_sub(1); - } - } - - #[cfg(test)] - pub(crate) fn inject_items(&mut self, items: Vec) { - self.entries = items; - self.open = true; - self.state.status = CompletionStatus::Ready; - self.refilter(); - } -} - -/// List `dir`'s entries. The optional prefix is used only by test helpers; -/// production scans collect a bounded candidate set and refilter it in memory. -#[cfg(test)] -fn scan_dir(dir: &Path, recursive: bool, _prefix: &str) -> ScanResults { - let cancel: ScanCancel = Arc::new(AtomicU64::new(1)); - scan_dir_with(dir, recursive, &cancel, 1) -} - -/// Like [`scan_dir`], but cooperative: the traversal bails as soon as -/// `cancel` no longer equals `my_epoch`. -fn scan_dir_with(dir: &Path, recursive: bool, cancel: &ScanCancel, my_epoch: u64) -> ScanResults { - let mut builder = ignore::WalkBuilder::new(dir); - builder - // `ignore` handles hidden files and .ignore/.gitignore files. Keep - // these application-level exclusions in addition to those filters. - .standard_filters(true) - .follow_links(false) - .min_depth(Some(1)) - .max_depth(Some(if recursive { MAX_SCAN_DEPTH } else { 1 })) - .filter_entry(|entry| entry.depth() == 0 || !is_skipped_name(entry.file_name())); - - let mut entries = Vec::new(); - for (visited, result) in builder.build().enumerate() { - if cancel.load(Ordering::Acquire) != my_epoch || visited >= VISIT_LIMIT { - break; - } - let entry = result.map_err(ignore_error)?; - - let Some(file_type) = entry.file_type() else { - continue; - }; - let Some(relative) = entry.path().strip_prefix(dir).ok() else { - continue; - }; - entries.push(DirEntry { - path: relative_path(relative), - is_dir: file_type.is_dir(), - }); - } - entries.sort_by(sort_entries); - entries.truncate(CANDIDATE_LIMIT); - Ok(entries) -} - -fn is_skipped_name(name: &std::ffi::OsStr) -> bool { - SKIPPED_DIRS - .iter() - .any(|skipped| name == std::ffi::OsStr::new(skipped)) -} - -fn relative_path(path: &Path) -> String { - path.components() - .filter_map(|component| match component { - Component::Normal(name) => Some(name.to_string_lossy().into_owned()), - _ => None, - }) - .collect::>() - .join("/") -} - -fn ignore_error(error: ignore::Error) -> io::Error { - let kind = error - .io_error() - .map_or(io::ErrorKind::Other, io::Error::kind); - io::Error::new(kind, error.to_string()) -} - -fn completion_error(error: &io::Error) -> String { - match error.kind() { - io::ErrorKind::NotFound => "directory not found".into(), - _ => error.to_string(), - } -} - -fn sort_entries(a: &DirEntry, b: &DirEntry) -> std::cmp::Ordering { - a.path - .matches('/') - .count() - .cmp(&b.path.matches('/').count()) - .then_with(|| b.is_dir.cmp(&a.is_dir)) - .then_with(|| a.path.cmp(&b.path)) -} - -/// Whether an entry should be included given the active prefix. -/// -/// Mirrors [`CompletionController::refilter`]: an empty prefix accepts -/// everything; otherwise the path must contain the prefix or its final -/// segment must start with it. -fn matches_prefix(rel_path: &str, file_name: &str, prefix: &str) -> bool { - prefix.is_empty() - || rel_path.to_lowercase().contains(prefix) - || file_name.to_lowercase().starts_with(prefix) -} - -#[cfg(test)] -mod tests { - use super::*; - use std::fs; - - fn unique_temp_dir(label: &str) -> PathBuf { - let dir = - std::env::temp_dir().join(format!("alan-completion-{label}-{}", std::process::id())); - let _ = fs::remove_dir_all(&dir); - fs::create_dir_all(&dir).unwrap(); - dir - } - - #[test] - fn scan_dir_sorts_dirs_first_and_skips_junk() { - let root = unique_temp_dir("scan"); - fs::create_dir(root.join(".git")).unwrap(); - fs::create_dir(root.join("target")).unwrap(); - fs::create_dir(root.join("src")).unwrap(); - fs::write(root.join("zeta.txt"), "").unwrap(); - fs::write(root.join("alpha.txt"), "").unwrap(); - - let entries = scan_dir(&root, true, "").unwrap(); - - assert_eq!( - entries, - vec![ - DirEntry { - path: "src".into(), - is_dir: true - }, - DirEntry { - path: "alpha.txt".into(), - is_dir: false - }, - DirEntry { - path: "zeta.txt".into(), - is_dir: false - }, - ] - ); - let _ = fs::remove_dir_all(&root); - } - - #[test] - fn missing_directory_shows_minimal_error() { - let root = unique_temp_dir("missing-controller"); - let error = scan_dir(&root.join("nope"), false, "").unwrap_err(); - - assert_eq!(completion_error(&error), "directory not found"); - - let _ = fs::remove_dir_all(root); - } - - fn entry(path: &str, is_dir: bool) -> DirEntry { - DirEntry { - path: path.into(), - is_dir, - } - } - - #[test] - fn update_filters_by_prefix_case_insensitively() { - let mut controller = CompletionController::new(); - // update() primes the prefix ("someth") and clears results pending a - // scan; inject stands in for the scan delivering. - controller.update("someth"); - controller.inject_items(vec![ - entry("Something.txt", false), - entry("other.md", false), - ]); - let state = controller.state().unwrap(); - assert_eq!(state.items.len(), 1); - assert_eq!(state.items[0].path, "Something.txt"); - - // Empty prefix shows everything again. - controller.update(""); - assert_eq!(controller.state().unwrap().items.len(), 2); - } - - #[test] - fn selection_clamps_when_filter_shrinks() { - let mut controller = CompletionController::new(); - controller.inject_items(vec![entry("a.txt", false), entry("b.txt", false)]); - controller.move_selection(5); - assert_eq!(controller.state().unwrap().selected, 1); - - controller.update("b"); - assert_eq!(controller.state().unwrap().selected, 0); - - controller.move_selection(-5); - assert_eq!(controller.state().unwrap().selected, 0); - } - - #[test] - fn accept_file_closes_and_dir_stays_open() { - let mut controller = CompletionController::new(); - controller.inject_items(vec![entry("src", true), entry("main.rs", false)]); - - let accepted = controller.accept().unwrap(); - assert_eq!( - accepted, - Accepted { - replacement: "src/".into(), - is_dir: true - } - ); - assert!(controller.is_open()); - - controller.move_selection(1); - let accepted = controller.accept().unwrap(); - assert_eq!( - accepted, - Accepted { - replacement: "main.rs".into(), - is_dir: false - } - ); - assert!(!controller.is_open()); - } - - #[test] - fn accept_prefixes_typed_directory_segment() { - let mut controller = CompletionController::new(); - // Token "@crates/al": directory segment "crates", prefix "al". - controller.update("crates/al"); - // The real scan of crates/ fails (missing directory); inject stands - // in for the scan delivering. - controller.inject_items(vec![entry("alan.rs", false)]); - - let accepted = controller.accept().unwrap(); - assert_eq!(accepted.replacement, "crates/alan.rs"); - } - - #[test] - fn poll_drops_stale_generations() { - let mut controller = CompletionController::new(); - controller.update("docs/"); - let fresh_generation = controller.generation; - let docs = PathBuf::from("docs"); - - controller - .tx - .send(Some(( - fresh_generation - 1, - docs.clone(), - Ok(vec![entry("stale.txt", false)]), - ))) - .unwrap(); - controller - .tx - .send(Some(( - fresh_generation, - docs.clone(), - Ok(vec![entry("fresh.txt", false)]), - ))) - .unwrap(); - assert_eq!(controller.poll(), Poll::Changed); - - let names: Vec<_> = controller - .state() - .unwrap() - .items - .iter() - .map(|e| e.path.clone()) - .collect(); - assert_eq!(names, vec!["fresh.txt"]); - assert_eq!(controller.poll(), Poll::Idle); - } - - #[test] - fn poll_drops_results_for_other_directories() { - let mut controller = CompletionController::new(); - controller.update("docs/"); - let generation = controller.generation; - - // The synchronous test path queues the real scan result immediately. - // Drain it so this test exercises only the deliberately wrong result. - let _ = controller.poll(); - controller - .tx - .send(Some(( - generation, - PathBuf::from("src"), - Ok(vec![entry("wrong.rs", false)]), - ))) - .unwrap(); - assert_eq!(controller.poll(), Poll::Idle); - assert!(controller.state().unwrap().items.is_empty()); - } - - /// Regression: parent and absolute paths cannot escape the workspace. - #[test] - fn rejects_paths_outside_workspace() { - let root = unique_temp_dir("root"); - let controller = CompletionController::with_root(root); - assert!(controller.resolve_relative_dir("../").is_none()); - assert!(controller.resolve_relative_dir("../../tmp").is_none()); - assert!(controller.resolve_relative_dir("/tmp").is_none()); - } - #[test] - fn rescan_only_when_directory_changes() { - let mut controller = CompletionController::new(); - controller.update("src/f"); - let first_generation = controller.generation; - - controller.update("src/fo"); - assert_eq!(controller.generation, first_generation); - - controller.update("docs/"); - assert_eq!(controller.generation, first_generation + 1); - } - - #[test] - fn dismiss_resets_scan_dedup() { - let mut controller = CompletionController::new(); - controller.update("src/"); - let generation = controller.generation; - - controller.dismiss(); - controller.update("src/"); - assert_eq!(controller.generation, generation + 1); - } - - #[test] - fn bare_token_matches_substring_of_nested_paths() { - let mut controller = CompletionController::new(); - controller.update("popup"); - controller.inject_items(vec![ - entry("Cargo.toml", false), - entry("crates/agent", true), - entry("crates/agent/src/agent.rs", false), - entry("crates/alan/src/views/components/popup.rs", false), - entry("crates/alan/src/views/components/header.rs", false), - ]); - - let names: Vec<_> = controller - .state() - .unwrap() - .items - .iter() - .map(|e| e.path.clone()) - .collect(); - assert_eq!(names, vec!["crates/alan/src/views/components/popup.rs"]); - } - - #[test] - fn bare_token_shows_directory_and_its_contents() { - let mut controller = CompletionController::new(); - controller.update("agent"); - controller.inject_items(vec![ - entry("Cargo.toml", false), - entry("crates/agent", true), - entry("crates/agent/src", true), - entry("crates/agent/src/agent.rs", false), - entry("crates/alan/src/main.rs", false), - ]); - - let names: Vec<_> = controller - .state() - .unwrap() - .items - .iter() - .map(|e| e.path.clone()) - .collect(); - assert_eq!( - names, - vec![ - "crates/agent", - "crates/agent/src", - "crates/agent/src/agent.rs", - ] - ); - } - - #[test] - fn bare_token_filename_prefix_match_ranks_with_substring() { - let mut controller = CompletionController::new(); - controller.update("main"); - controller.inject_items(vec![ - entry("domain.rs", false), - entry("crates/alan/src/main.rs", false), - ]); - - // Substring matching is intentional ("domain.rs" contains "main"); - // ordering follows the scan's shallow-first sort. - let names: Vec<_> = controller - .state() - .unwrap() - .items - .iter() - .map(|e| e.path.clone()) - .collect(); - assert_eq!(names, vec!["domain.rs", "crates/alan/src/main.rs"]); - } - - #[test] - fn recursive_scan_lists_nested_entries_with_relative_paths() { - let root = unique_temp_dir("recursive"); - fs::create_dir_all(root.join("crates/alan/src/views")).unwrap(); - fs::create_dir_all(root.join("crates/agent/src")).unwrap(); - fs::write(root.join("crates/alan/src/views/popup.rs"), "").unwrap(); - fs::write(root.join("crates/alan/src/main.rs"), "").unwrap(); - fs::write(root.join("crates/agent/src/lib.rs"), "").unwrap(); - fs::create_dir(root.join("target")).unwrap(); - - let entries = scan_dir(&root, true, "").unwrap(); - let paths: Vec<_> = entries.iter().map(|e| e.path.as_str()).collect(); - assert!(paths.contains(&"crates/alan/src/views")); - assert!(paths.contains(&"crates/alan/src/views/popup.rs")); - assert!(paths.contains(&"crates/agent/src/lib.rs")); - // Junk dirs are excluded everywhere in the tree. - assert!(!paths.iter().any(|p| p.contains("target"))); - // All paths are relative to the scan root. - assert!(paths.iter().all(|p| !p.starts_with("./"))); - - // A single-level scan of a subdirectory stays flat. - let flat = scan_dir(&root.join("crates/alan/src"), false, "").unwrap(); - let flat_paths: Vec<_> = flat.iter().map(|e| e.path.as_str()).collect(); - assert!(flat_paths.contains(&"main.rs")); - assert!(flat_paths.contains(&"views")); - assert!(!flat_paths.iter().any(|p| p.contains('/'))); - - let _ = fs::remove_dir_all(&root); - } - - /// Regression: the scan retains a bounded candidate set that can be - /// refiltered for prefixes typed after the scan starts. - #[test] - fn scan_truncates_at_scan_limit_before_filtering() { - let root = unique_temp_dir("trunc"); - // More than SCAN_LIMIT non-matching files to exhaust the old cap. - let count = SCAN_LIMIT + 50; - for i in 0..count { - fs::write(root.join(format!("filler_{i:04}.txt")), "").unwrap(); - } - // A file whose name matches the bare-token query `popup`, placed at - // the end so it would be truncated under the old implementation. - fs::write(root.join("popup_target.rs"), "").unwrap(); - - let entries = scan_dir(&root, true, "popup").unwrap(); - assert!(entries.iter().any(|e| e.path == "popup_target.rs")); - // The result set is bounded by the candidate budget. - assert!(entries.len() <= CANDIDATE_LIMIT); - - let _ = fs::remove_dir_all(&root); - } - - /// Regression: a scan whose epoch has been superseded bails instead of - /// producing results. `spawn_scan` bumps the shared epoch on every new - /// request and passes the per-task epoch to `scan_dir_with`; this keeps - /// a superseded traversal from finishing and delivering a stale result. - #[test] - fn scan_bails_when_epoch_superseded() { - let root = unique_temp_dir("cancel"); - fs::create_dir_all(root.join("deeply/nested/path")).unwrap(); - fs::write(root.join("deeply/nested/path/match.rs"), "").unwrap(); - fs::write(root.join("keep.rs"), "").unwrap(); - - // A token already bumped past this task's epoch: the traversal sees - // a stale epoch at its first directory and returns immediately. - let stale: ScanCancel = Arc::new(AtomicU64::new(2)); - let entries = scan_dir_with(&root, true, &stale, 1).unwrap(); - assert!(entries.is_empty()); - - // A matching epoch still scans normally. - let fresh: ScanCancel = Arc::new(AtomicU64::new(1)); - let entries = scan_dir_with(&root, true, &fresh, 1).unwrap(); - assert!(entries.iter().any(|e| e.path == "keep.rs")); - assert!( - entries - .iter() - .any(|e| e.path == "deeply/nested/path/match.rs") - ); - - let _ = fs::remove_dir_all(&root); - } -} diff --git a/crates/alan/src/core/completion/mod.rs b/crates/alan/src/core/completion/mod.rs new file mode 100644 index 0000000..7aed8e2 --- /dev/null +++ b/crates/alan/src/core/completion/mod.rs @@ -0,0 +1,294 @@ +//! Completion for the prompt editor. +//! +//! A [`CompletionBackend`] decides for itself whether a request is its own. +//! Ranking is not its concern: [`matcher`] orders every backend the same way. + +mod paths; + +use super::Poll; +use super::matcher; +use std::ops::Range; + +pub use paths::Paths; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct CompletionRequest { + pub line: String, + /// Byte offset of the cursor within `line`. + pub cursor: usize, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Backend { + FilePath, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct CompletionItem { + pub display: String, + /// Text substituted for [`CompletionResult::range`]. + pub replacement: String, + pub description: Option, + /// Directories keep the popup open so the user can drill deeper. + pub stay_open: bool, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum CompletionStatus { + Loading, + Ready, + Error(String), +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct CompletionResult { + pub backend: Backend, + /// Bytes of the line that accepting overwrites. + pub range: Range, + pub status: CompletionStatus, + /// Ranked best first. + pub items: Vec, +} + +pub trait CompletionBackend { + /// The completion offered at the cursor, or `None` when this backend has + /// nothing to do with the request. + fn complete(&self, request: &CompletionRequest) -> Option; + + /// Called when this backend becomes the active one. + fn refresh(&mut self) {} + + fn poll(&mut self) -> Poll { + Poll::Idle + } +} + +struct Active { + request: CompletionRequest, + result: CompletionResult, + selected: usize, +} + +pub struct CompletionController { + paths: Paths, + active: Option, +} + +impl CompletionController { + pub fn new() -> Self { + Self { + paths: Paths::new(), + active: None, + } + } + + #[cfg(test)] + pub(crate) fn with_paths(index: Vec) -> Self { + Self { + paths: Paths::with_index(index), + active: None, + } + } + + /// Re-evaluate after every editor change. + pub fn sync(&mut self, line: &str, cursor: usize) { + let request = CompletionRequest { + line: line.to_owned(), + cursor, + }; + let Some(result) = self.paths.complete(&request) else { + self.active = None; + return; + }; + // Becoming active is the one moment a backend's data is worth reading. + if self.active.as_ref().map(|active| active.result.backend) != Some(result.backend) { + self.paths.refresh(); + } + self.active = Some(Active { + request, + result, + selected: 0, + }); + } + + pub fn is_open(&self) -> bool { + self.active.is_some() + } + + pub fn item_count(&self) -> usize { + self.active + .as_ref() + .map_or(0, |active| active.result.items.len()) + } + + pub fn selected(&self) -> usize { + self.active.as_ref().map_or(0, |active| active.selected) + } + + pub fn status(&self) -> CompletionStatus { + self.active + .as_ref() + .map_or(CompletionStatus::Ready, |active| { + active.result.status.clone() + }) + } + + /// In rank order. + pub fn items(&self, start: usize, count: usize) -> &[CompletionItem] { + let Some(active) = self.active.as_ref() else { + return &[]; + }; + let start = start.min(active.result.items.len()); + let end = start.saturating_add(count).min(active.result.items.len()); + &active.result.items[start..end] + } + + pub fn dismiss(&mut self) { + self.active = None; + } + + pub fn move_selection(&mut self, delta: isize) { + let Some(active) = self.active.as_mut() else { + return; + }; + if active.result.items.is_empty() { + return; + } + let max = active.result.items.len() as isize - 1; + active.selected = (active.selected as isize + delta).clamp(0, max) as usize; + } + + /// The highlighted item and the byte range of the line it overwrites. + pub fn accept(&mut self) -> Option<(CompletionItem, Range)> { + let active = self.active.as_ref()?; + let item = active.result.items.get(active.selected)?.clone(); + let range = active.result.range.clone(); + if !item.stay_open { + self.active = None; + } + Some((item, range)) + } + + pub fn poll(&mut self) -> Poll { + let poll = self.paths.poll(); + if poll == Poll::Changed { + self.recompute(); + } + poll + } + + /// A backend whose data changed under an open popup answers again. + fn recompute(&mut self) { + let Some(request) = self.active.as_ref().map(|active| active.request.clone()) else { + return; + }; + let Some(result) = self.paths.complete(&request) else { + self.active = None; + return; + }; + if let Some(active) = self.active.as_mut() { + active.selected = active.selected.min(result.items.len().saturating_sub(1)); + active.result = result; + } + } +} + +/// Shared so no backend can invent its own ordering. +fn ranked_items(pattern: &str, candidates: &[String], item: F) -> Vec +where + F: Fn(usize, &str) -> CompletionItem, +{ + matcher::match_all(pattern, candidates) + .into_iter() + .map(|index| item(index, &candidates[index])) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + fn engine(index: &[&str]) -> CompletionController { + CompletionController::with_paths(index.iter().map(|path| (*path).to_owned()).collect()) + } + + fn displayed(engine: &CompletionController) -> Vec { + engine + .items(0, engine.item_count()) + .iter() + .map(|item| item.display.clone()) + .collect() + } + + #[test] + fn an_at_token_opens_completion_anywhere_in_the_line() { + let mut engine = engine(&["src/main.rs", "docs/"]); + engine.sync("explain @mai", 12); + + assert!(engine.is_open()); + assert_eq!(displayed(&engine), ["src/main.rs"]); + } + + #[test] + fn plain_text_closes_the_popup() { + let mut engine = engine(&["src/main.rs"]); + engine.sync("@src", 4); + engine.sync("hello", 5); + + assert!(!engine.is_open()); + } + + #[test] + fn selection_stays_inside_the_items() { + let mut engine = engine(&["a.txt", "b.txt"]); + engine.sync("@", 1); + assert_eq!(engine.item_count(), 2); + + engine.move_selection(50); + assert_eq!(engine.selected(), 1); + + engine.move_selection(-50); + assert_eq!(engine.selected(), 0); + } + + #[test] + fn accepting_reports_the_range_it_overwrites() { + let mut engine = engine(&["src/main.rs"]); + engine.sync("explain @mai", 12); + + let (item, range) = engine.accept().unwrap(); + assert_eq!(item.replacement, "src/main.rs"); + // The `@` at byte 8 is outside the range, so it survives. + assert_eq!(range, 9..12); + assert!(!engine.is_open()); + } + + #[test] + fn accepting_a_directory_keeps_the_popup_open() { + let mut engine = engine(&["crates/"]); + engine.sync("@crat", 5); + + let (item, _) = engine.accept().unwrap(); + assert!(item.stay_open); + assert!(engine.is_open()); + } + + #[test] + fn accepting_nothing_when_no_candidate_matched() { + let mut engine = engine(&["src/main.rs"]); + engine.sync("@zzz", 4); + + assert_eq!(engine.item_count(), 0); + assert!(engine.accept().is_none()); + } + + #[test] + fn items_are_bounded_by_the_window_asked_for() { + let mut engine = engine(&["a.txt", "b.txt", "c.txt"]); + engine.sync("@", 1); + + assert_eq!(engine.items(0, 2).len(), 2); + assert_eq!(engine.items(2, 5).len(), 1); + assert_eq!(engine.items(9, 5).len(), 0); + } +} diff --git a/crates/alan/src/core/completion/paths.rs b/crates/alan/src/core/completion/paths.rs new file mode 100644 index 0000000..62a84c7 --- /dev/null +++ b/crates/alan/src/core/completion/paths.rs @@ -0,0 +1,390 @@ +//! File-path completion. +//! +//! Typing `@` offers files and folders from one in-memory index of the +//! workspace, in which directories carry a trailing `/`. Scans run on the +//! blocking thread pool and land through a channel drained by +//! [`Paths::poll`], so the index is served stale rather than waited on. + +use super::{ + Backend, CompletionBackend, CompletionItem, CompletionRequest, CompletionResult, + CompletionStatus, ranked_items, +}; +use crate::core::Poll; +use std::io; +use std::path::{Component, Path, PathBuf}; +use std::sync::Arc; +use std::sync::atomic::{AtomicU64, Ordering}; +use tokio::sync::watch::{self, Receiver, Sender}; + +/// Maximum entries retained by one scan. +const CANDIDATE_LIMIT: usize = 5_000; +/// Maximum filesystem entries visited by one scan. +const VISIT_LIMIT: usize = 20_000; +/// Maximum recursive depth. +const MAX_SCAN_DEPTH: usize = 32; +/// Directories excluded from scans regardless of prefix. +const SKIPPED_DIRS: &[&str] = &[".git", "target", "node_modules"]; + +type ScanResults = io::Result>; + +/// Token shared with a scan task. Bumping it asks the traversal to bail early. +/// `Arc` is the cheapest cancellation primitive available on the +/// blocking thread pool: no `JoinHandle` polling, no runtime blocking. +type ScanCancel = Arc; + +pub struct Paths { + /// The whole workspace. Directories end in `/`. + index: Vec, + status: CompletionStatus, + root: PathBuf, + /// Staleness stamp bumped on every new scan request. + generation: u64, + /// Epoch token shared with a scan task so it can bail early. + cancel_epoch: ScanCancel, + tx: Sender>, + rx: Receiver>, +} + +impl Paths { + pub fn new() -> Self { + let root = std::env::current_dir() + .ok() + .and_then(|path| path.canonicalize().ok()) + .unwrap_or_else(|| PathBuf::from(".")); + let mut paths = Self::empty(root); + paths.refresh(); + paths + } + + fn empty(root: PathBuf) -> Self { + let (tx, rx) = watch::channel(None); + Self { + index: Vec::new(), + status: CompletionStatus::Loading, + root, + generation: 0, + cancel_epoch: Arc::new(AtomicU64::new(1)), + tx, + rx, + } + } + + #[cfg(test)] + pub(crate) fn with_index(index: Vec) -> Self { + let mut paths = Self::empty(PathBuf::from(".")); + paths.index = index; + paths.status = CompletionStatus::Ready; + paths + } +} + +impl CompletionBackend for Paths { + fn complete(&self, request: &CompletionRequest) -> Option { + let range = at_token(&request.line, request.cursor)?; + let pattern = &request.line[range.clone()]; + Some(CompletionResult { + backend: Backend::FilePath, + range, + status: self.status.clone(), + items: ranked_items(pattern, &self.index, |_, path| CompletionItem { + display: path.to_owned(), + replacement: path.to_owned(), + description: None, + stay_open: path.ends_with('/'), + }), + }) + } + + /// The previous index stays visible until the new one lands. + fn refresh(&mut self) { + self.generation += 1; + self.cancel_epoch.fetch_add(1, Ordering::Release); + if self.index.is_empty() { + self.status = CompletionStatus::Loading; + } + + let generation = self.generation; + let epoch = self.cancel_epoch.load(Ordering::Acquire); + let cancel = self.cancel_epoch.clone(); + let root = self.root.clone(); + let tx = self.tx.clone(); + let scan = move || { + let results = scan_dir(&root, &cancel, epoch); + let _ = tx.send(Some((generation, results))); + }; + match tokio::runtime::Handle::try_current() { + Ok(handle) => { + handle.spawn_blocking(scan); + } + Err(_) => scan(), + } + } + + fn poll(&mut self) -> Poll { + if !self.rx.has_changed().unwrap_or(false) { + return Poll::Idle; + } + let results = { + let received = self.rx.borrow_and_update(); + let Some((generation, results)) = received.as_ref() else { + return Poll::Idle; + }; + if *generation != self.generation { + return Poll::Idle; + } + match results { + Ok(index) => Ok(index.clone()), + Err(error) => Err(io::Error::new(error.kind(), error.to_string())), + } + }; + match results { + Ok(index) => { + self.index = index; + self.status = CompletionStatus::Ready; + } + Err(error) => self.status = CompletionStatus::Error(completion_error(&error)), + } + Poll::Changed + } +} + +/// Bytes of the `@` token under `cursor`, excluding the `@` so it survives +/// the replacement. A mention can sit anywhere in a prompt. +fn at_token(line: &str, cursor: usize) -> Option> { + let cursor = cursor.min(line.len()); + let start = line[..cursor] + .char_indices() + .rev() + .take_while(|&(_, character)| !character.is_whitespace()) + .map(|(index, _)| index) + .last()?; + let end = line[cursor..] + .find(char::is_whitespace) + .map_or(line.len(), |index| cursor + index); + line[start..end].starts_with('@').then_some(start + 1..end) +} + +/// Walk `root`, returning workspace-relative paths with `/` on directories. +/// +/// Cooperative: the traversal bails as soon as `cancel` no longer equals +/// `epoch`. +fn scan_dir(root: &Path, cancel: &ScanCancel, epoch: u64) -> ScanResults { + let mut builder = ignore::WalkBuilder::new(root); + builder + // `ignore` handles hidden files and .ignore/.gitignore files. Keep + // these application-level exclusions in addition to those filters. + .standard_filters(true) + .follow_links(false) + .min_depth(Some(1)) + .max_depth(Some(MAX_SCAN_DEPTH)) + .filter_entry(|entry| entry.depth() == 0 || !is_skipped_name(entry.file_name())); + + let mut index = Vec::new(); + for (visited, result) in builder.build().enumerate() { + if cancel.load(Ordering::Acquire) != epoch || visited >= VISIT_LIMIT { + break; + } + let entry = result.map_err(ignore_error)?; + + let Some(file_type) = entry.file_type() else { + continue; + }; + let Ok(relative) = entry.path().strip_prefix(root) else { + continue; + }; + let mut path = relative_path(relative); + if file_type.is_dir() { + path.push('/'); + } + index.push(path); + } + index.sort_by(|a, b| sort_paths(a, b)); + index.truncate(CANDIDATE_LIMIT); + Ok(index) +} + +/// The order shown before anything is typed. Any pattern overrides it. +fn sort_paths(a: &str, b: &str) -> std::cmp::Ordering { + fn depth(path: &str) -> usize { + path.trim_end_matches('/').matches('/').count() + } + depth(a) + .cmp(&depth(b)) + .then_with(|| b.ends_with('/').cmp(&a.ends_with('/'))) + .then_with(|| a.cmp(b)) +} + +fn is_skipped_name(name: &std::ffi::OsStr) -> bool { + SKIPPED_DIRS + .iter() + .any(|skipped| name == std::ffi::OsStr::new(skipped)) +} + +fn relative_path(path: &Path) -> String { + path.components() + .filter_map(|component| match component { + Component::Normal(name) => Some(name.to_string_lossy().into_owned()), + _ => None, + }) + .collect::>() + .join("/") +} + +fn ignore_error(error: ignore::Error) -> io::Error { + let kind = error + .io_error() + .map_or(io::ErrorKind::Other, io::Error::kind); + io::Error::new(kind, error.to_string()) +} + +fn completion_error(error: &io::Error) -> String { + match error.kind() { + io::ErrorKind::NotFound => "directory not found".into(), + _ => error.to_string(), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + + fn unique_temp_dir(label: &str) -> PathBuf { + let dir = + std::env::temp_dir().join(format!("alan-completion-{label}-{}", std::process::id())); + let _ = fs::remove_dir_all(&dir); + fs::create_dir_all(&dir).unwrap(); + dir + } + + fn scan(root: &Path) -> ScanResults { + scan_dir(root, &Arc::new(AtomicU64::new(1)), 1) + } + + #[test] + fn scan_sorts_dirs_first_and_skips_junk() { + let root = unique_temp_dir("scan"); + fs::create_dir(root.join(".git")).unwrap(); + fs::create_dir(root.join("target")).unwrap(); + fs::create_dir(root.join("src")).unwrap(); + fs::write(root.join("zeta.txt"), "").unwrap(); + fs::write(root.join("alpha.txt"), "").unwrap(); + + assert_eq!(scan(&root).unwrap(), ["src/", "alpha.txt", "zeta.txt"]); + + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn scan_lists_nested_entries_with_relative_paths() { + let root = unique_temp_dir("recursive"); + fs::create_dir_all(root.join("crates/alan/src/views")).unwrap(); + fs::create_dir_all(root.join("crates/agent/src")).unwrap(); + fs::write(root.join("crates/alan/src/views/popup.rs"), "").unwrap(); + fs::write(root.join("crates/agent/src/lib.rs"), "").unwrap(); + fs::create_dir(root.join("target")).unwrap(); + + let index = scan(&root).unwrap(); + + assert!(index.contains(&"crates/alan/src/views/".to_owned())); + assert!(index.contains(&"crates/alan/src/views/popup.rs".to_owned())); + assert!(index.contains(&"crates/agent/src/lib.rs".to_owned())); + // Junk dirs are excluded everywhere in the tree. + assert!(!index.iter().any(|path| path.contains("target"))); + // All paths are relative to the scan root. + assert!(!index.iter().any(|path| path.starts_with("./"))); + + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn missing_directory_shows_minimal_error() { + let root = unique_temp_dir("missing"); + let error = scan(&root.join("nope")).unwrap_err(); + + assert_eq!(completion_error(&error), "directory not found"); + + let _ = fs::remove_dir_all(root); + } + + /// A superseded traversal cannot deliver: it bails at the next entry. + #[test] + fn scan_bails_when_epoch_superseded() { + let root = unique_temp_dir("cancel"); + fs::create_dir_all(root.join("deeply/nested")).unwrap(); + fs::write(root.join("keep.rs"), "").unwrap(); + + let stale: ScanCancel = Arc::new(AtomicU64::new(2)); + assert!(scan_dir(&root, &stale, 1).unwrap().is_empty()); + + assert!(scan(&root).unwrap().contains(&"keep.rs".to_owned())); + + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn poll_drops_stale_generations() { + let mut paths = Paths::empty(PathBuf::from(".")); + paths.generation = 7; + + paths + .tx + .send(Some((6, Ok(vec!["stale.txt".into()])))) + .unwrap(); + paths + .tx + .send(Some((7, Ok(vec!["fresh.txt".into()])))) + .unwrap(); + + assert_eq!(paths.poll(), Poll::Changed); + assert_eq!(paths.index, ["fresh.txt"]); + assert_eq!(paths.poll(), Poll::Idle); + } + + #[test] + fn refresh_keeps_serving_the_previous_index() { + let mut paths = Paths::with_index(vec!["src/main.rs".into()]); + paths.refresh(); + + assert_eq!(paths.index, ["src/main.rs"]); + assert_eq!(paths.status, CompletionStatus::Ready); + } + + fn request(line: &str, cursor: usize) -> CompletionRequest { + CompletionRequest { + line: line.to_owned(), + cursor, + } + } + + #[test] + fn answers_only_for_an_at_token() { + let paths = Paths::with_index(Vec::new()); + + // The `@` sits at byte 8 and is deliberately outside the range. + let result = paths.complete(&request("explain @mai", 12)).unwrap(); + assert_eq!(result.range, 9..12); + + assert!(paths.complete(&request("explain this", 12)).is_none()); + assert!(paths.complete(&request("", 0)).is_none()); + } + + #[test] + fn directories_are_marked_by_a_trailing_slash() { + let paths = Paths::with_index(vec!["src/".into(), "main.rs".into()]); + let items = paths.complete(&request("@", 1)).unwrap().items; + + assert_eq!(items[0].display, "src/"); + assert!(items[0].stay_open); + assert!(!items[1].stay_open); + } + + #[test] + fn the_whole_token_is_replaced_from_inside_it() { + let paths = Paths::with_index(vec!["foobar".into()]); + let result = paths.complete(&request("@foo", 3)).unwrap(); + + assert_eq!(result.range, 1..4); + assert_eq!(result.items[0].replacement, "foobar"); + } +} diff --git a/crates/alan/src/core/matcher.rs b/crates/alan/src/core/matcher.rs new file mode 100644 index 0000000..d878eae --- /dev/null +++ b/crates/alan/src/core/matcher.rs @@ -0,0 +1,112 @@ +//! Pattern matching for completion candidates. + +/// Quality of a match. The derived ordering is the ranking, so reordering +/// these variants reorders every completion popup. +#[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Copy)] +enum Kind { + Exact, + Prefix, + Segment, + Contains, +} + +/// Indexes into `candidates` of everything matching `pattern`, best first. +pub fn match_all>(pattern: &str, candidates: &[S]) -> Vec { + if pattern.is_empty() { + return (0..candidates.len()).collect(); + } + let mut matched: Vec<(usize, (Kind, usize, usize))> = candidates + .iter() + .enumerate() + .filter_map(|(index, candidate)| Some((index, rank(pattern, candidate.as_ref())?))) + .collect(); + matched.sort_by_key(|&(_, rank)| rank); + matched.into_iter().map(|(index, _)| index).collect() +} + +/// Smaller is better in every field, so the tuple sorts best first. +fn rank(pattern: &str, candidate: &str) -> Option<(Kind, usize, usize)> { + let at = find(candidate, pattern)?; + // An exact match also starts at zero, so it has to be tested first. + let kind = if candidate.len() == pattern.len() { + Kind::Exact + } else if at == 0 { + Kind::Prefix + } else if at == segment_start(candidate) { + Kind::Segment + } else { + Kind::Contains + }; + Some((kind, at, candidate.len())) +} + +/// Byte-wise search is safe for UTF-8: an ASCII needle cannot match inside a +/// multi-byte sequence, whose bytes are all `>= 0x80`. +fn find(haystack: &str, needle: &str) -> Option { + let (haystack, needle) = (haystack.as_bytes(), needle.as_bytes()); + if needle.is_empty() { + // `windows(0)` panics. + return Some(0); + } + haystack + .windows(needle.len()) + .position(|window| window.eq_ignore_ascii_case(needle)) +} + +fn segment_start(candidate: &str) -> usize { + candidate.rfind('/').map_or(0, |slash| slash + 1) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn ranked<'a>(pattern: &str, candidates: &[&'a str]) -> Vec<&'a str> { + match_all(pattern, candidates) + .into_iter() + .map(|index| candidates[index]) + .collect() + } + + #[test] + fn empty_pattern_keeps_every_candidate_in_order() { + assert_eq!(ranked("", &["b", "a", "c"]), ["b", "a", "c"]); + } + + #[test] + fn drops_candidates_that_do_not_contain_the_pattern() { + assert_eq!(ranked("zz", &["alpha", "beta"]), Vec::<&str>::new()); + } + + #[test] + fn ranks_exact_then_prefix_then_segment_then_substring() { + let candidates = ["x/help", "helper", "help", "unhelpful"]; + assert_eq!( + ranked("help", &candidates), + ["help", "helper", "x/help", "unhelpful"] + ); + } + + /// Guards the order the [`Kind`] variants are declared in. + #[test] + fn match_kind_outranks_any_tie_break() { + let long_segment = format!("nested/{}", "a".repeat(300)); + let candidates = ["ba", long_segment.as_str()]; + assert_eq!(ranked("a", &candidates), [long_segment.as_str(), "ba"]); + } + + /// All three are segment matches, so only the tie-breaks separate them. + #[test] + fn tie_breaks_on_position_then_length() { + let candidates = ["crates/x/main.rs", "b/main_helper.rs", "a/main.rs"]; + assert_eq!( + ranked("main", &candidates), + ["a/main.rs", "b/main_helper.rs", "crates/x/main.rs"] + ); + } + + #[test] + fn ignores_ascii_case() { + assert_eq!(ranked("HeLp", &["/help"]), ["/help"]); + } +} diff --git a/crates/alan/src/core/mod.rs b/crates/alan/src/core/mod.rs index 9939d52..40df9ad 100644 --- a/crates/alan/src/core/mod.rs +++ b/crates/alan/src/core/mod.rs @@ -6,12 +6,11 @@ pub mod command; pub mod completion; pub mod controller; pub mod login; +pub mod matcher; pub use action::{Action, Command}; pub use chat::Entry; pub use command::SlashCommand; -#[cfg(test)] -pub use completion::DirEntry; -pub use completion::{CompletionController, CompletionState, CompletionStatus}; +pub use completion::{CompletionController, CompletionStatus}; pub use controller::{Controller, Overlay, Poll}; pub use login::LoginState; diff --git a/crates/alan/src/views/components/popup.rs b/crates/alan/src/views/components/popup.rs index dd40e7b..70daad1 100644 --- a/crates/alan/src/views/components/popup.rs +++ b/crates/alan/src/views/components/popup.rs @@ -1,4 +1,4 @@ -use crate::core::{CompletionState, Controller}; +use crate::core::{CompletionStatus, Controller}; use crate::views::UiState; use crate::views::component::Component; use crate::views::theme; @@ -8,8 +8,10 @@ use ratatui::style::Style; use ratatui::text::{Line, Span, Text}; use ratatui::widgets::{Block, Padding, Paragraph}; -/// Fixed number of rows in the completion popup. +/// Fixed height of the completion popup, including its padding. const POPUP_ROWS: u16 = 7; +/// Candidates visible inside that height. +const VISIBLE_ROWS: usize = 5; /// Generic list popup rendered above the editor cursor. Currently used for /// `@`-path completion; reusable for any short list anchored at the prompt. @@ -43,23 +45,19 @@ impl Component for PopupList { controller: &Controller, _state: &mut UiState, ) { - let Some(completion) = controller.completion().state() else { - return; - }; - let CompletionState { - items, - selected, - status, - } = completion; - if area.is_empty() { + let completion = controller.completion(); + if !completion.is_open() || area.is_empty() { return; } - if !matches!(status, crate::core::CompletionStatus::Ready) { - let message = match status { - crate::core::CompletionStatus::Loading => "Loading…".to_owned(), - crate::core::CompletionStatus::Error(error) => error.clone(), - crate::core::CompletionStatus::Ready => String::new(), - }; + let message = match completion.status() { + CompletionStatus::Loading => Some("Loading…".to_owned()), + CompletionStatus::Error(error) => Some(error), + CompletionStatus::Ready if completion.item_count() == 0 => { + Some("No matches".to_owned()) + } + CompletionStatus::Ready => None, + }; + if let Some(message) = message { frame.render_widget( Paragraph::new(message) .style(Style::default().bg(theme::EDITOR_BG)) @@ -68,46 +66,40 @@ impl Component for PopupList { ); return; } - if items.is_empty() { - frame.render_widget( - Paragraph::new("No matches") - .style(Style::default().bg(theme::EDITOR_BG)) - .block(Block::default().padding(Padding::new(2, 2, 1, 1))), - area, - ); - return; - } frame.render_widget(ratatui::widgets::Clear, area); + let selected = completion.selected(); let start = selected .saturating_sub(2) - .min(items.len().saturating_sub(5)); - let end = (start + 5).min(items.len()); + .min(completion.item_count().saturating_sub(VISIBLE_ROWS)); - let rows = items[start..end] + let rows = completion + .items(start, VISIBLE_ROWS) .iter() .enumerate() - .map(|(offset, entry)| { - let index = start + offset; - let (marker, marker_style) = if index == *selected { + .map(|(offset, item)| { + let (marker, marker_style) = if start + offset == selected { ("› ", Style::default().fg(theme::PROMPT_FG)) } else { (" ", Style::default()) }; - let name = if entry.is_dir { - format!("{}/", entry.path) - } else { - entry.path.clone() - }; - let name_style = if entry.is_dir { + // Directories carry a trailing `/`. + let label_style = if item.display.ends_with('/') { Style::default().fg(ratatui::style::Color::White) } else { Style::default().fg(theme::EDITOR_FG) }; - Line::from(vec![ + let mut spans = vec![ Span::styled(marker, marker_style), - Span::styled(name, name_style), - ]) + Span::styled(item.display.clone(), label_style), + ]; + if let Some(description) = &item.description { + spans.push(Span::styled( + format!(" {description}"), + Style::default().fg(theme::MUTED_FG), + )); + } + Line::from(spans) }) .collect::>(); frame.render_widget( diff --git a/crates/alan/src/views/mod.rs b/crates/alan/src/views/mod.rs index 8a7133e..27f19f0 100644 --- a/crates/alan/src/views/mod.rs +++ b/crates/alan/src/views/mod.rs @@ -136,7 +136,7 @@ impl UiState { } if let Event::Key(key) = &event && completion.is_open() - && (completion.has_items() || !matches!(key.code, KeyCode::Enter | KeyCode::Tab)) + && (completion.item_count() > 0 || !matches!(key.code, KeyCode::Enter | KeyCode::Tab)) && self.handle_completion_key(*key, completion) { return None; @@ -260,12 +260,12 @@ impl UiState { true } KeyCode::Enter | KeyCode::Tab if key.modifiers.is_empty() => { - let Some(accepted) = completion.accept() else { + let Some((item, range)) = completion.accept() else { return false; }; - self.replace_completion_token(&accepted.replacement); + self.replace_range(range, &item.replacement); self.dirty = true; - if accepted.is_dir { + if item.stay_open { self.sync_completion(completion); } true @@ -279,51 +279,21 @@ impl UiState { } } - fn replace_completion_token(&mut self, replacement: &str) { - let Some((start_col, end_col)) = self.token_span_containing_cursor() else { + /// Overwrite a byte range of the cursor's line. The editor addresses text + /// by character column, so the range is converted on the way in. + fn replace_range(&mut self, range: std::ops::Range, text: &str) { + let (row, _) = self.editor.cursor(); + let Some(line) = self.editor.lines().get(row) else { return; }; - let (row, _) = self.editor.cursor(); + let start_col = line[..range.start].chars().count(); + let chars = line[range].chars().count(); self.editor .move_cursor(CursorMove::Jump(row as u16, start_col as u16)); - self.editor.delete_str(end_col - start_col); - let text = format!("@{replacement}"); + self.editor.delete_str(chars); self.editor.insert_str(text); } - fn token_span_containing_cursor(&self) -> Option<(usize, usize)> { - self.completion_token_at_cursor() - .map(|(start, end, _)| (start, end)) - } - - /// Return the character-column span and text of the non-whitespace token - /// around the cursor when it starts with `@`. - fn completion_token_at_cursor(&self) -> Option<(usize, usize, String)> { - let (row, col) = self.editor.cursor(); - let line = self.editor.lines().get(row)?; - let col = col.min(line.chars().count()); - let at = Self::char_offset(line, col); - let start = line[..at] - .char_indices() - .rev() - .take_while(|&(_, c)| !c.is_whitespace()) - .map(|(i, _)| i) - .last()?; - let end = line[at..] - .char_indices() - .find(|&(_, c)| c.is_whitespace()) - .map(|(i, _)| at + i) - .unwrap_or(line.len()); - let text = &line[start..end]; - text.starts_with('@').then(|| { - ( - line[..start].chars().count(), - line[..end].chars().count(), - text[1..].to_owned(), - ) - }) - } - /// Convert a character-column index (as reported by `TextArea::cursor`) /// into a byte offset within `line`, clamped to the line length. fn char_offset(line: &str, col: usize) -> usize { @@ -333,30 +303,16 @@ impl UiState { .unwrap_or(line.len()) } - /// Read the `@token` at the cursor and keep the popup in step with it. + /// Which backend answers, and over what text, is the controller's call. fn sync_completion(&mut self, completion: &mut CompletionController) { let (row, col) = self.editor.cursor(); - let token = self.editor.lines().get(row).and_then(|line| { - let col = col.min(line.chars().count()); - let at = Self::char_offset(line, col); - let start = line[..at] - .char_indices() - .rev() - .take_while(|&(_, c)| !c.is_whitespace()) - .map(|(i, _)| i) - .last()?; - let end = line[at..] - .char_indices() - .find(|&(_, c)| c.is_whitespace()) - .map(|(i, _)| at + i) - .unwrap_or(line.len()); - let text = &line[start..end]; - text.starts_with('@').then(|| text[1..].to_owned()) - }); - match token { - Some(token) => completion.update(&token), - None => completion.dismiss(), - } + let line = self + .editor + .lines() + .get(row) + .map_or(String::new(), String::clone); + let cursor = Self::char_offset(&line, col.min(line.chars().count())); + completion.sync(&line, cursor); } fn handle_mouse_event( @@ -580,13 +536,11 @@ impl Default for UiState { } #[cfg(test)] -use crate::core::DirEntry; - #[cfg(test)] impl UiState { /// Test shim for the pre-completion call signature. fn handle_editor_event_for_test(&mut self, event: Event) -> Option { - let mut completion = CompletionController::new(); + let mut completion = CompletionController::with_paths(Vec::new()); self.handle_event(event, &[], &mut completion) } } @@ -1007,13 +961,12 @@ mod tests { assert!(state.take_dirty()); } - /// Drive the editor with a real CompletionController: typing `@` opens - /// the popup, injected scan results appear, and accepting replaces the - /// token in the editor without submitting. + /// Drive the editor with a real completion: typing `@` opens the popup + /// and accepting replaces the token without submitting. #[test] fn at_completion_opens_accepts_and_replaces_token() { let mut state = UiState::new(); - let mut completion = CompletionController::new(); + let mut completion = CompletionController::with_paths(vec!["something.txt".into()]); for character in "@som".chars() { state.handle_event( @@ -1024,12 +977,6 @@ mod tests { } assert!(completion.is_open()); - // Stand in for the blocking scan of the project root delivering. - completion.inject_items(vec![DirEntry { - path: "something.txt".into(), - is_dir: false, - }]); - let command = state.handle_event(key(KeyCode::Enter), &[], &mut completion); assert_eq!(command, None); @@ -1037,14 +984,13 @@ mod tests { assert!(!completion.is_open()); } - /// `TextArea::cursor()` reports the column in characters, but the token - /// span used to slice the line by byte offset. A multi-byte character - /// between `@` and the cursor used to panic with a byte-index out of - /// bounds. Regression test for that panic. + /// `TextArea::cursor()` reports columns in characters while the line is + /// sliced by byte offset, so a multi-byte character between `@` and the + /// cursor must not put the two out of step. #[test] fn at_completion_after_multibyte_char_does_not_panic() { let mut state = UiState::new(); - let mut completion = CompletionController::new(); + let mut completion = CompletionController::with_paths(vec!["éx.txt".into()]); // `@` preceded by other text and followed by a 2-byte character, // then more text — a common mid-sentence use of `@` mentions. @@ -1059,11 +1005,6 @@ mod tests { // Accepting should replace the whole `@éx` token without panicking // and without eating the `abc ` that precedes it. - completion.inject_items(vec![DirEntry { - path: "éx.txt".into(), - is_dir: false, - }]); - let command = state.handle_event(key(KeyCode::Enter), &[], &mut completion); assert_eq!(command, None); @@ -1074,23 +1015,13 @@ mod tests { #[test] fn at_completion_navigation_and_escape() { let mut state = UiState::new(); - let mut completion = CompletionController::new(); + let mut completion = CompletionController::with_paths(vec!["a.txt".into(), "b.txt".into()]); state.handle_event(key(KeyCode::Char('@')), &[], &mut completion); - completion.inject_items(vec![ - DirEntry { - path: "a.txt".into(), - is_dir: false, - }, - DirEntry { - path: "b.txt".into(), - is_dir: false, - }, - ]); - assert_eq!(completion.state().unwrap().selected, 0); + assert_eq!(completion.selected(), 0); state.handle_event(key(KeyCode::Down), &[], &mut completion); - assert_eq!(completion.state().unwrap().selected, 1); + assert_eq!(completion.selected(), 1); state.handle_event(key(KeyCode::Esc), &[], &mut completion); assert!(!completion.is_open()); @@ -1103,7 +1034,7 @@ mod tests { #[test] fn deleting_the_at_closes_the_popup() { let mut state = UiState::new(); - let mut completion = CompletionController::new(); + let mut completion = CompletionController::with_paths(vec!["a.txt".into()]); state.handle_event(key(KeyCode::Char('@')), &[], &mut completion); assert!(completion.is_open()); @@ -1115,30 +1046,23 @@ mod tests { #[test] fn accepting_a_directory_drills_down() { let mut state = UiState::new(); - let mut completion = CompletionController::new(); + let mut completion = CompletionController::with_paths(vec!["src/".into()]); state.handle_event(key(KeyCode::Char('@')), &[], &mut completion); - completion.inject_items(vec![DirEntry { - path: "src".into(), - is_dir: true, - }]); - state.handle_event(key(KeyCode::Enter), &[], &mut completion); assert_eq!(state.editor_text(), "@src/"); assert!(completion.is_open()); } - /// Regression: moving the cursor left inside the token before accepting - /// must replace the whole token, not just the prefix before the cursor. - /// - /// Accepting with the cursor at `@fo|o` used to delete only `@fo` and - /// insert the replacement, leaving the trailing `o` behind - /// (`@foobaroo`). The recorded span is now consumed at accept time. + /// Accepting at `@fo|o` replaces the whole token, not the prefix before + /// the cursor, so no trailing `o` survives. #[test] fn accepting_completion_with_cursor_inside_token_replaces_whole_token() { let mut state = UiState::new(); - let mut completion = CompletionController::new(); + // The path must match the typed prefix (`foo`) yet differ from it, + // so a leftover suffix would show. + let mut completion = CompletionController::with_paths(vec!["foobar".into()]); for character in "@foo".chars() { state.handle_event( @@ -1149,13 +1073,6 @@ mod tests { } assert!(completion.is_open()); - // The injected path must match the typed prefix (`foo`) to survive - // `refilter`, yet differ from it so a leftover suffix would show. - completion.inject_items(vec![DirEntry { - path: "foobar".into(), - is_dir: false, - }]); - // Move the cursor left twice: `@fo|o`. state.handle_event(key(KeyCode::Left), &[], &mut completion); state.handle_event(key(KeyCode::Left), &[], &mut completion); @@ -1173,7 +1090,7 @@ mod tests { #[test] fn accepting_completion_with_cursor_inside_multibyte_token() { let mut state = UiState::new(); - let mut completion = CompletionController::new(); + let mut completion = CompletionController::with_paths(vec!["éfoobar".into()]); for character in "@éfoo".chars() { state.handle_event( @@ -1184,11 +1101,6 @@ mod tests { } assert!(completion.is_open()); - completion.inject_items(vec![DirEntry { - path: "éfoobar".into(), - is_dir: false, - }]); - // Move the cursor left twice: `@éfo|o`. state.handle_event(key(KeyCode::Left), &[], &mut completion); state.handle_event(key(KeyCode::Left), &[], &mut completion); @@ -1205,7 +1117,7 @@ mod tests { #[test] fn accepting_completion_at_token_end_still_replaces() { let mut state = UiState::new(); - let mut completion = CompletionController::new(); + let mut completion = CompletionController::with_paths(vec!["foobar".into()]); for character in "@foo".chars() { state.handle_event( @@ -1214,10 +1126,6 @@ mod tests { &mut completion, ); } - completion.inject_items(vec![DirEntry { - path: "foobar".into(), - is_dir: false, - }]); let command = state.handle_event(key(KeyCode::Enter), &[], &mut completion); From 6bf43a531a3ae527325580ddee6c1030db524257 Mon Sep 17 00:00:00 2001 From: Azeem Shaik Date: Thu, 27 Aug 2026 18:08:55 +0530 Subject: [PATCH 02/13] Address review feedback on the completion refactor - Backends declare a trigger character and the controller keys them in a map, so the trait is dispatched rather than only declared - Token parsing moves into token.rs, shared by every backend; fixes a panic when the cursor sat inside a multi-byte character - Only one scan runs at a time, so the generation counter, cancel epoch and delivery channel are gone - Paths::new takes its root and no longer scans from the constructor - Scan limits collapse to MAX_INDEXED_PATHS and MAX_PATH_DEPTH, plus a cap on suggestions built per keystroke - @crates/main.rs matched nothing; pattern parts may now skip directories - Accepting a directory replaced it with itself forever, and left no separator so the next keystroke reopened the popup - One unreadable directory failed the entire scan --- crates/alan/src/core/completion/mod.rs | 176 +++++++--- crates/alan/src/core/completion/paths.rs | 421 +++++++++++++---------- crates/alan/src/core/completion/token.rs | 104 ++++++ crates/alan/src/core/controller.rs | 4 +- crates/alan/src/core/matcher.rs | 145 ++++++-- crates/alan/src/views/mod.rs | 97 ++++-- 6 files changed, 657 insertions(+), 290 deletions(-) create mode 100644 crates/alan/src/core/completion/token.rs diff --git a/crates/alan/src/core/completion/mod.rs b/crates/alan/src/core/completion/mod.rs index 7aed8e2..f145d77 100644 --- a/crates/alan/src/core/completion/mod.rs +++ b/crates/alan/src/core/completion/mod.rs @@ -1,26 +1,27 @@ //! Completion for the prompt editor. //! -//! A [`CompletionBackend`] decides for itself whether a request is its own. -//! Ranking is not its concern: [`matcher`] orders every backend the same way. +//! The character a token starts with picks the backend, so no backend parses +//! the line itself. Ranking is not their concern either: [`matcher`] orders +//! every backend the same way. mod paths; +mod token; use super::Poll; use super::matcher; +pub use paths::Paths; +use std::collections::HashMap; use std::ops::Range; -pub use paths::Paths; +/// How many matches one keystroke turns into popup items. Unlike the index +/// this costs nothing to miss: narrowing the pattern surfaces the rest. +const MAX_SUGGESTIONS: usize = 100; #[derive(Debug, Clone, PartialEq, Eq)] pub struct CompletionRequest { pub line: String, - /// Byte offset of the cursor within `line`. - pub cursor: usize, -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum Backend { - FilePath, + /// The token under the cursor, after its trigger character. + pub token: Range, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -29,8 +30,6 @@ pub struct CompletionItem { /// Text substituted for [`CompletionResult::range`]. pub replacement: String, pub description: Option, - /// Directories keep the popup open so the user can drill deeper. - pub stay_open: bool, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -42,7 +41,6 @@ pub enum CompletionStatus { #[derive(Debug, Clone, PartialEq, Eq)] pub struct CompletionResult { - pub backend: Backend, /// Bytes of the line that accepting overwrites. pub range: Range, pub status: CompletionStatus, @@ -51,8 +49,12 @@ pub struct CompletionResult { } pub trait CompletionBackend { - /// The completion offered at the cursor, or `None` when this backend has - /// nothing to do with the request. + /// The character a token must start with for this backend to answer. + fn trigger(&self) -> char; + + /// The completion offered at the cursor. `None` when the trigger matched + /// but this backend still does not apply, which closes the popup rather + /// than showing an empty one. fn complete(&self, request: &CompletionRequest) -> Option; /// Called when this backend becomes the active one. @@ -64,51 +66,73 @@ pub trait CompletionBackend { } struct Active { + /// Trigger of the backend that claimed the request, and so the key it is + /// filed under. + trigger: char, request: CompletionRequest, result: CompletionResult, selected: usize, } pub struct CompletionController { - paths: Paths, + backends: HashMap>, active: Option, } impl CompletionController { - pub fn new() -> Self { - Self { - paths: Paths::new(), - active: None, + /// Keyed by each backend's own [`CompletionBackend::trigger`], so the key + /// can never disagree with the backend filed under it. + /// + /// # Panics + /// + /// If two backends share a trigger. The list is written in source, so a + /// clash is a programming error with no sensible recovery: dropping one + /// silently would make completion mysteriously dead for that character. + pub fn new(backends: Vec>) -> Self { + let mut keyed = HashMap::with_capacity(backends.len()); + for backend in backends { + let trigger = backend.trigger(); + let clash = keyed.insert(trigger, backend).is_some(); + assert!( + !clash, + "two completion backends claim the trigger {trigger:?}" + ); } - } - - #[cfg(test)] - pub(crate) fn with_paths(index: Vec) -> Self { Self { - paths: Paths::with_index(index), + backends: keyed, active: None, } } /// Re-evaluate after every editor change. pub fn sync(&mut self, line: &str, cursor: usize) { + self.active = self.claim(line, cursor); + } + + /// Any step failing means no completion applies here: no token under the + /// cursor, no backend for its trigger, or the backend declining. + fn claim(&mut self, line: &str, cursor: usize) -> Option { + let token = token::at(line, cursor)?; + // Read before the backend is borrowed mutably. + let switching = self.active.as_ref().map(|active| active.trigger) != Some(token.trigger); + let backend = self.backends.get_mut(&token.trigger)?; + // Becoming active is the one moment a backend's data is worth reading. + if switching { + backend.refresh(); + } + let request = CompletionRequest { line: line.to_owned(), - cursor, - }; - let Some(result) = self.paths.complete(&request) else { - self.active = None; - return; + token: token.range, }; - // Becoming active is the one moment a backend's data is worth reading. - if self.active.as_ref().map(|active| active.result.backend) != Some(result.backend) { - self.paths.refresh(); - } - self.active = Some(Active { + + let result = backend.complete(&request)?; + Some(Active { + trigger: token.trigger, request, result, selected: 0, - }); + }) } pub fn is_open(&self) -> bool { @@ -163,26 +187,38 @@ impl CompletionController { let active = self.active.as_ref()?; let item = active.result.items.get(active.selected)?.clone(); let range = active.result.range.clone(); - if !item.stay_open { - self.active = None; - } + self.active = None; Some((item, range)) } pub fn poll(&mut self) -> Poll { - let poll = self.paths.poll(); + let poll = self + .backends + .values_mut() + .fold(Poll::Idle, |poll, backend| poll.combine(backend.poll())); + if poll == Poll::Changed { self.recompute(); } poll } - /// A backend whose data changed under an open popup answers again. + /// A backend whose data changed under an open popup answers again. The + /// request is unchanged, so the backend that claimed it still owns it. fn recompute(&mut self) { - let Some(request) = self.active.as_ref().map(|active| active.request.clone()) else { + let Some((trigger, request)) = self + .active + .as_ref() + .map(|active| (active.trigger, active.request.clone())) + else { return; }; - let Some(result) = self.paths.complete(&request) else { + + let Some(result) = self + .backends + .get(&trigger) + .and_then(|backend| backend.complete(&request)) + else { self.active = None; return; }; @@ -196,11 +232,12 @@ impl CompletionController { /// Shared so no backend can invent its own ordering. fn ranked_items(pattern: &str, candidates: &[String], item: F) -> Vec where - F: Fn(usize, &str) -> CompletionItem, + F: Fn(&str) -> CompletionItem, { - matcher::match_all(pattern, candidates) + matcher::rank_all(pattern, candidates) .into_iter() - .map(|index| item(index, &candidates[index])) + .take(MAX_SUGGESTIONS) + .map(|index| item(&candidates[index])) .collect() } @@ -209,7 +246,9 @@ mod tests { use super::*; fn engine(index: &[&str]) -> CompletionController { - CompletionController::with_paths(index.iter().map(|path| (*path).to_owned()).collect()) + CompletionController::new(vec![Box::new(Paths::with_index( + index.iter().map(|path| (*path).to_owned()).collect(), + ))]) } fn displayed(engine: &CompletionController) -> Vec { @@ -220,6 +259,25 @@ mod tests { .collect() } + /// A trigger no backend is filed under closes the popup, exactly as if + /// there were no token at all. + #[test] + fn an_unclaimed_trigger_opens_nothing() { + let mut engine = engine(&["src/main.rs"]); + engine.sync("/help", 5); + + assert!(!engine.is_open()); + } + + #[test] + #[should_panic(expected = "two completion backends claim the trigger")] + fn two_backends_cannot_share_a_trigger() { + CompletionController::new(vec![ + Box::new(Paths::with_index(Vec::new())), + Box::new(Paths::with_index(Vec::new())), + ]); + } + #[test] fn an_at_token_opens_completion_anywhere_in_the_line() { let mut engine = engine(&["src/main.rs", "docs/"]); @@ -263,14 +321,30 @@ mod tests { assert!(!engine.is_open()); } + /// A directory is a reference in its own right, so accepting one finishes. #[test] - fn accepting_a_directory_keeps_the_popup_open() { - let mut engine = engine(&["crates/"]); + fn accepting_a_directory_closes_the_popup() { + let mut engine = engine(&["crates/", "crates/alan/"]); engine.sync("@crat", 5); - let (item, _) = engine.accept().unwrap(); - assert!(item.stay_open); + let (item, range) = engine.accept().unwrap(); + assert_eq!(item.replacement, "crates/"); + assert_eq!(range, 1..5); + assert!(!engine.is_open()); + } + + /// Typing past the directory reopens the popup against the deeper paths. + #[test] + fn typing_past_a_directory_reopens_the_popup() { + let mut engine = engine(&["crates/", "crates/alan/main.rs"]); + engine.sync("@crates/", 8); + engine.accept(); + assert!(!engine.is_open()); + + engine.sync("@crates/m", 9); + assert!(engine.is_open()); + assert_eq!(displayed(&engine), ["crates/alan/main.rs"]); } #[test] diff --git a/crates/alan/src/core/completion/paths.rs b/crates/alan/src/core/completion/paths.rs index 62a84c7..29707e9 100644 --- a/crates/alan/src/core/completion/paths.rs +++ b/crates/alan/src/core/completion/paths.rs @@ -1,174 +1,163 @@ //! File-path completion. //! //! Typing `@` offers files and folders from one in-memory index of the -//! workspace, in which directories carry a trailing `/`. Scans run on the -//! blocking thread pool and land through a channel drained by -//! [`Paths::poll`], so the index is served stale rather than waited on. +//! workspace, in which directories carry a trailing `/`. The scan runs on the +//! blocking thread pool and its result is collected by [`Paths::poll`], so the +//! index is served stale rather than waited on. use super::{ - Backend, CompletionBackend, CompletionItem, CompletionRequest, CompletionResult, - CompletionStatus, ranked_items, + CompletionBackend, CompletionItem, CompletionRequest, CompletionResult, CompletionStatus, + ranked_items, }; use crate::core::Poll; +use futures_util::FutureExt; use std::io; use std::path::{Component, Path, PathBuf}; -use std::sync::Arc; -use std::sync::atomic::{AtomicU64, Ordering}; -use tokio::sync::watch::{self, Receiver, Sender}; - -/// Maximum entries retained by one scan. -const CANDIDATE_LIMIT: usize = 5_000; -/// Maximum filesystem entries visited by one scan. -const VISIT_LIMIT: usize = 20_000; -/// Maximum recursive depth. -const MAX_SCAN_DEPTH: usize = 32; +use tokio::task::JoinHandle; + +/// How many paths the index holds, and so how many are searchable at all. +/// A backstop, not a policy: past this the tail of the walk is missing and +/// those files can never be completed, so raise it if real workspaces reach it. +const MAX_INDEXED_PATHS: usize = 10_000; + +/// How deep the walk goes. Real source trees bottom out around seven. +const MAX_PATH_DEPTH: usize = 10; + /// Directories excluded from scans regardless of prefix. const SKIPPED_DIRS: &[&str] = &[".git", "target", "node_modules"]; type ScanResults = io::Result>; -/// Token shared with a scan task. Bumping it asks the traversal to bail early. -/// `Arc` is the cheapest cancellation primitive available on the -/// blocking thread pool: no `JoinHandle` polling, no runtime blocking. -type ScanCancel = Arc; - pub struct Paths { /// The whole workspace. Directories end in `/`. index: Vec, status: CompletionStatus, root: PathBuf, - /// Staleness stamp bumped on every new scan request. - generation: u64, - /// Epoch token shared with a scan task so it can bail early. - cancel_epoch: ScanCancel, - tx: Sender>, - rx: Receiver>, + /// The in-flight scan, which is also where its result arrives. At most one + /// runs at a time, so a delivered result is always a complete one. + scan: Option>, } impl Paths { - pub fn new() -> Self { - let root = std::env::current_dir() - .ok() - .and_then(|path| path.canonicalize().ok()) - .unwrap_or_else(|| PathBuf::from(".")); - let mut paths = Self::empty(root); - paths.refresh(); - paths - } - - fn empty(root: PathBuf) -> Self { - let (tx, rx) = watch::channel(None); + /// Completions are relative to `root`. The index starts empty and the + /// first scan is driven by [`CompletionBackend::refresh`] when the popup + /// opens, so constructing this touches no filesystem. + pub fn new(root: PathBuf) -> Self { Self { index: Vec::new(), status: CompletionStatus::Loading, root, - generation: 0, - cancel_epoch: Arc::new(AtomicU64::new(1)), - tx, - rx, + scan: None, } } + /// A ready index without a scan. Lives here rather than in a test module + /// because `index` and `status` are private to this one. #[cfg(test)] pub(crate) fn with_index(index: Vec) -> Self { - let mut paths = Self::empty(PathBuf::from(".")); - paths.index = index; - paths.status = CompletionStatus::Ready; - paths + Self { + index, + status: CompletionStatus::Ready, + ..Self::new(PathBuf::from(".")) + } + } + + /// A panicked scan reports finished, so a wedged backend recovers on the + /// next refresh rather than never scanning again. + fn scanning(&self) -> bool { + self.scan.as_ref().is_some_and(|scan| !scan.is_finished()) + } +} + +impl Default for Paths { + /// Rooted at the working directory, which is the workspace in practice. + /// Canonicalising it is the one filesystem call made outside a scan. + fn default() -> Self { + Self::new( + std::env::current_dir() + .ok() + .and_then(|path| path.canonicalize().ok()) + .unwrap_or_else(|| PathBuf::from(".")), + ) } } impl CompletionBackend for Paths { + fn trigger(&self) -> char { + '@' + } + + /// Always answers: the trigger was the only condition, and the controller + /// has already checked it. fn complete(&self, request: &CompletionRequest) -> Option { - let range = at_token(&request.line, request.cursor)?; - let pattern = &request.line[range.clone()]; + let pattern = &request.line[request.token.clone()]; Some(CompletionResult { - backend: Backend::FilePath, - range, + range: request.token.clone(), status: self.status.clone(), - items: ranked_items(pattern, &self.index, |_, path| CompletionItem { + items: ranked_items(pattern, &self.index, |path| CompletionItem { display: path.to_owned(), replacement: path.to_owned(), description: None, - stay_open: path.ends_with('/'), }), }) } - /// The previous index stays visible until the new one lands. + /// The previous index stays visible until the new one lands. A scan already + /// under way is left to finish rather than restarted, and without a runtime + /// to scan on the current index simply stands. fn refresh(&mut self) { - self.generation += 1; - self.cancel_epoch.fetch_add(1, Ordering::Release); + if self.scanning() { + return; + } + let Ok(runtime) = tokio::runtime::Handle::try_current() else { + return; + }; + if self.index.is_empty() { self.status = CompletionStatus::Loading; } - let generation = self.generation; - let epoch = self.cancel_epoch.load(Ordering::Acquire); - let cancel = self.cancel_epoch.clone(); let root = self.root.clone(); - let tx = self.tx.clone(); - let scan = move || { - let results = scan_dir(&root, &cancel, epoch); - let _ = tx.send(Some((generation, results))); - }; - match tokio::runtime::Handle::try_current() { - Ok(handle) => { - handle.spawn_blocking(scan); - } - Err(_) => scan(), - } + self.scan = Some(runtime.spawn_blocking(move || scan_dir(&root))); } fn poll(&mut self) -> Poll { - if !self.rx.has_changed().unwrap_or(false) { + let Some(mut scan) = self.scan.take() else { return Poll::Idle; - } - let results = { - let received = self.rx.borrow_and_update(); - let Some((generation, results)) = received.as_ref() else { - return Poll::Idle; - }; - if *generation != self.generation { - return Poll::Idle; - } - match results { - Ok(index) => Ok(index.clone()), - Err(error) => Err(io::Error::new(error.kind(), error.to_string())), - } }; - match results { - Ok(index) => { + let Some(finished) = (&mut scan).now_or_never() else { + // Still walking. Put it back for the next tick. + self.scan = Some(scan); + return Poll::Idle; + }; + + match finished { + Ok(Ok(index)) => { self.index = index; self.status = CompletionStatus::Ready; } - Err(error) => self.status = CompletionStatus::Error(completion_error(&error)), + Ok(Err(error)) => { + self.status = CompletionStatus::Error(match error.kind() { + io::ErrorKind::NotFound => "directory not found".into(), + _ => error.to_string(), + }); + } + // Panicked, or cancelled at shutdown: no new data, so nothing to + // report. An error status would hide a working index, because the + // popup renders a status message in place of its items. + Err(_) => return Poll::Idle, } Poll::Changed } } -/// Bytes of the `@` token under `cursor`, excluding the `@` so it survives -/// the replacement. A mention can sit anywhere in a prompt. -fn at_token(line: &str, cursor: usize) -> Option> { - let cursor = cursor.min(line.len()); - let start = line[..cursor] - .char_indices() - .rev() - .take_while(|&(_, character)| !character.is_whitespace()) - .map(|(index, _)| index) - .last()?; - let end = line[cursor..] - .find(char::is_whitespace) - .map_or(line.len(), |index| cursor + index); - line[start..end].starts_with('@').then_some(start + 1..end) -} - /// Walk `root`, returning workspace-relative paths with `/` on directories. /// -/// Cooperative: the traversal bails as soon as `cancel` no longer equals -/// `epoch`. -fn scan_dir(root: &Path, cancel: &ScanCancel, epoch: u64) -> ScanResults { +/// An unreadable root is fatal, an unreadable entry inside it is not: one +/// permission-denied folder must not cost the workspace its whole index. +fn scan_dir(root: &Path) -> ScanResults { + root.metadata()?; + let mut builder = ignore::WalkBuilder::new(root); builder // `ignore` handles hidden files and .ignore/.gitignore files. Keep @@ -176,15 +165,17 @@ fn scan_dir(root: &Path, cancel: &ScanCancel, epoch: u64) -> ScanResults { .standard_filters(true) .follow_links(false) .min_depth(Some(1)) - .max_depth(Some(MAX_SCAN_DEPTH)) + .max_depth(Some(MAX_PATH_DEPTH)) .filter_entry(|entry| entry.depth() == 0 || !is_skipped_name(entry.file_name())); let mut index = Vec::new(); - for (visited, result) in builder.build().enumerate() { - if cancel.load(Ordering::Acquire) != epoch || visited >= VISIT_LIMIT { + for result in builder.build() { + if index.len() >= MAX_INDEXED_PATHS { break; } - let entry = result.map_err(ignore_error)?; + let Ok(entry) = result else { + continue; + }; let Some(file_type) = entry.file_type() else { continue; @@ -199,7 +190,6 @@ fn scan_dir(root: &Path, cancel: &ScanCancel, epoch: u64) -> ScanResults { index.push(path); } index.sort_by(|a, b| sort_paths(a, b)); - index.truncate(CANDIDATE_LIMIT); Ok(index) } @@ -230,20 +220,6 @@ fn relative_path(path: &Path) -> String { .join("/") } -fn ignore_error(error: ignore::Error) -> io::Error { - let kind = error - .io_error() - .map_or(io::ErrorKind::Other, io::Error::kind); - io::Error::new(kind, error.to_string()) -} - -fn completion_error(error: &io::Error) -> String { - match error.kind() { - io::ErrorKind::NotFound => "directory not found".into(), - _ => error.to_string(), - } -} - #[cfg(test)] mod tests { use super::*; @@ -257,10 +233,6 @@ mod tests { dir } - fn scan(root: &Path) -> ScanResults { - scan_dir(root, &Arc::new(AtomicU64::new(1)), 1) - } - #[test] fn scan_sorts_dirs_first_and_skips_junk() { let root = unique_temp_dir("scan"); @@ -270,7 +242,7 @@ mod tests { fs::write(root.join("zeta.txt"), "").unwrap(); fs::write(root.join("alpha.txt"), "").unwrap(); - assert_eq!(scan(&root).unwrap(), ["src/", "alpha.txt", "zeta.txt"]); + assert_eq!(scan_dir(&root).unwrap(), ["src/", "alpha.txt", "zeta.txt"]); let _ = fs::remove_dir_all(&root); } @@ -284,7 +256,7 @@ mod tests { fs::write(root.join("crates/agent/src/lib.rs"), "").unwrap(); fs::create_dir(root.join("target")).unwrap(); - let index = scan(&root).unwrap(); + let index = scan_dir(&root).unwrap(); assert!(index.contains(&"crates/alan/src/views/".to_owned())); assert!(index.contains(&"crates/alan/src/views/popup.rs".to_owned())); @@ -297,94 +269,167 @@ mod tests { let _ = fs::remove_dir_all(&root); } + /// Also passes as root, where the folder is readable and simply gets + /// indexed: either way the readable files survive. + #[cfg(unix)] #[test] - fn missing_directory_shows_minimal_error() { + fn an_unreadable_directory_does_not_empty_the_index() { + use std::os::unix::fs::PermissionsExt; + + let root = unique_temp_dir("unreadable"); + fs::write(root.join("keep.rs"), "").unwrap(); + let locked = root.join("locked"); + fs::create_dir(&locked).unwrap(); + fs::set_permissions(&locked, fs::Permissions::from_mode(0o000)).unwrap(); + + let index = scan_dir(&root).expect("a locked folder is not a fatal error"); + assert!(index.contains(&"keep.rs".to_owned())); + + let _ = fs::set_permissions(&locked, fs::Permissions::from_mode(0o755)); + let _ = fs::remove_dir_all(&root); + } + + /// Poll until the in-flight scan lands, as the UI loop does every tick. + async fn drain(paths: &mut Paths) -> Poll { + for _ in 0..10_000 { + let poll = paths.poll(); + if poll != Poll::Idle { + return poll; + } + tokio::task::yield_now().await; + } + panic!("the scan never landed"); + } + + #[tokio::test] + async fn a_missing_root_shows_a_minimal_message() { let root = unique_temp_dir("missing"); - let error = scan(&root.join("nope")).unwrap_err(); + let mut paths = Paths::new(root.join("nope")); + + paths.refresh(); + assert_eq!(drain(&mut paths).await, Poll::Changed); - assert_eq!(completion_error(&error), "directory not found"); + assert_eq!( + paths.status, + CompletionStatus::Error("directory not found".into()) + ); let _ = fs::remove_dir_all(root); } - /// A superseded traversal cannot deliver: it bails at the next entry. - #[test] - fn scan_bails_when_epoch_superseded() { - let root = unique_temp_dir("cancel"); - fs::create_dir_all(root.join("deeply/nested")).unwrap(); - fs::write(root.join("keep.rs"), "").unwrap(); + /// At most one scan runs at a time, so every delivered result is complete. + #[tokio::test] + async fn refresh_leaves_a_running_scan_alone() { + let mut paths = Paths::new(PathBuf::from(".")); + // A task that never finishes stands in for a scan in flight. + paths.scan = Some(tokio::spawn(std::future::pending())); + let running = paths.scan.as_ref().unwrap().id(); - let stale: ScanCancel = Arc::new(AtomicU64::new(2)); - assert!(scan_dir(&root, &stale, 1).unwrap().is_empty()); + for _ in 0..5 { + paths.refresh(); + } - assert!(scan(&root).unwrap().contains(&"keep.rs".to_owned())); + assert_eq!( + paths.scan.as_ref().unwrap().id(), + running, + "a second scan was spawned alongside the running one" + ); + } - let _ = fs::remove_dir_all(&root); + /// A panicked scan reports finished, so the guard releases instead of + /// wedging the backend into never scanning again. + #[tokio::test] + async fn a_panicked_scan_does_not_wedge_the_guard() { + let mut paths = Paths::new(PathBuf::from(".")); + paths.scan = Some(tokio::spawn(async { panic!("scan blew up") })); + // Let the task run and unwind. + tokio::task::yield_now().await; + + assert!(!paths.scanning()); + // The previous index survives and the popup is not told anything. + assert_eq!(paths.poll(), Poll::Idle); } - #[test] - fn poll_drops_stale_generations() { - let mut paths = Paths::empty(PathBuf::from(".")); - paths.generation = 7; - - paths - .tx - .send(Some((6, Ok(vec!["stale.txt".into()])))) - .unwrap(); - paths - .tx - .send(Some((7, Ok(vec!["fresh.txt".into()])))) - .unwrap(); - - assert_eq!(paths.poll(), Poll::Changed); + #[tokio::test] + async fn poll_installs_a_delivered_index_once() { + let mut paths = Paths::new(PathBuf::from(".")); + paths.scan = Some(tokio::spawn(async { Ok(vec!["fresh.txt".to_owned()]) })); + + assert_eq!(drain(&mut paths).await, Poll::Changed); assert_eq!(paths.index, ["fresh.txt"]); assert_eq!(paths.poll(), Poll::Idle); } - #[test] - fn refresh_keeps_serving_the_previous_index() { - let mut paths = Paths::with_index(vec!["src/main.rs".into()]); + #[tokio::test] + async fn refresh_keeps_serving_the_previous_index() { + let root = unique_temp_dir("previous"); + let mut paths = Paths { + index: vec!["src/main.rs".into()], + status: CompletionStatus::Ready, + ..Paths::new(root.clone()) + }; + paths.refresh(); assert_eq!(paths.index, ["src/main.rs"]); assert_eq!(paths.status, CompletionStatus::Ready); - } - fn request(line: &str, cursor: usize) -> CompletionRequest { - CompletionRequest { - line: line.to_owned(), - cursor, - } + let _ = fs::remove_dir_all(&root); } - #[test] - fn answers_only_for_an_at_token() { - let paths = Paths::with_index(Vec::new()); + /// The whole chain a scan result travels: typing opens the popup on an + /// empty index, the walk lands on the blocking pool, and the popup refills + /// in place. + #[tokio::test] + async fn a_scan_reaches_an_open_popup_through_the_controller() { + use crate::core::completion::CompletionController; - // The `@` sits at byte 8 and is deliberately outside the range. - let result = paths.complete(&request("explain @mai", 12)).unwrap(); - assert_eq!(result.range, 9..12); + let root = unique_temp_dir("end-to-end"); + fs::create_dir(root.join("src")).unwrap(); + fs::write(root.join("src/main.rs"), "").unwrap(); - assert!(paths.complete(&request("explain this", 12)).is_none()); - assert!(paths.complete(&request("", 0)).is_none()); - } + let mut completion = CompletionController::new(vec![Box::new(Paths::new(root.clone()))]); - #[test] - fn directories_are_marked_by_a_trailing_slash() { - let paths = Paths::with_index(vec!["src/".into(), "main.rs".into()]); - let items = paths.complete(&request("@", 1)).unwrap().items; + // Typing `@mai` opens the popup and starts the scan. Nothing to show + // yet, because the index is still empty. + completion.sync("@mai", 4); + assert!(completion.is_open()); + assert_eq!(completion.item_count(), 0); - assert_eq!(items[0].display, "src/"); - assert!(items[0].stay_open); - assert!(!items[1].stay_open); + for _ in 0..10_000 { + if completion.poll() == Poll::Changed { + break; + } + tokio::task::yield_now().await; + } + + assert_eq!( + completion.item_count(), + 1, + "the scan never reached the popup" + ); + assert_eq!(completion.items(0, 1)[0].display, "src/main.rs"); + // Drained exactly once. + assert_eq!(completion.poll(), Poll::Idle); + + let _ = fs::remove_dir_all(&root); + } + + fn request(line: &str, token: std::ops::Range) -> CompletionRequest { + CompletionRequest { + line: line.to_owned(), + token, + } } + /// Finding the token is the controller's job, so this backend answers + /// whatever range it is handed and overwrites exactly that. #[test] - fn the_whole_token_is_replaced_from_inside_it() { - let paths = Paths::with_index(vec!["foobar".into()]); - let result = paths.complete(&request("@foo", 3)).unwrap(); + fn the_handed_token_is_what_gets_replaced() { + let paths = Paths::with_index(vec!["src/main.rs".into()]); + let result = paths.complete(&request("explain @mai", 9..12)).unwrap(); - assert_eq!(result.range, 1..4); - assert_eq!(result.items[0].replacement, "foobar"); + assert_eq!(result.range, 9..12); + assert_eq!(result.items[0].replacement, "src/main.rs"); } } diff --git a/crates/alan/src/core/completion/token.rs b/crates/alan/src/core/completion/token.rs new file mode 100644 index 0000000..8e6d92b --- /dev/null +++ b/crates/alan/src/core/completion/token.rs @@ -0,0 +1,104 @@ +//! The token under the cursor. +//! +//! Splitting the line is deliberately no backend's job: doing it once here is +//! what lets a trigger character select a backend, and keeps the byte-offset +//! handling in a single place rather than repeated per trigger. + +use std::ops::Range; + +/// A whitespace-delimited token, split at its first character. +pub struct Token { + /// First character: what selects the backend. + pub trigger: char, + /// Everything after the trigger. The trigger always survives the + /// replacement, so no backend does offset arithmetic. + pub range: Range, +} + +/// The token under `cursor`, or `None` when the cursor is not inside one. +/// Sitting directly before a token does not count: `abc |@src` is not a +/// mention yet. +pub fn at(line: &str, cursor: usize) -> Option { + let cursor = floor_char_boundary(line, cursor); + let start = line[..cursor] + .char_indices() + .rev() + .take_while(|&(_, character)| !character.is_whitespace()) + .map(|(index, _)| index) + .last()?; + let end = line[cursor..] + .find(char::is_whitespace) + .map_or(line.len(), |index| cursor + index); + // `start < end`, so the token has at least its trigger character. + let trigger = line[start..end].chars().next()?; + Some(Token { + trigger, + range: start + trigger.len_utf8()..end, + }) +} + +/// The cursor arrives as a raw byte offset, and slicing a `str` off a char +/// boundary panics. +fn floor_char_boundary(line: &str, cursor: usize) -> usize { + let mut cursor = cursor.min(line.len()); + // Terminates: byte 0 is always a boundary. + while !line.is_char_boundary(cursor) { + cursor -= 1; + } + cursor +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_token_is_split_at_its_trigger() { + let token = at("explain @mai", 12).unwrap(); + + assert_eq!(token.trigger, '@'); + // The `@` at byte 8 is outside the range, so it survives replacement. + assert_eq!(token.range, 9..12); + } + + /// The cursor sitting inside the token still yields the whole of it. + #[test] + fn the_whole_token_is_taken_from_inside_it() { + assert_eq!(at("@foo", 3).unwrap().range, 1..4); + } + + /// A bare trigger is an empty pattern, not the absence of a token. + #[test] + fn a_lone_trigger_is_still_a_token() { + let token = at("@", 1).unwrap(); + + assert_eq!(token.trigger, '@'); + assert_eq!(token.range, 1..1); + } + + #[test] + fn there_is_no_token_in_empty_space_or_before_one() { + assert!(at("", 0).is_none()); + // The cursor is before the `@`, so it is not inside the token yet. + assert!(at("abc @src", 4).is_none()); + } + + /// The cursor arrives as a raw byte offset and the line is sliced by byte, + /// so an offset inside a character must not panic. + #[test] + fn a_cursor_off_a_char_boundary_does_not_panic() { + // `@é` is 3 bytes: byte 2 lands inside the `é`, byte 99 past the end. + for cursor in [2, 99] { + assert_eq!(at("@é", cursor).unwrap().trigger, '@'); + } + } + + /// A multi-byte trigger is measured in bytes, not characters. + #[test] + fn a_multibyte_trigger_is_excluded_by_its_own_width() { + let token = at("émai", 4).unwrap(); + + assert_eq!(token.trigger, 'é'); + assert_eq!(token.range, 2..5); + } +} diff --git a/crates/alan/src/core/controller.rs b/crates/alan/src/core/controller.rs index b803de5..8cf522a 100644 --- a/crates/alan/src/core/controller.rs +++ b/crates/alan/src/core/controller.rs @@ -3,7 +3,7 @@ use super::action::Command; use super::chat::{ChatController, Entry}; use super::command::SlashCommand; -use super::completion::CompletionController; +use super::completion::{CompletionController, Paths}; use super::login::{LoginController, LoginState}; use agent::Agent; use llm::Usage; @@ -65,7 +65,7 @@ impl Controller { Self { chat: ChatController::new(agent), login: LoginController::new(providers, credentials), - completion: CompletionController::new(), + completion: CompletionController::new(vec![Box::new(Paths::default())]), overlay: Overlay::None, } } diff --git a/crates/alan/src/core/matcher.rs b/crates/alan/src/core/matcher.rs index d878eae..e4b0ae8 100644 --- a/crates/alan/src/core/matcher.rs +++ b/crates/alan/src/core/matcher.rs @@ -1,21 +1,41 @@ -//! Pattern matching for completion candidates. +//! Ranking candidates against a typed pattern. +//! +//! A pattern is a path fragment. Its `/`-separated parts must appear in the +//! candidate in order, but may skip whole directories, so `crates/main.rs` +//! finds `crates/alan/src/main.rs`. Matching ignores ASCII case. -/// Quality of a match. The derived ordering is the ranking, so reordering -/// these variants reorders every completion popup. +/// How good a match is. Lower sorts first, and the field order is the +/// tie-break order, so rearranging either reorders every completion popup. +#[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Copy)] +struct Rank { + kind: Kind, + /// Byte offset of the match. Earlier beats later. + at: usize, + /// Length of the candidate. Shorter wins an otherwise equal match. + length: usize, +} + +/// Where in the candidate the match landed. #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Copy)] enum Kind { + /// The candidate is the pattern. Exact, + /// The candidate begins with it. Prefix, - Segment, + /// The candidate's own name begins with it: `main` in `src/main.rs`. + Name, + /// Anywhere else. Contains, } /// Indexes into `candidates` of everything matching `pattern`, best first. -pub fn match_all>(pattern: &str, candidates: &[S]) -> Vec { +/// +/// Every function here takes the pattern before the text it is matched against. +pub fn rank_all>(pattern: &str, candidates: &[S]) -> Vec { if pattern.is_empty() { return (0..candidates.len()).collect(); } - let mut matched: Vec<(usize, (Kind, usize, usize))> = candidates + let mut matched: Vec<(usize, Rank)> = candidates .iter() .enumerate() .filter_map(|(index, candidate)| Some((index, rank(pattern, candidate.as_ref())?))) @@ -24,37 +44,59 @@ pub fn match_all>(pattern: &str, candidates: &[S]) -> Vec { matched.into_iter().map(|(index, _)| index).collect() } -/// Smaller is better in every field, so the tuple sorts best first. -fn rank(pattern: &str, candidate: &str) -> Option<(Kind, usize, usize)> { - let at = find(candidate, pattern)?; - // An exact match also starts at zero, so it has to be tested first. - let kind = if candidate.len() == pattern.len() { +fn rank(pattern: &str, candidate: &str) -> Option { + // Where the candidate's own name starts, a directory's trailing `/` aside. + let name = candidate + .trim_end_matches('/') + .rfind('/') + .map_or(0, |slash| slash + 1); + + // What was typed is usually the name, so a match there beats one in the + // directories above it: `tool` means `tools/src/tool.rs`, not `tools/`. + let at = match match_start(pattern, &candidate[name..]) { + Some(offset) => name + offset, + None => match_start(pattern, candidate)?, + }; + + // Equal lengths do not imply equal text, since a pattern may skip segments. + let kind = if candidate.eq_ignore_ascii_case(pattern) { Kind::Exact } else if at == 0 { Kind::Prefix - } else if at == segment_start(candidate) { - Kind::Segment + } else if at == name { + Kind::Name } else { Kind::Contains }; - Some((kind, at, candidate.len())) + Some(Rank { + kind, + at, + length: candidate.len(), + }) } -/// Byte-wise search is safe for UTF-8: an ASCII needle cannot match inside a -/// multi-byte sequence, whose bytes are all `>= 0x80`. -fn find(haystack: &str, needle: &str) -> Option { - let (haystack, needle) = (haystack.as_bytes(), needle.as_bytes()); - if needle.is_empty() { - // `windows(0)` panics. - return Some(0); +/// Byte offset in `candidate` where the first of `pattern`'s parts matches, +/// once every part has been found in order. `None` if any part is missing. +fn match_start(pattern: &str, candidate: &str) -> Option { + let mut start = None; + let mut from = 0; + for part in pattern.split('/').filter(|part| !part.is_empty()) { + let at = find_from(part, candidate, from)?; + start.get_or_insert(at); + from = at + part.len(); } - haystack - .windows(needle.len()) - .position(|window| window.eq_ignore_ascii_case(needle)) + start } -fn segment_start(candidate: &str) -> usize { - candidate.rfind('/').map_or(0, |slash| slash + 1) +/// Byte offset of `part` in `candidate` at or after `from`, ignoring ASCII +/// case. Byte-wise because lowercasing both sides would allocate for every +/// candidate on every keystroke, and slicing bytes cannot land off a `char` +/// boundary. +fn find_from(part: &str, candidate: &str, from: usize) -> Option { + candidate.as_bytes()[from..] + .windows(part.len()) + .position(|window| window.eq_ignore_ascii_case(part.as_bytes())) + .map(|at| from + at) } #[cfg(test)] @@ -62,7 +104,7 @@ mod tests { use super::*; fn ranked<'a>(pattern: &str, candidates: &[&'a str]) -> Vec<&'a str> { - match_all(pattern, candidates) + rank_all(pattern, candidates) .into_iter() .map(|index| candidates[index]) .collect() @@ -79,7 +121,7 @@ mod tests { } #[test] - fn ranks_exact_then_prefix_then_segment_then_substring() { + fn ranks_exact_then_prefix_then_name_then_anywhere() { let candidates = ["x/help", "helper", "help", "unhelpful"]; assert_eq!( ranked("help", &candidates), @@ -95,7 +137,7 @@ mod tests { assert_eq!(ranked("a", &candidates), [long_segment.as_str(), "ba"]); } - /// All three are segment matches, so only the tie-breaks separate them. + /// All three match on the name, so only the tie-breaks separate them. #[test] fn tie_breaks_on_position_then_length() { let candidates = ["crates/x/main.rs", "b/main_helper.rs", "a/main.rs"]; @@ -109,4 +151,49 @@ mod tests { fn ignores_ascii_case() { assert_eq!(ranked("HeLp", &["/help"]), ["/help"]); } + + /// A path typed from memory skips the segments in between. + #[test] + fn pattern_segments_may_skip_directories() { + let candidates = ["crates/alan/src/main.rs", "crates/agent/src/lib.rs"]; + + assert_eq!( + ranked("crates/main.rs", &candidates), + ["crates/alan/src/main.rs"] + ); + assert_eq!( + ranked("alan/main", &candidates), + ["crates/alan/src/main.rs"] + ); + } + + /// Skipping segments is not the same as ignoring their order. + #[test] + fn pattern_segments_must_appear_in_order() { + let candidates = ["crates/alan/src/main.rs"]; + + assert_eq!(ranked("main/crates", &candidates), Vec::<&str>::new()); + } + + /// The name is what was typed, not the directory it happens to repeat in. + #[test] + fn a_name_match_outranks_the_same_text_in_the_directory_path() { + let candidates = ["crates/tools/src/fs.rs", "crates/tools/src/tool.rs"]; + + assert_eq!( + ranked("tool", &candidates), + ["crates/tools/src/tool.rs", "crates/tools/src/fs.rs"] + ); + } + + /// A directory's own name is its last segment, trailing slash aside. + #[test] + fn a_directory_matches_on_its_own_name() { + let candidates = ["crates/tools/", "crates/tools/src/args.rs"]; + + assert_eq!( + ranked("tools", &candidates), + ["crates/tools/", "crates/tools/src/args.rs"] + ); + } } diff --git a/crates/alan/src/views/mod.rs b/crates/alan/src/views/mod.rs index 27f19f0..ae8e40c 100644 --- a/crates/alan/src/views/mod.rs +++ b/crates/alan/src/views/mod.rs @@ -263,11 +263,14 @@ impl UiState { let Some((item, range)) = completion.accept() else { return false; }; + let separate = self.needs_separator_after(range.end); self.replace_range(range, &item.replacement); - self.dirty = true; - if item.stay_open { - self.sync_completion(completion); + // An accepted mention is finished. Without a separator the next + // keystroke lands inside the token and reopens the popup. + if separate { + self.editor.insert_str(" "); } + self.dirty = true; true } KeyCode::Esc => { @@ -279,6 +282,17 @@ impl UiState { } } + /// Whether byte `at` on the cursor's line is not already followed by + /// whitespace, so an accepted completion needs one adding. + fn needs_separator_after(&self, at: usize) -> bool { + let (row, _) = self.editor.cursor(); + self.editor + .lines() + .get(row) + .and_then(|line| line.get(at..)) + .is_none_or(|rest| !rest.starts_with(char::is_whitespace)) + } + /// Overwrite a byte range of the cursor's line. The editor addresses text /// by character column, so the range is converted on the way in. fn replace_range(&mut self, range: std::ops::Range, text: &str) { @@ -535,12 +549,21 @@ impl Default for UiState { } } +/// A controller over a fixed index, so tests never touch the filesystem. #[cfg(test)] +fn completion_with(index: &[&str]) -> CompletionController { + use crate::core::completion::Paths; + + CompletionController::new(vec![Box::new(Paths::with_index( + index.iter().map(|path| (*path).to_owned()).collect(), + ))]) +} + #[cfg(test)] impl UiState { /// Test shim for the pre-completion call signature. fn handle_editor_event_for_test(&mut self, event: Event) -> Option { - let mut completion = CompletionController::with_paths(Vec::new()); + let mut completion = completion_with(&[]); self.handle_event(event, &[], &mut completion) } } @@ -966,7 +989,7 @@ mod tests { #[test] fn at_completion_opens_accepts_and_replaces_token() { let mut state = UiState::new(); - let mut completion = CompletionController::with_paths(vec!["something.txt".into()]); + let mut completion = completion_with(&["something.txt"]); for character in "@som".chars() { state.handle_event( @@ -980,7 +1003,7 @@ mod tests { let command = state.handle_event(key(KeyCode::Enter), &[], &mut completion); assert_eq!(command, None); - assert_eq!(state.editor_text(), "@something.txt"); + assert_eq!(state.editor_text(), "@something.txt "); assert!(!completion.is_open()); } @@ -990,7 +1013,7 @@ mod tests { #[test] fn at_completion_after_multibyte_char_does_not_panic() { let mut state = UiState::new(); - let mut completion = CompletionController::with_paths(vec!["éx.txt".into()]); + let mut completion = completion_with(&["éx.txt"]); // `@` preceded by other text and followed by a 2-byte character, // then more text — a common mid-sentence use of `@` mentions. @@ -1008,14 +1031,14 @@ mod tests { let command = state.handle_event(key(KeyCode::Enter), &[], &mut completion); assert_eq!(command, None); - assert_eq!(state.editor_text(), "abc @éx.txt"); + assert_eq!(state.editor_text(), "abc @éx.txt "); assert!(!completion.is_open()); } #[test] fn at_completion_navigation_and_escape() { let mut state = UiState::new(); - let mut completion = CompletionController::with_paths(vec!["a.txt".into(), "b.txt".into()]); + let mut completion = completion_with(&["a.txt", "b.txt"]); state.handle_event(key(KeyCode::Char('@')), &[], &mut completion); assert_eq!(completion.selected(), 0); @@ -1034,7 +1057,7 @@ mod tests { #[test] fn deleting_the_at_closes_the_popup() { let mut state = UiState::new(); - let mut completion = CompletionController::with_paths(vec!["a.txt".into()]); + let mut completion = completion_with(&["a.txt"]); state.handle_event(key(KeyCode::Char('@')), &[], &mut completion); assert!(completion.is_open()); @@ -1043,16 +1066,50 @@ mod tests { assert!(!completion.is_open()); } + /// A directory is a reference in its own right: accepting one inserts it + /// and finishes. #[test] - fn accepting_a_directory_drills_down() { + fn accepting_a_directory_inserts_it_and_closes() { let mut state = UiState::new(); - let mut completion = CompletionController::with_paths(vec!["src/".into()]); + let mut completion = completion_with(&["src/"]); state.handle_event(key(KeyCode::Char('@')), &[], &mut completion); state.handle_event(key(KeyCode::Enter), &[], &mut completion); - assert_eq!(state.editor_text(), "@src/"); - assert!(completion.is_open()); + assert_eq!(state.editor_text(), "@src/ "); + assert!(!completion.is_open()); + } + + /// The separator is what keeps the next keystroke out of the token. + #[test] + fn typing_after_accepting_is_prose_not_another_mention() { + let mut state = UiState::new(); + let mut completion = completion_with(&["src/"]); + + state.handle_event(key(KeyCode::Char('@')), &[], &mut completion); + state.handle_event(key(KeyCode::Enter), &[], &mut completion); + state.handle_event(key(KeyCode::Char('h')), &[], &mut completion); + + assert_eq!(state.editor_text(), "@src/ h"); + assert!(!completion.is_open()); + } + + /// Text already follows the token, so it does not need separating twice. + #[test] + fn accepting_mid_sentence_does_not_double_the_space() { + let mut state = UiState::new(); + let mut completion = completion_with(&["src/main.rs"]); + + for character in "@mai and more".chars() { + state.handle_event(key(KeyCode::Char(character)), &[], &mut completion); + } + // Back inside the `@mai` token: `@mai| and more`. + for _ in 0.." and more".len() { + state.handle_event(key(KeyCode::Left), &[], &mut completion); + } + state.handle_event(key(KeyCode::Enter), &[], &mut completion); + + assert_eq!(state.editor_text(), "@src/main.rs and more"); } /// Accepting at `@fo|o` replaces the whole token, not the prefix before @@ -1062,7 +1119,7 @@ mod tests { let mut state = UiState::new(); // The path must match the typed prefix (`foo`) yet differ from it, // so a leftover suffix would show. - let mut completion = CompletionController::with_paths(vec!["foobar".into()]); + let mut completion = completion_with(&["foobar"]); for character in "@foo".chars() { state.handle_event( @@ -1081,7 +1138,7 @@ mod tests { let command = state.handle_event(key(KeyCode::Enter), &[], &mut completion); assert_eq!(command, None); - assert_eq!(state.editor_text(), "@foobar"); + assert_eq!(state.editor_text(), "@foobar "); assert!(!completion.is_open()); } @@ -1090,7 +1147,7 @@ mod tests { #[test] fn accepting_completion_with_cursor_inside_multibyte_token() { let mut state = UiState::new(); - let mut completion = CompletionController::with_paths(vec!["éfoobar".into()]); + let mut completion = completion_with(&["éfoobar"]); for character in "@éfoo".chars() { state.handle_event( @@ -1109,7 +1166,7 @@ mod tests { let command = state.handle_event(key(KeyCode::Enter), &[], &mut completion); assert_eq!(command, None); - assert_eq!(state.editor_text(), "@éfoobar"); + assert_eq!(state.editor_text(), "@éfoobar "); assert!(!completion.is_open()); } @@ -1117,7 +1174,7 @@ mod tests { #[test] fn accepting_completion_at_token_end_still_replaces() { let mut state = UiState::new(); - let mut completion = CompletionController::with_paths(vec!["foobar".into()]); + let mut completion = completion_with(&["foobar"]); for character in "@foo".chars() { state.handle_event( @@ -1130,7 +1187,7 @@ mod tests { let command = state.handle_event(key(KeyCode::Enter), &[], &mut completion); assert_eq!(command, None); - assert_eq!(state.editor_text(), "@foobar"); + assert_eq!(state.editor_text(), "@foobar "); assert!(!completion.is_open()); } } From 7e6bd3c9022ee4aece1cf4ccf12e26cacc1098d1 Mon Sep 17 00:00:00 2001 From: Azeem Shaik Date: Thu, 27 Aug 2026 21:30:17 +0530 Subject: [PATCH 03/13] Highlight the selected completion row rather than directories - Brightness now tracks what Enter will take; the trailing `/` already says an entry is a directory - Pull the row styling out of the render loop into item_line --- crates/alan/src/views/components/popup.rs | 55 ++++++++++++----------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/crates/alan/src/views/components/popup.rs b/crates/alan/src/views/components/popup.rs index 70daad1..e76312b 100644 --- a/crates/alan/src/views/components/popup.rs +++ b/crates/alan/src/views/components/popup.rs @@ -1,3 +1,4 @@ +use crate::core::completion::CompletionItem; use crate::core::{CompletionStatus, Controller}; use crate::views::UiState; use crate::views::component::Component; @@ -73,37 +74,14 @@ impl Component for PopupList { .saturating_sub(2) .min(completion.item_count().saturating_sub(VISIBLE_ROWS)); - let rows = completion + let lines = completion .items(start, VISIBLE_ROWS) .iter() .enumerate() - .map(|(offset, item)| { - let (marker, marker_style) = if start + offset == selected { - ("› ", Style::default().fg(theme::PROMPT_FG)) - } else { - (" ", Style::default()) - }; - // Directories carry a trailing `/`. - let label_style = if item.display.ends_with('/') { - Style::default().fg(ratatui::style::Color::White) - } else { - Style::default().fg(theme::EDITOR_FG) - }; - let mut spans = vec![ - Span::styled(marker, marker_style), - Span::styled(item.display.clone(), label_style), - ]; - if let Some(description) = &item.description { - spans.push(Span::styled( - format!(" {description}"), - Style::default().fg(theme::MUTED_FG), - )); - } - Line::from(spans) - }) + .map(|(offset, item)| item_line(item, start + offset == selected)) .collect::>(); frame.render_widget( - Paragraph::new(Text::from(rows)) + Paragraph::new(Text::from(lines)) .style(Style::default().bg(theme::EDITOR_BG)) .block(Block::default().padding(Padding::new(2, 2, 1, 1))), area, @@ -111,6 +89,31 @@ impl Component for PopupList { } } +/// One popup line: selection marker, candidate, and its description. +/// +/// Brightness marks what the next Enter will take, so a trailing `/` is left +/// to say on its own that an entry is a directory. +fn item_line(item: &CompletionItem, is_selected: bool) -> Line<'static> { + // The marker is blank unless selected, so it can always carry the accent. + let (marker, label) = if is_selected { + ("› ", theme::SELECTION_FG) + } else { + (" ", theme::EDITOR_FG) + }; + + let mut spans = vec![ + Span::styled(marker, Style::default().fg(theme::PROMPT_FG)), + Span::styled(item.display.clone(), Style::default().fg(label)), + ]; + if let Some(description) = &item.description { + spans.push(Span::styled( + format!(" {description}"), + Style::default().fg(theme::MUTED_FG), + )); + } + Line::from(spans) +} + #[cfg(test)] mod tests { use super::*; From 78ebac0b14e64217699363202dc2c8aa763d6619 Mon Sep 17 00:00:00 2001 From: Azeem Shaik Date: Thu, 27 Aug 2026 21:45:41 +0530 Subject: [PATCH 04/13] Tidy the completion module boundary - matcher moves inside completion/, where its only caller lives, and stops being crate-public - Re-export CompletionItem from core alongside the other completion types - Drop CompletionItem::description: nothing sets it, so it can come back with the slash-command backend that will - replace_range bails instead of panicking if a range outlives its line --- crates/alan/src/core/{ => completion}/matcher.rs | 0 crates/alan/src/core/completion/mod.rs | 3 +-- crates/alan/src/core/completion/paths.rs | 1 - crates/alan/src/core/mod.rs | 3 +-- crates/alan/src/views/components/popup.rs | 16 ++++------------ crates/alan/src/views/mod.rs | 12 ++++++++++-- 6 files changed, 16 insertions(+), 19 deletions(-) rename crates/alan/src/core/{ => completion}/matcher.rs (100%) diff --git a/crates/alan/src/core/matcher.rs b/crates/alan/src/core/completion/matcher.rs similarity index 100% rename from crates/alan/src/core/matcher.rs rename to crates/alan/src/core/completion/matcher.rs diff --git a/crates/alan/src/core/completion/mod.rs b/crates/alan/src/core/completion/mod.rs index f145d77..9dd5e86 100644 --- a/crates/alan/src/core/completion/mod.rs +++ b/crates/alan/src/core/completion/mod.rs @@ -4,11 +4,11 @@ //! the line itself. Ranking is not their concern either: [`matcher`] orders //! every backend the same way. +mod matcher; mod paths; mod token; use super::Poll; -use super::matcher; pub use paths::Paths; use std::collections::HashMap; use std::ops::Range; @@ -29,7 +29,6 @@ pub struct CompletionItem { pub display: String, /// Text substituted for [`CompletionResult::range`]. pub replacement: String, - pub description: Option, } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/crates/alan/src/core/completion/paths.rs b/crates/alan/src/core/completion/paths.rs index 29707e9..e8a12d7 100644 --- a/crates/alan/src/core/completion/paths.rs +++ b/crates/alan/src/core/completion/paths.rs @@ -97,7 +97,6 @@ impl CompletionBackend for Paths { items: ranked_items(pattern, &self.index, |path| CompletionItem { display: path.to_owned(), replacement: path.to_owned(), - description: None, }), }) } diff --git a/crates/alan/src/core/mod.rs b/crates/alan/src/core/mod.rs index 40df9ad..ad4887a 100644 --- a/crates/alan/src/core/mod.rs +++ b/crates/alan/src/core/mod.rs @@ -6,11 +6,10 @@ pub mod command; pub mod completion; pub mod controller; pub mod login; -pub mod matcher; pub use action::{Action, Command}; pub use chat::Entry; pub use command::SlashCommand; -pub use completion::{CompletionController, CompletionStatus}; +pub use completion::{CompletionController, CompletionItem, CompletionStatus}; pub use controller::{Controller, Overlay, Poll}; pub use login::LoginState; diff --git a/crates/alan/src/views/components/popup.rs b/crates/alan/src/views/components/popup.rs index e76312b..1b684b6 100644 --- a/crates/alan/src/views/components/popup.rs +++ b/crates/alan/src/views/components/popup.rs @@ -1,5 +1,4 @@ -use crate::core::completion::CompletionItem; -use crate::core::{CompletionStatus, Controller}; +use crate::core::{CompletionItem, CompletionStatus, Controller}; use crate::views::UiState; use crate::views::component::Component; use crate::views::theme; @@ -89,7 +88,7 @@ impl Component for PopupList { } } -/// One popup line: selection marker, candidate, and its description. +/// One popup line: the selection marker and the candidate. /// /// Brightness marks what the next Enter will take, so a trailing `/` is left /// to say on its own that an entry is a directory. @@ -101,17 +100,10 @@ fn item_line(item: &CompletionItem, is_selected: bool) -> Line<'static> { (" ", theme::EDITOR_FG) }; - let mut spans = vec![ + Line::from(vec![ Span::styled(marker, Style::default().fg(theme::PROMPT_FG)), Span::styled(item.display.clone(), Style::default().fg(label)), - ]; - if let Some(description) = &item.description { - spans.push(Span::styled( - format!(" {description}"), - Style::default().fg(theme::MUTED_FG), - )); - } - Line::from(spans) + ]) } #[cfg(test)] diff --git a/crates/alan/src/views/mod.rs b/crates/alan/src/views/mod.rs index ae8e40c..0f33237 100644 --- a/crates/alan/src/views/mod.rs +++ b/crates/alan/src/views/mod.rs @@ -300,8 +300,16 @@ impl UiState { let Some(line) = self.editor.lines().get(row) else { return; }; - let start_col = line[..range.start].chars().count(); - let chars = line[range].chars().count(); + // The range was measured against this line, so it fits. Bail rather + // than panic if that ever stops being true. + let Some(before) = line.get(..range.start) else { + return; + }; + let Some(replaced) = line.get(range) else { + return; + }; + let start_col = before.chars().count(); + let chars = replaced.chars().count(); self.editor .move_cursor(CursorMove::Jump(row as u16, start_col as u16)); self.editor.delete_str(chars); From 5de6697fa911dabaf06f614a3392271b8ba9a2f0 Mon Sep 17 00:00:00 2001 From: Azeem Shaik Date: Thu, 27 Aug 2026 22:08:54 +0530 Subject: [PATCH 05/13] Hand backends the pattern instead of the whole line - CompletionRequest carries `pattern` and `range` rather than the line and a token span, so no backend slices and the same span has one name - Fix Event::Paste syncing completion before inserting, which left the popup shut after pasting a mention --- crates/alan/src/core/completion/mod.rs | 12 +++++++----- crates/alan/src/core/completion/paths.rs | 19 +++++++++---------- crates/alan/src/views/mod.rs | 14 +++++++++++++- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/crates/alan/src/core/completion/mod.rs b/crates/alan/src/core/completion/mod.rs index 9dd5e86..7806aeb 100644 --- a/crates/alan/src/core/completion/mod.rs +++ b/crates/alan/src/core/completion/mod.rs @@ -19,9 +19,10 @@ const MAX_SUGGESTIONS: usize = 100; #[derive(Debug, Clone, PartialEq, Eq)] pub struct CompletionRequest { - pub line: String, - /// The token under the cursor, after its trigger character. - pub token: Range, + /// What was typed after the trigger character. + pub pattern: String, + /// Bytes of the line the pattern occupies, which accepting overwrites. + pub range: Range, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -120,9 +121,10 @@ impl CompletionController { backend.refresh(); } + // Slicing the line happens here, once, rather than in every backend. let request = CompletionRequest { - line: line.to_owned(), - token: token.range, + pattern: line[token.range.clone()].to_owned(), + range: token.range, }; let result = backend.complete(&request)?; diff --git a/crates/alan/src/core/completion/paths.rs b/crates/alan/src/core/completion/paths.rs index e8a12d7..d4ed903 100644 --- a/crates/alan/src/core/completion/paths.rs +++ b/crates/alan/src/core/completion/paths.rs @@ -90,11 +90,10 @@ impl CompletionBackend for Paths { /// Always answers: the trigger was the only condition, and the controller /// has already checked it. fn complete(&self, request: &CompletionRequest) -> Option { - let pattern = &request.line[request.token.clone()]; Some(CompletionResult { - range: request.token.clone(), + range: request.range.clone(), status: self.status.clone(), - items: ranked_items(pattern, &self.index, |path| CompletionItem { + items: ranked_items(&request.pattern, &self.index, |path| CompletionItem { display: path.to_owned(), replacement: path.to_owned(), }), @@ -414,19 +413,19 @@ mod tests { let _ = fs::remove_dir_all(&root); } - fn request(line: &str, token: std::ops::Range) -> CompletionRequest { + fn request(pattern: &str, range: std::ops::Range) -> CompletionRequest { CompletionRequest { - line: line.to_owned(), - token, + pattern: pattern.to_owned(), + range, } } - /// Finding the token is the controller's job, so this backend answers - /// whatever range it is handed and overwrites exactly that. + /// Locating the token is the controller's job, so this backend ranks the + /// pattern it is handed and overwrites exactly the range it came with. #[test] - fn the_handed_token_is_what_gets_replaced() { + fn the_handed_range_is_what_gets_replaced() { let paths = Paths::with_index(vec!["src/main.rs".into()]); - let result = paths.complete(&request("explain @mai", 9..12)).unwrap(); + let result = paths.complete(&request("mai", 9..12)).unwrap(); assert_eq!(result.range, 9..12); assert_eq!(result.items[0].replacement, "src/main.rs"); diff --git a/crates/alan/src/views/mod.rs b/crates/alan/src/views/mod.rs index c856498..fcac764 100644 --- a/crates/alan/src/views/mod.rs +++ b/crates/alan/src/views/mod.rs @@ -246,9 +246,9 @@ impl UiState { } Event::Mouse(mouse) => self.handle_mouse_event(mouse, rendered_lines), Event::Paste(text) => { + self.editor.insert_str(text); self.dirty = true; self.sync_completion(completion); - self.editor.insert_str(text); None } event => { @@ -931,6 +931,18 @@ mod tests { assert_eq!(state.editor_text(), "first\nsecond"); } + /// Completion syncs against the text after the paste, not before it. + #[test] + fn pasting_a_mention_opens_the_popup() { + let mut state = UiState::new(); + let mut completion = completion_with(&["alpha.txt"]); + + state.handle_event(Event::Paste("@alp".into()), &[], &mut completion); + + assert_eq!(state.editor_text(), "@alp"); + assert!(completion.is_open()); + } + fn ctrl(code: char) -> crossterm::event::Event { crossterm::event::Event::Key(crossterm::event::KeyEvent::new( crossterm::event::KeyCode::Char(code), From 942ae6107463a335d98db079e398ad98d4ed2a84 Mon Sep 17 00:00:00 2001 From: Revantark Date: Fri, 28 Aug 2026 11:29:14 +0530 Subject: [PATCH 06/13] avoid string allocation --- crates/alan/src/views/mod.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/crates/alan/src/views/mod.rs b/crates/alan/src/views/mod.rs index fcac764..70674f1 100644 --- a/crates/alan/src/views/mod.rs +++ b/crates/alan/src/views/mod.rs @@ -370,11 +370,7 @@ impl UiState { /// Which backend answers, and over what text, is the controller's call. fn sync_completion(&mut self, completion: &mut CompletionController) { let (row, col) = self.editor.cursor(); - let line = self - .editor - .lines() - .get(row) - .map_or(String::new(), String::clone); + let line = self.editor.lines().get(row).map_or("", String::as_str); let cursor = Self::char_offset(&line, col.min(line.chars().count())); completion.sync(&line, cursor); } From 71e0e2662bea18616b81083906e3e4f15cc05124 Mon Sep 17 00:00:00 2001 From: Revantark Date: Fri, 28 Aug 2026 11:31:52 +0530 Subject: [PATCH 07/13] fix clippy warnings --- crates/alan/src/views/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/alan/src/views/mod.rs b/crates/alan/src/views/mod.rs index 70674f1..6969b50 100644 --- a/crates/alan/src/views/mod.rs +++ b/crates/alan/src/views/mod.rs @@ -371,8 +371,8 @@ impl UiState { fn sync_completion(&mut self, completion: &mut CompletionController) { let (row, col) = self.editor.cursor(); let line = self.editor.lines().get(row).map_or("", String::as_str); - let cursor = Self::char_offset(&line, col.min(line.chars().count())); - completion.sync(&line, cursor); + let cursor = Self::char_offset(line, col.min(line.chars().count())); + completion.sync(line, cursor); } fn handle_mouse_event( From 633081e23b698033c6e8a579aec786ce157ca421 Mon Sep 17 00:00:00 2001 From: Azeem Shaik Date: Fri, 28 Aug 2026 12:21:05 +0530 Subject: [PATCH 08/13] Rank a slashed pattern on its last part `@src/s` ranked every `.rs` file above `skill.rs`, because the name check looked for the whole pattern inside the name. `src` never appears in a filename, so nothing ever matched on its name and the trailing `s` of `.rs` satisfied the rest. The last part is the name being typed; the parts before it only locate it. --- crates/alan/src/core/completion/matcher.rs | 32 ++++++++++++++++------ 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/crates/alan/src/core/completion/matcher.rs b/crates/alan/src/core/completion/matcher.rs index e4b0ae8..7b29c9f 100644 --- a/crates/alan/src/core/completion/matcher.rs +++ b/crates/alan/src/core/completion/matcher.rs @@ -45,25 +45,24 @@ pub fn rank_all>(pattern: &str, candidates: &[S]) -> Vec { } fn rank(pattern: &str, candidate: &str) -> Option { + let at = match_start(pattern, candidate)?; + // Where the candidate's own name starts, a directory's trailing `/` aside. let name = candidate .trim_end_matches('/') .rfind('/') .map_or(0, |slash| slash + 1); - - // What was typed is usually the name, so a match there beats one in the - // directories above it: `tool` means `tools/src/tool.rs`, not `tools/`. - let at = match match_start(pattern, &candidate[name..]) { - Some(offset) => name + offset, - None => match_start(pattern, candidate)?, - }; + // The last part is the name being typed; the parts before it only say where + // to look. `src/s` means a name starting with `s`, not any path under `src` + // that happens to contain one. + let typed = pattern.rsplit('/').find(|part| !part.is_empty())?; // Equal lengths do not imply equal text, since a pattern may skip segments. let kind = if candidate.eq_ignore_ascii_case(pattern) { Kind::Exact } else if at == 0 { Kind::Prefix - } else if at == name { + } else if starts_ignoring_case(&candidate[name..], typed) { Kind::Name } else { Kind::Contains @@ -75,6 +74,11 @@ fn rank(pattern: &str, candidate: &str) -> Option { }) } +fn starts_ignoring_case(text: &str, prefix: &str) -> bool { + text.len() >= prefix.len() + && text.as_bytes()[..prefix.len()].eq_ignore_ascii_case(prefix.as_bytes()) +} + /// Byte offset in `candidate` where the first of `pattern`'s parts matches, /// once every part has been found in order. `None` if any part is missing. fn match_start(pattern: &str, candidate: &str) -> Option { @@ -175,6 +179,18 @@ mod tests { assert_eq!(ranked("main/crates", &candidates), Vec::<&str>::new()); } + /// The last part is the name being typed; the parts before it only locate + /// it. Every `.rs` file ends in `s`, which must not count as a name match. + #[test] + fn the_last_pattern_part_ranks_against_the_name() { + let candidates = ["crates/llm/src/lib.rs", "crates/agent/src/skill.rs"]; + + assert_eq!( + ranked("src/s", &candidates), + ["crates/agent/src/skill.rs", "crates/llm/src/lib.rs"] + ); + } + /// The name is what was typed, not the directory it happens to repeat in. #[test] fn a_name_match_outranks_the_same_text_in_the_directory_path() { From bacfc8dd9a3679d4c34922c56780a44de44925d4 Mon Sep 17 00:00:00 2001 From: Azeem Shaik Date: Fri, 28 Aug 2026 12:51:47 +0530 Subject: [PATCH 09/13] Break ranking ties on length before position MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `@src/s` put skill.rs third behind sse.rs and selection.rs. Every candidate matched on its name, so only the tie-break separated them, and the first field compared was the offset of the pattern's first part — effectively "how few directories precede src", which says nothing about match quality. Comparing length first is what fzf does by default. --- crates/alan/src/core/completion/matcher.rs | 30 ++++++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/crates/alan/src/core/completion/matcher.rs b/crates/alan/src/core/completion/matcher.rs index 7b29c9f..38a1b8f 100644 --- a/crates/alan/src/core/completion/matcher.rs +++ b/crates/alan/src/core/completion/matcher.rs @@ -9,10 +9,10 @@ #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Copy)] struct Rank { kind: Kind, - /// Byte offset of the match. Earlier beats later. - at: usize, - /// Length of the candidate. Shorter wins an otherwise equal match. + /// Length of the candidate. length: usize, + /// Byte offset of the match, to settle equal-length candidates. + at: usize, } /// Where in the candidate the match landed. @@ -69,8 +69,8 @@ fn rank(pattern: &str, candidate: &str) -> Option { }; Some(Rank { kind, - at, length: candidate.len(), + at, }) } @@ -143,7 +143,7 @@ mod tests { /// All three match on the name, so only the tie-breaks separate them. #[test] - fn tie_breaks_on_position_then_length() { + fn tie_breaks_on_length_then_position() { let candidates = ["crates/x/main.rs", "b/main_helper.rs", "a/main.rs"]; assert_eq!( ranked("main", &candidates), @@ -191,6 +191,26 @@ mod tests { ); } + /// When several names match equally well the shortest path wins, rather + /// than whichever has the fewest directories before the first part. + #[test] + fn equally_good_names_are_ordered_by_path_length() { + let candidates = [ + "crates/llm/src/apis/chat_completions/sse.rs", + "crates/alan/src/views/selection.rs", + "crates/agent/src/skill.rs", + ]; + + assert_eq!( + ranked("src/s", &candidates), + [ + "crates/agent/src/skill.rs", + "crates/alan/src/views/selection.rs", + "crates/llm/src/apis/chat_completions/sse.rs", + ] + ); + } + /// The name is what was typed, not the directory it happens to repeat in. #[test] fn a_name_match_outranks_the_same_text_in_the_directory_path() { From aa13adb2c0d9922782c0cb9881333d85b678ee86 Mon Sep 17 00:00:00 2001 From: Azeem Shaik Date: Fri, 28 Aug 2026 14:53:12 +0530 Subject: [PATCH 10/13] Show key hints that match what Enter actually does - Add an Activity the controller derives, so the footer stops assembling the status line out of booleans: thinking, suggesting, or idle - The status line becomes a table keyed on that, with plan mode and cost as badges layered on top - Anchor the completion popup above the prompt rather than the cursor, which had it painting over the status line describing it - Drop Controller::is_busy, which activity folds in --- crates/alan/src/core/controller.rs | 30 ++++-- crates/alan/src/core/mod.rs | 2 +- crates/alan/src/views/components/footer.rs | 101 +++++++++++++-------- crates/alan/src/views/components/popup.rs | 44 +++++---- 4 files changed, 114 insertions(+), 63 deletions(-) diff --git a/crates/alan/src/core/controller.rs b/crates/alan/src/core/controller.rs index d28be88..e0e910c 100644 --- a/crates/alan/src/core/controller.rs +++ b/crates/alan/src/core/controller.rs @@ -37,6 +37,17 @@ pub enum Overlay { Login, } +/// What the prompt is doing, and so what Enter does to it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Activity { + /// Streaming a response. + Thinking, + /// Offering completions, which take Enter before the editor sees it. + Suggesting, + /// Waiting on a prompt. + Idle, +} + /// Coordinates feature controllers. It does not render or handle terminal types. pub struct Controller { chat: ChatController, @@ -78,8 +89,15 @@ impl Controller { self.chat.revision() } - pub fn is_busy(&self) -> bool { - self.chat.is_busy() + /// Ordered by precedence: a streaming response outranks an open popup. + pub fn activity(&self) -> Activity { + if self.chat.is_busy() { + Activity::Thinking + } else if self.completion.item_count() > 0 { + Activity::Suggesting + } else { + Activity::Idle + } } pub fn plan_mode(&self) -> bool { @@ -256,7 +274,7 @@ mod tests { assert_eq!(controller.chat().len(), 2); assert!(matches!(&controller.chat()[0], Entry::Prompt(text) if text == "hi")); assert!(matches!(&controller.chat()[1], Entry::Response(text) if text == "hello")); - assert!(!controller.is_busy()); + assert_eq!(controller.activity(), Activity::Idle); } struct ReasoningFakeApi; @@ -307,7 +325,7 @@ mod tests { assert!(matches!(&controller.chat()[0], Entry::Prompt(text) if text == "hi")); assert!(matches!(&controller.chat()[1], Entry::Reasoning(text) if text == "thinking...")); assert!(matches!(&controller.chat()[2], Entry::Response(text) if text == "answer")); - assert!(!controller.is_busy()); + assert_eq!(controller.activity(), Activity::Idle); } #[test] @@ -315,7 +333,7 @@ mod tests { let mut controller = make_controller(); controller.submit(" ".into(), vec![]); assert!(controller.chat().is_empty()); - assert!(!controller.is_busy()); + assert_eq!(controller.activity(), Activity::Idle); } #[test] @@ -329,7 +347,7 @@ mod tests { Entry::Info(text) if text.contains("/help") )); // A command must never start an agent turn. - assert!(!controller.is_busy()); + assert_eq!(controller.activity(), Activity::Idle); } /// Streamed text merges into a trailing `Response`, and commands run diff --git a/crates/alan/src/core/mod.rs b/crates/alan/src/core/mod.rs index 11f6e58..7afe386 100644 --- a/crates/alan/src/core/mod.rs +++ b/crates/alan/src/core/mod.rs @@ -11,5 +11,5 @@ pub use action::{Action, Command, ImageAttachment}; pub use chat::Entry; pub use command::SlashCommand; pub use completion::{CompletionController, CompletionItem, CompletionStatus}; -pub use controller::{Controller, Overlay, Poll}; +pub use controller::{Activity, Controller, Overlay, Poll}; pub use login::LoginState; diff --git a/crates/alan/src/views/components/footer.rs b/crates/alan/src/views/components/footer.rs index 150457c..ac6a34d 100644 --- a/crates/alan/src/views/components/footer.rs +++ b/crates/alan/src/views/components/footer.rs @@ -1,4 +1,4 @@ -use crate::core::{Controller, LoginState}; +use crate::core::{Activity, Controller, LoginState}; use crate::views::UiState; use crate::views::component::Component; use crate::views::components::PopupList; @@ -16,6 +16,65 @@ pub struct Footer { popup: PopupList, } +/// How an [`Activity`] presents itself in the status line. +struct Status { + style: Style, + label: &'static str, + hints: &'static str, +} + +impl From for Status { + fn from(activity: Activity) -> Self { + match activity { + Activity::Thinking => Status { + label: " ● thinking", + hints: " Ctrl-C stop", + style: Style::default().italic().fg(ratatui::style::Color::Yellow), + }, + // The list on screen already says what is happening, so the dot + // only holds the column and carries the popup's own accent. + Activity::Suggesting => Status { + label: " ●", + hints: " Enter accept · ↑↓ move · Esc dismiss", + style: Style::default().fg(theme::PROMPT_FG), + }, + Activity::Idle => Status { + label: " ● idle", + hints: " Enter send · Ctrl-C quit", + style: Style::default().fg(ratatui::style::Color::Green), + }, + } + } +} + +/// Flags that layer onto any activity. +fn badges(controller: &Controller) -> Vec> { + let mut badges = Vec::new(); + if controller.plan_mode() { + badges.push(Span::styled( + " · Plan mode", + Style::default().fg(ratatui::style::Color::White), + )); + } + if let Some(cost) = controller.usage().cost { + badges.push(Span::styled( + format!(" · ${:.4}", (cost * 10_000.0).trunc() / 10_000.0), + Style::default().fg(theme::MUTED_FG), + )); + } + badges +} + +fn status_line(controller: &Controller) -> Line<'static> { + let status = Status::from(controller.activity()); + let mut spans = vec![ + Span::styled(status.label, status.style), + Span::styled(status.hints, Style::default().fg(theme::MUTED_FG)), + ]; + spans.extend(badges(controller)); + Line::from(spans) +} + impl Component for Footer { fn render( &mut self, @@ -64,38 +123,8 @@ impl Component for Footer { frame.render_widget(attachments, attachment_area); } - let (indicator, indicator_style, shortcuts) = if controller.is_busy() { - ( - " ● thinking", - Style::default().italic().fg(ratatui::style::Color::Yellow), - "· Ctrl-C stop", - ) - } else { - ( - " ● idle", - Style::default().fg(ratatui::style::Color::Green), - " Enter send · Ctrl-C quit", - ) - }; - let mut status_spans = vec![ - Span::styled(indicator, indicator_style), - Span::styled(shortcuts, Style::default().fg(theme::MUTED_FG)), - ]; - if controller.plan_mode() { - status_spans.push(Span::styled( - " · Plan mode", - Style::default().fg(ratatui::style::Color::White), - )); - } - if let Some(cost) = controller.usage().cost { - status_spans.push(Span::styled( - format!(" · ${:.4}", (cost * 10_000.0).trunc() / 10_000.0), - Style::default().fg(theme::MUTED_FG), - )); - } - let status = Line::from(status_spans); frame.render_widget( - Paragraph::new(status).style(Style::default().bg(theme::EDITOR_BG)), + Paragraph::new(status_line(controller)).style(Style::default().bg(theme::EDITOR_BG)), status_area, ); @@ -137,11 +166,9 @@ impl Component for Footer { state.editor().render(input_area, frame.buffer_mut()); if let Some(position) = state.cursor_screen_position() { frame.set_cursor_position(position); - if let Some(popup_area) = - PopupList::area_above_cursor(Rect::new(position.x, position.y, 1, 1), frame.area()) - { - self.popup.render(frame, popup_area, controller, state); - } + } + if let Some(popup_area) = PopupList::area_above(area, frame.area()) { + self.popup.render(frame, popup_area, controller, state); } } } diff --git a/crates/alan/src/views/components/popup.rs b/crates/alan/src/views/components/popup.rs index 1b684b6..c32a5f7 100644 --- a/crates/alan/src/views/components/popup.rs +++ b/crates/alan/src/views/components/popup.rs @@ -13,23 +13,24 @@ const POPUP_ROWS: u16 = 7; /// Candidates visible inside that height. const VISIBLE_ROWS: usize = 5; -/// Generic list popup rendered above the editor cursor. Currently used for +/// Generic list popup rendered above the prompt. Currently used for /// `@`-path completion; reusable for any short list anchored at the prompt. #[derive(Debug, Default)] pub struct PopupList; impl PopupList { - /// Popup area whose bottom edge sits directly above `cursor`, spanning - /// the editor column (the frame minus the prompt gutter). Returns `None` - /// when there is no room to show it. - pub fn area_above_cursor(cursor: Rect, frame_area: Rect) -> Option { - let top = cursor.y.checked_sub(POPUP_ROWS)?; - if top < frame_area.y || cursor.y >= frame_area.bottom() { + /// Popup area sitting directly above `prompt`, spanning the frame width. + /// + /// Anchored to the prompt rather than the cursor so it never covers the + /// status line, which is what describes the keys the popup has taken. + /// Returns `None` when there is no room above. + pub fn area_above(prompt: Rect, frame_area: Rect) -> Option { + let top = prompt.y.checked_sub(POPUP_ROWS)?; + if top < frame_area.y || prompt.y > frame_area.bottom() { return None; } - let x = frame_area.x; Some(Rect { - x, + x: frame_area.x, y: top, width: frame_area.width, height: POPUP_ROWS, @@ -110,22 +111,27 @@ fn item_line(item: &CompletionItem, is_selected: bool) -> Line<'static> { mod tests { use super::*; + /// Anchoring above the prompt is what keeps the status line, which sits + /// inside the prompt area, out from under the popup. #[test] - fn popup_sits_directly_above_cursor() { + fn popup_sits_directly_above_the_prompt() { let frame = Rect::new(0, 0, 80, 24); - let cursor = Rect::new(0, 20, 1, 1); - let area = PopupList::area_above_cursor(cursor, frame).unwrap(); + let prompt = Rect::new(0, 16, 80, 8); + + let area = PopupList::area_above(prompt, frame).unwrap(); + assert_eq!(area.height, POPUP_ROWS); - assert_eq!(area.bottom(), cursor.y); + assert_eq!(area.bottom(), prompt.y); + assert!(area.bottom() <= prompt.y, "overlaps the prompt"); } #[test] - fn no_room_above_cursor_means_no_popup() { + fn no_room_above_the_prompt_means_no_popup() { let frame = Rect::new(0, 0, 80, 24); - // Not enough rows above the cursor for the fixed height. - assert!(PopupList::area_above_cursor(Rect::new(0, 3, 1, 1), frame).is_none()); - assert!(PopupList::area_above_cursor(Rect::new(0, 0, 1, 1), frame).is_none()); - // Cursor off the bottom of the frame. - assert!(PopupList::area_above_cursor(Rect::new(0, 24, 1, 1), frame).is_none()); + // Not enough rows above the prompt for the fixed height. + assert!(PopupList::area_above(Rect::new(0, 3, 80, 8), frame).is_none()); + assert!(PopupList::area_above(Rect::new(0, 0, 80, 8), frame).is_none()); + // Prompt off the bottom of the frame. + assert!(PopupList::area_above(Rect::new(0, 25, 80, 8), frame).is_none()); } } From 64b6aa4936e08491b4e7a70465a00d7ff96fe41a Mon Sep 17 00:00:00 2001 From: Azeem Shaik Date: Fri, 28 Aug 2026 15:03:10 +0530 Subject: [PATCH 11/13] Drop comments that restate the code they sit on --- crates/alan/src/core/completion/paths.rs | 4 ---- crates/alan/src/views/components/footer.rs | 2 -- 2 files changed, 6 deletions(-) diff --git a/crates/alan/src/core/completion/paths.rs b/crates/alan/src/core/completion/paths.rs index d4ed903..fff1006 100644 --- a/crates/alan/src/core/completion/paths.rs +++ b/crates/alan/src/core/completion/paths.rs @@ -124,7 +124,6 @@ impl CompletionBackend for Paths { return Poll::Idle; }; let Some(finished) = (&mut scan).now_or_never() else { - // Still walking. Put it back for the next tick. self.scan = Some(scan); return Poll::Idle; }; @@ -344,7 +343,6 @@ mod tests { tokio::task::yield_now().await; assert!(!paths.scanning()); - // The previous index survives and the popup is not told anything. assert_eq!(paths.poll(), Poll::Idle); } @@ -388,8 +386,6 @@ mod tests { let mut completion = CompletionController::new(vec![Box::new(Paths::new(root.clone()))]); - // Typing `@mai` opens the popup and starts the scan. Nothing to show - // yet, because the index is still empty. completion.sync("@mai", 4); assert!(completion.is_open()); assert_eq!(completion.item_count(), 0); diff --git a/crates/alan/src/views/components/footer.rs b/crates/alan/src/views/components/footer.rs index ac6a34d..0200174 100644 --- a/crates/alan/src/views/components/footer.rs +++ b/crates/alan/src/views/components/footer.rs @@ -31,8 +31,6 @@ impl From for Status { hints: " Ctrl-C stop", style: Style::default().italic().fg(ratatui::style::Color::Yellow), }, - // The list on screen already says what is happening, so the dot - // only holds the column and carries the popup's own accent. Activity::Suggesting => Status { label: " ●", hints: " Enter accept · ↑↓ move · Esc dismiss", From 4e72c47b251636810c924faaddea60e26c01128a Mon Sep 17 00:00:00 2001 From: Azeem Shaik Date: Fri, 28 Aug 2026 15:03:52 +0530 Subject: [PATCH 12/13] Drop the marker styling comment --- crates/alan/src/views/components/popup.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/crates/alan/src/views/components/popup.rs b/crates/alan/src/views/components/popup.rs index c32a5f7..4f2f224 100644 --- a/crates/alan/src/views/components/popup.rs +++ b/crates/alan/src/views/components/popup.rs @@ -89,12 +89,7 @@ impl Component for PopupList { } } -/// One popup line: the selection marker and the candidate. -/// -/// Brightness marks what the next Enter will take, so a trailing `/` is left -/// to say on its own that an entry is a directory. fn item_line(item: &CompletionItem, is_selected: bool) -> Line<'static> { - // The marker is blank unless selected, so it can always carry the accent. let (marker, label) = if is_selected { ("› ", theme::SELECTION_FG) } else { From f6097ae59e2cd8aae318e4b515b5cb603190810f Mon Sep 17 00:00:00 2001 From: Azeem Shaik Date: Fri, 28 Aug 2026 14:57:59 +0530 Subject: [PATCH 13/13] Create the sessions root before narrowing its permissions The first session on a machine failed with "No such file or directory", because `create` chmodded the root before `create_dir_all` had made it. Every test pre-created the root, so the first-run path was never exercised. --- crates/agent/src/session/manager.rs | 32 ++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/crates/agent/src/session/manager.rs b/crates/agent/src/session/manager.rs index ea45816..f7103ca 100644 --- a/crates/agent/src/session/manager.rs +++ b/crates/agent/src/session/manager.rs @@ -52,13 +52,15 @@ impl SessionManager { None => unreachable!("file_path always builds root/key/name.jsonl"), }; - set_permissions(&self.root, true).await?; + // Builds the root as well as the key directory, so both have to be + // narrowed afterwards rather than before. tokio::fs::create_dir_all(&dir).await.map_err(|source| { SessionError::Store(StoreError::CreateDir { dir: dir.clone(), source, }) })?; + set_permissions(&self.root, true).await?; set_permissions(&dir, true).await?; let header = @@ -302,6 +304,34 @@ mod tests { cleanup(&root); } + /// The root is created on demand, so the first session on a machine that + /// has never run one starts without it. + #[tokio::test] + async fn create_makes_a_missing_root() { + let root = + std::env::temp_dir().join(format!("alan-session-fresh-{}", uuid::Uuid::new_v4())); + assert!(!root.exists(), "root must not exist yet"); + let manager = SessionManager::new(&root); + + manager + .create("/tmp/project", "openrouter", "test-model", None) + .await + .expect("create session without a pre-existing root"); + + assert!(root.is_dir(), "create built the root"); + // Narrowing it is the reason the call exists, so a root built here + // has to end up as private as one that already existed. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = std::fs::metadata(&root) + .expect("root metadata") + .permissions(); + assert_eq!(mode.mode() & 0o777, 0o700, "root mode"); + } + cleanup(&root); + } + #[tokio::test] async fn different_pwds_use_different_directories() { let root = temp_root("pwds");