feat(eval): separate candidateK from contextK, and run the production config - #194
Merged
Merged
Conversation
Two gates the v1.5 eval path needs to be trustworthy on every platform. **`npm run eval` could not start on Windows.** The three scripts resolve `node_modules/.bin/electron.cmd` and spawn it. Node 24 refuses to spawn a `.cmd`/`.bat` without `shell: true` and fails with `EINVAL`, so the harness was unrunnable on the platform this project is developed on. The `electron` package exports the path to the real executable the wrapper runs, which spawns directly on every platform and needs no shell. **A Windows run rewrote the committed baseline.** `path.relative` returns backslashes on Windows, so `config.corpus` was written as `eval\corpus` instead of `eval/corpus`. The file's whole contract is that it is identical on every machine — the CI determinism check diffs it — and a Windows run silently broke that. The label is now normalised to POSIX separators. Verified on Windows with the pinned model: `npm run eval` runs and leaves `docs/eval/baseline-v1.5.json` byte-identical to the committed file.
… config The harness measured a retriever nobody runs. Production retrieved at `topK: 3` with `threshold: 0.5`; the harness ran `topK: 10` with `threshold: 0` and reported `Recall@5 = 1.0000` for a pipeline that silently drops rank 4 at cosine 0.47. The first child of #192 asks for the two to be the same configuration. **One K was doing two jobs.** `RetrievalRequest.topK` was both the first-stage width (KNN neighbours, BM25 limit) and the number of passages delivered. In hybrid that made fusion nearly a no-op: dense contributed `topK`, BM25 contributed `topK`, RRF fused at most `2 * topK`, and the result was sliced straight back to `topK`. The two stages are now named: ``` RetrievalRequest: candidateK first-stage width per channel (default 20) topK passages delivered (default 5) ``` `effectiveCandidateK` enforces `candidateK >= topK`, so a caller that asks for more results than the default pool (the search palette, MCP) is never silently capped. `HybridRetriever` fuses the wide pool and truncates once, after fusion. `DenseRetriever` queries `candidateK` and slices to `topK` — equivalent to before for a single strategy, since a prefix of a ranking is the same ranking. The trace and the #157 snapshot gained `candidateK`. A snapshot written before this change backfills it from `topK`, which is what that retrieval actually did. **The harness now defaults to production** (`candidateK: 20`, `contextK: 3`, `threshold: 0.5`), with `--eval-candidate-k=` / `--eval-context-k=` / `--eval-threshold=` to move them deliberately. Ranking metrics are computed at `candidateK` depth, not `contextK`: `Recall@10` needs ten results, and truncation only takes a prefix, so it cannot change the ranking being measured. `contextK` is recorded so the report describes the whole online path. `docs/eval/baseline-v1.6.{json,md}` is the new frozen baseline; v1.5 is kept as history for the #77/#78 deltas, and the CI determinism check moves to v1.6. ## What this did and did not change On the current 19-chunk corpus the production configuration produces the **same** metrics as v1.5 — `threshold: 0.5` is non-binding and `candidateK: 20` exceeds the corpus, so nothing is filtered and no ranking changes. That is the corpus limitation #192 describes, not a result. What changed is that the harness now *states* the production parameters instead of assuming different ones. Hybrid still leads dense on Recall@1 (0.8667 vs 0.8333), MRR (0.9444 vs 0.9278) and nDCG@10 (0.9561 vs 0.9437) with a wide pool; dense stays the default because the adoption rule still cannot move on a saturated Recall@5. ## Testing - `npm run typecheck` — clean - `npm test` — 481 pass (3 new: trace keeps the two Ks apart, the width invariant, the pre-#77 snapshot backfill) - `npm run eval` twice — byte-identical `docs/eval/baseline-v1.6.json` - `npm run eval:retrieval` — dense/sparse/hybrid unchanged in ordering Part of #192 (child 1).
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 #193 (
fix/eval-windows-gates) — both toucheval/run.tsand the harness. Retarget tomainafter that merges.What does this PR do?
Makes the harness measure the pipeline the app actually runs, and separates the two Ks that one parameter was doing the work of. Child 1 of #192.
The problem
Production retrieved at
topK: 3, threshold: 0.5; the harness rantopK: 10, threshold: 0. A chunk at rank 4 with cosine 0.47 was a hit in the benchmark and did not exist in the product —Recall@5 = 1.0000could be reported while the shipped path silently dropped the evidence.And the one
topKwas two different parameters.HybridRetrieverdid:With
topK = 3the fusion pool was at most 6 and was sliced straight back to 3. RRF had almost no room to reorder anything — not a two-stage retrieval, a narrow single-stage one with fusion applied inside it.The change
effectiveCandidateKenforcescandidateK >= topK. Without it, any caller with a largetopK(the search palette, MCP) would be silently capped at the default pool.HybridRetrieverfuses the wide pool and truncates once, after fusion.DenseRetrieverqueriescandidateKand slices totopK— equivalent to before for a single strategy, because a prefix of a ranking is that ranking.RetrievalTraceand the [Feat] Retrieval explainability: snapshot what this answer retrieved and cited #157 snapshot carrycandidateK. A snapshot written before this change backfills it fromtopK, which is what that retrieval really did — that is history, not a default.candidateK: 20,contextK: 3,threshold: 0.5, with--eval-candidate-k=/--eval-context-k=/--eval-threshold=to move them deliberately.Ranking metrics run at
candidateKdepth, notcontextK.Recall@10needs ten results and production delivers three; truncation only takes a prefix, so measuring the ranking at full depth does not mix the two concerns together.contextKis recorded so the report describes the whole online path.docs/eval/baseline-v1.6.{json,md}is the new frozen baseline. v1.5 is kept as history for the #77/#78 deltas; the CI determinism check moves to v1.6.What this did and did not change
On the current 19-chunk corpus the production configuration produces the same metrics as v1.5.
threshold: 0.5is non-binding andcandidateK: 20exceeds the corpus, so nothing is filtered and no ranking moves. That is the corpus limitation #192 describes, not a result. What changed is that the harness now states the production parameters instead of assuming different ones.Hybrid still leads dense on Recall@1 (0.8667 vs 0.8333), MRR (0.9444 vs 0.9278) and nDCG@10 (0.9561 vs 0.9437) with the wide pool; dense stays the default because the adoption rule still cannot move on a saturated Recall@5.
Testing
npm run typecheck— cleannpm test— 481 pass, 3 new: the trace keeps the two Ks apart, thecandidateK >= topKinvariant, the pre-[Spike] Retrieval experiments: BM25 / hybrid / RRF / reranker on a frozen chunk baseline #77 snapshot backfillnpm run evaltwice — byte-identicaldocs/eval/baseline-v1.6.jsonnpm run eval:retrieval— dense / sparse / hybrid ordering unchangedRelated
Part of #192. Child 1 (eval config parity).