mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
Prevent stale file search results from being labeled with a new query (#49946)
## Why The asynchronous matcher can return results for the previous query while a new query is still running. Labeling those results with the latest query produces a mismatched search snapshot. ## What changed Only report a file search snapshot when its pattern atoms match the currently requested pattern. Extract snapshot conversion into `snapshot::for_query`, which skips pending results from an older pattern. ## Testing Add a regression test that changes the query from empty to `bet`, verifies that the old snapshot is withheld, and checks that the completed snapshot contains `beta.txt` with the expected match indices and counts. GitOrigin-RevId: 01b62b822db69e15480d3233b66ec55c45646fad
This commit is contained in:
@@ -35,6 +35,11 @@ use nucleo::pattern::AtomKind;
|
|||||||
use nucleo::pattern::Pattern;
|
use nucleo::pattern::Pattern;
|
||||||
|
|
||||||
mod cli;
|
mod cli;
|
||||||
|
mod snapshot;
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
#[path = "matcher_tests.rs"]
|
||||||
|
mod matcher_tests;
|
||||||
|
|
||||||
pub use cli::Cli;
|
pub use cli::Cli;
|
||||||
|
|
||||||
@@ -553,45 +558,15 @@ fn matcher_worker(
|
|||||||
recv(next_notify) -> _ => {
|
recv(next_notify) -> _ => {
|
||||||
will_notify = false;
|
will_notify = false;
|
||||||
let status = nucleo.tick(TICK_TIMEOUT_MS);
|
let status = nucleo.tick(TICK_TIMEOUT_MS);
|
||||||
if status.changed {
|
if status.changed
|
||||||
let snapshot = nucleo.snapshot();
|
&& let Some(snapshot) = snapshot::for_query(
|
||||||
let limit = inner.limit.min(snapshot.matched_item_count() as usize);
|
&inner,
|
||||||
let pattern = snapshot.pattern().column_pattern(0);
|
&nucleo,
|
||||||
let matches: Vec<_> = snapshot
|
&last_query,
|
||||||
.matches()
|
&mut indices_matcher,
|
||||||
.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::<u32>::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,
|
|
||||||
walk_complete,
|
walk_complete,
|
||||||
};
|
)
|
||||||
|
{
|
||||||
inner.reporter.on_update(&snapshot);
|
inner.reporter.on_update(&snapshot);
|
||||||
}
|
}
|
||||||
if !status.running && walk_complete {
|
if !status.running && walk_complete {
|
||||||
@@ -710,7 +685,7 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Default)]
|
#[derive(Default)]
|
||||||
struct RecordingReporter {
|
pub(super) struct RecordingReporter {
|
||||||
updates: Mutex<Vec<FileSearchSnapshot>>,
|
updates: Mutex<Vec<FileSearchSnapshot>>,
|
||||||
complete_times: Mutex<Vec<Instant>>,
|
complete_times: Mutex<Vec<Instant>>,
|
||||||
complete_cv: Condvar,
|
complete_cv: Condvar,
|
||||||
|
|||||||
@@ -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,
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -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<IndexedEntry>,
|
||||||
|
query: &str,
|
||||||
|
indices_matcher: &mut Option<Matcher>,
|
||||||
|
walk_complete: bool,
|
||||||
|
) -> Option<FileSearchSnapshot> {
|
||||||
|
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::<u32>::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,
|
||||||
|
})
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user