Skip to content

More tests to increase code coverage for diskann#953

Merged
JordanMaples merged 25 commits intomainfrom
jordanmaples/migrate_tests_pt3_v2
Apr 22, 2026
Merged

More tests to increase code coverage for diskann#953
JordanMaples merged 25 commits intomainfrom
jordanmaples/migrate_tests_pt3_v2

Conversation

@JordanMaples
Copy link
Copy Markdown
Contributor

This is in service of #927.

I was discussing the above issue with @hildebrandmw and he pointed out that one of the problems we have with code coverage is that there is a delta in reported coverage between runs where only diskann is specified and runs where diskann and diskann-providers is specified.

I ran the code coverage tool locally to identify some of these deltas and added a couple tests to gain coverage for index.rs.

move helper function to dedicated file so it can be used in next tests

adding so I can switch branches really quickly

inplace_delete and multi_inplace_delete tests

a couple more tests
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds additional unit tests in diskann’s graph test suite to increase coverage in graph/index.rs and related deletion/consolidation paths, helping reduce coverage deltas vs. runs that include diskann-providers.

Changes:

  • Adds new cases/index.rs tests covering QueryLabelProvider::on_visit default behavior and DiskANNIndex::count_reachable_nodes.
  • Expands inplace-delete test coverage across multiple InplaceDeleteMethod variants with post-delete graph validation.
  • Introduces shared 2D-square graph fixtures in cases/helpers.rs and adds consolidation tests for multiple ConsolidateKind outcomes.

Reviewed changes

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

Show a summary per file
File Description
diskann/src/graph/test/cases/mod.rs Registers new test modules (helpers, index).
diskann/src/graph/test/cases/helpers.rs Adds shared 2D-square index/graph setup helpers used by multiple test cases.
diskann/src/graph/test/cases/index.rs Adds tests targeting graph/index.rs code paths (QueryLabelProvider, count_reachable_nodes).
diskann/src/graph/test/cases/inplace_delete.rs Adds tests for additional inplace-delete methods and validates resulting neighbor structure.
diskann/src/graph/test/cases/consolidate.rs Extends consolidation test coverage for deleted/no-op/repair cases and transient failures.

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

Comment thread diskann/src/graph/test/cases/consolidate.rs Outdated
Comment thread diskann/src/graph/test/cases/helpers.rs
Comment thread diskann/src/graph/test/cases/helpers.rs Outdated
…pers

- Fix consolidate.rs doc comment: 'three code paths' → 'four code paths'
- Add assert for adjacency_lists.len() >= num_points in setup_2d_square
- Include start_adj fallback length in provider_max_degree calculation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 16, 2026

Codecov Report

❌ Patch coverage is 97.46193% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.47%. Comparing base (a90deb4) to head (1304820).

Files with missing lines Patch % Lines
diskann/src/graph/test/cases/inplace_delete.rs 94.47% 11 Missing ⚠️
diskann/src/graph/test/cases/index.rs 98.72% 5 Missing ⚠️
diskann/src/graph/test/cases/consolidate.rs 96.96% 2 Missing ⚠️
diskann/src/graph/test/cases/helpers.rs 97.72% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #953      +/-   ##
==========================================
+ Coverage   89.34%   89.47%   +0.12%     
==========================================
  Files         447      449       +2     
  Lines       83250    84038     +788     
==========================================
+ Hits        74378    75191     +813     
+ Misses       8872     8847      -25     
Flag Coverage Δ
miri 89.47% <97.46%> (+0.12%) ⬆️
unittests 89.31% <97.46%> (+0.13%) ⬆️

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

Files with missing lines Coverage Δ
diskann/src/graph/test/provider.rs 94.40% <100.00%> (+3.64%) ⬆️
diskann/src/graph/test/synthetic.rs 100.00% <100.00%> (ø)
diskann/src/graph/test/cases/consolidate.rs 97.65% <96.96%> (-0.74%) ⬇️
diskann/src/graph/test/cases/helpers.rs 97.72% <97.72%> (ø)
diskann/src/graph/test/cases/index.rs 98.72% <98.72%> (ø)
diskann/src/graph/test/cases/inplace_delete.rs 95.76% <94.47%> (-4.24%) ⬇️

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

JordanMaples and others added 16 commits April 16, 2026 14:12
- Add Two variant to synthetic::Grid for 2D grid data and neighbor generation
- Update setup_2d_square to accept Matrix<f32> from Grid::Two.data()
- Replace hand-crafted create_2d_unit_square with Grid::Two.data(2)
- Add IntoIterator for &Provider to support get_degree_stats
- Custom adjacency lists preserved for topology-sensitive tests

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- test_prune_range: verifies nodes with degree > pruned_degree get pruned
- test_get_degree_stats: asserts all four DegreeStats fields on 2D square

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…une boundary

- test_count_reachable_nodes_multiple_starts: two disconnected components,
  verifies union of reachable sets from multiple start points
- test_get_degree_stats_mixed: heterogeneous degrees (0, 1, 2, 2),
  validates min/max/avg/cnt_less_than_two independently
- prune_range boundary: asserts start node outside range is untouched

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…with_init_ids

Tests cover:
- Basic single-page paged search with result ordering
- Multi-page iteration with uniqueness validation
- Error cases: k > l_value, k == 0, undersized output buffer
- Equivalent tests for start_paged_search_with_init_ids variant

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add inplace_delete_onehop test with structural validation
- Add delete_isolated_node test for empty worklist edge case
- Extract delete_node_3_and_validate and multi_delete_2_and_3_and_validate
  helpers to reduce duplication across 5 tests

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Exercises multi_inplace_delete with 3 simultaneous deletes [0, 4, 6]
using multi_thread runtime to test parallel spawn + edge merge path.
Validates deleted nodes absent from all neighbor lists and remaining
graph stays fully reachable.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…neighbors

Exercises the path where consolidate_vector prunes a node that exceeds
pruned_degree but has no deleted neighbors (no edge absorption, just
robust_prune_list).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…oop paths

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Implement glue::IdIterator for the test Accessor so flat_search can
iterate all point IDs. Add two tests: basic (all points returned) and
filtered (excluded points omitted from results).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JordanMaples added a commit that referenced this pull request Apr 17, 2026
These tests are now covered by equivalent tests in diskann added in #953:
- test_return_refs_to_deleted_vertex
- test_is_any_neighbor_deleted
- test_drop_deleted_neighbors
- test_get_undeleted_neighbors

Also removes the setup_inplace_delete_test helper and unused
PartitionedNeighbors import.

Part of #927.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JordanMaples JordanMaples enabled auto-merge (squash) April 20, 2026 19:23
JordanMaples added a commit that referenced this pull request Apr 22, 2026
)

This is a follow up PR for #904, which has an outdated baseline and can
be closed with this superseding it.
Essentially the same work was done here just without the
provider/async_/caching changes. This closes #903.

When #953 merges, the from_dim definition will need to be updated to
support the '2' case.

Replace generate_structured_data.rs and
generate_synthetic_labels_utils.rs with
diskann::graph::test::synthetic::Grid and diskann-tools' own label
generator respectively.

- Rewrite GenerateGrid trait impls to delegate to Grid::data/data_as
- Replace all adj list generation calls with Grid::neighbors
- Rewire generate_synthetic_labels binary to diskann-tools version
- Delete both redundant utility files and clean up exports

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread diskann/src/graph/test/cases/consolidate.rs
@JordanMaples JordanMaples merged commit e247bfd into main Apr 22, 2026
26 checks passed
@JordanMaples JordanMaples deleted the jordanmaples/migrate_tests_pt3_v2 branch April 22, 2026 17:59
JordanMaples added a commit that referenced this pull request Apr 22, 2026
These tests are now covered by equivalent tests in diskann added in
#953:
- test_return_refs_to_deleted_vertex
- test_is_any_neighbor_deleted
- test_drop_deleted_neighbors
- test_get_undeleted_neighbors

Also removes the setup_inplace_delete_test helper and unused
PartitionedNeighbors import.

Part of #927. This will remain in draft state until #953 merges.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

5 participants