Skip to content

Fix L1 quality for multi-retriever - #1000

Merged
swethakann merged 2 commits into
mainfrom
RP-16289_fix_num_hits_to_collect_multiretriever
Jul 14, 2026
Merged

Fix L1 quality for multi-retriever#1000
swethakann merged 2 commits into
mainfrom
RP-16289_fix_num_hits_to_collect_multiretriever

Conversation

@swethakann

@swethakann swethakann commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

When a multi-retriever has an L1 rescorer with windowSize > topHits, expand collection to windowSize so the rescorer sees the full candidate pool, then truncate back to topHits after rescoring (keeping the best N by L1 score) before feeding to the blender.

  • RetrieverContext — added topHits field
  • SearchRequestProcessor — numHitsToCollect = max(topHits, rescorer.windowSize)
  • SearchHandler — truncate TopDocs to topHits after rescore

@swethakann swethakann changed the title Fix hits collected for retrievers Fix L1 quality for multi-retriever Jul 13, 2026
aprudhomme
aprudhomme previously approved these changes Jul 13, 2026
long rescoreStart = System.nanoTime();
topDocs = retrieverContext.getRescoreTask().rescore(topDocs, searchContext);
rescoreTimeMs = (System.nanoTime() - rescoreStart) / 1_000_000.0;
int topHits = retrieverContext.getTopHits();

@waziqi89 waziqi89 Jul 13, 2026

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.

Why do we add this truncation here? we don't do this in the default path, I don't see particular reason why this shall be different.

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.

I went through the code again. We do slice based on the topHits in default path. Then we can reuse this method getHitsFromOffset(topDocs, 0, retriever.topHits)

}

@Test
public void testL1RescorerTruncatesToTopHitsBeforeBlend() {

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.

Ain't the two tests the same

@waziqi89 waziqi89 Jul 14, 2026

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.

we may to do set up a stricter test:
topHits 3, rescorer window 10. docs ranked 8-10 are rescored to the top so they end up showing the results. This cannot be the case if only 3 got rescored

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have combined the tests in the follow up commit if that makes it easier for you.

We don't need the quality check at the moment imo. Re-ranking at L1 is done by inference plugin in prod so we only need a functionality check. I had verified the inference code does score descending yesterday before this change. So that's good.

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.

I think the gap missed testing is:
all docs: big number - firstPassSearch(topHits:3) -> filter docs: 10 (verify this, not 3) -> rescorer(window: 10) -> blender&response 3

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

After checking the code, the change is pretty trivial but the test set up was complex. Instead captured it in a simple method that is easy to isolate for testing.

@swethakann
swethakann merged commit b80037c into main Jul 14, 2026
2 checks passed
swethakann added a commit that referenced this pull request Jul 14, 2026
Fix hits collected for retrievers
swethakann added a commit that referenced this pull request Jul 14, 2026
swethakann added a commit that referenced this pull request Jul 14, 2026
Fix hits collected for retrievers
swethakann added a commit that referenced this pull request Jul 14, 2026
Fix hits collected for retrievers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants