fix(core): retriever_weights not applied in QueryFusionRetriever reciprocal_rerank mode#21445
fix(core): retriever_weights not applied in QueryFusionRetriever reciprocal_rerank mode#21445gautamvarmadatla wants to merge 5 commits into
retriever_weights not applied in QueryFusionRetriever reciprocal_rerank mode#21445Conversation
VANDRANKI
left a comment
There was a problem hiding this comment.
The fix is correct and addresses two separate issues in _reciprocal_rerank_fusion.
Bug 1 (weights ignored): for nodes_with_scores in results.values() threw away the retriever index, making self._retriever_weights unreachable. Switching to results.items() and multiplying by self._retriever_weights[retriever_idx] is the right fix. Note that _retriever_weights is already normalized to sum to 1.0 by QueryFusionRetriever.__init__, so the multiplication is safe.
Bug 2 (node mutation): reranked_nodes[-1].score = score was mutating the original NodeWithScore objects in hash_to_node. Creating a new NodeWithScore(node=..., score=score) is the right approach - it avoids side effects on any caller that retains references to the original nodes.
Tests are the best part of this PR. test_rrf_zero_weight_retriever_does_not_influence_results with [1.0, 0.0] and [0.0, 1.0] weights is a definitive regression test - it would have failed before this fix because the old code used 1.0 / (rank + k) for every retriever regardless of weight. The parametrize pattern keeps both cases together. test_rrf_higher_weight_retriever_scores_higher verifies the relative ordering.
LGTM.
|
I checked the PR diff, and this looks unrelated to the changes here. This PR only touches The failing CI errors are from integration dependency resolution issues and a separate Since this PR doesn’t touch those packages or dependency pins, I believe the failures are unrelated. |
Description
Fixed
_reciprocal_rerank_fusionto iterateresults.items()instead ofresults.values()and applyself._retriever_weights[retriever_idx]to each score, so user-specified weights are now honored in reciprocal rank mode the same way they already are in relative score fusion.Fixes #21444
New Package?
Did I fill in the
tool.llamahubsection in thepyproject.tomland provide a detailed README.md for my new integration or package?Version Bump?
Did I bump the version in the
pyproject.tomlfile of the package I am updating? (Except for thellama-index-corepackage)Type of Change
Please delete options that are not relevant.
How Has This Been Tested?
Your pull-request will likely not be merged unless it is covered by some form of impactful unit testing.
Suggested Checklist:
uv run make format; uv run make lintto appease the lint gods