fix(rank-fusion): show previous page on multi-page semantic-only results#3180
Merged
Conversation
The offset field in QueryResponseList was doing double duty: it is used by the rank-fusion merge step to inflate the effective record count when the primary (BM25/default) searcher returns few or zero hits while a secondary searcher returns many, but calculatePageInfo() also reused that same offset-adjusted value (startWithOffset) as a navigation origin-shift for pagination flags. When offset inflated to roughly the full result count, startWithOffset clamped to 0 on every page, making existPrevPage false on all pages past the first and producing a spurious next page one page beyond the actual end. This change decouples the navigation flags from that count-inflation role: existPrevPage now also considers currentPageNumber, and existNextPage is computed from the raw start (consistent with allPageCount, which is derived from allRecordCount) instead of the offset-deflated startWithOffset. No other pagination logic changed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When rank fusion runs and the primary (BM25 /
default) searcher returns few or zero hits while a secondary searcher (e.g. a semantic/multimodal searcher) returns many, pagination breaks on multi-page result sets:prev_pageisfalseon every page past the first, so themes that key their pager onprev_page/next_pagehide the whole control and the user cannot navigate back.page_countinflates by one, one page past the actual end.Example: a query that matches 37 documents only via the secondary searcher (4 pages of 10) reports
prev_page=falseon pages 2–4 andpage_count=5past the end. A normal keyword query paginates correctly, because BM25 contributes to the main searcher's hit set.Root cause
QueryResponseList'soffsetfield does double duty:offsetwhen the primary searcher contributes few/zero hits (allRecordCount = mainCount + offset).calculatePageInfo()also reused that same offset-adjusted value (startWithOffset = start - offset) as a navigation origin-shift for the pagination flags.When the primary searcher returns ~0 hits,
offsetinflates to roughly the full result count, sostartWithOffsetclamps to0on every page. That makesexistPrevPage = startWithOffset > 0false on all pages, and a spurious next page one page past the end is then collapsed into an inflatedallPageCount.Fix
Decouple the navigation flags from the count-inflation role inside
calculatePageInfo():existPrevPagenow also considerscurrentPageNumber— a previous page exists whenever we are past page 1.existNextPageis computed from the rawstart, keeping it consistent withallPageCount(which is derived fromallRecordCount), instead of the offset-deflatedstartWithOffset.No other pagination logic changes. For
offset == 0(all non-fusion / single-searcher paths) the behavior is provably identical to before, and normal deep-pagination behavior is preserved.Tests
Added
QueryResponseListTest#test_calculatePageInfo_rankFusionZeroMainMultiPagecovering the zero-primary multi-page case: previous page present on pages 2–4, no spurious next page,page_countstays 4, and no previous page on page 1.QueryResponseListTest(38 tests) andRankFusionProcessorTest(11 tests) pass with no regressions.