diff --git a/docs/eval-corpus/README.md b/docs/eval-corpus/README.md index 8f9d675..59e0632 100644 --- a/docs/eval-corpus/README.md +++ b/docs/eval-corpus/README.md @@ -7,9 +7,11 @@ review/coordinator agent **misfires** on a real task, the case is distilled into a machine-readable record and added here, so the failure can never silently regress. -This tier is the **staging ground** for the offline gold-scored output eval -(SKAL-1·4b). It is deliberately **not wired into CI** yet — these are committed -fixtures + a schema, not a scoring run. +This corpus is the gold that the **offline gold-scored output eval (SKAL-1·4b)** +scores against — that eval is now implemented and wired into `node --test` +(see [§Gold-scored output eval](#gold-scored-output-eval-skal-14b) below). The +corpus records here are committed fixtures + a schema; the scoring run is the +separate piece that consumes them. ## Seed example @@ -65,10 +67,38 @@ A corpus file is one JSON object: 3. Add (or extend) a loader test in the `tests/lib/gold-corpus.test.mjs` shape so the record's shape + catalogue membership are pinned under `node --test`. +## Gold-scored output eval (SKAL-1·4b) + +The offline scoring run that grades a recorded agent run against this corpus. +**Offline** = committed reviewer payloads, no live agent spawn, no LLM, no +network (the LLM-in-the-loop grading is the separate 4c tier). + +- **Committed runs** live under `tests/fixtures/bakeoff-rich/runs/`. Each file is + the JSON **reviewer payloads** (one object per reviewer, `{ reviewer, findings }`) + that a recorded run produced. `run-perfect.json` is the regression guard: fed + through the coordinator contract it must reproduce every seeded gold finding. +- **The contract** `lib/review/coordinator-contract.mjs::runContract(payloads)` + turns those payloads into the deterministic coordinator output (4a). +- **The scorer** `lib/review/gold-scorer.mjs`: + - `scoreFindings(runFindings, goldFindings)` matches at **`(file, rule_key)` + granularity** (line + severity ignored) → `{ tp, fp, fn, precision, recall, + f1, matched, missed, spurious }`. Vacuous-set conventions (empty run → + recall 0; empty gold → precision 0; f1 collapses to 0) are documented in the + module header. + - `scoreVerdict(runVerdict, goldVerdict)` → exact verdict match. +- **The scoring run** `tests/lib/gold-eval.test.mjs` asserts `run-perfect` + reproduces gold at precision/recall/f1 = 1.0 and `verdict === expected_verdict`. +- **Third test-census category.** `lib/util/test-census.mjs` now reports a + `goldEval` bucket (matched by `GOLD_EVAL_FILE_RE`) separately from `behavior` + and `docPins` — a scoring run is neither behavior coverage nor a prose pin, so + the honest-count invariant is now a 3-way sum. + ## Future hardening (not in this tier) -- SKAL-1·4b: an offline gold-scored output eval that scores recorded agent runs - against these records at `(file, rule_key)` granularity, with committed runs - and a third test-census category. - A prose↔JSON cross-assertion (parse the fixture README table, diff against `gold.json`) to mechanically bound the two-source-of-truth drift. +- Degraded-run fixtures (a run that misses or invents findings) to exercise the + scorer's discriminating path end-to-end; today that path is covered by + `tests/lib/gold-scorer.test.mjs` with inline synthetic findings. +- SKAL-1·4c: the LLM-in-the-loop eval that grades live agent runs (needs a + filesystem + model judgement, deliberately excluded from this deterministic tier). diff --git a/lib/review/gold-scorer.mjs b/lib/review/gold-scorer.mjs new file mode 100644 index 0000000..638611f Binary files /dev/null and b/lib/review/gold-scorer.mjs differ diff --git a/lib/util/test-census.mjs b/lib/util/test-census.mjs index 1281e38..2992ce4 100644 --- a/lib/util/test-census.mjs +++ b/lib/util/test-census.mjs @@ -1,10 +1,16 @@ // lib/util/test-census.mjs -// Census of the test suite: split top-level test() declarations into -// "behavior" tests vs "doc-consistency pins" (string/existence assertions that -// pin documentation against a source-of-truth). Makes the cited test count -// honest — a prose-pin is not the same coverage as a behavior test, so a single +// Census of the test suite: split top-level test() declarations into three +// honest categories: +// - "behavior" — ordinary behavior coverage (the default bucket). +// - "docPins" — doc-consistency pins (string/existence assertions that pin +// documentation against a source-of-truth). +// - "goldEval" — the offline gold-scored output eval scoring RUN (SKAL-1·4b): +// neither behavior nor prose-pin, but a run that scores a +// committed agent-run fixture against the golden corpus. +// Makes the cited test count honest — a prose-pin is not the same coverage as a +// behavior test, and a scoring run is a third thing again, so a single // conflated total oversells behavior coverage. -// Devil's-advocate audit §Top changes #8 (S19). +// Devil's-advocate audit §Top changes #8 (S19); third bucket added in SKAL-1·4b. // // Metric: top-level `test(` declarations (static, deterministic, in-process). // This is distinct from node:test's runtime total, which additionally counts @@ -18,6 +24,11 @@ import { join } from 'node:path'; // is bucketed correctly without editing this module. export const PIN_FILE_RE = /(doc-consistency|prose-pins)\.test\.mjs$/; +// Files whose tests are the offline gold-scored output eval scoring run +// (SKAL-1·4b). Kept as a regex (not a single filename) so a future split-out +// is bucketed correctly without editing this module. +export const GOLD_EVAL_FILE_RE = /gold-eval\.test\.mjs$/; + function walk(dir) { const out = []; for (const e of readdirSync(dir, { withFileTypes: true })) { @@ -32,18 +43,20 @@ function countTests(file) { return (readFileSync(file, 'utf-8').match(/^\s*test\(/gm) || []).length; } -// Returns { behavior, docPins, total, byFile } for all *.test.mjs under -// testsRoot. behavior + docPins === total by construction. +// Returns { behavior, docPins, goldEval, total, byFile } for all *.test.mjs +// under testsRoot. behavior + docPins + goldEval === total by construction. export function censusTests(testsRoot) { const files = walk(testsRoot).sort(); const byFile = {}; let behavior = 0; let docPins = 0; + let goldEval = 0; for (const f of files) { const n = countTests(f); byFile[f] = n; if (PIN_FILE_RE.test(f)) docPins += n; + else if (GOLD_EVAL_FILE_RE.test(f)) goldEval += n; else behavior += n; } - return { behavior, docPins, total: behavior + docPins, byFile }; + return { behavior, docPins, goldEval, total: behavior + docPins + goldEval, byFile }; } diff --git a/tests/fixtures/bakeoff-rich/runs/run-perfect.json b/tests/fixtures/bakeoff-rich/runs/run-perfect.json new file mode 100644 index 0000000..1c44cde --- /dev/null +++ b/tests/fixtures/bakeoff-rich/runs/run-perfect.json @@ -0,0 +1,17 @@ +[ + { + "reviewer": "brief-conformance-reviewer", + "findings": [ + { "severity": "BLOCKER", "rule_key": "UNIMPLEMENTED_CRITERION", "file": "lib/handlers/login.mjs", "line": 17 }, + { "severity": "MAJOR", "rule_key": "PLAN_EXECUTE_DRIFT", "file": "lib/handlers/login.mjs", "line": 13 } + ] + }, + { + "reviewer": "code-correctness-reviewer", + "findings": [ + { "severity": "BLOCKER", "rule_key": "SECURITY_INJECTION", "file": "lib/auth/jwt.mjs", "line": 19 }, + { "severity": "MAJOR", "rule_key": "MISSING_TEST", "file": "lib/auth/refresh.mjs", "line": 0 }, + { "severity": "MINOR", "rule_key": "MISSING_ERROR_HANDLING", "file": "lib/auth/refresh.mjs", "line": 10 } + ] + } +] diff --git a/tests/lib/gold-eval.test.mjs b/tests/lib/gold-eval.test.mjs new file mode 100644 index 0000000..a34c27f --- /dev/null +++ b/tests/lib/gold-eval.test.mjs @@ -0,0 +1,49 @@ +// tests/lib/gold-eval.test.mjs +// SKAL-1·4b — the offline gold-scored output eval (the scoring RUN). +// +// This is the third test-census category (see lib/util/test-census.mjs): +// neither a behavior unit test nor a doc-consistency pin, but a SCORING RUN — +// it feeds a committed agent-run fixture through the deterministic coordinator +// contract (4a) and scores the result against the golden corpus at +// (file, rule_key) granularity. Offline: committed reviewer payloads, no live +// agent spawn, no LLM, no network. The all-agree foundation; the +// LLM-in-the-loop eval is the separate 4c tier. + +import { test } from 'node:test'; +import { strict as assert } from 'node:assert'; +import { readFileSync } from 'node:fs'; +import { join, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { runContract } from '../../lib/review/coordinator-contract.mjs'; +import { scoreFindings, scoreVerdict } from '../../lib/review/gold-scorer.mjs'; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const ROOT = join(HERE, '..', '..'); +const gold = JSON.parse(readFileSync(join(ROOT, 'tests/fixtures/bakeoff-rich/gold.json'), 'utf-8')); +const runPerfect = JSON.parse( + readFileSync(join(ROOT, 'tests/fixtures/bakeoff-rich/runs/run-perfect.json'), 'utf-8'), +); + +test('gold-eval — run-perfect reproduces gold at (file,rule_key): precision/recall/f1 = 1.0', () => { + const result = runContract(runPerfect); + const score = scoreFindings(result.findings, gold.findings); + assert.equal(score.precision, 1, `precision ${score.precision}; spurious=${JSON.stringify(score.spurious)}`); + assert.equal(score.recall, 1, `recall ${score.recall}; missed=${JSON.stringify(score.missed)}`); + assert.equal(score.f1, 1); + assert.equal(score.tp, gold.findings.length); +}); + +test('gold-eval — run-perfect coordinator verdict matches gold expected_verdict (BLOCK)', () => { + const result = runContract(runPerfect); + assert.equal(result.verdict, gold.expected_verdict); + assert.ok(scoreVerdict(result.verdict, gold.expected_verdict)); +}); + +test('gold-eval — run-perfect drops nothing (no suppressed, no schema-skipped payloads)', () => { + // A clean run is the regression guard: any future contract change that + // silently suppresses or skips one of the 5 seeded findings breaks here. + const result = runContract(runPerfect); + assert.equal(result.skipped.length, 0, `skipped payloads: ${JSON.stringify(result.skipped)}`); + assert.equal(result.suppressed.length, 0, `suppressed findings: ${result.suppressed.length}`); + assert.equal(result.findings.length, gold.findings.length); +}); diff --git a/tests/lib/gold-scorer.test.mjs b/tests/lib/gold-scorer.test.mjs new file mode 100644 index 0000000..a9eae63 --- /dev/null +++ b/tests/lib/gold-scorer.test.mjs @@ -0,0 +1,126 @@ +// tests/lib/gold-scorer.test.mjs +// SKAL-1·4b — behavior test for the gold scorer. +// +// scoreFindings compares a recorded agent-run's findings against a golden +// corpus record at (file, rule_key) granularity (line + severity deliberately +// ignored). This pins the precision/recall/f1 math AND the discriminating +// paths (false negatives + false positives), not merely the all-match case — +// a scorer that always returned 1.0 would pass an all-match-only test. + +import { test } from 'node:test'; +import { strict as assert } from 'node:assert'; +import { scoreFindings, scoreVerdict } from '../../lib/review/gold-scorer.mjs'; + +// A minimal gold set: two distinct (file, rule_key) pairs. +const GOLD = [ + { file: 'a.mjs', line: 10, rule_key: 'SECURITY_INJECTION', severity: 'BLOCKER' }, + { file: 'b.mjs', line: 0, rule_key: 'MISSING_TEST', severity: 'MAJOR' }, +]; + +test('scoreFindings — perfect match scores precision/recall/f1 = 1', () => { + // Same pairs, different line/severity (must be ignored at (file,rule_key) granularity). + const run = [ + { file: 'a.mjs', line: 99, rule_key: 'SECURITY_INJECTION', severity: 'MINOR' }, + { file: 'b.mjs', line: 7, rule_key: 'MISSING_TEST', severity: 'BLOCKER' }, + ]; + const s = scoreFindings(run, GOLD); + assert.equal(s.tp, 2); + assert.equal(s.fp, 0); + assert.equal(s.fn, 0); + assert.equal(s.precision, 1); + assert.equal(s.recall, 1); + assert.equal(s.f1, 1); +}); + +test('scoreFindings — a missed gold pair is a false negative (recall < 1)', () => { + const run = [{ file: 'a.mjs', line: 10, rule_key: 'SECURITY_INJECTION', severity: 'BLOCKER' }]; + const s = scoreFindings(run, GOLD); + assert.equal(s.tp, 1); + assert.equal(s.fp, 0); + assert.equal(s.fn, 1); + assert.equal(s.precision, 1); + assert.equal(s.recall, 0.5); + assert.deepEqual(s.missed, ['b.mjs MISSING_TEST']); +}); + +test('scoreFindings — a spurious pair is a false positive (precision < 1)', () => { + const run = [ + { file: 'a.mjs', line: 10, rule_key: 'SECURITY_INJECTION', severity: 'BLOCKER' }, + { file: 'b.mjs', line: 0, rule_key: 'MISSING_TEST', severity: 'MAJOR' }, + { file: 'c.mjs', line: 3, rule_key: 'PLACEHOLDER_IN_CODE', severity: 'MAJOR' }, + ]; + const s = scoreFindings(run, GOLD); + assert.equal(s.tp, 2); + assert.equal(s.fp, 1); + assert.equal(s.fn, 0); + assert.equal(s.recall, 1); + assert.equal(s.precision, 2 / 3); + assert.deepEqual(s.spurious, ['c.mjs PLACEHOLDER_IN_CODE']); +}); + +test('scoreFindings — mixed FN + FP', () => { + const run = [ + { file: 'a.mjs', line: 10, rule_key: 'SECURITY_INJECTION', severity: 'BLOCKER' }, // match + { file: 'c.mjs', line: 3, rule_key: 'PLACEHOLDER_IN_CODE', severity: 'MAJOR' }, // spurious + ]; + const s = scoreFindings(run, GOLD); + assert.equal(s.tp, 1); + assert.equal(s.fp, 1); + assert.equal(s.fn, 1); + assert.equal(s.precision, 0.5); + assert.equal(s.recall, 0.5); + assert.equal(s.f1, 0.5); +}); + +test('scoreFindings — duplicate (file,rule_key) pairs collapse (set semantics)', () => { + const run = [ + { file: 'a.mjs', line: 10, rule_key: 'SECURITY_INJECTION', severity: 'BLOCKER' }, + { file: 'a.mjs', line: 22, rule_key: 'SECURITY_INJECTION', severity: 'BLOCKER' }, // same pair + { file: 'b.mjs', line: 0, rule_key: 'MISSING_TEST', severity: 'MAJOR' }, + ]; + const s = scoreFindings(run, GOLD); + assert.equal(s.tp, 2); + assert.equal(s.fp, 0); + assert.equal(s.fn, 0); +}); + +test('scoreFindings — empty run: recall 0 (nothing found), precision vacuously 1, f1 0', () => { + const s = scoreFindings([], GOLD); + assert.equal(s.tp, 0); + assert.equal(s.fp, 0); + assert.equal(s.fn, 2); + assert.equal(s.recall, 0); + assert.equal(s.precision, 1); + assert.equal(s.f1, 0); +}); + +test('scoreFindings — empty gold: precision 0 (all spurious), recall vacuously 1, f1 0', () => { + const run = [{ file: 'a.mjs', line: 10, rule_key: 'SECURITY_INJECTION', severity: 'BLOCKER' }]; + const s = scoreFindings(run, []); + assert.equal(s.tp, 0); + assert.equal(s.fp, 1); + assert.equal(s.fn, 0); + assert.equal(s.precision, 0); + assert.equal(s.recall, 1); + assert.equal(s.f1, 0); +}); + +test('scoreFindings — both empty: precision/recall/f1 = 1 (matched nothing perfectly)', () => { + const s = scoreFindings([], []); + assert.equal(s.precision, 1); + assert.equal(s.recall, 1); + assert.equal(s.f1, 1); +}); + +test('scoreFindings — tolerates null/undefined finding arrays', () => { + const s = scoreFindings(null, undefined); + assert.equal(s.tp, 0); + assert.equal(s.fp, 0); + assert.equal(s.fn, 0); +}); + +test('scoreVerdict — exact verdict match is true, mismatch is false', () => { + assert.equal(scoreVerdict('BLOCK', 'BLOCK'), true); + assert.equal(scoreVerdict('WARN', 'BLOCK'), false); + assert.equal(scoreVerdict('ALLOW', 'ALLOW'), true); +}); diff --git a/tests/lib/test-census.test.mjs b/tests/lib/test-census.test.mjs index 92fb0d9..81328a4 100644 --- a/tests/lib/test-census.test.mjs +++ b/tests/lib/test-census.test.mjs @@ -15,20 +15,21 @@ import { censusTests } from '../../lib/util/test-census.mjs'; const HERE = dirname(fileURLToPath(import.meta.url)); const TESTS_ROOT = join(HERE, '..'); -test('suite census splits behavior tests from doc-consistency pins (S19)', (t) => { +test('suite census splits behavior / doc-pins / gold-eval (S19 + SKAL-1·4b)', (t) => { const c = censusTests(TESTS_ROOT); - // Honest-count invariant: the two buckets must account for every top-level - // test() declaration — no silent drift between behavior and pin counts. - assert.equal(c.behavior + c.docPins, c.total, + // Honest-count invariant: the three buckets must account for every top-level + // test() declaration — no silent drift between behavior, pin, and eval counts. + assert.equal(c.behavior + c.docPins + c.goldEval, c.total, 'census buckets must sum to the total declaration count'); assert.ok(c.docPins > 0, 'doc-consistency pin bucket must be non-empty (regex/glob sanity)'); assert.ok(c.behavior > 0, 'behavior bucket must be non-empty (regex/glob sanity)'); + assert.ok(c.goldEval > 0, 'gold-eval scoring-run bucket must be non-empty (SKAL-1·4b present)'); // Report the split so the cited count is honest (audit §Top changes #8). // Metric = top-level test() declarations; node:test's runtime total counts // subtests too and is therefore ≥ this number. t.diagnostic( `behavior=${c.behavior} doc-consistency-pins=${c.docPins} ` + - `total=${c.total} (top-level test() declarations)`, + `gold-eval=${c.goldEval} total=${c.total} (top-level test() declarations)`, ); });