diff --git a/codex-rs/file-search/src/lib.rs b/codex-rs/file-search/src/lib.rs index bcad11f770..c189646ce2 100644 --- a/codex-rs/file-search/src/lib.rs +++ b/codex-rs/file-search/src/lib.rs @@ -35,6 +35,11 @@ use nucleo::pattern::AtomKind; use nucleo::pattern::Pattern; mod cli; +mod snapshot; + +#[cfg(test)] +#[path = "matcher_tests.rs"] +mod matcher_tests; pub use cli::Cli; @@ -553,45 +558,15 @@ fn matcher_worker( recv(next_notify) -> _ => { will_notify = false; let status = nucleo.tick(TICK_TIMEOUT_MS); - if status.changed { - let snapshot = nucleo.snapshot(); - let limit = inner.limit.min(snapshot.matched_item_count() as usize); - let pattern = snapshot.pattern().column_pattern(0); - let matches: Vec<_> = snapshot - .matches() - .iter() - .take(limit) - .filter_map(|match_| { - let item = snapshot.get_item(match_.idx)?; - let full_path = item.data.full_path.as_ref(); - let (root_idx, relative_path) = get_file_path(Path::new(full_path), &inner.search_directories)?; - let indices = if let Some(indices_matcher) = indices_matcher.as_mut() { - let mut idx_vec = Vec::::new(); - let haystack = item.matcher_columns[0].slice(..); - let _ = pattern.indices(haystack, indices_matcher, &mut idx_vec); - idx_vec.sort_unstable(); - idx_vec.dedup(); - Some(idx_vec) - } else { - None - }; - Some(FileMatch { - score: match_.score, - path: PathBuf::from(relative_path), - match_type: item.data.match_type, - root: inner.search_directories[root_idx].clone(), - indices, - }) - }) - .collect(); - - let snapshot = FileSearchSnapshot { - query: last_query.clone(), - matches, - total_match_count: snapshot.matched_item_count() as usize, - scanned_file_count: snapshot.item_count() as usize, + if status.changed + && let Some(snapshot) = snapshot::for_query( + &inner, + &nucleo, + &last_query, + &mut indices_matcher, walk_complete, - }; + ) + { inner.reporter.on_update(&snapshot); } if !status.running && walk_complete { @@ -710,7 +685,7 @@ mod tests { } #[derive(Default)] - struct RecordingReporter { + pub(super) struct RecordingReporter { updates: Mutex>, complete_times: Mutex>, complete_cv: Condvar, diff --git a/codex-rs/file-search/src/matcher_tests.rs b/codex-rs/file-search/src/matcher_tests.rs new file mode 100644 index 0000000000..8f9a5f728f --- /dev/null +++ b/codex-rs/file-search/src/matcher_tests.rs @@ -0,0 +1,100 @@ +//! Regression coverage for associating asynchronous matcher results with their query. + +use super::*; +use pretty_assertions::assert_eq; + +#[test] +fn query_update_does_not_relabel_pending_matches() { + let root = tempfile::tempdir().unwrap(); + let (work_tx, work_rx) = unbounded(); + let notify_tx = work_tx.clone(); + let mut nucleo = Nucleo::new( + Config::DEFAULT.match_paths(), + Arc::new(move || { + let _ = notify_tx.send(WorkSignal::NucleoNotify); + }), + /*num_threads*/ Some(1), + /*columns*/ 1, + ); + let injector = nucleo.injector(); + for (relative, match_type) in [("", MatchType::Directory), ("beta.txt", MatchType::File)] { + let full_path = root.path().join(relative); + injector.push( + IndexedEntry { + full_path: Arc::from(full_path.to_str().unwrap()), + match_type, + }, + |_, columns| columns[0] = Utf32String::from(relative), + ); + } + while nucleo.tick(/*timeout*/ 10).running { + work_rx.recv_timeout(Duration::from_secs(5)).unwrap(); + } + let inner = SessionInner { + search_directories: vec![root.path().to_path_buf()], + limit: 50, + threads: 1, + compute_indices: true, + respect_gitignore: true, + cancelled: Arc::new(AtomicBool::new(false)), + shutdown: Arc::new(AtomicBool::new(false)), + reporter: Arc::new(tests::RecordingReporter::default()), + work_tx, + }; + let mut indices_matcher = Some(Matcher::new(Config::DEFAULT.match_paths())); + let initial = snapshot::for_query( + &inner, + &nucleo, + "", + &mut indices_matcher, + /*walk_complete*/ true, + ) + .unwrap(); + assert_eq!(initial.matches.len(), 2); + + nucleo.pattern.reparse( + /*column*/ 0, + "bet", + CaseMatching::Ignore, + Normalization::Smart, + /*append*/ true, + ); + // The requested pattern has changed, but no tick has replaced the previous + // snapshot. This is the same intermediate state a busy tick can expose. + assert_eq!( + snapshot::for_query( + &inner, + &nucleo, + "bet", + &mut indices_matcher, + /*walk_complete*/ true + ), + None + ); + + while nucleo.tick(/*timeout*/ 10).running { + work_rx.recv_timeout(Duration::from_secs(5)).unwrap(); + } + assert_eq!( + snapshot::for_query( + &inner, + &nucleo, + "bet", + &mut indices_matcher, + /*walk_complete*/ true + ), + Some(FileSearchSnapshot { + query: "bet".to_string(), + matches: vec![FileMatch { + score: 84, + path: PathBuf::from("beta.txt"), + match_type: MatchType::File, + root: root.path().to_path_buf(), + indices: Some(vec![0, 1, 2]), + }], + total_match_count: 1, + scanned_file_count: 2, + walk_complete: true, + }), + ); +} diff --git a/codex-rs/file-search/src/snapshot.rs b/codex-rs/file-search/src/snapshot.rs new file mode 100644 index 0000000000..69edb40277 --- /dev/null +++ b/codex-rs/file-search/src/snapshot.rs @@ -0,0 +1,63 @@ +//! Converts matcher snapshots without associating old results with a new query. + +use crate::FileMatch; +use crate::FileSearchSnapshot; +use crate::IndexedEntry; +use crate::SessionInner; +use crate::get_file_path; +use nucleo::Matcher; +use nucleo::Nucleo; +use std::path::Path; +use std::path::PathBuf; + +pub(super) fn for_query( + inner: &SessionInner, + nucleo: &Nucleo, + query: &str, + indices_matcher: &mut Option, + walk_complete: bool, +) -> Option { + let snapshot = nucleo.snapshot(); + let pattern = snapshot.pattern().column_pattern(0); + // A tick can collect the previous query's results while the new query is + // still running. Wait for a matching snapshot before using the new label. + if pattern.atoms != nucleo.pattern.column_pattern(0).atoms { + return None; + } + let limit = inner.limit.min(snapshot.matched_item_count() as usize); + let matches = snapshot + .matches() + .iter() + .take(limit) + .filter_map(|match_| { + let item = snapshot.get_item(match_.idx)?; + let full_path = item.data.full_path.as_ref(); + let (root_idx, relative_path) = + get_file_path(Path::new(full_path), &inner.search_directories)?; + let indices = if let Some(indices_matcher) = indices_matcher.as_mut() { + let mut indices = Vec::::new(); + let haystack = item.matcher_columns[0].slice(..); + let _ = pattern.indices(haystack, indices_matcher, &mut indices); + indices.sort_unstable(); + indices.dedup(); + Some(indices) + } else { + None + }; + Some(FileMatch { + score: match_.score, + path: PathBuf::from(relative_path), + match_type: item.data.match_type, + root: inner.search_directories[root_idx].clone(), + indices, + }) + }) + .collect(); + Some(FileSearchSnapshot { + query: query.to_string(), + matches, + total_match_count: snapshot.matched_item_count() as usize, + scanned_file_count: snapshot.item_count() as usize, + walk_complete, + }) +}