mirror of
https://github.com/lancedb/lancedb.git
synced 2026-10-29 06:56:15 +00:00
fix(python): keep FTS scores in RRF and MRR rerankers with return_score="all" (#4677)
`RRFReranker.rerank_hybrid` and `MRRReranker.rerank_hybrid` merge the
vector and FTS results with the base `merge_results`, which keeps only
the first copy of a duplicated row. With `return_score="all"`, a row
that both searches found therefore comes back with `_score = None`, even
though the FTS search scored it. `LinearCombinationReranker` already
copies the FTS score onto such rows.
Reproduction against `lancedb` 0.40.0:
```python
import pyarrow as pa
from lancedb.rerankers import RRFReranker
vector = pa.table({"_rowid": pa.array([1, 2], pa.uint64()), "text": ["a", "b"],
"_distance": pa.array([0.1, 0.2], pa.float32())})
fts = pa.table({"_rowid": pa.array([2, 3], pa.uint64()), "text": ["b", "c"],
"_score": pa.array([5.0, 4.0], pa.float32())})
RRFReranker(return_score="all").rerank_hybrid("q", vector, fts).to_pylist()
# row 2: _distance=0.2, _score=None (expected _score=5.0)
```
`table.search(query_type="hybrid")` is not affected, because
`_combine_hybrid_results` restores the original `_distance` and `_score`
by `_rowid` after reranking. The wrong value shows up when
`rerank_hybrid` is called directly, for example from a custom reranker
that wraps RRF or MRR.
The fix uses `_merge_and_keep_scores` when `return_score="all"`, the
same pattern the model-based rerankers (`CohereReranker`,
`CrossEncoderReranker`, `JinaReranker`, `OpenaiReranker`,
`VoyageAIReranker`, ...) already use. The relevance-only path is
unchanged.
Tests: added `test_rank_fusion_return_score_all_keeps_both_scores` for
both rerankers. It fails without the fix and passes with it. I ran it
against the released 0.40.0 package with this branch's
`rerankers/{base,rrf,mrr}.py` dropped in; `rrf.py` and `mrr.py` on
`main` are identical to 0.40.0. The other reranker tests give the same
results with and without the change. `ruff format` and `ruff check` are
clean.
I used an AI assistant (Claude) while working on this; I reviewed and
tested the change.
Co-authored-by: Xuanwo <github@xuanwo.io>
This commit is contained in:
@@ -97,7 +97,12 @@ class MRRReranker(Reranker):
|
||||
weighted_mrr = self.weight_vector * vector_rr + self.weight_fts * fts_rr
|
||||
final_mrr_scores[result_id] = weighted_mrr
|
||||
|
||||
combined_results = self.merge_results(vector_results, fts_results)
|
||||
# With return_score="all", a row found by both searches must keep its
|
||||
# `_distance` and its `_score`.
|
||||
if self.score == "all":
|
||||
combined_results = self._merge_and_keep_scores(vector_results, fts_results)
|
||||
else:
|
||||
combined_results = self.merge_results(vector_results, fts_results)
|
||||
combined_row_ids = combined_results["_rowid"].to_pylist()
|
||||
relevance_scores = [final_mrr_scores[row_id] for row_id in combined_row_ids]
|
||||
combined_results = combined_results.append_column(
|
||||
|
||||
@@ -54,8 +54,12 @@ class RRFReranker(Reranker):
|
||||
for i, result_id in enumerate(ids, 1):
|
||||
rrf_score_map[result_id] += 1 / (i + self.K)
|
||||
|
||||
# Sort the results based on RRF score
|
||||
combined_results = self.merge_results(vector_results, fts_results)
|
||||
# Sort the results based on RRF score. With return_score="all", a row found
|
||||
# by both searches must keep its `_distance` and its `_score`.
|
||||
if self.score == "all":
|
||||
combined_results = self._merge_and_keep_scores(vector_results, fts_results)
|
||||
else:
|
||||
combined_results = self.merge_results(vector_results, fts_results)
|
||||
combined_row_ids = combined_results["_rowid"].to_pylist()
|
||||
relevance_scores = [rrf_score_map[row_id] for row_id in combined_row_ids]
|
||||
combined_results = combined_results.append_column(
|
||||
|
||||
@@ -428,6 +428,38 @@ def test_rrf_multivector_ranks_each_result_list():
|
||||
assert result["_rowid"].to_pylist() == [2, 1, 3]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("reranker_cls", [RRFReranker, MRRReranker])
|
||||
def test_rank_fusion_return_score_all_keeps_both_scores(reranker_cls):
|
||||
# A row found by both searches must keep its vector `_distance` and its FTS
|
||||
# `_score` when the reranker is asked to return all scores.
|
||||
vector_results = pa.table(
|
||||
{
|
||||
"_rowid": pa.array([1, 2], type=pa.uint64()),
|
||||
"text": ["a", "b"],
|
||||
"_distance": pa.array([0.1, 0.2], type=pa.float32()),
|
||||
}
|
||||
)
|
||||
fts_results = pa.table(
|
||||
{
|
||||
"_rowid": pa.array([2, 3], type=pa.uint64()),
|
||||
"text": ["b", "c"],
|
||||
"_score": pa.array([5.0, 4.0], type=pa.float32()),
|
||||
}
|
||||
)
|
||||
|
||||
result = reranker_cls(return_score="all").rerank_hybrid(
|
||||
"query", vector_results, fts_results
|
||||
)
|
||||
rows = {row["_rowid"]: row for row in result.to_pylist()}
|
||||
|
||||
assert rows[1]["_distance"] == pytest.approx(0.1)
|
||||
assert rows[1]["_score"] is None
|
||||
assert rows[2]["_distance"] == pytest.approx(0.2)
|
||||
assert rows[2]["_score"] == pytest.approx(5.0)
|
||||
assert rows[3]["_distance"] is None
|
||||
assert rows[3]["_score"] == pytest.approx(4.0)
|
||||
|
||||
|
||||
def test_rrf_reranker_distance():
|
||||
data = pa.table(
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user