fix(agent): semantic_search symbol index workspace-aware
SymbolIndex global sudah melacak workspace_path tapi SemanticSearch dan ListSymbols Cuma rebuild saat index kosong (is_empty). Akibat: setelah mengindeks workspace A, mencari di workspace B diam-diam mengembalikan simbol STALE dari A — menyesatkan coding agent (referensikan simbol yang tidak ada di repo aktif). Fix: - Tambah SymbolIndex::needs_rebuild(workspace) — true bila index kosong ATAU workspace diminta beda dari yang ter-cache. - Pakai di 2 call site (SemanticSearch::run, ListSymbols::run) menggantikan is_empty(), jadi pindah workspace otomatis trigger rebuild. - test: +1 (test_needs_rebuild_workspace_aware — verifikasi flip workspace memicu rebuild bolak-balik A -> B -> A). Catatan (bukan bug, dilaporkan): mutex SYMBOL_INDEX masih dipegang selama full rebuild di run() — bottleneck saat semantic_search dipanggil paralel; perbaikan butuh restrukturisasi double-checked rebuild, tak diubah di sini. Verifikasi: check/clippy -D warnings/fmt clean; test infra 63 (0 gagal).
This commit is contained in:
@@ -270,6 +270,17 @@ impl SymbolIndex {
|
||||
self.symbols.is_empty()
|
||||
}
|
||||
|
||||
/// Returns true when the cached index must be rebuilt for the given
|
||||
/// workspace — either because nothing has been indexed yet, or because the
|
||||
/// requested workspace differs from the one the index was built for.
|
||||
///
|
||||
/// Without this, searching a *different* workspace after the first one
|
||||
/// silently returns stale symbols from the previously indexed repo
|
||||
/// (a misleading result for a coding agent).
|
||||
pub fn needs_rebuild(&self, workspace: &str) -> bool {
|
||||
self.is_empty() || self.workspace_path.as_deref() != Some(workspace)
|
||||
}
|
||||
|
||||
pub fn len(&self) -> usize {
|
||||
self.symbols.len()
|
||||
}
|
||||
@@ -1255,7 +1266,7 @@ impl Tool for SemanticSearch {
|
||||
.map_err(|e| anyhow::anyhow!("index lock failed: {e}"))?;
|
||||
let index = guard.get_or_insert_with(SymbolIndex::new);
|
||||
|
||||
if rebuild || index.is_empty() {
|
||||
if rebuild || index.needs_rebuild(&workspace) {
|
||||
let count = index.rebuild(&workspace)?;
|
||||
debug!(symbol_count = count, "symbol index rebuilt");
|
||||
}
|
||||
@@ -1502,7 +1513,7 @@ impl Tool for ListSymbols {
|
||||
.map_err(|e| anyhow::anyhow!("index lock failed: {e}"))?;
|
||||
let index = guard.get_or_insert_with(SymbolIndex::new);
|
||||
|
||||
if rebuild || index.is_empty() {
|
||||
if rebuild || index.needs_rebuild(&workspace) {
|
||||
let count = index.rebuild(&workspace)?;
|
||||
info!(symbol_count = count, "symbol index rebuilt for list");
|
||||
}
|
||||
@@ -1746,4 +1757,38 @@ mod tests {
|
||||
let index = SymbolIndex::new();
|
||||
assert!(index.search("anything", 10).is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_needs_rebuild_workspace_aware() {
|
||||
let ws_a = std::env::temp_dir().join(format!("ws_a_{}", uuid::Uuid::new_v4()));
|
||||
let ws_b = std::env::temp_dir().join(format!("ws_b_{}", uuid::Uuid::new_v4()));
|
||||
std::fs::create_dir_all(&ws_a).unwrap();
|
||||
std::fs::create_dir_all(&ws_b).unwrap();
|
||||
std::fs::write(ws_a.join("a.rs"), "pub fn fn_in_a() {}\n").unwrap();
|
||||
std::fs::write(ws_b.join("b.rs"), "pub fn fn_in_b() {}\n").unwrap();
|
||||
|
||||
let mut index = SymbolIndex::new();
|
||||
let a = ws_a.to_string_lossy().to_string();
|
||||
let b = ws_b.to_string_lossy().to_string();
|
||||
|
||||
// Fresh index: needs rebuild for any workspace.
|
||||
assert!(index.needs_rebuild(&a));
|
||||
|
||||
// After rebuilding A, searching A needs no rebuild...
|
||||
index.rebuild(&a).unwrap();
|
||||
assert!(!index.needs_rebuild(&a));
|
||||
// ...but searching B DOES (stale index otherwise).
|
||||
assert!(
|
||||
index.needs_rebuild(&b),
|
||||
"workspace switch must trigger rebuild"
|
||||
);
|
||||
|
||||
// Rebuilding B flips the cached workspace.
|
||||
index.rebuild(&b).unwrap();
|
||||
assert!(!index.needs_rebuild(&b));
|
||||
assert!(index.needs_rebuild(&a));
|
||||
|
||||
std::fs::remove_dir_all(&ws_a).ok();
|
||||
std::fs::remove_dir_all(&ws_b).ok();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user