[Sparse ANN] Fold sparse vector tokens into the signed-short range

[chishui]

Description

Token IDs are folded into [0, MODULUS_FOR_SHORT) via token % 65536 and stored in a signed short[] to keep the memory footprint low. Reads went directly through tokens[i], so any token folding into [32768, 65535] was sign-extended back to a negative int, causing:

• getToken()/iterator() to return a negative token (e.g. 40000 -> -25536)
• toDenseVector() to throw NegativeArraySizeException on maxToken + 1
• dotProduct() to mis-score via a negative index / bad bounds check

Route every token read through a getToken(int) helper that widens with Short.toUnsignedInt. Writes are unchanged since the (short) cast is a lossless reinterpretation; only reads needed to treat the value as unsigned. Sorting in processListItems() is by the folded non-negative value, so ascending order and dotProduct()'s early-exit remain valid.

Note this path is reachable only when the pure-Java SEISMIC path runs (segment doc count >= approximate_threshold, default 1M); below that the query falls back to plain neural sparse, and the native engine passes raw ints to C++ where term_t is uint16_t. That gating is why the defect went unnoticed.

Adds unit tests covering the constructor/iterator round-trip, toDenseVector(), and dotProduct() for a token folding to a negative short.

Related Issues

Resolves #1925

Check List

• New functionality includes testing.
• New functionality has been documented.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit d0642fb)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Increased Hash Collisions Changing MODULUS_FOR_SHORT from 65536 to 32768 halves the token space, doubling the collision rate for tokens folded into short storage. Two distinct original tokens that previously mapped to different slots (e.g., 40000 and 7232, or 65535 and 32767) will now collide. The PR description mentions this only affects the pure-Java SEISMIC path (segment doc count >= 1M), but the increased collision rate could measurably degrade recall/scoring accuracy for large sparse vectors. Consider whether using Short.toUnsignedInt with modulus 65536 (as the description hints was the original intent) would better preserve the token space while still fixing the sign-extension bug. // Tokens are stored in a signed short[] to keep the memory footprint low, so they are folded // into the non-negative signed-short range [0, Short.MAX_VALUE] via this modulus. Using 32768 // (not 65536) keeps every folded value <= Short.MAX_VALUE, so it round-trips without being // sign-extended to a negative int on read. public static final int MODULUS_FOR_SHORT = 32768;

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Preserve backward compatibility for existing indices Changing MODULUS_FOR_SHORT from 65536 to 32768 is a breaking change for any previously indexed sparse vectors: existing on-disk short[] token arrays were written under modulus 65536 and will now be interpreted under modulus 32768, silently corrupting recall for old indices. Consider a backward-compatibility path (e.g., versioned codec, migration, or reading old segments with the old modulus) rather than switching the constant unconditionally. src/main/java/org/opensearch/neuralsearch/sparse/common/SparseConstants.java [22-26] // Tokens are stored in a signed short[] to keep the memory footprint low, so they are folded // into the non-negative signed-short range [0, Short.MAX_VALUE] via this modulus. Using 32768 // (not 65536) keeps every folded value <= Short.MAX_VALUE, so it round-trips without being // sign-extended to a negative int on read. +// NOTE: Older segments were written with modulus 65536; ensure a compatibility path exists +// for reading them before removing the old value entirely. public static final int MODULUS_FOR_SHORT = 32768; Suggestion importance[1-10]: 7 __ Why: The concern about backward compatibility is legitimate—changing the modulus from 65536 to 32768 would change how tokens are folded, potentially affecting previously indexed data. However, without knowing the release state of the feature, this may or may not be a real issue.	Medium

[github-actions]

Persistent review updated to latest commit d0642fb

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.41%. Comparing base (5facc2e) to head (d0642fb).

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1926      +/-   ##
============================================
- Coverage     83.45%   83.41%   -0.04%     
+ Complexity     3884     3881       -3     
============================================
  Files           291      291              
  Lines         13819    13819              
  Branches       2294     2294              
============================================
- Hits          11532    11527       -5     
- Misses         1454     1457       +3     
- Partials        833      835       +2     

☔ View full report in Codecov by Harness.
📢 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.

[chishui]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category **Suggestion                                                                                                                                    ** Impact
Possible issue
Preserve backward compatibility for existing indices
Changing MODULUS_FOR_SHORT from 65536 to 32768 is a breaking change for any previously indexed sparse vectors: existing on-disk short[] token arrays were written under modulus 65536 and will now be interpreted under modulus 32768, silently corrupting recall for old indices. Consider a backward-compatibility path (e.g., versioned codec, migration, or reading old segments with the old modulus) rather than switching the constant unconditionally.

src/main/java/org/opensearch/neuralsearch/sparse/common/SparseConstants.java [22-26]

 // Tokens are stored in a signed short[] to keep the memory footprint low, so they are folded
 // into the non-negative signed-short range [0, Short.MAX_VALUE] via this modulus. Using 32768
 // (not 65536) keeps every folded value <= Short.MAX_VALUE, so it round-trips without being
 // sign-extended to a negative int on read.
+// NOTE: Older segments were written with modulus 65536; ensure a compatibility path exists
+// for reading them before removing the old value entirely.
 public static final int MODULUS_FOR_SHORT = 32768;

Suggestion importance[1-10]: 7
__

Why: The concern about backward compatibility is legitimate—changing the modulus from 65536 to 32768 would change how tokens are folded, potentially affecting previously indexed data. However, without knowing the release state of the feature, this may or may not be a real issue.

Medium

Previously, token id from 65536 to 32768 will cause the NegativeArraySizeException, and since the hash and casting to short is runtime not persisted, so changing MODULUS_FOR_SHORT to 32768 won't cause backward incompatibility.

[opensearch-ci-bot]

The backport to 3.3 failed. Please backport manually. See failed workflow run: https://github.com/opensearch-project/neural-search/actions/runs/30889311098

[opensearch-ci-bot]

The backport to 3.5 failed. Please backport manually. See failed workflow run: https://github.com/opensearch-project/neural-search/actions/runs/30889319217

[opensearch-ci-bot]

The backport to 3.7 failed. Please backport manually. See failed workflow run: https://github.com/opensearch-project/neural-search/actions/runs/30889333452