From c2938f62fbfbe5b11bdac9c6fdf943404e276d2d Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 12 Jun 2026 12:38:34 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20pre-landing=20review=20fixes=20=E2=80=94?= =?UTF-8?q?=20specialist=20round=20(4=20reviewers,=2022=20findings)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The catch that mattered (testing + maintainability, independently): --llm extraction metrics were computed per fixture but never reached any scoreboard cell — dead matched_any_gold/stored_rows aggregation fields proved the unfinished wiring. Now aggregated Σ-style into the write-back cell when llm is on, pinned by a harness-level stubbed-transport test. Also: RUN-scoped --llm BudgetTracker (a per-invocation cap multiplied by fixture count — ~$550 worst case — now one tracker, exhaustion aborts the run loudly); continuity loop collapsed to one prep per pair + read-only reader per harness (the writer replay was provably observable-effect-free and orderings rebuilt byte-identical brains — 90 preps → 15, runtime ~12s → ~7s, identical scores; baseline counts updated 24 → 12 honestly); factKeywordProbe escapes ILIKE metacharacters; validator rejects unpassable slug-less retrieve-turns; ci-gate fails HARD on a broken ref instead of silently running ungated; privacy year-scan covers the gold dir; e2e tests self-sufficient (shared artifacts in beforeAll) with a minimal-env --llm gate; round4/cellKey single-sourced; explicit return in eval.ts dispatch; stale comment + type anchor fixes. --- docs/eval/BRAINBENCH.md | 2 +- docs/eval/METRIC_GLOSSARY.md | 4 +- evals/brainbench/baselines/main.json | 6 +- scripts/check-synthetic-corpus-privacy.sh | 2 +- scripts/ci-brainbench-gate.sh | 16 ++- src/commands/eval-brainbench.ts | 7 +- src/commands/eval-run-all.ts | 6 +- src/commands/eval.ts | 1 + src/commands/extract-conversation-facts.ts | 2 +- src/core/eval/metric-glossary.ts | 4 +- src/eval/brainbench/fixtures.ts | 33 ++++-- src/eval/brainbench/harness.ts | 116 +++++++++++---------- src/eval/brainbench/metrics/continuity.ts | 8 +- src/eval/brainbench/metrics/write-back.ts | 41 ++++++-- src/eval/brainbench/scoreboard.ts | 18 ++-- src/eval/brainbench/types.ts | 8 ++ test/brainbench-fixtures.test.ts | 12 +++ test/brainbench-writeback.test.ts | 38 +++++++ test/eval-brainbench-e2e.test.ts | 53 ++++++---- 19 files changed, 252 insertions(+), 125 deletions(-) diff --git a/docs/eval/BRAINBENCH.md b/docs/eval/BRAINBENCH.md index d4db118d9..766fdd56e 100644 --- a/docs/eval/BRAINBENCH.md +++ b/docs/eval/BRAINBENCH.md @@ -48,7 +48,7 @@ All micro-averaged per (harness × suite) cell; registered in - `push_recall` = Σ|injected ∩ gold| / Σ|gold| over should-retrieve turns. Pointer budgets cap this by design. - `write_back_fidelity` = |gold facts that survive the PRODUCTION conversation→memory pipeline and are keyword-findable with correct entity attribution| / |gold facts|. The deterministic mode injects a gold extractor at the pipeline's extractor seam so segmentation, batching, dedup, and provenance stamping execute shipped code with zero LLM calls. - `provenance_accuracy` = |surviving facts with correct {source, source_session, source_markdown_slug}| / |surviving facts|. -- `continuity_rate` = |decision probes recalled by the reader| / |probes|, over writer→reader pairs replayed through DIFFERENT harness adapters on a shared brain. A probe succeeds via pointer injection or stored-fact keyword lookup. Headline = mean over off-diagonal (writer ≠ reader) pairs. +- `continuity_rate` = |decision probes recalled by the reader| / |probes|, per READER harness. The writer fixture's decisions persist through the production write-back pipeline — which is harness-INDEPENDENT in v1 — so each pair preps once and every harness replays the read-only reader against the same persisted state (an ordered writer×reader sweep would rebuild byte-identical brains for identical scores). A probe succeeds via pointer injection or stored-fact keyword lookup. The per-writer axis activates when harness-specific write paths land. - `source_isolation_violations` = count of injected slugs from a non-active source. **Gates at zero**, every run, regardless of baseline — cross-source leakage is the data-leak invariant. - `avg_injected_tokens` = mean estimated tokens (chars/4) of injected context per replayed turn. Intrusion-budget diagnostic; reported, NOT gated (gating awaits calibration data — filed TODO). - `extraction_recall` / `extraction_precision` — `--llm` runs only: the real extractor's output vs gold keyword probes. diff --git a/docs/eval/METRIC_GLOSSARY.md b/docs/eval/METRIC_GLOSSARY.md index 81089b7d6..48be0b89b 100644 --- a/docs/eval/METRIC_GLOSSARY.md +++ b/docs/eval/METRIC_GLOSSARY.md @@ -222,9 +222,9 @@ Every metric `gbrain eval *` and `gbrain search stats` reports has a plain-Engli **Key:** `continuity_rate` -**Plain English:** A decision is recorded in one harness's session; a DIFFERENT harness asks about it later on the same brain. What fraction of those decision probes were recalled — by pointer injection or stored-fact lookup? This is the continuity-that-survives-the-harness-hop moat, measured. +**Plain English:** A decision is recorded in one session and persisted through the production write path; a different harness asks about it later on the same brain. What fraction of those decision probes were recalled — by pointer injection or stored-fact lookup? This is the continuity-that-survives-the-harness-hop moat, measured. -**Range:** 0..1, higher is better. Headline = mean over writer→reader pairs with different harnesses. +**Range:** 0..1, higher is better. Scored per reader harness (the v1 write path is harness-independent, disclosed in docs/eval/BRAINBENCH.md). ### Source-isolation violations (BrainBench) diff --git a/evals/brainbench/baselines/main.json b/evals/brainbench/baselines/main.json index a4b5b9c05..507a77190 100644 --- a/evals/brainbench/baselines/main.json +++ b/evals/brainbench/baselines/main.json @@ -68,7 +68,7 @@ }, "counts": { "claude-code/continuity": { - "gold_total": 24, + "gold_total": 12, "gold_failed": 0 }, "claude-code/know-to-ask": { @@ -84,7 +84,7 @@ "gold_failed": 0 }, "codex/continuity": { - "gold_total": 24, + "gold_total": 12, "gold_failed": 0 }, "codex/know-to-ask": { @@ -100,7 +100,7 @@ "gold_failed": 0 }, "openclaw/continuity": { - "gold_total": 24, + "gold_total": 12, "gold_failed": 0 }, "openclaw/know-to-ask": { diff --git a/scripts/check-synthetic-corpus-privacy.sh b/scripts/check-synthetic-corpus-privacy.sh index 131ae35ce..c7e9a724e 100755 --- a/scripts/check-synthetic-corpus-privacy.sh +++ b/scripts/check-synthetic-corpus-privacy.sh @@ -115,7 +115,7 @@ if [ -d "$BB_DIR/fixtures" ]; then echo " VIOLATION: out-of-range year in $match (fixtures use 2024-2026)" VIOLATIONS=$((VIOLATIONS + 1)) fi - done < <(grep -rEn '\b(201[0-9]|202[0-3]|202[7-9]|20[3-9][0-9])\b' "$BB_DIR/fixtures" --include='*.json' 2>/dev/null || true) + done < <(grep -rEn '\b(201[0-9]|202[0-3]|202[7-9]|20[3-9][0-9])\b' "$BB_DIR/fixtures" "$BB_DIR/gold" --include='*.json' 2>/dev/null || true) fi if [ "$VIOLATIONS" -gt 0 ]; then diff --git a/scripts/ci-brainbench-gate.sh b/scripts/ci-brainbench-gate.sh index 474c4be4f..b0607133a 100755 --- a/scripts/ci-brainbench-gate.sh +++ b/scripts/ci-brainbench-gate.sh @@ -17,16 +17,26 @@ set -euo pipefail BASELINE_PATH="evals/brainbench/baselines/main.json" MAIN_REF="${BRAINBENCH_MAIN_REF:-origin/master}" -OUT="${BRAINBENCH_OUT:-/tmp/brainbench-result.json}" +# mktemp default (review finding): a fixed world-writable /tmp path is a +# symlink-planting target on shared hosts. CI overrides via BRAINBENCH_OUT. +OUT="${BRAINBENCH_OUT:-$(mktemp /tmp/brainbench-result-XXXXXX.json)}" MAIN_BASELINE="$(mktemp /tmp/brainbench-main-baseline-XXXXXX.json)" trap 'rm -f "$MAIN_BASELINE"' EXIT +# Fail HARD when the ref itself is broken — only a genuinely-absent baseline +# may take the ungated first-landing path (review finding: an unfetched ref +# or typo'd BRAINBENCH_MAIN_REF must not silently disable the gate). +if ! git rev-parse --verify --quiet "${MAIN_REF}^{commit}" > /dev/null; then + echo "[brainbench-gate] ERROR: ref ${MAIN_REF} does not resolve — fetch it or fix BRAINBENCH_MAIN_REF" >&2 + exit 2 +fi + if git show "${MAIN_REF}:${BASELINE_PATH}" > "$MAIN_BASELINE" 2>/dev/null; then echo "[brainbench-gate] comparing against ${MAIN_REF}:${BASELINE_PATH}" bun src/cli.ts eval brainbench --compare "$MAIN_BASELINE" --out "$OUT" else - # First landing: main has no baseline yet. Run without a gate so the PR - # that introduces BrainBench can commit the initial baseline. + # First landing: the ref exists but carries no baseline yet. Run without a + # gate so the PR that introduces BrainBench can commit the initial baseline. echo "[brainbench-gate] no baseline on ${MAIN_REF} yet — running ungated (initial-landing path)" bun src/cli.ts eval brainbench --out "$OUT" fi diff --git a/src/commands/eval-brainbench.ts b/src/commands/eval-brainbench.ts index e83efc760..0f49b6ae8 100644 --- a/src/commands/eval-brainbench.ts +++ b/src/commands/eval-brainbench.ts @@ -16,7 +16,7 @@ */ import { readFileSync, writeFileSync, existsSync, mkdirSync } from 'node:fs'; -import { dirname, join } from 'node:path'; +import { dirname } from 'node:path'; import { execSync } from 'node:child_process'; import { flushThenExit } from '../core/cli-force-exit.ts'; import { cliOptsToProgressOptions, getCliOptions } from '../core/cli-options.ts'; @@ -26,6 +26,7 @@ import { isAvailable } from '../core/ai/gateway.ts'; import { FixtureValidationError, loadCorpus } from '../eval/brainbench/fixtures.ts'; import { runBrainBench } from '../eval/brainbench/harness.ts'; import { + cellKey, compareBaselines, parseBaseline, renderScoreboardMarkdown, @@ -417,7 +418,7 @@ export async function runBrainBenchCore(): Promise<{ } const cells: Record> = {}; for (const c of run.cells) { - cells[`${c.harness}/${c.suite}`] = { ...c.metrics, gold_failed: c.gold_failed, gold_total: c.gold_total }; + cells[cellKey(c)] = { ...c.metrics, gold_failed: c.gold_failed, gold_total: c.gold_total }; } return { status: 'completed', fixtures_hash: corpus.fixtures_hash, cells }; } catch (err) { @@ -426,4 +427,4 @@ export async function runBrainBenchCore(): Promise<{ } /** Test hook: everything above process.exit, without exiting. */ -export const _internal = { parseArgs, DEFAULT_BASELINE_PATH: join(DEFAULT_BASELINE_PATH) }; +export const _internal = { parseArgs, DEFAULT_BASELINE_PATH }; diff --git a/src/commands/eval-run-all.ts b/src/commands/eval-run-all.ts index ede0e0207..1616e94e2 100644 --- a/src/commands/eval-run-all.ts +++ b/src/commands/eval-run-all.ts @@ -135,8 +135,10 @@ export interface EvalRunRecord { * v3 (BrainBench wave, decision 16): `mode` widened to `SearchMode | 'n/a'` * for search-mode-independent suites — brainbench runs once per sweep and * records 'n/a' instead of fabricating a mode. v2 records (mode always a - * SearchMode) parse fine under v3 readers; eval-compare renders 'n/a' rows - * un-grouped. + * SearchMode) parse fine under v3 readers. NOTE: eval-compare's markdown + * renderer iterates SEARCH_MODES only, so 'n/a' rows surface in its --json + * `records` output, not the mode table (review finding; markdown surfacing + * is a follow-up). */ schema_version: 3; run_id: string; diff --git a/src/commands/eval.ts b/src/commands/eval.ts index 8785b40f8..577d3fa0e 100644 --- a/src/commands/eval.ts +++ b/src/commands/eval.ts @@ -59,6 +59,7 @@ export async function runEvalCommand(engine: BrainEngine, args: string[]): Promi // command owns its exit codes (0 pass / 1 regression / 2 error). const { runEvalBrainBench } = await import('./eval-brainbench.ts'); await runEvalBrainBench(args.slice(1)); + return; // unreachable — runEvalBrainBench always exits — but keeps control flow explicit } if (sub === 'code-retrieval') { // v0.33.3 pre-w0 — code-retrieval baseline / gate harness. Needs a brain diff --git a/src/commands/extract-conversation-facts.ts b/src/commands/extract-conversation-facts.ts index 8eb7cb684..b63328883 100644 --- a/src/commands/extract-conversation-facts.ts +++ b/src/commands/extract-conversation-facts.ts @@ -744,7 +744,7 @@ async function processPage( const text = renderSegmentForExtraction(page.title || page.slug, seg); const sessionId = `${PER_SEGMENT_SOURCE_PREFIX}:${page.slug}`; - let extracted: Awaited> = []; + let extracted: ExtractedFact[] = []; try { extracted = await state.extractor({ turnText: text, diff --git a/src/core/eval/metric-glossary.ts b/src/core/eval/metric-glossary.ts index 2738b125d..bb61a9b0d 100644 --- a/src/core/eval/metric-glossary.ts +++ b/src/core/eval/metric-glossary.ts @@ -184,8 +184,8 @@ export const METRIC_GLOSSARY: Readonly }), 'continuity_rate': Object.freeze({ industry_term: 'Cross-session continuity rate (BrainBench)', - eli10: 'A decision is recorded in one harness\'s session; a DIFFERENT harness asks about it later on the same brain. What fraction of those decision probes were recalled — by pointer injection or stored-fact lookup? This is the continuity-that-survives-the-harness-hop moat, measured.', - range: '0..1, higher is better. Headline = mean over writer→reader pairs with different harnesses.', + eli10: 'A decision is recorded in one session and persisted through the production write path; a different harness asks about it later on the same brain. What fraction of those decision probes were recalled — by pointer injection or stored-fact lookup? This is the continuity-that-survives-the-harness-hop moat, measured.', + range: '0..1, higher is better. Scored per reader harness (the v1 write path is harness-independent, disclosed in docs/eval/BRAINBENCH.md).', }), 'source_isolation_violations': Object.freeze({ industry_term: 'Source-isolation violations (BrainBench)', diff --git a/src/eval/brainbench/fixtures.ts b/src/eval/brainbench/fixtures.ts index d3e874821..e4c42099c 100644 --- a/src/eval/brainbench/fixtures.ts +++ b/src/eval/brainbench/fixtures.ts @@ -197,16 +197,21 @@ export function validateGold(file: string, raw: unknown, fixture: BrainBenchFixt if (typeof tg.should_retrieve !== 'boolean') { throw new FixtureValidationError(file, `gold.turns["${key}"].should_retrieve must be boolean`); } - if (tg.should_retrieve && !isStringArray(tg.gold_slugs)) { - // gold_slugs may legitimately be absent on write-back-only turns; require - // it only when the turn participates in retrieval suites. + { + // On a retrieval-suite fixture, should_retrieve=true REQUIRES non-empty + // gold_slugs — a slug-less retrieve-turn is a guaranteed miss no harness + // can pass (review finding: it silently poisons the failure rate). const retrievalSuites = fixture.suites.some( (s: BrainBenchSuite) => s === 'know-to-ask' || s === 'push', ); - if (retrievalSuites && !tg.gold_facts) { + if ( + retrievalSuites && + tg.should_retrieve && + (!isStringArray(tg.gold_slugs) || tg.gold_slugs.length === 0) + ) { throw new FixtureValidationError( file, - `gold.turns["${key}"].gold_slugs required when should_retrieve=true on a retrieval-suite fixture`, + `gold.turns["${key}"].gold_slugs must be non-empty when should_retrieve=true on a retrieval-suite fixture (a slug-less retrieve turn is an unpassable gold item)`, ); } } @@ -288,9 +293,16 @@ export async function loadCorpus(fixtureDir: string, goldDir: string): Promise(); - for (const gf of goldFiles) { - const path = join(goldDir, gf); - const content = await readFile(path, 'utf-8'); + // Parallel I/O; hashing stays in sorted order over the resolved array + // (determinism needs ordered hash.update, not serial reads). + const goldContents = await Promise.all(goldFiles.map((gf) => readFile(join(goldDir, gf), 'utf-8'))); + const fixtureContents = await Promise.all( + fixtureFiles.map((ff) => readFile(join(fixtureDir, ff), 'utf-8')), + ); + + for (let i = 0; i < goldFiles.length; i++) { + const gf = goldFiles[i]; + const content = goldContents[i]; hash.update(`gold/${gf}\n`); hash.update(content); let parsed: unknown; @@ -306,9 +318,10 @@ export async function loadCorpus(fixtureDir: string, goldDir: string): Promise(); - for (const ff of fixtureFiles) { + for (let i = 0; i < fixtureFiles.length; i++) { + const ff = fixtureFiles[i]; const path = join(fixtureDir, ff); - const content = await readFile(path, 'utf-8'); + const content = fixtureContents[i]; hash.update(`fixtures/${ff}\n`); hash.update(content); let parsed: unknown; diff --git a/src/eval/brainbench/harness.ts b/src/eval/brainbench/harness.ts index 147290404..860ff965d 100644 --- a/src/eval/brainbench/harness.ts +++ b/src/eval/brainbench/harness.ts @@ -9,14 +9,20 @@ * reset. A sentinel-slug test pins that sharing leaks nothing. * * Continuity pairs (writer fixture → production write-back → reader fixture) - * run per ordered (writerHarness ≠ readerHarness) pair on a shared brain; - * scores land on the READER's cell. With a single requested harness the - * writer==reader diagonal runs instead (disclosed, not the headline shape). + * run on a shared brain; scores land on the READER's cell. The write path is + * gbrain's pipeline — harness-INDEPENDENT in v1 (disclosed in + * docs/eval/BRAINBENCH.md) — so each pair preps ONCE (reset + seed + + * write-back) and every requested harness replays the read-only reader against + * that shared state. The per-(writer×reader)-ordering loop this replaces + * rebuilt byte-identical brains 6x per pair and replayed a writer whose state + * could not outlive the call (review findings: dead work, identical scores). + * The writer axis activates when harness-specific write paths actually land. * * Sealed gold: adapters only ever receive AdapterFixtureView + PublicTurn * (toPublicTurn picks fields; gold never crosses). */ +import { BudgetTracker } from '../../core/budget/budget-tracker.ts'; import { createBenchmarkBrain, resetTables } from '../longmemeval/harness.ts'; import type { PGLiteEngine } from '../../core/pglite-engine.ts'; import { ClaudeCodeAdapter } from './adapters/claude-code.ts'; @@ -28,6 +34,7 @@ import { runWriteBack, type WriteBackScore } from './metrics/write-back.ts'; import { scoreContinuityPair } from './metrics/continuity.ts'; import { SeedError, seedBrain, type SeedOutcome } from './seed.ts'; import { + round4, toPublicTurn, type AdapterFixtureView, type BrainBenchSuite, @@ -199,6 +206,12 @@ export async function runBrainBench( }; const continuityByReader = new Map(); + // RUN-scoped --llm budget (review finding: a per-invocation cap would + // multiply by fixture count — ~$550 worst case on the committed corpus). + const llmTracker = opts.llm + ? new BudgetTracker({ maxCostUsd: opts.budgetUsd ?? 5, label: 'brainbench:llm' }) + : undefined; + const engine = await createBenchmarkBrain(); let fixturesRun = 0; try { @@ -234,23 +247,13 @@ export async function runBrainBench( const score = await runWriteBack(engine, lf.fixture, lf.gold, { llm: opts.llm, budgetUsd: opts.budgetUsd, + budgetTracker: llmTracker, }); accumulateWriteBack(writeBackAgg, id, score); } } - // ---- continuity pairs ---- - const pairHarnesses: Array<[HarnessName, HarnessName]> = []; - if (opts.harnesses.length === 1) { - pairHarnesses.push([opts.harnesses[0], opts.harnesses[0]]); - } else { - for (const w of opts.harnesses) { - for (const r of opts.harnesses) { - if (w !== r) pairHarnesses.push([w, r]); - } - } - } - + // ---- continuity pairs: ONE prep per pair, every harness reads it ---- for (const [pairId, pair] of pairFixtures) { if (!pair.writer || !pair.reader) continue; // loader validates; belt+suspenders const writer = pair.writer; @@ -258,32 +261,31 @@ export async function runBrainBench( const decisions = reader.gold.continuity?.decisions ?? []; if (!decisions.length) continue; - for (const [writerHarness, readerHarness] of pairHarnesses) { - progress(`continuity ${pairId} ${writerHarness}→${readerHarness}`); - await resetTables(engine); - let writerSeed: SeedOutcome; - let readerSeed: SeedOutcome; - try { - writerSeed = await seedBrain(engine, writer.fixture); - readerSeed = await seedBrain(engine, reader.fixture); - } catch (err) { - if (err instanceof SeedError) { - seedFailures.push({ fixture_id: `${pairId} (${writerHarness}→${readerHarness})`, error: err.message }); - continue; - } - throw err; + progress(`continuity ${pairId}`); + await resetTables(engine); + let readerSeed: SeedOutcome; + try { + await seedBrain(engine, writer.fixture); + readerSeed = await seedBrain(engine, reader.fixture); + } catch (err) { + if (err instanceof SeedError) { + seedFailures.push({ fixture_id: pairId, error: err.message }); + continue; } + throw err; + } + fixturesRun += 2; - // Writer conversation replays through ITS harness (suppression state - // realistic), then its decisions persist via the production pipeline. - const writerAdapter = makeAdapter(writerHarness); - await replayFixture(engine, writerAdapter, writer, writerSeed, []); - await runWriteBack(engine, writer.fixture, writer.gold, { - llm: opts.llm, - budgetUsd: opts.budgetUsd, - }); + // The writer's decisions persist through the PRODUCTION pipeline — + // harness-independent in v1, so it runs once per pair. + await runWriteBack(engine, writer.fixture, writer.gold, { + llm: opts.llm, + budgetUsd: opts.budgetUsd, + budgetTracker: llmTracker, + }); - // Reader replays on the SAME brain through a different harness. + // Every requested harness reads the SAME persisted state (read-only). + for (const readerHarness of opts.harnesses) { const readerAdapter = makeAdapter(readerHarness); const readerRows = await replayFixture(engine, readerAdapter, reader, readerSeed, ['continuity']); turnRows.push(...readerRows); @@ -297,10 +299,9 @@ export async function runBrainBench( agg.gold_total += score.gold_total; agg.gold_failed += score.gold_failed; if (!agg.fixtures.includes(reader.fixture.fixture_id)) agg.fixtures.push(reader.fixture.fixture_id); - agg.failed_items.push(...score.failed_items.map((f) => `${f} [${writerHarness}→${readerHarness}]`)); + agg.failed_items.push(...score.failed_items.map((f) => `${f} [reader: ${readerHarness}]`)); continuityByReader.set(readerHarness, agg); } - fixturesRun += 2; } } finally { await engine.disconnect(); @@ -310,7 +311,7 @@ export async function runBrainBench( const cells: SuiteMetrics[] = []; for (const harness of opts.harnesses) { for (const suite of opts.suites) { - const cell = assembleCell(harness, suite, turnRows, writeBackAgg, continuityByReader); + const cell = assembleCell(harness, suite, turnRows, writeBackAgg, continuityByReader, opts.llm); if (cell) cells.push(cell); } } @@ -324,38 +325,47 @@ function accumulateWriteBack(agg: WriteBackAgg, fixtureId: string, score: WriteB agg.survived += score.survived; agg.provenance_ok += score.provenance_ok; agg.stored_rows += score.stored_rows; + agg.matched_any_gold += score.matched_any_gold; agg.fixtures.push(fixtureId); agg.failed_items.push(...score.failed_items); } -function round4(n: number): number { - return Math.round(n * 10000) / 10000; -} - function assembleCell( harness: HarnessName, suite: BrainBenchSuite, turnRows: TurnRow[], writeBackAgg: WriteBackAgg, continuityByReader: Map, + llm: boolean, ): SuiteMetrics | null { if (suite === 'write-back') { if (writeBackAgg.fixtures.length === 0) return null; // The write path is gbrain's pipeline — identical for every harness seam // in v1, so each harness cell carries the same (once-computed) numbers. // When harness-specific write paths land, this is where they diverge. + const metrics: Record = { + write_back_fidelity: round4( + writeBackAgg.gold_total > 0 ? writeBackAgg.survived / writeBackAgg.gold_total : 1, + ), + provenance_accuracy: round4( + writeBackAgg.survived > 0 ? writeBackAgg.provenance_ok / writeBackAgg.survived : 1, + ), + }; + if (llm) { + // Σ-aggregated extraction quality (review finding: per-fixture values + // were computed but never reached any cell). + metrics.extraction_recall = round4( + writeBackAgg.gold_total > 0 ? writeBackAgg.survived / writeBackAgg.gold_total : 1, + ); + metrics.extraction_precision = round4( + writeBackAgg.stored_rows > 0 ? writeBackAgg.matched_any_gold / writeBackAgg.stored_rows : 1, + ); + } return { suite, harness, seam: SEAM[harness], gold_total: writeBackAgg.gold_total, gold_failed: writeBackAgg.gold_failed, - metrics: { - write_back_fidelity: round4( - writeBackAgg.gold_total > 0 ? writeBackAgg.survived / writeBackAgg.gold_total : 1, - ), - provenance_accuracy: round4( - writeBackAgg.survived > 0 ? writeBackAgg.provenance_ok / writeBackAgg.survived : 1, - ), - }, + metrics, fixtures: [...writeBackAgg.fixtures], }; } diff --git a/src/eval/brainbench/metrics/continuity.ts b/src/eval/brainbench/metrics/continuity.ts index e0a59d78e..18bdeb56b 100644 --- a/src/eval/brainbench/metrics/continuity.ts +++ b/src/eval/brainbench/metrics/continuity.ts @@ -59,8 +59,12 @@ export async function factKeywordProbe( sourceId: string, keywords: string[], ): Promise { - const conds = keywords.map((_, i) => `fact ILIKE $${i + 2}`).join(' AND '); - const params = [sourceId, ...keywords.map((kw) => `%${kw}%`)]; + // Escape ILIKE metacharacters so a gold keyword containing % or _ can't + // silently broaden the match into a false pass (review finding — scoring + // integrity, not injection: everything is parameterized). + const escapeLike = (s: string) => s.replace(/[\\%_]/g, (m) => `\\${m}`); + const conds = keywords.map((_, i) => `fact ILIKE $${i + 2} ESCAPE '\\'`).join(' AND '); + const params = [sourceId, ...keywords.map((kw) => `%${escapeLike(kw)}%`)]; const rows = await engine.executeRaw<{ one: number }>( `SELECT 1 AS one FROM facts WHERE source_id = $1 AND expired_at IS NULL AND ${conds} diff --git a/src/eval/brainbench/metrics/write-back.ts b/src/eval/brainbench/metrics/write-back.ts index 2f740bc74..454fb09d6 100644 --- a/src/eval/brainbench/metrics/write-back.ts +++ b/src/eval/brainbench/metrics/write-back.ts @@ -21,6 +21,7 @@ * {source, source_session, source_markdown_slug} */ +import type { BudgetTracker } from '../../../core/budget/budget-tracker.ts'; import type { ExtractInput, ExtractedFact } from '../../../core/facts/extract.ts'; import { PER_SEGMENT_SOURCE_PREFIX, @@ -40,6 +41,8 @@ export interface WriteBackScore { provenance_ok: number; /** Total non-audit rows stored (extraction_precision denominator in --llm mode). */ stored_rows: number; + /** Stored rows matching ANY gold probe (extraction_precision numerator). */ + matched_any_gold: number; metrics: Record; failed_items: string[]; } @@ -133,7 +136,16 @@ export async function runWriteBack( engine: PGLiteEngine, fixture: BrainBenchFixture, gold: FixtureGold, - opts: { llm: boolean; budgetUsd?: number }, + opts: { + llm: boolean; + budgetUsd?: number; + /** + * RUN-scOPED tracker for --llm mode (review finding: a per-invocation cap + * would multiply by fixture count). The harness owns one tracker for the + * whole run and threads it here; the pipeline uses it as-is. + */ + budgetTracker?: BudgetTracker; + }, ): Promise { const sourceId = fixture.active_source ?? 'default'; const slug = conversationSlug(fixture.fixture_id); @@ -148,18 +160,25 @@ export async function runWriteBack( ); } - await runExtractConversationFactsCore(engine, { + const extractResult = await runExtractConversationFactsCore(engine, { sourceId, slug, types: ['conversation'], overrideDisabled: true, sleepMs: 0, // Deterministic CI mode injects the gold extractor; --llm runs the real - // one under the caller's budget cap (cost-guard pattern, decision 2). + // one under the RUN-scoped tracker (cost-guard pattern, decision 2). ...(opts.llm - ? { maxCostUsd: opts.budgetUsd } + ? { budgetTracker: opts.budgetTracker, maxCostUsd: opts.budgetUsd } : { extractor: makeGoldExtractor(fixture, gold) }), }); + if (extractResult.budget_exhausted) { + // Partial extraction would silently corrupt every downstream score — + // abort the run loudly instead (CLI maps this to exit 2). + throw new Error( + `brainbench --llm budget exhausted during ${fixture.fixture_id} (spent ~$${(extractResult.spent_usd ?? 0).toFixed(2)}) — raise --budget-usd or drop --llm`, + ); + } const stored = await engine.executeRaw( `SELECT fact, entity_slug, source, source_session, source_markdown_slug @@ -203,13 +222,14 @@ export async function runWriteBack( provenance_accuracy: survived > 0 ? provenanceOk / survived : 1, }; + // Computed in both modes so the harness can aggregate; only SCORED as + // extraction metrics when the real extractor ran (--llm). + const matchedAnyGold = stored.filter((row) => + goldItems.some(({ spec }) => + spec.match_keywords.every((kw) => row.fact.toLowerCase().includes(kw.toLowerCase())), + ), + ).length; if (opts.llm) { - // Extraction quality vs gold, only meaningful when the real extractor ran. - const matchedAnyGold = stored.filter((row) => - goldItems.some(({ spec }) => - spec.match_keywords.every((kw) => row.fact.toLowerCase().includes(kw.toLowerCase())), - ), - ).length; metrics.extraction_recall = goldItems.length > 0 ? survived / goldItems.length : 1; metrics.extraction_precision = stored.length > 0 ? matchedAnyGold / stored.length : 1; } @@ -222,6 +242,7 @@ export async function runWriteBack( survived, provenance_ok: provenanceOk, stored_rows: stored.length, + matched_any_gold: matchedAnyGold, metrics, failed_items: failed, }; diff --git a/src/eval/brainbench/scoreboard.ts b/src/eval/brainbench/scoreboard.ts index 97838a567..ea086d207 100644 --- a/src/eval/brainbench/scoreboard.ts +++ b/src/eval/brainbench/scoreboard.ts @@ -18,11 +18,12 @@ * decimals, keys sorted, receipts excluded (decision 10). */ -import type { - BrainBenchBaseline, - BrainBenchResult, - CompareOutcome, - SuiteMetrics, +import { + round4, + type BrainBenchBaseline, + type BrainBenchResult, + type CompareOutcome, + type SuiteMetrics, } from './types.ts'; /** Gated metrics + their good direction. Anything absent is diagnostic-only. */ @@ -41,11 +42,8 @@ export const GATED_METRICS: Readonly> = { const BASELINE_SCHEMA_VERSION = 1; -function round4(n: number): number { - return Math.round(n * 10000) / 10000; -} - -function cellKey(c: SuiteMetrics): string { +/** The canonical `${harness}/${suite}` cell key — baseline + run-all records share it. */ +export function cellKey(c: Pick): string { return `${c.harness}/${c.suite}`; } diff --git a/src/eval/brainbench/types.ts b/src/eval/brainbench/types.ts index 5d175d34f..5ed5aa76d 100644 --- a/src/eval/brainbench/types.ts +++ b/src/eval/brainbench/types.ts @@ -113,6 +113,14 @@ export interface PublicTurn { ts?: string; } +/** + * The canonical 4-decimal rounding (decision 10 — baseline diff-stability). + * ONE implementation; scoreboard + harness import it (review DRY finding). + */ +export function round4(n: number): number { + return Math.round(n * 10000) / 10000; +} + export function toPublicTurn(turn: FixtureTurn): PublicTurn { const out: PublicTurn = { turn_id: turn.turn_id, role: turn.role, text: turn.text }; if (turn.ts !== undefined) out.ts = turn.ts; diff --git a/test/brainbench-fixtures.test.ts b/test/brainbench-fixtures.test.ts index 3d5ce68d0..0a713a2a3 100644 --- a/test/brainbench-fixtures.test.ts +++ b/test/brainbench-fixtures.test.ts @@ -118,6 +118,18 @@ describe('validator rejection classes', () => { expect(() => validateGold('g.json', badGold, fixture)).toThrow(/no matching fixture turn/); }); + test('retrieval-suite gold with should_retrieve=true and empty gold_slugs rejected (unpassable item)', () => { + const fixture = validateFixture('f.json', structuredClone(BASE_FIXTURE)); + const badGold = structuredClone(BASE_GOLD) as { fixture_id: string; turns: Record> }; + badGold.turns['1'] = { should_retrieve: true, gold_slugs: [] }; + expect(() => validateGold('g.json', badGold, fixture)).toThrow(/must be non-empty/); + delete badGold.turns['1'].gold_slugs; + badGold.turns['1'].gold_facts = [ + { gist: 'x', fact: 'y', entity_slug: null, match_keywords: ['k'] }, + ]; + expect(() => validateGold('g.json', badGold, fixture)).toThrow(/must be non-empty/); + }); + test('gold_facts without match_keywords rejected', () => { const fixture = validateFixture('f.json', structuredClone(BASE_FIXTURE)); const badGold = structuredClone(BASE_GOLD) as { turns: Record> }; diff --git a/test/brainbench-writeback.test.ts b/test/brainbench-writeback.test.ts index 6880337ae..955528a51 100644 --- a/test/brainbench-writeback.test.ts +++ b/test/brainbench-writeback.test.ts @@ -132,6 +132,44 @@ describe('runWriteBack (deterministic, production pipeline)', () => { } }); + test('--llm extraction metrics reach the harness CELLS (review finding: they were dropped in aggregation)', async () => { + __setChatTransportForTests(async (): Promise => ({ + text: JSON.stringify({ + facts: [{ + fact: 'Alice Example flagged the pricing model undercutting gross margin', + kind: 'belief', + entity: 'people/alice-example', + confidence: 1.0, + notability: 'high', + }], + }), + blocks: [], + stopReason: 'end', + usage: { input_tokens: 10, output_tokens: 10, cache_read_tokens: 0, cache_creation_tokens: 0 }, + model: 'stub:stub', + providerId: 'stub', + })); + __setEmbedTransportForTests( + (async () => ({ embeddings: [Array.from({ length: 1536 }, () => 0.1)] })) as never, + ); + try { + const { loadCorpus } = await import('../src/eval/brainbench/fixtures.ts'); + const { runBrainBench } = await import('../src/eval/brainbench/harness.ts'); + const corpus = await loadCorpus('evals/brainbench/fixtures', 'evals/brainbench/gold'); + const sub = { ...corpus, fixtures: corpus.fixtures.filter((f) => f.fixture.fixture_id === 'wb-001-pricing-concern') }; + const out = await runBrainBench(sub, { + harnesses: ['openclaw'], suites: ['write-back'], includeHoldout: true, llm: true, budgetUsd: 1, + }); + const cell = out.cells.find((c) => c.suite === 'write-back'); + expect(cell).toBeDefined(); + expect(cell!.metrics.extraction_recall).toBeCloseTo(1 / 3); + expect(cell!.metrics.extraction_precision).toBe(1); + } finally { + __setChatTransportForTests(null); + __setEmbedTransportForTests(null); + } + }); + test('multi-segment conversations extract per segment (the 45-min gap splits)', async () => { await resetTables(engine); // gen-wb fixtures carry a deliberate >30min gap; use one. diff --git a/test/eval-brainbench-e2e.test.ts b/test/eval-brainbench-e2e.test.ts index 3001b446b..13c5db764 100644 --- a/test/eval-brainbench-e2e.test.ts +++ b/test/eval-brainbench-e2e.test.ts @@ -40,6 +40,15 @@ beforeAll(() => { cpSync(join(REPO, 'evals/brainbench/fixtures', `${id}.fixture.json`), join(fixtures, `${id}.fixture.json`)); cpSync(join(REPO, 'evals/brainbench/gold', `${id}.gold.json`), join(gold, `${id}.gold.json`)); } + // Shared artifacts every consumer test depends on, built ONCE here so each + // test is self-sufficient under -t filters / sharding (review finding: + // order-dependent file production across describe blocks). + const r = run(['--fixtures', fixtures, '--gold', gold, '--update-baseline', join(root, 'base1.json')]); + if (r.exitCode !== 0) throw new Error(`beforeAll baseline build failed: ${r.stderr}`); + const doctored = JSON.parse(readFileSync(join(root, 'base1.json'), 'utf-8')); + const cellKey = Object.keys(doctored.counts).find((k) => doctored.counts[k].gold_total > 0)!; + doctored.counts[cellKey].gold_failed = -1; // pretends fewer failures than any run can match + writeFileSync(join(root, 'doctored.json'), JSON.stringify(doctored, null, 2)); }); describe('exit contract over a multi-brain run (PGLite exitCode-hijack guard)', () => { @@ -56,11 +65,10 @@ describe('exit contract over a multi-brain run (PGLite exitCode-hijack guard)', }, 30_000); test('--update-baseline is byte-deterministic across two runs (decision 10)', () => { - const b1 = join(root, 'base1.json'); const b2 = join(root, 'base2.json'); - expect(run(['--fixtures', fixtures, '--gold', gold, '--update-baseline', b1]).exitCode).toBe(0); expect(run(['--fixtures', fixtures, '--gold', gold, '--update-baseline', b2]).exitCode).toBe(0); - expect(readFileSync(b1, 'utf-8')).toBe(readFileSync(b2, 'utf-8')); + // base1.json was produced by an entirely separate run in beforeAll. + expect(readFileSync(b2, 'utf-8')).toBe(readFileSync(join(root, 'base1.json'), 'utf-8')); }, 60_000); test('--compare against own baseline: exit 0 PASS', () => { @@ -71,14 +79,7 @@ describe('exit contract over a multi-brain run (PGLite exitCode-hijack guard)', }, 30_000); test('doctored main baseline (pretends fewer failures): exit 1 REGRESSION with named breach', () => { - const doctored = JSON.parse(readFileSync(join(root, 'base1.json'), 'utf-8')); - const cellKey = Object.keys(doctored.counts).find((k) => doctored.counts[k].gold_total > 0)!; - doctored.counts[cellKey].gold_failed = Math.max(0, doctored.counts[cellKey].gold_failed - 1); - // also make the run's count strictly greater by raising the bar impossibly - doctored.counts[cellKey].gold_failed = -1 as never; - const path = join(root, 'doctored.json'); - writeFileSync(path, JSON.stringify(doctored, null, 2)); - const r = run(['--fixtures', fixtures, '--gold', gold, '--compare', path]); + const r = run(['--fixtures', fixtures, '--gold', gold, '--compare', join(root, 'doctored.json')]); expect(r.exitCode).toBe(1); expect(r.stdout).toContain('## Gate: REGRESSION'); expect(r.stdout).toContain('newly-failed'); @@ -239,10 +240,11 @@ describe('--json stdout completeness', () => { describe('--llm availability gate', () => { test('no config + no keys: exit 2 with an actionable message, before any run', () => { const bareHome = mkdtempSync(join(tmpdir(), 'bb-home-')); - const env = { ...process.env, HOME: bareHome } as Record; - delete env.ANTHROPIC_API_KEY; - delete env.CLAUDE_API_KEY; - delete env.OPENAI_API_KEY; + // Minimal explicit env (review finding): spreading process.env and + // deleting a hardcoded key list leaks other provider keys (GOOGLE_*, + // per-recipe ${ID}_API_KEY) — on a dev machine that could flip the gate + // open and make REAL API calls from a test. + const env: Record = { PATH: process.env.PATH ?? '', HOME: bareHome }; const proc = Bun.spawnSync( ['bun', 'src/cli.ts', 'eval', 'brainbench', '--fixtures', fixtures, '--gold', gold, '--llm'], { cwd: REPO, env, stdout: 'pipe', stderr: 'pipe' }, @@ -273,13 +275,19 @@ describe('render-brainbench-delta.ts (the CI step-summary block)', () => { }); describe('privacy guard violation branches (negative path)', () => { - test('a fixture with a real dollar amount + out-of-range year fails the scan', () => { + test('a fixture with a real dollar amount + out-of-range year fails the scan (gold dir scanned too)', () => { const dirty = mkdtempSync(join(tmpdir(), 'bb-privacy-')); mkdirSync(join(dirty, 'fixtures'), { recursive: true }); mkdirSync(join(dirty, 'gold'), { recursive: true }); writeFileSync( join(dirty, 'fixtures', 'leak.fixture.json'), - JSON.stringify({ turns: [{ text: 'They raised $50M back in 2019 for the series B' }] }), + JSON.stringify({ turns: [{ text: 'They raised $50M for the series B' }] }), + ); + // The year violation lives in GOLD — pins that the year scan covers the + // gold dir as well (review finding: it previously scanned fixtures only). + writeFileSync( + join(dirty, 'gold', 'leak.gold.json'), + JSON.stringify({ fixture_id: 'leak', turns: { '1': { gold_facts: [{ fact: 'raised back in 2019' }] } } }), ); const proc = Bun.spawnSync(['bash', 'scripts/check-synthetic-corpus-privacy.sh'], { cwd: REPO, @@ -319,10 +327,11 @@ describe('run-all wiring (decision 16) — full corpus, in-process', () => { expect(core.status).toBe('completed'); expect(Object.keys(core.cells ?? {}).length).toBe(12); expect(core.fixtures_hash).toBeDefined(); - // committed baseline matches the committed corpus hash (drift guard) - if (existsSync('evals/brainbench/baselines/main.json')) { - const baseline = JSON.parse(readFileSync('evals/brainbench/baselines/main.json', 'utf-8')); - expect(baseline.fixtures_hash).toBe(core.fixtures_hash); - } + // Committed baseline matches the committed corpus hash (drift guard). + // UNCONDITIONAL (review finding): a conditional existsSync would turn a + // deleted baseline into a silent no-op instead of a failure. + expect(existsSync('evals/brainbench/baselines/main.json')).toBe(true); + const baseline = JSON.parse(readFileSync('evals/brainbench/baselines/main.json', 'utf-8')); + expect(baseline.fixtures_hash).toBe(core.fixtures_hash); }, 120_000); });