evals: offline regression harness over verified review misses - #81
evals: offline regression harness over verified review misses#81heskew wants to merge 5 commits into
Conversation
Replays fixtures (metadata-only YAML in ai-review-log, pinned by base/reviewed SHAs) against a chosen prompt ref + model: real checkout at the reviewed head, production-composed layer scope, claude -p review, cheap-model judge scoring caught/partial/missed against the known defect. Baseline file turns later runs into a regression gate. Motivated by the lgtmaybe evals/ pattern: calibration PRs currently ship blind and wait a week for production verdicts; this answers "does the new text catch the misses that motivated it?" in minutes. First full run doubles as the offline measure of how many historical misses the post-#73/#80 rules already catch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Smoke-test findings on the first end-to-end run: - macOS has no GNU timeout; run_with_timeout falls back to gtimeout/perl-alarm. - The shared-worktree design leaked THE FUTURE: the cache clone carries post-review refs, so git log --all exposed the fix commit (crediting the peer catch) and the reviewer treated the defect as already-handled. Eval checkouts are now standalone single-branch clones containing only the reviewed commit ancestry, remote removed. With isolation, the smoke fixture (whitespace Retry-After) scores caught with the exact mechanism. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Reviewed; no blockers found. |
There was a problem hiding this comment.
Code Review
This pull request introduces an offline regression evaluation harness for review prompts, adding a test runner script (run-eval.sh), judging instructions, repository configurations, and documentation. The feedback focuses on improving the robustness of the shell script, specifically by using jq instead of sed for parsing JSON judge outputs, and using awk instead of grep for exact string matching of fixture IDs in the baseline file.
10/17 caught, 4 partial, 3 missed. Every fixture was a total miss at its original ref, so this is the offline measure of the #70..#80 calibration arc: 59% of historical misses now caught outright, 82% at least surfaced. Survivors (the next calibration targets): harper-1858 (config-mirroring gap), harper-1906 (test-validity race), oauth-170 (cleartext issuer + JTI replay). Two fixtures vote-split across runs (1770, 1956 — recorded at their best-evidenced verdict; flakiness noted in the PR). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
First full-corpus baseline is in (dc2f92d, Every fixture was a total miss at its original ref, so this doubles as the offline measure of the #70→#80 calibration arc: 59% of historical misses now caught outright, 82% at least surfaced. Highlights: the stale-context.user CRITICAL (harper#1535), the chain-abort leak, the nested-SQL authz gap, and all three producing-side oauth cases (#183/#184/#192 — #80's brand-new rule going 3-for-3) now score Survivors — the next calibration targets:
Flakiness note: Run conditions: reviewer claude-sonnet-5, judge claude-haiku-4-5, working tree ≈ 224c2ad, ~2-3 min/fixture sequential. |
…act-match baseline gate Addresses dogfood + gemini-code-assist review on #81. The sed-based JSON extraction was demonstrably the cause of the empty evidence strings in the baseline runs (pretty-printed judge output). Flat-object grab + jq now handles fenced/pretty/escaped output; eval clones are reclaimed on clean verdicts; repo cache keys use the full org/name; baseline lookups use awk exact match instead of grep -E on ids. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Review disposition (9175f61) — all five findings applied:
(Three inline threads could not take replies — their comment IDs were superseded when the dogfood review re-ran on the baseline push — so dispositions are recorded here; all threads resolved.) |
heskew
left a comment
There was a problem hiding this comment.
Codex (OpenAI) review: four confirmed findings are posted inline. The existing checks are green, but the current Script tests do not exercise evals/run-eval.sh.
Four confirmed findings from the Codex review pass: - P1 startup surface: claude -p in a historical PR checkout loaded project .claude settings/hooks and project MCP — executable content from the fixture with the evaluator's credentials, before --allowedTools applies. Now --setting-sources user + --strict-mcp-config. - P1 future leak via tags/objects: --single-branch still imports tag refs, and local-path clones share the whole object store. Now --no-tags --no-local. - P2 stale --ref reruns: checkout <branch> reused the stale local branch after fetch; now resolves origin/<ref> and checks out detached, with slash-safe cache keys. - P1 gate holes: empty glob, ERROR/UNPARSEABLE rows, missing baseline file, and baseline fixtures absent from the run all exited 0; each now fails with a distinct error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The lgtmaybe-inspired eval harness: replay verified production review misses against a prompt ref + model, judge whether the review now catches each known defect. Calibration PRs currently ship blind and wait a week for production verdicts; this answers "does the new text catch the misses that motivated it?" in minutes.
Shape
evals/run-eval.sh— per fixture: standalone clone at the reviewed head (single-branch, ancestry-only — see below), production-composed layer scope at the ref under test (compose-review-scope.shreused),claude -pagentic review with read-only tools, cheap-model judge (judge-prompt.md) scoring caught/partial/missed, TSV results + optional baseline regression gate.evals/repos.yml— per-repo layer sets mirroring the fleet callers.Smoke-test findings (already fixed in this PR)
The first end-to-end run exposed an eval-design trap worth recording: a shared worktree leaks the future —
git log --allsees post-review refs including the fix commit, and the reviewer treats the defect as already-handled. Checkouts are now single-branch clones of the reviewed commit's ancestry with the remote removed. With isolation, the smoke fixture (harper#1770 whitespace Retry-After) scorescaughtwith the exact mechanism at the current ref — the numeric-coercion rule demonstrably working.Fidelity caveats (v1, documented in the README)
No PR title/body/comments context; single
claude -ppass rather than the full action harness; shared layers only (no caller repo-specific-checks). Good enough to regression-gate prompt edits; not a substitute for production calibration.Next
evals/baseline.tsvin a follow-up.Companion: ai-review-log fixtures PR.
🤖 Generated with Claude Code