Skip to content

[draft] Add null check on native create_index return and fix test callsites for new metricType param#1639

Open
Copilot wants to merge 4 commits intomainfrom
copilot/update-diskann-garnet-version
Open

[draft] Add null check on native create_index return and fix test callsites for new metricType param#1639
Copilot wants to merge 4 commits intomainfrom
copilot/update-diskann-garnet-version

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 18, 2026

DiskANNService.CreateIndex was not validating the return value from the native create_index FFI call. A null pointer would silently propagate and crash later with an opaque error.

Changes

  • DiskANNService.cs: Check create_index return for nint.Zero and throw GarnetException with a clear message. Since RecreateIndex delegates to CreateIndex, all callsites are covered.
  • DiskANNServiceTests.cs: Update 3 create_index callsites to pass the new metricType parameter added in the v1.0.25 P/Invoke signature change.
var index = NativeDiskANNMethods.create_index(context, dimensions, reduceDims, quantType,
    (int)distanceMetric, buildExplorationFactor, numLinks, ...);
if (index == nint.Zero)
    throw new GarnetException("Failed to create DiskANN index, native create_index returned null");

Copilot AI and others added 4 commits March 18, 2026 21:59
…rameter in create_index FFI

Co-authored-by: hailangx <3389245+hailangx@users.noreply.github.com>
… null

Co-authored-by: hailangx <3389245+hailangx@users.noreply.github.com>
Co-authored-by: hailangx <3389245+hailangx@users.noreply.github.com>
Co-authored-by: hailangx <3389245+hailangx@users.noreply.github.com>
@hailangx hailangx changed the title Add null check on native create_index return and fix test callsites for new metricType param [draft] Add null check on native create_index return and fix test callsites for new metricType param Mar 18, 2026
@hailangx hailangx marked this pull request as ready for review March 18, 2026 23:11
Copilot AI review requested due to automatic review settings March 18, 2026 23:11
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 a defensive check around DiskANN native index creation to fail fast with a clear managed exception, and updates tests/callers to match the updated native create_index signature introduced by the diskann-garnet package bump.

Changes:

  • Add nint.Zero validation for NativeDiskANNMethods.create_index and throw GarnetException on failure.
  • Update DiskANNServiceTests callsites to pass the newly-required metricType parameter.
  • Bump diskann-garnet package version from 1.0.23 to 1.0.25.

Reviewed changes

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

File Description
libs/server/Resp/Vector/DiskANNService.cs Adds null-pointer check after native create_index call; updates P/Invoke signature to include metricType.
test/Garnet.test/DiskANN/DiskANNServiceTests.cs Updates direct create_index usages to include VectorDistanceMetricType argument.
Directory.Packages.props Upgrades native DiskANN package dependency to 1.0.25.

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

You can also share your feedback on Copilot code review. Take the survey.

uint dimensions,
uint reduceDims,
VectorQuantType quantType,
int metricType,
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.

3 participants