Simplify the `DataProvider` contract for graph search by hildebrandmw · Pull Request #1067 · microsoft/DiskANN

hildebrandmw · 2026-05-14T01:43:04Z

Simplify graph search.

What

At a high level, this PR collapses Accessor, BuildQueryComputer, ExpandBeam, SearchExt, and AsNeighbor/NeighborAccessor into a single SearchAccessor trait. Along the way, we have the following changes:

Removes the following traits:
- Accessor: For search, SearchAccessor::expand_beam is now used. The indexing layer now has no notion of element types. For insert, workingset::Fill is used exclusively to get elements.
- BuildQueryComputer: Computing query distances has been fused into SearchAccessor.
- SearchExt: Merged into SearchAccessor.
- ExpandBeam: Merged into SearchAccessor.
  Additionally, search accessors no longer need to define DelegateNeighbor.
SearchStrategy: This gains a lifetime, removes the lifetime from the SearchAccessor associated type, and passes the query to search_accessor. These changes allow the constructed accessor to borrow the query if so desired.
DefaultSearchStrategy/DefaultPostProcessor: Also get the new lifetime to cooperate with SearchStrategy.
InsertStrategy: Also gets a lifetime because why not (and also to be compatible with SearchStrategy).
SearchPostProcess: Drops the QueryComputer argument and now only requires HasId on the accessor argument.
The default implementation of workingset::Fill for workingset::Map has been removed. This means that users now need to implement Fill manually if they were relying on the provided implementation. A new Map::fill synchronous method has been added to make this process as easy as possible.

One consequence of fusing everything into SearchAccessor is that structs used for SearchAccessors can no longer be easily shared with those used as PruneAccessors. This causes a split between accessors used for search and those used for pruning. However, with this PR, the traits needed for insert PruneAccessors and SearchAccessors are disjoint. In practice, I did not find it very hard to make this split.

Why

The whole Accessor -> BuildQueryComputer -> ExpandBeam originated from earlier patterns in the library of building up indexing algorithms from tiny pieces, enabling a user to implement a small set of methods and get the whole indexing algorithm for free.

This breaks down, though, when we consider performance: the more information we provide a backend database about the operation we want to perform, the better the database can optimize. This lead to on_elements_unordered (bulk element retrieval, allowing for example prefetching), distances_unordered (bulk retrieval with a concrete distance computations), and finally ExpandBeam. Of these, the latter has proved by far the most useful, being used by diskann-disk and diskann-garnet to specialize the algorithm to their preferred data access pattern.

I've been exploring how to make filtered search more of a first-class citizen to allow us to batch together predicate queries and potentially fuse predicate evaluation with data access. I was halfway through writing a parallel Accessor/Computer/ExpandBeam/Strategy hierarchy for predicate aware search (and struggling with expressing the more complicated type relationships required), when I realized that all we really needed were a few slightly different flavors of ExpandBeam. Instead of building a complex nest of interrelated traits to express these subtly different ExpandBeam methods, why not just require users to implement the necessary ExpandBeam? The required semantics are not that complicated and it gives the implementor full control of the data access.

Reviewing Order

I've broken this PR into commits that focus on one change at a time. Each commit should compile and pass tests on its own.

Lifting the lifetimes into {Search/Insert}Strategy.
Small cleanup off some test code that was relying on Accessor.
Remove the provided Fill<Map<_>> implementation.
Fuse distance computations into the Accessor trait. This is the biggest commit and results in the splitting of search and prune accessors. Note that the state of the code after this commit is a bit goofy, but it's setup for the next commit.
Add the SearchAccessor trait and collapse the inter-related search traits.
Documentation cleanup.
More documentation cleanup.

Upgrade Guide (AI helped)

Removed traits

Accessor — element-wise access is no longer required.
BuildQueryComputer — fused into SearchAccessor.
SearchExt — superseded.
ExpandBeam — superseded.
Search accessors no longer need to implement neighbor delegation.

What to implement instead

A single SearchAccessor with three required methods:

async fn starting_points(&self) -> ANNResult<Vec<Self::Id>>;
async fn start_point_distances<F>(&mut self, f: F) -> ANNResult<()>
    where F: FnMut(Self::Id, f32) + Send;
async fn expand_beam<Itr, P, F>(&mut self, ids: Itr, pred: P, on_neighbors: F) -> ANNResult<()>
    where Itr: Iterator<Item = Self::Id> + Send,
          P: HybridPredicate<Self::Id> + Send + Sync,
          F: FnMut(Self::Id, f32) + Send;

expand_beam is now responsible for distance computation; the query is provided once at accessor construction.

Splitting search and prune accessors

Search and prune now have orthogonal responsibilities. If your old accessor served both, split it:

Search accessor: implements SearchAccessor.
Prune accessor: implements HasElementRef + BuildDistanceComputer + AsNeighborMut + workingset::Fill.

PruneStrategy::prune_accessor produces the latter.

SearchStrategy lifetime

SearchStrategy<'a, Provider, T> now has a lifetime so accessors can borrow the query:

fn search_accessor(
    &'a self,
    provider: &'a Provider,
    context: &'a Provider::Context,
    query: T,
) -> Result<Self::SearchAccessor, Self::SearchAccessorError>;

InsertStrategy also has a lifetime and is now passed by reference to DiskANNIndex::insert.

`Fill<Map<...>>` blanket impl removed

If you previously relied on the blanket Fill<Map<_>> for accessors, you must implement Fill directly. The new Map::fill helper makes this a few lines:

fn fill<'a, Itr>(&'a mut self, set: &'a mut WorkingSet, itr: Itr)
    -> impl SendFuture<Result<Self::View<'a>, Self::Error>>
    where Itr: ExactSizeIterator<Item = Self::Id> + Clone + Send + Sync, Self: 'a
{
    let view = set.fill(itr, |id| self.fetch(id).allow_transient("..."));
    std::future::ready(view)
}

Migration checklist

Delete old Accessor/BuildQueryComputer/SearchExt/ExpandBeam impls.
Implement SearchAccessor (search path) and a separate prune accessor (build path).
Implement Fill directly on the prune accessor using Map::fill.
Update SearchStrategy/InsertStrategy impls for the new 'a lifetime.
Pass &strategy to DiskANNIndex::insert.

metajack · 2026-05-14T15:42:31Z

get_element is basically unused in garnet, except for implementing VEMB (return an vector) and other debugging-style functions.

This sounds like a very promising direction, and I am very supportive.

Remove `flat_search` from `DiskANNIndex` and the `IdIterator` trait from `diskann`. Since the only caller was from `diskann-disk`, add a new `flat_search` inherent method to `DiskIndexSearcher`. The flat search method is not compatible with the experimental direction in #1067 and with #983 on the horizon, this is safe to move for now.

Paged search has been causing all kinds of issues for our code base and is actively getting in the way of simplifications in #1067 due to interactions with the `PagedSearchState`. The TLDR of the issue is that `PagedSearchState` requires types to be `'static` and introduces the need to "pause" and "resume" search state in a way that is complex to describe in trait bounds. Since our code is already async, we can lean into that and use the usual Rust machinery to embed non-`'static` paged searcher inside an otherwise `'static` future. The recommended way to now interact with paged search is via [channels](https://docs.rs/tokio/latest/tokio/sync/mpsc/fn.channel.html). [Rendered RFC](https://github.com/microsoft/DiskANN/blob/b63d009893f362e07ace518314f7c85733cba212/rfcs/01078-paged-search.md) ## API Migration Guide | Old pattern | New pattern | |---|---| | `index.start_paged_search(s, ctx, q, l).await` | `index.paged_search(s, ctx, q, l).await` | | `index.next_search_results(ctx, &mut state, k, &mut buf).await` | `search.next_page(k).await` | | `SearchState<Id, (S, C)>` | `PagedSearch<'a, DP, S, T>` | | `PagedSearchState<DP, S, C>` | `PagedSearch<'a, DP, S, T>` | | Check return count for exhaustion | Check `page.is_empty()` | If existing code embedded the `SearchState` in some `'static` container, that is no longer viable because of the borrow. Instead, channels can be used for this communication: ```rust // Types are illustrative — adapt names to your crate. type PageResult = ANNResult<Vec<Neighbor<ExternalId>>>; /// Spawn a paged search session. The index is held by Arc so the task is 'static. /// /// Returns a request channel and a result channel. The caller sends the desired /// page size (`k`) and awaits the corresponding result on the other end. fn spawn_paged_session( index: Arc<DiskANNIndex<DP>>, context: Arc<DP::Context>, query: T, l: usize, ) -> (mpsc::Sender<usize>, mpsc::Receiver<PageResult>) { let (req_tx, mut req_rx) = mpsc::channel::<usize>(1); let (res_tx, res_rx) = mpsc::channel::<PageResult>(1); tokio::spawn(async move { // Borrow from the Arc — these references are scoped to the task. let mut search = index.paged_search(strategy, &*context, query, l).await.unwrap(); while let Some(k) = req_rx.recv().await { let page = search.next_page(k).await; if res_tx.send(page).await.is_err() { break; // caller dropped the result receiver } } // Request channel closed -> caller dropped sender -> clean shutdown. }); (req_tx, res_rx) } ``` If code was already explicitly using a `.await` loop with `SearchState`, then minimal changes should be needed. ## For Users of Paged Search via `wrapped_async` Users of paged search via `wrapped_async::DiskANNIndex` that know their inner futures will never suspend can use the new `wrapped_async::DiskANNIndex::paged_search_no_await`. This will use the new API transparently via `wrapped_async::noawait::PagedSearch` and efficiently run paged searches with minimal synchronization overhead. This should **only** be used if the implementation of `Accessor`, `BuildQueryComputer`, `SearchExt`, `DataProvider`, and `ExpandBeam` are known to never yield and always complete with `Poll::Ready`. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

In preparation for #1067, switch the implementation of `DynIndex::search_element` from using the `Accessor` trait to an inherent method on the underlying provider.

Refactor `DiskIndexSearcher::flat_search` to use the bulk `pq_distances` method instead of one-by-one computation. This does two things: 1. The bulk method is presumably a little more efficient. 2. This moves the implementation away from `Accessor`/`BuildQueryComputer` to help with #1067. The small tricky bit is deriving the maximum batch size from the size of scratch. To that end, I added a private accessor for `PQScratch` to return this bound. --------- Co-authored-by: Mark Hildebrand <mhildebrand@microsoft.com>

This PR introduces a trait interface and a light index to support brute-force search for providers that can be used as/are a flat-index. There is an associated RFC that walks through the interface and associated implementation in `diskann` as a new `flat` module. Rendered RFC [link](https://github.com/microsoft/DiskANN/blob/u/adkrishnan/flat-index/rfcs/00983-flat-search.md). ## Motivation The repo has no first-class surface for brute-force search. This PR adds a small trait hierarchy that gives flat search the same provider-agnostic shape that graph search has, so any backend (in-memory, quantized, disk, remote) can plug in once and reuse a shared algorithm. ## Traits (`flat/strategy.rs`) **`DistancesUnordered<C>`** — the single trait a backend must implement. Fuses iteration and scoring into one method: the implementation drives a full scan, scoring each element with a precomputed query computer `C`, and invokes a callback with `(id, distance)` pairs. Key associated types: - `ElementRef<'a>` -- the reference shape `C` scores against. - `Id` -- the id type yielded to the callback (decoupled from `HasId` so visitors can yield any id shape). - `C : for<'a> PreprocessedDistanceFunction<Self::ElementRef<'a>, f32>` -- the precomputer query computer. ```rust pub trait DistancesUnordered<C>: Send + Sync where C: for<'a> PreprocessedDistanceFunction<Self::ElementRef<'a>, f32>, { type ElementRef<'a>; type Id; type Error: ToRanked + Debug + Send + Sync + 'static; fn distances_unordered<F>( &mut self, computer: &C, f: F, ) -> impl SendFuture<Result<(), Self::Error>> where F: Send + FnMut(Self::Id, f32); } ``` **`SearchStrategy<P, T>`** — factory that creates a `DistancesUnordered` visitor from a provider + context, and builds the per-query computer. Mirrors the graph-side strategy pattern. Two fallible methods: - `create_visitor` — borrows provider + context, returns a `Visitor` - `build_query_computer` — preprocesses the query `T` into a `QueryComputer` ```rust pub trait SearchStrategy<P, T>: Send + Sync where P: DataProvider, { type ElementRef<'a>; type Id; type QueryComputer: for<'a> PreprocessedDistanceFunction<Self::ElementRef<'a>, f32>; type QueryComputerError: StandardError; type Visitor<'a>: for<'b> DistancesUnordered< Self::QueryComputer, ElementRef<'b> = Self::ElementRef<'b>, Id = Self::Id, >; type Error: StandardError; fn create_visitor<'a>(&'a self, provider: &'a P, context: &'a P::Context) -> Result<Self::Visitor<'a>, Self::Error>; fn build_query_computer(&self, query: T) -> Result<Self::QueryComputer, Self::QueryComputerError>; } ``` ## Index (`flat/index.rs`) **`FlatIndex<P>`** — thin `'static` wrapper around a `DataProvider`. Currently we have implemented the naive kNN search algorithm for the flat index. `knn_search` asks the strategy for a visitor, builds the query computer, drives `distances_unordered` through a priority queue, and writes results via `SearchPostProcess`. ## Test infrastructure (`flat/test/`) A self-contained test provider with dimension-validated `Strategy`, transient-error injection, and a `KnnOracleRun` harness that compares `knn_search` results against a brute-force reference with baseline caching for regression detection. ## Future work - **Post-processing** — `knn_search` currently uses the graph-side `SearchPostProcess` trait to write results into the output buffer. #1067 will introduce a flat-specific post-processing step that decouples flat search from the graph module's output machinery. - **Vector ids** - The vector id over which `DistancesUnordered` acts is an associated type, instead of the `InternalId` of the provider. This is due to overly restrictive trait bounds on `VectorId` trait. We plan on relaxing this allowing for more generic id types. --------- Co-authored-by: Alex Razumov (from Dev Box) <alrazu@microsoft.com>

@harsha-simhadri

# DiskANN v0.53.0 Release Notes ## Breaking Changes An AI generated, human reviewed list of changes is summarized below. ### Paged search overhauled — channel-based API ([#1078](#1078)) `PagedSearchState` and its `'static`-bound pause/resume model have been replaced with an async, channel-based interface. The recommended way to drive paged search is now via a `tokio::sync::mpsc` channel, with the searcher embedded in an otherwise-`'static` future. See the [rendered RFC](https://github.com/microsoft/DiskANN/blob/main/rfcs/01078-paged-search.md) for the new shape. Callers wired against `PagedSearchState` must migrate to the channel API. Users of paged search via `wrapped_async::DiskANNIndex` that know their inner futures will never suspend can use the new `wrapped_async::DiskANNIndex::paged_search_no_await`; this will efficiently run paged searches with minimal synchronization overhead. ### `DiskANNIndex::flat_search` removed ([#1076](#1076)) `DiskANNIndex::flat_search` and the `IdIterator` trait have been removed from the `diskann` crate. Equivalent functionality lives on the new inherent method `DiskIndexSearcher::flat_search` in `diskann-disk`. This unblocks the experimental directions in #1067 and #983. ```rust // Before diskann_index.flat_search(query, ...)?; // After disk_index_searcher.flat_search(query, ...).await?; ``` ### `DiskIndexSearcher::flat_search` now batched ([#1097](#1097)) The new `DiskIndexSearcher::flat_search` uses the bulk `pq_distances` path instead of one-vector-at-a-time `Accessor::build_query_computer` + `evaluate_similarity`. Downstream behavior is equivalent but tighter resource bounds apply. ### `centroid` removed from PQ interfaces ([#1010](#1010)) The dataset-centroid argument has been removed from `FixedChunkPQTable` construction, `populate`, and most other PQ APIs. The shift only ever worked for L2 distance and was silently ignored for inner-product / cosine, so passing it was a footgun. When an L2 shift is required, fold it into the PQ pivots instead (the library now does this internally). ```rust // Before let table = FixedChunkPQTable::new(.., centroid, ..); // After — drop the centroid argument let table = FixedChunkPQTable::new(.., ..); ``` ### Flat search interface ([#983](#983)) A new `flat` module in `diskann` adds a provider-agnostic brute-force search surface, mirroring the shape of graph search. Backends implement a single trait, `DistancesUnordered<C>` (in `flat/strategy.rs`), which fuses iteration and distance computation, allowing any backend (in-memory, quantized, disk, remote) to plug into a shared algorithm. See the [rendered RFC](https://github.com/microsoft/DiskANN/blob/main/rfcs/00983-flat-search.md). This is additive but is the new canonical surface — direct ad-hoc flat-search call sites should migrate. ### `bf_tree` extracted into `diskann-bftree` crate ([#1020](#1020)) The bf_tree provider has been moved out of `diskann-providers` (previously at `diskann-providers/src/model/graph/provider/async_/bf_tree/`) into a new standalone `diskann-bftree` crate. Along with the move: - Switched from PQ to spherical quantization. - Dropped dependencies on `DeletionCheck`, `AsDeletionCheck`, and `RemoveDeletedIdsAndCopy`. - Simplified generics. Consumers must update their `Cargo.toml` to depend on `diskann-bftree` and update import paths. ### `direct_distance_impl` and `inner_product_raw` re-exposed ([#1081](#1081)) `direct_distance_impl` (free function) and `FixedChunkPQTable::inner_product_raw` are `pub` again after being privatized in #1044. Restored to unblock a downstream user. Not breaking in the typical direction — this restores previously available API surface. ### MinMax `recompress` takes a grid-scale parameter ([#1109](#1109)) The MinMax `recompress` API now accepts a grid-scale parameter. ## New Features - SIMD-optimized L2-squared norm ([#1107](#1107)) - Significantly faster bitmap computation ([#1099](#1099)) - Large speedup on the bitmap construction path used by filtered search. - LLVM IR bloat regression check in CI ([#1083](#1083)) - CI now flags regressions in generated LLVM IR size, helping catch unintended monomorphization blow-ups. - Recall computation fix for under-k groundtruth ([#1069](#1069)) ## Merged PRs * Revise README for DiskANN3 by @harsha-simhadri in #1046 * [CI] Try to fix publishing step by @hildebrandmw in #1057 * [benchmark] Remove `DispatchRule` by @hildebrandmw in #1064 * [benchmark] Automatic Input Registration by @hildebrandmw in #1066 * Remove centroid from most PQ interfaces by @hildebrandmw in #1010 * [diskann/disk] Remove `flat_search` from `DiskANNIndex` by @hildebrandmw in #1076 * macos build and miri check to nightly by @harsha-simhadri in #1058 * [API] Make some methods public again by @hildebrandmw in #1081 * [benchmark] Simply `Inputs` more by @hildebrandmw in #1077 * Turn on stack protection for the diskann-garnet NuGet build by @jackmoffitt in #1082 * Fix options for diskann-garnet nuget pipeline by @jackmoffitt in #1091 * [CI] add LLVM IR bloat regression check by @arazumov in #1083 * Bump openssl from 0.10.79 to 0.10.80 by @dependabot[bot] in #1093 * [Disk CI benchmarks] Use 1ES.Pool=diskann-github by @arazumov in #869 * Fix recall computation for fewer than k groundtruth results by @magdalendobson in #1069 * bf_tree migration away from diskann-providers by @JordanMaples in #1020 * [RFC/diskann] Overhaul paged search by @hildebrandmw in #1078 * Remove unsafe code from compute_vec_l2sq by @arazumov in #1094 * Remove direct accessor call in `diskann-garnet` by @hildebrandmw in #1098 * Refactor `DiskIndexSearcher::flat_search` to use batching by @hildebrandmw in #1097 * [flat index] Flat Search Interface by @arkrishn94 in #983 * migrating multi-hop tests from diskann-providers to diskann by @JordanMaples in #928 * Significantly speed up bitmap computation by @magdalendobson in #1099 * `compute_vecs_l2sq`: Replace scalar L2 Squared norm with SIMD-optimized FastL2NormSquared by @arazumov in #1107 * [minmax] Add grid scaling to recompress API by @arkrishn94 in #1109 **Full Changelog**: v0.52.0...v0.53.0

Life the lifetime into SearchStrategy and InsertStrategy, removing it from the `SearchAccessor` GAT. This also requires changing the signature of `DiskANNIndex::insert` to accept the `InsertStrategy` by reference to avoid lifetime issues.

The reload tests in `index_storage.rs` currently rely on Accessor to function. With the pending removal of this trait, this PR instead implements comparison using inherent methods of the underlying structs instead.

Remove the blanket implementation of `Fill<workingset::Map<_>>` for accessors, instead requiring users to implement `Fill` themselves. To make the process easier, a new `Map::fill` method is added with a much simpler implementation than the old async method and documentation included to point users toward this method. Includes three more related changes: * Fill implementation for `diskann-bftree` that reuse allocations upon access to deleted IDs. This is an improvement over the provided implementation which would first copy, and then allocate (and copy again). Instead, we avoid that first copy. * Simplify the `Fill` implementation for `diskann-garnet` now that we no longer have a blanket implementation to dodge. * Remove the `Flaky` tests from `diskann-providers`. Since we're moving to a model with coarser grained traits, the need to test the fine-grained error handling of the various `diskann` provided implementations is significantly reduced.

This is an awkward stepping-stone in preparation for simplifying our traits. It removes the `BuildQueryComputer` trait, instead requiring Accessors to handle distance computations themselves. The contract for Accessor is updated to: * `get_distance(&mut self, id) -> Result<f32, Error>` * `distances_unordered(...). In response to this, accessors used for pruning need to be distinct from the accessors used for search, which is where most of the code churn comes from. There are a few upsides here: * The test provider becomes a little more efficient as it no longer needs to copy out data into a temporary buffer. * Inline beta-filtering can now be done without cloning out the RoaringTreeMap, removing a performance footgun. Overall, though, this makes the code a bit worse, but is in preparation for future cleanup.

This drops `Accessor`, `SearchExt`, and `ExpandBeam` in favor of a new `SearchAccessor` trait. This trait encompases what `SearchExt` and `ExpandBeam` used to do with the main function being `expand_beam` pretty much unaltered as before. High level changes per crate: * `diskann-providers`: Consolidate to the new `SearchAccessor` which is almost entirely just shuffling around existing implementations. The tests for `betafilter` have been rewritten to test the observable behavior of `expand_beam` directly. * `diskann-disk`: Consolidate into `SearchAccessor` and strip out now unused functions and structs. * `diskann-bftree`: More consolidation. * `diskann-label-filter`: Simplify the implementation now that we only need to focus on `expand_beam`. * `diskann-garnet`: Now that we no longer return element explicitly, we can remove the `DynVector` type.

codecov-commenter · 2026-06-02T19:36:12Z

Codecov Report

❌ Patch coverage is 87.80952% with 192 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.91%. Comparing base (77f9e9d) to head (47924ec).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
diskann-garnet/src/provider.rs	78.81%	61 Missing ⚠️
...rc/inline_beta_search/encoded_document_accessor.rs	0.00%	58 Missing ⚠️
...ilter/src/inline_beta_search/inline_beta_filter.rs	0.00%	25 Missing ⚠️
diskann/src/graph/test/provider.rs	87.76%	17 Missing ⚠️
diskann-bftree/src/provider.rs	96.38%	6 Missing ⚠️
diskann-providers/src/index/wrapped_async.rs	73.91%	6 Missing ⚠️
...ders/src/model/graph/provider/layers/betafilter.rs	94.18%	5 Missing ⚠️
...src/model/graph/provider/async_/inmem/spherical.rs	97.05%	3 Missing ⚠️
diskann/src/graph/glue.rs	85.71%	3 Missing ⚠️
diskann-providers/src/index/diskann_async.rs	96.29%	2 Missing ⚠️
... and 4 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1067      +/-   ##
==========================================
+ Coverage   88.87%   88.91%   +0.03%     
==========================================
  Files         485      484       -1     
  Lines       92112    91396     -716     
==========================================
- Hits        81866    81266     -600     
+ Misses      10246    10130     -116

Flag	Coverage Δ
miri	`88.91% <87.80%> (+0.03%)`	⬆️
unittests	`88.56% <87.80%> (+0.03%)`	⬆️

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

Files with missing lines	Coverage Δ
diskann-benchmark-core/src/build/graph/single.rs	`100.00% <100.00%> (ø)`
diskann-benchmark-core/src/search/graph/knn.rs	`95.08% <ø> (ø)`
...iskann-benchmark-core/src/search/graph/multihop.rs	`98.72% <ø> (ø)`
diskann-benchmark-core/src/search/graph/range.rs	`93.57% <ø> (ø)`
diskann-benchmark/src/backend/index/benchmarks.rs	`47.31% <100.00%> (ø)`
diskann-bftree/src/quant.rs	`91.41% <ø> (-0.11%)`	⬇️
diskann-disk/src/build/builder/inmem_builder.rs	`86.93% <100.00%> (ø)`
diskann-disk/src/search/provider/disk_provider.rs	`93.67% <100.00%> (+2.81%)`	⬆️
diskann-garnet/src/dyn_index.rs	`65.71% <100.00%> (ø)`
diskann-label-filter/src/document.rs	`0.00% <ø> (ø)`
... and 30 more

... and 4 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

This PR simplifies DiskANN graph-search integration by collapsing multiple search-related traits (Accessor, BuildQueryComputer, ExpandBeam, SearchExt, etc.) into a single SearchAccessor contract, while lifting lifetimes into strategies so accessors can borrow query inputs. It also updates insert/search call sites and working set filling to match the new separation between search accessors and prune/insert accessors.

Changes:

Introduces SearchAccessor as the unified search extension point and updates SearchStrategy/DefaultPostProcessor (and related search paths) to construct accessors with the query.
Refactors insert/search/paged-search internals to remove explicit QueryComputer plumbing, relying on SearchAccessor::{start_point_distances, expand_beam} to provide distances.
Removes the blanket Fill<Map<...>> impl and adds a synchronous Map::fill(...) helper to make manual Fill implementations straightforward.

Reviewed changes

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

Show a summary per file

File	Description
rfcs/01067-search-accessor.md	RFC documenting the new `SearchAccessor` contract and strategy lifetime changes.
diskann/src/graph/workingset/mod.rs	Updates `Fill` bounds from `Accessor` to `HasElementRef + HasId` and adjusts docs/imports accordingly.
diskann/src/graph/workingset/map.rs	Removes blanket `Fill<Map<...>>` impl; adds synchronous `Map::fill` helper and updates tests/docs.
diskann/src/graph/test/provider.rs	Splits test accessors into search vs prune accessors; implements new `SearchAccessor` shape.
diskann/src/graph/test/cases/paged_search.rs	Updates paged-search tests to pass strategies by reference (new lifetimes/traits).
diskann/src/graph/test/cases/multihop.rs	Removes `BuildQueryComputer` usage; constructs accessor with query.
diskann/src/graph/test/cases/inplace_delete.rs	Updates insert calls to pass `&strategy` per new `InsertStrategy` lifetime contract.
diskann/src/graph/test/cases/grid_insert.rs	Updates insert calls to pass `&strategy`.
diskann/src/graph/search/range_search.rs	Refactors range search to use `SearchAccessor` and query-at-construction strategy API.
diskann/src/graph/search/paged.rs	Refactors paged search state to hold a `SearchAccessor` rather than strategy/computer/query scaffolding.
diskann/src/graph/search/multihop_search.rs	Refactors multihop traversal to use `SearchAccessor` distance production APIs.
diskann/src/graph/search/mod.rs	Adds lifetime parameter to `Search` trait to align with lifetime-lifted `SearchStrategy`.
diskann/src/graph/search/knn_search.rs	Refactors KNN search to use `SearchAccessor` and updated postprocess signature.
diskann/src/graph/search/diverse_search.rs	Refactors diverse search to remove `BuildQueryComputer` plumbing and use new trait shapes.
diskann/src/graph/index.rs	Core refactor: insert/search/paged-search/search_internal now operate on `SearchAccessor` without separate query computer; insert now takes `&strategy`.
diskann-providers/src/storage/index_storage.rs	Updates provider storage tests and insert calls to match `InsertStrategy` lifetime/reference changes.
diskann-providers/src/model/graph/provider/layers/betafilter.rs	Updates beta-filter composition to wrap `SearchAccessor` behavior directly (no query-computer wrapper).
diskann-providers/src/model/graph/provider/async_/postprocess.rs	Updates postprocess bridge trait bounds to depend on `HasId` rather than `BuildQueryComputer`.
diskann-providers/src/model/graph/provider/async_/inmem/test.rs	Removes the old “flaky provider handling” test module (deleted file).
diskann-providers/src/model/graph/provider/async_/inmem/spherical.rs	Refactors spherical in-mem accessors: query computer moved into accessor construction; introduces dedicated prune accessor.
diskann-providers/src/model/graph/provider/async_/inmem/scalar.rs	Refactors scalar-quantized in-mem accessors similarly; introduces dedicated prune accessor and query-at-construction search accessor.
diskann-providers/src/model/graph/provider/async_/inmem/provider.rs	Removes test-only helper tied to old start-point filtering approach.
diskann-providers/src/model/graph/provider/async_/inmem/mod.rs	Stops exporting the removed test module.
diskann-providers/src/model/graph/provider/async_/inmem/full_precision.rs	Refactors full-precision in-mem accessors to store query distance computer inside the search accessor; introduces prune accessor.
diskann-providers/src/model/graph/provider/async_/fast_memory_vector_provider.rs	Adds test-only `compare_data` helper for validating stored vectors across reloads.
diskann-providers/src/model/graph/provider/async_/fast_memory_quant_vector_provider.rs	Adds test-only `compare_data` helper for validating quantized vectors/pivots across reloads.
diskann-providers/src/index/wrapped_async.rs	Updates sync wrapper APIs to pass strategies by reference and propagate new lifetimes/types in paged search.
diskann-providers/src/index/diskann_async.rs	Updates async tests/helpers for new lifetime-lifted strategy traits and `&strategy` insert/search calls; removes flaky-provider test block.
diskann-providers/benches/benchmarks/diskann_bench.rs	Updates benchmark insert calls to pass `&FullPrecision`.
diskann-providers/benches/benchmarks_iai/diskann_iai.rs	Updates IAI benchmark insert calls to pass `&FullPrecision`.
diskann-label-filter/src/inline_beta_search/inline_beta_filter.rs	Refactors inline beta logic away from query-computer wrapping toward accessor-time filtering and candidate filtering.
diskann-label-filter/src/inline_beta_search/encoded_document_accessor.rs	Reworks encoded document accessor to implement `SearchAccessor` and apply beta filtering via attribute lookup.
diskann-label-filter/src/document.rs	Removes unused `EncodedDocument` helper type; keeps simple `Document` container.
diskann-garnet/src/dyn_index.rs	Updates insert call to pass `&DynamicQuantization`.
diskann-disk/src/search/provider/disk_provider.rs	Refactors disk search provider to implement `SearchAccessor` directly; removes separate disk query computer type.
diskann-disk/src/build/builder/inmem_builder.rs	Updates builder inserts to pass `&FullPrecision` / `&Quantized` per new insert signature.
diskann-bftree/src/quant.rs	Removes unused `into_inner` helper from `QuantQueryComputer`.
diskann-benchmark/src/backend/index/benchmarks.rs	Updates benchmark trait bounds for lifetime-lifted `SearchStrategy`/`DefaultSearchStrategy`.
diskann-benchmark-core/src/search/graph/range.rs	Updates benchmark-core range search bounds for lifetime-lifted default strategies.
diskann-benchmark-core/src/search/graph/multihop.rs	Updates benchmark-core multihop search bounds for lifetime-lifted default strategies.
diskann-benchmark-core/src/search/graph/knn.rs	Updates benchmark-core KNN search bounds for lifetime-lifted default strategies.
diskann-benchmark-core/src/build/graph/single.rs	Updates benchmark-core single-insert build to pass `&strategy` and updated `InsertStrategy` bounds.

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

+                // TODO: If predicate evaluation fails, we are taking the approach that we
+                // will simply return the score returned by the inner computer, as though no
+                // predicate was specified.
+                tracing::warn!("Predicate evaluation failed in OnlineBetaComputer application");


hildebrandmw mentioned this pull request May 15, 2026

[flat index] Flat Search Interface #983

Merged

hildebrandmw changed the title ~~[DO NOT MERGE] Delete all the things!~~ Simplify the DataProvider contract for graph search May 15, 2026

This was referenced May 15, 2026

[diskann/disk] Remove flat_search from DiskANNIndex #1076

Merged

[RFC/diskann] Overhaul paged search #1078

Merged

hildebrandmw mentioned this pull request May 21, 2026

Refactor DiskIndexSearcher::flat_search to use batching #1097

Merged

hildebrandmw mentioned this pull request May 21, 2026

Remove direct accessor call in diskann-garnet #1098

Merged

hildebrandmw force-pushed the mhildebr/experiment branch 2 times, most recently from d4f04f4 to d832ede Compare May 27, 2026 22:30

arkrishn94 mentioned this pull request May 28, 2026

Bump version to 0.53.0 #1111

Merged

Mark Hildebrand added 6 commits June 2, 2026 10:17

Lift lifetimes into strategies.

4b3a04e

Life the lifetime into SearchStrategy and InsertStrategy, removing it from the `SearchAccessor` GAT. This also requires changing the signature of `DiskANNIndex::insert` to accept the `InsertStrategy` by reference to avoid lifetime issues.

Remove use of the Accessor trait in tests.

be0714e

The reload tests in `index_storage.rs` currently rely on Accessor to function. With the pending removal of this trait, this PR instead implements comparison using inherent methods of the underlying structs instead.

Documentation cleanup.

d431e8e

hildebrandmw force-pushed the mhildebr/experiment branch from d832ede to d431e8e Compare June 2, 2026 19:21

Update docs.

47924ec

hildebrandmw marked this pull request as ready for review June 2, 2026 22:31

hildebrandmw requested review from a team and Copilot June 2, 2026 22:31

Copilot started reviewing on behalf of hildebrandmw June 2, 2026 22:31 View session

Copilot AI reviewed Jun 2, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify the `DataProvider` contract for graph search#1067

Simplify the `DataProvider` contract for graph search#1067
hildebrandmw wants to merge 7 commits into
mainfrom
mhildebr/experiment

hildebrandmw commented May 14, 2026 •

edited

Loading

Uh oh!

metajack commented May 14, 2026

Uh oh!

codecov-commenter commented Jun 2, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

hildebrandmw commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Reviewing Order

Upgrade Guide (AI helped)

Removed traits

What to implement instead

Splitting search and prune accessors

SearchStrategy lifetime

Fill<Map<...>> blanket impl removed

Migration checklist

Uh oh!

metajack commented May 14, 2026

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hildebrandmw commented May 14, 2026 •

edited

Loading

`Fill<Map<...>>` blanket impl removed

codecov-commenter commented Jun 2, 2026 •

edited

Loading