Review bot: read every non-generated diff, and check that it did

[Deco354]

Note

Written by Claude Code at the author's direction. Do not merge until the pre-release test below has passed: merging moves v1 and releases to every consuming repo.

Implements #11. Leave the issue open until the two-week coverage check in its "Done when".

Why

On Deco354/factory-farm-em#29, all seven approvals (2026-10-06..08) said they had read only part of the PR. None of them read analysis/summary.py, which computes the PR's headline numbers. @claude reviews of the unread files then found a bug the approvals had missed. Across the consuming repos, 15 of 134 approvals since 2026-09-01 admit to unread code.

The cause is that gh pr diff was the review's only view of the diff, and it fails on large PRs:

• Above Claude Code's tool-output limit, the output arrives as a 2 KB preview plus a saved file.
• It takes no path argument, so the review can't ask for one file at a time.
• Nothing told the review to open the saved file.

All seven #29 runs tried to fetch a per-file diff, were refused, and approved after reading one to three files. The full diagnosis, with the Sonnet 5 vs 5.5 comparison, is in #11.

What changes (claude-pr-review.yml)

1. Write per-file diffs (new step). The step diffs the pull_request merge commit against its base parent with git diff. Checkout moves from fetch-depth: 1 to 2 for this.
   • Each changed path gets its own file under .claude-review/diffs/.
   • generated_paths patterns are excluded, so those files get no diff. Patterns may be separated by spaces or newlines, so a YAML | block works (d2dc44a; before, only its first line was used).
   • Binary files are listed but not diffed.
   • It also writes coverage.jq, the one definition of "read": the lines that successful Read calls returned cover every line of the diff. The count uses each result's startLine/numLines, not the call's limit, because Read stops at its 25,000-token cap, and not always with an error:
     • On the runner (Claude Code 2.1.294), an explicit limit: 2000 read over the cap returned an error.
     • Locally (2.1.295), a read with the default limit returned lines 1–935 of 2,000 as a normal result, marked only truncatedByTokenCap. A result with no line counts falls back to the call's offset/limit.
2. Compose review prompt.
   • The prompt lists every diff file and asks for each to be read to its last line, before anything else.
   • Tests, docs and configs are named as in scope.
   • The old "focus on the changed code only" and "you do not need [the tests]" lines now apply only to what is reported and to test results.
   • It no longer points the review at gh pr diff or at the old "skim the generated hunks" rule. Generated and binary files are listed by name only.
   • The review body is written for the PR author and describes the change, not how the review was done: no diff files, no counts and no gate. All three gated approvals on #40 had opened with "I read every diff in this PR to its last line" or similar.
   • Inline comments go on the changed file's own lines, not on the diff file's.
   • Long diffs are read about 800 lines at a time, to the diff's own line count. Read refuses a page over 25,000 tokens.
   • The tool wording is corrected. The old prompt named the Grep and Glob tools, which Claude Code no longer has, and claimed every other Bash command is denied, which read-only ones no longer are. It now asks for grep through Bash to search, and Read for diffs and everything else, because coverage counts only Read.
   • The verdict rules are unchanged.
3. Hold the verdict until every diff is read (new). The prompt alone does not get every diff read (see the results below), so this step writes a Claude Code PreToolUse hook and passes it through the action's settings input.
   • While any diff is unread, the hook refuses gh pr review and names the unread diffs, with their unread line ranges, e.g. "not read yet: line 4890". The follow-up prompt uses the same list.
   • It refuses at most twice per job, then lets the verdict through.
   • It also lets the verdict through whenever it cannot tell, for example with no transcript or a jq error. The step is continue-on-error.
   • It decides when the verdict is posted, never what it says.
4. Measure which diffs the review read (new). It reads the action's execution file.
5. Review the diffs the first pass did not read (new; runs only if any are unread). This is now a backstop.
   • A second action run resumes the review's session, using --resume with the action's session_id output.
   • It reads the remaining diffs and files one more verdict for the whole PR.
   • It is continue-on-error, so a failure here cannot take the first verdict down.
6. Report review coverage (new).
   • It writes "Read N of M", plus how many times the verdict was held back, to the job summary.
   • Any diff still unread gets a ::warning:: and a PR comment with a ready-to-paste @claude prompt. The comment is posted with GITHUB_TOKEN, so it triggers nothing itself; a human pastes the prompt.
7. --effort high on both passes. Claude Code defaults Sonnet 5.5 to medium and Sonnet 5 to high. At high, Sonnet 5.5's first pass read 24 of 25 diffs before its first verdict attempt, against 9–13 at medium (results below).

Coverage is report-only by decision: it never fails the check, and a verdict is never withheld or downgraded for unread files. These are unchanged: the verify step's jq, the tool allowlist (repeated verbatim for the follow-up pass), the escalation step, and the job's lack of a permissions: block. No input is renamed and callers need no change. The one behaviour change for callers is that generated_paths is now read as whitespace-separated git globs; a pattern with no slash matches at any depth. Both values in use today, outputs/** and uv.lock, already fit.

The README (what a repo gets, the inputs, a troubleshooting row, the changelog) and CLAUDE.md are updated to match. CLAUDE.md also loses two outdated statements: the contradiction guard that was removed on 2026-09-30, and the claim that the pipeline repo runs its own copies of these workflows (it has called @v1 since its #169).

generated_paths no longer suggests lockfiles (6b3f221). Under this PR a withheld file gets no diff, so the review cannot see it at all; before, its changes were in the diff it was told to skim. A lockfile's changes are small (factory-farm-em's uv.lock: 2–56 lines over its last 100 PRs, apart from its 4,885-line creation). They are also the only place indirect dependency and source changes show. So the input description, README and caller template drop uv.lock as an example, and the template's out-of-date "skim committed generated data" comment is corrected. Companion PRs:

• Review-bot template: generated_paths leaves files out entirely; not for lockfiles .github#4 mirrors the template edit.
• Review bot: review uv.lock instead of withholding it Deco354/factory-farm-em#44 stops withholding its uv.lock.

Cost: on the 24-diff replay of #29, a gated first pass that reads everything takes 29 turns and $0.46–0.48 in one pass. With a 4,890-line file added, it takes 35–38 turns and $0.85–0.98. Before the gate, the first pass cost $0.22–0.31 and the follow-up brought the reported total to $0.52–0.59. The September reviews cost $0.49–0.99 on PRs of 2,000–6,000 lines.

Release plan: test on a branch ref, not a new tag

This is not a breaking change, so it stays on v1. To test it before every consumer picks it up, the PR stays unmerged while one repo calls the branch directly: uses: sentfutures/devops/.github/workflows/claude-pr-review.yml@review-every-file. Fix-up pushes to this branch reach that repo straight away. Once the test passes, merge (which moves v1), point the test repo back at @v1, and only then delete the branch. A caller that names a deleted ref fails at startup and produces no check at all.

After merging, also merge sentfutures/.github#4 (this PR's template edit) and #3 (devops#9's template edit, open since 2026-09-30). With both merged, the org template matches callers/claude-pr-review.caller.yml byte for byte.

Pre-release results on Deco354/factory-farm-em (2026-10-08)

The caller there points at @review-every-file (Deco354/factory-farm-em#39). Draft Deco354/factory-farm-em#40 replays #29's final diff: 25 files, +2,526/−137, so 24 diffs plus uv.lock as generated.

Without the gate, the first pass skips half or more. Every run read the source diffs it judged important and stopped. It never opened a test file or a doc.

Run	Prompt	First pass	Its own claim	Follow-up
1, dc952ae (37807910253)	List + "Read EVERY diff"	12 of 24	"Reviewed all 23 hand-written diffs"	read the other 12; 24 of 24
2, c8fa570 (37808249622)	Same, plus a temporary CLAUDE.md note saying "never read run.py", meant to force the warning path; the note was ignored and has since been removed	12 of 24	"I skimmed the docs and test diffs" (it never opened them)	read the other 12; 24 of 24
3, a029fe3 (37823923183)	Rewritten prompt (8bbedcb): read everything first, tests and docs named	7 of 24	"I did not read every diff"	read the other 17; 24 of 24

Each follow-up, handed the list of what was unread, read all of it in one turn. That is why the gate hands over the list before the verdict.

With the gate (4987b9c), 3 of 3 runs read 24 of 24 in the first pass:

Run	Read before the first gh pr review	Held back	Then	Follow-up	Verdicts posted
2193455 (37828965409)	9	once	read the other 15 in one turn, rewrote the summary	none	1 APPROVED
ba2a788 (37829178598)	4	once	read the other 20 in one turn, rewrote the summary	none	1 APPROVED
639447b (37829415543)	6	once	read the other 18 in one turn, rewrote the summary	none	1 APPROVED

• The refusal works. The agent sees PreToolUse:Bash hook error: … Not yet: your review has not read these diffs to the end … as a failed tool call. The refused call posts nothing.
• Each approval is honest. Each says it read every diff to the end, and covers the tests and docs.
• Turns and cost: 29 turns and $0.46–0.48 each.

With one diff over Read's token cap (5601533, 2026-10-09). #40's largest diff was 543 lines, so the runs above never reached Read's 25,000-token cap. To test it, 9f0465e adds a test-only docs/archive/snapshot-2026-10.md: 4,884 lines (125 KB) of real repo files concatenated, which makes a 4,890-line diff.

Run	Read before the first gh pr review	Held back	Long file	Follow-up	Verdict	Turns, cost
9f0465e (37911979471)	10 of 25	once	limit: 2000 errored at 40,474 tokens; then paged 1–1000 … 4001–4891	none	1 APPROVED	35, $0.85
a06a125 (37912287240)	10 of 25	once	read 1–400; asked 2,000, 1,400 and 1,100 lines from 401, each over the cap; then 401–1300 … 4001–4891 in 900-line pages	none	1 APPROVED	38, $0.98

• Both read 25 of 25 in the first pass. Both flagged the file's own "Test-only … Not for merge" header as a non-blocking note, so they did read it.
• Neither review body mentions diffs or how it read them.
• Why the count changed: the committed count, given this session's own transcript of a default Read on a 2,000-line diff (cut off at line 935 by Claude Code 2.1.295), scores that diff as read. The new count scores it as unread.
• Logs: the job log cuts very long lines, so some Read results there lose their line counts. The coverage steps read the full execution file, not the log.

Sonnet 5 on the same PR and workflow (2026-10-09, via Deco354/factory-farm-em#41). These runs are on 5601533. Each first pass is compared on what it read before its first gh pr review attempt.

	Sonnet 5.5, 9f0465e / a06a125	Sonnet 5, 4c3f376 (37913331151) / ae963d9 (37914095042)
Read before the first verdict attempt	10 of 25	24 of 25, plus samples of the long file (lines 1–80, 2400–2459 and 4850–4889 in run 1)
Source read for context	none	7 and 2 reads
Held back	once	twice, the limit
First pass	25 of 25	24 of 25: its paging stopped 1 and 7 lines short of the long file's end (it read to 4889 of 4,890, then to 4883, using the file's 4,884 added lines)
Follow-up pass	none	ran, read those lines, and posted a second approval ("This replaces my earlier review of this commit.")
Turns, cost, time	35 / 38, $0.85 / $0.98, 37 / 46 s	65 / 52, $2.54 / $1.97, 303 / 295 s
Flagged the "Not for merge" header	both	run 1 only

• No quality comparison yet: neither model reported a problem. #40 has no known bug in it, because the max_tokens bug @claude found on #29 is fixed in its final diff.
• The hold message named diffs, not the unread lines: Sonnet 5's two follow-ups were each for the last few lines of one file. Replaying run 1's reads through the new message, its second hold would have said "not read yet: line 4890".

Unread lines named, 800-line pages (c89d871, Sonnet 5.5, 2026-10-09). Deco354/factory-farm-em#43 removed the model: line again.

Run	Read before the first gh pr review	Held back	Long file	Follow-up	Verdict	Turns, cost, time
b52dd90 (37916190371)	13 of 25	once	800-line pages 1 … 4801, with no failed reads	none	1 CHANGES_REQUESTED	40, $0.94, 51 s
f31f208 (37916389367)	9 of 25	once	800-line pages, the last sized to end at 4,890	none	1 CHANGES_REQUESTED	37, $0.93, 39 s

• Both verdicts are about the test file. Both asked for it to be removed, because its header says "Not for merge". Before, all four runs had made that a non-blocking note.
• Inline comments land on the right lines. Both put their comment on line 3 of snapshot-2026-10.md itself, the file's own line rather than the diff's.

Why it started now: effort changed with the model. Claude Code's docs (model-config, "Default effort levels") say: "high on every model that supports effort, except that Opus 5.5, Sonnet 5.5, and Haiku 5.5 default to medium". Effort controls adaptive reasoning, which "lets the model decide whether and how much to think on each step". So when the action's default moved to Sonnet 5.5, the review also moved from high to medium.

• Thinking per pass: Sonnet 5 used 3,701–16,089 thinking tokens (September and today). Sonnet 5.5 used 53–2,353 (October to today).
• An undocumented flag: every Sonnet 5.5 run's init message has per_turn_effort_active: true, and Sonnet 5's has false.

Sonnet 5.5 with --effort high (2026-10-09). This separates effort from the model. --effort high was added to claude_args in temporary [skip ci] commits on this branch: 4cefbcb on this workflow, and 5e4c691 on main's workflow, which has no per-file diffs. f47e495 reverted both, and its tree is identical to c89d871.

Workflow	Run	Read before the first gh pr review	Held back	Long file / saved diff	Verdict	Turns, cost, time
this branch	a040b2e (37930313238)	24 of 25 (the long file only to line 120), plus 2 source reads	once	800-line pages to the end	APPROVED	41, $1.04, 69 s
this branch	e7f960d (37930671913)	24 of 25 (the long file only to line 60), plus 2 source reads	once	800-line pages to the end	APPROVED	40, $0.99, 67 s
main	49a5a47 (37930925223)	no gate	—	gh pr diff saved 287.6 KB, 8,363 lines. It read lines 6315–7024 (the diffs of cli.py, config.py, export/rows.py, export/schema.py, run.py) and 2 source files	APPROVED	13, $0.27, 28 s
main	488c1b6 (37931183181)	no gate	—	ran gh pr diff four times (195–355 KB each) and opened none of the saved files. It read 6 source files	APPROVED: "Skimmed the configs, docs and tests"	18, $0.33, 38 s

• Effort explains most of the first-pass gap. On this workflow, high took Sonnet 5.5 from 9–13 of 25 diffs read before its first verdict attempt to 24 of 25, as Sonnet 5 did. The gate was needed only for the 4,884-line test file, which both runs had sampled.
• Effort does not fix the original bug. On main's workflow, high still approved after reading 5 of 25 diffs (run 1) or none (run 2). So the per-file diffs are still needed.
• Cost of high on this workflow: $0.99–1.04 and 67–69 s, against $0.93–0.94 and 39–51 s at the default. Adopted in af20b0f (item 7 above).

Found along the way, fixed in af20b0f's prompt:

• The prompt names tools the review doesn't have. It says to use "the Read, Grep, and Glob tools". Since at least Claude Code 2.1.293, the init tool list has neither Grep nor Glob. A call is refused with "No such tool available: Grep … search file contents with grep via the Bash tool instead".
• Read-only Bash commands get past the allowlist. On 2.1.295 they ran despite the gh pr-only allowlist: grep, sed -n and tail all returned output. python3 -c got "This command requires approval". Sonnet 5 read parts of the long diff with sed -n '1,40p' and tail -40 (37914095042).
  • The prompt's "any other Bash command is denied" is no longer true.
  • Coverage counts only Read, so diff lines read this way show as unread.

af20b0f on #40 (44aba8e, run 37932473654, Sonnet 5.5, high by default now).

• First pass: before its first verdict attempt it read 24 of 25 diffs with Read, and the long file's lines 61–4,890 in 800-line pages. It read lines 1–60 of that file with head -60 through Bash, despite the new wording.
• Hold: the gate held the verdict once, for those 60 lines. The review re-read them with Read and approved.
• Search: it used grep through Bash once. No "No such tool" errors.
• Result: 25 of 25 read in the first pass, with no follow-up. One APPROVED, with an inline comment on the test file's header. 39 turns, $1.08, 68 s.

Anthropic's own setup doesn't cover this either (checked 2026-10-09). Claude Code 2.1.295's /install-github-app installs a claude-code-review.yml that runs the code-review@claude-code-plugins plugin.

• Settings: it sets no model or effort, so it also runs Sonnet 5.5 at medium.
• How it reads: it reads the PR only through gh pr diff, with "4 agents in parallel to independently review the changes" (two of them Opus). One of them is told to "Focus only on the diff itself".
• No coverage check: nothing assigns files or checks coverage.
• No verdict: it posts comments, never an approval.
• Large PRs: its README's only advice is "Consider splitting large PRs into smaller ones".
• The preview limit: Claude Code's bashOutputMaxChars setting raises the 30,000-character ceiling to at most 128,000 characters, which is still under #29's 133–193 KB diff.

Warning comment: it has not been triggered live; with the gate it should be rarer still. I tested it locally by running the step's script with a stand-in gh, in two cases: a failed follow-up, and two passes that still left diffs unread. Both produced the ::warning::, the job summary and a single comment POST to the PR's issue-comments endpoint.

How to test

Already done locally:

• actionlint and shellcheck pass on all workflows.
• The "Write per-file diffs" and "Compose review prompt" scripts were run against a synthetic merge commit:
  • Diffs were written correctly for a rename, a deletion, [x].txt (taken literally, not as a glob) and dir with space/R&D ü.md.
  • outputs/** and uv.lock (at the root and in pkg/) were listed as generated, and the binary file was listed only.
  • A change on the base side was correctly left out.
• The coverage jq was run against the message stream rebuilt from the b55bf02 #29 run log:
  • It marks harvestbench.py as read and rows.py (lines 1–80 of 461) as unread.
  • Over two passes it adds up the line ranges. It treats a gap between ranges, an errored Read and a Grep call as not read, and handles relative paths and string offsets.
• The new count (5601533) was run against #40's execution files. It gives the same results as before: 12, 12 and 7 of 24 for the ungated first passes, 24 over both passes, and 24 for each gated run. Synthetic Read results check each range rule:
  • a page cut off at line 935 counts as unread;
  • three pages that meet count as read, with either key spelling (tool_use_result in the execution file, toolUseResult in the transcript);
  • a gap between pages, an errored Read, and a result without line counts each count as unread;
  • a result with no structured form falls back to offset/limit.
• The gate step's script and its hook were run against #40 run 3's first pass as a JSONL transcript, with a half-written last line:
  • With 7 of 24 read, the hook refused twice, naming the 17 unread diffs, then let the third call through.
  • It let the verdict through when everything had been read, and when the transcript was missing.
  • It ignored gh pr view, gh pr reviewx and other commands.
  • It caught gh pr review with leading spaces and with --request-changes.

Selftest on this PR:

• 6025705 (run 37791137234): the action ran the full review. The self-skip seems to apply only when the caller file changes. It read 3 of 3 diffs and approved. Its one note, that the measure step could fail the job, was fixed in fc59ec5.
• 4987b9c (run 37828944154): it read 3 of 3, the gate held it back 0 times, and it approved. Its note that the README said coverage "never changes the verdict" was right; 87cb7ca fixes the wording.
• 5601533 (run 37911901330): it read 3 of 3, with no hold, and approved. The body describes the change, not the reading. Its note that the changelog entry was dated 10-08 but cited a 10-09 finding is fixed in 40db5ee.
• c89d871 (run 37915653076): it read 3 of 3, with no hold, and approved. Its two notes were both known: the step fails on a non-pull_request trigger by design, and warning comments can repeat across pushes.
• f47e495 (run 37931391646), which reverts the effort-test commits and has the same tree as c89d871: approved. The [skip ci] effort commits had no selftest.
• af20b0f (run 37932462436): approved. The review checked that both passes use the same claude_args.
• 5792475 (comments only; run 37932942855): approved.
• 6b3f221 (run 37938142207) and d2dc44a (run 37938884314): approved. The selftest caller sets no generated_paths, so d2dc44a's run also covers the empty value on the runner's bash 5. The one-line, space-separated and |-block forms were tested locally by running the diff and prompt steps against a synthetic merge commit.

Before merge:

• The factory-farm-em runs above.
• The Sonnet 5 comparison is done (above), and factory-farm-em#43 removed the temporary model: line.
• Then remove the replay branches there (replay-29, replay-29-base) and close #40.

After merge: point factory-farm-em back at @v1 before deleting this branch. Then watch the "Review coverage" job summaries for two weeks. Record how many runs showed M of M, how often the gate held a verdict back, how often the follow-up ran, and the sample size.

[claude]

I read all three diffs to the end and found no significant issues. No change weakens the verify step's jq or the tool allowlist. Inputs still enter scripts only through env:, and claude-pr-review.yml adds no permissions: block and no new input. README and CLAUDE.md are updated, including the changelog entry.

I traced the new shell by hand and it holds up:

• The :(exclude,glob)… pathspec construction is valid.
• --numstat -z parsing handles renames and binary files.
• --literal-pathspecs is used for the per-file diff.
• The HEAD^2 guard matches what the pull_request callers provide.

Not blocking: "Measure which diffs the review read" runs under set -euo pipefail. If its jq, cp or wc failed unexpectedly, the step would fail the job. That would go against the "coverage is report-only" invariant this PR documents. Adding continue-on-error: true to that step would make the invariant hold. "Report review coverage" already degrades to a warning in the same situation.

[claude]

I read all three diffs to the end and found no significant issues.

• Per-file diffs: the numstat parsing is correct, including the -z rename records. The exclude pathspecs build correctly from generated_paths. The merge-commit guard and fetch-depth: 2 fit pull_request events. Inputs reach the scripts only through env:.
• Invariants: the job still declares no permissions: block. The verify step's jq and the first pass's tool allowlist are unchanged, and the follow-up repeats the allowlist as the new CLAUDE.md note says.
• Coverage: the measurement, the follow-up pass and the report step are all report-only. The first two are continue-on-error, and the report step never exits non-zero. A follow-up verdict is picked up by the existing "latest bodied review" check.
• Docs: the README and CLAUDE.md updates and the changelog entry match the change. No input was added, and generated_paths keeps its semantics, with the format now documented.

Minor, not blocking: the coverage warning comment is posted on every run that leaves diffs unread, so a PR with several pushes can collect several of them. Read's output-token truncation can also make the line-range count slightly optimistic. Both are acceptable for a report-only feature.

I could only review the YAML and could not run it. The selftest self-skips on this PR, so the first real exercise will be on a consuming repo after release.

[claude]

I read all three diffs and found no significant issues.

• Per-file diffs: the pathspec construction (:(exclude,glob)…), rename handling in the numstat -z parsing, the HEAD^2 guard with fetch-depth: 2, and the quoted heredocs with env-only inputs all hold up.
• Coverage jq: the range-merge logic, failed-Read filtering, and offset/limit defaults are correct.
• Follow-up pass: it is continue-on-error, mirrors the first pass's allowlist, and does not touch the verify step's jq, which still picks the latest bodied review for the head SHA.
• Invariants: no permissions: block was added, and no new inputs were added. The generated_paths semantics change is documented in the input description, the README, and the changelog.
• CLAUDE.md: the file is updated alongside the workflow, as its own rules require.

Minor, not blocking: the coverage-warning comment is posted on every push that leaves files unread, so a long-running PR can accumulate duplicates. I could not verify that the --resume second pass finds the first session on the runner. The report-only, continue-on-error design limits the cost if it does not.

[claude]

I found no significant issues in this PR. I could not run the workflow or read CI, so this is from reading the diffs only.

• Per-file diffs:
  • The -z numstat parsing handles renames and binary files correctly.
  • The exclude pathspec built from generated_paths comes out well-formed (:(exclude,glob)…).
  • --literal-pathspecs is used when diffing each file.
  • Workflow inputs enter run: scripts only through env:.
  • The claude-pr-review.yml job still declares no permissions: block.
• Verify step and tool allowlist: both are untouched. The follow-up pass repeats the allowlist and the diff says so.
• Verdict gate hook:
  • It is capped at two refusals.
  • It fails open on any error, so it cannot block a verdict permanently.
  • The coverage jq reads the transcript and execution-file shapes consistently.
• Docs: README documents the changed generated_paths semantics and adds a changelog entry. The CLAUDE.md step list matches the workflow.
• No new inputs: callers need no change.

Minor, not blocking:

• The coverage warning comment is posted on every run that leaves diffs unread, so repeated pushes can stack duplicate comments.
• The README says coverage "never changes the verdict", but the follow-up pass can file a new verdict. The wording could say it never gates the verdict.

[claude]

I read all three diffs to the end and found no significant issues. I could not run any of it, so the notes below come from reading the code.

• Invariants: the job still has no permissions: block. Inputs reach scripts only through env:. The verify step's jq and the tool allowlist are untouched.
• New inputs: none. The generated_paths semantics changed to whitespace-separated git globs. The input description and README are updated in the same PR, and the changelog says so.
• Per-file diffs: I checked the numstat -z rename handling, the :(exclude,glob) pathspec construction, the empty-array handling under set -u, and the heredoc indentation after YAML strips the block indent. They look correct.
• Coverage jq: the range-merge covers logic, the exclusion of failed Reads, and the offset/limit defaults look correct.
• Verdict gate: the hook fails open on every error path. It refuses at most twice, and it only affects when the verdict goes out. This matches the "coverage is report-only" decision from #11.
• Follow-up pass: it is continue-on-error, and its claude_args allowlist mirrors the first pass. Its verdict is the latest bodied review, so the verify step's existing checks cover it.
• Docs: the CLAUDE.md and README changes match the workflow. The changelog entry gives dates, the incident, and the cost.

Minor notes, not blocking:

• The coverage warning comment is posted on every run that leaves diffs unread, so repeated pushes can stack several of them.
• File names go into the prompt unescaped. I don't think this opens a new injection path, because the diff content is already untrusted input to the review.

[claude]

This PR replaces gh pr diff as the review's view of the diff with one diff file per changed path. It adds a PreToolUse hook that holds the verdict until every diff is read, a coverage measurement, a resumed follow-up pass, and a PR comment for anything still unread. I found no bugs, security issues or CLAUDE.md violations.

Things I checked:

• Invariants: The job still declares no permissions: block. Inputs reach scripts only through env:, and the prompt text uses quoted heredocs. The verify step's jq and the tool allowlist are untouched. The follow-up step's claude_args allowlist matches the first pass, as the new CLAUDE.md invariant requires.
• Inputs: No input is added. generated_paths changes meaning from "skim" to "exclude", and the README, the input description and the changelog all say so.
• Per-file diff step:
  • The pathspec construction is correct: :(exclude,glob)… is well-formed, and patterns with no slash also get the **/ form.
  • read -a avoids glob expansion.
  • The --numstat -z parsing handles renames, and binary files are listed separately.
  • fetch-depth: 2 is enough for HEAD^1..HEAD, and the step fails with a clear error on a non-merge checkout.
• Coverage logic: coverage.jq counts the lines Read actually returned and merges ranges correctly. The hook is capped at two refusals, and it lets the verdict through whenever it cannot tell. The measurement and the follow-up step are continue-on-error, and coverage stays report-only, as the new CLAUDE.md invariant requires.
• Docs: The CLAUDE.md step list and the README match the workflow's behaviour.

Two minor notes, neither blocking:

• The changelog entry is dated 2026-10-08 but cites a 2026-10-09 observation.
• The "Write per-file diffs" step fails the whole job on a non-pull_request trigger. That is intentional and documented, but it is a new startup-style failure mode for any caller that deviates from the template.

[claude]

This replaces gh pr diff with per-file diffs written from the merge commit, and adds a verdict gate hook, a coverage measurement, a follow-up pass and a PR warning comment. I found no significant issues.

• Diff generation: The rename and binary handling, the NUL-safe parsing, the exclude/glob pathspec construction and the empty generated_paths case all work. Inputs reach scripts only through env:.
• Invariants: The job still declares no permissions: block. The verify step's jq, the tool allowlist and Task being disallowed are unchanged. The follow-up step repeats the allowlist verbatim, as CLAUDE.md now requires.
• Fail-open paths: The hook, the measurement and the follow-up are continue-on-error and fail open. Coverage never touches the verdict, which matches the #11 decision.
• Documentation: The README and CLAUDE.md updates and the dated changelog entry are consistent with the code. No input is added, and generated_paths is now documented as git globs.

Non-blocking: the coverage-warning comment interpolates changed file names into a fenced block. A file name containing a backtick fence could break the formatting. Only a human pasting the prompt would be affected.

[claude]

This PR replaces the review's dependence on gh pr diff with per-file diffs written from the merge commit. It adds a capped verdict gate hook, a coverage measurement, a resumed follow-up pass and a coverage report. I found no significant problems.

• Invariants hold. The job still declares no permissions: block. generated_paths enters scripts only through env:, and read -a keeps it from being glob-expanded. The tool allowlist is unchanged and the verify step's jq is untouched. No new input was added, so the README input documentation is accurate as it stands.
• Gate and follow-up are fail-open. The hook refuses at most twice and lets the verdict through on any parse error or missing transcript. The measurement, gate and follow-up steps are continue-on-error, and the report step never fails the check. This matches the stated report-only decision.
• Docs. The README changelog entry and the CLAUDE.md step list and invariants are updated together, as the repo conventions require. The two claude_args allowlists are identical and cross-referenced.

Two behaviours to be aware of, neither blocking:

• The "Write per-file diffs" step hard-fails if the checkout is not a pull_request merge commit. This is documented and matches the callers' triggers. A caller on another trigger would get a red check, not a silent skip.
• The coverage warning comment is posted on every run that leaves diffs unread, so repeated pushes can stack duplicate comments.

[claude]

The change is correct, and I found no bugs, security issues, or CLAUDE.md violations.

• Per-file diffs: the numstat parsing handles renames and binary files correctly, and generated_paths become exclude pathspecs without shell glob expansion. fetch-depth: 2 is enough for diffing HEAD^1..HEAD, and the HEAD^2 guard fails clearly when the checkout isn't a merge commit.
• Invariants: inputs reach scripts only through env: or quoted heredocs. The job still declares no permissions:, and the tool allowlist is unchanged. The verify step's jq is untouched. The follow-up step repeats the first pass's allowlist, as CLAUDE.md requires.
• Gate and coverage: the hook caps refusals at two and exits 0 on any parse or jq failure. The coverage and follow-up steps are continue-on-error, so the verdict is never blocked. The coverage jq counts lines Read actually returned, not the lines requested.
• Docs: the changed generated_paths semantics are documented in the input description, the README, and the changelog. CLAUDE.md's step list is updated to match.

A note for a follow-up, not a blocker: the coverage comment needs issues: write or pull-requests: write on the caller. If a caller lacks both, the step already degrades to a warning.

[claude]

This replaces the gh pr diff view with one diff file per changed path, adds a verdict gate that holds gh pr review until the diffs are read, adds a follow-up pass and a coverage report, and sets --effort high. I found no bugs or CLAUDE.md violations.

Checked against the invariants in CLAUDE.md:

• Permissions and inputs: the job still declares no permissions: block. generated_paths reaches the script through env: and is split with read -a, so outputs/** is never globbed against the checkout. The only ${{ }} inside a script body is github.repository, which is not user-controlled.
• Verdict verification: the verify step's jq, the commit_id and submitted_at filters, and the tool allowlist are untouched. Both passes use the same claude_args, as the new CLAUDE.md invariant requires.
• Failure handling: a failed diffs step (no merge commit) skips the review step. The always-running verify and escalation steps then report no verdict and escalate to a human, so the failure is not silent. The gate, coverage and follow-up steps are all continue-on-error. The hook lets the verdict through when it errors and refuses at most twice.
• Shell and jq: the numstat -z parsing, including renames, the exclude pathspec construction, the heredoc nesting inside the YAML run: blocks, and the gaps range arithmetic in coverage.jq all look correct.
• Docs: the README changelog entry and the CLAUDE.md step list and invariants match the workflow. No new input is added. The generated_paths semantics change to git globs is documented.

Minor notes, not blocking:

• The coverage warning comment is posted on every run that leaves diffs unread, so repeated pushes will stack warnings on the PR.
• The gate's cap of two refusals is shared across the first pass and the follow-up. If the first pass uses both, the follow-up is not gated. That looks intended, but it is worth a line in the comments.
• Whether the resumed follow-up session keeps the hook and writes to the transcript path the hook expects depends on Claude Code and action behaviour. This PR edits the workflow, so the selftest self-skips it, and the first real PR after merge is the check.

[claude]

The change is correct, and I found no significant issues.

What I checked

• Per-file diff step. The HEAD^1..HEAD diff against the merge commit is sound with fetch-depth: 2, and the step fails with a clear error when HEAD isn't a merge commit. NUL-delimited numstat handling covers renames (empty path, then from and to) and binary files. exclude pathspecs are built from generated_paths via read -a, so nothing is glob-expanded. --literal-pathspecs is used for the per-file diff.
• coverage.jq. Range merging in gaps, the fallback to the call's offset/limit when a result has no numLines, and the failed-Read filtering all behave as intended. Zero-line diffs count as read.
• Verdict gate. The hook is capped at two refusals and exits 0 whenever it can't tell. It is continue-on-error, so it only decides when the verdict goes out and never what it says or whether the check fails.
• Follow-up pass. continue-on-error and report-only. Its claude_args match the first pass, as CLAUDE.md requires.
• Invariants. There is still no permissions: block. Inputs reach scripts only through env:. The allowlist and the verify step's jq are untouched, and no new input was added. The ${{ github.repository }} interpolations match the existing style.
• Docs. The README, the changelog entry and the CLAUDE.md step list agree with the workflow. The generated_paths semantic change (whitespace-separated git globs) is documented. The changelog also notes that the live values already conform, so no caller changes are needed.

Notes, none blocking

• The README says a verdict is "never withheld" while also saying the gate holds it back at most twice. Both are accurate as written, but a reader could trip on it.
• Because generated_paths is now parsed as git pathspecs, a directory-style pattern such as outputs/ with a trailing slash and no ** may not match files under it. The documented forms (outputs/**, uv.lock) work.
• The diff step now fails the job hard if a caller triggers on something other than pull_request. The error message says why, and the callers in this repo use pull_request.

[claude]

The change is correct as written, and I found no bugs, security problems or CLAUDE.md violations.

Invariants. The job still declares no permissions: block. generated_paths reaches the diff step only through env:, and every prompt template substitution is guarded by shopt -u patsub_replacement. The tool allowlist and the verify step's jq are unchanged. The new settings input, the follow-up step and the coverage steps all pass through $GITHUB_OUTPUT or env:. There is no new workflow_call input.

Per-file diffs.

• fetch-depth: 2 is enough for HEAD^1..HEAD, and the HEAD^2 guard fails loudly on a non-merge checkout.
• read -a avoids glob expansion of outputs/**.
• The exclude pathspec built with ${g#:(} comes out as :(exclude,glob)…, as intended.
• The -z numstat rename parsing and the -M diff of from/path are consistent. The read -d '' calls inside the loop consume the same fd as the <"$state/numstat" redirect.
• A non-glob value such as outputs/ now matches nothing, so those files get reviewed instead of skimmed. That is the safe direction, and the changelog documents the semantic change.

Coverage and gate.

• coverage.jq counts the lines Read actually returned, with a fallback to the call's offset/limit.
• The hook exits 0 on any parse failure, so it cannot block a verdict. It refuses at most twice, which keeps coverage report-only per #11.
• The follow-up step's claude_args match the first pass, as CLAUDE.md requires.
• The verify step still takes the last bodied review, so the follow-up's verdict is the one that counts.
• The coverage comment needs issues: write, which the caller template already grants. If posting it fails, it degrades to a warning.

Follow-ups, not blocking:

• generated_paths is read with a single read, so only the first line of a multi-line value is honored. Extra patterns silently fall back to being reviewed. The input description says "whitespace-separated", so this matches the docs, but a YAML | block would surprise someone.
• Per CLAUDE.md, the callers/claude-pr-review.caller.yml comment change needs mirroring in sentfutures/.github/workflow-templates/.
• A PR that adds both x and x.diff/y would make mkdir -p collide on diffs/x.diff and fail the diff step. It is very unlikely, but it would be a hard failure.
• As the CLAUDE.md notes say, this PR edits the workflow, so the selftest will take the NOT_REVIEWED path. The new steps get no live exercise until the change is on main. Watch the first few PRs after release.

[claude]

This looks correct, and I found no significant issues.

Per-file diffs. Diffing HEAD^1..HEAD of the pull_request merge commit is the right base, and fetch-depth: 2 is enough to get both parents.

• The NUL-delimited numstat parsing handles renames (the empty path field followed by the old and new paths) and binary files (- counts).
• The inner read -d '' calls consume the same stdin as the outer loop, which is what rename parsing needs.
• generated_paths is split with read -a rather than an unquoted expansion, so there is no glob expansion against the checkout.
• The :(exclude,glob)… pathspecs are built correctly, including the **/ variant for slash-less patterns.
• GENERATED_PATHS enters scripts only through env:, as the invariants require.

Coverage jq. The gap computation, the use of the result's startLine/numLines over the call's limit, and the fallback for older result formats are consistent with each other. The failed-call filtering is correct.

Verdict gate. It is capped at two refusals, it lets the verdict through on any parse or jq failure, and it never fails the check. That matches the "coverage is report-only" invariant. The step is continue-on-error, and the settings input is empty when the step fails, so a broken gate degrades to the old behaviour.

Unchanged invariants.

• The job still declares no permissions: block.
• The tool allowlist is unchanged.
• The verify step's jq is untouched.
• The follow-up step repeats the first pass's claude_args, as the new CLAUDE.md note requires.
• No new inputs were added. The generated_paths description and the README and caller-template text were updated together, and the README changelog entry is present.

Notes, not blocking:

• Report review coverage runs without continue-on-error. Its commands run under the default bash -e, so an unexpected jq failure past the early guard would fail the job. That is not the "never fails the check" behaviour the docs promise. In practice this is unlikely, because the guarded jq call has already succeeded on the same files.
• The coverage warning comment is posted on every run that leaves diffs unread, so a PR that keeps getting pushed can collect one per push. You could edit or replace the previous one later if that gets noisy.
• callers/claude-pr-review.caller.yml changed, so the mirrored template in sentfutures/.github/workflow-templates/ needs the same comment update. That repo isn't part of this PR, so I couldn't check it.
• A PR that edits this workflow will self-skip in the selftest. The startup_failure blind spot still applies, so make sure review / claude-review actually reports on this PR before it merges.