fix(sync): re-resolve every edge a name's candidate change can move - #303
Conversation
A sync re-resolved the cross-file edges to a name whose nodes changed, but never an edge whose source and target share an unchanged file, although an exact-name resolution's confidence counts every node of that name in the project. Excluding the committed viewer bundle from this repository's index left five such edges at 0.7 where index --force gives 0.9, and adding a competitor leaves them too high. A node that keeps its id while changing what resolution reads, such as an `export` added on the same line, touched no name at all, so a cross-file call kept a target a full index no longer picks. Sync now tells the names a change touched from those whose candidates changed as a ref outside their file sees them: a node arrived, left, or changed anything but its position. Cross-file edges and unresolved refs re-resolve for both kinds; same-file edges re-resolve for the second only. A line shift moves ids but no candidate, so a head edit of main.rs still syncs in 4.5 s, where re-resolving same-file edges for every touched name took 6.8 s.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5bc265708d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// move still counts as a change. | ||
| fn candidate_key(node: &Node) -> String { | ||
| let mut key = node.clone(); | ||
| key.id.clear(); |
There was a problem hiding this comment.
When same-named nodes exchange metadata while remaining on their original lines—for example, renaming two C++ namespaces A and B so their contained foo nodes exchange qualified_name values—the (id, name) sets remain unchanged, and clearing the IDs makes the sorted before/after key multisets identical. Consequently foo is not marked as changed and an existing A::foo edge is not refreshed, although a full rebuild targets the other foo ID. Keep resolution metadata associated with stable node identity while handling pure moves separately so incremental sync still converges with a clean index.
AGENTS.md reference: AGENTS.md:L39-L41
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #303 +/- ##
==========================================
+ Coverage 95.05% 95.06% +0.01%
==========================================
Files 197 197
Lines 108387 108505 +118
==========================================
+ Hits 103023 103147 +124
+ Misses 5364 5358 -6
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
1 participant
Summary
synccould leave the index different fromindex --force, which AGENTS.md lists as a hard invariant. Found while keeping the committed viewer bundle out of this repository's own index: after excluding it, five edges kept confidence 0.7 where a full index gives 0.9.Two holes in how sync decides which surviving edges to re-resolve after a change:
reference_sites_of_edges_to_named_targetsselects only cross-file edges, but 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 whose source and target both sit in an unchanged file. In this repository:api/mod.rs'sread → ok,palette.svelte.ts's calls toscheduleandquery, and two more, against the bundle's minifiedok,schedule,queryandr. The older path-level query's own doc already said "in any file, including the referencing file itself"; the site-level query of fix(sync): refresh only the reference sites a change can reach #201 dropped that.is_exported. Addingexporton the same line therefore left a cross-file call on a target a full index no longer picks (afootie between two files).The fix
Sync now keeps two sets of touched names (
ChangedNamesincrates/codegraph-watch/src/sync.rs):any: a node of the name arrived, left, changed id (a move included), or changed anything else. It drives what it drove before: cross-file edges and unresolved refs.candidates: the subset whose candidates changed as a ref outside their file sees them, meaning a node arrived, left, or changed anything but its position. It is compared as multisets of a key that blanks id, lines, columns and timestamp. It drives the new same-file query,reference_sites_of_same_file_edges_to_named_targets.The split keeps #201's performance. A line shift moves ids but no candidate, and name matching reads a candidate's line only when the candidate shares the ref's file (
find_best_match). A same-file target also gets +100 there, so a candidate elsewhere cannot outrank it by moving. Re-resolving same-file edges for every touched name was correct too, but a head edit ofmain.rsthen took 6.8 s instead of 4.5 s.Tests
Four new cases in
crates/codegraph-cli/tests/sync_incremental.rs, each throughassert_sync_equals_index_force: removing, adding and excluding a same-named competitor, and anexportadded on the same line.main'ssync.rs/queries.rs, for the confidence and the target respectively, while the other eight pass (/tmp/evidence-sync/red-sync-incremental.log).candidate_changes_ignore_a_move_and_catch_everything_else(sync) covers a move, anexportflip, an arrival and a duplicate leaving. The store test pins that the same-file query returns exactly the same-file site and that the cross-file query still leaves it out. Its red is inred-store-unit.log.Verification
make pre-cipassed on this commit: 4348 Rust tests, 0 failed; 569 frontend tests; bundle byte check; archive smoke (/tmp/evidence-sync/pre-ci.log).golden-drift.log). The change touches incremental sync only.main(690 files), nodes, edges and unresolved refs equalindex --forceafter syncing in both directions: excluding the bundle (3,289 nodes leave) and dropping the exclusion (they arrive). The old binary differs by 5 edges each way (equiv-repo-narrowed.log).logger.rs: 0.96 → 0.95 s;main.rs: 4.5 → 4.5 s;perf-*.log).🤖 Generated with Claude Code