diff --git a/crates/codegraph-cli/tests/sync_incremental.rs b/crates/codegraph-cli/tests/sync_incremental.rs index 632c2d3..9afaa90 100644 --- a/crates/codegraph-cli/tests/sync_incremental.rs +++ b/crates/codegraph-cli/tests/sync_incremental.rs @@ -256,6 +256,78 @@ fn sync_after_file_removal_equals_index_force() { .expect("sync after file removal must equal a full index --force from scratch"); } +/// A JavaScript `add` that competes, by name alone, with `src/math.ts`'s `add`. +/// While both are indexed, `Counter.increment`'s same-file call to `add` +/// resolves by exact name at a lower confidence than when `add` is unique, so +/// the competitor arriving or leaving changes an edge inside an unchanged file. +fn write_competing_add(project: &Path) { + fs::create_dir_all(project.join("web")).unwrap(); + fs::write( + project.join("web/bundle.js"), + "function add(a, b) {\n return a + b;\n}\n", + ) + .unwrap(); +} + +#[test] +fn sync_after_removing_a_same_named_competitor_equals_index_force() { + assert_sync_equals_index_force("remove-competitor", write_competing_add, |project| { + fs::remove_file(project.join("web/bundle.js")).unwrap(); + }); +} + +#[test] +fn sync_after_adding_a_same_named_competitor_equals_index_force() { + assert_sync_equals_index_force("add-competitor", |_| {}, write_competing_add); +} + +#[test] +fn sync_after_exporting_a_same_named_candidate_equals_index_force() { + // An `export` added on the same line keeps the node's id, yet it lets + // `src/f.ts`'s `foo` tie with `src/h.ts`'s for `main`'s bare call. + assert_sync_equals_index_force( + "export-candidate", + |project| { + fs::write( + project.join("src/h.ts"), + "export function foo(): number {\n return 1;\n}\n", + ) + .unwrap(); + fs::write( + project.join("src/f.ts"), + "function foo(): number {\n return 2;\n}\n", + ) + .unwrap(); + fs::write( + project.join("src/g.ts"), + "function main(): number {\n return foo();\n}\n\nmain();\n", + ) + .unwrap(); + }, + |project| { + fs::write( + project.join("src/f.ts"), + "export function foo(): number {\n return 2;\n}\n", + ) + .unwrap(); + }, + ); +} + +#[test] +fn sync_after_excluding_a_same_named_competitor_equals_index_force() { + // `[indexing] exclude` takes the competitor out of scope while it stays on + // disk: the same removal as a delete, reached through the scan. + assert_sync_equals_index_force("exclude-competitor", write_competing_add, |project| { + fs::create_dir_all(project.join(".codegraph")).unwrap(); + fs::write( + project.join(".codegraph/config.toml"), + "[app]\nname = \"mini\"\n\n[indexing]\nexclude = [\"web/\"]\n", + ) + .unwrap(); + }); +} + fn prepend_game_flow_comment(project: &Path) { let path = project.join("game_flow.gd"); let original = fs::read_to_string(&path).unwrap(); diff --git a/crates/codegraph-store/src/queries.rs b/crates/codegraph-store/src/queries.rs index 539d78e..b7a3ad9 100644 --- a/crates/codegraph-store/src/queries.rs +++ b/crates/codegraph-store/src/queries.rs @@ -1613,6 +1613,32 @@ impl Store { pub fn reference_sites_of_edges_to_named_targets( &self, names: &[String], + ) -> rusqlite::Result> { + self.reference_sites_to_named_targets(names, "!=") + } + + /// Same-file counterpart of + /// [`Self::reference_sites_of_edges_to_named_targets`]: surviving + /// non-`contains` edges whose source and target share a file and whose + /// target name is selected. An exact-name resolution's confidence counts every + /// node of that name in the project, so a competitor arriving in or leaving + /// another file changes an edge inside a file nobody touched. Sync selects + /// only names whose candidates changed, not names whose nodes merely moved: a + /// candidate outside the referencing file is scored without its position, and + /// the same-file target outranks it either way. + pub fn reference_sites_of_same_file_edges_to_named_targets( + &self, + names: &[String], + ) -> rusqlite::Result> { + self.reference_sites_to_named_targets(names, "=") + } + + /// The body of both named-target site queries; `file_relation` is the SQL + /// operator relating the edge's source and target files, `!=` or `=`. + fn reference_sites_to_named_targets( + &self, + names: &[String], + file_relation: &str, ) -> rusqlite::Result> { if names.is_empty() { return Ok(Vec::new()); @@ -1632,7 +1658,7 @@ impl Store { JOIN nodes src ON src.id = e.source WHERE tgt.name IN ({placeholders}) AND e.kind != 'contains' - AND src.file_path != tgt.file_path + AND src.file_path {file_relation} tgt.file_path ORDER BY src.file_path, e.source, e.line, e.col"# ); let params = chunk @@ -3611,6 +3637,28 @@ mod tests { file_path: "fallback.rs".to_string(), site: None, })); + assert!( + named.iter().all(|entry| entry.file_path != "b.rs"), + "the cross-file query leaves same-file edges out" + ); + let same_file = store + .reference_sites_of_same_file_edges_to_named_targets(&[ + "callee".to_string(), + "sibling".to_string(), + ]) + .unwrap(); + assert_eq!( + same_file, + vec![FileReferenceSite { + file_path: "b.rs".to_string(), + site: Some(ReferenceSite { + from_node_id: "function:self".to_string(), + line: 10, + col: 2, + }), + }], + "the same-file query returns exactly the edge whose source shares its target's file" + ); let unresolved = |name: &str, line: i64| UnresolvedRef { id: None, diff --git a/crates/codegraph-watch/src/sync.rs b/crates/codegraph-watch/src/sync.rs index 0e951f4..25d9b53 100644 --- a/crates/codegraph-watch/src/sync.rs +++ b/crates/codegraph-watch/src/sync.rs @@ -11,7 +11,7 @@ use codegraph_core::IndexPaths; use codegraph_core::config::Config; use codegraph_core::generated_header::detect_generated_file; use codegraph_core::node_id::hash_content; -use codegraph_core::types::FileRecord; +use codegraph_core::types::{FileRecord, Node}; use codegraph_extract::{ ExtensionOverrides, ExtractOptions, SourceText, detect_language_with, extract_file_with_options, is_source_file, read_source_file, @@ -549,7 +549,7 @@ fn sync_paths_with_store( let mut dependent_sites = BTreeMap::new(); let mut dependent_fallbacks = BTreeSet::new(); let mut reindexed = HashSet::new(); - let mut changed_names = HashSet::new(); + let mut changed_names = ChangedNames::default(); let paths = paths.into_iter().collect::>(); let total = paths.len(); @@ -597,12 +597,20 @@ fn sync_paths_with_store( } if changed { - let name_list: Vec = changed_names.iter().cloned().collect(); + let name_list: Vec = changed_names.any.iter().cloned().collect(); for affected in store.reference_sites_of_edges_to_named_targets(&name_list)? { if !reindexed.contains(&affected.file_path) { merge_dependent_site(&mut dependent_sites, &mut dependent_fallbacks, affected); } } + let candidate_list: Vec = changed_names.candidates.iter().cloned().collect(); + for affected in + store.reference_sites_of_same_file_edges_to_named_targets(&candidate_list)? + { + if !reindexed.contains(&affected.file_path) { + merge_dependent_site(&mut dependent_sites, &mut dependent_fallbacks, affected); + } + } let mut resolver = ReferenceResolver::new(project_root.to_string_lossy()) .with_max_file_size(scope.options.max_file_size); { @@ -635,7 +643,7 @@ fn sync_paths_with_store( store, &scope_files, &scope_sites, - &changed_names, + &changed_names.any, )?; // Cross-file framework finalization on every sync (upstream index.ts:464). resolver.run_post_extract(store)?; @@ -841,7 +849,7 @@ fn sync_one( outcome: &mut SyncOutcome, dependent_sites: &mut BTreeMap>, dependent_fallbacks: &mut BTreeSet, - changed_names: &mut HashSet, + changed_names: &mut ChangedNames, force_absent: bool, ) -> Result { let full = project_root.join(relative); @@ -922,7 +930,7 @@ fn remove_tracked_file( outcome: &mut SyncOutcome, dependent_sites: &mut BTreeMap>, dependent_fallbacks: &mut BTreeSet, - changed_names: &mut HashSet, + changed_names: &mut ChangedNames, ) -> Result { let was_tracked = store.file_by_path(relative)?.is_some(); if !was_tracked { @@ -932,9 +940,7 @@ fn remove_tracked_file( for dependent in store.reference_sites_dependent_on_file(relative)? { merge_dependent_site(dependent_sites, dependent_fallbacks, dependent); } - for name in node_names_in_file(store, relative)? { - changed_names.insert(name); - } + changed_names.note_removed(node_names_in_file(store, relative)?); delete_unresolved_refs_by_file(store, relative)?; store.delete_file_record(relative)?; outcome.files_removed += 1; @@ -947,7 +953,7 @@ fn reextract_into_store( store: &mut Store, relative: &str, scope: &ProjectScope, - changed_names: &mut HashSet, + changed_names: &mut ChangedNames, (metadata, source, content_hash): (&fs::Metadata, &SourceText, String), ) -> Result<()> { let result = codegraph_extract::engine::extraction_of(relative, source, &scope.options, |_| {}); @@ -979,24 +985,26 @@ fn reextract_into_store( }; // A name's resolution outcomes (confidence, chosen target) depend on the set - // of nodes carrying that name. Only names whose node identity in THIS file - // changed — a node id present before but not after, or vice versa — can alter - // any ref's resolution; a name whose `(id)` set is unchanged resolves exactly - // as before. Editing the tail of a file (no line shift for earlier symbols) - // therefore contributes no names, keeping the re-resolve scope minimal. - let old_nodes: HashSet<(String, String)> = store - .nodes_by_file_path(relative)? - .into_iter() - .map(|node| (node.id, node.name)) + // of nodes carrying that name. A node id present before but not after, or + // vice versa, touches its name; so does a node that keeps its id but changes + // anything else resolution reads, such as an added `export` + // (`ChangedNames::note_candidates`). Editing the tail of a file (no line + // shift for earlier symbols) therefore contributes no names, keeping the + // re-resolve scope minimal. + let old_nodes = store.nodes_by_file_path(relative)?; + let old_ids: HashSet<(&str, &str)> = old_nodes + .iter() + .map(|node| (node.id.as_str(), node.name.as_str())) .collect(); - let new_nodes: HashSet<(String, String)> = result + let new_ids: HashSet<(&str, &str)> = result .nodes .iter() - .map(|node| (node.id.clone(), node.name.clone())) + .map(|node| (node.id.as_str(), node.name.as_str())) .collect(); - for (_, name) in old_nodes.symmetric_difference(&new_nodes) { - changed_names.insert(name.clone()); + for (_, name) in old_ids.symmetric_difference(&new_ids) { + changed_names.any.insert((*name).to_string()); } + changed_names.note_candidates(&old_nodes, &result.nodes); delete_unresolved_refs_by_file(store, relative)?; store.delete_file_record(relative)?; @@ -1007,6 +1015,75 @@ fn reextract_into_store( Ok(()) } +/// The names a sync's changed files touched, which the resolution of refs in +/// the files it did not reindex depends on. +#[derive(Debug, Default)] +struct ChangedNames { + /// Every touched name: a node of it arrived, left, changed id (a move + /// included), or changed anything else resolution reads. Cross-file edges to + /// such a name are re-resolved, and so are the unresolved refs naming it. + any: HashSet, + /// The touched names whose candidates changed as a ref outside their file + /// sees them: a node arrived, left, or changed anything but its position. + /// Only these can move an edge whose source and target share an unchanged + /// file, so only these re-resolve such edges; a line shift elsewhere does not. + candidates: HashSet, +} + +impl ChangedNames { + /// Every node of a file left the index: all its names are touched. + fn note_removed(&mut self, names: HashSet) { + for name in names { + self.candidates.insert(name.clone()); + self.any.insert(name); + } + } + + /// Record each name whose nodes differ between `before` and `after` as a ref + /// outside the file sees them, comparing the two sides as multisets of + /// [`candidate_key`]s. + fn note_candidates(&mut self, before: &[Node], after: &[Node]) { + let mut keys: BTreeMap<&str, (Vec, Vec)> = BTreeMap::new(); + for node in before { + keys.entry(&node.name) + .or_default() + .0 + .push(candidate_key(node)); + } + for node in after { + keys.entry(&node.name) + .or_default() + .1 + .push(candidate_key(node)); + } + for (name, (mut old, mut new)) in keys { + old.sort_unstable(); + new.sort_unstable(); + if old != new { + self.any.insert(name.to_string()); + self.candidates.insert(name.to_string()); + } + } + } +} + +/// A node as a ref in ANOTHER file sees it during resolution: every field but +/// its id, its position and its timestamp. Two nodes with equal keys are +/// interchangeable candidates for such a ref wherever they sit in their own +/// file, since name matching reads a candidate's line only when the candidate +/// shares the ref's file. A node that cannot be serialized keys by its id, so a +/// move still counts as a change. +fn candidate_key(node: &Node) -> String { + let mut key = node.clone(); + key.id.clear(); + key.start_line = 0; + key.end_line = 0; + key.start_column = 0; + key.end_column = 0; + key.updated_at = 0; + serde_json::to_string(&key).unwrap_or_else(|_| format!("unserializable:{}", node.id)) +} + fn node_names_in_file(store: &Store, relative: &str) -> Result> { Ok(store .nodes_by_file_path(relative)? @@ -1609,6 +1686,68 @@ pub(crate) mod tests { ); } + #[test] + fn candidate_changes_ignore_a_move_and_catch_everything_else() { + use codegraph_core::types::{Language, NodeKind}; + + // Given: one `add` function, described by its id, line and export flag. + let node = |id: &str, line: i64, exported: bool| Node { + id: id.to_string(), + kind: NodeKind::Function, + name: "add".to_string(), + qualified_name: "add".to_string(), + file_path: "src/math.ts".to_string(), + language: Language::TypeScript, + start_line: line, + end_line: line + 2, + start_column: 0, + end_column: 1, + docstring: None, + signature: None, + visibility: None, + is_exported: exported, + is_async: false, + is_static: false, + is_abstract: false, + decorators: Vec::new(), + type_parameters: Vec::new(), + return_type: None, + updated_at: line, + }; + let changed = |before: &[Node], after: &[Node]| { + let mut names = ChangedNames::default(); + names.note_candidates(before, after); + assert!(names.candidates.is_subset(&names.any)); + names.candidates + }; + + // Then: a move (new id, lines and timestamp, nothing else) is no candidate change. + assert!( + changed( + &[node("function:a", 1, true)], + &[node("function:b", 9, true)] + ) + .is_empty() + ); + // And: an `export` added on the same line keeps the id and still counts. + assert!( + changed( + &[node("function:a", 1, false)], + &[node("function:a", 1, true)] + ) + .contains("add") + ); + // And: a node arriving counts, and so does one of two identical nodes leaving. + assert!(changed(&[], &[node("function:a", 1, true)]).contains("add")); + assert!( + changed( + &[node("function:a", 1, true), node("function:b", 9, true)], + &[node("function:a", 1, true)], + ) + .contains("add") + ); + } + #[test] fn interrupted_index_heals_on_bare_sync_and_clears_marker() { use codegraph_core::types::{EdgeKind, Language, UnresolvedRef};