Skip to content

Support @claude /review new to review only what changed since the last review #110

Description

@thecodedrift

@claude /review always reviews the entire PR. On a PR that has already been reviewed and then revised, that means re-reading thousands of lines to find the few hundred that are new — and the genuinely new work competes for attention with everything already agreed.

Add a second closed token:

@claude /review        # today: full review
@claude /review new    # only what changed since the last completed review

Why a token and not free text

The obvious version of this is @claude /review followed by prose describing what to look at. That is deliberately not what this proposes.

The workflow currently uses the comment body for exactly one thing: the contains(github.event.comment.body, '@claude /review') gate. The body never reaches the model. Forwarding it into the prompt would add an injection channel, and the natural mitigation — "treat the following as review focus, not as instructions" — is a prompt-level guard against prompt injection, which is not a boundary. Asking a model to correctly classify attacker-influenced text that has already been concatenated into its prompt does not hold.

A closed vocabulary avoids the problem by construction. The workflow matches the token against known arms and selects a prompt that already lives in the repo and was reviewed like any other code. The untrusted string is consumed by the workflow and never forwarded. Unrecognized token falls back to today's full review.

This costs expressiveness — you get the focuses someone thought about in advance. new appears to be the one that carries most of the value.

Deriving the range without storing state

The data already exists in the workflow run history. Each run records the SHA it examined:

gh run list --workflow=claude-code-review-on-demand.yml \
  --branch "$HEAD_REF" --limit 40 \
  --json headSha,createdAt,conclusion,event \
  --jq '[.[] | select(.conclusion=="success" and .event=="issue_comment")] | .[0].headSha'

Verified against this repo: PR #103's last completed review ran at headSha=6165963, and its head is now 05b3c88. 6165963..05b3c88 is exactly the delta a reviewer wants.

No new state to persist, and the SHA is derived by the workflow rather than supplied by the commenter, so it is safe to interpolate into the prompt.

Note when filtering: most runs of this workflow have conclusion: skipped (34 of 40 sampled). Every PR comment starts a run and the job-level if gate skips the ones that are not @claude /review. Only success runs represent a review that actually happened.

Open questions

  • Stacked PRs. A plain lastReviewed..head range includes commits absorbed from the parent branch during a restack, which can dwarf the PR's own new work. The range probably wants intersecting with the PR's own diff against its base, or expressing as a three-dot range.

  • Rewritten history. Do NOT detect this with git cat-file -e. Force-pushed objects usually still resolve on GitHub, so an existence check passes and then yields a wrong answer: diffing an old tip against a rebased tip renders every base change as if it were PR work. The correct test is ancestry:

    git merge-base --is-ancestor "$lastReviewed" "$head" || : fall back

    If the last-reviewed SHA is not an ancestor of the head, the range is meaningless whether or not the object resolves. Fails closed, and is exact.

  • No prior review. Fall back to a full review.

  • Staleness. If the last review is old and the PR has been rebased repeatedly, "new" may be misleading. Stating the resolved range in the review output makes whatever it did legible.

Acceptance

  • @claude /review new reviews only the derived range and names that range in its output.
  • Any unrecognized token, missing prior review, or unreachable SHA falls back to a full review and says why.
  • The comment body is still never interpolated into the prompt.

How the stack is restacked decides whether this works at all

new derives its range from lastReviewed..head, which is only meaningful while the last-reviewed SHA remains an ancestor of the head. That is a property of how the stack is propagated, so the restack method and this feature have to be decided together.

Measured on the #71#106 stack, propagated by merge on 2026-08-17: all eight branches kept the last-reviewed SHA as an ancestor, so a per-PR delta was recoverable for every one of them. A rebase-based propagation would have left none of them recoverable.

Rebase

For

  • Linear history. Each PR's commit list reads as its own work, with no propagation merge commits mixed in.
  • Undo is tractable. Reverting a merge commit needs git revert -m 1, and afterwards git treats the merged branch as already merged — re-merging it later silently brings back nothing. Linear history avoids that trap entirely.
  • git bisect over a linear history is straightforward.
  • It is what most stacking tools do, including this repo's propagate_stack.py.

Against

  • Breaks new: the last-reviewed SHA stops being an ancestor, so every restacked PR falls back to a full review. On an actively restacked stack that is most PRs, most of the time.
  • Review threads are detached or marked outdated, so a resolved conversation stops pointing at the code it resolved.
  • Requires force-push, with the --force-with-lease and shallow-clone caveats in CLAUDE.md, and the risk of clobbering concurrent work.
  • A rebase can land on the wrong parent; propagate_stack.py carries a balloon guard specifically to catch that before it reaches the remote.
  • The same conflict can be re-resolved once per replayed commit.

Merge

For

  • Preserves ancestry, so new works and review threads stay anchored.
  • No force-push, so no lease/shallow-clone hazards.
  • A conflict is resolved once and recorded in the merge commit.
  • Matches the merge-down landing this repo already uses, where merge commits are expected regardless.

Against

  • Merge commits accumulate in every branch and clutter each PR's commit list.
  • Undo is the hard case described above.
  • History becomes a lattice rather than a line, which is harder to reason about after the fact.

If rebase wins, new needs a different mechanism

A two-dot range cannot survive a rewrite, but git range-diff is built for exactly this — comparing two versions of a rebased series:

git range-diff "$oldBase..$oldHead" "$newBase..$newHead"

That requires recording the base as well as the head at review time. The run history stores only headSha, so this would need the reviewed base captured too — either written into the tracking comment, or read from the PR timeline's head_ref_force_pushed / base_ref_changed events, which do record before/after SHAs.

Worth scoping before committing to new: under a rebase policy, the two-dot implementation would be correct but almost always inert, and range-diff is the version that actually delivers the feature.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions