Repository navigation
Stage 1: HybridRetriever becomes a plain BaseRetriever - #182
Merged
Merged
Conversation
Structural only. All five reaches into LangChain internals are gone: subclassing MultiQueryRetriever -> BaseRetriever overriding a required field via SkipJsonSchema -> field removed stealing .llm_chain from a throwaway instance -> the chain is built directly assigning _retrievers outside pydantic -> a declared field EnsembleRetriever(retrievers=[]) for one method-> RRF vendored here Query expansion, unique_union and the LineListOutputParser are reproduced rather than inherited. The original query is still appended AFTER the generated ones, which matters: RRF resolves ties by first appearance, so reordering there would change the ranking silently. The plan's exit criterion -- zero difference from an end-to-end capture -- turned out to be unachievable, and that is worth recording. SelfQueryRetriever makes an LLM call, so the SAME code run twice already differs on 2 of 8 questions. The old-vs-new difference was also 2 of 8, i.e. entirely within that noise floor, so the comparison could not have shown anything either way. Replaced with a decisive test instead: the vendored RRF is compared directly against EnsembleRetriever.weighted_reciprocal_rank over 2000 random inputs -- varying list counts, overlaps, orderings and empty lists -- and agrees on all of them, including the constant. Perturbing RRF_K to 61 makes that test fail, so it is not vacuous. Also verified through the full chain: sync and async return identical documents in identical order for a fixed query set, and invoke() still yields 40 documents. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
First stage of the plan. Structural only — expansion and SelfQuery both stay; nothing about what reaches the model changes deliberately.
All five reaches into LangChain internals are gone
MultiQueryRetrieverBaseRetrieverNoneviaSkipJsonSchemaMultiQueryRetrieverto steal.llm_chain_retrieversoutside pydanticEnsembleRetriever(retrievers=[])to borrow one methodExpansion,
unique_unionand the line parser are reproduced rather than inherited. The original query is still appended after the generated ones — RRF resolves ties by first appearance, so reordering there would change ranking silently.The exit criterion in the plan was wrong, and that is worth recording
The plan said Stage 1 must show zero difference in an end-to-end capture. That is unachievable:
SelfQueryRetrievermakes an LLM call, so the noise floor is 2 of 8. The old-vs-new difference sits entirely inside it — the comparison could not have demonstrated anything either way, in either direction. I would have "passed" this criterion by luck or failed it by luck.Replaced with a decisive test
The vendored RRF is compared directly against
EnsembleRetriever.weighted_reciprocal_rankover 2000 random inputs — varying list counts, overlaps, orderings, empty lists — plus the constant itself:Committed as
tests/retrievers/test_rrf_equivalence.py. PerturbingRRF_Kfrom 60 to 61 makes it fail, so it is not vacuous.Also verified
invokeandainvoke: identical documents in identical order for a fixed query setNot in this PR
Stage 2 (D1 — plain semantic search replaces SelfQuery) and Stage 3 (caller-supplied budget). Deliberately separate so a behaviour change is never mixed with a structural one.
🤖 Generated with Claude Code