diff --git a/src/graph/ladybug_store.rs b/src/graph/ladybug_store.rs index d2c4753..b3008fd 100644 --- a/src/graph/ladybug_store.rs +++ b/src/graph/ladybug_store.rs @@ -357,22 +357,29 @@ impl GraphStore for LadybugGraphStore { fn symbol_type_relations(&self, symbol_name: &str) -> Result> { let _guard = self.lock.lock().unwrap(); let conn = self.conn()?; - // Four directed traversals; each returns (file_path, reason). + // Four directed traversals; each returns (file_path, reason). Name + // matching is case-insensitive (lowercased param vs `lower(name)`) to + // align with the case-insensitive seed used by pack/related. + let name_lc = symbol_name.to_ascii_lowercase(); let queries = [ ( - "MATCH (s:Symbol {name: $n})-[:INHERITS]->(t:Symbol) RETURN DISTINCT t.filePath", + "MATCH (s:Symbol)-[:INHERITS]->(t:Symbol) WHERE lower(s.name) = $n \ + RETURN DISTINCT t.filePath", "base type (inherits)", ), ( - "MATCH (s:Symbol)-[:INHERITS]->(t:Symbol {name: $n}) RETURN DISTINCT s.filePath", + "MATCH (s:Symbol)-[:INHERITS]->(t:Symbol) WHERE lower(t.name) = $n \ + RETURN DISTINCT s.filePath", "subtype (inherits)", ), ( - "MATCH (s:Symbol {name: $n})-[:IMPLEMENTS]->(t:Symbol) RETURN DISTINCT t.filePath", + "MATCH (s:Symbol)-[:IMPLEMENTS]->(t:Symbol) WHERE lower(s.name) = $n \ + RETURN DISTINCT t.filePath", "implemented interface/trait", ), ( - "MATCH (s:Symbol)-[:IMPLEMENTS]->(t:Symbol {name: $n}) RETURN DISTINCT s.filePath", + "MATCH (s:Symbol)-[:IMPLEMENTS]->(t:Symbol) WHERE lower(t.name) = $n \ + RETURN DISTINCT s.filePath", "implementor", ), ]; @@ -382,10 +389,7 @@ impl GraphStore for LadybugGraphStore { .prepare(cypher) .map_err(|e| anyhow!("preparing symbol_type_relations: {e}"))?; let result = conn - .execute( - &mut stmt, - vec![("n", Value::String(symbol_name.to_string()))], - ) + .execute(&mut stmt, vec![("n", Value::String(name_lc.clone()))]) .map_err(|e| anyhow!("executing symbol_type_relations: {e}"))?; for row in result { out.push(RelatedItem { @@ -403,16 +407,17 @@ impl GraphStore for LadybugGraphStore { fn symbol_references(&self, symbol_name: &str) -> Result> { let _guard = self.lock.lock().unwrap(); let conn = self.conn()?; - // Incoming REFERENCES edges = files that reference this symbol. - let cypher = - "MATCH (s:Symbol)-[:REFERENCES]->(t:Symbol {name: $n}) RETURN DISTINCT s.filePath"; + // Incoming REFERENCES edges = files that reference this symbol. Name + // matching is case-insensitive to align with the seed lookup. + let cypher = "MATCH (s:Symbol)-[:REFERENCES]->(t:Symbol) WHERE lower(t.name) = $n \ + RETURN DISTINCT s.filePath"; let mut stmt = conn .prepare(cypher) .map_err(|e| anyhow!("preparing symbol_references: {e}"))?; let result = conn .execute( &mut stmt, - vec![("n", Value::String(symbol_name.to_string()))], + vec![("n", Value::String(symbol_name.to_ascii_lowercase()))], ) .map_err(|e| anyhow!("executing symbol_references: {e}"))?; let reason = format!("references {symbol_name}"); @@ -564,13 +569,17 @@ impl GraphStore for LadybugGraphStore { { let _guard = self.lock.lock().unwrap(); let conn = self.conn()?; - let cypher = - "MATCH (s:Symbol {name: $name}) RETURN DISTINCT s.filePath ORDER BY s.filePath"; + // Case-insensitive to align with the seed lookup. + let cypher = "MATCH (s:Symbol) WHERE lower(s.name) = $name \ + RETURN DISTINCT s.filePath ORDER BY s.filePath"; let mut stmt = conn .prepare(cypher) .map_err(|e| anyhow!("preparing related query: {e}"))?; let result = conn - .execute(&mut stmt, vec![("name", Value::String(symbol.to_string()))]) + .execute( + &mut stmt, + vec![("name", Value::String(symbol.to_ascii_lowercase()))], + ) .map_err(|e| anyhow!("executing related query: {e}"))?; for row in result { out.push(RelatedItem { diff --git a/src/graph/memory_store.rs b/src/graph/memory_store.rs index d4b62db..621a0af 100644 --- a/src/graph/memory_store.rs +++ b/src/graph/memory_store.rs @@ -47,6 +47,36 @@ fn ci_contains(haystack: &str, needle: &str) -> bool { .contains(&needle.to_ascii_lowercase()) } +/// Files that reference `symbol_name` via incoming `REFERENCES` edges (callers +/// / instantiation sites), deterministic and de-duplicated by path. Name match +/// is case-insensitive to align with the seed lookup. Lock-free so it can be +/// called from methods that already hold the inner mutex (the single source of +/// truth for both `symbol_references` and `related_to_symbol`'s reference half). +fn collect_references(g: &Inner, symbol_name: &str) -> Vec { + let target_ids: BTreeSet<&str> = g + .symbols + .values() + .filter(|s| s.name.eq_ignore_ascii_case(symbol_name)) + .map(|s| s.id.as_str()) + .collect(); + let reason = format!("references {symbol_name}"); + let mut out: Vec = Vec::new(); + for (from, to) in &g.sym_references { + if target_ids.contains(to.as_str()) + && let Some(s) = g.symbols.get(from) + { + out.push(RelatedItem { + path: s.file_path.clone(), + reason: reason.clone(), + depth: 1, + }); + } + } + out.sort_by(|a, b| a.path.cmp(&b.path)); + out.dedup_by(|a, b| a.path == b.path); + out +} + impl GraphStore for MemoryGraphStore { fn initialize_schema(&self) -> Result<()> { Ok(()) @@ -72,14 +102,16 @@ impl GraphStore for MemoryGraphStore { // Drop the file's IMPORTS_PACKAGE edges and project membership. g.file_pkgs.retain(|(f, _)| f != &fid); g.project_files.retain(|(_, f)| f != &fid); - // Drop symbol-level edges originating from this file's symbols. - let sym_prefix = format!("sym:{path}#"); - g.sym_inherits - .retain(|(from, _)| !from.starts_with(&sym_prefix)); - g.sym_implements - .retain(|(from, _)| !from.starts_with(&sym_prefix)); - g.sym_references - .retain(|(from, _)| !from.starts_with(&sym_prefix)); + // Drop symbol-level edges whose *either* endpoint no longer exists. The + // symbols are already removed above, so retaining only edges with both + // endpoints live cleans up references in both directions — including + // edges that merely *point at* a symbol declared in the removed file. + // (A `from`-only sweep would leak those and inflate `referenceEdges`.) + let live: BTreeSet = g.symbols.keys().cloned().collect(); + let both_live = |(from, to): &(String, String)| live.contains(from) && live.contains(to); + g.sym_inherits.retain(both_live); + g.sym_implements.retain(both_live); + g.sym_references.retain(both_live); Ok(()) } @@ -165,38 +197,17 @@ impl GraphStore for MemoryGraphStore { fn symbol_references(&self, symbol_name: &str) -> Result> { let g = self.inner.lock().unwrap(); - // Symbol ids declaring this name (the reference targets). - let ids: BTreeSet<&str> = g - .symbols - .values() - .filter(|s| s.name == symbol_name) - .map(|s| s.id.as_str()) - .collect(); - let reason = format!("references {symbol_name}"); - let mut out: Vec = Vec::new(); - for (from, to) in &g.sym_references { - if ids.contains(to.as_str()) - && let Some(s) = g.symbols.get(from) - { - out.push(RelatedItem { - path: s.file_path.clone(), - reason: reason.clone(), - depth: 1, - }); - } - } - out.sort_by(|a, b| a.path.cmp(&b.path)); - out.dedup_by(|a, b| a.path == b.path); - Ok(out) + Ok(collect_references(&g, symbol_name)) } fn symbol_type_relations(&self, symbol_name: &str) -> Result> { let g = self.inner.lock().unwrap(); - // Symbol ids declaring this name. + // Symbol ids declaring this name (case-insensitive, to align with the + // seed lookup used by pack/related). let ids: BTreeSet<&str> = g .symbols .values() - .filter(|s| s.name == symbol_name) + .filter(|s| s.name.eq_ignore_ascii_case(symbol_name)) .map(|s| s.id.as_str()) .collect(); let file_of = |id: &str| g.symbols.get(id).map(|s| s.file_path.clone()); @@ -293,11 +304,9 @@ impl GraphStore for MemoryGraphStore { fn related_to_symbol(&self, symbol: &str, _depth: usize) -> Result> { let g = self.inner.lock().unwrap(); let mut out = Vec::new(); - // Files declaring an exactly-named symbol are depth 0. - let mut target_ids: BTreeSet<&str> = BTreeSet::new(); + // Files declaring a symbol of this name (case-insensitive) are depth 0. for s in g.symbols.values() { - if s.name == symbol { - target_ids.insert(s.id.as_str()); + if s.name.eq_ignore_ascii_case(symbol) { out.push(RelatedItem { path: s.file_path.clone(), reason: "exact symbol declaration".to_string(), @@ -306,21 +315,10 @@ impl GraphStore for MemoryGraphStore { } } // Files that reference the symbol via incoming REFERENCES edges are - // depth 1 (callers/instantiation sites). This traversal is what makes - // usages visible to `pack`/`related`. Mirror the lock-free inline form - // of `symbol_references` to avoid re-locking the mutex. - let reason = format!("references {symbol}"); - for (from, to) in &g.sym_references { - if target_ids.contains(to.as_str()) - && let Some(s) = g.symbols.get(from) - { - out.push(RelatedItem { - path: s.file_path.clone(), - reason: reason.clone(), - depth: 1, - }); - } - } + // depth 1 (callers/instantiation sites) — the traversal that makes + // usages visible to `pack`/`related`. Shares `collect_references` (the + // single source of truth) while still holding the lock once. + out.extend(collect_references(&g, symbol)); // Deterministic; declaration (depth 0) wins over reference (depth 1) // when the same file both declares and references the symbol. out.sort_by(|a, b| a.path.cmp(&b.path).then(a.depth.cmp(&b.depth))); diff --git a/src/indexer/mod.rs b/src/indexer/mod.rs index 741efd7..d97e2dd 100644 --- a/src/indexer/mod.rs +++ b/src/indexer/mod.rs @@ -322,6 +322,16 @@ fn resolve_imports( Ok(edges) } +/// Whether `candidate_path` lives in the same project directory as a file whose +/// parent directory is `project_dir`. Segment-safe: `src` does not match +/// `src2/foo` — the match must fall on a `/` boundary (or be the dir itself). +fn same_project_dir(candidate_path: &str, project_dir: &str) -> bool { + !project_dir.is_empty() + && candidate_path + .strip_prefix(project_dir) + .is_some_and(|rest| rest.starts_with('/')) +} + /// Resolve collected supertype relationships into `INHERITS`/`IMPLEMENTS` edges. /// Returns the number of edges created. /// @@ -378,9 +388,7 @@ fn resolve_supertypes( } else { let same_proj: Vec<_> = targets .iter() - .filter(|s| { - !project_prefix.is_empty() && s.file_path.starts_with(project_prefix) - }) + .filter(|s| same_project_dir(&s.file_path, project_prefix)) .cloned() .collect(); if !same_proj.is_empty() { @@ -427,36 +435,66 @@ fn resolve_references( store: &dyn GraphStore, pending: &[(String, Vec)], ) -> Result { - use crate::graph::model::SymbolSearchQuery; + use crate::graph::model::{IndexedSymbol, SymbolSearchQuery}; + use std::collections::HashMap; + + // Cache of target candidates keyed by referenced name, shared across every + // file — references to the same type recur constantly, so this turns an + // O(#refs) query pattern into O(#distinct names). Each entry holds the full + // exact-name candidate set (any file); same-file/same-project narrowing is + // applied per reference below. + let mut to_cache: HashMap> = HashMap::new(); let mut edges = 0; for (file, refs) in pending { let project_prefix = file.rsplit_once('/').map(|(d, _)| d).unwrap_or(""); + + // Preload every declaration in this file once, indexed by name, for the + // `from` (enclosing-declaration) lookup — one query per file instead of + // one per reference. Candidates are sorted so selection is deterministic + // when a file has multiple same-named declarations (overloads, etc.). + let mut from_by_name: HashMap<&str, Vec<&IndexedSymbol>> = HashMap::new(); + let file_symbols = store.symbols_matching(&SymbolSearchQuery { + file: Some(file.clone()), + ..Default::default() + })?; + for sym in &file_symbols { + from_by_name.entry(sym.name.as_str()).or_default().push(sym); + } + for list in from_by_name.values_mut() { + list.sort_by(|a, b| { + a.start_line + .cmp(&b.start_line) + .then(a.end_line.cmp(&b.end_line)) + .then(a.id.cmp(&b.id)) + }); + } + for r in refs { // The referring symbol must be a declaration in this file. if r.from.is_empty() { continue; } - let from_candidates = store.symbols_matching(&SymbolSearchQuery { - name: Some(r.from.clone()), - file: Some(file.clone()), - ..Default::default() - })?; - let Some(from) = from_candidates.into_iter().find(|s| s.name == r.from) else { + let Some(from) = from_by_name.get(r.from.as_str()).and_then(|v| v.first()) else { continue; }; - // Candidate target symbols (exact name match, any file). An empty - // set means the name isn't a declared symbol (e.g. a local var) — - // no edge, which is the false-positive guard. - let mut targets: Vec<_> = store - .symbols_matching(&SymbolSearchQuery { - name: Some(r.to.clone()), - ..Default::default() - })? - .into_iter() - .filter(|s| s.name == r.to && s.id != from.id) - .collect(); + // Candidate target symbols (exact name match, any file), cached + // across files. An empty set means the name isn't a declared symbol + // (e.g. a local var) — no edge, the false-positive guard. + if !to_cache.contains_key(&r.to) { + let candidates = store + .symbols_matching(&SymbolSearchQuery { + name: Some(r.to.clone()), + ..Default::default() + })? + .into_iter() + .filter(|s| s.name == r.to) + .collect(); + to_cache.insert(r.to.clone(), candidates); + } + let mut targets: Vec<&IndexedSymbol> = + to_cache[&r.to].iter().filter(|s| s.id != from.id).collect(); if targets.is_empty() { continue; } @@ -466,25 +504,22 @@ fn resolve_references( let same_file: Vec<_> = targets .iter() .filter(|s| s.file_path == *file) - .cloned() + .copied() .collect(); if !same_file.is_empty() { targets = same_file; } else { let same_proj: Vec<_> = targets .iter() - .filter(|s| { - !project_prefix.is_empty() && s.file_path.starts_with(project_prefix) - }) - .cloned() + .filter(|s| same_project_dir(&s.file_path, project_prefix)) + .copied() .collect(); if !same_proj.is_empty() { targets = same_proj; } } - // Deterministic order when an ambiguous name produces multiple - // edges (sort by target file path). + // Deterministic order when an ambiguous name produces multiple edges. targets.sort_by(|a, b| a.file_path.cmp(&b.file_path).then(a.id.cmp(&b.id))); for target in targets { store.link_symbol_references(&from.id, &target.id)?; diff --git a/src/indexer/tree_sitter.rs b/src/indexer/tree_sitter.rs index 04818f6..03d90c0 100644 --- a/src/indexer/tree_sitter.rs +++ b/src/indexer/tree_sitter.rs @@ -30,7 +30,7 @@ pub fn extract(path: &str, lang: Language, text: &str) -> Result Ok(Vec::new()), } .unwrap_or_else(|e| { - tracing::debug!("symbol extraction failed for {path}: {e}"); + tracing::warn!("symbol extraction failed for {path}: {e}"); Vec::new() }); @@ -1125,7 +1125,7 @@ pub fn extract_supertypes(path: &str, lang: Language, text: &str) -> Vec Ok(Vec::new()), }; let mut out = result.unwrap_or_else(|e| { - tracing::debug!("supertype extraction failed for {path}: {e}"); + tracing::warn!("supertype extraction failed for {path}: {e}"); Vec::new() }); out.sort_by(|a, b| { @@ -1468,7 +1468,7 @@ pub fn extract_references(path: &str, lang: Language, text: &str) -> Vec Ok(Vec::new()), }; let mut out = result.unwrap_or_else(|e| { - tracing::debug!("reference extraction failed for {path}: {e}"); + tracing::warn!("reference extraction failed for {path}: {e}"); Vec::new() }); out.sort_by(|a, b| a.from.cmp(&b.from).then_with(|| a.to.cmp(&b.to))); diff --git a/tests/unit.rs b/tests/unit.rs index 64e312b..05308b3 100644 --- a/tests/unit.rs +++ b/tests/unit.rs @@ -1206,6 +1206,156 @@ fn reference_local_variable_creates_no_edge() { let _ = std::fs::remove_dir_all(&tmp); } +/// Store reference/relation lookups must be case-insensitive to match the +/// case-insensitive seed: `symbol_references("widget")` must find usages of the +/// declared `Widget`, otherwise `--symbol widget` would seed on the declaration +/// but return no usages. +#[test] +fn reference_lookup_is_case_insensitive() { + use synapse::indexer::index_repo; + use synapse::repo::Repo; + + let tmp = std::env::temp_dir().join(format!("synapse-ref-ci-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&tmp); + std::fs::create_dir_all(&tmp).unwrap(); + std::fs::write( + tmp.join("widget.rs"), + "pub struct Widget; impl Widget { pub fn new() -> Widget { Widget } }", + ) + .unwrap(); + std::fs::write(tmp.join("app.rs"), "fn run() { let _w = Widget::new(); }").unwrap(); + + let repo = Repo { root: tmp.clone() }; + let store = MemoryGraphStore::new(); + index_repo( + &repo, + &SynapseConfig::default(), + &store, + true, + false, + "2026-06-01T00:00:00+00:00", + None, + ) + .unwrap(); + + // Lower-case query resolves to the PascalCase declaration's referrers. + let refs = store.symbol_references("widget").unwrap(); + assert!( + refs.iter().any(|r| r.path == "app.rs"), + "case-insensitive lookup must find Widget references via `widget`: {refs:?}" + ); + // related_to_symbol (the pack/related consumer path) too. + let related = store.related_to_symbol("WIDGET", 1).unwrap(); + assert!( + related.iter().any(|r| r.path == "app.rs" && r.depth == 1), + "case-insensitive related_to_symbol must include the caller: {related:?}" + ); + + let _ = std::fs::remove_dir_all(&tmp); +} + +/// Segment-safe same-project resolution: a declaration in `src2/` must NOT be +/// treated as same-project for a referrer in `src/` just because "src" is a +/// string prefix of "src2". With two ambiguous declarations (src/ and src2/), +/// the src/ referrer must resolve to the src/ declaration only — proving the +/// same-project filter fell on a path boundary, not a raw prefix. +#[test] +fn reference_same_project_is_segment_safe() { + use synapse::indexer::index_repo; + use synapse::repo::Repo; + + let tmp = std::env::temp_dir().join(format!("synapse-ref-seg-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&tmp); + std::fs::create_dir_all(tmp.join("src")).unwrap(); + std::fs::create_dir_all(tmp.join("src2")).unwrap(); + // Same type name declared in both src/ and src2/. + std::fs::write( + tmp.join("src/Foo.cs"), + "namespace A { public class Foo {} }", + ) + .unwrap(); + std::fs::write( + tmp.join("src2/Foo.cs"), + "namespace B { public class Foo {} }", + ) + .unwrap(); + // Referrer in src/ (NOT src2/). project_prefix is "src"; "src2/Foo.cs" + // starts_with("src") but is a different directory. + std::fs::write( + tmp.join("src/User.cs"), + "namespace A { class User { void Go() { var x = new Foo(); } } }", + ) + .unwrap(); + + let repo = Repo { root: tmp.clone() }; + let store = MemoryGraphStore::new(); + index_repo( + &repo, + &SynapseConfig::default(), + &store, + true, + false, + "2026-06-01T00:00:00+00:00", + None, + ) + .unwrap(); + + // Exactly one edge: the src/ referrer resolves to the src/ Foo only. If the + // prefix check were unsafe, src2/Foo would also count as same-project and + // the same-project narrowing would behave differently. + let n = store.stats().unwrap().reference_edges; + assert_eq!( + n, 1, + "src/ referrer must link only the src/ Foo (segment-safe), got {n} edges" + ); + + let _ = std::fs::remove_dir_all(&tmp); +} + +/// Removing a file must prune reference edges that *point at* a symbol it +/// declared, not just edges that originate from it — otherwise the +/// `referenceEdges` stat is inflated by dangling edges. +#[test] +fn remove_file_prunes_incoming_reference_edges() { + use synapse::indexer::index_repo; + use synapse::repo::Repo; + + let tmp = std::env::temp_dir().join(format!("synapse-ref-rm-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&tmp); + std::fs::create_dir_all(&tmp).unwrap(); + // Widget declared ONLY in widget.rs; app.rs references it cross-file. + std::fs::write(tmp.join("widget.rs"), "pub struct Widget;").unwrap(); + std::fs::write(tmp.join("app.rs"), "fn run() -> Widget { Widget {} }").unwrap(); + let repo = Repo { root: tmp.clone() }; + let store = MemoryGraphStore::new(); + index_repo( + &repo, + &SynapseConfig::default(), + &store, + true, + false, + "2026-06-01T00:00:00+00:00", + None, + ) + .unwrap(); + let before = store.stats().unwrap().reference_edges; + assert!( + before >= 1, + "expected at least one reference edge, got {before}" + ); + + // Removing the file that DECLARES Widget must prune the incoming edge from + // app.rs (whose `to` endpoint no longer exists), not leave it dangling. + store.remove_file("widget.rs").unwrap(); + let after = store.stats().unwrap().reference_edges; + assert_eq!( + after, 0, + "incoming reference edges into removed widget.rs must be pruned: before={before} after={after}" + ); + + let _ = std::fs::remove_dir_all(&tmp); +} + #[test] fn pack_format_parsing() { use synapse::pack::PackFormat;