Refactor(diskann-benchmark): consolidate disk search config under DiskSearchMode by dyhyfu · Pull Request #1232 · microsoft/DiskANN

dyhyfu · 2026-07-07T09:09:45Z

Summary

Consolidates disk-index search configuration under DiskSearchMode and decouples the benchmark's JSON input schema from the optional diskann-disk backend.

Previously, the input schema both defined the config and constructed diskann_disk::SearchMode, forcing several #[cfg(feature = "disk-index")] gates and scattering related fields (vector_filters_file, post_processor) across DiskSearchPhase. This PR moves the backend-specific construction into the search execution path and groups the search-mode fields together.

Changes

Move SearchMode construction to the backend. The match logic that builds diskann_disk::SearchMode now lives in a build_search_mode helper in the search execution module, so the input schema is pure config data with no dependency on the disk backend's SearchMode type.
Consolidate fields into DiskSearchMode. vector_filters_file and post_processor are nested inside DiskSearchMode alongside is_flat_search and adaptive_l. Validation and Display moved accordingly.
Remove unnecessary feature gates. DiskSearchMode and its fields no longer need #[cfg(feature = "disk-index")]. The only remaining gates are for QuantizationType, which is a diskann-disk type (intentionally left as-is — see below).
Migrate JSON fixtures (examples and perf test inputs) to the nested search_mode format.

…e construction to search.rs

Copilot

Pull request overview

This PR refactors the diskann-benchmark disk-index benchmark input schema to consolidate disk search configuration under DiskSearchMode, and moves diskann_disk::SearchMode construction out of the JSON schema layer into the disk search execution path.

Changes:

Nested vector_filters_file and post_processor under DiskSearchMode, alongside is_flat_search and adaptive_l, and moved validation accordingly.
Added a build_search_mode helper in disk_index/search.rs to construct backend SearchMode at execution time.
Updated benchmark JSON fixtures (examples + perf inputs) to the new nested search_mode format.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
diskann-benchmark/src/inputs/disk.rs	Refactors disk-index JSON schema to centralize mode-specific config/validation in `DiskSearchMode`.
diskann-benchmark/src/disk_index/search.rs	Builds backend `SearchMode` during execution using new helper; updates access paths to nested config.
diskann-benchmark/perf_test_inputs/wikipedia-100K-disk-index.json	Migrates perf input to nested `search_mode` object.
diskann-benchmark/perf_test_inputs/openai-100K-disk-index.json	Migrates perf input to nested `search_mode` object.
diskann-benchmark/example/disk-index.json	Migrates example input to nested `search_mode` object.
diskann-benchmark/example/disk-index-filter.json	Migrates filter example to nested `search_mode.vector_filters_file`.
diskann-benchmark/example/disk-index-determinant-diversity.json	Migrates post-processor example to nested `search_mode.post_processor`.

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

codecov-commenter · 2026-07-07T09:23:56Z

Codecov Report

❌ Patch coverage is 42.46575% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.44%. Comparing base (2ee97ff) to head (a624d45).

Files with missing lines	Patch %	Lines
diskann-benchmark/src/inputs/disk.rs	42.46%	42 Missing ⚠️

❌ Your patch status has failed because the patch coverage (42.46%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1232      +/-   ##
==========================================
- Coverage   91.46%   91.44%   -0.03%     
==========================================
  Files         516      516              
  Lines       98276    98340      +64     
==========================================
+ Hits        89891    89928      +37     
- Misses       8385     8412      +27

Flag	Coverage Δ
miri	`91.44% <42.46%> (-0.03%)`	⬇️
unittests	`91.12% <42.46%> (-0.03%)`	⬇️

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

Files with missing lines	Coverage Δ
diskann-benchmark/src/main.rs	`92.05% <ø> (ø)`
diskann-benchmark/src/inputs/disk.rs	`12.91% <42.46%> (+11.46%)`	⬆️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:

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

suri-kumkaran · 2026-07-14T14:15:44Z

 #[derive(Debug, Serialize, Deserialize, Default)]
 pub(crate) struct DiskSearchMode {
    pub(crate) is_flat_search: bool,
    #[serde(default)]
    pub(crate) adaptive_l: Option<AdaptiveL>,
+    #[serde(default)]
+    pub(crate) vector_filters_file: Option<InputFile>,
+    #[serde(default)]
+    pub(crate) post_processor: Option<TopkPostProcessor>,
 }


Can we use a enum here to avoid impossible combinations? Something similar and in spirit of:

pub enum SearchMode<'a> { FlatScan { filter: Option<SearchPredicate<'a>>, }, Graph { filter: Option<SearchPredicate<'a>>, }, InlineFilter { filter: Box<dyn QueryLabelProvider<u32> + 'a>, adaptive_l: Option<AdaptiveL>, }, DiverseGraph { filter: Option<SearchPredicate<'a>>, params: DeterminantDiversityParams, }, }

I understand SearchMode is per query and has relevant types, but can we have something similiar on the benchmark input level eliminating the impossible scenarios?

Copilot

Pull request overview

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

Suppressed comments (2)

diskann-benchmark/src/inputs/disk.rs:72

DiskSearchMode does not use #[serde(deny_unknown_fields)], so typos or legacy keys inside the nested search_mode object (e.g. { "mode": "graph", "is_flat_search": true }) will be silently ignored by Serde. This undermines the intent of adding #[serde(deny_unknown_fields)] on DiskSearchPhase to hard-fail old schemas.

Consider denying unknown fields on DiskSearchMode as well so invalid/legacy keys under search_mode are rejected deterministically.

#[derive(Debug, Serialize, Deserialize)]
#[serde(tag = "mode", rename_all = "kebab-case")]
pub(crate) enum DiskSearchMode {

diskann-benchmark/src/inputs/disk.rs:138

DiskSearchMode::Graph currently allows specifying both adaptive_l and post_processor, but build_search_mode will always pick the determinant-diversity SearchMode::{diverse_graph,_} when post_processor is set, effectively ignoring adaptive_l. Since diskann_disk::search::search_mode::SearchMode::DiverseGraph has no adaptive_l support, this should be rejected (or at least made explicit) rather than silently dropping part of the config.

            Self::Graph {
                adaptive_l,
                vector_filters_file,
                post_processor,
            } => {
                if let Some(adaptive_l) = adaptive_l.as_mut() {

Copilot

Pull request overview

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

Suppressed comments (3)

diskann-benchmark/src/main.rs:721

Using save_path.to_str().unwrap() can panic on non-UTF-8 temp paths (e.g., if TMPDIR contains non-UTF-8). Since this is only for test JSON rewriting, prefer to_string_lossy() to avoid spurious test failures on such environments.

            let save_path = tempdir.path().join(format!("disk_index_filter_job_{i}"));
            job["content"]["source"]["save_path"] =
                serde_json::Value::String(save_path.to_str().unwrap().to_string());

diskann-benchmark/src/inputs/disk.rs:72

DiskSearchMode does not deny unknown fields, so typos or legacy fields nested under search_mode (e.g. { "mode": "graph", "is_flat_search": true }) may be silently ignored during deserialization. Adding deny_unknown_fields here would make the JSON schema stricter and align with the intent of rejecting legacy/unknown parameters.

#[derive(Debug, Serialize, Deserialize)]
#[serde(tag = "mode", rename_all = "kebab-case")]
pub(crate) enum DiskSearchMode {

diskann-benchmark/src/disk_index/search.rs:165

The doc comment for build_search_mode says the post-processor is supplied at search time, but the post-processor now comes from the JSON-driven DiskSearchMode config (only the vector filter is per-query). Updating this comment would avoid confusion about where the post-processor is sourced.

/// Construct the disk [`SearchMode`] from the JSON-driven [`DiskSearchMode`]
/// config plus the per-query filter and post-processor supplied at search time.
fn build_search_mode<'a>(

Copilot

Pull request overview

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

Suppressed comments (3)

diskann-benchmark/src/inputs/disk.rs:133

DiskSearchMode::Graph allows both adaptive_l and post_processor to be set, but the search execution path currently ignores adaptive_l whenever a post_processor is present (see build_search_mode in disk_index/search.rs, which matches determinant-diversity first and discards the computed adaptive_l). This makes part of the JSON config silently ineffective. Consider rejecting this combination during validation (or otherwise making the precedence explicit).

            Self::Graph {
                adaptive_l,
                vector_filters_file,
                post_processor,
            } => {

diskann-benchmark/src/disk_index/search.rs:165

Doc comment for build_search_mode says the post-processor is "supplied at search time", but the function signature only takes mode and vector_filter (the post-processor comes from DiskSearchMode::Graph { post_processor, .. }). This is misleading when reading the code and debugging configuration-driven behavior.

/// Construct the disk [`SearchMode`] from the JSON-driven [`DiskSearchMode`]
/// config plus the per-query filter and post-processor supplied at search time.
fn build_search_mode<'a>(

diskann-benchmark/src/inputs/disk.rs:80

Doc comment for DiskSearchMode::Graph says it can be used with adaptive-L, vector filters, and/or a post-processor. However the backend SearchMode does not support combining determinant-diversity post-processing with adaptive_l (and build_search_mode currently drops adaptive_l when post_processor is set). If you enforce mutual exclusivity in validation, this comment should be updated to avoid implying the combination is supported.

This issue also appears on line 129 of the same file.

    /// Greedy graph search, optionally with inline adaptive-L, a per-query
    /// vector filter, and/or a top-k post-processor.
    Graph {

Copilot

Pull request overview

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

Suppressed comments (4)

diskann-benchmark/src/inputs/disk.rs:88

DiskSearchMode does not deny unknown fields, so serde will silently ignore unexpected keys (e.g., {"mode":"flat","adaptive_l":...} would deserialize as Flat and drop adaptive_l). That undermines the goal of making invalid combinations unrepresentable and makes typos easy to miss; consider denying unknown fields for the enum variants.

#[derive(Debug, Serialize, Deserialize)]
#[serde(tag = "mode", rename_all = "kebab-case")]
pub(crate) enum DiskSearchMode {
    /// Brute-force flat scan, optionally restricted by a per-query vector filter.
    Flat {

diskann-benchmark/src/inputs/disk.rs:144

DiskSearchMode::Graph currently allows adaptive_l and post_processor to be set together, but build_search_mode will always pick SearchMode::DiverseGraph when a post-processor is present, silently ignoring adaptive_l. Consider rejecting this combination during validation to avoid surprising config behavior.

            Self::Graph {
                adaptive_l,
                vector_filters_file,
                post_processor,
            } => {

diskann-benchmark/src/main.rs:721

This test builds a JSON string path via save_path.to_str().unwrap(), which can panic on non-UTF8 paths. Using to_string_lossy() avoids a hard panic and is consistent with other path-to-string conversions in the benchmark code.

        for (i, job) in jobs.iter_mut().enumerate() {
            let save_path = tempdir.path().join(format!("disk_index_filter_job_{i}"));
            job["content"]["source"]["save_path"] =
                serde_json::Value::String(save_path.to_str().unwrap().to_string());

diskann-benchmark/src/disk_index/search.rs:165

The doc comment for build_search_mode says the post-processor is supplied at search time, but the implementation reads it from DiskSearchMode::Graph { post_processor, .. } (JSON config). Updating the comment would avoid confusion about where this value comes from.

/// Construct the disk [`SearchMode`] from the JSON-driven [`DiskSearchMode`]
/// config plus the per-query filter and post-processor supplied at search time.
fn build_search_mode<'a>(

yaohongdeng added 2 commits July 7, 2026 17:03

consolidate disk search config into DiskSearchMode and move SearchMod…

5c86c16

…e construction to search.rs

Merge branch 'main' into u/yaohongdeng/fixDiskModeBenchmarkCfg

3014754

dyhyfu requested review from a team and a lite review from Copilot July 7, 2026 09:09

Copilot started reviewing on behalf of dyhyfu July 7, 2026 09:10 View session

Copilot AI reviewed Jul 7, 2026

View reviewed changes

Comment thread diskann-benchmark/src/inputs/disk.rs

Comment thread diskann-benchmark/src/inputs/disk.rs

Comment thread diskann-benchmark/src/disk_index/search.rs Outdated

hildebrandmw reviewed Jul 7, 2026

View reviewed changes

Comment thread diskann-benchmark/src/inputs/disk.rs

remove unexpected output files from disk-filter test

6fe7448

wuw92 reviewed Jul 9, 2026

View reviewed changes

Comment thread diskann-benchmark/src/disk_index/search.rs Outdated

Comment thread diskann-benchmark/src/disk_index/search.rs Outdated

yaohongdeng added 3 commits July 14, 2026 18:28

Fix disk benchmark search-mode configuration validation

29907bf

Merge branch 'main' into u/yaohongdeng/fixDiskModeBenchmarkCfg

521ca01

force not tolerant for old is_flat_search json config

ca59956

suri-kumkaran reviewed Jul 15, 2026

View reviewed changes

yaohongdeng added 2 commits August 4, 2026 17:04

update benchmark input for DiskSearchMode, change to enum structure

2e088a2

Merge branch 'main' into u/yaohongdeng/fixDiskModeBenchmarkCfg

968b79c

Copilot AI review requested due to automatic review settings August 4, 2026 09:05

Copilot started reviewing on behalf of dyhyfu August 4, 2026 09:06 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

fix format issue

acd030b

Copilot AI review requested due to automatic review settings August 4, 2026 09:10

Copilot started reviewing on behalf of dyhyfu August 4, 2026 09:10 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

remove unused function

92d8cae

Copilot AI review requested due to automatic review settings August 4, 2026 09:54

Copilot started reviewing on behalf of dyhyfu August 4, 2026 09:55 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

change benchmark baseline config file to use main branch config file

a624d45

Copilot AI review requested due to automatic review settings August 5, 2026 02:17

Copilot started reviewing on behalf of dyhyfu August 5, 2026 02:18 View session

Copilot AI reviewed Aug 5, 2026

View reviewed changes

Navigation Menu

Uh oh!

Refactor(diskann-benchmark): consolidate disk search config under DiskSearchMode - #1232

Refactor(diskann-benchmark): consolidate disk search config under DiskSearchMode#1232
dyhyfu wants to merge 11 commits into
mainfrom
u/yaohongdeng/fixDiskModeBenchmarkCfg

dyhyfu commented Jul 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Jul 7, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

suri-kumkaran Jul 14, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

dyhyfu commented Jul 7, 2026

Summary

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

suri-kumkaran Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov-commenter commented Jul 7, 2026 •

edited

Loading

suri-kumkaran Jul 14, 2026 •

edited

Loading