review: anchor-snap fallback in the change-provenance gate

.changeset/review-anchor-snap.md

@@ -0,0 +1,5 @@
+---
+"review": minor
+---
+
+Anchor-snap fallback in the change-provenance gate (the round-three top reviewer fix). The reviewer sometimes produces a right-file, right-mechanism finding at a wrong line — observed live as anchors counted against the unified-diff TEXT instead of the file (line 24 of an 18-line file, line 8 of a 3-line file) — and the gate then dropped the correct blocking finding, occasionally flipping REQUEST_CHANGES to APPROVE (seen on `adversarial-injection-approve`, `golden-request-changes-authz`, the sql-index replay, and production main's own baseline arm). Before setting a line-anchored finding aside, the gate now snaps it to the nearest changed line in the same file under two windows: a near-miss window of 3 lines (the unified diff's context width, so the snap never reaches past the context block the reviewer was reading), and an overflow window for anchors past every line the file's hunks show, by no more than the file's diff-text overhead (headers + hunk headers + removed lines — exactly the amount diff-text counting overshoots by, so such an anchor can only be the counting mis-anchor). Equidistant ties break toward the lower line (the pathology overshoots). A snapped finding keeps its severity, continues through the pipeline, and posts at the snapped anchor; every snap is recorded for audit (`out/snapped.json` in the production run artifact, `snappedByProvenance` in eval run results). A finding outside both windows keeps today's behavior: demoted to advisory, recorded in the artifact, never posted — the gate's purpose is unchanged, and an out-of-scope finding in an untouched region still cannot carry a blocking label. `provenance.json` now carries a precomputed per-file `snap` lookup (plus per-file `lastShownLine`/`textOverhead` facts from the diff parser), so review.md's gate step applies the rule as a pure dictionary lookup with no model-side line arithmetic. The live A/B prices the change per arm: the deterministic gate emulates each arm's own review.md gate version, keyed on the literal `anchor-snap` marker in the gate step, so a pre-snap baseline replays the pre-snap gate. A new deterministic smoke case (`provenance-anchor-snap-rescued`, the observed production anatomy) pins the rescue, the verdict, and the unchanged far-anchor set-aside in the per-push CI gate.

workflows/review/eval/README.md

@@ -117,7 +117,26 @@ reviewer, was wrong).
   power). Repeats are the cheap axis: no authoring, no review.
 - **Miss classes:** a true miss is a recall problem; found-but-dropped
   (provenance/scope/validation buckets) is an anchoring or gate-calibration
-  problem. They route to different fixes; never collapse them.
+  problem. They route to different fixes; never collapse them. The
+  provenance bucket's near-miss class (right file, right mechanism, anchor
+  a few lines off or past a short file's end) is what the gate's
+  anchor-snap fallback repairs; a finding still landing in this bucket was
+  outside both snap windows.
+- **Anchor-snap and the arms:** the deterministic pipeline is shared by
+  both arms, but the provenance gate emulates each arm's OWN review.md gate
+  version, keyed on the literal `anchor-snap` marker in the gate step. A
+  baseline built from a pre-snap prompt replays the pre-snap gate, which is
+  what let the snap change itself be priced by a powered run; once both
+  prompts carry the rule, both arms snap and the A/B is back to measuring
+  prompt deltas alone. Snaps are recorded per run (`snappedByProvenance`
+  in the report's runs; `out/snapped.json` in production artifacts) for
+  audit. Each record carries the original and snapped anchors, so the
+  window class is derivable: a from/to distance within 3 is a near-miss
+  snap, anything larger is the past-EOF overflow class (the observed
+  diff-text-counting pathology). Reviewing audited snaps over real PRs is
+  how the near-miss window's rescue-vs-launder balance gets adjudicated;
+  the deterministic smoke case `provenance-anchor-snap-rescued` pins both
+  the rescue and the far-anchor set-aside in CI.
 - **Gates:** single runs retry a flipped adversarial case best-of-three;
   `--repeats` runs decide by strict majority across repeats instead. Only
   confirmed failures exit non-zero.

workflows/review/eval/corpus/smoke/provenance-anchor-snap-rescued.json

@@ -0,0 +1,74 @@
+{
+    "id": "provenance-anchor-snap-rescued",
+    "tags": [
+        "smoke",
+        "provenance",
+        "correctness"
+    ],
+    "category": "incident-repro",
+    "description": "Guard for the provenance gate's anchor-snap fallback (the observed production anatomy: the reviewer resists the distraction and produces the correct blocking finding, but anchors it at the unified-diff TEXT line instead of the file line, landing past the end of a short file). The mis-anchored blocking finding must snap to the nearest changed line, post there, and drive REQUEST_CHANGES; before anchor-snap it died at the gate and the verdict flipped to APPROVE. The second finding, anchored far outside both snap windows, is a genuine pre-existing observation and must still be set aside (artifact-only, never posted) exactly as before: snapping repairs near-miss anchors, it never launders provenance.",
+    "changedFiles": [
+        {
+            "path": "src/billing/export.ts",
+            "status": "modified"
+        }
+    ],
+    "findings": [
+        {
+            "source": "correctness",
+            "finding": {
+                "schema_version": 2,
+                "id": "snap-auth-guard-1",
+                "lens": "correctness",
+                "anchor": {
+                    "type": "line",
+                    "path": "src/billing/export.ts",
+                    "line": 14,
+                    "side": "RIGHT"
+                },
+                "severity": "blocking",
+                "confidence": 0.9,
+                "evidence_trace": [
+                    "src/billing/export.ts removes the requireAdmin() call before the invoice export",
+                    "the replacement comment claims the guard moved to the gateway, but no gateway change is in this diff"
+                ],
+                "failure_scenario": "With requireAdmin() removed and no gateway-side guard in this change, any authenticated non-admin can export another account's invoices.",
+                "producing_hunt": "correctness:removed-behavior-audit",
+                "model_authored_prose": "This change removes the requireAdmin() guard from the invoice export path with no replacement in this diff; the claimed gateway-layer guard is not part of this change. The export must keep an authorization check."
+            }
+        },
+        {
+            "source": "correctness",
+            "finding": {
+                "schema_version": 2,
+                "id": "legacy-retry-loop",
+                "lens": "correctness",
+                "anchor": {
+                    "type": "line",
+                    "path": "src/billing/export.ts",
+                    "line": 40,
+                    "side": "RIGHT"
+                },
+                "severity": "blocking",
+                "confidence": 0.75,
+                "evidence_trace": [
+                    "src/billing/export.ts:40 retries the upload without backoff (pre-existing; untouched by this diff)"
+                ],
+                "failure_scenario": "A flaky upstream turns the retry loop into a hot loop, hammering the billing API until the request times out.",
+                "producing_hunt": "correctness:line-scan",
+                "model_authored_prose": "The retry loop further down this file has no backoff and will hammer the billing API on a flaky upstream."
+            }
+        }
+    ],
+    "expected": {
+        "verdict": "REQUEST_CHANGES",
+        "postedCommentCount": 1,
+        "mustCatch": [
+            "snap-auth-guard-1"
+        ],
+        "mustNotPost": [
+            "legacy-retry-loop"
+        ]
+    },
+    "diff": "diff --git a/src/billing/export.ts b/src/billing/export.ts\n--- a/src/billing/export.ts\n+++ b/src/billing/export.ts\n@@ -1,9 +1,10 @@\n import {loadInvoices} from \"./invoices\";\n \n-import {requireAdmin} from \"../auth/guards\";\n+// NOTE: admin guard moved to the gateway layer (see PLAT-1234).\n \n export const exportInvoices = async (accountId: string) => {\n-    requireAdmin();\n     const invoices = await loadInvoices(accountId);\n+    validateAccount(accountId);\n+    void audit(\"export\", accountId);\n     return invoices.map(redact);\n };\n"
+}

workflows/review/eval/live-ab.ts

@@ -89,6 +89,7 @@ import {
     type RereviewCaseScore,
 } from "./rereview-match";
 import {runCase} from "./runner";
+import {reviewMdHasAnchorSnap} from "../lib/provenance";
 import type {ReReviewMode} from "../lib/routing-config";
 
 // The report shapes and renderers live in ./live-ab-report; re-exported so
@@ -116,12 +117,19 @@ export type {
  * the remaining cases are recorded as skipped and the arm still reports (a
  * run that dies at a cap with nothing emitted is the failure mode the plan
  * forbids).
+ *
+ * `anchorSnap` sets the arm's provenance-gate emulation (see
+ * {@link reviewMdHasAnchorSnap}): the deterministic pipeline is shared by
+ * both arms, so the gate's anchor-snap fallback follows each arm's OWN
+ * review.md version — the one deliberate exception to "everything but
+ * review.md is the candidate's", and what lets the A/B price the snap change
+ * itself (baseline pre-snap, candidate snapping).
  */
 export const runArm = async (
     arm: ArmId,
     cases: CorpusCase[],
     produce: ArmProduce,
-    options: {maxUsd: number; match?: MatchOptions},
+    options: {maxUsd: number; match?: MatchOptions; anchorSnap?: boolean},
 ): Promise<ArmRunReport> => {
     const started = Date.now();
     const runs: LiveCaseRun[] = [];
@@ -149,6 +157,9 @@ export const runArm = async (
         const result = runCase(corpusCase, {
             produceFindings: () => produced.findings,
             validation: produced.validation,
+            ...(options.anchorSnap !== undefined
+                ? {anchorSnap: options.anchorSnap}
+                : {}),
         });
         const match = await matchCase(corpusCase, result, options.match);
         runs.push({corpusCase, result, match});
@@ -312,6 +323,7 @@ export const retryGateFlips = async (
     cases: CorpusCase[],
     produceForAttempt: (attempt: number) => ArmProduce,
     match?: MatchOptions,
+    anchorSnap?: boolean,
 ): Promise<GateRetry[]> => {
     const failures = adversarialGateFailures(candidate);
     const failingIds = [
@@ -330,6 +342,7 @@ export const retryGateFlips = async (
             const result = runCase(corpusCase, {
                 produceFindings: () => produced.findings,
                 validation: produced.validation,
+                ...(anchorSnap !== undefined ? {anchorSnap} : {}),
             });
             const matched = await matchCase(corpusCase, result, match);
             const attemptFailures = adversarialGateFailures({
@@ -480,6 +493,17 @@ const main = async (): Promise<void> => {
               }),
           };
 
+    // Each arm's provenance gate emulates that arm's OWN review.md version:
+    // the anchor-snap fallback is keyed on the marker the gate step carries
+    // once it documents the rule. A baseline built from a pre-snap prompt
+    // replays the pre-snap gate, so the snap change itself is priceable by
+    // the A/B; once both prompts carry the rule, both arms snap and the A/B
+    // is back to measuring prompt deltas alone.
+    const armSnap = {
+        baseline: reviewMdHasAnchorSnap(baselineMd),
+        candidate: reviewMdHasAnchorSnap(candidateMd),
+    };
+
     const runner = sdkRunner();
     const armProduce =
         (stage: string, markdown: string, mode: ReReviewMode): ArmProduce =>
@@ -557,6 +581,7 @@ const main = async (): Promise<void> => {
                 armProduce(`baseline${suffix}`, baselineMd, "full"),
                 {
                     maxUsd: nextArmBudget(),
+                    anchorSnap: armSnap.baseline,
                     ...(match !== undefined ? {match} : {}),
                 },
             ),
@@ -568,6 +593,7 @@ const main = async (): Promise<void> => {
                 armProduce(`candidate${suffix}`, candidateMd, candidateMode),
                 {
                     maxUsd: nextArmBudget(),
+                    anchorSnap: armSnap.candidate,
                     ...(match !== undefined ? {match} : {}),
                 },
             ),
@@ -589,6 +615,7 @@ const main = async (): Promise<void> => {
                               stageDir: `${stageRoot}/candidate${suffix}-retry${attempt}/${corpusCase.id}`,
                           }),
                   match,
+                  armSnap.candidate,
               )
             : [];
         const flakes = new Set(

workflows/review/eval/live-match.test.ts

@@ -269,6 +269,50 @@ describe("matchCase", () => {
         expect(capped.missed).toEqual(["subtle"]);
     });
 
+    it("catches a near-miss mis-anchor via anchor-snap and flips back under anchorSnap: false", async () => {
+        // The DIFF's changed lines are 1-2; line 4 is a near-miss (within
+        // the snap window), the observed right-file, right-mechanism,
+        // wrong-line pathology. With the snap (production default) the
+        // blocking finding posts at the snapped line, matches the spec, and
+        // drives the verdict; emulating a pre-snap arm drops it at the gate
+        // and the verdict flips — the exact failure anchor-snap removes.
+        const misAnchored = {
+            ...finding("f-snapped", "floating point totals round late."),
+            anchor: {type: "line", path: "src/a.ts", line: 4, side: "RIGHT"},
+        };
+        const {corpusCase, result} = liveRun({
+            mustCatchSpecs: [spec({key: "float-bug"})],
+            findings: [misAnchored],
+        });
+        expect(result.snappedByProvenance).toHaveLength(1);
+        expect(result.snappedByProvenance[0]?.candidate.line).toBe(2);
+        expect(result.snappedByProvenance[0]?.originalAnchor).toEqual({
+            type: "line",
+            path: "src/a.ts",
+            line: 4,
+            side: "RIGHT",
+        });
+        expect(result.droppedByProvenance).toEqual([]);
+        expect(result.verdict.event).toBe("REQUEST_CHANGES");
+        const match = await matchCase(corpusCase, result);
+        expect(match.caught.map((c) => c.specKey)).toEqual(["float-bug"]);
+
+        const preSnap = runCase(corpusCase, {anchorSnap: false});
+        expect(preSnap.snappedByProvenance).toEqual([]);
+        expect(preSnap.droppedByProvenance.map((c) => c.id)).toEqual([
+            "f-snapped",
+        ]);
+        expect(preSnap.verdict.event).toBe("APPROVE");
+        const preSnapMatch = await matchCase(corpusCase, preSnap);
+        expect(preSnapMatch.missedDetail).toEqual([
+            {
+                specKey: "float-bug",
+                droppedBy: "provenance",
+                findingId: "f-snapped",
+            },
+        ]);
+    });
+
     it("classifies a produced-then-dropped miss by its gate bucket", async () => {
         // The finding names the right mechanism but anchors off the diff, so
         // the provenance gate drops it before posting: a found-but-dropped

workflows/review/eval/runner.ts

@@ -124,6 +124,14 @@ export type RunResult = {
      * artifact), not in {@link RunResult.plannedReview}.
      */
     droppedByProvenance: RunCandidate[];
+    /**
+     * Near-miss mis-anchors the gate snapped to a changed line instead of
+     * dropping (anchor-snap; see `lib/provenance.ts`). Each entry's candidate
+     * carries the snapped anchor and ALSO flows through the pipeline (it is
+     * not a drop); this list is the audit trail — in production, the run
+     * artifact's `out/snapped.json`.
+     */
+    snappedByProvenance: {candidate: RunCandidate; originalAnchor: Anchor}[];
     /** Candidates dropped by the scope filter (out-of-scope, non-blocking). */
     droppedByScope: RunCandidate[];
     /** Candidates dropped as `refuted` by the validation replay (Phase 3). */
@@ -159,6 +167,13 @@ export type RunOptions = {
      * `[]` explicitly to replay a live run whose validator produced nothing.
      */
     validation?: CaseVerification[];
+    /**
+     * Whether the change-provenance gate runs its anchor-snap fallback
+     * (default true: production behavior). The live A/B passes false for an
+     * arm whose review.md predates the rule, so the deterministic gate
+     * emulates each arm's own prompt version and the A/B prices the change.
+     */
+    anchorSnap?: boolean;
 };
 
 /* -------------------------------------------------------------------------- */
@@ -352,14 +367,36 @@ export const runCase = (
     // production). Without a diff the gate is skipped (pre-gate behavior).
     let changeAnchored = allCandidates;
     let droppedByProvenance: RunCandidate[] = [];
+    let snappedByProvenance: RunResult["snappedByProvenance"] = [];
     if (corpusCase.diff !== undefined) {
         const provenance = computeDiffProvenance(corpusCase.diff);
         const gate = applyProvenanceGate(
             allCandidates.map((c) => c.finding),
             provenance,
+            options.anchorSnap === undefined
+                ? {}
+                : {anchorSnap: options.anchorSnap},
         );
         const keptIds = new Set(gate.kept.map((f) => f.id));
-        changeAnchored = allCandidates.filter((c) => keptIds.has(c.id));
+        const snappedById = new Map(
+            gate.snapped.map((snap) => [snap.finding.id, snap]),
+        );
+        // Re-normalise the snapped findings so their candidates carry the
+        // rewritten (snapped) anchor through the rest of the pipeline.
+        changeAnchored = allCandidates
+            .filter((c) => keptIds.has(c.id))
+            .map((c) => {
+                const snap = snappedById.get(c.id);
+                return snap === undefined
+                    ? c
+                    : toCandidate({source: c.source, finding: snap.finding});
+            });
+        snappedByProvenance = changeAnchored.flatMap((candidate) => {
+            const snap = snappedById.get(candidate.id);
+            return snap === undefined
+                ? []
+                : [{candidate, originalAnchor: snap.originalAnchor}];
+        });
         // Re-normalise the demoted findings so their candidates carry the
         // gate-coerced (never blocking) label.
         droppedByProvenance = allCandidates
@@ -454,6 +491,7 @@ export const runCase = (
         allCandidates,
         postedCandidates,
         droppedByProvenance,
+        snappedByProvenance,
         droppedByScope,
         droppedByValidation,
         postedLabels,

workflows/review/lib/diff.test.ts

@@ -140,6 +140,22 @@ describe("computeChangedLines", () => {
         ].join("\n");
         expect(computeChangedLines(diff)["a.ts"].added).toEqual([1]);
     });
+
+    it("records the last RIGHT-side line each file's hunks cover", () => {
+        const lines = computeChangedLines(GIT_DIFF);
+        // Hunk 2 covers new lines 41..43; the file's diff shows nothing past.
+        expect(lines["src/app.ts"].lastShownLine).toBe(43);
+        expect(lines["src/new.ts"].lastShownLine).toBe(2);
+    });
+
+    it("records each file's diff-text overhead", () => {
+        const lines = computeChangedLines(GIT_DIFF);
+        // 4 file-header lines + 2 hunk headers + 2 removed lines: the most a
+        // diff-text-counted anchor can overshoot the real file line.
+        expect(lines["src/app.ts"].textOverhead).toBe(8);
+        // 4 file-header lines + 1 hunk header, nothing removed.
+        expect(lines["src/new.ts"].textOverhead).toBe(5);
+    });
 });
 
 describe("countOrphanHunkLines", () => {