Skip to content

review: per-PR live A/B workflow (phase 4)#237

Merged
jwbron merged 43 commits into
mainfrom
jwbron/live-eval-ab-ci
Jul 10, 2026
Merged

review: per-PR live A/B workflow (phase 4)#237
jwbron merged 43 commits into
mainfrom
jwbron/live-eval-ab-ci

Conversation

@jwbron

@jwbron jwbron commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Phase 4 of the live A/B eval plan (#232), stacked on the Phase 3 runner (#236): the Review Eval A/B workflow.

Behavior

  • Triggers: every non-draft PR touching workflows/review/** (opened/synchronize/reopened/ready_for_review), plus workflow_dispatch with base_ref, max_usd, and full inputs.
  • Runs live-ab.ts against the PR's merge-base (origin/<base>; fetch-depth: 0 so git show can read the baseline review.md), posting the delta report three ways: a sticky PR comment (hidden-marker upsert, edited in place on later pushes), the job summary, and an uploaded artifact (if: always(), so a partial or gate-failing run still surfaces what it wrote).
  • Scope and cost: per-PR runs cover only live cases also tagged smoke (the runner gains --smoke-only); a full-eval label or the dispatch input lifts that to all live cases. The runner's own budget cap (default $40 total, half per arm) reports skipped cases rather than dying. Per-PR concurrency cancels superseded runs on a new push.
  • Opt-outs and guards: skip-live-eval label skips entirely; drafts wait until ready-for-review; the changeset-release/main branch is excluded (it matches the path filter via workflows/review/package.json but changes no behavior); a missing ANTHROPIC_API_KEY (fork PRs) skips green, same pattern as review-eval-full.yml.
  • Gate: report-only except the runner's exit code, which is non-zero only when the candidate arm mishandles an adversarial-injection case (the playbook's standing rule). Metric deltas never fail the job until thresholds are ratified (plan's open human decision).

The workflow name is distinct from every gh-aw workflow per the operational-floor rule (same-named workflows share a gh-aw concurrency group per PR and cancel each other).

Test plan:

  • YAML parses; pnpm run test --run (696) and typecheck/lint stay green with the --smoke-only and markdown-sibling additions to live-ab.ts.
  • Real validation requires the repo secret: after this stack merges (or via workflow_dispatch on the branch), the acceptance runs from the plan are (1) a PR editing only a review.md comment produces a near-zero-delta report comment, and (2) a PR deliberately weakening a reviewer instruction shows a recall regression. Those two runs should be linked here before undrafting.

Next steps (human)

  1. Labels DONE: full-eval and skip-live-eval exist in this repo (created 2026-07-09).
  2. Secret DONE: confirmed working by the acceptance runs below.
  3. Acceptance DONE and PASSED, run from the two (now closed) do-not-merge PRs [do-not-merge] live A/B acceptance: control (comment-only) #239 and [do-not-merge] live A/B acceptance: weakened reviewer (recall signal) #240: the control (comment-only edit, run 29052332224) held recall and verdict agreement flat at +0% with the adversarial gate passing, and the weakened-reviewer arm (run 29052334404) showed recall -17% with the lost spec named (incident-money-rounding:money-fp-rounding-1) and verdict agreement -14%. ~26 min and ~$7.70 per full run. Details are in the plan's acceptance section (review: add the live A/B eval plan #232) and the evidence comment below.
  4. Decide the default budget: $40 total per run is the shipped default; at a few review-bot PRs a week that is bounded, but it is your spend to ratify.
  5. Cross-check workflow-name distinctness once more against any consumer-repo gh-aw workflows that could run on this repo's PRs (the shipped name Review Eval A/B collides with nothing in Khan/actions today).

Update 2026-07-09: baseline semantics documented

The workflow passes the base-branch tip while its prose said merge-base. The code was right: pull_request runs check out the PR merge commit, so the candidate tree already contains the base tip, and baselining on that same tip isolates the PR's own review.md delta (a merge-base baseline would fold upstream review.md movement into the measured numbers). Prose updated to match; no behavior change.

jwbron added 5 commits July 9, 2026 12:47
…live cases

Phase 1 of the live A/B eval plan (#232). The corpus gains
an opt-in live block carrying what a real model run needs and the
deterministic replay does not:

- prContext (PR title/description/author/base; the description is
  untrusted author text, so an adversarial case can carry its payload
  there or in the diff),
- a post-change file tree on disk next to the case, via a new
  <id>/case.json + <id>/tree/ layout that coexists with flat <id>.json
  (a directory containing case.json is one case; its tree is never
  parsed as corpus JSON), and
- labeled defect specs (mustCatchSpecs / mustNotFlagSpecs: path, line
  window, mechanism keyword alternates). Live runs choose their own
  finding ids, so ground truth matches on anchor window + mechanism
  rather than id.

The loader enforces the invariants: the live tag and the live block
imply each other, a live case needs a cleanly-parseable diff, spec
paths must appear in changedFiles and the diff, and every non-removed
changed file must exist in the tree. loadLiveCorpus() returns the
subset. The live half lives in corpus/live.ts; loader.ts re-exports it
as the single public surface.

Ten cases are converted with hand-authored real diffs and trees: five
smoke incidents, both clean cases, one adversarial injection whose
payload is a code comment in the diff, one golden holdout, and one
synthetic mutation. Their recorded line anchors are rewritten to the
authored defect lines (natural files beat content padded to synthetic
line numbers), which activates the provenance gate on these cases in
the deterministic suite; all expectations hold unchanged.
…action and case staging

Phases 2a/2b of the live A/B eval plan (#232), stacked on the Phase 1
corpus format (#233).

agent-extract.ts turns review.md's '## agent:' sections into data
(name, description, pinned model, prompt body). It takes the markdown
as a string with no fs access, because the baseline arm of an A/B
reads the merge-base review.md via git show. Parsing is strict; a
malformed or model-less section throws listing every problem, since a
silently dropped agent would skew an eval arm without failing it. An
integration test extracts the real review.md (21 agents) and pins the
staging-root reference so a future rename fails a test instead of
silently staging nothing.

live-stage.ts materializes the production staging layout for one
live-enabled corpus case: pr-context.json (review.md Step 1's shape,
synthetic identity fields), full.diff / pr.diff / full-stripped.diff
(all the case diff; corpus diffs carry no generated files and no
pattern-triage pass runs), files.json + review-files.json with the
hasPatch cross-check derived from the diff parse, provenance.json,
routing.json from the deterministic router, an out/ directory, and
the post-change checkout copied from the case tree.
rewriteAgentPrompt swaps the production staging root for the staged
context dir. Everything sits behind an injected-fs seam and is
memfs-tested.

Model dispatch (phase 2c) is deliberately absent; it needs the Agent
SDK dependency decision and arrives separately.
… runner (phase 2c)

live-producer.ts runs the live roster over one staged case behind an
injected LiveAgentRunner seam (the judge.ts pattern), so its logic is
stub-tested with zero model calls. Roster: the default finders
(correctness-reviewer, skill-auditor) plus the router's lensesToSpawn;
no pattern-triage or thread-reconciler in eval. It maps all three
sub-agent output contracts into the shapes the deterministic runner
consumes: label-shape findings (correctness lens, or conventions for
the skill-auditor so labelForFinding reproduces the best-practice
variants; confidence defaulted to 0.7 per the production claims rule),
structured-schema lens findings validated as-is, and the validator's
{claims: [...]} verdicts into CaseVerification[]. It stages
claims.json for the validator, resolves {{#runtime-import}} directives
against the case tree (a case opts into a skills index by carrying the
file), retries once on malformed output with the rejection fed back,
keeps partial results when an agent fails twice, prefixes colliding
finding ids, and reports per-agent cost/turns/wall-clock.

live-runner.ts is the one module that touches a real model runtime:
an Agent SDK query() per dispatch with Read/Grep/Glob only, cwd pinned
to the staged checkout, the agent's pinned model, and hard turn and
wall-clock caps; plus the CLI smoke entry
(tsx live-runner.ts --case <id>, requires ANTHROPIC_API_KEY). The
investigation-cap CLI the prompts reference is not staged; its own
denied-budget fallback applies. Adds @anthropic-ai/claude-agent-sdk
as a dev dependency.
live-match.ts scores a live run against a case's labeled defect
specs: a posted candidate satisfies a spec when its anchor agrees
with the spec's path (and line window when both carry one) and any
mechanism alternate matches the finding's failure_scenario or prose;
each candidate satisfies at most one spec and vice versa. An injected
fallback arbiter (hard-capped, same-file only, recorded as
via: fallback for audit) can rescue recall on vague prose; false
flags are decided by the deterministic rule alone. computeLiveMetrics
aggregates recall, verdict agreement, clean false-flag (including a
clean case that blocks), and noise.

live-ab.ts is the arm orchestrator and CLI: baseline review.md from
git show <merge-base>, candidate from the working tree, both arms
over the same live corpus with everything else (corpus, lib, runner,
metrics, judge) from the candidate, per the plan's settled decision
to isolate the model seam. Each arm runs under half the --max-usd
budget with sticky exhaustion: once spend plus the running per-case
average crosses the cap, remaining cases are recorded skipped and
the report still emits. Spec-level regressions are diffed only over
cases both arms scored. Judge scoring reuses the pinned judge
(quality aggregates only; judge-vs-ground-truth disagreement keys on
recorded ids a live arm does not use); the fetch model moves to
judge-live-model.ts and live-judge.ts now imports it. runner.ts
gains an optional RunOptions.validation override so a live
validator's output replaces the recorded block.

Report-only except the standing rule: adversarial-injection failures
on the candidate arm exit non-zero.
Review Eval A/B runs on every non-draft PR touching
workflows/review/** (and on workflow_dispatch): both review.md arms
over the live corpus via live-ab.ts, with the delta report posted as
a sticky PR comment (hidden-marker upsert), appended to the job
summary, and uploaded as an artifact even on partial or gate-failing
runs. Per-PR scope is the smoke-tagged live subset; the full-eval
label or dispatch input lifts it, skip-live-eval opts out, drafts
wait until ready, the changeset-release branch is excluded (it
matches the path filter via package.json but changes no behavior),
secretless runs skip green so fork PRs never fail, and per-PR
concurrency cancels superseded runs. The workflow name is distinct
from every gh-aw workflow per the operational-floor rule about shared
concurrency groups. live-ab.ts gains --smoke-only and writes
out/live-ab-report.md alongside the JSON for the comment step.
@jwbron jwbron self-assigned this Jul 9, 2026
@changeset-bot

changeset-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 9baccb7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
review Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

jwbron added 4 commits July 9, 2026 13:48
… and vitest

Tree directories are byte-exact case fixtures paired with each case's
diff: prettier auto-formatting one would silently desync it from the
diff the provenance gate parses, and a tree may carry a *.test.ts
whose tests fail by design (test-adequacy cases), so vitest must not
execute them either. CI's lint job caught the first drift (prettier
wanting to rewrap a fixture ternary).
…rpus' into jwbron/live-eval-producer-staging
jwbron added 8 commits July 9, 2026 14:21
…s with the case id

Live agents choose their own finding ids, so every case's first
correctness finding was live-correctness-reviewer-1; ids were unique
within a case but collided across cases, and judge.ts's score join
requires arm-global uniqueness. Caught by the first real A/B run
(both arms completed, then judge aggregation threw). Ids are now
<caseId>:<id> from the moment of parsing, so claims.json, the
validator round-trip, the matcher, and the judge all see the same
namespaced id.
…eport instead of killing it

The first real A/B run spent both arms' budgets and then died in
judge aggregation, writing no report: the exact
everything-spent-nothing-posted failure mode the plan forbids. Judge
scoring is additive, so a per-arm failure is now caught, recorded as
judgeError on the arm, rendered as a degradation note in the report,
and the run proceeds to write JSON + markdown and evaluate the
adversarial gate as usual.
@jwbron

jwbron commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Both acceptance runs from the test plan are done and PASSED; this satisfies next-steps item 3.

  • Control ([do-not-merge] live A/B acceptance: control (comment-only) #239, run 29052332224): identical prompts, zero recall/verdict delta, adversarial gate passed. Measured noise floor: about 5 points on the noise metric, about 0.1 on judge mean quality, roughly $7.75 and 13 minutes per PR run (both arms, 7 smoke-live cases).
  • Signal ([do-not-merge] live A/B acceptance: weakened reviewer (recall signal) #240, run 29052334404): weakened correctness reviewer detected exactly as designed: recall 83% to 67% with the lost spec named, verdict agreement 100% to 86%, adversarial gate still passing. Notably, judge mean quality ROSE on the regressed arm (fewer, surer comments score better per comment): the recall-against-specs metric is load-bearing, judge quality alone is not.

The runs also validated the two earlier fixes live (case-namespaced finding ids; judge failures degrading instead of killing the report) and surfaced two follow-ups:

  1. incident-sql-missing-index:dm-missing-index-1 was missed by all four arms. The case's recorded finding is a data-migrations LENS finding, but the converted live cases carry no routerConfig lens rules, so the live roster never spawns the specialist. Fix: add per-case routerConfig lens rules to the live incident cases mirroring the lens each recorded finding came from (a review: live-enabled corpus format and ten live cases #233-corpus data change).
  2. The claim-validator failed on incident-cache-missing-key in the baseline arm of BOTH runs, and the report's per-case table only names the failed agent, not its failed reason. Fix: carry the failure message into perCase so the next occurrence is diagnosable from the report alone (a review: the live A/B runner (phase 3) #236 change), then investigate the validator failure itself.

jwbron added 4 commits July 9, 2026 15:20
…ive case

The first live acceptance runs missed
incident-sql-missing-index:dm-missing-index-1 in all four arms: the
recorded finding is a data-migrations LENS finding, but live cases
carried no routerConfig lens rules, so the live roster never spawned
the specialist that catches it. Every live case whose recorded
finding belongs to a specialist lens now routes that lens on the
finding's file (seven cases across five lenses), mirroring how a
consumer ROUTING file would route the same paths in production.
…rpus' into jwbron/live-eval-producer-staging
… the A/B report

The acceptance runs surfaced a claim-validator failure that the
report could only name, not explain (perCase carried agent names
only, and the PerAgentReport.failed detail never reached the
markdown). perCase failedAgents entries are now '<agent>: <reason>',
so the next failure is diagnosable from the sticky comment alone.
@jwbron jwbron marked this pull request as ready for review July 10, 2026 18:36
@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Review live A/B

No reviewable delta: review.md is byte-identical in both arms (baseline origin/main, sha 5ea0f5d4e3c1), so the extracted prompts and the orchestrator body match and no arms were run. Pass --force-arms for a deliberate wobble control.

@khan-actions-bot khan-actions-bot requested review from a team, kevinb-khan and somewhatabstract and removed request for a team July 10, 2026 18:36
@jeresig jeresig requested review from jeresig and removed request for kevinb-khan and somewhatabstract July 10, 2026 19:05
@khan-actions-bot khan-actions-bot requested review from a team and kevinb-khan and removed request for a team July 10, 2026 19:48
name: Review Eval A/B

on:
pull_request:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it seems like we're running this once per PR here in this repo? (maybe twice if we start as a draft?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slightly more than that: it runs on every push to a non-draft PR touching workflows/review/**, with cancel-in-progress superseding a stale run on a new push. Drafts skip entirely until ready_for_review, so starting as a draft does not add a run. Each run is the smoke subset (7 live cases per arm), measured at ~$7.70 and ~26 minutes in the acceptance runs; skip-live-eval opts a PR out, full-eval widens to the whole live corpus, and the runner enforces a hard USD ceiling either way. If per-push proves too spendy we can gate on a label instead, but per-push is what makes it a tripwire between a prompt edit and production.

@github-actions

Copy link
Copy Markdown
Contributor

Review Guidance

github-actions (2 files)
File Reason
review-eval-ab.yml New workflow under .github/workflows/** with a permissions block, ANTHROPIC_API_KEY secret usage, pull_request/workflow_dispatch triggers, and an inline github-script step that posts PR comments — its if:/expression/secret logic gates whether the reviewer CI behaves correctly.
live-ab.ts The eval-suite runner that decides arm-to-arm live results; the new --smoke-only filter is the per-PR cost gate and a wrong predicate would run the full paid corpus or zero cases.

@khan-actions-bot khan-actions-bot requested review from a team and jaredly and removed request for a team July 10, 2026 20:04
if [ "$FULL_EVAL" = "true" ]; then
SCOPE=""
fi
pnpm dlx tsx workflows/review/eval/live-ab.ts \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought (non-blocking): The header and PR description say the job fails only when the adversarial gate trips, but the runner also exits non-zero from its top-level catch and the "no case scored on the candidate arm" path (live-ab.ts:708-710, 720-723). A transient Anthropic API outage, git failure, or budget exhaustion before any case scores would red this check on a PR whose change is fine. Consider distinguishing the adversarial-gate exit from infra errors, or documenting the caveat.


on:
pull_request:
types: [opened, synchronize, reopened, ready_for_review]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (non-blocking): types has no labeled/unlabeled, so the full-eval and skip-live-eval labels only take effect on the next push, not when the label is added. If immediate effect is intended, add those event types and gate the job to only proceed on them when github.event.label.name is one of the two eval labels (otherwise every unrelated label re-runs a paid eval). Otherwise, documenting "labels apply on the next push" sets the right expectation.

if-no-files-found: ignore
- name: Post the sticky PR comment
if: always() && github.event_name == 'pull_request'
uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (non-blocking): The other hand-authored workflows pin github-script to v8 (node-ci.yml:59, release.yml:16, SHA ed597411...); this step uses v7 (60a0d83...). Aligning avoids running an older major and a third distinct version of the same action.

Suggested change
uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7
uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8

const allCases = loadLiveCorpus();
const allCases = loadLiveCorpus().filter(
(c) =>
!process.argv.includes("--smoke-only") ||

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (non-blocking): The --smoke-only predicate is the per-PR cost gate but lives inline in the unexported main(), so it has no direct test (only SMOKE_TAG/tag-filtering are covered, in corpus/loader.test.ts). Extracting it to a small exported helper would make it a one-line vitest case:

export const selectLiveCases = (cases: CorpusCase[], smokeOnly: boolean): CorpusCase[] =>
    cases.filter((c) => !smokeOnly || c.tags.includes(SMOKE_TAG));

ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}
# The tip of the PR's base branch (see the header comment for why
# tip, not merge-base). workflow_dispatch may override it.
BASE_REF: ${{ inputs.base_ref || format('origin/{0}', github.base_ref || 'main') }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (non-blocking): BASE_REF=origin/<base> is resolved at fetch time, not when the PR merge commit was created, so it can drift from the base tip the candidate tree actually contains if the base branch moves before this job fetches — the upstream-movement bias the header says the design avoids. On pull_request events, HEAD^1 (the merge commit's base parent) is the race-free tip. A minor edge case, but the header's rationale rests on it. (Also: the PR description says "against the PR's merge-base", which reads as contradicting the header's deliberate tip-not-merge-base choice.)

# Conflicts:
#	workflows/review/eval/corpus/live.ts
Base automatically changed from jwbron/live-eval-ab-runner to main July 10, 2026 21:23
jwbron added a commit that referenced this pull request Jul 10, 2026
Phase 3 of the live A/B eval plan (#232), stacked on the Phase 2 producer (#234): score live runs against labeled ground truth and diff two review.md versions arm to arm.

## 3a: `eval/live-match.ts`

Live runs choose their own finding ids, so the recorded metrics (which key on `expected.mustCatch` ids) cannot score them. The matcher maps a run's POSTED candidates onto the case's labeled defect specs: a candidate satisfies a spec when its anchor agrees with the spec's path (and line window, when both carry one) and any mechanism alternate matches the finding's `failure_scenario` or prose, case-insensitively. Each candidate satisfies at most one spec and each spec at most one candidate, so one comment cannot claim two defects. An injected fallback arbiter (hard-capped, same-file candidates only, recorded as `via: "fallback"` for human audit) can rescue recall on vague prose; false flags are decided by the deterministic rule alone. `computeLiveMetrics` aggregates the live analogues of the suite's numbers: must-catch recall, verdict agreement, clean false-flag (a clean case that blocks counts), and noise (posted candidates matching no spec).

## 3b: `eval/live-ab.ts`

The arm orchestrator and CLI. Baseline `review.md` comes from `git show <merge-base>` (or `--base-ref`), candidate from the working tree; both arms run over the same live corpus with everything else (corpus, `lib/`, runner, metrics, judge) from the candidate, per the plan's settled decision to isolate the model-behavior seam. Each arm runs under half the `--max-usd` budget (default 40 total) with sticky exhaustion: once spend plus the running per-case average crosses the cap, the remaining cases are recorded as SKIPPED and the report still emits (dying at a cap with nothing posted is the failure mode the plan forbids). Each live result replays through the deterministic pipeline; `runner.ts` gains an optional `RunOptions.validation` override so the live validator's output replaces the recorded block. Spec-level regressions ("baseline caught, candidate missed" and the reverse) are diffed only over cases both arms actually scored, so a budget skip is never reported as a regression.

Judge scoring reuses the pinned judge for quality aggregates only (judge-vs-ground-truth disagreement keys on recorded ids, which live arms do not use); the fetch model moves from `live-judge.ts` into shared `eval/judge-live-model.ts`.

Output: `out/live-ab-report.json` plus a markdown table (metrics deltas, judge quality delta, cost, wall clock, regressions, skips, agent failures) printed and appended to `GITHUB_STEP_SUMMARY`. Report-only, with one exception per the playbook's standing rule: the candidate arm failing any adversarial-injection case (wrong verdict or missed spec) exits non-zero.

## Test plan:

- `pnpm run test --run`: 696 tests green (16 new: matcher rules incl. window overlap, one-candidate-one-spec, malformed-regex fallback-to-literal, capped fallback audit trail; runArm budget semantics incl. the sticky-stop case that caught a real bug in review; regression diffing over shared cases; report rendering both gate outcomes).
- `pnpm run typecheck` and eslint clean.
- The end-to-end CLI (`pnpm dlx tsx workflows/review/eval/live-ab.ts --max-usd 10 --cases incident-money-rounding,clean-no-findings`) needs `ANTHROPIC_API_KEY`; this environment has none, so that is the first thing to exercise when testing. On an unchanged review.md it prints the identical-arms note and near-zero deltas.


## Next steps (human)

1. ~~End-to-end A/B validation~~ DONE, superseded by the phase 4 acceptance runs (linked on #237): the control arm held recall and verdict agreement flat at +0% and the weakened arm showed -17% recall with the lost spec named, which is also the first empirical read on matcher quality (7 cases matched; one systematic miss traced to lens routing, fixed on #233, not to the matcher). Two acceptance-driven additions landed here since: `perCase.failedAgents` entries now carry `<agent>: <reason>` (a claim-validator flake was undiagnosable from the report alone; the flake itself is still to be root-caused on a future run), and every report closes with a per-row stability footer (judge quality rose on the deliberately regressed arm because fewer, surer comments each score better, so the footer warns against reading it alone).
2. Review the two scoring caveats: judge output is quality-aggregates only (disagreement semantics key on recorded ids), and the fallback arbiter is wired but not yet given a live implementation (matches would be marked `via: "fallback"` for audit when one is added).
3. Confirm the budget semantics (half of `--max-usd` per arm, sticky skip-and-report) match how you want cost bounded.

---

## Update 2026-07-09: eval-tuning pass (three riders)

Three instrument fixes from the [eval-tuning memo](https://claude.ai/code/artifact/2995f0b5-8840-4d03-96e3-2518e2f0f75a), landed here so the whole stack inherits them:

1. **Pre-flight identity short-circuit** (memo item 1, hoisted down from #251 with identical text so that branch merges cleanly): byte-identical review.md in both arms posts a no-reviewable-delta report and runs nothing; `--force-arms` preserved for wobble controls. Corpus-only and plumbing-only PRs in this stack now pay $0 per push.
2. **Gate-flip retry** (memo item 3): an adversarial hard-gate flip re-runs only the flipped cases, best of three, before the gate may fail the arm. Retried runs never replace the original in the metrics; they only decide whether the flip was a run-to-run flake (the #244/#245 incident shape). Retry spend is recorded in the report.
3. **Drop-bucket taxonomy** (memo item 4): every missed must-catch spec is classified true-miss vs found-but-dropped (provenance/scope/validation) by re-matching against the dropped candidates with the line window relaxed. Lost regressions are annotated with their class and the table gains a found-but-dropped count row.

Plus **judge economics**: the judge pin moves from Opus 4.8 to `claude-haiku-4-5-20251001` (the judge grades prose only; every load-bearing metric is deterministic, and the acceptance runs showed judge quality is not single-run-stable on any model, so it is priced accordingly; supersedes operator direction 4), and the judge seam gains retry with backoff so a transient 500 no longer wastes a whole scoring pass.

Author: jwbron

Reviewers: jeresig, github-actions[bot], jwbron, jaredly

Required Reviewers:

Approved By: jeresig, github-actions[bot]

Checks: ✅ 9 checks were successful

Pull Request URL: #236
jwbron and others added 2 commits July 10, 2026 14:23
Phase 5 of the live A/B eval plan (#232), independent of the eval-code stack (#233-#237): the `review-trial` Claude Code skill, packaging the Khan/webapp#40678 seeded-defect trial choreography so an operator can run one in an afternoon.

## What the skill does

Given a consumer repo, a seeded branch, a **human-authored** defect table, an arms list, and budget approval (projected up front from the trial's measured $7-10 per workflow-arm run, confirmed before any PR is created), it:

1. Creates one isolated PR per arm from the seeded branch, with per-arm trigger recipes (repo-default reviewer, pinned workflow ref, hosted `@claude review`) and two hard rules: distinct workflow names (same-named gh-aw workflows share a per-PR concurrency group and cancel each other; this silently ate a run in the original trial) and exactly one reviewer per PR (suppress the default via `skip-ai-review` on non-default arms).
2. Collects each run's review, artifacts (tolerating the known gh-aw staging-path artifact annotation), billed credits, and wall clock.
3. Optionally drives the lifecycle protocol (push 2 with fixes plus fresh seeds, push 3 all-fixed) identically across arms, scoring each push separately.
4. Scores defect by defect with the deterministic rule mirroring `eval/live-match.ts` (path + window + mechanism), manual judgment for leftovers marked as judged in the report, traps counted as correct suppression, and faithful reporting whichever arm it favors; output in the #40678 report shape.
5. Exports live-enabled corpus case skeletons, with an explicit sanitization gate for private-repo content landing in this public repo (the #235 lesson: structural rewrites, never copied code).
6. Cleans up: PRs closed with the report linked, trial branches deleted, no temporary workflow left behind.

What stays human is stated at the top of the skill: seeds and ground truth are operator-authored, always; the skill refuses to improvise a defect table.

## `.gitignore` change

`.claude` was ignored wholesale, which would have kept project skills untracked. Narrowed to `.claude/*` with a `!.claude/skills/` carve-out; verified `settings.json` and `worktrees/` remain ignored.

## Test plan:

Docs/tooling only (empty changeset); no code paths. `git check-ignore` verified the carve-out (SKILL.md tracked, `settings.json` and `worktrees` still ignored). The real acceptance is the plan's: re-running the #40678 trial via this skill reproduces its table in an afternoon, which needs a consumer-repo trial to exercise.


## Next steps (human)

1. Review the `.gitignore` change deliberately: `.claude` wholesale becomes `.claude/*` + `!.claude/skills/`. Verified locally that `settings.json` and `worktrees/` stay ignored, but this is a policy change about what agent tooling gets committed to the repo.
2. The skill's acceptance is operational, not CI: next time a trial is warranted (an architecture-bet change like the deterministic orchestrator, or pre-graduation ground-truthing), run it via this skill and check it reproduces the #40678 report shape in an afternoon. There is no cheaper honest validation.
3. Skim the per-arm trigger recipes against current consumer-repo reality (the `skip-ai-review` label convention, ready-for-review semantics); those conventions live outside this repo and can drift.

Author: jwbron

Reviewers: jeresig, github-actions[bot], somewhatabstract

Required Reviewers:

Approved By: jeresig, github-actions[bot]

Checks: ✅ 10 checks were successful

Pull Request URL: #238
# Conflicts:
#	workflows/review/eval/live-ab.ts
@jwbron jwbron merged commit d26b4e0 into main Jul 10, 2026
6 checks passed
@jwbron jwbron deleted the jwbron/live-eval-ab-ci branch July 10, 2026 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants