Design for the Open SWE Reviewer Agent. Goal is to match Devin Review's *perceived* quality (won internal A/B vs Graphite) on PR review, with a single evolving findings list and a "watch" mode that re-reviews on new commits. Eval plan is tracked separately in `REVIEWER_EVAL_PLAN.md`.
## Goals
- A reviewer agent that produces high-signal findings on a PR, surfaced as **inline GitHub review comments** at the relevant file/line range, with **```suggestion blocks** (the "Commit suggestion" UX) where the agent can offer a concrete fix.
- Filter what's surfaced to GitHub by severity — a clean PR shouldn't have five filler comments appended to one critical one.
- A single evolving findings list per PR — resolved findings are kept (status=resolved), not pruned.
- A "watch" mode that re-reviews on new commits to a previously reviewed PR, reconciling existing findings (resolved / still-open / updated), adding new ones, and resolving GitHub review threads for findings that the new commits addressed.
- One canonical reviewer thread per PR, regardless of how the review was triggered.
- Headless first; eventually a UI for the full findings list.
## Non-goals (for now)
- Multi-pass / multi-agent review orchestration. One reviewer agent, one thread, one evolving list.
- Deterministic resolution-detection logic. Reconciliation is LLM reasoning over (previous findings, new diff, sandbox state).
## Dispatch model
**One canonical reviewer thread per PR.** Thread id is derived deterministically from the PR URL (or `owner/repo#number`), the same way the existing webhook handlers in `agent/utils/` already derive thread ids per source. Every trigger surface routes to the same thread, so findings, sandbox, and watch state stay continuous.
Trigger surfaces:
1.**Slack** — handled by the main agent. The user asks the bot to review a PR; the main agent calls a `request_pr_review(pr_url, message)` tool. That tool resolves the canonical reviewer thread id from the PR URL and triggers a run via `langgraph_sdk` (same pattern as the existing GitHub PR webhook path). This already exists in some form (see `agent/tools/request_pr_review.py`).
2.**GitHub webhook — review requested.** Direct invocation of the reviewer graph against the canonical thread.
3.**GitHub webhook — push to a watched PR.** Direct invocation against the same canonical thread; the watch flag in thread metadata gates whether we trigger.
This unifies all paths: the reviewer graph never has to ask "who triggered me?" — it always sees the same thread and the same state, with a structured user message describing what to do.
## Findings as first-class state
Findings live in **LangGraph thread state**, not in sandbox files. Sandboxes are evictable; thread state survives.
category: str # e.g. "correctness", "security", "perf", "style", "flag"
file: str
start_line: int | None # None when the finding is file-level
end_line: int | None # equals start_line for single-line findings; >start_line for ranges
side: Literal["LEFT", "RIGHT"] # LEFT = base/old, RIGHT = head/new — almost always RIGHT
description: str # the body the user sees
suggestion: str | None # if set, rendered as a ```suggestion block — gives the user a "Commit suggestion" button on GitHub. Must replace exactly start_line..end_line.
status: Literal["open", "resolved", "dismissed"]
first_seen_sha: str # SHA at which this finding was first introduced
last_confirmed_sha: str # most recent SHA where this finding was still open
github_review_comment_id: int | None # populated after publish; used to resolve the thread on re-review when status moves to resolved
diff_hunk: str | None # snippet of the relevant diff cached at finding-creation time, so the future UI / Slack / Linear renderers can render the diff alongside the finding without re-fetching from GitHub or the sandbox (which is evictable)
```
**Storage: thread metadata.** Findings live in LangGraph thread metadata under the reviewer thread, queried via the langgraph SDK client. Same pattern as existing thread metadata (`sandbox_id`, `github_token_encrypted`). Avoids fighting deepagents' state abstraction. The future UI lists "PRs being reviewed" by querying threads filtered on `metadata.kind == "reviewer"`, and reads each thread's findings list directly.
Reviewer-thread metadata schema:
-`kind: "reviewer"` — sentinel so the UI / SDK queries can filter to reviewer threads
-`pr: {owner, name, number, url, title, head_ref, base_ref}` — PR identity, what the UI displays in the list
-`findings: list[Finding]`
-`last_reviewed_sha: str | None`
-`watch: bool` (whether push events should re-trigger)
- File-level finding: both `None` (publishes as a top-level review body line, not inline).
-`suggestion`, when set, must be the exact replacement text for lines `start_line..end_line` inclusive — that's how GitHub's ```suggestion block works.
-`update_finding(id, *, status?, severity?, title?, description?, suggestion?, note?)` — single tool for any post-creation mutation, including marking resolved/dismissed and revising a suggestion after the agent looks more carefully.
Resolved findings are kept in the list (status=`resolved`), hidden from the default top-K GitHub surfacing, surfaced in the eventual UI as "what's already addressed". This prevents the agent from re-finding the same issue across runs.
## Publishing to GitHub (decoupled from finding production)
The agent's job is to produce findings. A separate step publishes them to GitHub as a Review with inline comments.
**Surfacing format: GitHub Pull Request Review.** Single API call (`POST /repos/{owner}/{repo}/pulls/{n}/reviews`) creates one review with:
- a top-level **review body** — agent-authored summary / overall take on the PR
- an array of **inline comments**, one per surfaced finding, anchored to `path` + `line` (+ `start_line` for ranges) + `side`
-`event: "COMMENT"` (not `REQUEST_CHANGES` — we don't want the reviewer agent to gate merges)
Findings with a `suggestion` get the suggestion appended to the comment body as a ```suggestion fenced block, which gives the user GitHub's native **"Commit suggestion"** / **"Add suggestion to batch"** UX. For multi-line ranges, a multi-line ```suggestion block replaces the entire range.
Severity-threshold filter (not pure top-K): publish all findings with severity ≥ `medium` by default, with a hard cap of 4 to avoid review spam. Pure top-K means a clean PR with one critical issue gets four filler findings appended — bad UX. `informational` and `low` findings are produced into state and visible in the eventual UI / full list, but not surfaced to the GitHub PR by default.
**Mechanism: a `publish_review` tool the agent calls deliberately** at the end of its run. Why a tool, not after-agent middleware:
- Clearer in traces — explicit step in LangSmith.
- The agent can choose to skip publishing on a re-review run where nothing changed surface-worthy.
- Failure modes (line not in diff, suggestion conflict, GitHub 422) surface back to the agent, which can adjust and retry.
- We don't have to bake the severity-threshold policy into middleware; the agent can override (e.g. publish a `low`-severity nit on a tiny PR if it's the only finding).
Decoupling finding production from publishing is what lets us swap the GitHub Review surfacing for a richer UI later without touching the agent loop.
### Inline comment constraints (and how to handle them)
GitHub's inline review comments require the line to be **part of the PR diff** — meaning `line` (and `start_line..line` for ranges) must fall within an actual hunk in the PR's diff against its base. Implications:
- **Findings on lines the PR didn't touch** (e.g. agent notices a pre-existing bug while reviewing context) cannot be inline comments. Two options:
1. Drop them from the review (agent should be discouraged from reporting these in the first place — system prompt says "review the diff, not the surrounding code").
2. Append them to the **review body** as a "Pre-existing observations" section.
Default to (1); allow (2) only when severity is `high`/`critical`.
- **Suggestions only apply to lines in the diff**, since GitHub applies them as a follow-up commit on the PR branch. Suggestions on out-of-diff lines are a hard error from the GitHub API.
- **The reviewer's prep node should compute the set of (file, line) tuples in the diff** and pass it to the agent as part of the review context. The `add_finding` tool can validate against that set and reject (or auto-flag) findings that fall outside it, rather than failing at publish time.
### Re-review and resolved threads
On a re-review run, when a finding moves from `open` → `resolved`, the reviewer should also **resolve the corresponding GitHub review comment thread** (via the GraphQL `resolveReviewThread` mutation, since REST doesn't expose this). That's why `Finding.github_review_comment_id` is stored — without it, we can't reconcile back to the thread on GitHub.
For findings that move from `open` → `open-but-updated` (e.g. the agent revises severity or suggestion based on new context), the simplest behavior is to leave the existing GitHub thread alone and let it stand; richer behavior (post a follow-up reply in that thread) is a follow-up.
## Cold-start / warm-path entry contract
Sandbox handling was reworked recently (#1249): `ensure_sandbox_for_thread` now does cache → ping → start-if-idle → refresh proxy → recreate only on hard failure. So warm-path is realistic between an initial review and a follow-up push, especially for tight feedback loops where the gap is short.
**Deterministic prep node before the agent's first model call:**
For each existing open finding, decide whether the new commits:
- resolved it (mark resolved with update_finding(status="resolved"))
- left it unchanged (no action)
- changed it materially (update via update_finding with a note, or update + revised suggestion, or close + add_finding)
Then review the new diff for any net-new issues and add them with add_finding.
Finally call publish_review to post inline comments + suggestions for the new findings, and resolve the GitHub threads for findings that just moved to resolved.
```
Diff scope is the diff *since `last_reviewed_sha`*, not the full base...head PR diff. That's what resolution detection actually needs. The agent can fetch the full diff on demand if it wants broader context.
First-review just has empty findings list and a full base...head diff.
## Watch mode
-`watch: bool` flag on the reviewer thread, set to `True` implicitly on first successful review.
- New webhook handler for GitHub `push` events: if the push is to a PR's head ref and that PR's reviewer thread has `watch=True`, emit the structured re-review user message into the thread (via `langgraph_sdk`, same dispatch path as other webhook triggers).
- An explicit `unwatch_pr` tool / API endpoint stops watching (e.g. once the PR is merged or the user is done). Closing/merging a PR can auto-unwatch via the existing `pull_request` webhook.
Edge cases worth thinking about (not blockers):
- **Empty diff since last SHA** (rebase, force-push that resolves to same tree) — skip the run at the webhook level, don't trigger the agent.
- **Force-push that drops `last_reviewed_sha` from history** — `git fetch` will lose the old SHA's reachability. The diff scope falls back to base...head; treat as a fresh review of the new state, but keep existing findings as starting context for reconciliation.
- **Long gap between review and push** — sandbox evicted, falls into cold-start path automatically via the existing recreation logic. No special handling needed.
build_review_context # NEW: assemble findings + diff + diff-line-set into user message (or branch on first-review vs re-review)
agent # create_deep_agent with reviewer-specific prompt + finding tools
# (publish_review is a tool the agent calls; not a node)
```
`prep_sandbox`, `prep_repo`, and `build_review_context` are all deterministic graph nodes, not agent tool calls. `build_review_context` reads thread state and the diff, computes the set of (file, line) tuples that are part of the PR diff (so `add_finding` can validate against it), and produces the user message that seeds the agent run.
## Open questions
1.**Severity threshold for default surfacing.** Default to `medium`+? `high`+? Worth tuning against the eval set. Independent of the cap (currently 4). `informational` is always below the threshold by design — it's a UI-only tier.
2.**Pre-existing-bug findings.** Drop entirely from the review (clean), or surface in the review body for `high`/`critical` only (more complete)? Currently leaning drop, with a system prompt instruction telling the agent not to report them in the first place.
3.**Findings dedup across runs.** When the agent calls `add_finding` on a re-review, do we trust it not to duplicate, or do we do server-side dedup based on (file, line, category, description-similarity)? Probably trust the agent for now (it sees existing findings in the prompt) and add server-side dedup only if we observe duplicates in eval.
4.**Updated-but-not-resolved findings on re-review.** When a finding stays open but the agent revises its suggestion (e.g. new commits made the original fix obsolete but the issue stands), do we leave the existing GitHub thread alone, post a follow-up reply, or close + repost? Leaning leave-alone for v1.
5.**Reviewer system prompt.** Needs to bake in: single evolving findings list, prefer updating existing findings over adding new ones, only surface in-diff findings, write actionable suggestions where possible, severity calibration matches Devin Review. Highest-leverage piece for matching Devin's perceived quality — iterate against the eval set.
## Implementation order
1.**Findings state + tools.** Add `Finding` schema (with `start_line`/`end_line`/`suggestion`/`side`), `add_finding` / `update_finding` / `list_findings` tools, thread-state extensions. No watch, no publish — just the agent producing a findings list visible in state.
2.**Prep nodes.**`prep_repo` (clone-or-fetch + checkout) and `build_review_context` (computes diff + diff-line-set, branches on first-review vs re-review) as deterministic graph nodes before the agent.
3.**`publish_review` tool.** Posts a GitHub PR Review with body + inline comments + ```suggestion blocks. Stores `github_review_comment_id` back on each Finding for later reconciliation.
4.**Reviewer system prompt iteration.** Calibrate against the eval set in `REVIEWER_EVAL_PLAN.md` — severity calibration, in-diff-only discipline, suggestion quality.
5.**Watch mode.**`watch` flag, push webhook handler, re-review user message format, `unwatch_pr` on PR close/merge, GraphQL `resolveReviewThread` on findings that move to resolved.
6.**(Future)** UI for full findings list, server-side dedup if needed, follow-up replies in existing threads when findings are revised.
## Followups / notes
- **Protected-attribute access in `_start_langsmith_sandbox_if_needed`** (`agent/server.py:76-77`) reaches into `sandbox._sandbox._client.get_sandbox_status`. If those internals shift upstream, the warm path silently degrades to always-recreate. Worth either pinning the assumption with a comment or a focused test against a mocked LangSmith client that exercises the start-if-idle branch specifically. Not urgent.
