mirror of
https://github.com/lancedb/lancedb.git
synced 2026-11-13 14:16:54 +00:00
fix(python)!: drop vector and FTS scores from an empty hybrid result
The `Reranker` docstring says the default `return_score="relevance"` "will return only the relevance score" (`rerankers/base.py`), but a hybrid search whose filter matches nothing also returns `_distance` and `_score`. The early return for two empty result sets in `_combine_hybrid_results` (from #3030) skips the reranker's `_keep_relevance_score`, which every built-in reranker applies to a non-empty result. Before, on `45dbf5e`: ``` $ python repro.py match: ['id', 'text', 'vector', '_relevance_score'] no match: ['id', 'text', 'vector', '_distance', '_score', '_relevance_score'] ``` After, `no match` also prints `['id', 'text', 'vector', '_relevance_score']`. <details><summary>repro.py</summary> ```python import tempfile, lancedb, pyarrow as pa db = lancedb.connect(tempfile.mkdtemp()) t = db.create_table("t", pa.table({"id": [1, 2], "text": ["red fox", "blue fox"], "vector": pa.array([[1.0, 2.0], [3.0, 4.0]], pa.list_(pa.float32(), 2))})) t.create_fts_index("text") q = lambda where: t.search(query_type="hybrid").vector([1.0, 2.0]).text("fox").where(where, prefilter=True).limit(5) print("match: ", q("id > 0").to_arrow().column_names) print("no match:", q("id > 100").to_arrow().column_names) ``` </details> No issue exists for this. The fix calls `reranker._keep_relevance_score(empty)` before returning. Sync and async hybrid queries both go through it. With `return_score="all"` both columns stay, as before. Code reading `_distance` from an empty result now gets a `KeyError`, which it already got from every non-empty one. The new test asserts, for both settings, that a filter matching nothing gives the same columns as one matching every row. ### Verification ``` # Python 3.12.6; lancedb 0.41.0b15 wheel from pypi.fury.io, its _lancedb.abi3.so in the tree; no pylance (none >= 15 resolves) # the wheel predates #4688, so a 3-line -p plugin sets _lancedb._experimental_functions_enabled = False for tests/conftest.py $ PYTHONPATH=python python -m pytest -q -p no:cacheprovider python/tests/test_rerankers.py python/tests/test_hybrid_query.py python/tests/test_query.py45dbf5e: 5 failed, 197 passed, 57 skipped this branch: 5 failed, 199 passed, 57 skipped query.py reverted, test kept: 6 failed, 198 passed, 57 skipped FAILED test_rerankers.py::test_empty_hybrid_result_keeps_reranker_columns[relevance] At index 2 diff: '_distance' != '_relevance_score' $ ruff format --check <both files> && ruff check <both files> # ruff 0.9.9, as pre-commit pins it 2 files already formatted All checks passed! # the 5 failures fail identically on45dbf5e: 4 test_query_serialization_async_search_options (wheel older than #4659) and # test_hybrid_query_with_stale_fixed_size_binary_prefilter (needs pylance) # not run: pyright, the Rust build, the remote client ``` @Xuanwo, a custom reranker that keeps extra columns still gets the generic empty table. I'd keep it that way, since #3030 returns early so that no reranker runs on empty input. If you'd rather call `rerank_hybrid` on the empty tables instead, I'll change it in this PR.
This commit is contained in:
@@ -2788,7 +2788,10 @@ class LanceHybridQueryBuilder(LanceQueryBuilder):
|
||||
)
|
||||
if not with_row_ids and "_rowid" in empty.column_names:
|
||||
empty = empty.drop(["_rowid"])
|
||||
return empty
|
||||
# Match the columns a non-empty result has for this reranker. Call the
|
||||
# base method so an override that assumes rows never sees the empty
|
||||
# table (#3030).
|
||||
return Reranker._keep_relevance_score(reranker, empty)
|
||||
|
||||
original_distances = None
|
||||
original_scores = None
|
||||
|
||||
@@ -836,6 +836,56 @@ def test_empty_hybrid_result_reranker():
|
||||
assert "_rowid" in result.column_names
|
||||
|
||||
|
||||
@pytest.mark.parametrize("reranker_cls", [LinearCombinationReranker, RRFReranker])
|
||||
@pytest.mark.parametrize("return_score", ["relevance", "all"])
|
||||
def test_empty_hybrid_result_keeps_reranker_columns(
|
||||
tmp_path, reranker_cls, return_score
|
||||
):
|
||||
# A filter that matches nothing must not change which columns come back.
|
||||
db = lancedb.connect(tmp_path)
|
||||
table = db.create_table(
|
||||
"t",
|
||||
pa.table(
|
||||
{
|
||||
"text": ["red fox", "blue fox"],
|
||||
"vector": pa.array([[1.0, 2.0], [3.0, 4.0]], pa.list_(pa.float32(), 2)),
|
||||
}
|
||||
),
|
||||
)
|
||||
table.create_fts_index("text")
|
||||
reranker = reranker_cls(return_score=return_score)
|
||||
|
||||
def columns(where):
|
||||
query = table.search(query_type="hybrid").vector([1.0, 2.0]).text("fox")
|
||||
query = query.where(where, prefilter=True).rerank(reranker)
|
||||
return query.to_arrow().column_names
|
||||
|
||||
assert columns("text = 'none'") == columns("text != 'none'")
|
||||
|
||||
|
||||
def test_empty_hybrid_result_skips_keep_relevance_score_override(tmp_path):
|
||||
# An override written for non-empty results must not run on the empty path.
|
||||
class FirstRowReranker(LinearCombinationReranker):
|
||||
def _keep_relevance_score(self, combined_results):
|
||||
combined_results["_relevance_score"][0]
|
||||
return super()._keep_relevance_score(combined_results)
|
||||
|
||||
db = lancedb.connect(tmp_path)
|
||||
table = db.create_table(
|
||||
"t",
|
||||
pa.table(
|
||||
{
|
||||
"text": ["red fox", "blue fox"],
|
||||
"vector": pa.array([[1.0, 2.0], [3.0, 4.0]], pa.list_(pa.float32(), 2)),
|
||||
}
|
||||
),
|
||||
)
|
||||
table.create_fts_index("text")
|
||||
query = table.search(query_type="hybrid").vector([1.0, 2.0]).text("fox")
|
||||
query = query.where("text = 'none'", prefilter=True).rerank(FirstRowReranker())
|
||||
assert query.to_arrow().num_rows == 0
|
||||
|
||||
|
||||
@pytest.mark.parametrize("norm", ["score", "rank"])
|
||||
def test_hybrid_return_all_restores_original_scores(norm):
|
||||
from lancedb.query import LanceHybridQueryBuilder
|
||||
|
||||
Reference in New Issue
Block a user