review: the re-review mode dial (full | scoped | flip-gated | fast)

.changeset/review-rereview-mode-dial.md

@@ -0,0 +1,5 @@
+---
+"review": minor
+---
+
+The re-review mode dial, the runs-per-PR cost lever: a per-repo `re-review` line in `.github/aw/review/ROUTING` (`full` | `scoped` | `flip-gated` | `fast`, default `full`) decides how much of the roster a repeat review of the same PR runs. `scoped` runs the whole roster staged only the hunks new since the last fully-reviewed fingerprint; `flip-gated` runs reconciliation plus the correctness pass, whose validated blocking findings veto a REQUEST_CHANGES→APPROVE flip; `fast` is reconcile-only. Three deterministic guards (`lib/rereview-mode.ts`): the anchoring full review is taken at ready-for-review (never on a draft skeleton); the flip gate; and a divergence tripwire that stamps a content-hashed hunk signature into the review body as a hidden comment (surviving cache eviction, dismiss-stale-approvals, and COMMENTED-only histories) and re-arms full review when the unreviewed share reaches 0.4. Adversarial lifecycle cases (rewrite-after-approval, sparse-PR-then-payload) live in `eval/lifecycle/` and are replayed deterministically; run cost now aggregates per executed depth (`costByRereviewDepth`) so the live A/B can price `scoped` against `full` on recall and dollars. Ships default-`full`: no consumer's behavior changes until its repo adds a mode line. Motivated by measured lifecycle costs on the seeded trial (v1.3.1: $7.19/$8.77/$9.14; v1.4.0: $8.31/$9.87/$11.29 per push), where a fresh seeded defect was caught on a re-review push that a reconcile-only path would have missed; the case for `scoped` as the recommended reduced depth. Supersedes the consumer-side stopgap in Khan/frontend#13420.

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

@@ -31,6 +31,13 @@ missing, stop and ask; do not improvise a defect table from the branch diff.
    - `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`).
+
+   A `workflow` arm may also carry a **ROUTING override**: extra or changed
+   lines for the arm branch's `.github/aw/review/ROUTING`. This is how the
+   re-review mode dial is priced; two arms at the SAME ref, one with
+   `re-review full` (control) and one with `re-review scoped` (or
+   `flip-gated`/`fast`), differ only in the mode line, so the lifecycle
+   tables isolate the dial's recall and dollar deltas.
 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).
@@ -85,6 +92,15 @@ 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.
 
+When an arm runs a reduced re-review mode (`re-review` in ROUTING), also
+record per push: the executed depth and tripwire fields from the run's
+`out/rereview-plan.json` artifact, and the billed cost, so the report can
+price depth against recall. If the lifecycle plan includes an adversarial
+push (a rewrite-after-approval or a payload onto a sparse PR; the
+`eval/lifecycle/` cases), score it as: the tripwire re-armed AND the payload
+got a full review; a reduced-mode arm that approves such a push without
+re-arming has failed the trial whatever it cost.
+
 ## Step 4: score
 
 Match each arm's posted comments against the defect table, per push:

workflows/review/README.md

@@ -153,12 +153,14 @@ rule per line:
 ```
 # <pattern> [lens=<lens>,…] [tier=trivial|low|medium|high] [direction-dependent]
 # enable <reviewer>[,<reviewer>…]
+# re-review full|scoped|flip-gated|fast
 services/**/migrations/**  tier=high lens=data-migrations
 **/*.graphql               lens=api-federation-compat
 pkg/auth/**                tier=high direction-dependent lens=security-auth
 services/**/testdata/**    tier=trivial
 docs/**                    tier=trivial
 enable holistic,test-adequacy
+re-review scoped
 ```
 
 - `lens=` names the specialist lenses to spawn when the pattern is touched; when
@@ -176,6 +178,9 @@ enable holistic,test-adequacy
   (tightening vs. loosening a check); the router emits the file as a pending risk
   question instead of guessing, and it applies only when its own rule is the
   winning tier rule for the path.
+- `re-review` sets the repo's re-review mode (see the next section). Default
+  `full`; when several lines set it, the last one wins with a warning. An
+  unknown mode degrades to `full`: toward more review, never less.
 
 Glob semantics are a practical subset of gitignore/CODEOWNERS: `**` crosses
 directories, `*` and `?` stay within a segment, a trailing `/` matches everything
@@ -187,6 +192,49 @@ and are skipped; routing degrades to fewer lenses, never to a crashed review.
 stays the model-facing prose about file *contents*; team ownership stays in
 `.github/REVIEWERS`, unchanged.
 
+### Re-review modes (the runs-per-PR cost lever)
+
+The workflow reviews every push, so a PR's lifetime cost is runs-per-PR times
+cost-per-run, and measured lifecycles showed cost *rising* per run as review
+threads accumulate, with the final approval run the most expensive while
+emitting the least. The `re-review` line in `ROUTING` dials how much of the
+roster a *repeat* review runs; the first full review of a ready PR always runs
+everything, whatever the mode:
+
+| Mode | Repeat review runs | When to use |
+| --- | --- | --- |
+| `full` | The whole roster over the whole diff (today's behavior). **Default.** | Until the live A/B has priced a cheaper mode for the repo. |
+| `scoped` | The whole roster, staged only the hunks that are new since the last fully-reviewed fingerprint (`scoped.diff`); comments stay scoped to those hunks. | The recommended first step down: measured lifecycles caught fresh seeded defects on re-review pushes, which a reconcile-only path would miss. |
+| `flip-gated` | Thread reconciliation plus the correctness pass over the new hunks. A REQUEST_CHANGES→APPROVE flip is vetoed by any validated blocking finding from that pass; the pass gates the flip instead of being discarded. | Cheap re-reviews that still cannot flip to approval over a fresh validated defect. |
+| `fast` | Thread reconciliation only. | Maximum savings; fresh code on a re-push is guarded only by the tripwire below. |
+
+Three guards keep the cheaper modes honest (`lib/rereview-mode.ts`, deterministic):
+
+- **Ready-for-review anchor.** A fingerprint taken while the PR was a draft
+  never anchors cheap re-reviews of the ready PR: the ready PR gets the one
+  full review the cheaper modes lean on.
+- **Flip gate.** In `flip-gated` mode the dispatched correctness pass's
+  validated blocking findings veto the approval flip.
+- **Divergence tripwire.** Every full-depth review stamps a content-hashed
+  hunk signature into its review body as a hidden comment (it survives cache
+  eviction and branch protection's dismiss-stale-approvals, and it (not the
+  review state) is what marks a full review as having happened, so a
+  COMMENTED-only or dismissed history never wedges the dial). Each push is
+  compared against that last fully-reviewed fingerprint; when the unreviewed
+  share reaches the threshold (default 0.4), full-review mode re-arms and the
+  divergent push gets the whole roster. This is what defeats
+  rewrite-after-approval and sparse-PR-then-payload
+  (`eval/lifecycle/`, replayed in `eval/lifecycle.test.ts`).
+
+The dial is a measured change, not a default change: it ships `full`
+everywhere, so no consumer's behavior moves until its repo adds a `re-review`
+line, and that line should be earned the way `enable` lines are, through the
+live A/B (arms identical except the ROUTING mode line, priced on recall and
+dollars; each run's executed depth lands in `out/rereview-plan.json`, and the
+counters aggregate `costByRereviewDepth`). Lifecycle-class changes like this
+one are trialed with the seeded-defect skill
+(`.claude/skills/review-trial/SKILL.md`).
+
 ### Models and effort per role
 
 Each sub-agent pins its model in its own definition inside `review.md` (with a

workflows/review/eval/corpus/live.ts

@@ -72,7 +72,19 @@ export type LiveDefectSpec = {
  */
 export type CaseLive = {
     prContext: LivePrContext;
-    /** Case-dir-relative path to the post-change tree (default `tree`). */
+    /**
+     * Case-dir-relative path to the post-change tree (default `tree`).
+     *
+     * Trees are deliberately minimal: validation requires only the changed
+     * files to exist. The staged copy of this tree is the sub-agent's whole
+     * world (its cwd, with no network), so the agent WILL often try to read
+     * an imported module or caller that is not there; that read returns an
+     * ordinary not-found tool error the agent tolerates and works around,
+     * not a run failure. The cost of a missing file is realism, not a crash:
+     * include the context files whose absence would change what a reviewer
+     * concludes (e.g. the decorator module a sibling handler imports), and
+     * nothing more.
+     */
     tree: string;
     /** Labeled defects a live run must catch. */
     mustCatchSpecs?: LiveDefectSpec[];

workflows/review/eval/lifecycle.test.ts

@@ -0,0 +1,162 @@
+/**
+ * Lifecycle replay for the re-review mode dial: the adversarial cases the
+ * dial must survive, as data (`eval/lifecycle/*.json`), replayed
+ * deterministically through the same lib functions the production CLI runs.
+ *
+ * These cases do not fit the corpus format (a corpus case is one PR state;
+ * these are push *sequences*), so they live in their own dataset. Each case
+ * lists the modes it must hold under, a sequence of pushes (each a unified
+ * diff and the verdict that run submits), and per-push expectations. The
+ * harness threads the review-body stamp from push to push exactly as
+ * production does: compute the signature, decide the depth, then stamp the
+ * simulated review the next push reads.
+ *
+ * The two adversarial cases are scored as "the tripwire re-armed and the
+ * payload got a full review"; the economy control is scored as "the reduced
+ * path actually ran"; a dial that trips on every fix push is a no-op cost
+ * lever, so both directions are load-bearing. A live-PR trial
+ * (`.claude/skills/review-trial/SKILL.md`) exercises the same sequences
+ * against the real workflow when a lifecycle change ships.
+ */
+
+import {readdirSync, readFileSync} from "node:fs";
+import {join} from "node:path";
+
+import {describe, it, expect} from "vitest";
+
+import {
+    computeHunkSignature,
+    decideReReviewDepth,
+    findLatestStamp,
+    renderRereviewStamp,
+    STAMP_SCHEMA_VERSION,
+} from "../lib/rereview-mode";
+import type {PriorReview, ReReviewPlan} from "../lib/rereview-mode";
+import type {ReReviewMode} from "../lib/routing-config";
+
+/* -------------------------------------------------------------------------- */
+/* Dataset shape and loading                                                  */
+/* -------------------------------------------------------------------------- */
+
+type LifecyclePush = {
+    name: string;
+    isDraft: boolean;
+    /** The verdict this run submits (rides into the stamp the next push reads). */
+    verdict: string;
+    /** The push's full unified diff, one line per array entry. */
+    diff: string[];
+    expected: {
+        /** A depth, or `same-as-mode`: the configured mode's own cheap path. */
+        depth: string;
+        tripwireRearmed: boolean;
+    };
+};
+
+type LifecycleCase = {
+    id: string;
+    tags: string[];
+    description: string;
+    /** The reduced modes the expectations must hold under, each replayed. */
+    modes: ReReviewMode[];
+    scoring: "payload-full-review" | "reduced-path-taken";
+    pushes: LifecyclePush[];
+};
+
+const LIFECYCLE_DIR = join(__dirname, "lifecycle");
+
+const loadLifecycleCases = (): LifecycleCase[] =>
+    readdirSync(LIFECYCLE_DIR)
+        .filter((name) => name.endsWith(".json"))
+        .sort()
+        .map(
+            (name) =>
+                JSON.parse(
+                    readFileSync(join(LIFECYCLE_DIR, name), "utf8"),
+                ) as LifecycleCase,
+        );
+
+/* -------------------------------------------------------------------------- */
+/* Replay                                                                     */
+/* -------------------------------------------------------------------------- */
+
+/** Replay one case under one mode; returns the per-push plans in order. */
+const replay = (
+    lifecycleCase: LifecycleCase,
+    mode: ReReviewMode,
+): ReReviewPlan[] => {
+    const priorReviews: PriorReview[] = [];
+    const plans: ReReviewPlan[] = [];
+    for (const [index, push] of lifecycleCase.pushes.entries()) {
+        const plan = decideReReviewDepth({
+            mode,
+            isDraft: push.isDraft,
+            priorStamp: findLatestStamp(priorReviews),
+            currentSignature: computeHunkSignature(push.diff.join("\n")),
+        });
+        plans.push(plan);
+        priorReviews.push({
+            body: renderRereviewStamp({
+                schemaVersion: STAMP_SCHEMA_VERSION,
+                depth: plan.depth,
+                verdict: push.verdict,
+                anchorDraft: plan.stampAnchorDraft,
+                anchorHunks: plan.stampHunks,
+            }),
+            submittedAt: `2026-07-0${index + 1}T00:00:00Z`,
+        });
+    }
+    return plans;
+};
+
+/* -------------------------------------------------------------------------- */
+/* The suite                                                                  */
+/* -------------------------------------------------------------------------- */
+
+const CASES = loadLifecycleCases();
+
+describe("re-review lifecycle dataset", () => {
+    it("loads the adversarial pair and at least one economy control", () => {
+        const ids = CASES.map((c) => c.id);
+        expect(ids).toContain("rereview-rewrite-after-approval");
+        expect(ids).toContain("rereview-sparse-pr-then-payload");
+        expect(CASES.some((c) => c.scoring === "reduced-path-taken")).toBe(
+            true,
+        );
+    });
+});
+
+describe.each(CASES)("$id", (lifecycleCase) => {
+    describe.each(lifecycleCase.modes)("under mode %s", (mode) => {
+        const plans = replay(lifecycleCase, mode);
+
+        it.each(
+            lifecycleCase.pushes.map((push, index) => ({
+                push,
+                plan: plans[index],
+            })),
+        )("push $push.name meets its expected depth", ({push, plan}) => {
+            const expectedDepth =
+                push.expected.depth === "same-as-mode"
+                    ? mode
+                    : push.expected.depth;
+            expect(plan.depth).toBe(expectedDepth);
+            expect(plan.tripwireRearmed).toBe(push.expected.tripwireRearmed);
+        });
+
+        if (lifecycleCase.scoring === "payload-full-review") {
+            it("scores: the tripwire re-armed and the payload got a full review", () => {
+                const payload = plans[plans.length - 1];
+                expect(payload.tripwireRearmed).toBe(true);
+                expect(payload.depth).toBe("full");
+                expect(payload.dispatch).toBe("all");
+                expect(payload.staging).toBe("whole-diff");
+            });
+        } else {
+            it("scores: the reduced path actually ran on the follow-up push", () => {
+                const followUp = plans[plans.length - 1];
+                expect(followUp.depth).toBe(mode);
+                expect(followUp.tripwireRearmed).toBe(false);
+            });
+        }
+    });
+});

workflows/review/eval/lifecycle/fix-push-stays-cheap.json

@@ -0,0 +1,85 @@
+{
+    "id": "rereview-fix-push-stays-cheap",
+    "tags": ["rereview", "economy"],
+    "description": "The economics control: a reviewed six-hunk PR gets a routine one-hunk fix push. The unreviewed share (1/6) stays under the tripwire threshold, so each reduced mode actually runs its cheap path; this is the case the dial exists for, and the case the live A/B prices against full. A dial that trips on every fix push would be a no-op cost lever.",
+    "modes": ["scoped", "flip-gated", "fast"],
+    "scoring": "reduced-path-taken",
+    "pushes": [
+        {
+            "name": "initial",
+            "isDraft": false,
+            "verdict": "REQUEST_CHANGES",
+            "diff": [
+                "diff --git a/src/models/order.ts b/src/models/order.ts",
+                "--- a/src/models/order.ts",
+                "+++ b/src/models/order.ts",
+                "@@ -5,2 +5,3 @@",
+                " export type Order = {",
+                "+    status: OrderStatus;",
+                "     id: string;",
+                "@@ -20,2 +21,3 @@",
+                " export const load = (id: string) => {",
+                "+    metrics.count(\"order.load\");",
+                "     return db.get(id);",
+                "@@ -40,2 +42,3 @@",
+                " export const save = (order: Order) => {",
+                "+    metrics.count(\"order.save\");",
+                "     return db.put(order);",
+                "diff --git a/src/models/order.test.ts b/src/models/order.test.ts",
+                "--- a/src/models/order.test.ts",
+                "+++ b/src/models/order.test.ts",
+                "@@ -8,2 +8,3 @@",
+                " describe(\"order\", () => {",
+                "+    it(\"carries a status\", () => run(statusCase));",
+                "     it(\"loads\", () => run(loadCase));",
+                "@@ -30,2 +31,3 @@",
+                " const loadCase = {",
+                "+    status: \"open\",",
+                "     id: \"o-1\",",
+                "@@ -50,2 +52,3 @@",
+                " const saveCase = {",
+                "+    status: \"open\",",
+                "     id: \"o-2\","
+            ],
+            "expected": {"depth": "full", "tripwireRearmed": false}
+        },
+        {
+            "name": "review-fix",
+            "isDraft": false,
+            "verdict": "APPROVE",
+            "diff": [
+                "diff --git a/src/models/order.ts b/src/models/order.ts",
+                "--- a/src/models/order.ts",
+                "+++ b/src/models/order.ts",
+                "@@ -5,2 +5,3 @@",
+                " export type Order = {",
+                "+    status: OrderStatus;",
+                "     id: string;",
+                "@@ -20,2 +21,3 @@",
+                " export const load = (id: string) => {",
+                "+    metrics.count(\"order.load\");",
+                "     return db.get(id);",
+                "@@ -40,2 +42,3 @@",
+                " export const save = (order: Order) => {",
+                "+    metrics.count(\"order.save\", {status: order.status});",
+                "     return db.put(order);",
+                "diff --git a/src/models/order.test.ts b/src/models/order.test.ts",
+                "--- a/src/models/order.test.ts",
+                "+++ b/src/models/order.test.ts",
+                "@@ -8,2 +8,3 @@",
+                " describe(\"order\", () => {",
+                "+    it(\"carries a status\", () => run(statusCase));",
+                "     it(\"loads\", () => run(loadCase));",
+                "@@ -30,2 +31,3 @@",
+                " const loadCase = {",
+                "+    status: \"open\",",
+                "     id: \"o-1\",",
+                "@@ -50,2 +52,3 @@",
+                " const saveCase = {",
+                "+    status: \"open\",",
+                "     id: \"o-2\","
+            ],
+            "expected": {"depth": "same-as-mode", "tripwireRearmed": false}
+        }
+    ]
+}