Skip to content

MOD-13177 add support for RERANK parameter in HNSW disk#943

Merged
BenGoldberger merged 5 commits intomainfrom
beng-add-rerank-config
May 4, 2026
Merged

MOD-13177 add support for RERANK parameter in HNSW disk#943
BenGoldberger merged 5 commits intomainfrom
beng-add-rerank-config

Conversation

@BenGoldberger
Copy link
Copy Markdown
Collaborator

@BenGoldberger BenGoldberger commented Apr 30, 2026

Describe the changes in the pull request

add support for rerank config

Which issues this PR fixes

  1. #...
  2. MOD...

Main objects this PR modified

  1. ...
  2. ...

Mark if applicable

  • This PR introduces API changes
  • This PR introduces serialization changes

Note

Medium Risk
Adds new query-parameter parsing for disk-based HNSW and changes query-param initialization semantics, which could affect runtime query behavior if incorrectly gated or defaulted.

Overview
Adds support for a new RERANK runtime query parameter for disk-based HNSW: introduces VecSimCommonStrings::HNSW_RERANK_STRING, a TRUE/FALSE validator, and resolver logic that initializes hnswDiskRuntimeParams.shouldRerank to VecSimBool_UNSET and rejects RERANK on non-disk or non-HNSW indexes.

Refactors VecSimIndex_ResolveParams to use basicInfo().isDisk and a dedicated _InitQueryParams initializer instead of a raw zeroing, and adds test-only setIsDiskForTesting plus new unit tests covering defaults, acceptance/rejection, and error paths for RERANK on “disk” vs RAM HNSW.

Minor follow-ups: fixes TieredSVSIndex::debugInfo() to use the boolean result of atomic_flag::test(), and updates tiered-info tests to compare backgroundIndexing against VecSimBool_TRUE/FALSE.

Reviewed by Cursor Bugbot for commit e26e87e. Bugbot is set up for automated code reviews on this repo. Configure here.

@jit-ci
Copy link
Copy Markdown

jit-ci Bot commented Apr 30, 2026

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR


Security scan by Jit

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.90%. Comparing base (5bcc53e) to head (e26e87e).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #943      +/-   ##
==========================================
+ Coverage   96.71%   96.90%   +0.19%     
==========================================
  Files         129      129              
  Lines        8057     7683     -374     
==========================================
- Hits         7792     7445     -347     
+ Misses        265      238      -27     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

GuyAv46
GuyAv46 previously approved these changes Apr 30, 2026
Comment thread src/VecSim/vec_sim.cpp Outdated
// Zero-initialize qparams and explicitly set fields whose "absent" state is not
// representable by all-zeros (e.g. tristate enums where 0 maps to a valid value).
static void _InitQueryParams(VecSimQueryParams *qparams, VecSimAlgo index_type, bool is_disk) {
bzero(qparams, sizeof(VecSimQueryParams));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

bzero is actually deprecated. Consider taking this opportunity and replacing it with memset.

@BenGoldberger BenGoldberger added this pull request to the merge queue May 3, 2026
@BenGoldberger BenGoldberger removed this pull request from the merge queue due to a manual request May 3, 2026
@BenGoldberger BenGoldberger added this pull request to the merge queue May 3, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 3, 2026
@BenGoldberger BenGoldberger added this pull request to the merge queue May 3, 2026
@BenGoldberger BenGoldberger removed this pull request from the merge queue due to a manual request May 3, 2026
@BenGoldberger BenGoldberger added this pull request to the merge queue May 3, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 3, 2026
@BenGoldberger BenGoldberger added this pull request to the merge queue May 3, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 3, 2026
@BenGoldberger BenGoldberger added this pull request to the merge queue May 4, 2026
Merged via the queue into main with commit 4df68c9 May 4, 2026
17 checks passed
@BenGoldberger BenGoldberger deleted the beng-add-rerank-config branch May 4, 2026 06:18
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.

2 participants