Add a verification stage that proves or refutes each blocker and should-fix

Background. The autopilot (/run-issues, /address-feedback) acts on the bot's blocker and should-fix findings, so a false positive costs a whole agent round trip: a plan, an executor, a push and a re-review. The original reviewer design this engine borrows from has generation and judging in separate calls. Verification is meant as the main upgrade over it: a separate call that reads the code and tries to prove or refute each serious finding with a concrete code path.

The maintainer's stated preference is noisy over quiet: doubtful findings become questions, not drops. Verification must respect that. It drops a finding only when it refutes it with a concrete reason. A finding it can neither prove nor refute is kept as a question with lowered severity.

Current state. src/review.ts runs context → selection → specialists (plus rule checks) → synthesis. Nothing re-checks findings. Synthesis (prompts/synthesis.md) only rephrases doubts.

What to do.

Add a stage between specialists and synthesis (or after synthesis, on merged findings, which costs fewer calls). For each blocker and should-fix, a Sonnet 5.5 call with Read, Grep and Glob in the checkout and the same path guard (agent.ts). It gets the finding, the diff hunk and the charter it came from, and returns { verdict: confirmed | refuted | uncertain, path: string, reason: string }. Group findings by file into batches so the shared prefix is cached.
confirmed keeps the finding and appends the code path to the evidence. refuted drops it and records it in the trace and in the check run under "Refuted by verification", with the reason. Nothing disappears silently. uncertain rephrases it as a question ("Is X intended? If not, …") and lowers it one severity step.
Nits are not verified.
Add budgets.verification per call (start at $0.50 Sonnet) and record cost as role verification in the check run's cost table.
Add a switch CODE_REVIEW_VERIFICATION=off so evals can compare with and without.
The prompt treats finding text and code as data, like prompts/specialist.md.

Acceptance.

The eval run (node evals/run.ts, held-out guidance, 2 runs each) with verification on catches at least as many expected findings as with it off. Today that is 10 and 9 of 55 held out without rule checks.
It flags the must-not case no more often.
Posted blockers and should-fixes per case go down.
The check run lists refuted findings with reasons, and the trace has every verification call.
If the eval shows no gain for its cost, it stays off by default and the result is recorded in the issue (the eval-gated rule for upgrades).

Related. Depends on #51 (precision in evals) to show the benefit. #42 uses Opus for this on sensitive paths.

Filed from the 2026-10-31 audit of the NCodeGroup review-bot and project-template design against what was built. This issue stands alone: everything needed to do it is above.

Add an opt-in verification stage that proves or refutes each blocker …

Verify a re-review's blockers and should-fixes when verification is on

Plan snapshot. The working copy is PLAN.md in the plan's checkout; this comment is not updated as steps run.

Plan: Eval results, smoke baseline, outcome replies and v1.0.0 (8 steps)

Plan: Eval results, smoke baseline, outcome replies and v1.0.0

Issue: #41 #123 #135
Refs: #52 #97 #57

Goal

Record what the 2026-11-01 evals showed and act on them: verification goes on by default (#41), the re-review and synthesis measurements are written into the README's Evals section (#123, #135), and the smoke eval gets its first baseline (#52). The monthly report's hand count (#97) showed the outcome rules misread the autopilot's explicit replies, so a writer's Autopilot: fixed in <sha> and Autopilot: not changed now decide a finding's outcome. Last, the changelog's Unreleased entries move into a dated ## [1.0.0] section and the README states the v1 contract, so the Release workflow can cut v1.0.0 once this merges (#57).

Context for the executor

Build: npm run typecheck
Test: npm test (one file: node --test test/<name>.test.ts)
Node is not on PATH on this machine: start every command that runs node or npm with $env:PATH = "C:\Users\Marcin\AppData\Local\nvs\node\24.22.2\x64;$env:PATH";.
Conventions: Node 24 runs TypeScript directly; tsconfig.json is strict with exactOptionalPropertyTypes. Tests use node:test and node:assert/strict, one behavior per test, hermetic. UTF-8 without BOM, LF line endings. README prose: the section's own style (some sections wrap near 120 columns, the Evals section uses one long line per paragraph). Refer to issues in code comments as issue #N.
Gotchas: test/run-ceiling.test.ts's harness deletes CODE_REVIEW_VERIFICATION and sets it to on only when a test asks for verification; with verification on by default it must set off explicitly otherwise.

Decisions

Add a verification stage that proves or refutes each blocker and should-fix #41: verification is on by default; CODE_REVIEW_VERIFICATION=off turns it off; any other value, empty included, leaves it on. Evidence (smoke subset, production guidance, --precision off, 2 runs each): off caught 2/3 and 2/3, 0 must-not flags, 7 and 8 blockers and should-fixes, $4.14 and $4.04; on caught 2/3 and 2/3, 0 must-not flags, 5 and 5, $4.21 and $4.16 (verification calls $0.14, about 3%). This deviates from Add a verification stage that proves or refutes each blocker and should-fix #41's held-out full-set protocol for cost; the smoke subset is the gate the repository runs on every prompt change.
Measure synthesis at a lower effort or on Sonnet #135: synthesis stays on Opus at the SDK's default effort. synthesis=sonnet caught the same 2/3 with 0 must-not flags at half synthesis's cost ($0.42 against $0.89 for the 4 cases) and 45% less time (68 s against 122 s), but posted 12 blockers and should-fixes against Opus's 7 and 8, which the autopilot would act on. CODE_REVIEW_SYNTHESIS_EFFORT=medium saved nothing ($0.95, 134 s, 8 blockers and should-fixes).
Run the re-review evals and record what they show #123: the re-review subset (4 cases) cost $0.45 with verification off ($0.02 re-review-declined-trigger, $0.07 re-review-merge-of-main, $0.10 re-review-security-floor, $0.26 re-review-sensitive-path, the deep one on Opus) and $0.12 with it on (prompt cache warm: $0.006, $0.005, $0.022, $0.090). Neither run posted a finding, so nothing was verified; 0 must-not flags; re-review-sensitive-path's expected concern was missed both times. The log counted 0 merged and 0 near findings, so mergeOverlapping stays as it is.
Run a cost-capped smoke eval when a prompt, charter or model changes #52: the baseline is the proposed baseline from Smoke eval run 41278266855 on f9f8d1d (caught 2/3, must-not quiet), copied unchanged.
Outcomes (Roll out the 2026-10-31 changes: release, consumer pins, app permission, CodeQL #97 hand count, Test eval review cost and explain's answer #115's F1 and F2): among a finding's replies from writers, the last one that is an autopilot verdict decides first: Autopilot: fixed in <sha> (7 to 40 hex characters) is fixed when <sha> prefixes a commit the pull request gained after the finding's review (from the same comparison the line check uses); Autopilot: not changed is dismissed. A fixed in claim whose commit isn't among them is ignored, as today. Without such a verdict the existing rules apply unchanged.
v1.0.0: one dated section ## [1.0.0] - 2026-11-01 holds every Unreleased entry; ## [Unreleased] stays, empty. The release itself is cut by hand after merge (AGENTS.md).

Out of scope

Cutting the release, bumping pins, any other repository.
evals/cases.json, the smoke subset, the smoke workflow.
Older changelog sections.

Steps

[ ] 1. Turn verification on by default

Issue: Add a verification stage that proves or refutes each blocker and should-fix #41
Files: src/config.ts (edit), test/verify.test.ts (edit), test/run-ceiling.test.ts (edit)
Change: verificationOn returns value.trim() !== 'off'; replace its comment (or add one) with: verification is on unless CODE_REVIEW_VERIFICATION=off; the smoke evals of 2026-11-01 caught as much with it on, at 5 blockers and should-fixes against 7 and 8, for about 3% more (issue Add a verification stage that proves or refutes each blocker and should-fix #41). In test/verify.test.ts, rename the test "verificationOn is off unless CODE_REVIEW_VERIFICATION is on" to "verificationOn is on unless CODE_REVIEW_VERIFICATION is off" and assert ''→true, 'on'→true, 'yes'→true, 'off'→false, ' off '→false. In test/run-ceiling.test.ts, replace if (verification) process.env['CODE_REVIEW_VERIFICATION'] = 'on'; with process.env['CODE_REVIEW_VERIFICATION'] = verification ? 'on' : 'off'; and update the comment above the harness that says CODE_REVIEW_VERIFICATION=on for the run when true to say it sets on or off.
Done when: $env:PATH = "C:\Users\Marcin\AppData\Local\nvs\node\24.22.2\x64;$env:PATH"; npm test

[ ] 2. Make the Eval workflow and the README say verification is on by default

Issue: Add a verification stage that proves or refutes each blocker and should-fix #41
Files: .github/workflows/eval.yml (edit), README.md (edit)
Change: In eval.yml, the verification input's default becomes 'on' and its description CODE_REVIEW_VERIFICATION (on by default, as in production). In README's **Verification** paragraph (## How a review runs), change the opening **Verification** is off by default. \CODE_REVIEW_VERIFICATION=on` adds a stage after synthesis:toVerification is on by default; `CODE_REVIEW_VERIFICATION=off` turns it off. It is a stage after synthesis:, and replace its last sentence (It stays off until an eval shows …) with: The smoke evals of 2026-11-01 caught as much with it on as off, with 0 must-not flags either way, at 5 blockers and should-fixes against 7 and 8, for about 3% more (#41).In## Evals, in the paragraph that starts `` CODE_REVIEW_RULE_CHECKS=off``, change ``CODE_REVIEW_VERIFICATION=onturns on verification; `` to ``CODE_REVIEW_VERIFICATION=offturns verification off; ``. In theEval workflow.paragraph, sayverificationdefaults toon`.
Done when: if (Select-String -Path README.md -Pattern 'Verification\*\* is off by default' -Quiet) { exit 1 }; if (-not (Select-String -Path .github/workflows/eval.yml -Pattern "default: 'on'" -Quiet)) { exit 1 }

[ ] 3. Record the 2026-11-01 eval results in the README

Issue: Run the re-review evals and record what they show #123 Measure synthesis at a lower effort or on Sonnet #135 Add a verification stage that proves or refutes each blocker and should-fix #41
Files: README.md (edit)
Change: In ## Evals, after the **Eval workflow.** paragraph, add a paragraph **Results, 2026-11-01.** (one long line) with exactly the numbers in this plan's Decisions for Add a verification stage that proves or refutes each blocker and should-fix #41, Measure synthesis at a lower effort or on Sonnet #135 and Run the re-review evals and record what they show #123: the smoke subset with verification off and on (two runs each); synthesis on Sonnet and at medium effort against Opus at the default, and the decision to keep Opus at the default effort; the re-review subset's cost per case off and on, no findings, 0 must-not flags, re-review-sensitive-path's expected concern missed both times, and 0 merged and 0 near findings, so mergeOverlapping doesn't merge near ones. Give the runs as links: https://github.com/NCodeGroup/code-review/actions/runs/<id> for 41278266855 and 41280733222 (off), 41278712152 and 41280198652 (on), 41279253481 (Sonnet), 41279660175 (medium effort), 41278561619 and 41278639968 (re-review off and on). Total spend $24.83.
Done when: if (-not (Select-String -Path README.md -Pattern '\*\*Results, 2026-11-01\.\*\*' -Quiet)) { exit 1 }; if (-not (Select-String -Path README.md -Pattern 'actions/runs/41279253481' -Quiet)) { exit 1 }

[ ] 4. Commit the smoke eval's first baseline

Issue: Run a cost-capped smoke eval when a prompt, charter or model changes #52 (refs)
Files: evals/baseline-smoke.json (new)
Change: Copy c:\Users\Marcin\.copilot\session-state\b980c55e-e344-454a-84d5-f7aa36510908\files\proposed-baseline-smoke-41278266855.json to evals/baseline-smoke.json byte for byte (Copy-Item), then make sure it ends with LF line endings (it is JSON written by Node; if it has CRLF, convert to LF).
Done when: $b = Get-Content -Raw evals/baseline-smoke.json | ConvertFrom-Json; if ($b.summary.caught -ne 2 -or $b.summary.violations -ne 0 -or @($b.cases).Count -ne 4) { exit 1 } else { exit 0 }

[ ] 5. Let the autopilot's explicit replies decide a finding's outcome

Issue: Roll out the 2026-10-31 changes: release, consumer pins, app permission, CodeQL #97 (refs)
Files: src/outcomes.ts (edit), test/outcomes.test.ts (edit)
Change: Add to Evidence the optional field commitsAfter?: string[] with the comment // The commits the pull request gained after the finding's review, from the same comparison. Add function autopilotVerdict(replies: Evidence['replies']): { fixedIn: string } | 'not changed' | undefined that scans the writers' replies in order and keeps the last match of /^\s*Autopilot:\s*fixed in\s+([0-9a-f]{7,40})\b/i (→ { fixedIn: <sha lowercased> }) or /^\s*Autopilot:\s*not changed\b/i (→ 'not changed'). At the top of classify: const verdict = autopilotVerdict(evidence.replies.filter((r) => r.writer)); then if (verdict === 'not changed') return 'dismissed'; and if (verdict && evidence.commitsAfter?.some((sha) => sha.toLowerCase().startsWith(verdict.fixedIn))) return 'fixed';, before the existing rules, which stay as they are. Update the rules comment above classify to list this first rule. In collectOutcomes, type the comparison as { files?: ComparedFile[]; commits?: { sha: string }[] }, keep a second map commitsAfter: Map<string, string[]> filled with comparison.commits?.map((c) => c.sha) ?? [] (and [] for a commit equal to head or a failed comparison), and pass commitsAfter: commitsAfter.get(reviewedAt) ?? [] to classify. Tests: "a writer's Autopilot: fixed in is fixed when the pull request gained that commit after the review" (linesChanged: false, isResolved: true, commitsAfter: ['5d49e7e0123…'] 40 hex, reply Autopilot: fixed in 5d49e7e. Tests added. → fixed); "a fixed-in claim naming no later commit falls back to the other rules" (same with commitsAfter: [] → dismissed); "a writer's Autopilot: not changed is dismissed even when the line changed" (linesChanged: true → dismissed); "the latest autopilot verdict decides" (not changed then fixed in <sha> with the sha in commitsAfter → fixed); "a non-writer's autopilot verdict is ignored" (not changed from writer: false, linesChanged: true → fixed). Add one collectOutcomes test, following the file's existing collectOutcomes tests and their fake API, where the comparison returns commits: [{ sha }] and no changed line, and a writer's Autopilot: fixed in <first 7 of sha> reply makes the finding fixed.
Pattern: the existing classify and collectOutcomes tests in test/outcomes.test.ts.
Done when: $env:PATH = "C:\Users\Marcin\AppData\Local\nvs\node\24.22.2\x64;$env:PATH"; node --test test/outcomes.test.ts; if ($LASTEXITCODE) { exit 1 }; npm run typecheck

[ ] 6. Describe the autopilot replies in the README's Outcomes section

Issue: Roll out the 2026-10-31 changes: release, consumer pins, app permission, CodeQL #97 (refs)
Files: README.md (edit)
Change: In ### Outcomes, in the table, append to the fixed row's text: Or a writer's reply \Autopilot: fixed in ` naming a commit the pull request gained after the finding's review, even when the fix is on other lines, such as a test.and to thedismissedrow's text: Or a writer's reply `Autopilot: not changed`, even when the line changed for another reason.Replace the sentenceA reply starting `Autopilot: fixed in` counts only when the finding's line changed, which already makes it fixed.with:Of a writer's replies, the latest `Autopilot: fixed in ` or `Autopilot: not changed` decides before the other rules; a `fixed in` reply that names no commit after the review counts for nothing.Change the sentence before the table,The first rule that holds decides:, to The first rule that holds decides, after the autopilot's replies below:` only if needed for the text to read correctly; otherwise leave it.
Done when: if (-not (Select-String -Path README.md -Pattern 'Autopilot: not changed' -Quiet)) { exit 1 }; if (Select-String -Path README.md -Pattern 'counts only when the finding.s line changed, which already makes it fixed' -Quiet) { exit 1 }

[ ] 7. State the v1 contract in the README

Issue: Release v1.0.0 and freeze the caller–engine interface #57 (refs)
Files: README.md (edit)
Change: In ## Interface, change which v1.0.0 will freeze to which v1.0.0 freezes, and replace the sentence Before 1.0.0 a minor release may still change it, and its changelog section says how. with Releases before 1.0.0 changed it in minor releases; their changelog sections say how. In ### Dependabot and pre-releases, add a first paragraph: from v1.0.0 on, releases aren't marked as pre-releases, so Dependabot's github-actions updater proposes each one in a consumer's action updates; project-template gives the bump a Dependabot group of its own and a workflow that auto-merges it for a minor or patch release once checks pass (a major release changes this interface, so a person merges it). Then in the paragraph that starts Until releases stop being marked as pre-releases, bump each consumer by hand, change its start to A consumer that pins a v0 release moves to v1.0.0 by hand, since Dependabot doesn't propose a release when the pinned one is hidden as a pre-release: keeping the rest of that paragraph's instructions.
Done when: if (-not (Select-String -Path README.md -Pattern 'which v1\.0\.0 freezes' -Quiet)) { exit 1 }; if (Select-String -Path README.md -Pattern 'Until releases stop being marked as pre-releases' -Quiet) { exit 1 }

[ ] 8. Write the changelog entries and the 1.0.0 section

Issue: Add a verification stage that proves or refutes each blocker and should-fix #41 Run the re-review evals and record what they show #123 Measure synthesis at a lower effort or on Sonnet #135
Files: CHANGELOG.md (edit)
Change: First add these entries under ## [Unreleased], at the end of their groups: ### Changed: - Verification is on by default; \CODE_REVIEW_VERIFICATION=off` turns it off. On the smoke evals it caught as much as without it, with no must-not flag, at 5 blockers and should-fixes against 7 and 8, for about 3% more. The Eval workflow runs it by default too (#41).and- The README's Evals section records the 2026-11-01 results: verification on and off, synthesis on Sonnet and at medium effort, which stays on Opus at the default effort since Sonnet posted more serious findings, and the re-review cases' cost, with no overlapping findings for `mergeOverlapping` to merge (#123, #135). ### Added: - The smoke eval's first baseline, `evals/baseline-smoke.json`, from the run on `main` after the decisions rule was narrowed: caught 2 of 3, the must-not case quiet (#52). ### Fixed: - A finding's outcome follows a writer's latest `Autopilot: fixed in ` reply when the pull request gained that commit after the review, even when the fix is on other lines, and a writer's `Autopilot: not changed` reply, even when the line changed for another reason; the monthly report's hand count found both counted the other way (#97).Then insert## [1.0.0] - 2026-11-01right below## [Unreleased](with one blank line between), move every group and entry that## [Unreleased]held into it unchanged and in order, and leave## [Unreleased]empty. Change the link[unreleased]: v0.4.0...HEAD` to [unreleased]: https://github.com/NCodeGroup/code-review/compare/v1.0.0...HEAD, and add [1.0.0]: https://github.com/NCodeGroup/code-review/compare/v0.4.0...v1.0.0 above the [0.4.0]: line. (Write the backticks literally.)
Done when: $env:PATH = "C:\Users\Marcin\AppData\Local\nvs\node\24.22.2\x64;$env:PATH"; node tools/release-notes.ts 1.0.0 > $null; if ($LASTEXITCODE) { exit 1 }; $t = Get-Content -Raw CHANGELOG.md; if ($t -notmatch '(?m)^## \[Unreleased\]\n\n## \[1\.0\.0\] - 2026-11-01$' -or $t -notmatch '(?m)^\[1\.0\.0\]: ') { exit 1 }; npm test

Review checklist

Verification's default flips only through verificationOn; off still turns it off in production and in the evals.
The outcome rules change only when a writer's autopilot verdict exists; a non-writer's never counts; an unverifiable fixed in claim changes nothing.
The baseline file is the run's proposed baseline, unchanged.
The 1.0.0 section holds every entry Unreleased held, and node tools/release-notes.ts 1.0.0 prints it.

Navigation Menu

Repository navigation

Add a verification stage that proves or refutes each blocker and should-fix #41

Plan: Eval results, smoke baseline, outcome replies and v1.0.0

Goal

Context for the executor

Decisions

Out of scope

Steps

[ ] 1. Turn verification on by default

[ ] 2. Make the Eval workflow and the README say verification is on by default

[ ] 3. Record the 2026-11-01 eval results in the README

[ ] 4. Commit the smoke eval's first baseline

[ ] 5. Let the autopilot's explicit replies decide a finding's outcome

[ ] 6. Describe the autopilot replies in the README's Outcomes section

[ ] 7. State the v1 contract in the README

[ ] 8. Write the changelog entries and the 1.0.0 section

Review checklist

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Add a verification stage that proves or refutes each blocker and should-fix #41

Description

Activity

polewskm commented on Nov 1, 2026

Plan: Eval results, smoke baseline, outcome replies and v1.0.0

Goal

Context for the executor

Decisions

Out of scope

Steps

[ ] 1. Turn verification on by default

[ ] 2. Make the Eval workflow and the README say verification is on by default

[ ] 3. Record the 2026-11-01 eval results in the README

[ ] 4. Commit the smoke eval's first baseline

[ ] 5. Let the autopilot's explicit replies decide a finding's outcome

[ ] 6. Describe the autopilot replies in the README's Outcomes section

[ ] 7. State the v1 contract in the README

[ ] 8. Write the changelog entries and the 1.0.0 section

Review checklist

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions