Re-review only what changed since the last review, and say when it's done

CHANGELOG.md

@@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 - `npm run typecheck` fails on what a linter would warn about: unused locals and parameters, implicit returns, switch fall-through, missing `override`, `undefined` written into an optional property, dotted access to an index signature, unreachable code and unused labels. CI also runs the template's commit attribution check, on pull requests, merge queue entries and pushes to `main`.
 - Consumers pin the action by a release's full commit SHA, with the tag in a comment, as the README now says: CodeQL's `extended` suite reports a tag pin, even an immutable release's, as an unpinned action. This repository's own workflow pins v0.3.1 that way.
 - Each review posts its own summary comment, at the bottom of the conversation, and the previous summary collapses to one line that links to it, instead of the first summary being edited in place, so a `/review` or re-review shows its verdict and findings where it ran ([#21](https://github.com/NCodeGroup/code-review/issues/21)).
+- Re-reviews converge. A review of a pull request the bot has reviewed before reviews only the hunks of its merge-base diff that differ from that diff at the last reviewed head, and what the author took out since, so nothing a merge of the default branch brings in is reviewed, and a head a force-push dropped gets a full review. Only `correctness` reviews them, on Sonnet, with the earlier findings and the replies in their threads, and checks each unresolved finding against the code now. It posts only blockers and should-fixes on lines changed since the last review, never nits, and its summary says "No further findings above nits" when nothing at or above should-fix is left, and never approves when the earlier review threads can't be read or no reviewer saw what changed. With verification on, what a re-review would post goes through it as a first review's blockers and should-fixes do; a should-fix it can't settle becomes a nit and isn't posted, and a re-review whose findings it refuted never approves or says it's done. The first review is unchanged. A review whose reviewers all failed records the head reviewed before it, so the next re-review covers what it missed. Two eval cases replay re-reviews of #29 and #30, one with main merged into the head ([#24](https://github.com/NCodeGroup/code-review/issues/24)).
 - A review whose API credits run out partway no longer loses the reviewer that hit it: with a Bedrock key, that call reruns on Bedrock at the same tier and every later call goes straight there. The trace records both calls with their providers, and the check run and summary say "Switched to Bedrock partway: credits ran out" with the cost on each provider. A call that ends on an API error now counts as failed rather than as a run with no findings ([#49](https://github.com/NCodeGroup/code-review/issues/49)).
 - A missing test is a should-fix only when the untested path is new behavior with a plausible failure a user would see and no test below it covers it. A wrapper over tested pieces, a test that would only pin current behavior, or a second test for a covered path is a nit. Eval concerns can name a `minSeverity`, and five cases from [#30](https://github.com/NCodeGroup/code-review/pull/30) check both sides ([#37](https://github.com/NCodeGroup/code-review/issues/37)).
 - The inline threads read the pull request's earlier review comments through the same pager as the summary comment, up to 10,000 instead of 1,000. When a pull request has more, no threads are posted, with a warning, instead of findings already posted on comments past the limit being posted again; the check run still has every finding ([#38](https://github.com/NCodeGroup/code-review/issues/38)).

README.md

@@ -21,7 +21,7 @@ A rule in the learned guidance (`guidance/`) that came only from instruction fil
 
 Each model call has its own cost cap, and a whole review has a **cost ceiling** that guards against a runaway review only: $8 plus $4 for each part of the diff, so $12 for most pull requests and $32 for one reviewed in six parts, far above the $0.60 to $2.10 a normal review costs and the $6 to $7 of a 2 MB pull request. Once the calls so far reach it, the reviewer and rule-check calls that haven't started are skipped, each such reviewer shows `skipped: run budget` in the check run, synthesis, and verification when it's on, still run on what was found, and the summary warns that the review is incomplete and doesn't approve. To change it, set `CODE_REVIEW_MAX_RUN_USD` to a positive number of dollars in the calling job's `env`; any other value warns and keeps the default.
 
-**Verification** is off by default. `CODE_REVIEW_VERIFICATION=on` adds a stage after synthesis: each blocker and should-fix it merged goes to a Sonnet call with Read, Grep and Glob in the checkout, behind the same path guard as the reviewers, with its diff hunk and its reviewers' charters, a file's findings per call at up to $0.50 each. The call proves or refutes the finding with a concrete code path. A confirmed finding stays, with the path added to its evidence; a refuted one is dropped and listed in the check run under "Refuted by verification", with the reason; one it can neither prove nor refute stays as a question, one severity lower. Nits aren't verified. The summary, which synthesis wrote before verification, gets a sentence naming the findings verification refuted and counting the ones it made questions. Verification never makes a review an approval: a request for changes with no blocker or should-fix left becomes a comment, since a verifier misled by the change's own code can refute a real finding. The calls show as `verification` in the check run's cost table and the trace records each one. It stays off until an eval shows it catches as much at fewer blockers and should-fixes, for what it costs ([#41](https://github.com/NCodeGroup/code-review/issues/41)).
+**Verification** is off by default. `CODE_REVIEW_VERIFICATION=on` adds a stage after synthesis: each blocker and should-fix it merged goes to a Sonnet call with Read, Grep and Glob in the checkout, behind the same path guard as the reviewers, with its diff hunk and its reviewers' charters, a file's findings per call at up to $0.50 each. The call proves or refutes the finding with a concrete code path. A confirmed finding stays, with the path added to its evidence; a refuted one is dropped and listed in the check run under "Refuted by verification", with the reason; one it can neither prove nor refute stays as a question, one severity lower. Nits aren't verified. The summary, which synthesis wrote before verification, gets a sentence naming the findings verification refuted and counting the ones it made questions. Verification never makes a review an approval: a request for changes with no blocker or should-fix left becomes a comment, since a verifier misled by the change's own code can refute a real finding. A [re-review](#re-reviews)'s findings go through it too. The calls show as `verification` in the check run's cost table and the trace records each one. It stays off until an eval shows it catches as much at fewer blockers and should-fixes, for what it costs ([#41](https://github.com/NCodeGroup/code-review/issues/41)).
 
 ## Using it
 
@@ -136,6 +136,32 @@ Each review, all from `ncode-review[bot]`:
   review's summary collapses to one line that links to the new one, so the latest summary is at the bottom;
 - posts a neutral "Code review" check run with all the findings and annotations.
 
+### Re-reviews
+
+A review of a pull request the bot has reviewed before, such as a `/review` after changes, is a re-review, so rounds
+of review converge:
+
+- it reviews only what changed since the last review: the hunks of the pull request's diff against its merge base
+  that differ from that diff at the last reviewed head, which the summary comment records, and what the author took
+  out since. Merging the default branch in or rebasing onto it changes nothing by itself, and nothing from the
+  default branch is reviewed;
+- only `correctness` reviews them, on Sonnet, with the earlier findings and the replies in their threads (from people
+  with write access). It checks each unresolved finding against the code now (fixed, still open, or answered), and it
+  doesn't raise again what was fixed or answered with a reason, or ask to undo what an earlier finding asked for;
+- it posts a new finding only if it is a blocker or should-fix on a line changed since the last review, never a nit;
+- with [verification](#how-a-review-runs) on, those findings go through it as a first review's do: a refuted one is
+  dropped and listed in the check run, and one it can't settle is a question one severity lower, so a should-fix it
+  can't settle becomes a nit and isn't posted. A re-review whose findings verification refuted or made nits never
+  approves or says it's done, since the maintainer should look at what it dropped;
+- its summary lists the earlier findings it checked, and says "No further findings above nits" when nothing at or
+  above should-fix is left, new or unresolved, so the author knows they can merge. When the earlier review threads
+  can't be read, or what changed is only in generated files or files over the size budget, so no reviewer saw it, it
+  says so instead, and its verdict is never Approve.
+
+A `/review` of the commit already reviewed, or a last reviewed head that can't be fetched any more (after a force-push),
+gets a full review instead. A review whose reviewers all failed doesn't count as the last review: its summary records
+the head reviewed before it, so the next re-review still covers what it missed.
+
 ## Opting a repository in
 
 The review's two keys are organization secrets, stored once and visible only to the repositories that opted in:
@@ -172,6 +198,8 @@ The cases come from review comments the learned guidance has read too, so the ev
 
 `CODE_REVIEW_RULE_CHECKS=off` turns off the rule checks ([How a review runs](#how-a-review-runs)) to measure what they add. `CODE_REVIEW_VERIFICATION=on` turns on verification; the report counts the blockers and should-fixes posted, and the findings verification refuted, to compare with it off. Run each setup at least twice: a run of the first 16 cases varies by about 2 of their 55 expected findings.
 
+A case with a `reviewed` head is a [re-review](#re-reviews): it carries the findings earlier reviews posted, with their replies, as `earlier`, and `mergeIntoHead` merges a base branch commit into the head first, as `/ship-plan` does. Besides its must-not-flag concerns, a re-review case counts a violation for each finding it posts below should-fix or off the lines changed since its reviewed head, such as code the merge brought in. `re-review-declined-trigger` (#29) and `re-review-merge-of-main` (#30, with main merged in) are the two; `--only re-review-declined-trigger,re-review-merge-of-main` runs just them.
+
 **Precision.** Every posted blocker and should-fix that matches no expected concern goes to an Opus judge, which reads the case's checkout, locked down like the reviewers, with the finding passed as data, and answers `real`, `not-real` or `unsure`: is it a real defect the maintainer would act on? A finding that raises a must-not-flag concern counts as not real without a call. Precision is (matched + real) ÷ all posted blockers and should-fixes, with `unsure` counted apart. Verdicts are cached by case and finding fingerprint (file, severity and the title's words) in `evals/verdicts.json`, so a rerun judges only findings it hasn't seen; commit it with `cases.json` so every run is judged the same way. Each `evals/out/<label>.json` has the precision counts per case and in `summary.precision`, findings per case (`summary.findingsPerCase`: mean, max) and the review's own cost, without the eval's judges (`reviewCostUsd`, and `summary.reviewCostPerCase`).
 
 **Comparing two setups.** `npm run eval:compare -- <a.json> <b.json>` (or bare labels from `evals/out/`) prints recall, precision, findings per pull request and cost for both runs, then each case's differences. A recall difference of 2 or less is marked `(noise)`, within one run's variance; the other differences aren't marked, since their variance hasn't been measured. Outputs from before precision show `—` for it.

action.yml

@@ -18,7 +18,7 @@ inputs:
     required: false
     default: base
   github-token:
-    description: A read-only token (contents and pull requests), used to read the pull request a /review comment is on and to fetch the pull request's head.
+    description: A read-only token (contents and pull requests), used to read the pull request a /review comment is on, to fetch the pull request's head and the head the last review read, and to read the earlier review threads for a re-review.
     required: false
     default: ${{ github.token }}
   app-client-id: