mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 20:53:15 +00:00
* feat: implement reviewer findings, publish_review, and watch mode
Build out the reviewer agent end-to-end against the design in
REVIEWER_DESIGN.md:
- Findings as first-class state on the reviewer thread metadata
(`agent/reviewer_findings.py`): Finding TypedDict with start_line/end_line
ranges, suggestion text for ```suggestion blocks, github_review_comment_id
for cross-run reconciliation, diff_hunk for UI rendering. Thread-level
metadata gets `kind=reviewer`, `pr`, `last_reviewed_sha`, `watch` so a
future frontend can list reviewer threads via the langgraph SDK.
- Diff utilities (`agent/reviewer_diff.py`): parse_unified_diff,
compute_diff_line_set for in-diff validation, extract_diff_hunk for
caching the hunk on a Finding, compute_diff_in_sandbox for SHA-to-SHA
diffs against the prepped repo.
- Tools: `add_finding` (validates against the diff line set so out-of-diff
ranges fail at creation, not at GitHub-publish), `update_finding`,
`list_findings`, `publish_review`. The reviewer agent's tool list is
swapped from `[]` (direct shell `gh api` calls) to these four.
- Publish path (`agent/reviewer_publish.py` + `agent/tools/publish_review.py`):
one POST /reviews call with body + inline comments + ```suggestion blocks,
per-comment IDs stored back on findings, GraphQL `resolveReviewThread`
fired for findings transitioning open->resolved on a re-review.
- Reviewer graph: deterministic clone-or-fetch + checkout in the factory
before the agent's first model call (warm- and cold-path symmetric);
computed diff and in-diff line set passed via runnable config; system
prompt rewritten for the single-evolving-findings model, severity ladder,
in-diff-only discipline, and watch-mode reconciliation flow.
- Watch mode in webapp.py: `push` event + `pull_request` closed/reopened
added to supported events. New `process_github_push_event` resolves the
open PR for the pushed branch, gates on the reviewer thread's `watch`
flag, builds a re-review configurable, and triggers a run on the same
canonical thread. `process_github_pr_close` toggles watch on
closed/reopened. `set_reviewer_thread_metadata` is called on first
review to install `kind=reviewer` + PR identity + watch=True.
- Eval harness: target.py now extracts `add_finding` calls (mapped to the
legacy {file, line, body, severity} shape the judge expects) and passes
the right configurable so the prep step has base/head SHAs.
- Tests: new unit suites for findings helpers, diff parsing, finding tools,
publish rendering + GraphQL resolve, and watch-mode webhook handlers
(push triggers re-review only when watching, idempotent on unchanged
head SHA, PR close disables watch). Updated existing reviewer-webhook
tests to mock `set_reviewer_thread_metadata`.
- REVIEWER_EVAL_PLAN.md removed per user request; folded relevant context
into REVIEWER_DESIGN.md.
* fix(reviewer): correct git diff flags, scope, dedup, and review-comments URL
Address PR #1253 review findings:
- compute_diff_in_sandbox dropped the invalid `--no-prefix=false` flag
(`option no-prefix takes no value` — every prep run was failing
silently and the agent saw an empty diff).
- compute_diff_in_sandbox grew a `merge_base` flag. First-review path
now uses three-dot `base...head` (the merge-base diff GitHub renders
on Files-changed) so we don't pick up changes that landed on the base
branch after the PR diverged. Re-review delta keeps two-dot
`last_reviewed_sha..head` since that's exactly the new commits.
- publish_review skips findings that already carry
`github_review_comment_id`. Without this, watched re-reviews
re-posted every previously surfaced finding, and only the most-recent
duplicate's id would later resolve when the issue got addressed.
- fetch_review_comments URL now includes `{pull_number}` —
`/repos/{owner}/{repo}/pulls/{pr_number}/reviews/{review_id}/comments`
is the canonical endpoint; the old form 404s, so comment ids were
never stored and watch-mode resolution couldn't run.
Three new tests cover: three-dot vs two-dot wiring, no `--no-prefix`
flag in the executed command, and that publish_review does not re-post
findings whose `github_review_comment_id` is set.
* fix(reviewer): default publish cap from 15 to 4
A clean PR with one critical issue padded out by three lower-severity
findings is fine; fifteen is review spam. The agent can override per
call when a PR genuinely warrants more.
120 lines
4.2 KiB
Python
120 lines
4.2 KiB
Python
"""Target function for the reviewer eval.
|
|
|
|
Spawns the reviewer graph over `langgraph_sdk` for one PR, waits for
|
|
completion, and returns every `add_finding` tool call the agent made as the
|
|
structured output for the eval. Findings are normalized into the legacy
|
|
``{file, line, body, severity}`` shape so the judge prompt can stay the
|
|
verbatim form martian published.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import os
|
|
import threading
|
|
from typing import Any
|
|
|
|
from langgraph_sdk import get_client
|
|
|
|
REVIEWER_ASSISTANT_ID = os.getenv("REVIEWER_ASSISTANT_ID", "reviewer")
|
|
LANGGRAPH_URL = os.getenv("LANGGRAPH_URL", "http://localhost:2024")
|
|
|
|
_THREAD_IDS: set[str] = set()
|
|
_THREAD_IDS_LOCK = threading.Lock()
|
|
|
|
|
|
def _record_thread_id(thread_id: str) -> None:
|
|
with _THREAD_IDS_LOCK:
|
|
_THREAD_IDS.add(thread_id)
|
|
|
|
|
|
def drain_thread_ids() -> set[str]:
|
|
"""Return and clear thread IDs created by ``review_pr`` so far.
|
|
|
|
Used by ``run_eval`` to delete threads after the experiment finishes.
|
|
Underlying provider sandboxes time out via their own TTL — deleting the
|
|
LangGraph thread frees the checkpoint/metadata records, not the sandbox.
|
|
"""
|
|
with _THREAD_IDS_LOCK:
|
|
snapshot = set(_THREAD_IDS)
|
|
_THREAD_IDS.clear()
|
|
return snapshot
|
|
|
|
|
|
def _build_user_message(inputs: dict[str, Any]) -> str:
|
|
return (
|
|
f"Review pull request {inputs['pr_url']}.\n\n"
|
|
f"- repo: {inputs['repo']}\n"
|
|
f"- pr_number: {inputs['pr_number']}\n"
|
|
f"- title: {inputs.get('pr_title', '')}\n"
|
|
f"- base_sha: {inputs['base_sha']}\n"
|
|
f"- head_sha: {inputs['head_sha']}\n"
|
|
f"- base_ref: {inputs.get('base_ref', '')}\n"
|
|
f"- head_ref: {inputs.get('head_ref', '')}\n\n"
|
|
f"Record each issue you find with the `add_finding` tool, then call "
|
|
f"`publish_review` once at the end."
|
|
)
|
|
|
|
|
|
def _build_configurable(inputs: dict[str, Any]) -> dict[str, Any]:
|
|
repo = inputs.get("repo", "")
|
|
owner, _, name = repo.partition("/") if isinstance(repo, str) else ("", "", "")
|
|
return {
|
|
"__is_for_execution__": True,
|
|
"repo": {"owner": owner, "name": name},
|
|
"pr_number": inputs.get("pr_number"),
|
|
"pr_url": inputs.get("pr_url", ""),
|
|
"base_sha": inputs.get("base_sha", ""),
|
|
"head_sha": inputs.get("head_sha", ""),
|
|
"branch_name": inputs.get("head_ref", ""),
|
|
}
|
|
|
|
|
|
async def review_pr(inputs: dict[str, Any]) -> dict[str, Any]:
|
|
"""LangSmith target: run the reviewer agent on one PR."""
|
|
client = get_client(url=LANGGRAPH_URL)
|
|
thread = await client.threads.create()
|
|
thread_id: str = thread["thread_id"]
|
|
_record_thread_id(thread_id)
|
|
result = await client.runs.wait(
|
|
thread_id,
|
|
assistant_id=REVIEWER_ASSISTANT_ID,
|
|
input={"messages": [{"role": "user", "content": _build_user_message(inputs)}]},
|
|
config={"configurable": _build_configurable(inputs)},
|
|
)
|
|
return {"comments": _extract_comments(result)}
|
|
|
|
|
|
def _extract_comments(result: Any) -> list[dict[str, Any]]:
|
|
"""Collect every ``add_finding`` tool call from the run's message stream.
|
|
|
|
Normalizes the new finding shape (``start_line``/``end_line``/``description``)
|
|
into the legacy ``{file, line, body, severity}`` shape the judge prompt
|
|
consumes verbatim from martian's benchmark.
|
|
"""
|
|
if not isinstance(result, dict):
|
|
return []
|
|
comments: list[dict[str, Any]] = []
|
|
for msg in result.get("messages") or []:
|
|
if not isinstance(msg, dict):
|
|
continue
|
|
for tc in msg.get("tool_calls") or []:
|
|
if tc.get("name") != "add_finding":
|
|
continue
|
|
args = tc.get("args") or {}
|
|
file = args.get("file")
|
|
severity = args.get("severity")
|
|
description = args.get("description") or args.get("body") or ""
|
|
line = args.get("end_line")
|
|
if line is None:
|
|
line = args.get("start_line")
|
|
if not file or not severity:
|
|
continue
|
|
comments.append(
|
|
{
|
|
"file": file,
|
|
"line": line,
|
|
"body": description,
|
|
"severity": severity,
|
|
}
|
|
)
|
|
return comments
|