Skip to content

[diskann-garnet] FFI updates and additions - #1336

Open
Jack Moffitt (metajack) wants to merge 6 commits into
mainfrom
push-ykmnvnykyzpw
Open

[diskann-garnet] FFI updates and additions#1336
Jack Moffitt (metajack) wants to merge 6 commits into
mainfrom
push-ykmnvnykyzpw

Conversation

@metajack

Copy link
Copy Markdown
Contributor

This series of commits add several new FFI functions and makes a few FFI updates.

  1. VRANDMEMBER is implemented via a new random_members() function.
  2. VLINKS is implemented via a new search_neighbors() function.
  3. A new logCallback is now passed which is used to send log messages to Garnet, and several log messages were added.
  4. A value length hint was added to readCallback, which is allowed to be wrong but is better to overestimate.
  5. A return result has been added to backfill_quant_vectors().
  6. The filterCallback now takes a ptr and length to the attribute data instead of the internal id. Now diskann-garnet is responsible for looking up attributes, and attributes have moved to being key off the internal ID, reducing keys read during filtered search from 3 to 2 (old: extid map, attributes, vector data; new: attributes, vector data).

Each commit is self contained, but the version is bumped in the first commit.

Comment thread diskann-garnet/src/lib.rs Dismissed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the diskann-garnet FFI surface to support additional Garnet commands and to improve observability and efficiency (logging callback, read length hints, and attribute-based filtering keyed by internal IDs). It also introduces new FFI entrypoints for random member selection and neighbor listing, and bumps the package version to 5.0.0.

Changes:

  • Extend FFI callbacks: add logCallback, add a length_hint parameter to readCallback, and change filterCallback to accept attribute bytes rather than internal IDs.
  • Add new FFI functions: random_members() (VRANDMEMBER) and search_neighbors() (VLINKS), plus provider-side support (random_members, neighbors).
  • Change attribute storage to be keyed by internal ID (reducing reads during filtered search), and make backfill_quant_vectors() return a bool status.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
diskann-garnet/src/lib.rs Adds new FFI entrypoints and extends the create_index ABI with a log callback.
diskann-garnet/src/provider.rs Implements provider-side random member selection and neighbor extraction; updates attribute storage semantics.
diskann-garnet/src/garnet.rs Updates callback type signatures (read length hint, filter data pointer/len, log callback) and threads hints through read helpers.
diskann-garnet/src/test_utils.rs Updates test callback shims to match new ABI and captures log messages.
diskann-garnet/src/dyn_index.rs Extends the dynamic index trait to include new FFI-backed operations.
diskann-garnet/src/fsm.rs Updates FSM metadata existence checks to pass a read length hint.
diskann-garnet/src/ffi_tests.rs Wires the new log callback into existing FFI tests.
diskann-garnet/src/ffi_recall_tests.rs Wires the new log callback into recall tests.
diskann-garnet/Cargo.toml Bumps crate version to 5.0.0.
diskann-garnet/diskann-garnet.nuspec Bumps NuGet package version to 5.0.0.
Cargo.lock Updates locked diskann-garnet version.
kann-garnet New file added; appears to be an accidental terminal/help-text dump.
╭───────────┬──────────────────────────────╮ New file added; appears to be an accidental terminal/help-text dump.
Suppressed comments (2)

╭───────────┬──────────────────────────────╮:5

  • This file name/content looks like an accidental terminal/UI capture (duplicate less help text) rather than a real project file. It should not be checked in as it can cause tooling issues (paths, packaging) and adds repository clutter.
    diskann-garnet/src/garnet.rs:300
  • Same truncation issue here: length_hint as u32 can wrap/truncate on 64-bit if the hint exceeds u32::MAX. Clamping to u32::MAX keeps the "hint" semantics without changing behavior for normal sizes.
        unsafe {
            (self.read_callback)(
                ctx.inner,
                ids.len() as u32 / 2,
                length_hint as u32,

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread kann-garnet Outdated
Comment thread diskann-garnet/src/lib.rs
Comment thread diskann-garnet/src/lib.rs Outdated
Comment thread diskann-garnet/src/provider.rs
Comment thread diskann-garnet/src/garnet.rs
Comment thread diskann-garnet/src/garnet.rs
@codecov-commenter

Codecov Comments Bot (codecov-commenter) commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.36684% with 134 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.50%. Comparing base (3218478) to head (9e7189b).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
diskann-garnet/src/provider.rs 55.50% 105 Missing ⚠️
diskann-garnet/src/garnet.rs 53.33% 14 Missing ⚠️
diskann-garnet/src/test_utils.rs 55.17% 13 Missing ⚠️
diskann-garnet/src/lib.rs 99.19% 2 Missing ⚠️

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

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1336      +/-   ##
==========================================
- Coverage   91.55%   91.50%   -0.06%     
==========================================
  Files         522      522              
  Lines       99541    99991     +450     
==========================================
+ Hits        91139    91500     +361     
- Misses       8402     8491      +89     
Flag Coverage Δ
miri 91.50% <76.36%> (-0.06%) ⬇️
unittests 91.18% <76.36%> (-0.05%) ⬇️

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

Files with missing lines Coverage Δ
diskann-garnet/src/dyn_index.rs 79.81% <100.00%> (+3.47%) ⬆️
diskann-garnet/src/fsm.rs 94.10% <100.00%> (ø)
diskann-garnet/src/lib.rs 94.45% <99.19%> (+2.02%) ⬆️
diskann-garnet/src/test_utils.rs 92.89% <55.17%> (-5.52%) ⬇️
diskann-garnet/src/garnet.rs 93.97% <53.33%> (-2.81%) ⬇️
diskann-garnet/src/provider.rs 79.34% <55.50%> (-2.36%) ⬇️

... and 44 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread diskann-garnet/src/lib.rs
Comment thread diskann-garnet/src/lib.rs
return -1;
};

for nbr in neighbors {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could output.extend(neighbors) work?

Comment thread diskann-garnet/src/garnet.rs
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