Add FlashRank reranker to HybridRetriever to improve retrieval quality - #116
GovindhKishore wants to merge 1 commit into
Conversation
|
Hi @adamjohnwright @GFJHogue , Just flagging this PR for your attention when you get a chance. This directly addresses the retrieval noise issue mentioned across several issues, and since it touches Happy to:
Looking forward to any feedback! |
|
@heliamoh are you able to take a look to see if this resolves the issue(s)? |
|
Still open deliberately, and queued rather than ignored — sorry it has been quiet for six months. This PR changes what reaches the LLM, and this repository's constitution requires a before-and-after measurement on real questions for exactly that kind of change. "This should be better" is explicitly not a finding here, because retrieval quality has no right answer, only "did this move". The reason there has been no verdict is that the tool which produces that measurement was not trustworthy. #211 fixes that last part. Once it lands, this PR gets a real answer: the golden questions, before and after, with the noise floor reported, and the result is a number rather than an opinion. Two things that will need doing first, so they are not a surprise:
Nothing is needed from you right now. Thank you for the contribution, and for your patience with the delay. |
|
Reviewed against current It stops the app from starting
The config it adds is never readreranker:
enabled: true
top_n: 5
model: "ms-marco-MiniLM-L-12-v2"
cache_dir: "/tmp"
max_length: 512
The model loads at importranker = Ranker(model_name="ms-marco-MiniLM-L-12-v2") # module scopeThat downloads and loads an ONNX model when the module is first imported, which means it happens during startup, on the network. In the deployed container What is genuinely good# ranker.rerank() is a blocking CPU operation (neural network inference)
# calling it directly inside async would freeze the entire event loop
results = await asyncio.to_thread(ranker.rerank, request)That is exactly right, and it is the kind of thing that is usually got wrong. Blocking inference on the event loop would have stalled every concurrent request, and the comment shows it was reasoned about rather than copied. On whether reranking helpsUnanswered, and it should be answered with a number rather than an argument — this repository requires a before-and-after on real questions for anything that changes what reaches the LLM. The evaluator that produces that is now fixed and hardened (#211), so the measurement is available to whoever picks this up. Worth noting Closing because it cannot start the app as written, not because the direction is wrong. Sorry it took six months to get a real review, and thank you for the async care — that part would have been easy to get wrong. |
Summary
Adds a reranking layer to
HybridRetrieverincsv_chroma.pyto address the issue of responses becoming increasingly long and noisy as more data sources are integrated into the retrieval pipeline.Problem
The current pipeline retrieves documents from multiple subdirectories using BM25 + SelfQuery + MultiQuery expansion, resulting in ~90 documents being passed directly to
create_stuff_documents_chain.There is no cross-subdirectory relevance filtering - all retrieved documents are stuffed into the LLM prompt regardless of how relevant they are to the original user query. This causes:
Solution
A new module
src/retrievers/reranker.pyis introduced using FlashRank (ms-marco-MiniLM-L-12-v2). Afterweighted_reciprocal_rankmerges results across all subdirectories, the reranker scores every retrieved document against the original user query using a cross-encoder model and returns only the top N most relevant documents.Two functions are provided:
rerank()- sync, called byretrieve_documents()arerank()- async, called byaretrieve_documents()arerank()usesasyncio.to_threadto run the blocking FlashRank inference in a background thread without freezing the async event loop.Changes
src/retrievers/reranker.py- new module containing reranking logicsrc/retrievers/csv_chroma.py- import reranker, update return statements in bothretrieve_documents()andaretrieve_documents()config_default.yml- add reranker configuration blockpyproject.toml/poetry.lock- add flashrank dependencyWhy FlashRank
list[Document]typereturned throughout
Impact
Since
csv_chroma.pyis shared by both Reactome and UniProt retrievers, reranking applies automatically to all current and future database integrations without any additional changes.Test
Note
This contribution was developed with AI assistance (Claude) for understanding the codebase and implementation guidance. All code has been reviewed and understood.
Closes #115
Happy to make any changes based on maintainer feedback.