alibaba / alibaba/open-code-review

Review gate loops re-review the whole range every push: no cross-push incremental reuse despite per-item fingerprints already existing

Open
#854 5 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
24.4k
Forks
1.8k
Avg merge
2d 6h
Merged PRs (30d)
105

Description

### Problem

In a PR review-gate loop (review → fix → push → review again), every round re-reviews the **entire** `--from --to ` range. The GitHub Actions posting helper already avoids *re-posting* duplicate comments (`scripts/github-actions/post-review-comments.js`, IoU-based `incremental` overlap dedupe), but the expensive part — the LLM re-reviewing files the new push did not touch — has no reuse path. On one iterative PR we measured 19 gate rounds at roughly 2M tokens per round; the large majority of that spend was files that were byte-identical between rounds.

### The building blocks already exist, but are locked to same-range crash recovery

- Every review item has a content fingerprint: `sha256(mode + oldPath + newPath + diffText)` (`internal/agent/agent.go`, `reviewItemFingerprint`). A file untouched by a new push keeps a byte-identical per-file diff against the same merge-base, hence the same fingerprint.
- `--resume ` already reuses fingerprint-matched completed items (`applyResume` → `RecordReviewItemReused`), and the run manifest already has a `coverage.reused` bucket for exactly this.
- But `ResumeState.ValidateOptions` (`internal/session/resume.go`) requires the resumed session's `--from`/`--to` to match the current run **exactly**, so the moment a new commit is pushed (`--to` moves) resume refuses, and the whole range is re-reviewed. Resume also needs the local session store, which an ephemeral CI runner has already thrown away.

### Prior art: this is table stakes for PR reviewers

**qodo-ai/pr-agent** has incremental review (`/review -i`), and its design maps cleanly onto what is missing here (all verified against `pr_agent/tools/pr_reviewer.py` and `pr_agent/git_providers/github_provider.py`):

- **State lives in the PR, not on the runner.** `get_previous_review` finds the bot's last review comment by its header prefix; no server-side storage is needed.
- **Delta detection**: `get_commit_range` splits the PR's commit list at the previous review's timestamp → `first_new_commit` / `last_seen_commit`.
- **Scope = files touched by the new commits**: `unreviewed_files_map` is the union of `commit.files` over the new range (merge commits from the default branch skipped); `get_files()` returns only those in incremental mode — i.e. **file-granularity increments**, exactly the granularity OCR's fingerprints already encode.
- **Safety**: no previous review → automatic fall back to full review; optional thresholds (`minimal_commits_for_incremental_review`, `minimal_minutes_for_incremental_review`) gate auto-triggered incremental runs; the review output marks where the increment starts ("Starting from commit …").

CodeRabbit likewise reviews each push incrementally rather than re-reviewing the full PR every time.

### Proposal

Two composable pieces:

1. **Cross-push reuse for range mode.** Allow resuming with the same `--from` but a moved `--to` (either by relaxing `ValidateOptions`, or under a distinct flag so classic resume semantics stay strict). Selection stays exactly as today — compute the current `merge-base..new-head` diff and each item's fingerprint; items whose fingerprint matches a completed item in the reuse source are recorded as `reused` (with their previous comments, as `applyResume` already does), everything else is reviewed fresh. The fingerprint definition makes this safe by construction: any change to a file's diff — including base drift after a rebase, since the diff text changes — invalidates the match and forces a fresh review.

2. **An ephemeral-CI-friendly reuse source.** Accept a previous run's output as the reuse source (e.g. `--reuse-from `) instead of requiring the local session store. The JSON result and manifest already carry everything needed: per-item fingerprints in `coverage.completed` and per-file comments. The reference GitHub Actions flow already uploads that artifact and stamps the producing run id into the sticky summary (``), so a workflow can locate the previous artifact the same way pr-agent locates its previous review comment — from the PR itself.

Mirroring pr-agent's safety behavior: no reuse source (or an unusable one) → silently fall back to today's full review; reused items are already visible in the manifest's `coverage.reused`, so a consumer can always tell a reused verdict from a fresh one.

Happy to work on this if the direction sounds right — starting with piece 1, which is small and keeps the current CLI surface.

Contributor guide

Open the contributing guide

Research direction

Start with internal/agent/agent.go and internal/session/resume.go, tracing reviewItemFingerprint, applyResume, and ResumeState.ValidateOptions. Then inspect the result and manifest coverage fields plus scripts/github-actions/post-review-comments.js. Done means unchanged review items can be reused across a moved --to range and reuse can fall back safely when no usable source exists.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions, go, javascript
Domain
ci-cd, devtools
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.