fix(lumina): break vector search score ties deterministically - #725
fix(lumina): break vector search score ties deterministically#725jackylee-ch wants to merge 1 commit into
Conversation
| min_heap.push(ScoredRow { row_id, score }); | ||
| } | ||
| min_heap.push(entry); | ||
| } else if min_heap |
There was a problem hiding this comment.
Both production callers pass exactly effective_k labels and the same value as top_k, so labels.len() <= top_k and this replacement branch cannot execute. The new tests trigger it only by passing more candidates than production can return. If ties are discarded at the native top-k cutoff, those rows never reach this comparator. Please enforce the tie-break before that cutoff (or return more than k candidates) and test through the real call contract.
| impl ScoredRow { | ||
| fn is_stronger_than(&self, other: &Self) -> bool { | ||
| self.score | ||
| .total_cmp(&other.score) |
There was a problem hiding this comment.
[P2] Do not promote NaN scores above finite candidates
f32::total_cmp ranks a positive NaN above every finite score, so this changes more than the tie-break. With top_k = 1 and native results [(row 8, 0.5), (row 7, NaN)], the current code ignores the later NaN because NaN > 0.5 is false, while this comparator evicts row 8 and returns the NaN-scored row. A non-finite stored vector can produce such a distance; the PK-vector metric code uses java_float_compare specifically to keep all NaNs worst. Please skip/reject non-finite scores or use NaN-worst comparison semantics here, and change the regression test so a finite candidate wins regardless of arrival order.
collect_resultsin the Lumina reader kept the top-k by score alone, so rowssharing a score were retained on a first-come basis and the arrival order comes
from the native searcher. The same query against the same index could therefore
return a different row set, and the eviction check used a bare
score > peek.scorerather than a total order.This is the one search backend the invariant established by #613 and #614 did
not reach.
vector_search::ScoredRowalready breaks ties on the row id androutes eviction through
is_stronger_than;full_text::top_kcarries the samerule with a comment stating the order must be deterministic regardless of input
or shard order.
Fix: mirror the sibling comparator —
total_cmpon the score, then the rowid — and evict through it instead of
>. Among equal scores the smallest row idnow wins, in both the single-query and the batch path, which slice the same
helper.
Verified at the unit level: the three new tests feed tied scores in permuted
orders and fail on current main. Reproducing the drift end to end needs
LUMINA_LIB_PATH, which CI does not provide, so no run-time evidence is claimedhere — the tests cover the selection logic directly.