Skip to content

feat(review): incremental re-review — review only new commits on synchronize, with a dedicated finding-resolution check [v0.9] #667

Description

@devops-thiago

Problem Statement

Every synchronize event re-reviews the full PR diff, no matter how small the push. Prompt size therefore grows with PR size, not push size, and the same already-reviewed hunks are re-sent (and re-billed) every round. Previous findings are condensed to fit (DiffBudgetPlanner.boundPreviousFindings, capped at 25% of the per-call budget — added after this repo's own release PR hit ~437K tokens in that block), but the diff itself has no such incremental treatment.

Proposed Solution

Bound follow-up rounds by push size:

  • Persist the last-reviewed head SHA on ReviewSession. On synchronize, fetch GET /compare/{lastReviewedHead}...{newHead} instead of the full /pulls/{n}/files. Fall back to a full review whenever the compare fails or is misleading (force-push/rebase, base branch changed, no completed prior round) — the existing stale-head guard (ReviewContextLoader.java:498-515) already detects head movement mid-review.
  • Dedicated resolution-check call, modeled on FindingVerificationService (blocking concise call, JSON verdicts keyed by id, fail-open, ledger-gated, salvage-on-truncation): input = condensed open findings (the entryLinesOnly shape from DiffBudgetPlanner.java:441: id, risk, file:line, title, plus suggestion_old) + only the new hunks touching those findings' files; output = resolved / unresolved / superseded per id. This takes over the previous_findings_status duty from the main review call, so the reviewer prompt no longer needs the reconciliation rule block (PrReviewPrompts.java:550-586) or the full previous-findings context — a second large prompt section shrinks.
  • The review call runs over the incremental diff only → prompt size is bounded by the push, giving the predictable prompt sizes this roadmap is after. FollowUpAnalyzer thread matching, supersede logic, and the batch-scoping rules for statuses (FindingPipeline.scopeStatusesToBatch) are reused: the resolution-check call may only resolve findings whose touching hunks it actually saw.
  • Escape hatches: manual /review keeps forcing a full-diff pass; a periodic full pass (e.g. every Nth round, or when ready_for_review fires) catches cross-commit interactions that incremental rounds can miss. Document the tradeoff.

Why a dedicated call beats the current reconciliation

Today reconciliation quality depends on the full re-review happening to look at the right places, and statuses must be carefully scoped per batch. A dedicated call sees exactly the pairs that matter (finding ↔ new hunks in its file) and nothing else — cheaper, more reliable, and it removes the largest conditional sections from the reviewer prompt.

Touchpoints

dashboard/ReviewSession (+ head SHA column/migration), review/ReviewContextLoader.java (compare-based incremental load + fallbacks), new review/ai/ResolutionChecker service + prompts (+ content test), review/FollowUpAnalyzer.java, review/FindingPipeline.java, review/DiffBudgetPlanner.java (call reservation), webhook/WebhookController.java (synchronize routing).

Acceptance criteria

  • Push of K lines to an N-thousand-line PR produces prompts sized by K, not N (token metrics).
  • Force-push/rebase and missing-history cases demonstrably fall back to full review.
  • Resolution statuses match current behavior on the existing FollowUpAnalyzer test scenarios; a finding is never marked resolved by a call that didn't see its file's new hunks.
  • Feature gate (REVIEW_INCREMENTAL_ENABLED), default off for one release.

Alternatives Considered

  • Keep full-diff rounds and rely on tighter condensation — bounds the findings block but never the diff; cost still scales with PR size.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestjavaPull requests that update java code

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions