Skip to content

fix: hybrid search silently ignores .offset() - #3769

Merged
wjones127 merged 2 commits into
lancedb:mainfrom
Adityaj0:fix/hybrid-query-offset-ignored
Aug 17, 2026
Merged

wjones127 merged 2 commits into
lancedb:mainfrom
Adityaj0:fix/hybrid-query-offset-ignored

Conversation

@Adityaj0

@Adityaj0 Adityaj0 commented Aug 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

LanceHybridQueryBuilder (sync hybrid search, table.search(query_type="hybrid")) silently ignored .offset(). self._offset was never forwarded to the vector/FTS sub-queries and never applied when slicing the final combined/reranked result, so .offset(N) behaved identically to .offset(0) — no error, just wrong pagination.

Fixes #3765

Changes

  • _create_query_builders(): each sub-query now fetches limit + offset rows so there's enough data to slice the correct window out of after combining/reranking.
  • _combine_hybrid_results() / to_arrow(): the final table is sliced with offset=self._offset instead of always starting at 0.

Test plan

  • New regression test test_hybrid_query_offset in python/python/tests/test_hybrid_query.py
  • uv run --extra tests pytest python/tests/test_hybrid_query.py -vv — 13 passed
  • uv run --extra dev ruff format / ruff check — clean

LanceHybridQueryBuilder never forwarded self._offset to its vector/FTS
sub-queries or applied it when slicing the final combined, reranked
result, so .offset(N) behaved identically to .offset(0) with no error.

Fixes lancedb#3765

Co-Authored-By: Claude Sonnet 5 <[email protected]>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@github-actions github-actions Bot added bug Something isn't working Python Python SDK labels Aug 3, 2026
@Adityaj0

Adityaj0 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

@wjones127 can you review this too?

@wjones127
wjones127 self-requested a review August 3, 2026 21:45

@wjones127 wjones127 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!

@wjones127
wjones127 merged commit d742b17 into lancedb:main Aug 17, 2026
12 of 13 checks passed
wjones127 pushed a commit that referenced this pull request Sep 8, 2026
Fixes #4027

## Summary

`AsyncHybridQuery`
(`table.query().nearest_to(...).nearest_to_text(...)`) paginates
incorrectly when `.offset()` is used: the second page repeats rows from
the first page and silently drops others.

`offset()` on a hybrid query pushes the offset down into *both*
sub-queries (`HybridQuery::offset` in `python/src/query.rs` forwards to
`inner_vec` and `inner_fts`), so each sub-query independently skips its
own first `offset` rows before the results are fused.
`AsyncHybridQuery.to_batches` then called `_combine_hybrid_results(...,
limit=self._inner.get_limit())` without an `offset`, so the reranked
table was sliced starting at position 0 and the sub-query limits were
never raised to cover the skipped prefix.

On the 4-row fixture in `test_hybrid_query.py`, with `_rowid` ordering
`[3, 0, 2, 1]`:

| query | before | after |
| --- | --- | --- |
| `.limit(2)` | `[0, 3]` | `[0, 3]` |
| `.offset(2).limit(2)` | `[3, 1]` | `[2, 1]` |

Row `3` was returned on both pages and row `2` was never returned at
all.

This is the async counterpart of #3769 (`Fixes #3765`), which fixed the
same bug in the synchronous `LanceHybridQueryBuilder`. #3765 explicitly
deferred the async path; this PR closes that gap and reuses the `offset`
parameter that #3769 already added to `_combine_hybrid_results`. The
synchronous path is unaffected — it was fixed in #3769.

## Changes

`python/python/lancedb/query.py`, `AsyncHybridQuery.to_batches`:

- Each sub-query now fetches `limit + offset` rows and its own offset is
reset to 0, so the fused result contains the full prefix the window is
sliced out of.
- The combined, reranked table is sliced with `offset=` instead of
always starting at 0.

Both halves are needed: raising the sub-query limits without the final
slice still returns page 1, and slicing without raising the limits still
misses rows.

`nodejs` has no equivalent hybrid combine path, so there is no SDK
parity gap here.

## Test plan

- [x] New regression test `test_async_hybrid_query_offset` in
`python/python/tests/test_hybrid_query.py`, mirroring the sync
`test_hybrid_query_offset`. It asserts the offset window is a suffix of
the un-offset result *and* that page 1 + page 2 together cover every row
exactly once (a row-count-only assertion would pass even with
duplicates).
- [x] `pytest python/tests/test_hybrid_query.py` — 16 passed
- [x] `pytest python/tests/test_rerankers.py` — 9 passed, 11 skipped
- [x] `pytest python/tests/test_query.py` — 86 passed
- [x] `pytest --doctest-modules python/lancedb/query.py` — 13 passed
- [x] `ruff format --check` / `ruff check` — clean
---

## Scope, after review

@lancedb-gatekeeper raised three points. Two were mine and are fixed in
`04d07c2`; the third is deliberately left alone and I'd like a
maintainer's call on it.

**Fixed — effective limit was read from the FTS child only.**
`HybridQuery::get_limit()` (`python/src/query.rs:1159`) returns
`self.inner_fts.inner.current_request().limit`, so an FTS-first hybrid
with no explicit `.limit()` yielded `None`, skipped the widening branch
and passed `limit=None` to the combiner — returning the union of both
candidate lists instead of the documented default of 10. The limit is
now derived from both children with a `DEFAULT_HYBRID_LIMIT = 10`
fallback, so construction order no longer matters.

**Fixed — `explain_plan()` / `analyze_plan()` described a different
query than the one that ran.** Both built their children straight from
`self._inner`, bypassing the limit/offset rewrite in `to_batches`, and
reported `skip=2, fetch=2` while execution used `skip=0, fetch=4`. Child
preparation now lives in one `_create_child_queries()` helper used by
all three.

> **Visible change to `explain_plan()` output:** because the plan is now
built from the real execution children, which carry `with_row_id()`, the
two `ProjectionExec` lines gain a `_rowid` column. The doctest is
updated to match. This is the diagnostic becoming truthful rather than
the assertion being weakened — it is still an exact-match comparison.

**Not fixed here — RRF candidate-pool invariance.** Widening each
sub-query to `limit + offset` does change the candidate pool between
page requests, so the fused ranking can shift and pagination can still
repeat rows. That's a real problem, but it is exactly what the merged
sync path does today:

```python
# LanceHybridQueryBuilder (sync), merged in #3769
sub_query_limit = self._limit + (self._offset or 0)
```

Making the pool invariant means choosing a contract — a fixed candidate
pool, or an explicit cursor — and that ought to apply to sync and async
together rather than letting the two paths diverge. I've asked in the
review thread which way you'd prefer, and I'm happy to do it here or in
a follow-up covering both paths.

So, to be precise about what this PR delivers: it makes `.offset()` take
effect on the async hybrid path and makes the diagnostics honest. It
does not make hybrid pagination stable across pages under reranking —
that needs the contract decision above.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Python Python SDK

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(python): hybrid search silently ignores .offset()

2 participants