From bfcb714c5c00c8315cc6fa00d2e4351afda427d5 Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Thu, 11 Jun 2026 16:11:10 -0700 Subject: [PATCH] feat: PR review page (#1495) * feat: PR review page * feat: manual review trigger card on admin page * feat: finding focus mode with floating card and dimmed backdrop * fix: dim only info panel, subtle block highlight for focused finding * refactor: move PR reviews into agents page with file-tree sidebar * fix: address review feedback - paginate list_reviews until enough accessible records collected - include rename/metadata-only files in diff API with empty hunks - remount ReviewBody on head_sha change so viewed-files state resets * fix: review tree background + finding card anchoring * fix: remove no-op PR size chip and dead check links * fix: parallel diff fetch, complete reReview type, shared github_headers --------- Co-authored-by: open-swe[bot] --- agent/dashboard/review_api.py | 448 +++++++ agent/dashboard/routes.py | 61 + agent/review_style_collector.py | 4 +- agent/utils/github_checks.py | 6 +- tests/test_review_api.py | 125 ++ ui/public/gh-open-in-open-swe-dark.svg | 6 + ui/public/gh-open-in-open-swe-light.svg | 6 + ui/src/components/AppSidebar.tsx | 46 +- ui/src/components/agents/AgentGitPanel.tsx | 2 +- ui/src/components/agents/AgentsSidebar.tsx | 77 +- ui/src/components/agents/ReviewSidebar.tsx | 131 ++ ui/src/lib/api.ts | 142 ++ ui/src/routeTree.gen.ts | 43 + ui/src/routes/admin.tsx | 291 +++-- ui/src/routes/agents.tsx | 5 +- .../agents/reviews/$owner.$repo.$number.tsx | 1144 +++++++++++++++++ ui/src/routes/agents/reviews/index.tsx | 125 ++ 17 files changed, 2506 insertions(+), 156 deletions(-) create mode 100644 agent/dashboard/review_api.py create mode 100644 tests/test_review_api.py create mode 100644 ui/public/gh-open-in-open-swe-dark.svg create mode 100644 ui/public/gh-open-in-open-swe-light.svg create mode 100644 ui/src/components/agents/ReviewSidebar.tsx create mode 100644 ui/src/routes/agents/reviews/$owner.$repo.$number.tsx create mode 100644 ui/src/routes/agents/reviews/index.tsx diff --git a/agent/dashboard/review_api.py b/agent/dashboard/review_api.py new file mode 100644 index 00000000..a6672961 --- /dev/null +++ b/agent/dashboard/review_api.py @@ -0,0 +1,448 @@ +"""Read API for the PR review UI. + +Reviewer threads (``metadata.kind == "reviewer"``) hold the durable review +state for a PR: identity (``pr``), findings, watch flag, and head SHA. These +endpoints surface that state plus live PR details/diff fetched from GitHub +with the App installation token. +""" + +from __future__ import annotations + +import logging +import re +from collections.abc import Awaitable, Callable +from typing import Any, Literal + +import httpx +from fastapi import HTTPException + +from ..reviewer_diff import parse_unified_diff +from ..reviewer_findings import REVIEWER_THREAD_KIND +from ..utils.github_app import get_github_app_installation_token +from ..utils.github_checks import github_headers +from ..utils.thread_ops import langgraph_client + +logger = logging.getLogger(__name__) + +_GITHUB_API = "https://api.github.com" +_GITHUB_TIMEOUT = httpx.Timeout(15.0, connect=5.0) + +DiffLineKind = Literal["context", "add", "del"] + +_DIFF_NEW_FILE_RE = re.compile(r"^new file mode", re.MULTILINE) +_DIFF_DELETED_FILE_RE = re.compile(r"^deleted file mode", re.MULTILINE) +_DIFF_RENAME_RE = re.compile(r"^rename from ", re.MULTILINE) + + +async def _require_app_token() -> str: + token = await get_github_app_installation_token() + if not token: + raise HTTPException(503, "GitHub App token unavailable") + return token + + +async def _github_get( + path: str, token: str, *, accept: str | None = None, params: dict[str, Any] | None = None +) -> Any: + headers = github_headers(token) + if accept: + headers["Accept"] = accept + async with httpx.AsyncClient(timeout=_GITHUB_TIMEOUT) as client: + response = await client.get(f"{_GITHUB_API}{path}", headers=headers, params=params) + if response.status_code == 404: + raise HTTPException(404, "not found on GitHub") + if response.status_code >= 400: + logger.warning("GitHub GET %s failed: %s", path, response.status_code) + raise HTTPException(502, f"GitHub request failed ({response.status_code})") + if accept and "json" not in accept: + return response.text + return response.json() + + +def reviewer_thread_id(owner: str, repo: str, pr_number: int) -> str: + import uuid + + stable_key = f"{owner}/{repo}/pr/{pr_number}/reviewer" + return str(uuid.uuid5(uuid.NAMESPACE_URL, stable_key)) + + +def _findings_list(metadata: dict[str, Any]) -> list[dict[str, Any]]: + findings = metadata.get("findings") + if not isinstance(findings, list): + return [] + return [f for f in findings if isinstance(f, dict) and isinstance(f.get("id"), str)] + + +def _serialize_finding(finding: dict[str, Any], head_sha: str | None) -> dict[str, Any]: + last_confirmed = finding.get("last_confirmed_sha") + outdated = bool( + head_sha + and isinstance(last_confirmed, str) + and last_confirmed + and last_confirmed != head_sha + ) + interactions = finding.get("interactions") + return { + "id": finding.get("id"), + "severity": finding.get("severity", "low"), + "confidence": finding.get("confidence", "medium"), + "category": finding.get("category", ""), + "title": finding.get("title") or "", + "description": finding.get("description", ""), + "suggestion": finding.get("suggestion"), + "file": finding.get("file", ""), + "start_line": finding.get("start_line"), + "end_line": finding.get("end_line"), + "side": finding.get("side", "RIGHT"), + "in_diff": bool(finding.get("in_diff", True)), + "status": finding.get("status", "open"), + "outdated": outdated, + "resolution_note": finding.get("resolution_note"), + "diff_hunk": finding.get("diff_hunk"), + "github_thread_resolved": bool(finding.get("github_thread_resolved")), + "github_review_comment_id": ( + finding["github_review_comment_id"] + if isinstance(finding.get("github_review_comment_id"), int) + else None + ), + "interactions": interactions if isinstance(interactions, list) else [], + } + + +_BUG_SEVERITIES = frozenset({"high", "critical"}) + + +def classify_finding(finding: dict[str, Any]) -> Literal["bug", "investigate", "informational"]: + """Map our severity/confidence model onto the UI's Bugs/Flags split.""" + severity = finding.get("severity", "low") + confidence = finding.get("confidence", "medium") + if severity in _BUG_SEVERITIES and confidence == "high": + return "bug" + if severity != "low": + return "investigate" + return "informational" + + +def _finding_counts(findings: list[dict[str, Any]]) -> dict[str, int]: + counts = {"open": 0, "resolved": 0, "dismissed": 0, "bugs": 0, "flags": 0} + for finding in findings: + status = finding.get("status", "open") + if status in counts: + counts[status] += 1 + if status == "open": + if classify_finding(finding) == "bug": + counts["bugs"] += 1 + else: + counts["flags"] += 1 + return counts + + +def _run_status(thread: dict[str, Any], metadata: dict[str, Any]) -> str: + if thread.get("status") == "busy": + return "running" + latest = metadata.get("latest_run_status") + if latest in {"pending", "running"}: + return "running" + if latest in {"error", "failed", "timeout", "interrupted"}: + return "error" + return "idle" + + +def _thread_review_summary(thread: dict[str, Any]) -> dict[str, Any] | None: + metadata = thread.get("metadata") if isinstance(thread.get("metadata"), dict) else {} + pr = metadata.get("pr") + if not isinstance(pr, dict): + return None + owner = pr.get("owner") + name = pr.get("name") + number = pr.get("number") + if not (isinstance(owner, str) and isinstance(name, str) and isinstance(number, int)): + return None + findings = _findings_list(metadata) + updated_at = thread.get("updated_at") + return { + "thread_id": thread.get("thread_id"), + "owner": owner, + "repo": name, + "full_name": f"{owner}/{name}", + "number": number, + "title": pr.get("title") or f"PR #{number}", + "url": pr.get("url") or f"https://github.com/{owner}/{name}/pull/{number}", + "head_ref": pr.get("head_ref") or "", + "base_ref": pr.get("base_ref") or "", + "head_sha": metadata.get("head_sha") or "", + "watch": bool(metadata.get("watch")), + "status": _run_status(thread, metadata), + "counts": _finding_counts(findings), + "updated_at": updated_at if isinstance(updated_at, str) else None, + } + + +async def list_reviews( + limit: int = 100, + *, + is_accessible: Callable[[dict[str, Any]], Awaitable[bool]] | None = None, + page_size: int = 100, + max_scan: int = 1000, +) -> list[dict[str, Any]]: + """List review summaries, newest first. + + When ``is_accessible`` is given, keeps paging through reviewer threads + until ``limit`` accessible summaries are collected (or ``max_scan`` + threads have been examined), so inaccessible records don't crowd + accessible ones out of a single fixed-size page. + """ + client = langgraph_client() + summaries: list[dict[str, Any]] = [] + offset = 0 + while len(summaries) < limit and offset < max_scan: + threads = await client.threads.search( + metadata={"kind": REVIEWER_THREAD_KIND}, + limit=page_size, + offset=offset, + sort_by="updated_at", + sort_order="desc", + ) + if not threads: + break + for thread in threads: + if not isinstance(thread, dict): + continue + summary = _thread_review_summary(thread) + if not summary: + continue + if is_accessible is not None and not await is_accessible(summary): + continue + summaries.append(summary) + if len(summaries) >= limit: + break + if len(threads) < page_size: + break + offset += page_size + return summaries + + +def _user_ref(value: Any) -> dict[str, Any] | None: + if not isinstance(value, dict): + return None + login = value.get("login") + if not isinstance(login, str): + return None + return {"login": login, "avatar_url": value.get("avatar_url")} + + +def _serialize_pr_details(payload: dict[str, Any]) -> dict[str, Any]: + labels = payload.get("labels") + state = payload.get("state") + if payload.get("merged"): + state = "merged" + elif payload.get("draft"): + state = "draft" + return { + "state": state if isinstance(state, str) else "open", + "title": payload.get("title") or "", + "body": payload.get("body") or "", + "additions": payload.get("additions") or 0, + "deletions": payload.get("deletions") or 0, + "changed_files": payload.get("changed_files") or 0, + "commits": payload.get("commits") or 0, + "head_sha": (payload.get("head") or {}).get("sha") or "", + "head_ref": (payload.get("head") or {}).get("ref") or "", + "base_ref": (payload.get("base") or {}).get("ref") or "", + "author": _user_ref(payload.get("user")), + "assignees": [ + user + for user in (_user_ref(value) for value in payload.get("assignees") or []) + if user is not None + ], + "requested_reviewers": [ + user + for user in (_user_ref(value) for value in payload.get("requested_reviewers") or []) + if user is not None + ], + "labels": [ + {"name": label.get("name"), "color": label.get("color")} + for label in (labels if isinstance(labels, list) else []) + if isinstance(label, dict) and isinstance(label.get("name"), str) + ], + } + + +async def _fetch_check_runs(owner: str, repo: str, sha: str, token: str) -> list[dict[str, Any]]: + if not sha: + return [] + try: + payload = await _github_get( + f"/repos/{owner}/{repo}/commits/{sha}/check-runs", + token, + params={"per_page": 50}, + ) + except HTTPException: + return [] + runs = payload.get("check_runs") if isinstance(payload, dict) else None + out: list[dict[str, Any]] = [] + for run in runs if isinstance(runs, list) else []: + if not isinstance(run, dict): + continue + out.append( + { + "name": run.get("name") or "", + "status": run.get("status") or "", + "conclusion": run.get("conclusion"), + "url": run.get("html_url"), + } + ) + return out + + +async def get_review(owner: str, repo: str, pr_number: int) -> dict[str, Any]: + thread_id = reviewer_thread_id(owner, repo, pr_number) + client = langgraph_client() + try: + thread = await client.threads.get(thread_id) + except Exception as exc: # noqa: BLE001 + raise HTTPException(404, "review not found") from exc + if not isinstance(thread, dict): + raise HTTPException(404, "review not found") + metadata = thread.get("metadata") if isinstance(thread.get("metadata"), dict) else {} + summary = _thread_review_summary(thread) + if not summary: + raise HTTPException(404, "review not found") + + token = await _require_app_token() + pr_payload = await _github_get(f"/repos/{owner}/{repo}/pulls/{pr_number}", token) + details = _serialize_pr_details(pr_payload if isinstance(pr_payload, dict) else {}) + head_sha = details["head_sha"] or summary["head_sha"] + checks = await _fetch_check_runs(owner, repo, head_sha, token) + + findings = [_serialize_finding(finding, head_sha) for finding in _findings_list(metadata)] + findings.sort( + key=lambda f: ( + f["status"] != "open", + {"bug": 0, "investigate": 1, "informational": 2}[classify_finding(f)], + f["file"], + f["start_line"] or 0, + ) + ) + for finding in findings: + finding["group"] = classify_finding(finding) + + return {**summary, "pr": details, "checks": checks, "findings": findings} + + +def parse_diff_files(diff_text: str) -> list[dict[str, Any]]: + """Parse a unified diff into renderable per-file hunk/line records. + + Iterates the raw ``diff --git`` sections so metadata-only changes + (pure renames, mode changes) still appear, with empty hunks. + """ + raw_sections = _split_file_sections(diff_text) + parsed_by_file = {file_diff.file: file_diff for file_diff in parse_unified_diff(diff_text)} + files: list[dict[str, Any]] = [] + for path, section in raw_sections.items(): + status = "modified" + if _DIFF_NEW_FILE_RE.search(section): + status = "added" + elif _DIFF_DELETED_FILE_RE.search(section): + status = "deleted" + elif _DIFF_RENAME_RE.search(section): + status = "renamed" + file_diff = parsed_by_file.get(path) + hunks = [] + additions = 0 + deletions = 0 + for hunk in file_diff.hunks if file_diff else (): + lines: list[dict[str, Any]] = [] + old_line = hunk.old_start + new_line = hunk.new_start + body_lines = hunk.body.splitlines() + for raw in body_lines[1:]: + if raw.startswith("+"): + lines.append({"kind": "add", "new_line": new_line, "text": raw[1:]}) + new_line += 1 + additions += 1 + elif raw.startswith("-"): + lines.append({"kind": "del", "old_line": old_line, "text": raw[1:]}) + old_line += 1 + deletions += 1 + elif raw.startswith("\\"): + continue + else: + lines.append( + { + "kind": "context", + "old_line": old_line, + "new_line": new_line, + "text": raw[1:] if raw.startswith(" ") else raw, + } + ) + old_line += 1 + new_line += 1 + hunks.append( + { + "header": body_lines[0] if body_lines else "", + "old_start": hunk.old_start, + "new_start": hunk.new_start, + "lines": lines, + } + ) + files.append( + { + "path": path, + "status": status, + "additions": additions, + "deletions": deletions, + "hunks": hunks, + } + ) + return files + + +def _split_file_sections(diff_text: str) -> dict[str, str]: + sections: dict[str, str] = {} + current_file: str | None = None + current_lines: list[str] = [] + header_re = re.compile(r"^diff --git a/(?P.+?) b/(?P.+?)$") + for line in diff_text.splitlines(): + match = header_re.match(line) + if match: + if current_file is not None: + sections[current_file] = "\n".join(current_lines) + current_file = match.group("b") + current_lines = [line] + elif current_file is not None: + current_lines.append(line) + if current_file is not None: + sections[current_file] = "\n".join(current_lines) + return sections + + +async def get_review_diff(owner: str, repo: str, pr_number: int) -> dict[str, Any]: + token = await _require_app_token() + diff_text = await _github_get( + f"/repos/{owner}/{repo}/pulls/{pr_number}", + token, + accept="application/vnd.github.diff", + ) + files = parse_diff_files(diff_text if isinstance(diff_text, str) else "") + return { + "files": files, + "total_additions": sum(f["additions"] for f in files), + "total_deletions": sum(f["deletions"] for f in files), + } + + +async def trigger_re_review(owner: str, repo: str, pr_number: int, login: str) -> dict[str, Any]: + from ..utils.slack import GitHubPrRef + from ..webapp import trigger_pr_review_from_ref + + pr_ref = GitHubPrRef( + owner=owner, + repo=repo, + number=pr_number, + url=f"https://github.com/{owner}/{repo}/pull/{pr_number}", + ) + result = await trigger_pr_review_from_ref(pr_ref, source="dashboard", github_login=login) + if not result.get("success"): + raise HTTPException(502, str(result.get("error") or "could not trigger review")) + return result diff --git a/agent/dashboard/routes.py b/agent/dashboard/routes.py index 721351ad..423ddcce 100644 --- a/agent/dashboard/routes.py +++ b/agent/dashboard/routes.py @@ -58,6 +58,12 @@ from .profiles import ( upsert_profile, ) from .repo_access import require_repo_access_for_user +from .review_api import ( + get_review, + get_review_diff, + list_reviews, + trigger_re_review, +) from .review_style_jobs import ( cancel_review_style_analysis, start_bootstrap_analysis, @@ -703,6 +709,61 @@ async def api_list_review_styles( return out +@router.get("/reviews") +async def api_list_reviews( + session: dict[str, Any] = _SESSION_DEP, +) -> list[dict[str, Any]]: + login = session["sub"] + access_cache: dict[str, bool] = {} + + async def is_accessible(summary: dict[str, Any]) -> bool: + full_name = summary["full_name"] + if full_name not in access_cache: + try: + await require_repo_access_for_user(login, full_name) + access_cache[full_name] = True + except HTTPException as exc: + if exc.status_code not in {403, 404}: + raise + access_cache[full_name] = False + return access_cache[full_name] + + return await list_reviews(is_accessible=is_accessible) + + +@router.get("/reviews/{owner}/{repo}/{pr_number}") +async def api_get_review( + owner: str, + repo: str, + pr_number: int, + session: dict[str, Any] = _SESSION_DEP, +) -> dict[str, Any]: + await require_repo_access_for_user(session["sub"], f"{owner}/{repo}") + return await get_review(owner, repo, pr_number) + + +@router.get("/reviews/{owner}/{repo}/{pr_number}/diff") +async def api_get_review_diff( + owner: str, + repo: str, + pr_number: int, + session: dict[str, Any] = _SESSION_DEP, +) -> dict[str, Any]: + await require_repo_access_for_user(session["sub"], f"{owner}/{repo}") + return await get_review_diff(owner, repo, pr_number) + + +@router.post("/reviews/{owner}/{repo}/{pr_number}/re-review") +async def api_re_review( + owner: str, + repo: str, + pr_number: int, + session: dict[str, Any] = _SESSION_DEP, +) -> dict[str, Any]: + await require_repo_access_for_user(session["sub"], f"{owner}/{repo}") + return await trigger_re_review(owner, repo, pr_number, session["sub"]) + + @router.post("/review-styles") async def api_create_review_style( body: ReviewStyleCreate, diff --git a/agent/review_style_collector.py b/agent/review_style_collector.py index 81d2f80e..0c0520eb 100644 --- a/agent/review_style_collector.py +++ b/agent/review_style_collector.py @@ -47,7 +47,7 @@ class ReviewStyleSamples: reviews_scanned: int = 0 -def _github_headers(token: str) -> dict[str, str]: +def github_headers(token: str) -> dict[str, str]: return { "Authorization": f"Bearer {token}", "Accept": "application/vnd.github+json", @@ -157,7 +157,7 @@ async def collect_review_samples( ) -> ReviewStyleSamples: """Sample recent merged PR feedback to identify reviewer style.""" full_name = f"{owner}/{repo}" - headers = _github_headers(token) + headers = github_headers(token) raw_entries: list[tuple[str, int, ReviewSample]] = [] reviewer_counts: Counter[str] = Counter() diff --git a/agent/utils/github_checks.py b/agent/utils/github_checks.py index e4faf81f..5a71ec7e 100644 --- a/agent/utils/github_checks.py +++ b/agent/utils/github_checks.py @@ -26,7 +26,7 @@ _GITHUB_API_BASE = "https://api.github.com" CheckConclusion = Literal["success", "neutral", "failure"] -def _github_headers(token: str) -> dict[str, str]: +def github_headers(token: str) -> dict[str, str]: return { "Authorization": f"Bearer {token}", "Accept": "application/vnd.github+json", @@ -67,7 +67,7 @@ async def create_review_check_run( try: async with httpx.AsyncClient() as client: response = await client.post( - url, headers=_github_headers(token), json=payload, timeout=30 + url, headers=github_headers(token), json=payload, timeout=30 ) response.raise_for_status() except httpx.HTTPError: @@ -105,7 +105,7 @@ async def complete_review_check_run( try: async with httpx.AsyncClient() as client: response = await client.patch( - url, headers=_github_headers(token), json=payload, timeout=30 + url, headers=github_headers(token), json=payload, timeout=30 ) response.raise_for_status() except httpx.HTTPError: diff --git a/tests/test_review_api.py b/tests/test_review_api.py new file mode 100644 index 00000000..1fcef768 --- /dev/null +++ b/tests/test_review_api.py @@ -0,0 +1,125 @@ +from agent.dashboard.review_api import ( + _finding_counts, + _serialize_finding, + _thread_review_summary, + classify_finding, + parse_diff_files, + reviewer_thread_id, +) +from agent.webapp import generate_reviewer_thread_id + +DIFF = """\ +diff --git a/src/app.py b/src/app.py +index 111..222 100644 +--- a/src/app.py ++++ b/src/app.py +@@ -1,4 +1,5 @@ + import os +-x = 1 ++x = 2 ++y = 3 + print(x) +diff --git a/new.txt b/new.txt +new file mode 100644 +index 000..333 +--- /dev/null ++++ b/new.txt +@@ -0,0 +1,2 @@ ++hello ++world +diff --git a/gone.txt b/gone.txt +deleted file mode 100644 +index 444..000 +--- a/gone.txt ++++ /dev/null +@@ -1,1 +0,0 @@ +-bye +""" + + +def test_parse_diff_files_statuses_and_counts(): + files = parse_diff_files(DIFF) + by_path = {f["path"]: f for f in files} + assert by_path["src/app.py"]["status"] == "modified" + assert by_path["src/app.py"]["additions"] == 2 + assert by_path["src/app.py"]["deletions"] == 1 + assert by_path["new.txt"]["status"] == "added" + assert by_path["new.txt"]["additions"] == 2 + assert by_path["gone.txt"]["status"] == "deleted" + assert by_path["gone.txt"]["deletions"] == 1 + + +def test_parse_diff_files_line_numbers(): + files = parse_diff_files(DIFF) + app = next(f for f in files if f["path"] == "src/app.py") + lines = app["hunks"][0]["lines"] + assert lines[0] == {"kind": "context", "old_line": 1, "new_line": 1, "text": "import os"} + assert lines[1] == {"kind": "del", "old_line": 2, "text": "x = 1"} + assert lines[2] == {"kind": "add", "new_line": 2, "text": "x = 2"} + assert lines[3] == {"kind": "add", "new_line": 3, "text": "y = 3"} + assert lines[4] == {"kind": "context", "old_line": 3, "new_line": 4, "text": "print(x)"} + + +def test_classify_finding(): + assert classify_finding({"severity": "critical", "confidence": "high"}) == "bug" + assert classify_finding({"severity": "high", "confidence": "high"}) == "bug" + assert classify_finding({"severity": "high", "confidence": "medium"}) == "investigate" + assert classify_finding({"severity": "medium", "confidence": "high"}) == "investigate" + assert classify_finding({"severity": "low", "confidence": "high"}) == "informational" + + +def test_finding_counts_only_open_in_groups(): + findings = [ + {"id": "f_1", "severity": "high", "confidence": "high", "status": "open"}, + {"id": "f_2", "severity": "medium", "confidence": "high", "status": "open"}, + {"id": "f_3", "severity": "high", "confidence": "high", "status": "resolved"}, + {"id": "f_4", "severity": "low", "confidence": "low", "status": "dismissed"}, + ] + counts = _finding_counts(findings) + assert counts == {"open": 2, "resolved": 1, "dismissed": 1, "bugs": 1, "flags": 1} + + +def test_serialize_finding_outdated(): + finding = {"id": "f_1", "last_confirmed_sha": "aaa"} + assert _serialize_finding(finding, "bbb")["outdated"] is True + assert _serialize_finding(finding, "aaa")["outdated"] is False + assert _serialize_finding(finding, None)["outdated"] is False + assert _serialize_finding({"id": "f_2"}, "bbb")["outdated"] is False + + +def test_thread_review_summary(): + thread = { + "thread_id": "t1", + "status": "idle", + "updated_at": "2026-06-10T00:00:00Z", + "metadata": { + "kind": "reviewer", + "pr": { + "owner": "acme", + "name": "repo", + "number": 7, + "title": "Fix things", + "head_ref": "fix", + "base_ref": "main", + }, + "head_sha": "abc", + "watch": True, + "latest_run_status": "success", + "findings": [{"id": "f_1", "severity": "high", "confidence": "high", "status": "open"}], + }, + } + summary = _thread_review_summary(thread) + assert summary is not None + assert summary["owner"] == "acme" + assert summary["number"] == 7 + assert summary["status"] == "idle" + assert summary["watch"] is True + assert summary["counts"]["bugs"] == 1 + + +def test_thread_review_summary_requires_pr_meta(): + assert _thread_review_summary({"metadata": {"kind": "reviewer"}}) is None + + +def test_reviewer_thread_id_matches_webapp(): + assert reviewer_thread_id("acme", "repo", 7) == generate_reviewer_thread_id("acme", "repo", 7) diff --git a/ui/public/gh-open-in-open-swe-dark.svg b/ui/public/gh-open-in-open-swe-dark.svg new file mode 100644 index 00000000..279928a1 --- /dev/null +++ b/ui/public/gh-open-in-open-swe-dark.svg @@ -0,0 +1,6 @@ + + + + + Open in Open SWE + diff --git a/ui/public/gh-open-in-open-swe-light.svg b/ui/public/gh-open-in-open-swe-light.svg new file mode 100644 index 00000000..02c57e93 --- /dev/null +++ b/ui/public/gh-open-in-open-swe-light.svg @@ -0,0 +1,6 @@ + + + + + Open in Open SWE + diff --git a/ui/src/components/AppSidebar.tsx b/ui/src/components/AppSidebar.tsx index a7d8ec14..aa1b2502 100644 --- a/ui/src/components/AppSidebar.tsx +++ b/ui/src/components/AppSidebar.tsx @@ -1,4 +1,4 @@ -import { Link } from "@tanstack/react-router"; +import { Link } from "@tanstack/react-router" import { IoArrowBackOutline, IoCloudOutline, @@ -6,25 +6,25 @@ import { IoOptionsOutline, IoSettingsOutline, IoStatsChartOutline, -} from "react-icons/io5"; -import type { ComponentType, SVGProps } from "react"; +} from "react-icons/io5" +import type { ComponentType, SVGProps } from "react" -import type { SessionUser } from "@/lib/api"; -import { SidebarUserMenu } from "@/components/SidebarUserMenu"; +import type { SessionUser } from "@/lib/api" +import { SidebarUserMenu } from "@/components/SidebarUserMenu" import { SidebarCollapseButton, SidebarFrame, useSidebarLayout, -} from "@/components/sidebar-layout"; -import { cn } from "@/lib/utils"; +} from "@/components/sidebar-layout" +import { cn } from "@/lib/utils" -type IconType = ComponentType>; +type IconType = ComponentType> interface NavItem { - to: string; - label: string; - icon: IconType; - adminOnly?: boolean; + to: string + label: string + icon: IconType + adminOnly?: boolean } const NAV: Array = [ @@ -33,12 +33,15 @@ const NAV: Array = [ { to: "/review", label: "Open SWE Review", icon: IoGitPullRequestOutline }, { to: "/usage", label: "Usage", icon: IoStatsChartOutline }, { to: "/admin", label: "Admin", icon: IoSettingsOutline, adminOnly: true }, -]; +] export function AppSidebar({ user }: { user: SessionUser }) { - const layout = useSidebarLayout(); + const layout = useSidebarLayout() return ( - +
Back to Agents {NAV.filter((n) => !n.adminOnly || user.is_admin).map((item) => { - const Icon = item.icon; + const Icon = item.icon return ( {item.label} - ); + ) })} @@ -88,5 +92,5 @@ export function AppSidebar({ user }: { user: SessionUser }) {
- ); + ) } diff --git a/ui/src/components/agents/AgentGitPanel.tsx b/ui/src/components/agents/AgentGitPanel.tsx index 050ce3dd..e2871505 100644 --- a/ui/src/components/agents/AgentGitPanel.tsx +++ b/ui/src/components/agents/AgentGitPanel.tsx @@ -141,7 +141,7 @@ function PanelResizeHandle({ ) } -function treeThemeStyle(): React.CSSProperties { +export function treeThemeStyle(): React.CSSProperties { return { "--trees-theme-sidebar-bg": "var(--ui-surface)", "--trees-theme-sidebar-fg": "var(--ui-text)", diff --git a/ui/src/components/agents/AgentsSidebar.tsx b/ui/src/components/agents/AgentsSidebar.tsx index f0b96c2a..4fff720d 100644 --- a/ui/src/components/agents/AgentsSidebar.tsx +++ b/ui/src/components/agents/AgentsSidebar.tsx @@ -24,6 +24,11 @@ import type { ComponentType, SVGProps } from "react" import type { SessionUser } from "@/lib/api" import type { AgentSource, AgentThread } from "@/lib/agents/types" import { SidebarUserMenu } from "@/components/SidebarUserMenu" +import { + ReviewFileTree, + ReviewSidebarProvider, + useReviewSidebarData, +} from "@/components/agents/ReviewSidebar" import { Button } from "@/components/ui/button" import { SidebarCollapseButton, @@ -84,6 +89,7 @@ interface AgentsSidebarProps { const NAV = [ { to: "/agents/automations", label: "Automations", icon: LightningIcon }, { to: "/my-settings", label: "Dashboard", icon: ChartLineUpIcon }, + { to: "/agents/reviews", label: "Reviews", icon: GitPullRequestIcon }, ] as const export function AgentsSidebar({ user, activeThreadId }: AgentsSidebarProps) { @@ -92,6 +98,7 @@ export function AgentsSidebar({ user, activeThreadId }: AgentsSidebarProps) { useSeedAgentThreadDetails(threads, activeThreadId) const groups = groupThreads(threads) const layout = useSidebarLayout() + const reviewSidebar = useReviewSidebarData() return ( -
- - - - -
+ {reviewSidebar ? ( + + ) : ( +
+ + + + +
+ )}
@@ -410,9 +421,11 @@ export function AgentsShell({ children: React.ReactNode }) { return ( -
- -
{children}
-
+ +
+ +
{children}
+
+ ) } diff --git a/ui/src/components/agents/ReviewSidebar.tsx b/ui/src/components/agents/ReviewSidebar.tsx new file mode 100644 index 00000000..d04f37bb --- /dev/null +++ b/ui/src/components/agents/ReviewSidebar.tsx @@ -0,0 +1,131 @@ +import { createContext, useContext, useEffect, useMemo, useState } from "react" +import { + FileTree, + useFileTree, + useFileTreeSelection, +} from "@pierre/trees/react" + +import type { GitStatusEntry } from "@pierre/trees" +import type { ReviewDiffFile } from "@/lib/api" +import { Skeleton } from "@/components/ui/skeleton" +import { treeThemeStyle } from "@/components/agents/AgentGitPanel" + +export interface ReviewSidebarData { + title: string + files: Array | null + selected: string | null + viewed: Set + onSelect: (path: string) => void +} + +const ReviewSidebarContext = createContext<{ + data: ReviewSidebarData | null + setData: (data: ReviewSidebarData | null) => void +} | null>(null) + +export function ReviewSidebarProvider({ + children, +}: { + children: React.ReactNode +}) { + const [data, setData] = useState(null) + const value = useMemo(() => ({ data, setData }), [data]) + return ( + + {children} + + ) +} + +export function useReviewSidebarData(): ReviewSidebarData | null { + return useContext(ReviewSidebarContext)?.data ?? null +} + +export function useRegisterReviewSidebar(data: ReviewSidebarData) { + const setData = useContext(ReviewSidebarContext)?.setData + useEffect(() => { + if (!setData) return + setData(data) + return () => setData(null) + }, [setData, data]) +} + +export function ReviewFileTree({ data }: { data: ReviewSidebarData }) { + return ( +
+
+ {data.title} +
+ {!data.files ? ( +
+ +
+ ) : ( + + )} +
+ ) +} + +function ReviewFileTreeExplorer({ + files, + selected, + onSelect, +}: { + files: Array + selected: string | null + onSelect: (path: string) => void +}) { + const paths = useMemo(() => files.map((file) => file.path), [files]) + const gitStatus = useMemo>( + () => files.map((file) => ({ path: file.path, status: file.status })), + [files] + ) + + const { model } = useFileTree({ + paths, + gitStatus, + initialExpansion: "open", + flattenEmptyDirectories: true, + icons: "standard", + }) + + useEffect(() => { + model.resetPaths(paths) + }, [model, paths]) + + useEffect(() => { + model.setGitStatus(gitStatus) + }, [model, gitStatus]) + + const selection = useFileTreeSelection(model) + useEffect(() => { + const path = selection[0] + if (path) onSelect(path) + }, [selection, onSelect]) + + useEffect(() => { + if (selected) { + model.scrollToPath(selected, { focus: false }) + } + }, [model, selected]) + + return ( +
+ +
+ ) +} diff --git a/ui/src/lib/api.ts b/ui/src/lib/api.ts index 4d123164..33d6c6f0 100644 --- a/ui/src/lib/api.ts +++ b/ui/src/lib/api.ts @@ -259,6 +259,129 @@ export interface AgentInstructions { updated_at?: string; } +export type FindingSeverity = "low" | "medium" | "high" | "critical"; +export type FindingConfidence = "low" | "medium" | "high"; +export type FindingStatus = "open" | "resolved" | "dismissed"; +export type FindingGroup = "bug" | "investigate" | "informational"; + +export interface FindingInteraction { + kind: "human_reply" | "bot_reply"; + author?: string; + body?: string; + created_at?: string; +} + +export interface ReviewFinding { + id: string; + severity: FindingSeverity; + confidence: FindingConfidence; + category: string; + title: string; + description: string; + suggestion: string | null; + file: string; + start_line: number | null; + end_line: number | null; + side: "LEFT" | "RIGHT"; + in_diff: boolean; + status: FindingStatus; + outdated: boolean; + resolution_note: string | null; + diff_hunk: string | null; + github_thread_resolved: boolean; + github_review_comment_id: number | null; + interactions: Array; + group: FindingGroup; +} + +export interface ReviewCounts { + open: number; + resolved: number; + dismissed: number; + bugs: number; + flags: number; +} + +export interface ReviewSummary { + thread_id: string; + owner: string; + repo: string; + number: number; + title: string; + url: string; + head_ref: string; + base_ref: string; + head_sha: string; + watch: boolean; + status: "running" | "error" | "idle"; + counts: ReviewCounts; + updated_at: string | null; + full_name?: string; +} + +export interface ReviewUserRef { + login: string; + avatar_url?: string | null; +} + +export interface ReviewCheckRun { + name: string; + status: string; + conclusion: string | null; + url: string | null; +} + +export interface ReviewPrDetails { + state: string; + title: string; + body: string; + additions: number; + deletions: number; + changed_files: number; + commits: number; + head_sha: string; + head_ref: string; + base_ref: string; + author: ReviewUserRef | null; + assignees: Array; + requested_reviewers: Array; + labels: Array<{ name: string; color: string | null }>; +} + +export interface ReviewDetail extends ReviewSummary { + pr: ReviewPrDetails; + checks: Array; + findings: Array; +} + +export interface ReviewDiffLine { + kind: "context" | "add" | "del"; + old_line?: number; + new_line?: number; + text: string; +} + +export interface ReviewDiffHunk { + header: string; + old_start: number; + new_start: number; + lines: Array; +} + +export interface ReviewDiffFile { + path: string; + status: "added" | "deleted" | "modified" | "renamed"; + additions: number; + deletions: number; + hunks: Array; +} + +export interface ReviewDiffPayload { + files: Array; + total_additions: number; + total_deletions: number; +} + export const api = { me: () => request("/me"), options: () => request("/options"), @@ -347,6 +470,25 @@ export const api = { `/admin/user-mappings/${encodeURIComponent(github_login)}`, { method: "DELETE" }, ), + listReviews: () => request>("/reviews"), + getReview: (owner: string, repo: string, number: number) => + request( + `/reviews/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/${number}`, + ), + getReviewDiff: (owner: string, repo: string, number: number) => + request( + `/reviews/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/${number}/diff`, + ), + reReview: (owner: string, repo: string, number: number) => + request<{ + success: boolean; + queued: boolean; + thread_id: string; + pr_url: string; + }>( + `/reviews/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/${number}/re-review`, + { method: "POST" }, + ), logout: () => request("/auth/logout", { method: "POST" }), }; diff --git a/ui/src/routeTree.gen.ts b/ui/src/routeTree.gen.ts index e9ff5def..769e8540 100644 --- a/ui/src/routeTree.gen.ts +++ b/ui/src/routeTree.gen.ts @@ -22,10 +22,12 @@ import { Route as AgentsIndexRouteImport } from './routes/agents/index' import { Route as ReviewStylesRouteImport } from './routes/review_.styles' import { Route as AgentsInstructionsRouteImport } from './routes/agents_.instructions' import { Route as AgentsThreadIdRouteImport } from './routes/agents/$threadId' +import { Route as AgentsReviewsIndexRouteImport } from './routes/agents/reviews/index' import { Route as AgentsAutomationsIndexRouteImport } from './routes/agents/automations/index' import { Route as ReviewRepositoriesOwnerRouteImport } from './routes/review_.repositories.$owner' import { Route as AgentsAutomationsNewRouteImport } from './routes/agents/automations/new' import { Route as AgentsAutomationsScheduleIdRouteImport } from './routes/agents/automations/$scheduleId' +import { Route as AgentsReviewsOwnerRepoNumberRouteImport } from './routes/agents/reviews/$owner.$repo.$number' const UsageRoute = UsageRouteImport.update({ id: '/usage', @@ -92,6 +94,11 @@ const AgentsThreadIdRoute = AgentsThreadIdRouteImport.update({ path: '/$threadId', getParentRoute: () => AgentsRoute, } as any) +const AgentsReviewsIndexRoute = AgentsReviewsIndexRouteImport.update({ + id: '/reviews/', + path: '/reviews/', + getParentRoute: () => AgentsRoute, +} as any) const AgentsAutomationsIndexRoute = AgentsAutomationsIndexRouteImport.update({ id: '/automations/', path: '/automations/', @@ -113,6 +120,12 @@ const AgentsAutomationsScheduleIdRoute = path: '/automations/$scheduleId', getParentRoute: () => AgentsRoute, } as any) +const AgentsReviewsOwnerRepoNumberRoute = + AgentsReviewsOwnerRepoNumberRouteImport.update({ + id: '/reviews/$owner/$repo/$number', + path: '/reviews/$owner/$repo/$number', + getParentRoute: () => AgentsRoute, + } as any) export interface FileRoutesByFullPath { '/': typeof IndexRoute @@ -132,6 +145,8 @@ export interface FileRoutesByFullPath { '/agents/automations/new': typeof AgentsAutomationsNewRoute '/review/repositories/$owner': typeof ReviewRepositoriesOwnerRoute '/agents/automations/': typeof AgentsAutomationsIndexRoute + '/agents/reviews/': typeof AgentsReviewsIndexRoute + '/agents/reviews/$owner/$repo/$number': typeof AgentsReviewsOwnerRepoNumberRoute } export interface FileRoutesByTo { '/': typeof IndexRoute @@ -150,6 +165,8 @@ export interface FileRoutesByTo { '/agents/automations/new': typeof AgentsAutomationsNewRoute '/review/repositories/$owner': typeof ReviewRepositoriesOwnerRoute '/agents/automations': typeof AgentsAutomationsIndexRoute + '/agents/reviews': typeof AgentsReviewsIndexRoute + '/agents/reviews/$owner/$repo/$number': typeof AgentsReviewsOwnerRepoNumberRoute } export interface FileRoutesById { __root__: typeof rootRouteImport @@ -170,6 +187,8 @@ export interface FileRoutesById { '/agents/automations/new': typeof AgentsAutomationsNewRoute '/review_/repositories/$owner': typeof ReviewRepositoriesOwnerRoute '/agents/automations/': typeof AgentsAutomationsIndexRoute + '/agents/reviews/': typeof AgentsReviewsIndexRoute + '/agents/reviews/$owner/$repo/$number': typeof AgentsReviewsOwnerRepoNumberRoute } export interface FileRouteTypes { fileRoutesByFullPath: FileRoutesByFullPath @@ -191,6 +210,8 @@ export interface FileRouteTypes { | '/agents/automations/new' | '/review/repositories/$owner' | '/agents/automations/' + | '/agents/reviews/' + | '/agents/reviews/$owner/$repo/$number' fileRoutesByTo: FileRoutesByTo to: | '/' @@ -209,6 +230,8 @@ export interface FileRouteTypes { | '/agents/automations/new' | '/review/repositories/$owner' | '/agents/automations' + | '/agents/reviews' + | '/agents/reviews/$owner/$repo/$number' id: | '__root__' | '/' @@ -228,6 +251,8 @@ export interface FileRouteTypes { | '/agents/automations/new' | '/review_/repositories/$owner' | '/agents/automations/' + | '/agents/reviews/' + | '/agents/reviews/$owner/$repo/$number' fileRoutesById: FileRoutesById } export interface RootRouteChildren { @@ -338,6 +363,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof AgentsThreadIdRouteImport parentRoute: typeof AgentsRoute } + '/agents/reviews/': { + id: '/agents/reviews/' + path: '/reviews' + fullPath: '/agents/reviews/' + preLoaderRoute: typeof AgentsReviewsIndexRouteImport + parentRoute: typeof AgentsRoute + } '/agents/automations/': { id: '/agents/automations/' path: '/automations' @@ -366,6 +398,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof AgentsAutomationsScheduleIdRouteImport parentRoute: typeof AgentsRoute } + '/agents/reviews/$owner/$repo/$number': { + id: '/agents/reviews/$owner/$repo/$number' + path: '/reviews/$owner/$repo/$number' + fullPath: '/agents/reviews/$owner/$repo/$number' + preLoaderRoute: typeof AgentsReviewsOwnerRepoNumberRouteImport + parentRoute: typeof AgentsRoute + } } } @@ -375,6 +414,8 @@ interface AgentsRouteChildren { AgentsAutomationsScheduleIdRoute: typeof AgentsAutomationsScheduleIdRoute AgentsAutomationsNewRoute: typeof AgentsAutomationsNewRoute AgentsAutomationsIndexRoute: typeof AgentsAutomationsIndexRoute + AgentsReviewsIndexRoute: typeof AgentsReviewsIndexRoute + AgentsReviewsOwnerRepoNumberRoute: typeof AgentsReviewsOwnerRepoNumberRoute } const AgentsRouteChildren: AgentsRouteChildren = { @@ -383,6 +424,8 @@ const AgentsRouteChildren: AgentsRouteChildren = { AgentsAutomationsScheduleIdRoute: AgentsAutomationsScheduleIdRoute, AgentsAutomationsNewRoute: AgentsAutomationsNewRoute, AgentsAutomationsIndexRoute: AgentsAutomationsIndexRoute, + AgentsReviewsIndexRoute: AgentsReviewsIndexRoute, + AgentsReviewsOwnerRepoNumberRoute: AgentsReviewsOwnerRepoNumberRoute, } const AgentsRouteWithChildren = diff --git a/ui/src/routes/admin.tsx b/ui/src/routes/admin.tsx index d15afa5b..8080fd82 100644 --- a/ui/src/routes/admin.tsx +++ b/ui/src/routes/admin.tsx @@ -1,6 +1,6 @@ -import { Navigate, createFileRoute } from "@tanstack/react-router"; -import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { useEffect, useState } from "react"; +import { Link, Navigate, createFileRoute } from "@tanstack/react-router" +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query" +import { useEffect, useMemo, useState } from "react" import type { DatadogConnectBody, @@ -8,41 +8,41 @@ import type { ModelOption, TeamSettings, UserMapping, -} from "@/lib/api"; -import { AppShell, SettingsRow, SettingsSection } from "@/components/AppShell"; -import { Button } from "@/components/ui/button"; -import { Input } from "@/components/ui/input"; +} from "@/lib/api" +import { AppShell, SettingsRow, SettingsSection } from "@/components/AppShell" +import { Button } from "@/components/ui/button" +import { Input } from "@/components/ui/input" import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue, -} from "@/components/ui/select"; -import { Skeleton } from "@/components/ui/skeleton"; -import { api } from "@/lib/api"; -import { useSession } from "@/lib/session"; +} from "@/components/ui/select" +import { Skeleton } from "@/components/ui/skeleton" +import { api } from "@/lib/api" +import { useSession } from "@/lib/session" -export const Route = createFileRoute("/admin")({ component: AdminPage }); +export const Route = createFileRoute("/admin")({ component: AdminPage }) function AdminPage() { - const session = useSession(); + const session = useSession() const options = useQuery({ queryKey: ["options"], queryFn: api.options, enabled: !!session.data?.is_admin, - }); + }) if (session.isLoading) { return (
- ); + ) } - if (!session.data) return ; - if (!session.data.is_admin) return ; + if (!session.data) return + if (!session.data.is_admin) return return ( + + - ); + ) } -const PAGE_SIZE = 20; +const PR_URL_RE = /^https:\/\/github\.com\/([^/\s]+)\/([^/\s]+)\/pull\/(\d+)/ + +function TriggerReviewSection() { + const [url, setUrl] = useState("") + const [error, setError] = useState(null) + const [message, setMessage] = useState(null) + + const parsed = useMemo(() => { + const match = PR_URL_RE.exec(url.trim()) + if (!match) return null + const [, owner, repo, number] = match + if (!owner || !repo || !number) return null + return { owner, repo, number: Number(number) } + }, [url]) + + const trigger = useMutation({ + mutationFn: () => { + if (!parsed) throw new Error("invalid PR URL") + return api.reReview(parsed.owner, parsed.repo, parsed.number) + }, + onSuccess: (result) => { + setError(null) + setMessage( + result.queued + ? "Review queued — a run is already in progress on this PR." + : "Review started." + ) + }, + onError: (e: Error) => { + setMessage(null) + setError(e.message) + }, + }) + + return ( + +
+
+ { + setUrl(e.target.value) + setMessage(null) + setError(null) + }} + /> + +
+ {url.trim() && !parsed && ( +

+ Enter a full PR URL like https://github.com/owner/repo/pull/123 +

+ )} + {message && parsed && ( +

+ {message}{" "} + + View review + +

+ )} + {error &&

{error}

} +
+ + ) +} + +const PAGE_SIZE = 20 function UserMappingsSection({ enabled }: { enabled: boolean }) { - const [error, setError] = useState(null); - const [page, setPage] = useState(1); + const [error, setError] = useState(null) + const [page, setPage] = useState(1) const mappings = useQuery({ queryKey: ["adminUserMappings", page], queryFn: () => api.adminListUserMappings(page, PAGE_SIZE), enabled, - }); + }) - const total = mappings.data?.total ?? 0; - const pageCount = Math.max(1, Math.ceil(total / PAGE_SIZE)); + const total = mappings.data?.total ?? 0 + const pageCount = Math.max(1, Math.ceil(total / PAGE_SIZE)) useEffect(() => { if (!mappings.isFetching && page > pageCount) { - setPage(pageCount); + setPage(pageCount) } - }, [mappings.isFetching, page, pageCount]); + }, [mappings.isFetching, page, pageCount]) const remove = useMutation({ mutationFn: (gh: string) => api.adminDeleteUserMapping(gh), onSuccess: () => void mappings.refetch(), onError: (e: Error) => setError(e.message), - }); + }) - const items = mappings.data?.items ?? []; + const items = mappings.data?.items ?? [] return ( PAGE_SIZE && (
- {total} mapping{total === 1 ? "" : "s"} · page {page} of {pageCount} + {total} mapping{total === 1 ? "" : "s"} · page {page} of{" "} + {pageCount}
{error &&

{error}

} - ); + ) } interface RolePickerProps { - label: string; - description: string; - models: Array; - model: string | null; - effort: string | null; - onChange: (model: string, effort: string) => void; - disabled: boolean; + label: string + description: string + models: Array + model: string | null + effort: string | null + onChange: (model: string, effort: string) => void + disabled: boolean } function RolePicker({ @@ -473,34 +568,34 @@ function RolePicker({ onChange, disabled, }: RolePickerProps) { - const [localModel, setLocalModel] = useState(model ?? ""); - const [localEffort, setLocalEffort] = useState(effort ?? ""); + const [localModel, setLocalModel] = useState(model ?? "") + const [localEffort, setLocalEffort] = useState(effort ?? "") useEffect(() => { - setLocalModel(model ?? ""); - setLocalEffort(effort ?? ""); - }, [model, effort]); + setLocalModel(model ?? "") + setLocalEffort(effort ?? "") + }, [model, effort]) - const selectedModel = models.find((m) => m.id === localModel); - const availableEfforts = selectedModel?.efforts ?? []; + const selectedModel = models.find((m) => m.id === localModel) + const availableEfforts = selectedModel?.efforts ?? [] const handleModelChange = (value: string | null) => { - if (!value) return; - const nextModel = models.find((m) => m.id === value); - if (!nextModel) return; + if (!value) return + const nextModel = models.find((m) => m.id === value) + if (!nextModel) return const nextEffort = nextModel.efforts.includes(localEffort) ? localEffort - : nextModel.default_effort; - setLocalModel(value); - setLocalEffort(nextEffort); - onChange(value, nextEffort); - }; + : nextModel.default_effort + setLocalModel(value) + setLocalEffort(nextEffort) + onChange(value, nextEffort) + } const handleEffortChange = (value: string | null) => { - if (!value || !localModel) return; - setLocalEffort(value); - onChange(localModel, value); - }; + if (!value || !localModel) return + setLocalEffort(value) + onChange(localModel, value) + } return ( - @@ -539,5 +638,5 @@ function RolePicker({
} /> - ); + ) } diff --git a/ui/src/routes/agents.tsx b/ui/src/routes/agents.tsx index 80a5ead7..ef9f45b0 100644 --- a/ui/src/routes/agents.tsx +++ b/ui/src/routes/agents.tsx @@ -25,7 +25,10 @@ function AgentsLayout() { }) const [, section, threadId] = pathname.split("/") const activeThreadId = - section === "agents" && threadId && threadId !== "automations" + section === "agents" && + threadId && + threadId !== "automations" && + threadId !== "reviews" ? threadId : undefined diff --git a/ui/src/routes/agents/reviews/$owner.$repo.$number.tsx b/ui/src/routes/agents/reviews/$owner.$repo.$number.tsx new file mode 100644 index 00000000..41c34a6d --- /dev/null +++ b/ui/src/routes/agents/reviews/$owner.$repo.$number.tsx @@ -0,0 +1,1144 @@ +import { Link, Navigate, createFileRoute } from "@tanstack/react-router" +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query" +import { useCallback, useEffect, useMemo, useRef, useState } from "react" +import { + ArrowClockwiseIcon, + ArrowLeftIcon, + BugBeetleIcon, + CaretDownIcon, + CheckCircleIcon, + CheckIcon, + CircleIcon, + CopyIcon, + FlagIcon, + GitPullRequestIcon, + InfoIcon, + XCircleIcon, + XIcon, +} from "@phosphor-icons/react" +import { IoLogoGithub } from "react-icons/io5" + +import type { + ReviewCheckRun, + ReviewDetail, + ReviewDiffFile, + ReviewDiffLine, + ReviewFinding, + ReviewUserRef, +} from "@/lib/api" +import { Markdown } from "@/components/agents/ported" +import { useRegisterReviewSidebar } from "@/components/agents/ReviewSidebar" +import { Skeleton } from "@/components/ui/skeleton" +import { api } from "@/lib/api" +import { useSession } from "@/lib/session" +import { cn } from "@/lib/utils" + +export const Route = createFileRoute("/agents/reviews/$owner/$repo/$number")({ + component: ReviewDetailPage, +}) + +type SideTab = "info" | "chat" + +const GROUP_STYLES = { + bug: { label: "Bug", className: "text-destructive", Icon: BugBeetleIcon }, + investigate: { + label: "Investigate", + className: "text-amber-500", + Icon: FlagIcon, + }, + informational: { + label: "Informational", + className: "text-muted-foreground", + Icon: InfoIcon, + }, +} as const + +function findingAnchorLabel(finding: ReviewFinding): string { + if (finding.start_line === null || finding.end_line === null) + return finding.file + if (finding.start_line === finding.end_line) + return `${finding.file}:${finding.end_line}` + return `${finding.file}:${finding.start_line}-${finding.end_line}` +} + +function isAnchored(finding: ReviewFinding): boolean { + return Boolean(finding.file) && finding.in_diff && finding.end_line !== null +} + +type HighlightEdge = { top: boolean; bottom: boolean } | null + +function highlightRange( + lines: Array, + finding: ReviewFinding +): { start: number; end: number } | null { + let start = -1 + let end = -1 + for (let index = 0; index < lines.length; index++) { + const line = lines[index] + if (line && lineMatchesFinding(line, finding)) { + if (start === -1) start = index + end = index + } + } + return start === -1 ? null : { start, end } +} + +function lineMatchesFinding( + line: ReviewDiffLine, + finding: ReviewFinding +): boolean { + if (finding.end_line === null) return false + const start = finding.start_line ?? finding.end_line + const lineNumber = finding.side === "LEFT" ? line.old_line : line.new_line + if (lineNumber === undefined) return false + if (finding.side === "LEFT" && line.kind !== "del") return false + if (finding.side === "RIGHT" && line.kind === "del") return false + return lineNumber >= start && lineNumber <= finding.end_line +} + +function isFindingAnchorRow( + line: ReviewDiffLine, + finding: ReviewFinding +): boolean { + if (finding.end_line === null) return false + const lineNumber = finding.side === "LEFT" ? line.old_line : line.new_line + if (finding.side === "LEFT" && line.kind !== "del") return false + if (finding.side === "RIGHT" && line.kind === "del") return false + return lineNumber === finding.end_line +} + +function findingClipboardText(finding: ReviewFinding): string { + const style = GROUP_STYLES[finding.group] + const lines = [ + `**${style.label}: ${finding.title}**`, + `${findingAnchorLabel(finding)}`, + "", + finding.description, + ] + if (finding.suggestion) + lines.push("", "```suggestion", finding.suggestion, "```") + return lines.join("\n") +} + +function ReviewDetailPage() { + const { owner, repo, number } = Route.useParams() + const prNumber = Number(number) + const session = useSession() + const detail = useQuery({ + queryKey: ["review", owner, repo, prNumber], + queryFn: () => api.getReview(owner, repo, prNumber), + enabled: !!session.data && Number.isFinite(prNumber), + refetchInterval: (query) => + query.state.data?.status === "running" ? 5000 : false, + }) + const diff = useQuery({ + queryKey: ["reviewDiff", owner, repo, prNumber], + queryFn: () => api.getReviewDiff(owner, repo, prNumber), + enabled: !!session.data && Number.isFinite(prNumber), + }) + + const queryClient = useQueryClient() + const headSha = detail.data?.head_sha + const seenShaRef = useRef(headSha) + useEffect(() => { + if (headSha && seenShaRef.current && headSha !== seenShaRef.current) { + void queryClient.invalidateQueries({ + queryKey: ["reviewDiff", owner, repo, prNumber], + }) + } + if (headSha) seenShaRef.current = headSha + }, [headSha, queryClient, owner, repo, prNumber]) + + if (session.isLoading) { + return ( +
+ +
+ ) + } + if (!session.data) return + + return ( +
+
+ + + Reviews + + / + + + + {owner}/{repo}#{number} + {detail.data ? ` ${detail.data.pr.title}` : ""} + + +
+ + {detail.error ? ( +
+ {detail.error.message} +
+ ) : !detail.data ? ( +
+ + +
+ ) : ( + + )} +
+ ) +} + +function ReviewBody({ + detail, + diffFiles, +}: { + detail: ReviewDetail + diffFiles: Array | null +}) { + const [sideTab, setSideTab] = useState("info") + const [selectedFile, setSelectedFile] = useState(null) + const fileRefs = useRef>({}) + const anchorRefs = useRef>({}) + const [expandedFiles, setExpandedFiles] = useState>( + {} + ) + const [focused, setFocused] = useState(null) + const [cardPos, setCardPos] = useState<{ top: number; left: number | null }>( + { top: 96, left: null } + ) + + const viewedStorageKey = `open-swe.review.viewed.${detail.owner}/${detail.repo}/${detail.number}.${detail.head_sha}` + const [viewed, setViewed] = useState>(() => { + if (typeof window === "undefined") return new Set() + try { + const raw = window.localStorage.getItem(viewedStorageKey) + return new Set(raw ? (JSON.parse(raw) as Array) : []) + } catch { + return new Set() + } + }) + const toggleViewed = useCallback( + (path: string) => { + setViewed((prev) => { + const next = new Set(prev) + if (next.has(path)) next.delete(path) + else next.add(path) + window.localStorage.setItem( + viewedStorageKey, + JSON.stringify(Array.from(next)) + ) + return next + }) + }, + [viewedStorageKey] + ) + + const readStorageKey = `open-swe.review.read.${detail.thread_id}` + const [read, setRead] = useState>(() => { + if (typeof window === "undefined") return new Set() + try { + const raw = window.localStorage.getItem(readStorageKey) + return new Set(raw ? (JSON.parse(raw) as Array) : []) + } catch { + return new Set() + } + }) + const persistRead = useCallback( + (next: Set) => { + setRead(next) + window.localStorage.setItem( + readStorageKey, + JSON.stringify(Array.from(next)) + ) + }, + [readStorageKey] + ) + const markRead = useCallback( + (id: string) => { + persistRead(new Set(read).add(id)) + }, + [persistRead, read] + ) + const markAllRead = useCallback(() => { + persistRead(new Set(detail.findings.map((f) => f.id))) + }, [detail.findings, persistRead]) + + const findingsByFile = useMemo(() => { + const byFile = new Map>() + for (const finding of detail.findings) { + if (!isAnchored(finding)) continue + const list = byFile.get(finding.file) ?? [] + list.push(finding) + byFile.set(finding.file, list) + } + return byFile + }, [detail.findings]) + + const linesLeft = useMemo(() => { + if (!diffFiles) return null + return diffFiles + .filter((file) => !viewed.has(file.path)) + .reduce((acc, file) => acc + file.additions + file.deletions, 0) + }, [diffFiles, viewed]) + + const scrollToFile = useCallback((path: string) => { + setSelectedFile(path) + setExpandedFiles((prev) => ({ ...prev, [path]: true })) + requestAnimationFrame(() => { + fileRefs.current[path]?.scrollIntoView({ + block: "start", + behavior: "smooth", + }) + }) + }, []) + + const openFinding = useCallback( + (finding: ReviewFinding) => { + markRead(finding.id) + setFocused(finding) + if (!isAnchored(finding)) { + setCardPos({ top: 96, left: null }) + return + } + setExpandedFiles((prev) => ({ ...prev, [finding.file]: true })) + requestAnimationFrame(() => { + const node = + anchorRefs.current[finding.id] ?? fileRefs.current[finding.file] + node?.scrollIntoView({ block: "center" }) + requestAnimationFrame(() => { + const rect = anchorRefs.current[finding.id]?.getBoundingClientRect() + if (rect) { + setCardPos({ + top: Math.min( + Math.max(rect.top, 64), + window.innerHeight - 360 + ), + left: Math.min(rect.right + 12, window.innerWidth - 412 - 8), + }) + } else { + setCardPos({ top: 96, left: null }) + } + }) + }) + }, + [markRead] + ) + + const closeFinding = useCallback(() => setFocused(null), []) + + const sidebarData = useMemo( + () => ({ + title: `PR #${detail.number}`, + files: diffFiles, + selected: selectedFile, + viewed, + onSelect: scrollToFile, + }), + [detail.number, diffFiles, selectedFile, viewed, scrollToFile] + ) + useRegisterReviewSidebar(sidebarData) + + useEffect(() => { + if (!focused) return + const onKeyDown = (event: KeyboardEvent) => { + if (event.key === "Escape") setFocused(null) + } + window.addEventListener("keydown", onKeyDown) + return () => window.removeEventListener("keydown", onKeyDown) + }, [focused]) + + return ( +
+
+
+ +
+ {detail.pr.body ? ( + + ) : ( +

+ This PR has no description. +

+ )} +
+ +
+
+

Changes

+ {linesLeft !== null && ( + + {linesLeft === 0 + ? "All lines reviewed" + : `${linesLeft} lines left`} + + )} +
+ {!diffFiles ? ( + + ) : diffFiles.length === 0 ? ( +

+ No diff available. +

+ ) : ( +
+ {diffFiles.map((file) => ( + toggleViewed(file.path)} + expanded={ + expandedFiles[file.path] ?? !viewed.has(file.path) + } + onToggleExpanded={() => + setExpandedFiles((prev) => ({ + ...prev, + [file.path]: !( + prev[file.path] ?? !viewed.has(file.path) + ), + })) + } + onFindingClick={openFinding} + sectionRef={(node) => { + fileRefs.current[file.path] = node + }} + anchorRef={(id, node) => { + anchorRefs.current[id] = node + }} + /> + ))} +
+ )} +
+
+
+ + + + {focused && ( + + )} +
+ ) +} + +function PrHeader({ detail }: { detail: ReviewDetail }) { + const { pr } = detail + const stateStyles: Record = { + open: "border-emerald-600/40 text-emerald-500", + draft: "border-border text-muted-foreground", + merged: "border-purple-600/40 text-purple-500", + closed: "border-red-600/40 text-red-500", + } + return ( +
+ + + {pr.state} + +

+ + {pr.title} + +

+
+ {pr.author && ( + {pr.author.login} + )} + + {pr.base_ref} + + ← + + {pr.head_ref} + + + {pr.changed_files} file{pr.changed_files === 1 ? "" : "s"} + + +{pr.additions} + -{pr.deletions} +
+
+ ) +} + +function FileDiffCard({ + file, + findings, + focused, + viewed, + onToggleViewed, + expanded, + onToggleExpanded, + onFindingClick, + sectionRef, + anchorRef, +}: { + file: ReviewDiffFile + findings: Array + focused: ReviewFinding | null + viewed: boolean + onToggleViewed: () => void + expanded: boolean + onToggleExpanded: () => void + onFindingClick: (finding: ReviewFinding) => void + sectionRef: (node: HTMLDivElement | null) => void + anchorRef: (id: string, node: HTMLDivElement | null) => void +}) { + const fileFocused = + focused?.file === file.path && isAnchored(focused) ? focused : null + + return ( +
+
+ + + +{file.additions} + -{file.deletions} + + {findings.length > 0 && ( + + + {findings.length} + + )} + +
+ {expanded && ( +
+ {file.hunks.map((hunk, hunkIndex) => { + const range = + fileFocused !== null + ? highlightRange(hunk.lines, fileFocused) + : null + const anchorRows = new Map>() + for (const finding of findings) { + const index = hunk.lines.findIndex((hunkLine) => + lineMatchesFinding(hunkLine, finding) + ) + if (index !== -1) { + anchorRows.set(index, [ + ...(anchorRows.get(index) ?? []), + finding, + ]) + } + } + return ( +
+
+ {hunk.header} +
+ {hunk.lines.map((line, lineIndex) => { + const lineFindings = findings.filter((finding) => + isFindingAnchorRow(line, finding) + ) + const anchorFindings = anchorRows.get(lineIndex) ?? [] + let highlight: HighlightEdge = null + if ( + range && + lineIndex >= range.start && + lineIndex <= range.end + ) { + highlight = { + top: lineIndex === range.start, + bottom: lineIndex === range.end, + } + } + return ( + + ) + })} +
+ ) + })} +
+ )} +
+ ) +} + +function DiffLineRow({ + line, + findings, + anchorFindings, + highlight, + onFindingClick, + anchorRef, +}: { + line: ReviewDiffLine + findings: Array + anchorFindings: Array + highlight: HighlightEdge + onFindingClick: (finding: ReviewFinding) => void + anchorRef: (id: string, node: HTMLDivElement | null) => void +}) { + const first = findings[0] + return ( +
0 + ? (node) => { + for (const finding of anchorFindings) anchorRef(finding.id, node) + } + : undefined + } + className={cn( + "flex", + line.kind === "add" && "bg-emerald-500/10", + line.kind === "del" && "bg-red-500/10", + highlight && "border-x border-sky-400/50 bg-sky-400/10", + highlight?.top && "rounded-t-sm border-t", + highlight?.bottom && "rounded-b-sm border-b" + )} + > + + {line.old_line ?? ""} + + + {line.new_line ?? ""} + + + {line.kind === "add" ? "+" : line.kind === "del" ? "-" : ""} + + {line.text} + {first && ( + + )} +
+ ) +} + +function FindingFloatingCard({ + detail, + finding, + top, + left, + onClose, +}: { + detail: ReviewDetail + finding: ReviewFinding + top: number + left: number | null + onClose: () => void +}) { + const [copied, setCopied] = useState(false) + const style = GROUP_STYLES[finding.group] + const Icon = style.Icon + const githubUrl = + finding.github_review_comment_id !== null + ? `${detail.url}#discussion_r${finding.github_review_comment_id}` + : null + const rangeLabel = + finding.end_line !== null + ? `${finding.side === "LEFT" ? "L" : "R"}${finding.start_line ?? finding.end_line}${ + finding.start_line !== null && finding.start_line !== finding.end_line + ? `-${finding.end_line}` + : "" + }` + : finding.file + + const copy = () => { + void navigator.clipboard + .writeText(findingClipboardText(finding)) + .then(() => { + setCopied(true) + window.setTimeout(() => setCopied(false), 1500) + }) + } + + return ( +
+
+ + + {style.label} + + + {rangeLabel} + + {finding.outdated && Outdated} + {finding.status !== "open" && {finding.status}} + +
+
+
{finding.title}
+
+ +
+ {finding.resolution_note && ( +

+ Resolution: {finding.resolution_note} +

+ )} +
+
+ + {githubUrl && ( + + + View on GitHub + + )} +
+
+ ) +} + +function Badgeish({ children }: { children: React.ReactNode }) { + return ( + + {children} + + ) +} + +function SidePanel({ + detail, + tab, + onTabChange, + read, + dimmed, + onMarkAllRead, + onFindingClick, +}: { + detail: ReviewDetail + tab: SideTab + onTabChange: (tab: SideTab) => void + read: Set + dimmed: boolean + onMarkAllRead: () => void + onFindingClick: (finding: ReviewFinding) => void +}) { + const qc = useQueryClient() + const reReview = useMutation({ + mutationFn: () => api.reReview(detail.owner, detail.repo, detail.number), + onSuccess: () => { + void qc.invalidateQueries({ + queryKey: ["review", detail.owner, detail.repo, detail.number], + }) + }, + }) + + const bugs = detail.findings.filter((f) => f.group === "bug") + const flags = detail.findings.filter((f) => f.group !== "bug") + const openBugs = bugs.filter((f) => f.status === "open") + const openFlags = flags.filter((f) => f.status === "open") + + return ( + + ) +} + +function FindingSection({ + icon: HeaderIcon, + label, + emptyLabel, + findings, + read, + onFindingClick, + action, +}: { + icon: (typeof GROUP_STYLES)["bug"]["Icon"] + label: string + emptyLabel: string + findings: Array + read: Set + onFindingClick: (finding: ReviewFinding) => void + action?: React.ReactNode +}) { + const [collapsed, setCollapsed] = useState(false) + return ( +
+
+ + {action} +
+ {!collapsed && + (findings.length === 0 ? ( +

{emptyLabel}

+ ) : ( +
+ {findings.map((finding) => { + const style = GROUP_STYLES[finding.group] + const Icon = style.Icon + const isRead = read.has(finding.id) + const muted = finding.status !== "open" || isRead + return ( + + ) + })} +
+ ))} +
+ ) +} + +function ChecksSection({ checks }: { checks: Array }) { + return ( +
+

Checks

+ {checks.length === 0 ? ( +

No checks reported.

+ ) : ( +
+ {checks.map((check, index) => + check.url ? ( + + + {check.name} + + ) : ( + + + {check.name} + + ) + )} +
+ )} +
+ ) +} + +function CheckStatusIcon({ check }: { check: ReviewCheckRun }) { + if (check.status !== "completed") { + return ( + + ) + } + if (check.conclusion === "success" || check.conclusion === "neutral") { + return + } + if (check.conclusion === "skipped") { + return + } + return +} + +function PeopleSection({ + title, + people, +}: { + title: string + people: Array +}) { + return ( +
+

{title}

+ {people.length === 0 ? ( +

None

+ ) : ( +
+ {people.map((person) => ( +
+ {person.avatar_url ? ( + + ) : ( + + )} + {person.login} +
+ ))} +
+ )} +
+ ) +} diff --git a/ui/src/routes/agents/reviews/index.tsx b/ui/src/routes/agents/reviews/index.tsx new file mode 100644 index 00000000..65e801bf --- /dev/null +++ b/ui/src/routes/agents/reviews/index.tsx @@ -0,0 +1,125 @@ +import { Link, createFileRoute } from "@tanstack/react-router" +import { useQuery } from "@tanstack/react-query" +import { + BugBeetleIcon, + FlagIcon, + GitPullRequestIcon, +} from "@phosphor-icons/react" + +import type { ReviewSummary } from "@/lib/api" +import { Skeleton } from "@/components/ui/skeleton" +import { api } from "@/lib/api" +import { useSession } from "@/lib/session" +import { cn } from "@/lib/utils" + +export const Route = createFileRoute("/agents/reviews/")({ + component: ReviewsPage, +}) + +function statusBadge(review: ReviewSummary) { + if (review.status === "running") { + return ( + + + Reviewing + + ) + } + if (review.status === "error") { + return Failed + } + return null +} + +function ReviewsPage() { + const session = useSession() + const reviews = useQuery({ + queryKey: ["reviews"], + queryFn: api.listReviews, + enabled: !!session.data, + refetchInterval: (query) => + query.state.data?.some((r) => r.status === "running") ? 5000 : false, + }) + + return ( +
+
+

+ PR Reviews +

+

+ Pull requests reviewed by Open SWE Review. Click into one for the full + analysis. +

+ +
+ {reviews.isLoading && ( +
+ +
+ )} + {reviews.error && ( +

+ {reviews.error.message} +

+ )} + {reviews.data && reviews.data.length === 0 && ( +

+ No reviews yet. Enable repositories under Open SWE Review settings + and open a PR. +

+ )} +
+ {(reviews.data ?? []).map((review) => ( + +
+ +
+
+ {review.title} +
+
+ {review.owner}/{review.repo}#{review.number} + {review.head_ref && ( + + {review.head_ref} + + )} +
+
+
+
+ {statusBadge(review)} + 0 + ? "text-[var(--ui-danger)]" + : "text-[var(--ui-text-muted)]" + )} + > + + {review.counts.bugs} + + + + {review.counts.flags} + +
+ + ))} +
+
+
+
+ ) +}