Refine Vamana index builder architecture

[partychen]

Summary

• replace the trait-object in-memory builder facade with an explicit VamanaBuildIndex enum
• split Vamana construction into private strategy, one-shot, merged, index, and test modules
• align builder and telemetry terminology with the Vamana algorithm
• preserve FP, SQ, PQ, one-shot, and merged build behavior

Why this refinement matters

This is more than a naming cleanup. The previous core/inmem structure obscured the actual algorithm and mixed strategy selection, index dispatch, graph construction, shard merging, and tests in the same surface.

The refined structure makes FP/SQ/PQ dispatch explicit, gives one-shot and merged builds clear ownership boundaries, and removes the dynamic builder facade. This reduces accidental coupling and makes future Vamana changes substantially safer to review, test, and extend without changing index output behavior.

Validation

• cargo fmt --all --check
• cargo test -p diskann-disk --lib build::builder — 22 passed
• cargo clippy -p diskann-disk --all-targets -- -D warnings

[Copilot]

Pull request overview

This PR refactors the diskann-disk Vamana (in-memory) index build path to make FP/SQ/PQ dispatch explicit via a VamanaBuildIndex enum, and reorganizes the builder implementation into clearer strategy/one-shot/merged/index/test modules while preserving build behavior.

Changes:

• Replaces the trait-object in-memory builder facade with VamanaBuildIndex (enum) and moves one-shot build logic into vamana/one_shot.rs.
• Extracts build-strategy selection and RAM estimation into vamana/strategy.rs, and moves merged-shard build/merge logic into vamana/merged.rs.
• Aligns telemetry checkpoint naming from “InmemIndexBuild” to “VamanaIndexBuild” and updates test imports to the new module layout.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
diskann-disk/src/utils/instrumentation/perf_logger.rs	Renames disk build checkpoint to VamanaIndexBuild to match Vamana terminology.
diskann-disk/src/search/provider/disk_provider.rs	Updates tests to import builder fixtures from the new location.
diskann-disk/src/build/builder/vamana/tests.rs	Removes production builder code from this file, keeping only test fixtures/utilities.
diskann-disk/src/build/builder/vamana/strategy.rs	Adds RAM estimation + strategy selection (one-shot vs merged) and corresponding tests.
diskann-disk/src/build/builder/vamana/one_shot.rs	Introduces the one-shot Vamana builder implementation.
diskann-disk/src/build/builder/vamana/merged.rs	Introduces merged (partition + build shards + merge) Vamana builder implementation.
diskann-disk/src/build/builder/vamana/index.rs	Adds VamanaBuildIndex enum encapsulating FP/SQ/PQ async index implementations.
diskann-disk/src/build/builder/vamana/mod.rs	Wires up the new Vamana submodules and re-exports builder-internal entrypoints.
diskann-disk/src/build/builder/tests.rs	Updates tests to import builder fixtures from the new location.
diskann-disk/src/build/builder/mod.rs	Removes core/inmem_builder modules and hooks in the new private vamana module + test re-export.
diskann-disk/src/build/builder/inmem_builder.rs	Deletes the prior trait-object in-memory builder facade.
diskann-disk/src/build/builder/build.rs	Switches disk index build to call the new Vamana builders and logs the renamed checkpoint.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[partychen]

Fixed in 94feca0. save_path now moves into DiskGraphOnly, and the (start_point, DiskGraphOnly) value is bound to graph_output before the .await. I also added direct tests for the one-shot and exact-budget merged strategy decisions to close the reported coverage gap.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 90.27778% with 63 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.64%. Comparing base (b7cf3b0) to head (94feca0).

Files with missing lines	Patch %	Lines
diskann-disk/src/build/builder/vamana/index.rs	77.77%	26 Missing ⚠️
diskann-disk/src/build/builder/vamana/one_shot.rs	86.20%	20 Missing ⚠️
diskann-disk/src/build/builder/vamana/merged.rs	97.00%	9 Missing ⚠️
diskann-disk/src/build/builder/vamana/strategy.rs	88.57%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1306      +/-   ##
==========================================
+ Coverage   90.68%   91.64%   +0.96%     
==========================================
  Files         515      518       +3     
  Lines       99230    98890     -340     
==========================================
+ Hits        89987    90629     +642     
+ Misses       9243     8261     -982     

Flag	Coverage Δ
miri	91.64% <90.27%> (+0.96%)	⬆️
unittests	91.61% <90.27%> (+1.24%)	⬆️

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

Files with missing lines	Coverage Δ
diskann-disk/src/build/builder/build.rs	92.10% <100.00%> (+1.94%)	⬆️
diskann-disk/src/search/provider/disk_provider.rs	94.00% <ø> (ø)
...kann-disk/src/utils/instrumentation/perf_logger.rs	81.35% <100.00%> (ø)
diskann-disk/src/build/builder/vamana/strategy.rs	88.57% <88.57%> (ø)
diskann-disk/src/build/builder/vamana/merged.rs	97.00% <97.00%> (ø)
diskann-disk/src/build/builder/vamana/one_shot.rs	86.20% <86.20%> (ø)
diskann-disk/src/build/builder/vamana/index.rs	77.77% <77.77%> (ø)

... and 45 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (2)

diskann-disk/src/utils/instrumentation/perf_logger.rs:28

• DiskIndexBuildCheckpoint is logged/spanned via fmt::Display, which currently prints {:?} (Debug). Renaming the enum variant changes the emitted checkpoint string (and OpenTelemetry span name), which can break any log/metrics consumers that key off the previous value. Consider making Display return explicit, stable strings so future refactors don’t accidentally change telemetry again.

#[derive(Debug)]
pub enum DiskIndexBuildCheckpoint {
    PqConstruction,
    VamanaIndexBuild,
    DiskLayout,

diskann-disk/src/build/builder/mod.rs:11

• diskann_disk::build::builder is part of the crate’s public API (pub mod build; pub mod builder;). Removing the previously-public core and inmem_builder submodules is a breaking change for downstream users that import those paths. If this crate aims to preserve semver compatibility, consider keeping deprecated shim modules (or re-exporting the old paths) for at least one release.

//! Disk index builders and related functionality.
pub mod build;
pub mod quantizer;

pub mod tokio;
mod vamana;

[partychen]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (2)
diskann-disk/src/utils/instrumentation/perf_logger.rs:28

• DiskIndexBuildCheckpoint is logged/spanned via fmt::Display, which currently prints {:?} (Debug). Renaming the enum variant changes the emitted checkpoint string (and OpenTelemetry span name), which can break any log/metrics consumers that key off the previous value. Consider making Display return explicit, stable strings so future refactors don’t accidentally change telemetry again.

#[derive(Debug)]
pub enum DiskIndexBuildCheckpoint {
    PqConstruction,
    VamanaIndexBuild,
    DiskLayout,

diskann-disk/src/build/builder/mod.rs:11

• diskann_disk::build::builder is part of the crate’s public API (pub mod build; pub mod builder;). Removing the previously-public core and inmem_builder submodules is a breaking change for downstream users that import those paths. If this crate aims to preserve semver compatibility, consider keeping deprecated shim modules (or re-exporting the old paths) for at least one release.

//! Disk index builders and related functionality.
pub mod build;
pub mod quantizer;

pub mod tokio;
mod vamana;

These modules were publicly named but contained no downstream-accessible items; their contents were restricted to the crate or parent module. Keeping deprecated empty shims would preserve only unusable namespace paths and perpetuate an accidental API surface, so their removal is intentional.