Make the baseline harness measure what the pipeline actually does - #180
Merged
Conversation
bin/retrieval_baseline built its vector retriever as as_retriever(k=10), raw. Since #169 and #177 the pipeline asks for k * VECTOR_OVERFETCH and collapses the result to k distinct entities by st_id. So the harness was comparing against a configuration the pipeline no longer uses. That matters because the spec merged in #179 names recapturing the #171 overlap figures as the first verification step before implementing D1. Run against the harness as it was, those numbers would have looked freshly measured while describing a retriever that no longer exists -- worse than not measuring, because a decision would have been taken on them. The constants and the de-duplication are now imported from csv_chroma rather than re-declared, so the two cannot drift apart again. BM25 is left alone: it returns distinct rows already, which is why only the vector side needs the collapse. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The spec named recapturing #171's overlap figures as the first verification step before implementing D1. Done, with both retrievers at the configuration the pipeline actually uses -- k * VECTOR_OVERFETCH collapsed to k distinct entities: collection set overlap rank agreement complexes 0.49 0.18 ewas 0.54 0.17 reactions 0.58 0.14 summations 0.57 0.24 overall 0.55 0.18 Set overlap moved 0.48 -> 0.55 once both sides were measured fairly; rank agreement was unchanged at ~0.18. Only 21 of 80 selfquery results were identical between the old and new captures, so the movement is attributable to the configuration change rather than to run-to-run variance. The conclusion is unmoved. SelfQuery and plain semantic search return about half the same documents in almost entirely different order, so D1 is a substantial change to what reaches the model. Overlap cannot say which set is better; that is the ragas work. Also: distinct/k is measured after de-duplication, so it is 1.00 by construction and reporting it as a finding was circular. Relabelled as what it actually is -- a guard that catches VECTOR_OVERFETCH being too low to fill k. 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.
The measurement tool had drifted from the code it measures.
The drift
bin/retrieval_baselinebuilt itsvectorretriever asas_retriever(k=10)— raw. Since #169 (de-duplicate per Reactome entity) and #177 (over-fetch 4x), the pipeline asks fork * VECTOR_OVERFETCHand collapses tokdistinct entities.So the harness compared against a configuration
HybridRetrieverno longer uses.Why it matters now
The spec merged in #179 names recapturing the #171 overlap figures as the first verification step before implementing D1 (replacing
SelfQueryRetrieverwith plain semantic search, Helia's decision).Run against the harness as it was, those numbers would have looked freshly measured while describing a retriever that no longer exists. That is worse than not measuring, because a decision would have been taken on them.
The fix
VECTOR_OVERFETCHanddedupe_by_entityare now imported fromcsv_chromarather than re-declared, so harness and pipeline cannot drift apart again.BM25 is deliberately left alone — it returns distinct rows already, which is why only the vector side needs the collapse.
The comparison D1 actually needs is now apples to apples:
k=40, collapsed to 10k=40, collapsed to 10Same budget, same de-duplication, one variable different.
🤖 Generated with Claude Code