Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

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

Open
  • Is something wrong?
#667 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
12/100
Issue type
Feature
Clarity
Mostly clear
Activity status
Quiet
Tech stack
github, github-actions, java
Domain
backend, devtools

Research direction

Start with review/ReviewContextLoader.java (the stale-head guard near lines 498-515) and review/FollowUpAnalyzer.java, then review/ai/FindingVerificationService as the model for the new resolution check. The work spans the dashboard/ReviewSession migration, the webhook synchronize routing and prompt changes, so it needs design agreement first. Done means an incremental compare path with full-diff fallbacks, behind a REVIEW_INCREMENTAL_ENABLED gate, matching existing follow-up tests.

Written by the indexing model from the issue text.

Description

enhancement java
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.
Dominant language
Java
Stars
35
Forks
7
Avg merge
9h 32m
Merged PRs (30d)
144

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from devops-thiago/ThrillhouseBot

All issues in devops-thiago/ThrillhouseBot

Similar issues

More Java issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.