[MOD-17795] Compare merge scores exactly, so a mid-ingest label is not returned twice

[dor-forer]

Describe the changes in the pull request

An FT.SEARCH ... =>[KNN ...] reply can carry the same document key twice, and on a full reply each duplicate costs a real document (reported: 2000 keys, 1999 distinct, over a 2000-document corpus; worst seen 18 repeats in one reply). The duplicate is produced inside VecSim, in the tiered frontend/backend result merge.

VecSimTieredIndex::topKQueryImp queries the flat buffer and backend separately and merges the two lists. The old single-value path passed withSet=false, so it relied on a shared id reaching both cursors at the same time. This PR now gives every public one-shot tiered top-K and range query explicit cross-tier ID deduplication whenever both tiers can contribute more than one result.

It did not agree. Both lists are ordered by exact score — HNSWIndex::topKQuery and BruteForceIndex::topKQuery drain a max-heap keyed on (distance, label), and the batch iterators sort by score then id — while the comparator called any two scores within 1e-6 equal and fell back to ordering by id. Wherever those two orders disagree, the flat buffer's copy of a label and the backend's copy miss each other and both land in the reply.

A label is in both indexes for as long as an ingest job takes to get from inserting into HNSW to erasing it from the flat buffer, so a corpus dense enough to put unequal distances within 1e-6 of each other duplicates documents under any sustained write load. That matches the report: it reproduces during the background scan and also after it (percent_indexed=1.0, ingest queue not yet drained), on both FLOAT32 and FLOAT16, with a bit-identical __dist on both rows and neighbouring distances ~1.2e-7 apart — inside the tolerance.

This drops the tolerance. The rewritten comparator also fixes the id comparison, which computed (int)(res1->id - res2->id) and so truncated a 64-bit label difference to int, giving the wrong sign for ids more than 2^31 apart.

The exact comparator fixes inconsistent merge ordering everywhere it is used. Public one-shot topKQuery and rangeQuery additionally use a smaller-side ID state map, so a shared ID is returned once even when its two scores differ. The batch iterator still uses its existing merge/persistent-state logic and remains a separate follow-up for arbitrary cross-tier score disparity.

Which issues this PR fixes

1. MOD-17795

Main objects this PR modified

1. cmpVecSimQueryResultByScoreThenId() — exact score comparison and sign-correct ID comparison.
2. merge_results_with_cross_tier_dedup() — an allocator-aware unordered_map containing only the smaller input's IDs and their emitted state.
3. merge_result_lists() — skips deduplication state when the limit is at most one or one input is empty, because those shapes cannot emit a cross-tier duplicate.
4. Public tiered topKQuery() / rangeQuery() — perform cross-tier ID deduplication for non-trivial one-shot queries.
5. Regression tests — cover near-tied exact ordering, unequal duplicate scores in both directions, exact duplicates, large IDs, top-K filling, trivial-merge fast paths, and public top-K/range dispatch (BY_SCORE and BY_ID).

Testing

The new test builds the minimal shape: two labels whose L2 distances differ by less than 1e-6 but are not equal, with the farther one also sitting in the flat buffer, as an in-flight ingest job leaves it. It asserts the distance gap as a precondition so it cannot silently stop exercising the case.

Verified on dorer-intel (Ubuntu 24.04, gcc 13.3):

• Before the fix, the test fails with the reported signature — id: 1 twice at a bit-identical score:

  Value of: allUniqueResults(res)
    Actual: false
  id: 1, score: 1.0000004768371582
  id: 2, score: 1
  id: 1, score: 1.0000004768371582
  

• After the final trivial-merge fast path: 100% tests passed, 0 tests failed out of 2752 (8 expected skips).

Why exact comparison and explicit deduplication are both needed

The comparator must match the lists' exact (score, id) ordering; a tolerance makes the merge order inconsistent and is not transitive. Exact comparison fixes MOD-17795's near-tied-neighbour case, where the duplicated label itself has a bit-identical score.

Exact comparison alone cannot deduplicate a shared ID whose frontend/backend scores differ. Public one-shot top-K and range queries therefore perform ID deduplication whenever a reply could contain the duplicate twice. merge_result_lists preloads only the smaller input's IDs into a reserved allocator-aware unordered_map and marks the first occurrence emitted. IDs absent from the smaller input cannot be cross-tier duplicates and pass without insertion. If limit <= 1 or either input is empty, merge_result_lists uses the compile-time merge_results<false> path because that reply cannot emit a cross-tier duplicate.

Seven-repetition dorer-intel measurements compare this standard map with the removed custom flat hash in the same binary: the map was +0.8%, +3.1%, and +6.2% for top-K 10/100/1000, and +0.6% for a 1,026-result range query. The isolated merge is 2–6× slower, but vector search dominates end-to-end latency. The custom table can return in a separate optimization PR if those query-level costs prove material.

The trivial-merge dispatch was then measured in a 15-repetition, randomized-interleaved run on dorer-intel using the same release binary for both paths. In the isolated merge, K=1 improved from 128 ns to 57 ns (~55%); merging 1,000 results with an empty side improved from 3,102 ns to 2,654 ns when the first input was empty and from 3,042 ns to 1,394 ns when the second was empty. End-to-end top-K K=1 changed from 278.695 us to 278.120 us (-0.21%). K=10 through K=1,000 stayed within -0.21% to +0.22%, showing no meaningful cost from the runtime condition on ordinary queries.

The remaining merge_results<false> calls are the trivial one-shot cases above, internal same-iterator merges where duplicate IDs are impossible by construction, plus the existing single-value HNSW batch path. Exact comparison also fixes the single-value batch case when bit-identical cross-tier copies meet in the same merge: they now align and are consumed together. It does not make batch iteration robust to arbitrary score disparity or copies arriving in different batches; that requires persistent cross-batch state and remains a separate follow-up.

This reverts #694, and why the implementation changed

Before #694 the comparator used exact score comparison. #694 added the tolerance so a shared label with slightly different frontend/backend scores could align and deduplicate without an ID set. That solved one case but made the comparator disagree with the exact ordering of the input lists, allowing different near-tied labels to move the cursors out of alignment.

This PR restores exact ordering and gives deduplication a separate owner. The smaller-side state map handles unequal scores directly for public one-shot queries, rather than encoding approximate identity into the ordering comparator.

Known debt left behind: MOD-17920

spaces::normalizeVector_imp() divides by the vector norm with no zero-norm guard, so a zero vector in a cosine index normalizes to all-NaN. The integer merge comparator does not treat such a score as equal: NaN != score is true and NaN > score is false, so it returns -1 regardless of which operand is NaN. That makes the merge order inconsistent and can select or truncate the wrong results. Separately, the score comparators passed to std::sort do not provide a strict weak ordering in the presence of NaN. This predates the PR and is untouched by it; filed as MOD-17920 with a proposed fix.

The explicit != form avoids treating double's std::partial_ordering::unordered result as equality and then advancing both merge cursors. It does not make NaN scores supported; MOD-17920 must either prevent them or define a total ordering for them. The comparator can be simplified after NaN scores are made unreachable.

Mark if applicable

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

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes core tiered KNN/range result merging for all backends (HNSW, SVS), which affects search correctness under background indexing; batch iterator merge behavior is largely unchanged.

Overview
Fixes duplicate labels in tiered top-K and range replies when the same document sits in both the flat buffer and backend during ingest. The merge comparator no longer treats scores within 1e-6 as equal; it compares exact (score, id) order to match how HNSW and brute-force lists are built, and compares IDs without truncating to int.

Public one-shot tiered queries always merge through merge_result_lists, which uses merge_results_with_cross_tier_dedup (IDs tracked from the smaller list) when more than one result can come back. That emits a shared label once even when frontend and backend scores differ. Range queries ordered by ID use filter_results_by_id that keeps the better score for cross-tier duplicates.

Removes the withSet template split on VecSimTieredIndex and the tiered SVS overrides that forced set-based merge. Adds regression tests for near-tied scores, merge dedup edge cases, and BY_ID range on tiered search.

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.30%. Comparing base (f30a540) to head (8bd95c7).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1022      +/-   ##
==========================================
+ Coverage   97.21%   97.30%   +0.09%     
==========================================
  Files         141      141              
  Lines        8432     8657     +225     
==========================================
+ Hits         8197     8424     +227     
+ Misses        235      233       -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.