fix(eval): select the strategy on validation, and stop calling candidates passages - #204
Merged
mrsibe merged 1 commit intoSep 30, 2026
Conversation
…ates "passages" Four follow-ups from the #192 review. The first two are semantics that would have spread into corpus v2 if they were left alone. **Strategy adoption is now held out.** `eval-retrieval.mjs` ran with `split = all`, so the strategy was chosen and scored on the same 44 questions — the exact mistake the threshold experiment had already been fixed for. It now: 1. runs every strategy on **validation** and decides there with `decideAdoption`; 2. re-runs the shipped strategy and the selected one on **test**, and only reports them. The choice never sees `test`. If validation selects nothing, `test` reports the shipped strategy alone and the report says there is no adoption candidate. The consequence is a more conservative and more trustworthy result than before: on validation, hybrid is **identical** to dense (Recall@5 0.9231, nDCG@10 0.8276 on both), so nothing is adopted — whereas the `split = all` run had hybrid ahead on nDCG@10. The old number was the choice being scored on its own questions. **`meanRetrieved` was a misleading name.** The harness fetches `candidateK` in order to compute `Recall@10`; the context window is `results.slice(0, contextK)`. So `meanRetrieved = 19` never meant "19 passages go to the model" — it meant "19 candidates passed the threshold". The unanswerable group now reports both sizes: ``` meanCandidatesRetrieved 19 // passed the threshold, capped by candidateK meanContextPassages 3 // actually reach the window, min(candidates, contextK) ``` The accurate description of the FIFA case is therefore: **19/19 chunks pass `threshold: 0.5`, and the top 3 irrelevant ones go into the prompt.** Still a real problem, but not "19 passages are stuffed into the model". **And it is retrieval abstention, not refusal.** No generator runs in this harness, so it can show that nothing passed the threshold; it cannot show that the model would decline to answer. Fields renamed (`abstentionCount`, `retrievalAbstentionRate`) and the report says plainly that a true system refusal rate needs a generator eval. **The manifest rationale was wrong.** The docs claimed a hash of the id "moves other questions between the sides" when one is added. That is false: `hash(id) % 3` is computed per id, so it is stable. The real reasons for an explicit manifest are that a hash cannot stratify a small corpus (which is how `multi-hop` and `cross-lingual` ended up entirely on one side) and that a new question would be assigned silently rather than deliberately. Corrected in `types.ts`, `eval/README.md` and the split tests. Regenerated: baseline, retrieval, threshold and sweep reports. The threshold table now shows `Unans. cands` and `Unans. ctx` as separate columns instead of conflating them. ## Testing - `npm run typecheck` — clean - `npm test` — 504 pass - `npm run eval` twice — byte-identical baseline - `npm run eval:retrieval` — validation selects, test reports; no adoption candidate - `npm run eval:threshold`, `npm run eval:sweep` — regenerated Part of #192 (review follow-ups).
mrsibe
changed the base branch from
feat/eval-cross-lingual-corpus
to
fix/retrieval-hybrid-trace-threshold
September 30, 2026 10:06
mrsibe
force-pushed
the
fix/retrieval-hybrid-trace-threshold
branch
from
September 30, 2026 10:06
edfc1ad to
b1bdb0b
Compare
mrsibe
force-pushed
the
fix/eval-review-followups
branch
from
September 30, 2026 10:07
22f070c to
84e9e0a
Compare
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
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.
Stacked on #203 (
feat/eval-cross-lingual-corpus). Retarget tomainafter the chain merges.What does this PR do?
Four follow-ups from the review. The first two are semantics that would have spread into corpus v2 if they were left alone.
1. Strategy adoption is now held out
eval-retrieval.mjsran withsplit = all, so the strategy was chosen and scored on the same 44 questions — exactly the mistake the threshold experiment had already been fixed for. It now:decideAdoption;The choice never sees
test. If validation selects nothing,testreports the shipped strategy alone and the report says there is no adoption candidate.The consequence is a more conservative and more trustworthy result than before. On validation, hybrid is identical to dense:
So nothing is adopted — whereas the
split = allrun had hybrid ahead on nDCG@10 (0.8574 vs 0.8476). That difference was the choice being scored on its own questions.2.
meanRetrievedwas a misleading nameThe harness fetches
candidateKin order to computeRecall@10; the context window isresults.slice(0, contextK). SomeanRetrieved = 19never meant "19 passages go to the model" — it meant "19 candidates passed the threshold". The unanswerable group now reports both sizes:The accurate description of the FIFA case is therefore: 19/19 chunks pass
threshold: 0.5, and the top 3 irrelevant ones go into the prompt. Still a real problem, but not "19 passages are stuffed into the model". The sweep and threshold tables now carrycandsandctxas separate columns instead of conflating them.3. It is retrieval abstention, not refusal
No generator runs in this harness, so it can show that nothing passed the threshold; it cannot show that the model would decline to answer. Fields renamed (
abstentionCount,retrievalAbstentionRate) and the report states plainly that a true system refusal rate needs a generator eval.4. The manifest rationale was wrong
The docs claimed a hash of the id "moves other questions between the sides" when a new question is added. That is false for
hash(id) % 3— it is computed per id, so it is stable. The real reasons for an explicit manifest are:multi-hopandcross-lingualended up entirely on one side; andtestis the side a choice must not be fitted to.Corrected in
src/main/eval/types.ts,eval/README.mdandtest/evalSplit.test.ts.Also
#200 was a sibling, not part of the chain. Merging
#193 → … → #199 → #201 → #202 → #203would have silently skipped it. I retargeted #200's base tofeat/eval-cross-lingual-corpusso the final stack is linear and it cannot be missed.Testing
npm run typecheck— cleannpm test— 504 passnpm run evaltwice — byte-identical baselinenpm run eval:retrieval— validation selects, test reports; no adoption candidatenpm run eval:threshold,npm run eval:sweep— regeneratedRelated
Part of #192 (review follow-ups). Next: the hard-negative corpus expansion.