[MOD-14956] Add SQ8 quantization support for HNSW index

[dor-forer]

Describe the changes in the pull request

Cherry-pick of ARM-software/VectorSimilarity-for-Arm#4 (head 125ea15d), plus a follow-up review pass. Fourth PR in the SQ8 series, after #999, #1000 and #1002.

Adds 8-bit scalar quantization (SQ8) to the standalone HNSW index:

• VecSimQuantType plus quantType / quantParams on HNSWParams. Both fields are appended at the end of the struct and VecSimQuant_NONE is 0, so existing zero-initialized and designated-initializer construction is unaffected.
• HNSWFactory can build SQ8 indexes for FLOAT32 and FLOAT16 data types with the L2 and IP metrics, wiring QuantPreprocessor and DistanceCalculatorWithNorm, and accounts for SQ8 in EstimateInitialSize and EstimateElementSize.
• For SQ8, quantParams points to a float[dim] mean vector; a null pointer selects quantization without mean normalization.
• New test_hnsw_sq8 unit-test target and suite (44 tests: FP32/FP16 x L2/IP).

SQ8 support for the tiered HNSW index, serialization and benchmarks is deferred to later PRs in the series.

Commit 1: the cherry-pick

ARM's four commits squashed into one, with no functional change to their code. Two Redis-side adjustments:

• Dropped the added SPDX-FileCopyrightText Arm line from the two modified files, matching how [MOD-14953] Add calcDistanceForQuery to IndexCalculatorInterface #999, [MOD-14952] Support normalization in QuantPreprocessor #1000 and [MOD-14955] Add DistanceCalculatorWithNorm #1002 landed. It is kept on the new tests/unit/test_hnsw_sq8.cpp, where the BSD-3-Clause identifier was replaced by this repo's Redis tri-license header.
• Wrapped that header so make check-format passes at the 100-column limit.

Commit 2: review follow-up

No behavioural change intended. Interface, single-source-of-truth and idiom fixes:

• quantParams is now const void *. Every use reads it and two already cast to const float *. Layout-identical, so not an ABI break, and callers passing non-const still compile. Better to fix before the field ships and freezes.
• The VecSimQuant_SQ8 comment claimed "with mean normalization", but mean normalization is optional and selected by quantParams. Reworded.
• GetSQ8StoredDataSize re-derived the stored blob size that QuantPreprocessor's constructors already computed. Two formulas for one layout drift silently, which is the bug class fixed in MOD-15303. The formula now lives once as sq8::storage_bytes_count<Metric, WithNorm>(dim), beside the storage_metadata_count it builds on, and both callers use it. This is why the diff touches types/sq8.h and spaces/computer/preprocessors.h, two files beyond ARM's original four: having one shared definition is the entire point of the fix.
• Restored the return NULL closing the SQ8 branch. Unreachable today, since the type and metric checks leave only FP32/FP16 x L2/IP, but without it adding a type or metric silently falls through and builds an unquantized index.
• assert(ret == 0) on addPreprocessor is now assert(ret != -1). The function returns -1 on failure, 0 when the container is full, and the next free index otherwise, so 0 is merely the only success value at the current container size of one. != -1 is the documented contract and the existing repo idiom.
• Hoisted the tail both branches duplicated (container construction, addPreprocessor, assert, IndexComponents, return); only the preprocessor and calculator differ.
• The mean vector is copied with one assign instead of a zero-filling constructor followed by memcpy, which wrote every element twice.
• Obtaining the query alignment required calling GetDistFunc for a function that is never used, since spaces.h offers no alignment-only query and the asymmetric hint covers the storage operand. That call now lives in a small GetQueryAlignment<DataType> adapter returning the hint, so the call site neither discards a value nor keeps a third distance function in scope beside sym_func and asym_func that must never be called. query_alignment is const.
• GetSQ8StoredDataSize is [[nodiscard]] constexpr and dim / with_norm are const.

Commit 3: reject quantized tiered indexes until MOD-14957

Adding quantType to HNSWParams makes it reachable on the tiered path, where nothing handles it. TieredHNSWFactory::NewIndex forwards primaryIndexParams into HNSWFactory::NewIndex, so the primary index quantizes its storage, while NewBFParams does not copy quantType and the frontend stays unquantized. Reachable from any direct C API caller with algo = VecSimAlgo_TIERED and quantType = VecSimQuant_SQ8:

• FP32/FP16: the getStoredDataSize() assert at tiered_factory.cpp:54 aborts on a debug build; under NDEBUG the index is built with mismatched frontend and backend layouts.
• FP64/BF16/INT8/UINT8: HNSWFactory::NewIndex returns NULL for these types under SQ8, and the result is reinterpret_cast and dereferenced with no null check, so the process segfaults.

The catch (...) in index_factory.cpp does not help, since neither an abort nor a null dereference is an exception. RediSearch cannot set quantType until MOD-14958, so there is no product exposure today; the guard exists so main does not carry the defect between cherry-picks. MOD-14957 should replace this check and its test rather than delete them.

Commit 4: cover SQ8 rejection of unsupported data types

HNSWSQ8ParamsTest.RejectsUnsupportedDataType asserts that FLOAT64, BFLOAT16, INT8 and UINT8 with VecSimQuant_SQ8 all return NULL from index creation. Verified red without the fix: with both the type fence and the fall-through return NULL removed, all four are silently built as unquantized indexes. Also documents at EstimateElementSize why the estimate deliberately does not repeat the check (see the Bugbot thread on this PR).

Commit 5: fix two defects found in review (63271c1)

Both raised by @lerman25, both verified before fixing.

• Out-of-bounds read through the public C API. VecSimIndex_GetDistanceFrom_Unsafe documents blob as a raw dim-by-type vector, but the SQ8 kernels read query metadata appended past that, so honouring the documented contract read past the caller's buffer. Reproduced under ASan: heap-buffer-overflow, READ of size 4 in SQ8_FP32_InnerProduct_Impl via vec_sim.cpp:231. getDistanceFrom_Unsafe now returns INVALID_SCORE for a quantized index, the value getDistanceFromInternal already uses for "no answer". AbstractIndexInitParams gains isQuantized for this, parallel to isDisk. Preprocessing internally was rejected because preprocessQuery also normalizes cosine queries, so it would change behaviour for every existing cosine index; a public prepared-query API is the real answer and belongs with MOD-14958.
• Mean-centred FP16 L2 loses correctness. The query is centred then narrowed back to FP16 while storage keeps FP32 min/delta. Verified numerically: mean 10000 gives a per-component error of 1.0 and a self-distance of 4.0 for an identical vector/query pair, and centring -40000 with mean 40000 gives -inf. Now rejected at construction. FP16 + mean + IP is unaffected and still supported, since that path does not centre the query.

The test that should have caught the first one passed a preprocessed blob obtained through a C++-only path no C caller has. It now checks the maths via calcDistanceForQuery and separately asserts the public API reports no answer for a raw vector.

Commit 6: serialization guard and IP graph coverage (9e69259)

• saveIndexIMP refuses a quantized index. The V4 format records neither quantType nor the mean, and the loading path always builds unquantized components, so a saved SQ8 index reloaded with the wrong stride and consumed graph bytes as vector data. Same argument as the tiered guard: fail closed. Caveat: the encoding version is written before this check runs, so a rejected save leaves a stub file, which still fails closed on load.
• GraphConstructionIP. Every other functional test uses L2, so the symmetric SQ8-to-SQ8 IP kernel that graph construction selects was never executed, and this series is the first thing to put it on the insert path. Its vectors vary per component, so it avoids the degenerate min == max branch the other tests take.
• Review nits: assert(false && "...") before the unreachable return NULL (kept alongside it, since assert-only would reopen the silent-fallthrough hole under NDEBUG), and the stray blank line removed.

Commit 7: make the size estimates reject what index creation rejects (d5d93d6)

Closes the three Bugbot findings that were still open, plus one gap none of them covered. No behavioural change on any supported configuration.

The supported-SQ8-combination test now lives in one place instead of being open-coded in NewIndex only: ResolveSQ8Metric applies the is_normalized Cosine-to-IP remap, and SQ8ParamsSupported holds all the fences. Same single-source-of-truth argument as sq8::storage_bytes_count in commit 2.

• NewIndex rejects a quantType that is neither NONE nor SQ8. Previously any other value fell through to the full-precision path and silently built an unquantized index for a caller that asked for a quantized one. Deliberately untested: the enum holds only those two values, so forming a third is undefined behaviour. The guard is what stops adding SQ4 from reopening the hole.
• EstimateInitialSize rejects the same set as NewIndex, replacing a check that validated only the data type. This closes the reported metric gap and one nobody raised: the mean-centred FP16 L2 fence added in 63271c14 was never mirrored into the estimate, so that combination still reported a size.
• TieredHNSWFactory::EstimateInitialSize rejects quantType != NONE, matching its NewIndex guard. It needs its own check rather than inheriting one, since the primary index accepts FP32 with L2 and SQ8 happily and nothing propagates up.

EstimateElementSize is left lax on purpose: size_t return with no sentinel, and VecSimIndex_EstimateElementSize is extern "C", so a throw would carry an exception into the C host. It answers for whatever params it is handed, exactly as VecSimParams_GetStoredDataSize always has on the unquantized path. Both functions now say which one validates and why the other cannot.

Flagged rather than buried: this adds two throw sites reachable through VecSimIndex_EstimateInitialSize, which has no try/catch. That is the existing idiom in both files rather than a new hazard, but the error model for the estimate functions is unsettled and belongs with MOD-14958.

Verified red without the fix: 5 failures, all "throws nothing". RejectsUnsupportedDataType stayed green, since the old code already threw for a bad type.

Commit 8: reject an out-of-range metric, and document the SQ8 distance sentinel (9b1615c)

The two findings from @lerman25's second review round that are inside MOD-14956.

• An out-of-range VecSimMetric aborted the process. FP32 with SQ8 and metric = (VecSimMetric)123 passed the Cosine-only check, matched neither dispatch branch, and hit the assert(false) added in 9e69259c. The unquantized dispatcher throws and VecSimIndex_New catches it, so quantized params were the only way to abort an assertions-enabled host. SQ8ParamsSupported now whitelists L2 and IP. Note commit 7's refactor preserved this hole verbatim rather than introducing it: the check it replaced also tested only for Cosine. RejectsOutOfRangeMetric uses (VecSimMetric)3, outside the valid set but inside the enum's value range, so the test does not itself rely on UB.
• The INVALID_SCORE sentinel is now documented on vec_sim.h. 63271c14 made getDistanceFrom_Unsafe return it for a quantized index, but the declaration still promised a distance, and it is the same NaN used for a missing label, so a caller could treat valid candidates as absent. The declaration now states the restriction, the collision, and that callers must check quantType.

Deferred to new tickets, all under epic MOD-6132

Findings from the second review round that fall outside MOD-14956 ("wire SQ8 into HNSWIndex + factory") and outside the HLD. Each ticket carries the reproduction and the boundary.

Ticket	Finding	Why not here
MOD-17526	Both FP32 SQ8 L2 kernels lose precision through ||x||^2 + ||y||^2 - 2*IP in FP32. Stored [100000, 100008] against query [100000, 100000] returns 0.0 where the truth is 64.0, and quantization is exact for that input. Onset around offset / spread > ~4000.	Pre-existing kernels. The identity is what makes the SQ8-to-SQ8 inner loop a uint8 dot product, so a direct-difference rewrite trades a rare wrong answer for a slowdown nobody measured. Mean normalization removes it entirely, verified to offset 1e6, so the HLD default TRAINING_THRESHOLD is the safe configuration.
MOD-17527	SQ8-to-SQ8 IP SIMD kernels overflow int32 from dim 33,027 (boundary corrected from 33,026: every non-constant quantization has one q = 0).	Pre-existing kernels, and the shared uint8 helper means a fix changes int8/uint8 behaviour too.
MOD-17528	QuantPreprocessor casts NaN to uint8 for finite input when the derived range overflows to infinity. UB on AddVector.	MOD-14952's component. Needs a contract decision, since AddVector cannot report "unquantizable".
MOD-17529	HNSWSerializer::saveIndex truncates the destination before validating, so a rejected save destroys a valid existing snapshot.	Pre-existing ordering that applies to any validation failure, not only the SQ8 guard. Correcting my own earlier claim on that thread that the stub "fails closed".
MOD-17530	BUILD_TESTS getDataByLabel copies dim * sizeof(DataType) from a shorter SQ8 blob.	Test-only, no current caller, so the ASan run here is clean. A landmine for MOD-14957 and MOD-14959.
MOD-17531	GraphConstructionIP executes the symmetric IP kernel but never validates its result (every vector quantizes to the same byte payload); metric and multi axes still uncovered.	Adding the metric axis means reworking hard-coded L2 expectations across several tests, and this is ARM's suite.

Verification

Run on the final tree, after both commits:

Check	Result
./check-format.sh	clean
g++ -Wall -Werror -fsyntax-only, with and without -DNDEBUG	clean
make build DEBUG=1	clean, no warnings
test_hnsw_sq8	43/43 passed
make unit_test DEBUG=1	2650/2650 passed
make asan	2649/2649 passed, 0 sanitizer reports

Not run: FP_64=1 variants, since this change is FP32/FP16 only.
Both new guards were confirmed red without their fix, and the ASan repro above is clean after it.

Test count moved 2653 -> 2650. Dropping FLOAT16-with-mean from the functional type set removed 11 typed tests for a combination that is now rejected, and the two new tests added 6 back. FP16 + mean + IP is left with construction coverage only, and the multi-label path and full metric parameterization remain uncovered; both are tracked in MOD-17531 rather than silently dropped.

Reviewed and deliberately left alone

• The query_alignment hint comes from the symmetric DataType dispatcher while the asymmetric kernel that consumes the query uses unaligned loads (_mm512_loadu_ps). This costs nothing: QuantPreprocessor::preprocessQuery always allocates a fresh blob via allocate_aligned, so the hint only selects that allocation's alignment. It matches the asymmetric-types contract in spaces.h.
• The SQ8 branch of EstimateInitialSize uses <float> for the index class even on the FP16 path. Verified correct with a static_assert on sizeof for both the single and multi index classes.
• mean_sum_squares accumulates in float. It is a constant additive term on the IP path only, identical for every candidate, so it cannot affect ranking, only the absolute reported distance. Left as is.
• Three raw new (allocator) calls with no RAII between them leak if a later constructor throws. preprocessors_factory.h does the same, so this is repo-wide debt rather than something this PR introduced.

Which issues this PR fixes

1. MOD-14956

Main objects this PR modified

1. HNSWFactory index creation and memory estimation
2. HNSWParams and the new VecSimQuantType public API
3. sq8::storage_bytes_count, now the single definition of the SQ8 storage layout size
4. SQ8 HNSW unit-test target and test suite

Mark if applicable

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

🤖 Generated with Claude Code

---

Note

Medium Risk
Touches the public HNSW params API, index construction, distance calculation, and memory estimation. Unsupported paths (tiered, serialization, bad types/metrics) are rejected rather than silently falling through.

Overview
Adds SQ8 quantization to standalone HNSW via new quantType / quantParams fields on HNSWParams (appended, default NONE). FLOAT32 and FLOAT16 with L2 or IP are supported; optional mean centering is copied from quantParams.

HNSWFactory builds quantized indexes with QuantPreprocessor and the matching distance calculators, and size estimates account for the SQ8 stored layout (sq8::storage_bytes_count). getDistanceFrom_Unsafe preprocesses the query on quantized indexes so the public C API does not read past a raw blob.

Unsupported combinations fail closed: unknown quantizers, mean-centered FP16 L2, Cosine without pre-normalization, quantized tiered indexes, and serialization of quantized indexes. A new test_hnsw_sq8 suite covers search, range, batch iteration, distance, and those rejections. Tiered SQ8, save/load, and benchmarks are deferred.

^{Reviewed by Cursor Bugbot for commit 0367f36. Bugbot is set up for automated code reviews on this repo. Configure here.}

[codecov]

Codecov Report

❌ Patch coverage is 97.24771% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.21%. Comparing base (f30a540) to head (0367f36).

Files with missing lines	Patch %	Lines
src/VecSim/index_factories/hnsw_factory.cpp	96.70%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1007      +/-   ##
==========================================
+ Coverage   97.18%   97.21%   +0.02%     
==========================================
  Files         141      141              
  Lines        8432     8533     +101     
==========================================
+ Hits         8195     8295     +100     
- Misses        237      238       +1     

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

[lerman25]

I lack context for this,
Left some comments, some are AI that seem reasonable
Also there are other AI comments if you can address them

[lerman25]

Suggested change

Nit

[dor-forer]

Will take it, thanks. Cosmetic only, so I have grouped it with the assert suggestion below rather than pushing a commit for a blank line on its own.

[dor-forer]

Done in 9e69259.

[lerman25]

This test bypasses the public VecSimIndex_GetDistanceFrom_Unsafe contract by manually constructing an internal processed query. The API documents blob as a raw type×dimension vector, and C callers have no preprocessing API. For FP32 dim=4 L2, a valid raw query is 16 bytes, but the SQ8 kernel reads the appended y_sum and y_sum_squares at bytes 16–23, causing out-of-bounds reads. Please test this API with query directly and either preprocess internally, expose a public reusable prepared-query context, or reject direct-distance lookups for SQ8.

[dor-forer]

Confirmed and fixed in 63271c1. You were right, and it is worse than a contract mismatch: it is an out-of-bounds read reachable from the public C API. I reproduced it under AddressSanitizer with a dim=4 FP32 L2 SQ8 index and a correctly sized 16-byte heap query:

ERROR: AddressSanitizer: heap-buffer-overflow, READ of size 4
  #0 SQ8_FP32_InnerProduct_Impl            IP.cpp:65
  #6 VecSimIndex_GetDistanceFrom_Unsafe    vec_sim.cpp:231

Your diagnosis of why the suite missed it is also exactly right: the test obtained a preprocessed blob through CastToHNSW()->preprocessQuery(...), a C++-only path no C caller has.

getDistanceFrom_Unsafe now returns INVALID_SCORE for a quantized index, which is already the value getDistanceFromInternal uses for "no answer", so it needs no new error channel. I considered your first suggestion, preprocessing internally, and did not take it here: preprocessQuery also normalizes cosine queries, so applying it would change behaviour for every existing cosine index, and it would add a per-call allocation on RediSearch's scoring path. Your third suggestion, a public reusable prepared-query context, is the right long-term answer and belongs with MOD-14958, which is what first exposes any of this to the host.

The test now checks the distance maths through calcDistanceForQuery and separately asserts the public API reports no answer for a raw vector, so the raw-blob call is exercised under ASan by every type parameter.

[lerman25]

The out-of-bounds read is fixed by the rejection path. One API-contract detail remains: vec_sim.h still promises that this function returns the distance for a matching type×dimension blob, while an SQ8 index now returns INVALID_SCORE for every label—the same NaN used for a missing label. A standalone caller cannot distinguish “SQ8 operation unsupported” from “label absent.” If the prepared-query API is intentionally deferred, please document this SQ8 restriction and sentinel behavior on the public declaration so callers and MOD-14958 do not treat valid candidates as missing.

[dor-forer]

Fair, and fixed in 9b1615c. The sentinel collision is a real trap: INVALID_SCORE is the same NaN getDistanceFromInternal returns for a missing label, so a standalone caller cannot tell "SQ8 unsupported" from "label absent" and could drop valid candidates as missing. That is exactly the failure mode MOD-14958 would hit.

vec_sim.h now documents it on the declaration rather than leaving the old promise standing:

 * NOT SUPPORTED for a quantized index (HNSWParams::quantType != VecSimQuant_NONE), which always
 * returns INVALID_SCORE here, for every label. The quantized kernels read query metadata appended
 * past the raw vector, which a blob matching the documented type and dimension does not carry, so
 * honouring the contract above would read past the caller's buffer. Note that INVALID_SCORE is the
 * same NaN returned when the label is absent, so a caller cannot distinguish the two: check
 * quantType rather than inferring it from the result. Obtaining a real distance needs a prepared
 * query, which has no public API yet.

The prepared-query API is still the real answer and still belongs with MOD-14958, but a caller reading the header now finds out before writing the bug rather than after.

[lerman25]

Re-verified against 9e69259c6adc6f5e3118459c2fe565c3490af517 with focused runtime reproducers.

[lerman25]

Blocking: the compact storage enabled here also reaches the BUILD_TESTS getDataByLabel helpers, which still copy dim * sizeof(DataType) from the stored pointer. I reproduced this on the current head with FP32/L2, dim=128, blockSize=1: the SQ8 data is 144 bytes, and ASan reports a 512-byte read past the resulting 160-byte element allocation at hnsw_single.h:55. hnsw_multi.h:80 has the same copy. Since this flag is now available, please either reject that helper for quantized indexes or safely dequantize into DataType, with single and multi regressions.

[dor-forer]

Confirmed. Both call sites do exactly what you describe:

memcpy(vec.data(), this->getDataByInternalId(id), this->dim * sizeof(DataType));

hnsw_single.h:55 and hnsw_multi.h:80, and for a quantized index the stored blob is one byte per component plus 16 to 20 bytes of metadata, so dim * sizeof(DataType) overshoots by design. Your dim=128 FP32 numbers line up: 144 bytes stored, 512 read.

Tracked as MOD-17530 rather than fixed here. Two reasons, and one caveat against myself.

MOD-14956 is scoped to the factory path, the calcDistanceForQuery call sites and the new HNSWParams fields; these are BUILD_TESTS helpers that no product path reaches, and no current test calls them on a quantized index, which is why the ASan run on this PR is clean. So there is no exposure to close today.

The caveat: this is a landmine for the next tickets in the epic. MOD-14957 and MOD-14959 will naturally call getDataByLabel on a quantized index and get a heap overread instead of a clear failure. The ticket says so, and prefers dequantizing into DataType over rejecting, since recovering vectors for comparison is exactly what these helpers exist for. If you would rather have the guard in this PR so main never carries it, that is a four-line change plus the two regressions you asked for and I will add it.

[lerman25]

Okay to defer to MOD-17530. I rechecked the scope: both affected helpers are compiled only under BUILD_TESTS, no product path reaches them, and this PR does not call them for a quantized index. The ticket should remain a prerequisite for tests in MOD-14957 or MOD-14959, but I do not consider it merge-blocking for this PR.

[cursor]

getDataByLabel SQ8 over-read

High Severity

getDataByLabel still copies dim * sizeof(DataType) from the stored pointer. SQ8 blobs are much smaller (bytes plus metadata), so under BUILD_TESTS this reads past the element allocation. The public distance path was guarded for quantization, but these helpers were not.

Additional Locations (1)

• src/VecSim/algorithms/hnsw/hnsw_multi.h#L70-L82

 

^{Reviewed by Cursor Bugbot for commit d5d93d6. Configure here.}

[dor-forer]

Confirmed. Both getDataByLabel overloads copy dim * sizeof(DataType) from stored data, which overshoots an SQ8 blob of one byte per component plus 16 to 20 bytes of metadata. Same finding @lerman25 raised, with an ASan repro at FP32 dim=128: 144 bytes stored, 512 read.

Tracked as MOD-17530. Both helpers are behind BUILD_TESTS and no current test calls them on a quantized index, which is why the ASan run on this PR is clean, so there is nothing to close today; the ticket exists because MOD-14957 and MOD-14959 will call them and deserve a clear failure rather than a heap overread.

[cursor]

Quantization hits UB on finite input

High Severity

quantize derives delta and inv_delta from max - min in FP32 with no finiteness check. Finite inputs such as [-FLT_MAX, +FLT_MAX] make the range and delta infinite, inv_delta zero, then Inf * 0 yields NaN; casting that NaN to uint8_t is undefined behavior. AddVector on an SQ8 index therefore executes C++ UB for otherwise valid finite vectors.

 

^{Reviewed by Cursor Bugbot for commit 9b1615c. Configure here.}

[dor-forer]

Confirmed, same defect @lerman25 raised on this PR, tracked as MOD-17528. Mechanism verified from preprocessors.h:273-275: diff = inf, delta = inf, inv_delta = 0, then inf * 0 is NaN and the static_cast<uint8_t> is UB.

Two things worth adding to the record, because they cut in opposite directions.

Against deferring, and I will not lean on "pre-existing" here: QuantPreprocessor merged with MOD-14952 in #1000, but before this PR nothing constructed an index that used it, so it was reachable only from test_components. This PR is what first puts it on AddVector. So while the buggy code is not new, its reachability from the public API is.

For deferring: the fix is not a finiteness check away, which is why it needs the preprocessor owner rather than a cherry-pick. I measured the obvious repair:

current, all FP32:            diff=inf  delta=inf  inv_delta=0   -> value = nan
widen diff to double only:    diff=6.8e38  delta=2.67e36 (finite, fits FP32)
                              but per-element (x - min_val) in FP32 is still inf
                              -> value = inf, and casting inf to uint8_t is equally UB
widen the per-element
subtraction to double too:     -> value = 255.0, correct

So the range is not the only thing that overflows: the per-element (x - min_val) does too. Getting this right means either double arithmetic per element on the insert path, which is a throughput decision on the hot path, or a reformulation such as x * inv_delta - min_val * inv_delta. Either way it is a design call with a benchmark attached, and AddVector has no way to report "unquantizable" if the answer turns out to be rejection.

The ticket carries both the UBSan output and this table, and asks for a regression at [-FLT_MAX, +FLT_MAX] plus one where the range overflows without either endpoint being extreme.

[lerman25]

Blocking: SQ8 L2 mixes metadata from the original stored vector with a cross term computed from its quantized reconstruction, so it can return a negative distance even at small magnitudes. On ac05e54 through the public C API, storing [0, 0.25, 1] and querying [0, 0.2501, 1] makes a radius-0 query return the label with score -0.000490427. This is distinct from MOD-17526: there is no large offset, and the failure comes from the norm and cross term describing different vectors. Please make all terms use the same representation, or use a stable direct-difference formulation, and add this non-grid radius-zero regression.

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit ace52fa. Configure here.}

[cursor]

Estimate API throws into C callers

High Severity

HNSWFactory::EstimateInitialSize and TieredHNSWFactory::EstimateInitialSize throw std::invalid_argument for rejected SQ8 combinations. That exception is not caught by the extern "C" VecSimIndex_EstimateInitialSize wrapper, so a C host (for example FP32 plus SQ8 plus Cosine) terminates instead of getting a failure size. Index creation already returns NULL for the same params.

Additional Locations (1)

• src/VecSim/index_factories/tiered_factory.cpp#L68-L74

 

^{Reviewed by Cursor Bugbot for commit ace52fa. Configure here.}