Judge the whole PR on every push, not just the newest commit - #83
Merged
Conversation
An incremental review trimmed context.files to the delta, then handed the model that slice alongside the WHOLE-PR title and description — plus a system-prompt rule asking for a PR-level finding when "a claimed change is missing". The model duly reported the earlier commits' work as never written. On mk7luke/atlas-timeclock#89 a docs-only follow-up commit flipped an approved feature branch to CHANGES_REQUESTED over four files that had been reviewed and accepted a week earlier. #81 fixed this for the drift pass; the main review call had the same blind spot, and PR-level findings skip the hallucination verifier (no path/line ⇒ fail-open). - Review prompt gains an "Already reviewed earlier in this PR (context only)" section carrying the earlier diffs, with explicit rules: the diff is partial, the description covers this work too, absence is not evidence, comment only on Changed Files. - Sized against the headroom the delta left (buildPriorReviewContext), so it never displaces the diff under review; degrades to naming the files when the budget is spent — naming alone is most of the value. - System prompt: never raise a description-vs-diff finding when the diff shown is partial. - dropContextOnlyFindings() is the deterministic backstop, so a push can't re-litigate already-reviewed files. - The walkthrough (and the PR-description summary it rewrites), risk score, coverage signal, and split suggestion now use the full branch too — they render in a whole-PR comment that every push replaces, so computing them from the delta reported "no test changes" for a PR whose tests landed two commits ago. Full reviews are unchanged: every path is gated on prior files existing. Tests: 14 new unit tests (tests/unit/incremental-scope.test.ts) covering the prompt section, the budget degradation, and the backstop; they fail against the old code. New e2e scenario incremental-full-pr reproduces #89's shape, with a reviewBodyNotContains assertion added to the harness. Not run — e2e needs a live GitHub App + AI provider. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
📝 WalkthroughWalkthroughFixed incremental reviews so every push judges the whole PR instead of only the newest commit, by attaching already-reviewed files as read-only prompt context and computing walkthrough, risk, coverage, and split signals from the full branch. Changes
Sequence Diagram(s)sequenceDiagram
participant Push as GitHub Push
participant Reviewer
participant Budget as Diff Budget
participant AI as AI Review
participant Walkthrough as Walkthrough
Push->>Reviewer: synchronize on open PR
Reviewer->>Reviewer: partition delta vs already-reviewed files
Reviewer->>Budget: size prior context from remaining headroom
Budget-->>Reviewer: patches or names-only prior context
Reviewer->>AI: review(delta + priorReview context)
AI-->>Reviewer: findings
Reviewer->>Reviewer: dropContextOnlyFindings
Reviewer->>Walkthrough: generate from whole PR files
Walkthrough-->>Reviewer: summary risk coverage split
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested Labels
Risk AssessmentScore: 8/100 — 🟢 Low
Test Coverage Signal🟢 242 prod / 303 test lines added.
Linked Issues
🚥 Pre-merge checks | ✅ 5 | ❌ 0✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in your ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Generate docstrings (beta)
🧹 Simplify (beta)
🪄 Autofix unresolved comments (beta)
Comment |
![diffsentry[bot]](https://avatars.githubusercontent.com/in/3360270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
This PR addresses the incremental-review blind spot comprehensively: the prompt now carries earlier PR files as read-only context, whole-PR summary signals are computed from the full branch, and the new unit/e2e coverage is targeted and convincing. I did not find any correctness or contract regressions in the changed code paths. The implementation is well-scoped, budget-aware, and preserves full-review behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: .diffsentry.yaml
Review profile: ASSERTIVE
Run ID: 03b0142d-5098-4572-87ae-932ff1577a04
📥 Commits
Reviewing files at 3efae37 (base SHA unavailable).
📒 Files selected for processing (9)
CHANGELOG.mdsrc/ai/prompt.tssrc/reviewer.tssrc/types.tstests/e2e/runner.tstests/e2e/scenarios/incremental-full-pr.tstests/e2e/scenarios/index.tstests/e2e/types.tstests/unit/incremental-scope.test.ts
ℹ️ The primary review model was unavailable; this review was generated by the configured backup provider.
📌 Status — last updated
|
| Risk score | 8/100 (Low) ▁ |
| Unresolved threads | 0 |
| Failing checks | 0 |
| Pending checks | 1 |
| Files reviewed | 9 |
| Updated | 2026-08-02 23:04Z |
Live-updated by DiffSentry on every push. Use @diffsentry ship for a verdict, @diffsentry timeline for full history.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
On a push to an open PR, the incremental review trims
context.filesto the delta — then hands the model that slice alongside the whole-PR title and description, plus a system-prompt rule that explicitly asks for a PR-level finding when "a claimed change is missing" from the diff. The model does what it's told.Observed on mk7luke/atlas-timeclock#89: a docs-only follow-up commit turned an approved feature branch into
— CHANGES_REQUESTED, naming the four backend files that had been reviewed and accepted a week earlier.
#81 fixed this for the drift pass only (
detectDescriptionDriftgetsallReviewableFiles). The mainai.review()call — the one that sets the verdict — kept the same blind spot, and the finding sails past the hallucination verifier because PR-level findings have no path/line and are kept fail-open.What changed
The model sees the whole PR on every push
## Already reviewed earlier in this PR (context only — do NOT comment on these)section inbuildReviewPrompt, carrying the earlier commits' diffs. It states outright that the diff is partial, that the description covers the earlier work, that absence is not evidence, and that findings go only against Changed Files.buildPriorReviewContext()sizes it against the headroom the delta + related-context left, so it never displaces the diff under review. When there's no room for even one more file it degrades to naming them — most of the value, since the false finding comes from the model believing those files aren't in the PR at all.dropContextOnlyFindings()is the deterministic backstop — inline findings against context-only files are dropped, so a push can't re-litigate the whole branch. PR-level findings are untouched; whole-PR reasoning is the point of the context.Other whole-PR judgments stopped being computed from the delta
These render in the walkthrough comment that every push replaces, so a delta-derived value misreports the branch:
Full reviews are byte-identical to before — every path is gated on previously-reviewed files existing.
Verification
npx vitest run— 392 passed (27 files), including 14 new tests intests/unit/incremental-scope.test.ts. They fail against the old code.npx tsc --noEmitclean;eslint0 errors (remaining warnings pre-existing).incremental-full-prreproduces #89's shape (feature PR → docs-only follow-up commit) with a newreviewBodyNotContainsassertion in the harness. Not run — e2e needs a live GitHub App + AI provider, so the model's actual behavior on the new prompt is unverified.Database / schema changes
None.
🤖 Generated with Claude Code
Summary
Fixed incremental reviews so every push judges the whole PR instead of only the newest commit, by attaching already-reviewed files as read-only prompt context and computing walkthrough, risk, coverage, and split signals from the full branch.
Changes
CHANGELOG.mdsrc/ai/prompt.tssrc/reviewer.tssrc/types.tstests/e2e/runner.tstests/e2e/scenarios/incremental-full-pr.tstests/e2e/scenarios/index.tstests/e2e/types.tstests/unit/incremental-scope.test.ts