Skip to content

[diskann-disk] Preserve virtual start point out-edges when writing the disk index - #1483

Open
Dongliang Wu (doliawu) wants to merge 4 commits into
microsoft:mainfrom
doliawu:fix-num-pts-assertion
Open

Dongliang Wu (doliawu) wants to merge 4 commits into
microsoft:mainfrom
doliawu:fix-num-pts-assertion

Conversation

@doliawu

@doliawu Dongliang Wu (doliawu) commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor
  • Does this PR have a descriptive title that could go in our release notes?
  • Does this PR add any new dependencies? No.
  • Does this PR modify any existing APIs? No.
  • Is the change to the API backwards compatible? Yes. No format change; newly built disk indexes have a slightly different start node adjacency list.
  • Should this result in any changes to our documentation, either updating existing docs or adding new ones? No.

Reference Issues/PRs

Follow-up to #1350.

What does this implement/fix? Briefly explain your changes.

The disk build uses the in-memory index, which keeps a virtual start point V (a copy of the medoid's vector) as its search entry point. When the graph is serialized to the disk format, V is folded into the medoid M, but DiskAdaptor kept only M's own adjacency list and dropped V's out-edges. V and M share a vector but build their adjacency lists independently, so V's edges (the ones in-memory search expands first) were lost.

In the worst case, M's in-memory list is just [V] (e.g. small graphs with concurrent inserts). Before #1350 this became a useless [M] self-loop; since #1350 it becomes empty. Either way, disk search, which starts only at the medoid, cannot reach any other point. Debug builds hit debug_assert!(num_nbrs > 0) in DiskIndexWriter::read_neighbors.

Changes:

  • DiskAdaptor now merges V's out-edges into M's disk adjacency list: inherited edges first, then M's own, remapped, deduplicated, without self-loops, and capped at the longer of the two in-memory lists so the serialized max degree does not grow.
  • DiskIndexWriter::read_neighbors allows num_nbrs == 0 for a 1-point graph, where an empty list is valid.
  • Tests: adaptor unit tests for the [V]-only medoid case and the cap, and a disk layout test for a 1-point graph (fails with the old assertion).
  • Fixtures: regenerated truth_sift_learn_R4_L50_disk.index; only the medoid's list changes ([118, 108, 86] -> [7, 2, 10, 118]). The associated-data search test now has its own truth results, because the shared truth file belongs to the prebuilt search index, which does not match current builds.

Any other comments?

Search quality is unchanged. Recall@10 on the 256-point R4/L50 index, using all 256 data points as queries:

L before after
10 0.7695 0.7613
15 0.8688 0.8688
20 0.9059 0.9125
30 0.9367 0.9398
50 0.9672 0.9688

Individual results shift because the medoid's first-hop edges changed (27 of 256 queries at L=20, 3 at L=50), but gains and losses roughly cancel.

Keeping V as a real node in the disk index would preserve the in-memory graph exactly, but needs a format and search change; that could be a follow-up.

Validation:

  • cargo test -p diskann-providers -p diskann-disk: passed
  • cargo fmt --all --check: passed
  • cargo clippy --workspace --all-targets -- -D warnings: passed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The singleton case needs a regression test that verifies successful layout creation and a serialized zero neighbor count.

1 open finding
What changed in this PR

Corrects disk-index writing’s debug assertion to allow a single-point graph with no neighbors.

Changes:

  • Allows zero neighbors when state.num_pts == 1, preserving the existing maximum-degree check.
File Description
diskann-disk/​src/​storage/​disk_index_writer.rs Adds the singleton exception to the neighbor-count assertion.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread diskann-disk/src/storage/disk_index_writer.rs
@codecov-commenter

Codecov Comments Bot (codecov-commenter) commented Oct 14, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.04%. Comparing base (ed8a2ec) to head (4d841df).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1483      +/-   ##
==========================================
+ Coverage   90.93%   91.04%   +0.10%     
==========================================
  Files         586      586              
  Lines      118318   120008    +1690     
==========================================
+ Hits       107597   109262    +1665     
- Misses      10721    10746      +25     
Flag Coverage Δ
miri 91.04% <100.00%> (+0.10%) ⬆️
unittests 90.74% <100.00%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
diskann-disk/src/search/provider/disk_provider.rs 95.93% <100.00%> (ø)
diskann-disk/src/storage/disk_index_writer.rs 96.69% <100.00%> (+0.22%) ⬆️
.../graph/provider/async_/simple_neighbor_provider.rs 97.49% <100.00%> (+0.02%) ⬆️

... and 15 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@doliawu Dongliang Wu (doliawu) changed the title Fix num_nbrs assertion in DiskIndexWriter::read_neighbors [diskann-disk] Fix num_nbrs assertion in DiskIndexWriter::read_neighbors Oct 14, 2026
Dongliang Wu (from Dev Box) and others added 2 commits October 15, 2026 12:52
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e2c6b1e3-5f90-48bb-b41d-4123079c371c
The disk format folds the in-memory virtual start point into the medoid, but
DiskAdaptor only kept the medoid's own adjacency list and dropped the virtual
start point's out-edges. If the medoid only linked to the virtual start point,
the serialized start node had no out-edges, so disk search could not reach any
other point (and debug builds hit the num_nbrs > 0 assertion).

Merge the virtual start point's out-edges into the medoid's list, with the
inherited edges first and capped at the longer of the two in-memory lists.

Regenerate the R4/L50 build truth fixture (only the medoid's list changes) and
give the associated-data search test its own truth results, since the shared
truth file belongs to a prebuilt index.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e2c6b1e3-5f90-48bb-b41d-4123079c371c
@doliawu Dongliang Wu (doliawu) changed the title [diskann-disk] Fix num_nbrs assertion in DiskIndexWriter::read_neighbors [diskann-disk] Preserve virtual start point out-edges when writing the disk index Oct 15, 2026
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e2c6b1e3-5f90-48bb-b41d-4123079c371c

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants