feat(eval): adopt on the first metric with headroom, not a saturated one - #197
Merged
Merged
Conversation
The v1.5 rule was "Recall@5 must improve and nDCG@10 must not regress". On a corpus where dense already scores Recall@5 = 1.0000, no strategy can improve Recall@5, so the rule was not strict — it was **unsatisfiable**. Every comparison came back "inconclusive", including a hybrid that was better on Recall@1, MRR and nDCG@10. The default never moved, not because hybrid lost but because the rule could not return a verdict. Child 10 of #192. **The amendment.** A metric at its maximum has no headroom and is not allowed to decide. The deciding metric is the first one with headroom, in the order `recallAt5`, `ndcgAt10`, `mrr`, `mapAt10`; a strategy clears the rule when it improves that metric and regresses none of the others. Saturation is detected and reported rather than silently blocking every change. **The rule is code, not prose.** It lives in `src/main/eval/adoption.ts` with 8 unit tests, because it decides whether a shipped default moves and testing it by reading the sentence the script prints would only test the sentence. The experiment script imports it, so there is one definition. ## The v1.5 stalemate resolves ``` | Strategy | Recall@1 | Recall@5 | MRR | nDCG@10 | MAP@10 | Query p95 | | dense (vector) | 0.8333 | 1.0000 | 0.9278 | 0.9437 | 0.9222 | 14.06 ms | | sparse (BM25) | 0.7333 | 0.8667 | 0.8056 | 0.8184 | 0.8000 | 2.63 ms | | hybrid (RRF dense + BM25) | 0.8667 | 1.0000 | 0.9444 | 0.9561 | 0.9389 | 22.32 ms | ``` `recallAt5` is reported as saturated; the deciding metric is `ndcgAt10`; **hybrid clears the rule** and regresses none of the four metrics, at a p95 cost of ~8 ms. The script reports the measurement and does not flip the default — changing the shipped strategy is a product decision, and it is stated that way in the output rather than implied by a green checkmark. `docs/eval/retrieval-v1.6.{json,md}` is the regenerated experiment against the v1.6 baseline; `retrieval-v1.5.*` is kept as the record of the superseded rule. ## Testing - `npm run typecheck` — clean - `npm test` — 497 pass, 8 new: saturation detection, deciding-metric priority, the float-slack regression check, the resolved stalemate, a trade that regresses another metric being refused, the baseline not clearing against itself, all-saturated, and best-candidate selection - `npm run eval` + `npm run eval:retrieval` — regenerated and re-run Part of #192 (child 10).
This was referenced Sep 30, 2026
mrsibe
added a commit
that referenced
this pull request
Sep 30, 2026
The epic asks for a grid over `candidateK` and `contextK` with latency, index size and context size recorded next to quality, and explicitly not for a single aggregate "RAG score". Child 7 of #192. `npm run eval:sweep` runs the real harness over `strategy × candidateK {5,10,20,40} × contextK {3,5,8}` (24 runs, chunking fixed) and writes `docs/eval/sweep-v1.6.{json,md}`. The harness gained `contextChars` per question — the size of the context window, reported as **characters, not tokens**, because the harness pins an embedding model and no generation tokenizer. ## What the dashboard already shows Two of the three axes are decided by this corpus, and one of them decisively: - **`candidateK` changes nothing.** Every metric is identical from 5 to 40, because the corpus is 19 chunks and the relevant passages are already inside the top 5. This is the saturation problem #192 describes, now visible on the axis it affects. - **`contextK` has a clear optimum here: 3.** Context recall is 1.0000 at every width, while precision falls `0.3556 → 0.2133 → 0.1333` and the window grows `2079 → 3327 → 5312` characters as it widens. Wider adds prompt cost and noise for **no** recall. The production default is already 3, and this is the first evidence that it is the right 3 rather than a guess. - **hybrid beats dense** on nDCG@10 (0.9561 vs 0.9437) and MAP@10 (0.9389 vs 0.9222) at every setting, with no metric regressing — consistent with #197. The report labels the grid maximum as **not** a recommendation, because selecting on the same questions is how a benchmark becomes a lookup table; the adoption rule and the `validation`/`test` split are what keep the decision honest. Timing p95 is reported per row but is noisy at this sample size; it is in the table so a latency cost can be seen, not so it can be ranked. ## Testing - `npm run typecheck` — clean - `npm test` — 497 pass - `npm run eval` — regenerated (`contextChars` added to `perQuestion`) - `npm run eval:sweep` — 24 runs, dashboard written Part of #192 (child 7).
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.
Stacked on #196 (
feat/eval-threshold-sweep). Retarget tomainafter that merges.What does this PR do?
Replaces the unsatisfiable adoption rule with one that detects saturation and decides on a metric that can actually move. Child 10 of #192.
The defect
The v1.5 rule: "Recall@5 must improve and nDCG@10 must not regress." On a corpus where dense already scores Recall@5 = 1.0000, no strategy can improve Recall@5. So the rule was not strict — it was unsatisfiable. Every comparison came back "inconclusive", including a hybrid that was better on Recall@1, MRR and nDCG@10. The default never moved, not because hybrid lost but because the rule could not return a verdict.
The amendment
A metric at its maximum has no headroom and is not allowed to decide. The deciding metric is the first one with headroom, in priority order
recallAt5,nDCG@10,MRR,MAP@10; a strategy clears the rule when it improves that metric and regresses none of the others. Saturation is detected and reported, rather than silently blocking every change.The rule is code, not prose. It lives in
src/main/eval/adoption.tswith 8 unit tests. It decides whether a shipped default moves, and testing it by reading the sentence the experiment script prints would only test the sentence. The experiment script imports it, so there is one definition.The v1.5 stalemate resolves
recallAt5is reported as saturated; the deciding metric isndcgAt10; hybrid clears the rule and regresses none of the four metrics, at a p95 cost of ~8 ms.The script reports the measurement and does not flip the default — changing the shipped strategy is a product decision, and the output says so rather than implying it with a green checkmark. If you want hybrid to become the shipped strategy, that is a one-line follow-up worth making deliberately.
docs/eval/retrieval-v1.6.{json,md}is the regenerated experiment against the v1.6 baseline;retrieval-v1.5.*is kept as the record of the superseded rule.Testing
npm run typecheck— cleannpm test— 497 pass, 8 new: saturation detection, deciding-metric priority, the float-slack regression check, the resolved stalemate, a trade that regresses another metric being refused, the baseline not clearing against itself, the all-saturated case, and best-candidate selectionnpm run eval+npm run eval:retrieval— regenerated and re-runRelated
Part of #192. Child 10 (amend the adoption rule).