Scale skill evals: compare mode, per-prompt budgets, efficiency grader (uv + shared lib)

[Bwvolleyball]

Summary

Adds three capabilities to the skill-eval system, built on a uv-managed shared library so every future grader improvement lands once instead of per-harness:

• Compare mode (uv run compare) — runs each should_trigger prompt with and without the skill loaded and reports the verdict + cost delta. This is the missing "did the skill actually help?" signal: previously a skill that never fired and a useless skill graded identically.
• Per-prompt expected behavior + budgets — prompts.csv → prompts.yaml; each prompt can carry an expected list (signal / anti_pattern / check_id) and a budget (cost_usd / bash_commands / output_tokens / wall_seconds). Process-checks can be scoped to specific prompts via applies_to.
• Three-state efficiency grader — PASS / PASS-SLOW / FAIL. A correct run that breaches a budget grades PASS-SLOW, catching over-orchestration without losing the "it worked" signal.

Supporting scaffolding:

• uv project + unified CLI: uv run evals | compare | regrade | validate.
• Pydantic strict-validated config (extra="forbid") — config typos become load-time errors naming the offending prompt, before any API spend.
• Replay-from-trace (uv run regrade <trace.jsonl>) — regrade a saved run with zero API cost; the iteration loop for the CI-only testbed.
• Shared lib evals/lib/ (models, config, grading, harness, replay, compare, reporting) + claude-code adapter. The 6 old run-evals.py scripts become thin shims into the CLI (−2447 lines of duplicated parsing/grading).
• Tiered CI: every PR runs keys-free validate + pytest + a cheap claude-code/Haiku run; merge-to-main runs the full Sonnet+Opus+Haiku matrix. codex/agy/cursor are dispatch-only until their adapters land.

Test Plan

Verified locally (free layers):

• uv run pytest — 39 unit tests (models, config, grading incl. all check types + 3-state verdict, harness parse/trigger, replay, compare lift, reporting)
• uv run validate — both skills' prompts.yaml + process-checks.json parse & validate
• uv run regrade tests/fixtures/hw-07.trace.jsonl --skill hawkscan → PASS
• actionlint clean on the workflow

Exercised by this PR's CI (first live agent run):

• eval-claude-code (Haiku, hawkscan + api) — the live launch() → claude -p path
• PR comment posts trigger-accuracy + process-score table

Notes / follow-ups

• Requires ANTHROPIC_API_KEY repo secret for eval-claude-code; the two free jobs pass without it.
• This PR is the first end-to-end run of the live agent path — local verification covered parsing/grading/validation/replay only.
• Future work (documented, non-blocking): per-platform adapter.py for codex/cursor/copilot/agy (dispatch-only until then); rubric grading lives only in the manual harness (not yet ported to the lib); output_tokens/wall_seconds budget axes are graded but not yet populated by the claude-code adapter.

🤖 Generated with Claude Code

[claude]

Claude finished @Bwvolleyball's task in 3m 0s —— View job

---

Claude Code Review

• Gather context and understand the PR
• Read changed files and diff
• Review correctness, security, performance, testing, conventions
• Post final verdict

---

⚠️ Needs Attention

Solid, well-tested refactor. The shared lib, strict Pydantic config, replay-from-trace, and 39-unit-test suite are all real improvements. Three issues from the previous round are confirmed fixed.

✅ Fixed from prior review

• Codex duplicate --sandbox — correctly branches on CI env var now
• Cursor CURSOR_API_KEY CLI exposure — injected via environment, never on the command line
• codex/agy/cursor running on every PR — workflow is workflow_dispatch-only

Remaining warnings

• evals/lib/compare.py:21-29 — compare_skill() has no per-prompt try/except; a single adapter.launch() failure aborts the entire compare run and produces a partial lift.json. The cli.main() loop already handles this correctly — compare should mirror it. Fix this →
• evals/lib/reporting.py:217-219 — render_job_summary() leaves why blank when a blocking process-check fails but trigger_correct=True. The _fail_reason() helper in the same file already handles this with "blocking check failed" — use it. Fix this →
• evals/harnesses/agy/adapter.py — load_skill is a no-op for agy; both with and without compare arms run the same globally-installed skill, so compare always reports effect: "none" for agy. At minimum add a note; ideally filter agy from compare runs.
• evals/harnesses/codex/adapter.py:82-87 — seen set deduplicates item.started events by command string, so a command that legitimately runs twice is only counted once — under-counting bash_commands for budget checks.
• evals/cli.py:102-103 — import json and from pathlib import Path inside compare() are redundant; both already imported at module level.

[claude]

[critical] --sandbox is specified twice with conflicting values. workspace-write is always added on line 128, then when full_auto=False (the default), read-only is appended on line 134. The resulting command is:

codex exec --json --sandbox workspace-write --skip-git-repo-check --sandbox read-only <prompt>

Depending on how Codex resolves duplicate flags this will either use the wrong sandbox level or error out. Fix by making it conditional:

sandbox = "workspace-write" if full_auto else "read-only"
cmd = ["codex", "exec", "--json", "--sandbox", sandbox, "--skip-git-repo-check"]

Fix this →

[claude]

[warning] The CURSOR_API_KEY is passed as a command-line argument, making it visible in process listings (ps aux, /proc/<pid>/cmdline). Prefer injecting secrets via environment variable rather than --api-key <value>. The cursor CLI likely reads CURSOR_API_KEY from the environment already (consistent with how ANTHROPIC_API_KEY is used in the claude-code adapter — never passed on the command line there).

[claude]

[warning] The agy adapter documents load_skill as a no-op because skills are installed globally via agy plugin install in CI. This means compare_skill() cannot actually test "without skill" for agy — both the load=True and load=False runs use the same globally-installed skill, so the compare will always report effect: "none" for this platform, producing misleading lift data. Consider either: (a) filtering agy out of compare runs until per-run skill toggle is supported, or (b) emitting a warning in compare output when the adapter returns noop for load_skill=False.

[claude]

[warning] codex, agy, and cursor jobs all run on every PR (github.event_name == pull_request), but they produce empty results because the API keys are not set for PR builds. The existing CI comment shows "No results" for all three platforms. Each job installs a CLI, fails at plugin install, then exits — burning ~30 CI min per PR (3 platforms × 2 skills). Consider gating them to workflow_dispatch only until the adapters and API keys are ready.

[claude]

[warning] compare_skill() has no per-prompt exception handling. If adapter.launch() throws (e.g. subprocess failure, disk error), the entire compare run aborts mid-loop with an unhandled exception rather than recording the failure for that prompt and continuing. The evals CLI wraps each prompt in try/except for exactly this reason — compare should do the same:

for p in prompts:
    graded = {}
    for load in (True, False):
        try:
            run = adapter.launch(...)
            did = adapter.detect_trigger(run, skill)
            graded[load] = grade(p, run, cfg.checks, ...)
        except Exception as e:
            graded[load] = grade(p, ParsedRun(error=str(e)), cfg.checks, ...)

Note: the CI step already uses || true so a full abort won't break the workflow, but a half-written lift.json artifact will be confusing.

[claude]

[warning] When trigger_correct=True but a blocking process check fails (verdict = FAIL), the why column in the table will be empty — the user sees ❌ FAIL with no explanation. The condition chain falls through to the empty string branch:

why = "; ".join(r.budget_breaches) if r.budget_breaches else (
    "" if r.trigger_correct else          # ← empty when trigger correct but proc-check failed
    ("false-positive" if r.did_trigger else "false-negative"))

Consider adding a fallback for this case:

failed_checks = [c.id for c in r.process_checks if not c.passed and c.severity == "blocking"]
why = "; ".join(r.budget_breaches) or       ("; ".join(failed_checks) if r.verdict.value == "fail" else "") or       ("" if r.trigger_correct else ("false-positive" if r.did_trigger else "false-negative"))

[claude]

[suggestion] json and Path are already imported at the top of cli.py — these in-function imports are redundant dead weight. Remove them.

[claude]

⚠️ Claude Code Review — Needs Attention

Substantial, well-architected PR. The shared evals/lib/ refactor is clean, Pydantic strict-validated config is a real safety improvement, and the 39-test suite gives good coverage across models/grading/reporting. Three issues from the previous review round are confirmed fixed (codex duplicate --sandbox, cursor API key CLI-exposure, and CI jobs running on every PR). A few warnings remain open.

✅ Fixes confirmed from prior review round

• Codex duplicate --sandbox (evals/harnesses/codex/adapter.py) — now correctly branches on CI env var; --dangerously-bypass-approvals-and-sandbox in CI, conditional workspace-write / read-only locally.
• Cursor CURSOR_API_KEY CLI exposure (evals/harnesses/cursor/adapter.py) — key now injected via env=dict(os.environ), never on the command line.
• codex/agy/cursor running on every PR (skill-evals.yml) — workflow is now workflow_dispatch-only; free jobs (validate, pytest) are the only automatic gates.

🚨 Issues Found

[warning] evals/lib/compare.py:21-29 — compare_skill() has no per-prompt exception handling. If adapter.launch() raises mid-loop (subprocess failure, disk error, timeout not wrapped), the entire compare run aborts with an unhandled exception, producing a half-written or missing lift.json artifact. cli.main() wraps each prompt in try/except for exactly this reason; compare_skill() should do the same. The || true in the CI step masks the abort but leaves the artifact in an indeterminate state.

[warning] evals/lib/reporting.py:217-219 — render_job_summary() leaves the why column empty when a blocking process-check fails but the trigger fired correctly (i.e., trigger_correct=True, budget_breaches=[], verdict=FAIL). The condition chain short-circuits to "" in that branch. The _fail_reason() helper in the same file handles this gracefully with a "blocking check failed" fallback — render_job_summary should use the same logic. A user sees ❌ FAIL with no explanation in the table.

[warning] evals/harnesses/agy/adapter.py / evals/lib/compare.py — load_skill is documented as a no-op for agy (skills are globally installed). This means both the load=True and load=False compare arms run against the same globally-installed skill, so compare will always report effect: "none" for agy — the lift measurement is meaningless. Consider either filtering agy out of compare runs or emitting a warning in compare_skill() when the adapter doesn't support per-run skill toggling.

[warning] evals/harnesses/codex/adapter.py:82-87 — The seen: set() deduplication for item.started events means a command that legitimately runs twice (e.g., two hawk scan invocations in a rescan flow) is only counted once. This under-counts bash_commands for budget checks, potentially letting a prompt pass a bash_commands budget it actually violated.

Suggested before merge

• Add try/except in compare_skill() loop — mirror the resilience already in cli.main().
• Fix the empty why in render_job_summary for blocking-check failures — use _fail_reason(r) or add the "blocking check failed" fallback.
• Remove the redundant import json / from pathlib import Path at the top of compare() in evals/cli.py:102-103 (both already imported at module level).
• Document or guard the agy compare limitation — a # NOTE: load_skill is a no-op; compare results for agy are always "none" comment in compare_skill() at minimum.