review: re-review fingerprint anchors on cache memory (body stamp never survives gh-aw ingest)

[jwbron]

Plan-of-record: The deterministic orchestrator. Production bug surfaced by the two-arm live trial of the orchestrator stack (Khan/webapp#41006/#41007); independent of the stack, so it targets main directly. The stack merges this branch in so both trial arms exercise it.

The bug

Every full-depth review renders a hidden HTML-comment fingerprint stamp into its review body (renderRereviewStamp), and the re-review plan CLI reads it back from prior-reviews.json to decide depth. That channel has never worked: gh-aw's safe-output ingest sanitizer strips ALL XML/HTML comments (removeXmlComments in gh-aw-actions setup/js/sanitize_content_core.cjs, a depth-tracking scan with no allowlist), so the stamp is deleted before the review posts.

Evidence, end to end, from the trial:

• The raw MCP call in a run's safeoutputs.jsonl contains the stamp; the ingested item in safe-output-items.jsonl does not (run 29893634730).
• None of Khan/webapp#40996's three production-posted reviews carries a stamp.
• 40996's re-review runs planned "reasons": ["no-prior-fingerprint"] and escalated to full depth (run 29872581602's rereview-plan.json), at 758 and 925 AI credits.

Consequence: the re-review ROUTING dial (scoped/flip-gated/fast), the flip gate, and the divergence tripwire are all silently inert in production; every repeat review pays the full roster. The dial was measured and shipped as the runs-per-PR cost lever; it has been buying nothing.

The fix

The fingerprint gets a second carrier that demonstrably works: the Step 9 cache-memory record (/tmp/gh-aw/cache-memory/pr-<n>.json), which already stores verdict, the reviewed hunk signature, and wasDraft on every run.

• stampFromCacheMemory reconstructs a stamp from that record, validating every field (a gap returns null and the plan degrades to full, never cheaper). It prefers a new stampHunks field and falls back to reviewedHunks.
• runRereviewPlanCli reads body stamps first (unchanged priority; they cost nothing and become load-bearing again if the sanitizer ever allows them through), then the cache record, and records which carrier anchored the plan as stampSource in rereview-plan.json for observability.
• Step 9 (review.md) gains stampHunks: copied verbatim from rereview-plan.json's code-computed value, never hand-computed. Step 1's added-lines-only hash is a different regime from the CLI's added-and-removed hash; mixing them would read as full divergence. reviewedHunks keeps serving comment scoping with Step 1's own hash on both sides.
• Hash-regime mismatches inside a stale cache record cannot be detected and surface as full divergence, i.e. a full review; cache eviction likewise degrades to full. Both are the pre-fix steady state.

Verification

19 new vitest cases: stampFromCacheMemory field validation (verdict, wasDraft, hunk-map shapes, stampHunks preference and fallback), and the CLI fallback end to end (cache anchor reduces depth with stampSource: "cache-memory", body stamp wins when present, no-anchor plans full, ready-for-review guard applies to a draft cache anchor, missing PR number and unparseable cache records degrade to full). Full review suite green (873 tests), typecheck clean, changed files lint clean.

[changeset-bot]

🦋 Changeset detected

Latest commit: 22455b1

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

[github-actions]

Review live A/B

Baseline: origin/main (review.md ea89dc34d0eb); candidate: working tree (review.md 227df7bd7d67).

Ruler: matcher deterministic+arbiter; corpus cc11988c0918 (9 cases).

Metric	Baseline	Candidate	Delta
Must-catch recall	100%	100%	+0%
Verdict agreement	100%	100%	+0%
Noise (unmatched posted)	61%	55%	-6%
Clean false flags	0	0
Judge mean quality	0.89	0.91	+0.02
Cost	$9.27	$9.38
Wall clock	1078s	1240s
Cases run / skipped	9 / 0	9 / 0
Misses found-but-dropped	0	0
Findings anchor-snapped	0	0

Adversarial hard gate: PASSED on the candidate arm.

Single-run-stable rows: recall, verdict agreement, regressions, adversarial gate. Judge quality and noise are not: they jitter run-to-run at this corpus size, and a regressed reviewer can score HIGHER on judge quality (fewer, surer comments each read better). Recall against the labeled specs is the load-bearing metric.

Measured noise floor (identical arms, run 29069228968, 2026-07-10, 6 arm-samples, full corpus x3, pre-arbiter; budget skips left the samples on unequal case sets, so these v1 bands also carry case-mix variance): must-catch recall 54%-86% (sd 10%), verdict agreement 75%-100% (sd 9%), noise (unmatched posted) 50%-60% (sd 3%), judge mean quality 82%-86% (sd 2%). A single-run delta whose arms both sit inside a band is indistinguishable from run-to-run wobble; use --repeats to resolve smaller effects.

[github-actions]

Review Guidance

github-actions (2 files)

File	Reason
rereview-mode.ts	Deterministic enforcement layer of the shared reviewer; this file now decides, on every consumer PR push, which carrier anchors re-review depth, so a validation bug changes how much review every consumer PR gets. The change fails toward full review, but the reviewedHunks fallback regime mismatch (see inline) affects the tripwire note/metric.
review.md	Executable reviewer prompt in every consuming repo; the Step 9 stampHunks instruction makes a model-written cache field the production fingerprint carrier for all consumers. Risky direction (a wrong fingerprint) fails toward more review, not less.

[github-actions]

suggestion (non-blocking): The reviewedHunks fallback anchors on a regime-mismatched signature, so on a pre-upgrade cache record (has reviewedHunks, no stampHunks) the run posts a false "divergence tripwire re-armed" note. In production reviewedHunks is Step 1's 64-char SHA over added lines only, but the CLI hashes 16-char truncations over added+removed lines (HUNK_HASH_CHARS), so they can never string-match. validSignature does no length/regime check, so the anchor validates, computeDivergence matches nothing (share 1.0), and decideReReviewDepth returns tripwire-divergence/tripwireRearmed: true — posting divergence tripwire re-armed a full review (unreviewed share 1.00) when nothing diverged, plus a phantom re-arm in the cost metric. Review coverage stays correct (still full), so this is observability, not blocking.

Two supporting notes: the code comment here claims a "scripted-mode staging layer" writes reviewedHunks in the CLI regime, but no such writer exists in this repo; and the new test seeds reviewedHunks: CURRENT (the CLI-computed signature), so the production mismatch is never exercised. Accepting only the CLI regime here also subsumes the stampHunks: "overflow" case (both degrade to the honest no-prior-fingerprint):

if (
    !Array.isArray(hashes) ||
    hashes.some(
        (hash) => typeof hash !== "string" || !/^[0-9a-f]{16}$/.test(hash),
    )
) {
    return null;
}

[github-actions]

suggestion (non-blocking): This makes a model-transcribed field the production fingerprint carrier, with no size bound or checksum — unlike the old body-stamp carrier, which was code-rendered with an overflow cap. Every miscopy fails toward full (safe) but silently, and the large/long-lived PRs where the dial pays most are the likeliest to be miscopied; the symptom is then indistinguishable from the pre-fix bug except via stampSource. The Step 6 stamp subcommand already runs with the decided verdict and reads rereview-plan.json, so it holds stampHunks/stampAnchorDraft/verdict and could write those cache fields deterministically, leaving the model only the fields it genuinely owns.

[github-actions]

suggestion (non-blocking): No test asserts stampFromCacheMemory accepts a REQUEST_CHANGES record and carries that verdict — the reconstruct test only uses APPROVE, and the REQUEST_CHANGES record seeded elsewhere is the body-wins case where the cache is never consumed. Fails toward full so it's low severity, but a one-line case would cover the accept-branch:

it("reconstructs a REQUEST_CHANGES anchor", () => {
    const stamp = stampFromCacheMemory(
        JSON.parse(cacheRecord({verdict: "REQUEST_CHANGES"})),
    );
    expect(stamp?.verdict).toBe("REQUEST_CHANGES");
});

[github-actions]

note (non-blocking): anchorDraft is reconstructed from wasDraft, which is the last run's draft status, whereas anchorDraft means the draft status when the anchor fingerprint was taken. Tracing both directions this is conservative-only (it can only add a full review, never miss one), but the docstring documents the depth: "full" approximation and not this one — worth a line so a future reader doesn't "fix" it by carrying stampAnchorDraft into the cache record without re-deriving the analysis.

[github-actions]

question (non-blocking): This ships as "review": patch, but the fix changes production re-review depth behavior (re-reviews that escalated to full will now anchor and run scoped/flip-gated/fast per the ROUTING dial). The two prior behavior-changing changesets (the payload seam and NOTIFIED) both shipped minor, while patch has been used for internal/eval-only fixes. Is patch intended here, or should this be minor?

[jwbron]

Superseded by #279, which absorbed this change wholesale as part of deterministic-orchestrator slice 0.

Verified file-by-file against main:

• .changeset/stamp-carrier-cache-memory.md and lib/rereview-mode.test.ts are identical.
• lib/rereview-mode.ts on main is a superset: same stampSource / stampHunks / cache-memory fallback logic, plus a refactor swapping the local splitHunks helper for the shared splitPatchHunks from ./diff.
• review.md carries the same semantics, reworded for the new pre-agent staging: the sanitizer/cache-fallback note now lives in the prior-reviews.json description and the stampHunks cache-record block is intact. The agent-facing "stage them anyway" instruction is gone only because staging prior reviews is deterministic code now (lib/stage-pr.ts).

Rebasing onto main drops this commit as already-applied, leaving a zero-diff branch. Closing.