review: render the quoted skill rule into the comment the author reads

.changeset/review-dispatch-tax.md

@@ -0,0 +1,5 @@
+---
+"review": patch
+---
+
+Dispatch-tax trim: dedupe the specialist-lens discipline snippets. The v1.4.0 re-run's cost premium vs the v1.3.1 baseline sat at dispatch time (uncached input +51-65%, cache writes +18-34% per run), partly because the shared discipline blocks (bounded investigation, untrusted input, lens-owned skills with quote-the-rule, the schema-rules trailer, the tri-state hunt contract) were stamped verbatim into every one of the eleven specialist-lens definitions and paid on every dispatch. The shared text now lives once, in a marker-delimited "REVIEW DISCIPLINES" section of review.md's main body; Step 1 stages it to `/tmp/gh-aw/review/disciplines.md` with one whole-line-anchored `sed` extraction of the engine-provided rendered prompt (`$GH_AW_PROMPT`), verified by grep with a byte-for-byte heredoc fallback, and each lens carries only a pointer plus its own domain notes (its investigation examples, review rules, hunts, and output JSON, all unchanged). Behavior-neutral by construction: the instruction content is unchanged, the three variants that had already drifted apart are unified on the fullest wording, and the label-shape reviewers (whose variants differ materially) keep their own copies. Pinned by `lib/disciplines.test.ts`: the section extracts cleanly under the exact sed range semantics, every lens points at the staged file and carries no residual copy, and the label-shape reviewers still carry theirs.

.changeset/review-live-ab-ci.md

@@ -0,0 +1,5 @@
+---
+"review": patch
+---
+
+CI wiring for the live A/B (live A/B plan, phase 4): the `Review Eval A/B` workflow runs on every non-draft PR touching `workflows/review/**` (plus `workflow_dispatch`), executing the arm-to-arm live eval against the PR's merge-base and posting the delta report as a sticky PR comment, a job summary, and an artifact. Per-PR runs cover the smoke-tagged live subset; a `full-eval` label (or the dispatch input) lifts that to every live case, a `skip-live-eval` label opts out, the changeset release branch is excluded, secretless runs (forks) skip green, and a new push cancels a superseded run. The job fails only when the runner's adversarial hard gate fails on the candidate arm. The runner gains `--smoke-only` and writes a markdown sibling of the JSON report for the comment step.

.changeset/review-live-ab-runner.md

@@ -0,0 +1,5 @@
+---
+"review": minor
+---
+
+The live A/B runner (live A/B plan, phase 3). `eval/live-match.ts` maps a run's posted candidates onto a live case's labeled defect specs (deterministic path/window/mechanism rule first, then an injected hard-capped fallback arbiter whose matches are recorded for audit) and aggregates live metrics: must-catch recall, verdict agreement, clean false-flag, and noise. `eval/live-ab.ts` runs the model sub-agents from two review.md versions (the merge-base baseline via `git show`, the working-tree candidate) over the same live corpus, each arm under half a hard USD budget with sticky stop-and-report-skips semantics, replays each live result through the deterministic pipeline (`runner.ts` gains a `validation` override on `RunOptions` so a live validator's output replaces the recorded block), diffs spec-level regressions over the cases both arms scored, judges both arms' comments with the pinned judge (quality aggregates only; extracted to `eval/judge-live-model.ts`, now shared with `live-judge.ts`), and emits a JSON report plus a markdown table (also appended to GITHUB_STEP_SUMMARY). Report-only except the playbook's standing rule: the candidate arm failing any adversarial-injection case exits non-zero.

.changeset/review-live-producer-staging.md

@@ -0,0 +1,5 @@
+---
+"review": minor
+---
+
+The live producer (live A/B plan, phase 2): everything needed to run the REAL model sub-agents over a live-enabled corpus case. `eval/agent-extract.ts` parses `review.md`'s `## agent:` sections into name/description/model/prompt as pure data (string in, no fs), so an A/B can extract the baseline arm from a `git show` of the merge-base; parsing is strict and lists every malformed section at once, and an integration test runs it over the real `review.md`. `eval/live-stage.ts` materializes the production `/tmp/gh-aw/review/` staging layout for a case (pr-context.json, full/pr/full-stripped diffs, files.json with hasPatch, review-files.json, provenance.json, routing.json, out/, and the post-change checkout) and rewrites extracted prompts to point at it. `eval/live-producer.ts` dispatches the default finders plus the router's lenses through an injected runner seam, maps all three output contracts (label-shape reviewers, structured-schema lenses, the claim-validator's three-state verdicts) into the exact `RecordedFinding`/`CaseVerification` shapes the deterministic runner consumes, stages `claims.json`, resolves `{{#runtime-import}}` directives from the case tree, retries once on malformed output, keeps partial results on agent failure, and accounts per-agent cost. `eval/live-runner.ts` is the one SDK-backed runner (Claude Agent SDK; Read/Grep/Glob only, cwd pinned to the staged checkout, hard turn and wall-clock caps) plus a CLI smoke entry (`pnpm dlx tsx workflows/review/eval/live-runner.ts --case <id>`, requires ANTHROPIC_API_KEY).

.changeset/review-out-artifact-upload.md

@@ -0,0 +1,5 @@
+---
+"review": patch
+---
+
+Fix the out/ run-artifact upload, which failed on every run under gh-aw v0.81.6. The orchestrator passed the absolute path `/tmp/gh-aw/review/out/` exactly as instructed, but gh-aw's upload_artifact tool stages an uploaded directory under its basename and records only the staging-relative name (`out`), and the safe_outputs job then filters the staged files (`out/<agent>.json`) against `allowed-paths` with a fully anchored matcher; the absolute pattern `/tmp/gh-aw/review/out/**` therefore matched nothing, every run annotated `ERR_VALIDATION: upload_artifact: no files matched the selection criteria`, and no sub-agent outputs or `pre-existing.json` reached the run artifacts, blocking post-hoc claim auditing, the live counters, and the A/B runner, which all read them. `allowed-paths` now lists the staging-relative `out/**` (plus the absolute form, since the filter is an OR, in case a future gh-aw matches original paths). Step 9 also copies `claims.json` into `out/` before uploading, so the artifact carries the validator's input alongside its verdicts (`claim-validator.json`) and the provenance gate's set-asides (`pre-existing.json`).

.changeset/review-out-of-lane.md

@@ -0,0 +1,5 @@
+---
+"review": minor
+---
+
+Out-of-lane observations are handed off, never dropped. Observed on the review-v1.4.0 re-run lifecycle: the skill-auditor found a real eventual-consistency bug (dedup reads via a Query immediately after PutMulti) and discarded it, verbatim "that's a correctness concern, not a quotable skill-rule violation, so I'll leave it"; correct under quote-the-rule, but the observation died and the defect shipped unflagged. The skill-auditor and every specialist lens may now return `out_of_lane_observations[]` (path, optional line, the concern stated concretely, a required concrete `failure_scenario`, optional `suggested_lane`) for real concerns their own mandate does not let them report. The orchestrator converts each one into a candidate comment with the code-assigned label `question (non-blocking)` (a handoff is not a vetted finding and can never block on its own; the claim-validator never upgrades severity) and routes it through the identical provenance gate, scope filter, claims.json, and validation path as every other candidate. The shape is validated by the new `validateOutOfLaneObservation` in `lib/finding-schema.ts` (a sibling type, so `FINDING_SCHEMA_VERSION` stays 2: no serialized finding is invalidated).

.changeset/review-rereview-accountability.md

@@ -0,0 +1,5 @@
+---
+"review": minor
+---
+
+Re-review accountability, code-rendered. Observed on the review-v1.4.0 re-run lifecycle (Khan/webapp#40730): run 2 resolved the fixed threads and said nothing about the three blocking threads it left open, under a bare "Changes requested" body; run 3 approved with an empty body while resolving 11 threads. The verdict body now accounts for every prior thread, rendered deterministically from the `thread-reconciler`'s keep/resolve lists by the new `lib/rereview.ts` (never composed by the model): each still-unaddressed prior thread is enumerated as a link to its earlier comment ("as of \<sha\>", blocking first, with a verbatim excerpt of the earlier comment's first line), with the resolved count stated; an approval that resolved the last open threads says every prior thread is resolved. `threads.json` staging gains a `url` field (the thread's first comment `html_url`) to power the links, `renderReviewBody` gains a `rereviewSection` slot, and the redundant-approval skip treats a non-empty section as content. Fail-open: a missing or unparseable staging input renders an empty section, leaving the body exactly as before.

.changeset/review-skill-rule-quote.md

@@ -0,0 +1,5 @@
+---
+"review": minor
+---
+
+Skill findings show the author the actual rule. Quote-the-rule already requires the exact rule text in a lens skill finding's `evidence_trace`, but evidence traces never reach the PR: the author reads only `model_authored_prose`, so they see a paraphrase of a rule they cannot check. The finding schema gains an optional `rule_quote` field (the exact rule text, verbatim from the skill file; validated non-empty when present, and optional, so `FINDING_SCHEMA_VERSION` stays 2), each lens's lens-owned-skills discipline says to fill it, and `renderComment` plus the orchestrator's normalization step surface it into the posted comment as a `> **Rule:** …` blockquote between the prose and any suggestion block. Only the wrapping is code-owned; the quote is skill-file text copied verbatim.

.changeset/review-trial-skill.md

@@ -0,0 +1,4 @@
+---
+---
+
+Docs/tooling only: add the `review-trial` Claude Code skill (`.claude/skills/review-trial/SKILL.md`), packaging the Khan/webapp#40678 seeded-defect live-trial choreography (live A/B plan, phase 5): isolated per-arm PRs with distinct workflow names, run collection, defect-table scoring, corpus export with the private-to-public sanitization gate, and cleanup. Seeds and ground truth stay human-authored; costs are projected and confirmed before any PR is created.

.claude/skills/review-trial/SKILL.md

@@ -0,0 +1,135 @@
+---
+name: review-trial
+description: Run a seeded-defect live trial of the PR review workflow, comparing reviewer arms on isolated copies of one seeded PR (the Khan/webapp#40678 pattern). Use when evaluating an architecture-class review change, a re-review/lifecycle behavior change, or ground-truthing before graduating a repo to automatic mode. Invoke with a consumer repo, a seeded branch, a defect table, and an arms list.
+---
+
+# Seeded-defect live trial
+
+Choreograph the trial pattern from Khan/webapp#40678: one human-seeded PR,
+copied into isolated PRs so each reviewer arm reviews identical content
+without seeing another arm's comments, optionally driven through a re-review
+lifecycle, then scored defect by defect and exported into the eval corpus.
+This is the playbook's architecture-bet instrument; per-change evals belong to
+the corpus A/B (`workflows/review/eval/live-ab-plan.md`), not to this skill.
+
+## What stays human
+
+Do NOT author seeds or ground truth yourself. The operator supplies both; a
+model authoring its own seeds would grade its own homework. If either is
+missing, stop and ask; do not improvise a defect table from the branch diff.
+
+## Required inputs (collect all before doing anything)
+
+1. **Consumer repo** (e.g. `Khan/webapp`) and the **seeded branch** in it.
+2. **The defect table**: one row per seed with `key`, `path`, an approximate
+   line window, `mechanism` (2-4 keyword/regex alternates describing the
+   causal defect), intended severity, and any deliberate non-defect traps
+   (rows the reviewers must NOT flag, with the exculpating evidence named,
+   e.g. "the wrapper chunks DeleteMulti; see internal/datastore/client.go").
+3. **The arms**: for each, a name plus its reviewer source, one of:
+   - `repo-default`: whatever reviewer the consumer repo already runs.
+   - `workflow @ <ref>`: the shared review workflow pinned to a tag/branch
+     (a candidate build or a prior release).
+   - `hosted`: the Claude Code GitHub app review (`@claude review`).
+4. **Lifecycle plan** (optional): the push-2 content (fixes mixed with fresh
+   seeds, plus their defect-table rows) and the push-3 content (everything
+   fixed).
+5. **Budget approval**: project the cost FIRST and confirm. Measured basis:
+   roughly $7-10 per workflow-arm run; total is arms x (1 + lifecycle
+   pushes), hosted-arm runs billed separately by the app. Print the
+   projection and get an explicit yes before creating any PR.
+
+## Step 1: isolated arm PRs
+
+For each arm, create `trial/<slug>/<arm>` from the seeded branch and open a
+PR in the consumer repo (title prefixed `[reviewer-trial]`, body stating it
+is a trial and will be closed unmerged). One PR per arm; never point two arms
+at the same PR, or their comments contaminate each other.
+
+Per-arm trigger setup:
+
+- `repo-default`: nothing extra; the repo's reviewer triggers normally.
+- `workflow @ ref`: commit the compiled workflow for that ref onto the arm
+  branch. **Give it a workflow name distinct from the repo's own reviewer
+  and from every other arm**: same-named gh-aw workflows share a per-PR
+  concurrency group and silently cancel each other (this ate a run in the
+  original trial). Then suppress the repo's default reviewer on this PR via
+  its documented opt-out (`skip-ai-review` label in Khan repos), so exactly
+  one reviewer runs per PR.
+- `hosted`: suppress the default reviewer the same way, then trigger with an
+  `@claude review` comment.
+
+Draft PRs generally do not trigger reviewers; open the PRs ready-for-review.
+
+## Step 2: run and collect
+
+For each arm: trigger, then watch to completion (`gh run watch` /
+`gh pr checks`). Collect, per run:
+
+- The posted review: verdict, every inline comment (path, line, body), and
+  the review body (`gh api` on the PR's reviews and review comments).
+- Run artifacts when the arm is the shared workflow (findings, claims,
+  verdict JSONs): `gh run download`. Tolerate the known gh-aw staging-path
+  artifact bug (an ERR_VALIDATION annotation; the artifacts still upload).
+- Cost and wall clock: billed AI credits from the run summary, run duration
+  from `gh run view`. Hosted-arm cost is app-billed; record it as opaque
+  unless the operator supplies it.
+
+Record everything into a working directory as you go; a trial that loses its
+transcripts cannot be scored or exported.
+
+## Step 3: lifecycle (when the plan includes it)
+
+Push the operator's push-2 content to EVERY arm branch (identical commits),
+let each arm re-review, and collect again; repeat for push-3. Score each push
+separately: re-review behavior (thread resolution, duplicate suppression,
+scoping to new hunks) is half the point of the lifecycle.
+
+## Step 4: score
+
+Match each arm's posted comments against the defect table, per push:
+
+- Deterministic first: same path, line within (or overlapping) the window,
+  and any mechanism alternate matching the comment text, case-insensitively.
+  This mirrors `workflows/review/eval/live-match.ts`; one comment satisfies
+  at most one seed.
+- Ambiguous leftovers: judge manually, and mark judged matches in the report
+  so a reader can audit them.
+- Non-defect traps: a comment matching a trap row is a false flag; silence
+  on a trap is correct suppression and counts FOR the arm.
+
+Produce the #40678 report shape: a headline table (seeds caught, verdict,
+comment count, cost, wall clock, per arm), a defect-by-defect table (one row
+per seed, one column per arm), a lifecycle table when applicable, and a short
+writeup naming where each margin came from. Report faithfully: misses,
+wrong severities, and noise comments all go in, whichever arm they favor.
+
+## Step 5: export to the corpus
+
+Every trial compounds the corpus. Emit case skeletons in the live-enabled
+format (`workflows/review/eval/corpus/`, layout `<id>/case.json` + `tree/`;
+format spec in `corpus/live.ts`): the seeded diff, the post-change tree,
+`live.mustCatchSpecs` straight from the defect table, traps as
+`mustNotFlagSpecs` on a clean case, and recorded findings taken from the
+best arm's artifacts.
+
+**Sanitization gate**: if the consumer repo is private and the corpus repo
+public (Khan/webapp into Khan/actions is exactly this), never copy code,
+paths, or identifiers. Author structural rewrites that reproduce each
+defect mechanism with generic naming, the way the #40678 seeds landed in
+Khan/actions#235. When in doubt, rewrite.
+
+## Step 6: clean up
+
+Close every trial PR with a comment linking the report, delete the
+`trial/<slug>/*` branches, and confirm no temporary workflow file survived
+onto a long-lived branch. Leave the collected transcripts with the operator
+(attach the report to the PR or issue that motivated the trial).
+
+## Guardrails (recap)
+
+- Costs projected and approved before the first PR exists.
+- Seeds and ground truth are operator-authored, always.
+- One arm per PR; distinct workflow names; exactly one reviewer per PR.
+- Score before cleanup; export before cleanup; never lose transcripts.
+- Faithful reporting, including the arm you expected to win losing.