Deduplicate virtual start point edges during disk serialization - #1350
Deduplicate virtual start point edges during disk serialization#1350juchen-ms (partychen) wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a disk-serialization edge case in diskann-providers where remapping the in-memory “virtual start point” ID to the actual medoid ID could introduce duplicate medoid edges and (on the medoid’s own adjacency list) create a self-loop in the serialized disk graph. The change is scoped to the disk-graph adaptor used during serialization and is validated by a new regression test plus an updated disk-index fixture.
Changes:
- Update
DiskAdaptor::get_adjacency_listto remap the virtual start point to the actual start point while preserving neighbor order, deduplicating the resulting medoid edge (keep first), and removing self-loops after remapping. - Add a unit test covering both the remapping collision (duplicate medoid) and medoid self-loop scenarios.
- Update the expected disk-index fixture (Git LFS pointer) to match the corrected serialized graph output.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
diskann-providers/src/model/graph/provider/async_/simple_neighbor_provider.rs |
Adjusts disk-serialization adjacency list remapping to deduplicate the remapped medoid edge and remove self-loops; adds a regression test for both failure modes. |
test_data/disk_index_build/truth_sift_learn_R4_L50_disk.index |
Updates the expected on-disk index fixture pointer to reflect the corrected serialization output. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1350 +/- ##
==========================================
+ Coverage 91.55% 92.53% +0.97%
==========================================
Files 521 521
Lines 100347 100371 +24
==========================================
+ Hits 91877 92876 +999
+ Misses 8470 7495 -975
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Reference Issues/PRs
N/A
What does this implement/fix? Briefly explain your changes.
The in-memory graph uses a virtual start point ID, while the serialized disk graph must replace it with the actual medoid ID.
Previously, this replacement happened after adjacency-list uniqueness had already been established. If a list contained both the virtual start point and the actual medoid, the remapping produced a duplicate medoid edge. On the medoid's own adjacency list, it could also produce a self-loop.
This PR updates
DiskAdaptorto:It does not backfill removed entries with arbitrary neighbors, so affected adjacency lists may contain one fewer unique edge.
The regression test covers both a remapping collision and the medoid self-loop case. The expected disk-index fixture is updated for the corrected serialized graph.
Any other comments?
The in-memory adjacency list is already unique before serialization, so only the alias introduced by virtual-start-point remapping needs post-remap deduplication. This keeps the change limited to disk serialization and does not alter graph construction or pruning behavior.