From aee2f89befc88bd38b3e233e62a4caa8d0fbc4b2 Mon Sep 17 00:00:00 2001 From: MrSibe Date: Wed, 30 Sep 2026 17:23:35 +0800 Subject: [PATCH] fix(eval): refuse a context window wider than the retrieval depth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sweep contained cells the harness cannot fill. The harness retrieves `candidateK` passages and the context metrics look at the first `contextK` of them, so `candidateK=5, contextK=8` reports on five passages while claiming eight. The dashboard showed `contextK=5` and `contextK=8` at `candidateK=5` as **identical** — and because `evidencePrecisionAtK` divides by the passages actually retrieved, nothing exposed it. Found in the #192 review. A wrong number that looks like a measurement is worse than a failure, so the invariant is enforced rather than documented: - **The harness refuses it.** `contextK > candidateK` throws before indexing, naming both numbers: `contextK (8) cannot exceed candidateK (5): the harness retrieves candidateK passages, so a wider context window can never be filled.` - **The sweep skips those cells** and says so. The report lists the skipped combinations in a "Skipped cells" section instead of quietly omitting rows, because "we did not measure this" and "this measured the same as its neighbour" are different statements and the old output showed the second while meaning the first. `docs/eval/sweep-v1.6.*` is regenerated: 22 rows instead of 24, and the misleading `candidateK=5 / contextK=8` row is gone rather than silently equal to `5`. The production cells (`candidateK=20, contextK ∈ {3,5,8}`) are unaffected. ## Testing - `npm run typecheck` — clean - `npm test` — 502 pass, including 3 new harness-invariant tests that need no vector store, because the check runs before any database work: the refusal, the message naming both numbers, and `contextK == candidateK` being accepted - `node_modules/electron/dist/electron.exe . --eval-harness --eval-candidate-k=5 --eval-context-k=8` — refused with the message above - `npm run eval:sweep` — 22 rows, 2 skipped and listed Part of #192 (review follow-up). --- docs/eval/sweep-v1.6.json | 72 ++++++++++--------------------- docs/eval/sweep-v1.6.md | 63 ++++++++++++++++----------- eval/README.md | 7 +++ scripts/eval-sweep.mjs | 91 +++++++++++++++++++++++++++------------ src/main/eval/harness.ts | 14 ++++++ test/evalHarness.test.ts | 63 +++++++++++++++++++++++++++ 6 files changed, 207 insertions(+), 103 deletions(-) create mode 100644 test/evalHarness.test.ts diff --git a/docs/eval/sweep-v1.6.json b/docs/eval/sweep-v1.6.json index 7f7fce1..c6e7860 100644 --- a/docs/eval/sweep-v1.6.json +++ b/docs/eval/sweep-v1.6.json @@ -13,7 +13,7 @@ "noResultRate": 0, "meanContextChars": 2078.9333333333334, "chunkCount": 19, - "latencyP95Ms": 88.15 + "latencyP95Ms": 13.28 }, { "strategy": "dense", @@ -27,21 +27,7 @@ "noResultRate": 0, "meanContextChars": 3326.8333333333335, "chunkCount": 19, - "latencyP95Ms": 89.14 - }, - { - "strategy": "dense", - "candidateK": 5, - "contextK": 8, - "recallAt5": 1, - "ndcgAt10": 0.94375, - "mapAt10": 0.922222, - "contextPrecision": 0.213333, - "contextRecall": 1, - "noResultRate": 0, - "meanContextChars": 3326.8333333333335, - "chunkCount": 19, - "latencyP95Ms": 98.4 + "latencyP95Ms": 13.14 }, { "strategy": "dense", @@ -55,7 +41,7 @@ "noResultRate": 0, "meanContextChars": 2078.9333333333334, "chunkCount": 19, - "latencyP95Ms": 107.2 + "latencyP95Ms": 13.97 }, { "strategy": "dense", @@ -69,7 +55,7 @@ "noResultRate": 0, "meanContextChars": 3326.8333333333335, "chunkCount": 19, - "latencyP95Ms": 77.76 + "latencyP95Ms": 14.81 }, { "strategy": "dense", @@ -83,7 +69,7 @@ "noResultRate": 0, "meanContextChars": 5311.633333333333, "chunkCount": 19, - "latencyP95Ms": 91.92 + "latencyP95Ms": 15.14 }, { "strategy": "dense", @@ -97,7 +83,7 @@ "noResultRate": 0, "meanContextChars": 2078.9333333333334, "chunkCount": 19, - "latencyP95Ms": 94.88 + "latencyP95Ms": 16.19 }, { "strategy": "dense", @@ -111,7 +97,7 @@ "noResultRate": 0, "meanContextChars": 3326.8333333333335, "chunkCount": 19, - "latencyP95Ms": 85.49 + "latencyP95Ms": 18.66 }, { "strategy": "dense", @@ -125,7 +111,7 @@ "noResultRate": 0, "meanContextChars": 5311.633333333333, "chunkCount": 19, - "latencyP95Ms": 84.23 + "latencyP95Ms": 14.61 }, { "strategy": "dense", @@ -139,7 +125,7 @@ "noResultRate": 0, "meanContextChars": 2078.9333333333334, "chunkCount": 19, - "latencyP95Ms": 91.85 + "latencyP95Ms": 17.65 }, { "strategy": "dense", @@ -153,7 +139,7 @@ "noResultRate": 0, "meanContextChars": 3326.8333333333335, "chunkCount": 19, - "latencyP95Ms": 86.76 + "latencyP95Ms": 19.74 }, { "strategy": "dense", @@ -167,7 +153,7 @@ "noResultRate": 0, "meanContextChars": 5311.633333333333, "chunkCount": 19, - "latencyP95Ms": 89.21 + "latencyP95Ms": 17.64 }, { "strategy": "hybrid", @@ -181,7 +167,7 @@ "noResultRate": 0, "meanContextChars": 2044.1333333333334, "chunkCount": 19, - "latencyP95Ms": 81.29 + "latencyP95Ms": 13.37 }, { "strategy": "hybrid", @@ -195,21 +181,7 @@ "noResultRate": 0, "meanContextChars": 3361.6, "chunkCount": 19, - "latencyP95Ms": 79.52 - }, - { - "strategy": "hybrid", - "candidateK": 5, - "contextK": 8, - "recallAt5": 1, - "ndcgAt10": 0.956053, - "mapAt10": 0.938889, - "contextPrecision": 0.213333, - "contextRecall": 1, - "noResultRate": 0, - "meanContextChars": 3361.6, - "chunkCount": 19, - "latencyP95Ms": 85.98 + "latencyP95Ms": 14.7 }, { "strategy": "hybrid", @@ -223,7 +195,7 @@ "noResultRate": 0, "meanContextChars": 2062.733333333333, "chunkCount": 19, - "latencyP95Ms": 86.15 + "latencyP95Ms": 22.31 }, { "strategy": "hybrid", @@ -237,7 +209,7 @@ "noResultRate": 0, "meanContextChars": 3525.633333333333, "chunkCount": 19, - "latencyP95Ms": 92.74 + "latencyP95Ms": 18.66 }, { "strategy": "hybrid", @@ -251,7 +223,7 @@ "noResultRate": 0, "meanContextChars": 5509.8, "chunkCount": 19, - "latencyP95Ms": 89.97 + "latencyP95Ms": 15.84 }, { "strategy": "hybrid", @@ -265,7 +237,7 @@ "noResultRate": 0, "meanContextChars": 2056.9333333333334, "chunkCount": 19, - "latencyP95Ms": 59.56 + "latencyP95Ms": 20.78 }, { "strategy": "hybrid", @@ -279,7 +251,7 @@ "noResultRate": 0, "meanContextChars": 3445.866666666667, "chunkCount": 19, - "latencyP95Ms": 77.17 + "latencyP95Ms": 23.35 }, { "strategy": "hybrid", @@ -293,7 +265,7 @@ "noResultRate": 0, "meanContextChars": 5572.6, "chunkCount": 19, - "latencyP95Ms": 91.41 + "latencyP95Ms": 22.32 }, { "strategy": "hybrid", @@ -307,7 +279,7 @@ "noResultRate": 0, "meanContextChars": 2056.9333333333334, "chunkCount": 19, - "latencyP95Ms": 89.62 + "latencyP95Ms": 23.47 }, { "strategy": "hybrid", @@ -321,7 +293,7 @@ "noResultRate": 0, "meanContextChars": 3445.866666666667, "chunkCount": 19, - "latencyP95Ms": 66.45 + "latencyP95Ms": 21.71 }, { "strategy": "hybrid", @@ -335,7 +307,7 @@ "noResultRate": 0, "meanContextChars": 5572.6, "chunkCount": 19, - "latencyP95Ms": 76.65 + "latencyP95Ms": 21.68 } ] } diff --git a/docs/eval/sweep-v1.6.md b/docs/eval/sweep-v1.6.md index 2168fa4..6af90da 100644 --- a/docs/eval/sweep-v1.6.md +++ b/docs/eval/sweep-v1.6.md @@ -5,35 +5,48 @@ Generated by `node scripts/eval-sweep.mjs`. Numbers are harness output; do not e ## What was measured The real harness, the same corpus, chunking held fixed, over -dense / hybrid × candidateK {5, 10, 20, 40} × contextK {3, 5, 8} — 24 runs. +dense / hybrid × candidateK {5, 10, 20, 40} × contextK {3, 5, 8} — 22 runs. Each row differs from its neighbour in one parameter. +2 further cell(s) were **skipped** because `contextK > candidateK`; see below. + | Strategy | candidateK | contextK | Recall@5 | nDCG@10 | MAP@10 | Context P | Context R | No-result | Context chars | Index | p95 | | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | -| dense | 5 | 3 | 1.0000 | 0.9437 | 0.9222 | 0.3556 | 1.0000 | 0.0000 | 2079 | 19 | 88.15 ms | -| dense | 5 | 5 | 1.0000 | 0.9437 | 0.9222 | 0.2133 | 1.0000 | 0.0000 | 3327 | 19 | 89.14 ms | -| dense | 5 | 8 | 1.0000 | 0.9437 | 0.9222 | 0.2133 | 1.0000 | 0.0000 | 3327 | 19 | 98.40 ms | -| dense | 10 | 3 | 1.0000 | 0.9437 | 0.9222 | 0.3556 | 1.0000 | 0.0000 | 2079 | 19 | 107.20 ms | -| dense | 10 | 5 | 1.0000 | 0.9437 | 0.9222 | 0.2133 | 1.0000 | 0.0000 | 3327 | 19 | 77.76 ms | -| dense | 10 | 8 | 1.0000 | 0.9437 | 0.9222 | 0.1333 | 1.0000 | 0.0000 | 5312 | 19 | 91.92 ms | -| dense | 20 | 3 | 1.0000 | 0.9437 | 0.9222 | 0.3556 | 1.0000 | 0.0000 | 2079 | 19 | 94.88 ms | -| dense | 20 | 5 | 1.0000 | 0.9437 | 0.9222 | 0.2133 | 1.0000 | 0.0000 | 3327 | 19 | 85.49 ms | -| dense | 20 | 8 | 1.0000 | 0.9437 | 0.9222 | 0.1333 | 1.0000 | 0.0000 | 5312 | 19 | 84.23 ms | -| dense | 40 | 3 | 1.0000 | 0.9437 | 0.9222 | 0.3556 | 1.0000 | 0.0000 | 2079 | 19 | 91.85 ms | -| dense | 40 | 5 | 1.0000 | 0.9437 | 0.9222 | 0.2133 | 1.0000 | 0.0000 | 3327 | 19 | 86.76 ms | -| dense | 40 | 8 | 1.0000 | 0.9437 | 0.9222 | 0.1333 | 1.0000 | 0.0000 | 5312 | 19 | 89.21 ms | -| hybrid | 5 | 3 | 1.0000 | 0.9561 | 0.9389 | 0.3556 | 1.0000 | 0.0000 | 2044 | 19 | 81.29 ms | -| hybrid | 5 | 5 | 1.0000 | 0.9561 | 0.9389 | 0.2133 | 1.0000 | 0.0000 | 3362 | 19 | 79.52 ms | -| hybrid | 5 | 8 | 1.0000 | 0.9561 | 0.9389 | 0.2133 | 1.0000 | 0.0000 | 3362 | 19 | 85.98 ms | -| hybrid | 10 | 3 | 1.0000 | 0.9561 | 0.9389 | 0.3556 | 1.0000 | 0.0000 | 2063 | 19 | 86.15 ms | -| hybrid | 10 | 5 | 1.0000 | 0.9561 | 0.9389 | 0.2133 | 1.0000 | 0.0000 | 3526 | 19 | 92.74 ms | -| hybrid | 10 | 8 | 1.0000 | 0.9561 | 0.9389 | 0.1333 | 1.0000 | 0.0000 | 5510 | 19 | 89.97 ms | -| hybrid | 20 | 3 | 1.0000 | 0.9561 | 0.9389 | 0.3556 | 1.0000 | 0.0000 | 2057 | 19 | 59.56 ms | -| hybrid | 20 | 5 | 1.0000 | 0.9561 | 0.9389 | 0.2133 | 1.0000 | 0.0000 | 3446 | 19 | 77.17 ms | -| hybrid | 20 | 8 | 1.0000 | 0.9561 | 0.9389 | 0.1333 | 1.0000 | 0.0000 | 5573 | 19 | 91.41 ms | -| hybrid | 40 | 3 | 1.0000 | 0.9561 | 0.9389 | 0.3556 | 1.0000 | 0.0000 | 2057 | 19 | 89.62 ms | -| hybrid | 40 | 5 | 1.0000 | 0.9561 | 0.9389 | 0.2133 | 1.0000 | 0.0000 | 3446 | 19 | 66.45 ms | -| hybrid | 40 | 8 | 1.0000 | 0.9561 | 0.9389 | 0.1333 | 1.0000 | 0.0000 | 5573 | 19 | 76.65 ms | +| dense | 5 | 3 | 1.0000 | 0.9437 | 0.9222 | 0.3556 | 1.0000 | 0.0000 | 2079 | 19 | 13.28 ms | +| dense | 5 | 5 | 1.0000 | 0.9437 | 0.9222 | 0.2133 | 1.0000 | 0.0000 | 3327 | 19 | 13.14 ms | +| dense | 10 | 3 | 1.0000 | 0.9437 | 0.9222 | 0.3556 | 1.0000 | 0.0000 | 2079 | 19 | 13.97 ms | +| dense | 10 | 5 | 1.0000 | 0.9437 | 0.9222 | 0.2133 | 1.0000 | 0.0000 | 3327 | 19 | 14.81 ms | +| dense | 10 | 8 | 1.0000 | 0.9437 | 0.9222 | 0.1333 | 1.0000 | 0.0000 | 5312 | 19 | 15.14 ms | +| dense | 20 | 3 | 1.0000 | 0.9437 | 0.9222 | 0.3556 | 1.0000 | 0.0000 | 2079 | 19 | 16.19 ms | +| dense | 20 | 5 | 1.0000 | 0.9437 | 0.9222 | 0.2133 | 1.0000 | 0.0000 | 3327 | 19 | 18.66 ms | +| dense | 20 | 8 | 1.0000 | 0.9437 | 0.9222 | 0.1333 | 1.0000 | 0.0000 | 5312 | 19 | 14.61 ms | +| dense | 40 | 3 | 1.0000 | 0.9437 | 0.9222 | 0.3556 | 1.0000 | 0.0000 | 2079 | 19 | 17.65 ms | +| dense | 40 | 5 | 1.0000 | 0.9437 | 0.9222 | 0.2133 | 1.0000 | 0.0000 | 3327 | 19 | 19.74 ms | +| dense | 40 | 8 | 1.0000 | 0.9437 | 0.9222 | 0.1333 | 1.0000 | 0.0000 | 5312 | 19 | 17.64 ms | +| hybrid | 5 | 3 | 1.0000 | 0.9561 | 0.9389 | 0.3556 | 1.0000 | 0.0000 | 2044 | 19 | 13.37 ms | +| hybrid | 5 | 5 | 1.0000 | 0.9561 | 0.9389 | 0.2133 | 1.0000 | 0.0000 | 3362 | 19 | 14.70 ms | +| hybrid | 10 | 3 | 1.0000 | 0.9561 | 0.9389 | 0.3556 | 1.0000 | 0.0000 | 2063 | 19 | 22.31 ms | +| hybrid | 10 | 5 | 1.0000 | 0.9561 | 0.9389 | 0.2133 | 1.0000 | 0.0000 | 3526 | 19 | 18.66 ms | +| hybrid | 10 | 8 | 1.0000 | 0.9561 | 0.9389 | 0.1333 | 1.0000 | 0.0000 | 5510 | 19 | 15.84 ms | +| hybrid | 20 | 3 | 1.0000 | 0.9561 | 0.9389 | 0.3556 | 1.0000 | 0.0000 | 2057 | 19 | 20.78 ms | +| hybrid | 20 | 5 | 1.0000 | 0.9561 | 0.9389 | 0.2133 | 1.0000 | 0.0000 | 3446 | 19 | 23.35 ms | +| hybrid | 20 | 8 | 1.0000 | 0.9561 | 0.9389 | 0.1333 | 1.0000 | 0.0000 | 5573 | 19 | 22.32 ms | +| hybrid | 40 | 3 | 1.0000 | 0.9561 | 0.9389 | 0.3556 | 1.0000 | 0.0000 | 2057 | 19 | 23.47 ms | +| hybrid | 40 | 5 | 1.0000 | 0.9561 | 0.9389 | 0.2133 | 1.0000 | 0.0000 | 3446 | 19 | 21.71 ms | +| hybrid | 40 | 8 | 1.0000 | 0.9561 | 0.9389 | 0.1333 | 1.0000 | 0.0000 | 5573 | 19 | 21.68 ms | + + +## Skipped cells + +`contextK > candidateK` cannot be filled: the harness fetches `candidateK` +passages, so a wider window would contain fewer passages than it claims. These +2 cell(s) are excluded rather than reported as equal to a narrower one: + +- `dense` candidateK=5, contextK=8 +- `hybrid` candidateK=5, contextK=8 + +The harness refuses the same combination at the flag level, so a typo fails loudly. + ## How to read it diff --git a/eval/README.md b/eval/README.md index f715942..290648f 100644 --- a/eval/README.md +++ b/eval/README.md @@ -45,6 +45,13 @@ writes one dashboard with quality, context precision/recall, prompt size, index and latency side by side. Its grid maximum is labelled as **not** a recommendation: selecting on the same questions is how a benchmark becomes a lookup table. +`contextK > candidateK` is not a cell in that grid. The harness fetches `candidateK` +passages, so a wider window can never be filled; the sweep skips those combinations +and names them in the report, and the harness refuses the same combination from the +command line. Before this was enforced, `contextK=8` at `candidateK=5` was reported as +identical to `contextK=5` — not because 8 assessed the same as 5, but because +passages 6–8 did not exist. + `eval:prepare` downloads the pinned `multilingual-e5-small` revision into the app's model cache and verifies it. `eval` never touches the network: if the model is missing it stops with diff --git a/scripts/eval-sweep.mjs b/scripts/eval-sweep.mjs index 0ed7dc5..46faefd 100644 --- a/scripts/eval-sweep.mjs +++ b/scripts/eval-sweep.mjs @@ -105,37 +105,58 @@ function readTiming(mdPath) { const mean = (values) => (values.length === 0 ? 0 : values.reduce((a, b) => a + b, 0) / values.length) +/** + * `contextK > candidateK` is not a cell, it is an arithmetic mistake: the harness only + * fetches `candidateK` passages, so the window can never be filled. Skipping them is why + * the default grid no longer contains rows that looked like "8 is as good as 5" when + * passages 6-8 were simply never retrieved (#192 review). The harness refuses the same + * combination, so a typo on the command line fails loudly instead of silently. + */ +const grid = [] +const skipped = [] +for (const strategy of STRATEGIES) { + for (const candidateK of CANDIDATE_KS) { + for (const contextK of CONTEXT_KS) { + if (contextK > candidateK) skipped.push({ strategy, candidateK, contextK }) + else grid.push({ strategy, candidateK, contextK }) + } + } +} + +if (skipped.length > 0) { + console.log( + `[sweep] skipped ${skipped.length} cell(s) with contextK > candidateK: ` + + skipped.map((c) => `${c.strategy} ${c.candidateK}/${c.contextK}`).join(', ') + ) +} + const workDir = mkdtempSync(join(tmpdir(), 'knownote-sweep-')) const rows = [] try { - for (const strategy of STRATEGIES) { - for (const candidateK of CANDIDATE_KS) { - for (const contextK of CONTEXT_KS) { - const label = `${strategy} candidateK=${candidateK} contextK=${contextK}` - console.log(`[sweep] ${label}`) - const outDir = join(workDir, `${strategy}-${candidateK}-${contextK}`) - mkdirSync(outDir, { recursive: true }) - const report = await runOne(strategy, candidateK, contextK, outDir) - const perQuestion = report.perQuestion ?? [] - const noResult = perQuestion.filter((q) => q.retrievedCount === 0).length - - rows.push({ - strategy, - candidateK, - contextK, - recallAt5: report.metrics.recallAt5, - ndcgAt10: report.metrics.ndcgAt10, - mapAt10: report.metrics.mapAt10, - contextPrecision: report.metrics.contextPrecision, - contextRecall: report.metrics.contextRecall, - noResultRate: perQuestion.length === 0 ? 0 : noResult / perQuestion.length, - meanContextChars: mean(perQuestion.map((q) => q.contextChars)), - chunkCount: report.config.chunkCount, - ...readTiming(join(outDir, 'baseline-v1.6.md')) - }) - } - } + for (const cell of grid) { + const { strategy, candidateK, contextK } = cell + console.log(`[sweep] ${strategy} candidateK=${candidateK} contextK=${contextK}`) + const outDir = join(workDir, `${strategy}-${candidateK}-${contextK}`) + mkdirSync(outDir, { recursive: true }) + const report = await runOne(strategy, candidateK, contextK, outDir) + const perQuestion = report.perQuestion ?? [] + const noResult = perQuestion.filter((q) => q.retrievedCount === 0).length + + rows.push({ + strategy, + candidateK, + contextK, + recallAt5: report.metrics.recallAt5, + ndcgAt10: report.metrics.ndcgAt10, + mapAt10: report.metrics.mapAt10, + contextPrecision: report.metrics.contextPrecision, + contextRecall: report.metrics.contextRecall, + noResultRate: perQuestion.length === 0 ? 0 : noResult / perQuestion.length, + meanContextChars: mean(perQuestion.map((q) => q.contextChars)), + chunkCount: report.config.chunkCount, + ...readTiming(join(outDir, 'baseline-v1.6.md')) + }) } } finally { rmSync(workDir, { recursive: true, force: true }) @@ -156,6 +177,19 @@ const tableRows = rows const best = (key, filter = () => true) => rows.filter(filter).reduce((a, b) => (a === null || b[key] > a[key] ? b : a), null) +/** + * A skipped cell is reported, not silently dropped: "we did not measure this" and "this + * measured the same as its neighbour" are different statements, and the earlier version + * of this file showed the second when it meant the first. + */ +const skippedNote = + skipped.length === 0 + ? '' + : `\n\n## Skipped cells\n\n\`contextK > candidateK\` cannot be filled: the harness fetches \`candidateK\`\npassages, so a wider window would contain fewer passages than it claims. These +${skipped.length} cell(s) are excluded rather than reported as equal to a narrower one:\n\n${skipped + .map((c) => `- \`${c.strategy}\` candidateK=${c.candidateK}, contextK=${c.contextK}`) + .join('\n')}\n\nThe harness refuses the same combination at the flag level, so a typo fails loudly.\n` + const bestNdcg = best('ndcgAt10') const bestContextPrecision = best('contextPrecision') @@ -167,11 +201,12 @@ Generated by \`node scripts/eval-sweep.mjs\`. Numbers are harness output; do not The real harness, the same corpus, chunking held fixed, over ${STRATEGIES.join(' / ')} × candidateK {${CANDIDATE_KS.join(', ')}} × contextK {${CONTEXT_KS.join(', ')}} — ${rows.length} runs. -Each row differs from its neighbour in one parameter. +Each row differs from its neighbour in one parameter.${skipped.length > 0 ? `\n\n${skipped.length} further cell(s) were **skipped** because \`contextK > candidateK\`; see below.` : ''} | Strategy | candidateK | contextK | Recall@5 | nDCG@10 | MAP@10 | Context P | Context R | No-result | Context chars | Index | p95 | | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | ${tableRows} +${skippedNote} ## How to read it diff --git a/src/main/eval/harness.ts b/src/main/eval/harness.ts index 727f775..418ddf7 100644 --- a/src/main/eval/harness.ts +++ b/src/main/eval/harness.ts @@ -217,6 +217,20 @@ export async function runEvalHarness( knowledgeService: KnowledgeService, options: EvalHarnessOptions ): Promise { + // A context window wider than the retrieval depth can never be filled: the harness + // fetches `candidateK` passages and the context metrics look at `contextK` of them. + // + // Without this check the sweep silently produced rows where `contextK=5` and + // `contextK=8` at `candidateK=5` were **identical**, not because 8 assessed the same + // as 5 but because passages 6-8 did not exist (#192 review). A wrong number that + // looks like a measurement is worse than a failure. + if (options.contextK > options.candidateK) { + throw new Error( + `contextK (${options.contextK}) cannot exceed candidateK (${options.candidateK}): ` + + 'the harness retrieves candidateK passages, so a wider context window can never be filled.' + ) + } + const { documentIds, chunkCount, indexingMs } = await indexCorpus( db, knowledgeService, diff --git a/test/evalHarness.test.ts b/test/evalHarness.test.ts new file mode 100644 index 0000000..a5e650b --- /dev/null +++ b/test/evalHarness.test.ts @@ -0,0 +1,63 @@ +import { test } from 'node:test' +import assert from 'node:assert/strict' +import { runEvalHarness } from '../src/main/eval/harness.ts' + +/** + * The harness's own invariants (#192). These run before any database or retrieval work, + * so they can be pinned without a store: the check is the first statement of + * `runEvalHarness`, and a violation must fail loudly rather than produce a number. + */ + +const options = (overrides: Record = {}): Record => ({ + corpusDir: 'eval/corpus', + corpusLabel: 'eval/corpus', + questionsPath: 'eval/questions.jsonl', + baseline: 'test', + split: 'all', + candidateK: 20, + contextK: 3, + threshold: 0.5, + chunkOptions: { + chunkSize: 1000, + chunkOverlap: 100, + minChunkSize: 100, + allowSpanPages: false, + respectHeadings: false + }, + strategy: 'dense', + ...overrides +}) + +/** + * A context window wider than the retrieval depth can never be filled. The sweep used + * to contain `candidateK=5, contextK=8` and report it as identical to `contextK=5` — + * not because 8 assessed the same as 5, but because passages 6-8 did not exist (#192 + * review). A wrong number that looks like a measurement is worse than a failure. + */ +test('a context window wider than the retrieval depth is refused', async () => { + await assert.rejects( + () => runEvalHarness({} as never, {} as never, options({ candidateK: 5, contextK: 8 }) as never), + /contextK \(8\) cannot exceed candidateK \(5\)/ + ) +}) + +test('the refusal names both numbers, so the fix is obvious', async () => { + await assert.rejects( + () => runEvalHarness({} as never, {} as never, options({ candidateK: 3, contextK: 10 }) as never), + (error: Error) => { + assert.match(error.message, /candidateK \(3\)/) + assert.match(error.message, /contextK \(10\)/) + assert.match(error.message, /can never be filled/) + return true + } + ) +}) + +test('contextK equal to candidateK is allowed and reaches further invariants', async () => { + // No database is provided, so the call must fail *after* the window check — proving the + // boundary value is accepted rather than rejected. + await assert.rejects( + () => runEvalHarness({} as never, {} as never, options({ candidateK: 5, contextK: 5 }) as never), + (error: Error) => !/cannot exceed/.test(error.message) + ) +})