Slim down `ANNError` by hildebrandmw · Pull Request #1282 · microsoft/DiskANN

hildebrandmw · 2026-07-28T14:59:52Z

Remove ANNErrorKind from ANNError, removing all the old log_ constructors and slimming the constructors to new, message, and `context.

Since diskann-disk leans pretty heavily to using the kinds in its tests, a new diskann_disk::error::Error type is introduces along with a diskann_error! macro to largely preserve test structure and error kind tagging. See the review notes for more information.

Why

The error kind enum is largely unused at this point, is not applied consistently, and complicates the API for ANNError. It's time to remove it and bring the size of ANNError down to 8-bytes.

Review Order

diskann

error/ann_error.rs: Removal of the ANNErrorKind enum and the log_* style constructors. As a bonus, I added a convert_error! macro (happy to bike-shed) to stamp out simple conversion cases while preserving track_caller.

The rest of the changes in diskann are removing the ANNErrorKind and switching over to the convert_error! macro.

diskann-tools/diskann-providers/diskann-garnet/diskann-label-filter/diskann-bftree/diskann-benchmark-core/diskann-benchmark
These mainly consist of updating the constructors and conversions. Note that where possible (and easily achievable), I use the moving form of lazy_format! to avoid eager string formatting.

**diskann-disk
This is the perhaps a more controversial change. diskann-disk's tests lean heavily into the error kind and log* style constructors. I tried to preserve as much of the original behavior as possible by introducing an error type diskann_disk::error::Error consisting of a string-message an a diskann_disk::error::ErrorKind enum. A macro diskann_disk::error::diskann_error! helps embed this new Error inside of an ANNError. While this goes against the ethos of "don't have crate level errors", here is my rationale:

By using the log_* constructors, diskann_disk's error types are already fundamentally tagged string messages.
Introducing an Error type means we can still check the Kind enum by downcasting the resulting ANNError down to the original Error.
Since the existing diskann_disk code is already doing eager string formatting/allocating at its error construction callsites, this isn't introduction a regression. Indeed, I've added some tricks to avoid allocating when constructing diskann_disk::error::Error from string literals, which is fairly common.
Moving the kind tag to diskann_disk allows the disk index to add error types without disrupting ANNError.

Alternatives

This is an alternative to Remove the old log* style constructors #1278 that I was unfortunately working on in parallel.
One functionality we lost with this is the ability to recover the inner error types from the tag. Theoretically, the tag type could be used by third-party crates to guide the proper type for a downcast. In practice, no one is doing this.

Copilot

Pull request overview

This PR refactors DiskANN’s error handling to slim down ANNError (removing the embedded “kind” tagging and legacy log_* constructors), standardize conversions via a new convert_error! macro, and update downstream crates to use the new constructors/messages. It also introduces a diskann-disk-local typed error payload (diskann-disk::error::{Error, ErrorKind}) to retain category-style assertions in that crate’s tests without re-expanding ANNError.

Changes:

Replace ANNError(kind, payload) + ANNErrorKind with a smaller ANNError(payload) and ANNError::message(display); remove legacy log_* constructors and opaque().
Add convert_error! macro to reduce boilerplate From<T> for ANNError impls and update many crates accordingly.
Add diskann-disk crate-local ErrorKind + diskann_error! helper to preserve typed categorization in that crate.

Reviewed changes

Copilot reviewed 89 out of 89 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
diskann/src/utils/vector_repr.rs	Switch `NativeTypeLengthError -> ANNError` conversion to `convert_error!`.
diskann/src/utils/async_tools.rs	Use `convert_error!` for async utility error conversions.
diskann/src/test/cmp.rs	Update test helpers to use `ANNError::new(...)` instead of removed `opaque()`.
diskann/src/lib.rs	Remove top-level export of `ANNErrorKind`; keep only `ANNError`/`ANNResult`.
diskann/src/graph/test/provider.rs	Migrate test provider errors to `convert_error!` / `ANNError::new`.
diskann/src/graph/search/range_search.rs	Replace explicit `From` impl with `convert_error!`.
diskann/src/graph/search/paged.rs	Replace legacy paged-search error constructors with `ANNError::message`.
diskann/src/graph/search/knn_search.rs	Replace explicit `From` impl with `convert_error!`.
diskann/src/graph/search/inline_filter_search.rs	Replace explicit `From` impl with `convert_error!`.
diskann/src/graph/internal/prune.rs	Update transient escalation path to use new `ANNError::new(self)` signature.
diskann/src/graph/index.rs	Update async/join/mutex error mapping to `ANNError::new` / `ANNError::message`.
diskann/src/graph/config/mod.rs	Convert config error promotion to `convert_error!`.
diskann/src/flat/test/provider.rs	Convert test provider errors to `convert_error!`.
diskann/src/flat/strategy.rs	Convert local test error promotion to `convert_error!`.
diskann/src/error/ranked.rs	Update tests and conversions to new `ANNError` APIs; adopt `convert_error!`.
diskann/src/error/mod.rs	Remove re-exports of kind/tag types; adjust `message!` macro to new `message()` API.
diskann/src/error/ann_error.rs	Core change: shrink `ANNError`, remove `ANNErrorKind`/legacy constructors, add `convert_error!`.
diskann-tools/src/utils/search_index_utils.rs	Replace legacy index-error constructors with `ANNError::message`.
diskann-tools/src/utils/cmd_tool_error.rs	Update test to construct `ANNError` via new signature.
diskann-providers/src/utils/vector_data_iterator.rs	Convert `SkipElementsError` promotion to `diskann::convert_error!`; adjust tests accordingly.
diskann-providers/src/utils/sampling.rs	Replace legacy constructors with `ANNError::message`; adopt `lazy_format!` in hot error paths.
diskann-providers/src/utils/rayon_util.rs	Map Rayon thread-pool build errors via `ANNError::new`.
diskann-providers/src/utils/medoid.rs	Replace legacy constructors with `ANNError::message`.
diskann-providers/src/utils/file_util.rs	Replace legacy constructors with `ANNError::message`; use `lazy_format!` for deferred formatting.
diskann-providers/src/storage/pq_storage.rs	Replace PQ/index-config legacy constructors with `ANNError::message`.
diskann-providers/src/storage/index_storage.rs	Replace legacy constructors with `ANNError::message` + `lazy_format!`.
diskann-providers/src/storage/bin.rs	Replace legacy constructors with `ANNError::message`.
diskann-providers/src/model/pq/views.rs	Replace legacy PQ error conversions with `convert_error!` / `ANNError::new`.
diskann-providers/src/model/pq/strided.rs	Replace legacy PQ error conversions with `ANNError::new`.
diskann-providers/src/model/pq/pq_construction.rs	Replace legacy PQ constructors and mapping with `ANNError::message` / `ANNError::new`.
diskann-providers/src/model/pq/generate_pivot_arguments.rs	Replace legacy PQ error conversion with `diskann::convert_error!`; update tests.
diskann-providers/src/model/pq/fixed_chunk_pq_table.rs	Replace legacy PQ constructors; use `lazy_format!`; convert bridge error via `diskann::convert_error!`.
diskann-providers/src/model/pq/distance/dynamic.rs	Replace dimension mismatch legacy constructor with `ANNError::message(lazy_format!(...))`; adjust tests.
diskann-providers/src/model/graph/provider/determinant_diversity.rs	Replace custom kind-mapping conversion with `diskann::convert_error!`.
diskann-providers/src/model/graph/provider/async_/simple_neighbor_provider.rs	Replace legacy constructors with `ANNError::message(lazy_format!(...))`.
diskann-providers/src/model/graph/provider/async_/memory_vector_provider.rs	Replace legacy constructors with `ANNError::message`.
diskann-providers/src/model/graph/provider/async_/memory_quant_vector_provider.rs	Replace legacy constructors with `ANNError::message`.
diskann-providers/src/model/graph/provider/async_/inmem/spherical.rs	Replace SQ error-kind conversions with `convert_error!` and new error APIs.
diskann-providers/src/model/graph/provider/async_/inmem/scalar.rs	Replace SQ error conversion with `diskann::convert_error!`.
diskann-providers/src/model/graph/provider/async_/inmem/provider.rs	Replace legacy constructors with `ANNError::message(lazy_format!(...))`.
diskann-providers/src/model/graph/provider/async_/fast_memory_vector_provider.rs	Replace legacy constructors with `ANNError::message`.
diskann-providers/src/model/graph/provider/async_/fast_memory_quant_vector_provider.rs	Replace kind assertions in tests with message-based assertions.
diskann-providers/src/model/graph/provider/async_/experimental/multi_pq_async.rs	Replace legacy constructors with `ANNError::message` / `lazy_format!`.
diskann-providers/src/model/graph/provider/async_/common.rs	Replace legacy constructors with `ANNError::message`.
diskann-providers/src/index/wrapped_async.rs	Replace internal invariant conversion with `diskann::convert_error!`; update `message()` call shape.
diskann-providers/src/common/minmax_repr.rs	Switch conversion to `ANNError::message(lazy_format!(...))`.
diskann-label-filter/src/inline_beta_search/encoded_document_accessor.rs	Update `message()` usage to new signature (no kind).
diskann-label-filter/src/encoded_attribute_provider/roaring_attribute_store.rs	Remove kind-based messages; replace with `ANNError::message(...)` on lock errors and invariants.
diskann-label-filter/src/encoded_attribute_provider/document_provider.rs	Replace `ANNErrorKind` uses and `ANNError::new(kind, e)` with `ANNError::new(e)` / `lazy_format!`.
diskann-label-filter/src/encoded_attribute_provider/ast_label_id_mapper.rs	Remove `ANNErrorKind` usage; update `new(...)` and `message(...)` calls; adjust tests.
diskann-garnet/src/provider.rs	Replace provider error promotion with `diskann::convert_error!`.
diskann-disk/src/utils/partition.rs	Switch to `diskann-disk` typed error payload (`diskann_error!(ErrorKind::...)`).
diskann-disk/src/utils/math_util.rs	Switch to `diskann-disk` typed error payload and map `sgemm` errors accordingly.
diskann-disk/src/utils/kmeans.rs	Switch to `diskann-disk` typed error payload; update tests to assert via `error_kind()`.
diskann-disk/src/storage/quant/pq/pq_generation.rs	Switch PQ generation errors to `diskann_error!(ErrorKind::PQError, ...)`.
diskann-disk/src/storage/quant/pq/pq_dataset.rs	Switch PQ dataset errors to `diskann_error!(...)`.
diskann-disk/src/storage/quant/generator.rs	Switch quant generator validation errors to `diskann_error!(...)`.
diskann-disk/src/storage/disk_index_writer.rs	Replace legacy constructors with `diskann_error!(...)` and simplify IO error propagation.
diskann-disk/src/storage/disk_index_reader.rs	Remove kind assertions; rely on message content.
diskann-disk/src/storage/cached_reader.rs	Replace legacy constructors with `diskann_error!(...)`.
diskann-disk/src/search/provider/disk_vertex_provider.rs	Replace legacy constructors with `diskann_error!(...)` for bounds/serde errors.
diskann-disk/src/search/provider/disk_vertex_provider_factory.rs	Replace legacy constructors with `diskann_error!(...)` when allocating/alignment fails.
diskann-disk/src/search/provider/disk_sector_graph.rs	Replace legacy constructors with `diskann_error!(...)` for allocation and bounds checks.
diskann-disk/src/search/provider/disk_provider.rs	Replace kind-based errors with `diskann_error!(...)`; map `Knn` construction error into disk error kind.
diskann-disk/src/search/provider/aligned_file_reader/reader/windows.rs	Simplify Windows IO error handling to use `?`/`From` conversions.
diskann-disk/src/search/provider/aligned_file_reader/reader/linux.rs	Replace legacy push/io errors with `diskann_error!(...)` / `Into<ANNError>`.
diskann-disk/src/search/provider/aligned_file_reader/aligned_read.rs	Switch alignment errors to `diskann_error!(ErrorKind::DiskIOAlignmentError, ...)`.
diskann-disk/src/search/pq/pq_scratch.rs	Switch PQ scratch allocation/dim errors to `diskann_error!(...)`; update tests to assert via `error_kind()`.
diskann-disk/src/lib.rs	Export new `diskann-disk::error` module publicly.
diskann-disk/src/error.rs	New: crate-local `ErrorKind` + payload `Error` + `diskann_error!` helper + test-only `error_kind()` extractor.
diskann-disk/src/data_model/graph_metadata.rs	Replace legacy parse-slice error construction with `diskann_error!(ErrorKind::SerdeError, ...)`.
diskann-disk/src/data_model/graph_layout_version.rs	Replace legacy parse-slice error construction with `diskann_error!(ErrorKind::SerdeError, ...)`.
diskann-disk/src/data_model/graph_header.rs	Replace legacy conversions with `diskann_error!(...)`; update tests to assert via `error_kind()`.
diskann-disk/src/data_model/cache.rs	Replace legacy constructors with `diskann_error!(...)` for capacity/dimension errors.
diskann-disk/src/build/configuration/disk_index_build_parameter.rs	Replace index-config legacy constructors with `diskann_error!(ErrorKind::IndexConfigError(...), ...)`; update tests.
diskann-disk/src/build/builder/tokio.rs	Replace legacy tokio-runtime init error construction with `diskann_error!(...)`.
diskann-disk/src/build/builder/quantizer.rs	Replace legacy index-config error construction with `diskann_error!(...)`.
diskann-disk/src/build/builder/build.rs	Replace legacy constructors with `diskann_error!(...)` for task/thread/mutex failures.
diskann-bftree/src/vectors.rs	Replace legacy constructors with `ANNError::message` / `lazy_format!`; update `ANNError::new` signature usage.
diskann-bftree/src/quant.rs	Replace legacy constructors and SQ error mapping with `ANNError::new` / `ANNError::message(lazy_format!(...))`; relax tests.
diskann-bftree/src/provider.rs	Replace legacy constructors with `ANNError::message` / `lazy_format!`.
diskann-bftree/src/neighbors.rs	Replace legacy constructors with `ANNError::message`.
diskann-bftree/src/lib.rs	Update error promotions and transient escalation to new `ANNError` APIs.
diskann-benchmark/src/inputs/save_and_load.rs	Replace legacy index-config error creation with `ANNError::message`.
diskann-benchmark/src/index/streaming/full_precision.rs	Update message construction to new `ANNError::message` signature.
diskann-benchmark/src/index/build.rs	Replace kind-tagging of start-point compute errors with `ANNError::new`.
diskann-benchmark-core/src/search/graph/strategy.rs	Replace `opaque()` conversion with `diskann::convert_error!`; adjust imports for tests.
diskann-benchmark-core/src/build/ids.rs	Replace `opaque()` and kind-based messages with `ANNError::new` and `lazy_format!`.
diskann-benchmark-core/src/build/graph/multi.rs	Replace kind-based `message(...)` call with new single-arg `message(...)`.

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

            if idx >= self.npts {
-                return Err(ANNError::log_index_error(format!(
+                let npts = self.npts - 1;
+                return Err(ANNError::message(lazy_format!(
+                    move,
                    "Vector index {} is out of bounds (max: {})",


 // Top level exports.
-pub use error::ann_error::{ANNError, ANNErrorKind, ANNResult};
+pub use error::ann_error::{ANNError, ANNResult};


codecov-commenter · 2026-07-28T15:15:02Z

Codecov Report

❌ Patch coverage is 73.03371% with 168 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.54%. Comparing base (e197b43) to head (c7d9686).

Files with missing lines	Patch %	Lines
...n-disk/src/search/provider/disk_vertex_provider.rs	0.00%	20 Missing ⚠️
diskann-bftree/src/provider.rs	40.00%	9 Missing ⚠️
diskann-bftree/src/quant.rs	50.00%	9 Missing ⚠️
diskann-bftree/src/vectors.rs	18.18%	9 Missing ⚠️
...rc/search/provider/disk_vertex_provider_factory.rs	40.00%	9 Missing ⚠️
diskann-disk/src/utils/kmeans.rs	75.00%	9 Missing ⚠️
diskann-disk/src/build/builder/build.rs	36.36%	7 Missing ⚠️
diskann-disk/src/storage/disk_index_writer.rs	63.15%	7 Missing ⚠️
.../src/model/graph/provider/async_/inmem/provider.rs	30.00%	7 Missing ⚠️
diskann-providers/src/storage/pq_storage.rs	36.36%	7 Missing ⚠️
... and 32 more

❌ Your patch status has failed because the patch coverage (73.03%) 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    #1282      +/-   ##
==========================================
- Coverage   91.50%   90.54%   -0.96%     
==========================================
  Files         497      498       +1     
  Lines       95524    95165     -359     
==========================================
- Hits        87411    86171    -1240     
- Misses       8113     8994     +881

Flag	Coverage Δ
miri	`90.54% <73.03%> (-0.96%)`	⬇️
unittests	`90.21% <73.03%> (-1.26%)`	⬇️

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/multi.rs	`98.90% <100.00%> (-0.03%)`	⬇️
diskann-benchmark-core/src/build/ids.rs	`96.77% <100.00%> (-0.20%)`	⬇️
...iskann-benchmark-core/src/search/graph/strategy.rs	`98.69% <ø> (-0.02%)`	⬇️
diskann-benchmark/src/index/build.rs	`85.49% <100.00%> (-0.22%)`	⬇️
...nn-benchmark/src/index/streaming/full_precision.rs	`93.61% <100.00%> (ø)`
diskann-benchmark/src/inputs/save_and_load.rs	`100.00% <100.00%> (+13.33%)`	⬆️
diskann-disk/src/build/builder/quantizer.rs	`92.85% <100.00%> (ø)`
diskann-disk/src/data_model/cache.rs	`100.00% <100.00%> (ø)`
diskann-disk/src/data_model/graph_header.rs	`100.00% <100.00%> (ø)`
...iskann-disk/src/data_model/graph_layout_version.rs	`100.00% <100.00%> (ø)`
... and 77 more

... and 38 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 89 out of 89 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

diskann/src/error/ann_error.rs:144

ANNError::new wraps errors as Located<E>, but is::<E>() only checks for E and will therefore return false for most errors created by this type. This makes the method misleading/inaccurate.

Mirror the downcast_ref behavior by also checking for Located<E>.

                if num_nodes_to_cache == 0 {
-                    ANNError::log_index_error(
+                    diskann_error!(
+                        ErrorKind::IndexError,
                        "num_nodes_to_cache should be greater than 0 for StaticCacheWithBfsNodes caching strategy",
                    );


Copilot

Pull request overview

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

Comments suppressed due to low confidence (3)

diskann-disk/src/search/provider/disk_vertex_provider_factory.rs:160

diskann_error!(...) is constructed but not returned when num_nodes_to_cache == 0, so the function continues with an invalid configuration. This also leaves a likely-unused expression (warning under -D warnings).

                if num_nodes_to_cache == 0 {
                    diskann_error!(
                        ErrorKind::IndexError,
                        "num_nodes_to_cache should be greater than 0 for StaticCacheWithBfsNodes caching strategy",
                    );

diskann-providers/src/utils/sampling.rs:185

When self.npts == 0, the out-of-bounds branch computes self.npts - 1, which underflows (panic in debug; wraps in release) and produces a misleading max value. Handle the empty-dataset case explicitly before subtracting.
diskann/src/error/ann_error.rs:139
Doc comment grammar/typos: "Return true is the the type" / "type ... of in the" reads incorrectly and makes rustdoc harder to follow.

Copilot

Pull request overview

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

Comments suppressed due to low confidence (3)

diskann/src/error/ann_error.rs:139

Doc comment grammar/typos: "is the the" / "of in" reads incorrectly and is confusing.
diskann-label-filter/src/encoded_attribute_provider/roaring_attribute_store.rs:137
Error message typo: "atleast" should be "at least".
diskann-disk/src/utils/partition.rs:261
Error message typo: "dont" should be "don't".

Mark Hildebrand added 8 commits July 27, 2026 11:59

Port Diskann.

59effe0

Migrate diskann-providers.

6058f5b

Update diskann-benchmark-core.

fec88b0

Migrate diskann-label-filter.

28b4e07

Migrate diskann-tools.

16396f8

Update diskann-disk.

5b96256

Format and migrate diskann-bftree.

f511105

Migrade the last residuals.

857d12c

Copilot AI review requested due to automatic review settings July 28, 2026 14:59

Copilot started reviewing on behalf of hildebrandmw July 28, 2026 15:00 View session

Copilot AI reviewed Jul 28, 2026

View reviewed changes

Add docs to diskann_disk::error::Error.

5f723a6

Copilot AI review requested due to automatic review settings July 28, 2026 15:36

Copilot started reviewing on behalf of hildebrandmw July 28, 2026 15:37 View session

Copilot AI reviewed Jul 28, 2026

View reviewed changes

Clean up ANNError.

f699dd9

Copilot AI review requested due to automatic review settings July 28, 2026 15:44

hildebrandmw changed the title ~~Slim down ANNError.~~ Slim down ANNError Jul 28, 2026

Copilot started reviewing on behalf of hildebrandmw July 28, 2026 15:44 View session

Copilot AI reviewed Jul 28, 2026

View reviewed changes

Copilot feedback.

d189163

hildebrandmw marked this pull request as ready for review July 28, 2026 16:19

hildebrandmw requested a review from a team July 28, 2026 16:19

hildebrandmw mentioned this pull request Jul 28, 2026

Remove the old log* style constructors #1278

Draft

5 tasks

Put back full error source formatting in diskann-disk.

c7d9686

Copilot AI review requested due to automatic review settings July 28, 2026 17:29

Copilot started reviewing on behalf of hildebrandmw July 28, 2026 17:29 View session

Copilot AI reviewed Jul 28, 2026

View reviewed changes

harsha-simhadri approved these changes Jul 28, 2026

View reviewed changes

harsha-simhadri requested review from arkrishn94 and randybird July 28, 2026 20:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Slim down `ANNError` - #1282

Slim down `ANNError`#1282
hildebrandmw wants to merge 12 commits into
mainfrom
mhildebr/testing

hildebrandmw commented Jul 28, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

codecov-commenter commented Jul 28, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

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

Uh oh!

Why

Review Order

Alternatives

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

codecov-commenter commented Jul 28, 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

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!

4 participants

hildebrandmw commented Jul 28, 2026 •

edited

Loading

codecov-commenter commented Jul 28, 2026 •

edited

Loading