mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 11:33:14 +00:00
feat: render Reviews page diffs with pierre MultiFileDiff (#1517)
* feat: render Reviews page diffs with pierre MultiFileDiff The Reviews detail page used a hand-rolled hunk renderer with no syntax highlighting. Switch it to the same pierre MultiFileDiff + theming the agent chat git panel uses. - review-diff API now returns full original/modified file contents instead of hunks, via a shared build_pr_diff_files helper extracted from thread_api - findings render as right-anchored markers (pierre line annotations); focus highlight uses selectedLines; floating finding card still anchors to the marker * feat: auto-collapse a review diff card when marked as viewed * fix: URL-encode file path in Contents API fetch Filenames containing reserved URL characters (#, ?) were truncated, so those files rendered as empty/unrenderable. quote(path, safe='/') preserves the path separators while escaping the rest. --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
This commit is contained in:
parent
7dac89a3b4
commit
09d5d00e59
7 changed files with 250 additions and 427 deletions
124
agent/dashboard/pr_diff.py
Normal file
124
agent/dashboard/pr_diff.py
Normal file
|
|
@ -0,0 +1,124 @@
|
|||
"""Shared builder for full-content PR diffs.
|
||||
|
||||
Fetches a PR's changed files and their full original/modified contents so the
|
||||
UI can render syntax-highlighted diffs with pierre's ``MultiFileDiff``. Used by
|
||||
both the thread PR diff endpoint (user token) and the review diff endpoint
|
||||
(App installation token).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
from typing import Any
|
||||
from urllib.parse import quote
|
||||
|
||||
import httpx
|
||||
from fastapi import HTTPException
|
||||
|
||||
_GITHUB_API = "https://api.github.com"
|
||||
|
||||
PR_DIFF_MAX_FILES = 50
|
||||
PR_DIFF_MAX_FILE_BYTES = 200_000
|
||||
PR_DIFF_FETCH_CONCURRENCY = 5
|
||||
|
||||
|
||||
async def _fetch_file_at_ref(
|
||||
client: httpx.AsyncClient,
|
||||
semaphore: asyncio.Semaphore,
|
||||
full_name: str,
|
||||
path: str,
|
||||
ref: str,
|
||||
) -> str | None:
|
||||
async with semaphore:
|
||||
response = await client.get(
|
||||
f"{_GITHUB_API}/repos/{full_name}/contents/{quote(path, safe='/')}",
|
||||
params={"ref": ref},
|
||||
headers={"Accept": "application/vnd.github.raw+json"},
|
||||
)
|
||||
if response.status_code == 404:
|
||||
return ""
|
||||
if response.status_code != 200:
|
||||
return None
|
||||
if len(response.content) > PR_DIFF_MAX_FILE_BYTES:
|
||||
return None
|
||||
try:
|
||||
return response.content.decode("utf-8")
|
||||
except UnicodeDecodeError:
|
||||
return None
|
||||
|
||||
|
||||
async def build_pr_diff_files(
|
||||
client: httpx.AsyncClient,
|
||||
full_name: str,
|
||||
pr_number: int,
|
||||
) -> dict[str, Any]:
|
||||
"""Return ``{base_sha, head_sha, truncated, files}`` for a PR.
|
||||
|
||||
Each file carries full ``originalContent``/``modifiedContent`` (or ``None``
|
||||
for binary/oversized blobs, flagged via ``unrenderable``). ``client`` must
|
||||
already be configured with auth headers.
|
||||
"""
|
||||
pull_response = await client.get(f"{_GITHUB_API}/repos/{full_name}/pulls/{pr_number}")
|
||||
if pull_response.status_code == 404:
|
||||
raise HTTPException(404, "pull request not found")
|
||||
if pull_response.status_code != 200:
|
||||
raise HTTPException(502, f"github API error ({pull_response.status_code})")
|
||||
pull = pull_response.json()
|
||||
base_sha = pull.get("base", {}).get("sha")
|
||||
head_sha = pull.get("head", {}).get("sha")
|
||||
if not isinstance(base_sha, str) or not isinstance(head_sha, str):
|
||||
raise HTTPException(502, "github API returned an unexpected pull request payload")
|
||||
|
||||
files_response = await client.get(
|
||||
f"{_GITHUB_API}/repos/{full_name}/pulls/{pr_number}/files",
|
||||
params={"per_page": 100},
|
||||
)
|
||||
if files_response.status_code != 200:
|
||||
raise HTTPException(502, f"github API error ({files_response.status_code})")
|
||||
raw_files = files_response.json()
|
||||
if not isinstance(raw_files, list):
|
||||
raise HTTPException(502, "github API returned an unexpected files payload")
|
||||
|
||||
truncated = len(raw_files) > PR_DIFF_MAX_FILES
|
||||
raw_files = raw_files[:PR_DIFF_MAX_FILES]
|
||||
|
||||
semaphore = asyncio.Semaphore(PR_DIFF_FETCH_CONCURRENCY)
|
||||
|
||||
async def build_entry(raw: dict[str, Any]) -> dict[str, Any] | None:
|
||||
path = raw.get("filename")
|
||||
if not isinstance(path, str):
|
||||
return None
|
||||
status = raw.get("status") if isinstance(raw.get("status"), str) else "modified"
|
||||
previous = raw.get("previous_filename")
|
||||
original_path = previous if isinstance(previous, str) else path
|
||||
|
||||
original: str | None = ""
|
||||
modified: str | None = ""
|
||||
if status != "added":
|
||||
original = await _fetch_file_at_ref(
|
||||
client, semaphore, full_name, original_path, base_sha
|
||||
)
|
||||
if status != "removed":
|
||||
modified = await _fetch_file_at_ref(client, semaphore, full_name, path, head_sha)
|
||||
|
||||
return {
|
||||
"path": path,
|
||||
"previousPath": previous if isinstance(previous, str) else None,
|
||||
"status": status,
|
||||
"additions": raw.get("additions") if isinstance(raw.get("additions"), int) else 0,
|
||||
"deletions": raw.get("deletions") if isinstance(raw.get("deletions"), int) else 0,
|
||||
"originalContent": original,
|
||||
"modifiedContent": modified,
|
||||
# Binary or oversized blobs come back as None — the client renders a
|
||||
# placeholder instead of file contents.
|
||||
"unrenderable": original is None or modified is None,
|
||||
}
|
||||
|
||||
entries = await asyncio.gather(*(build_entry(raw) for raw in raw_files))
|
||||
|
||||
return {
|
||||
"base_sha": base_sha,
|
||||
"head_sha": head_sha,
|
||||
"truncated": truncated,
|
||||
"files": [entry for entry in entries if entry is not None],
|
||||
}
|
||||
|
|
@ -9,30 +9,23 @@ 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
|
||||
from .pr_diff import build_pr_diff_files
|
||||
|
||||
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()
|
||||
|
|
@ -342,105 +335,21 @@ async def get_review(owner: str, repo: str, pr_number: int) -> dict[str, Any]:
|
|||
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<a>.+?) b/(?P<b>.+?)$")
|
||||
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]:
|
||||
"""Return the PR's changed files with full original/modified contents.
|
||||
|
||||
Uses the App installation token so the diff is available regardless of who
|
||||
is viewing the review. The client renders these with pierre's MultiFileDiff.
|
||||
"""
|
||||
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 "")
|
||||
async with httpx.AsyncClient(headers=github_headers(token), timeout=_GITHUB_TIMEOUT) as client:
|
||||
diff = await build_pr_diff_files(client, f"{owner}/{repo}", pr_number)
|
||||
files = diff["files"]
|
||||
return {
|
||||
"files": files,
|
||||
"total_additions": sum(f["additions"] for f in files),
|
||||
"total_deletions": sum(f["deletions"] for f in files),
|
||||
"truncated": diff["truncated"],
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -2,7 +2,6 @@
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import base64
|
||||
import binascii
|
||||
import json
|
||||
|
|
@ -26,6 +25,7 @@ from ..utils.thread_ops import (
|
|||
)
|
||||
from .agent_overrides import normalize_profile_overrides
|
||||
from .options import SUPPORTED_MODEL_IDS, model_supports_effort, model_supports_images
|
||||
from .pr_diff import build_pr_diff_files
|
||||
from .profiles import get_profile, get_valid_access_token
|
||||
from .team_settings import get_team_default_model
|
||||
from .user_mappings import email_for_login
|
||||
|
|
@ -901,12 +901,6 @@ async def get_dashboard_thread_state(
|
|||
return result
|
||||
|
||||
|
||||
_PR_DIFF_MAX_FILES = 50
|
||||
_PR_DIFF_MAX_FILE_BYTES = 200_000
|
||||
_PR_DIFF_FETCH_CONCURRENCY = 5
|
||||
_GITHUB_API = "https://api.github.com"
|
||||
|
||||
|
||||
# No app-installation-token fallback: PR file contents must be fetched with
|
||||
# the user's own credential so GitHub enforces their current repo access.
|
||||
async def _github_token_for_login(login: str) -> str:
|
||||
|
|
@ -916,31 +910,6 @@ async def _github_token_for_login(login: str) -> str:
|
|||
return token
|
||||
|
||||
|
||||
async def _fetch_file_at_ref(
|
||||
client: httpx.AsyncClient,
|
||||
semaphore: asyncio.Semaphore,
|
||||
full_name: str,
|
||||
path: str,
|
||||
ref: str,
|
||||
) -> str | None:
|
||||
async with semaphore:
|
||||
response = await client.get(
|
||||
f"{_GITHUB_API}/repos/{full_name}/contents/{path}",
|
||||
params={"ref": ref},
|
||||
headers={"Accept": "application/vnd.github.raw+json"},
|
||||
)
|
||||
if response.status_code == 404:
|
||||
return ""
|
||||
if response.status_code != 200:
|
||||
return None
|
||||
if len(response.content) > _PR_DIFF_MAX_FILE_BYTES:
|
||||
return None
|
||||
try:
|
||||
return response.content.decode("utf-8")
|
||||
except UnicodeDecodeError:
|
||||
return None
|
||||
|
||||
|
||||
async def get_dashboard_thread_pr_diff(
|
||||
thread_id: str, login: str, *, email: str | None = None
|
||||
) -> dict[str, Any]:
|
||||
|
|
@ -957,70 +926,14 @@ async def get_dashboard_thread_pr_diff(
|
|||
"X-GitHub-Api-Version": "2022-11-28",
|
||||
}
|
||||
async with httpx.AsyncClient(headers=headers, timeout=_PROXY_REQUEST_TIMEOUT) as client:
|
||||
pull_response = await client.get(f"{_GITHUB_API}/repos/{full_name}/pulls/{pr_number}")
|
||||
if pull_response.status_code == 404:
|
||||
raise HTTPException(404, "pull request not found")
|
||||
if pull_response.status_code != 200:
|
||||
raise HTTPException(502, f"github API error ({pull_response.status_code})")
|
||||
pull = pull_response.json()
|
||||
base_sha = pull.get("base", {}).get("sha")
|
||||
head_sha = pull.get("head", {}).get("sha")
|
||||
if not isinstance(base_sha, str) or not isinstance(head_sha, str):
|
||||
raise HTTPException(502, "github API returned an unexpected pull request payload")
|
||||
|
||||
files_response = await client.get(
|
||||
f"{_GITHUB_API}/repos/{full_name}/pulls/{pr_number}/files",
|
||||
params={"per_page": 100},
|
||||
)
|
||||
if files_response.status_code != 200:
|
||||
raise HTTPException(502, f"github API error ({files_response.status_code})")
|
||||
raw_files = files_response.json()
|
||||
if not isinstance(raw_files, list):
|
||||
raise HTTPException(502, "github API returned an unexpected files payload")
|
||||
|
||||
truncated = len(raw_files) > _PR_DIFF_MAX_FILES
|
||||
raw_files = raw_files[:_PR_DIFF_MAX_FILES]
|
||||
|
||||
semaphore = asyncio.Semaphore(_PR_DIFF_FETCH_CONCURRENCY)
|
||||
|
||||
async def build_entry(raw: dict[str, Any]) -> dict[str, Any] | None:
|
||||
path = raw.get("filename")
|
||||
if not isinstance(path, str):
|
||||
return None
|
||||
status = raw.get("status") if isinstance(raw.get("status"), str) else "modified"
|
||||
previous = raw.get("previous_filename")
|
||||
original_path = previous if isinstance(previous, str) else path
|
||||
|
||||
original: str | None = ""
|
||||
modified: str | None = ""
|
||||
if status != "added":
|
||||
original = await _fetch_file_at_ref(
|
||||
client, semaphore, full_name, original_path, base_sha
|
||||
)
|
||||
if status != "removed":
|
||||
modified = await _fetch_file_at_ref(client, semaphore, full_name, path, head_sha)
|
||||
|
||||
return {
|
||||
"path": path,
|
||||
"previousPath": previous if isinstance(previous, str) else None,
|
||||
"status": status,
|
||||
"additions": raw.get("additions") if isinstance(raw.get("additions"), int) else 0,
|
||||
"deletions": raw.get("deletions") if isinstance(raw.get("deletions"), int) else 0,
|
||||
"originalContent": original,
|
||||
"modifiedContent": modified,
|
||||
# Binary or oversized blobs come back as None — the client
|
||||
# renders a placeholder instead of file contents.
|
||||
"unrenderable": original is None or modified is None,
|
||||
}
|
||||
|
||||
entries = await asyncio.gather(*(build_entry(raw) for raw in raw_files))
|
||||
diff = await build_pr_diff_files(client, full_name, pr_number)
|
||||
|
||||
return {
|
||||
"prNumber": pr_number,
|
||||
"baseSha": base_sha,
|
||||
"headSha": head_sha,
|
||||
"truncated": truncated,
|
||||
"files": [entry for entry in entries if entry is not None],
|
||||
"baseSha": diff["base_sha"],
|
||||
"headSha": diff["head_sha"],
|
||||
"truncated": diff["truncated"],
|
||||
"files": diff["files"],
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -3,62 +3,10 @@ from agent.dashboard.review_api import (
|
|||
_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"
|
||||
|
|
|
|||
|
|
@ -5,11 +5,18 @@ import {
|
|||
useFileTreeSelection,
|
||||
} from "@pierre/trees/react"
|
||||
|
||||
import type { GitStatusEntry } from "@pierre/trees"
|
||||
import type { GitStatus, GitStatusEntry } from "@pierre/trees"
|
||||
import type { ReviewDiffFile } from "@/lib/api"
|
||||
import { Skeleton } from "@/components/ui/skeleton"
|
||||
import { treeThemeStyle } from "@/components/agents/AgentGitPanel"
|
||||
|
||||
function reviewFileGitStatus(status: ReviewDiffFile["status"]): GitStatus {
|
||||
if (status === "removed") return "deleted"
|
||||
if (status === "added") return "added"
|
||||
if (status === "renamed") return "renamed"
|
||||
return "modified"
|
||||
}
|
||||
|
||||
export interface ReviewSidebarData {
|
||||
title: string
|
||||
files: Array<ReviewDiffFile> | null
|
||||
|
|
@ -82,7 +89,11 @@ function ReviewFileTreeExplorer({
|
|||
}) {
|
||||
const paths = useMemo(() => files.map((file) => file.path), [files])
|
||||
const gitStatus = useMemo<Array<GitStatusEntry>>(
|
||||
() => files.map((file) => ({ path: file.path, status: file.status })),
|
||||
() =>
|
||||
files.map((file) => ({
|
||||
path: file.path,
|
||||
status: reviewFileGitStatus(file.status),
|
||||
})),
|
||||
[files]
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -361,32 +361,22 @@ export interface ReviewDetail extends ReviewSummary {
|
|||
findings: Array<ReviewFinding>;
|
||||
}
|
||||
|
||||
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<ReviewDiffLine>;
|
||||
}
|
||||
|
||||
export interface ReviewDiffFile {
|
||||
path: string;
|
||||
status: "added" | "deleted" | "modified" | "renamed";
|
||||
previousPath: string | null;
|
||||
status: "added" | "removed" | "modified" | "renamed";
|
||||
additions: number;
|
||||
deletions: number;
|
||||
hunks: Array<ReviewDiffHunk>;
|
||||
originalContent: string;
|
||||
modifiedContent: string;
|
||||
unrenderable?: boolean;
|
||||
}
|
||||
|
||||
export interface ReviewDiffPayload {
|
||||
files: Array<ReviewDiffFile>;
|
||||
total_additions: number;
|
||||
total_deletions: number;
|
||||
truncated: boolean;
|
||||
}
|
||||
|
||||
export const api = {
|
||||
|
|
|
|||
|
|
@ -24,17 +24,22 @@ import {
|
|||
XIcon,
|
||||
} from "@phosphor-icons/react"
|
||||
import { IoLogoGithub } from "react-icons/io5"
|
||||
import { MultiFileDiff } from "@pierre/diffs/react"
|
||||
import type { DiffLineAnnotation, SelectedLineRange } from "@pierre/diffs"
|
||||
|
||||
import type {
|
||||
ReviewCheckRun,
|
||||
ReviewDetail,
|
||||
ReviewDiffFile,
|
||||
ReviewDiffLine,
|
||||
ReviewFinding,
|
||||
ReviewUserRef,
|
||||
} from "@/lib/api"
|
||||
import { Markdown } from "@/components/agents/ported"
|
||||
import { useRegisterReviewSidebar } from "@/components/agents/ReviewSidebar"
|
||||
import {
|
||||
useDiffOptions,
|
||||
warmDiffHighlighter,
|
||||
} from "@/components/agents/utils/diffUtils"
|
||||
import { Skeleton } from "@/components/ui/skeleton"
|
||||
import { api } from "@/lib/api"
|
||||
import { useSession } from "@/lib/session"
|
||||
|
|
@ -72,46 +77,19 @@ function isAnchored(finding: ReviewFinding): boolean {
|
|||
return Boolean(finding.file) && finding.in_diff && finding.end_line !== null
|
||||
}
|
||||
|
||||
type HighlightEdge = { top: boolean; bottom: boolean } | null
|
||||
function findingSide(finding: ReviewFinding): "deletions" | "additions" {
|
||||
return finding.side === "LEFT" ? "deletions" : "additions"
|
||||
}
|
||||
|
||||
function highlightRange(
|
||||
lines: Array<ReviewDiffLine>,
|
||||
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
|
||||
}
|
||||
function findingSelectedRange(finding: ReviewFinding): SelectedLineRange | null {
|
||||
if (finding.end_line === null) return null
|
||||
const side = findingSide(finding)
|
||||
return {
|
||||
start: finding.start_line ?? finding.end_line,
|
||||
end: finding.end_line,
|
||||
side,
|
||||
endSide: side,
|
||||
}
|
||||
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 {
|
||||
|
|
@ -215,14 +193,18 @@ function ReviewBody({
|
|||
const [sideTab, setSideTab] = useState<SideTab>("info")
|
||||
const [selectedFile, setSelectedFile] = useState<string | null>(null)
|
||||
const fileRefs = useRef<Record<string, HTMLDivElement | null>>({})
|
||||
const anchorRefs = useRef<Record<string, HTMLDivElement | null>>({})
|
||||
const anchorRefs = useRef<Record<string, HTMLElement | null>>({})
|
||||
const [expandedFiles, setExpandedFiles] = useState<Record<string, boolean>>(
|
||||
{}
|
||||
)
|
||||
const [focused, setFocused] = useState<ReviewFinding | null>(null)
|
||||
const [anchorEl, setAnchorEl] = useState<HTMLDivElement | null>(null)
|
||||
const [anchorEl, setAnchorEl] = useState<HTMLElement | null>(null)
|
||||
const scrollRef = useRef<HTMLDivElement | null>(null)
|
||||
|
||||
useEffect(() => {
|
||||
void warmDiffHighlighter()
|
||||
}, [])
|
||||
|
||||
const viewedStorageKey = `open-swe.review.viewed.${detail.owner}/${detail.repo}/${detail.number}.${detail.head_sha}`
|
||||
const [viewed, setViewed] = useState<Set<string>>(() => {
|
||||
if (typeof window === "undefined") return new Set()
|
||||
|
|
@ -405,12 +387,14 @@ function ReviewBody({
|
|||
focused={focused}
|
||||
viewed={viewed.has(file.path)}
|
||||
onToggleViewed={() => {
|
||||
const collapses =
|
||||
!viewed.has(file.path) &&
|
||||
expandedFiles[file.path] === undefined
|
||||
if (collapses && focused?.file === file.path)
|
||||
const becomingViewed = !viewed.has(file.path)
|
||||
if (becomingViewed && focused?.file === file.path)
|
||||
setFocused(null)
|
||||
toggleViewed(file.path)
|
||||
setExpandedFiles((prev) => ({
|
||||
...prev,
|
||||
[file.path]: !becomingViewed,
|
||||
}))
|
||||
}}
|
||||
expanded={
|
||||
expandedFiles[file.path] ?? !viewed.has(file.path)
|
||||
|
|
@ -549,17 +533,33 @@ function FileDiffCard({
|
|||
onToggleExpanded: () => void
|
||||
onFindingClick: (finding: ReviewFinding) => void
|
||||
sectionRef: (node: HTMLDivElement | null) => void
|
||||
anchorRef: (id: string, node: HTMLDivElement | null) => void
|
||||
anchorRef: (id: string, node: HTMLElement | null) => void
|
||||
}) {
|
||||
const fileFocused =
|
||||
focused?.file === file.path && isAnchored(focused) ? focused : null
|
||||
const diffOptions = useDiffOptions()
|
||||
|
||||
const lineAnnotations = useMemo<Array<DiffLineAnnotation<ReviewFinding>>>(
|
||||
() =>
|
||||
findings
|
||||
.filter((finding) => finding.end_line !== null)
|
||||
.map((finding) => ({
|
||||
side: findingSide(finding),
|
||||
lineNumber: finding.end_line as number,
|
||||
metadata: finding,
|
||||
})),
|
||||
[findings]
|
||||
)
|
||||
|
||||
const selectedLines =
|
||||
focused?.file === file.path && isAnchored(focused)
|
||||
? findingSelectedRange(focused)
|
||||
: null
|
||||
|
||||
return (
|
||||
<div
|
||||
ref={sectionRef}
|
||||
className="scroll-mt-4 overflow-hidden rounded-lg border border-border"
|
||||
className="scroll-mt-4 overflow-hidden rounded-lg border border-[var(--ui-border)]"
|
||||
>
|
||||
<div className="flex items-center gap-2 bg-muted/40 px-3 py-2 text-xs">
|
||||
<div className="flex items-center gap-2 bg-[var(--ui-panel-2)] px-3 py-2 text-xs">
|
||||
<button
|
||||
type="button"
|
||||
onClick={onToggleExpanded}
|
||||
|
|
@ -599,131 +599,59 @@ function FileDiffCard({
|
|||
</button>
|
||||
</label>
|
||||
</div>
|
||||
{expanded && (
|
||||
<div className="overflow-x-auto bg-card font-mono text-[11px] leading-5">
|
||||
{file.hunks.map((hunk, hunkIndex) => {
|
||||
const range =
|
||||
fileFocused !== null
|
||||
? highlightRange(hunk.lines, fileFocused)
|
||||
: null
|
||||
const anchorRows = new Map<number, Array<ReviewFinding>>()
|
||||
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 (
|
||||
<div key={hunkIndex}>
|
||||
<div className="bg-muted/60 px-3 py-1 text-muted-foreground">
|
||||
{hunk.header}
|
||||
</div>
|
||||
{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 (
|
||||
<DiffLineRow
|
||||
key={lineIndex}
|
||||
line={line}
|
||||
findings={lineFindings}
|
||||
anchorFindings={anchorFindings}
|
||||
highlight={highlight}
|
||||
onFindingClick={onFindingClick}
|
||||
anchorRef={anchorRef}
|
||||
/>
|
||||
)
|
||||
})}
|
||||
</div>
|
||||
)
|
||||
})}
|
||||
</div>
|
||||
)}
|
||||
{expanded &&
|
||||
(file.unrenderable ? (
|
||||
<div className="bg-[var(--ui-panel)] p-4 text-center text-xs text-[var(--ui-text-dim)]">
|
||||
Binary or large file — diff not shown.
|
||||
</div>
|
||||
) : (
|
||||
<div className="overflow-x-auto bg-[var(--ui-panel)] font-mono text-[11px] leading-5">
|
||||
<MultiFileDiff<ReviewFinding>
|
||||
oldFile={{ name: file.path, contents: file.originalContent }}
|
||||
newFile={{ name: file.path, contents: file.modifiedContent }}
|
||||
options={diffOptions}
|
||||
lineAnnotations={lineAnnotations}
|
||||
selectedLines={selectedLines}
|
||||
renderAnnotation={(annotation) => (
|
||||
<FindingRailMarker
|
||||
finding={annotation.metadata}
|
||||
onFindingClick={onFindingClick}
|
||||
anchorRef={anchorRef}
|
||||
/>
|
||||
)}
|
||||
/>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
function DiffLineRow({
|
||||
line,
|
||||
findings,
|
||||
anchorFindings,
|
||||
highlight,
|
||||
function FindingRailMarker({
|
||||
finding,
|
||||
onFindingClick,
|
||||
anchorRef,
|
||||
}: {
|
||||
line: ReviewDiffLine
|
||||
findings: Array<ReviewFinding>
|
||||
anchorFindings: Array<ReviewFinding>
|
||||
highlight: HighlightEdge
|
||||
finding: ReviewFinding
|
||||
onFindingClick: (finding: ReviewFinding) => void
|
||||
anchorRef: (id: string, node: HTMLDivElement | null) => void
|
||||
anchorRef: (id: string, node: HTMLElement | null) => void
|
||||
}) {
|
||||
const first = findings[0]
|
||||
const style = GROUP_STYLES[finding.group]
|
||||
const Icon = style.Icon
|
||||
return (
|
||||
<div
|
||||
ref={
|
||||
anchorFindings.length > 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"
|
||||
)}
|
||||
>
|
||||
<span className="w-10 shrink-0 px-1 text-right text-muted-foreground/60 select-none">
|
||||
{line.old_line ?? ""}
|
||||
</span>
|
||||
<span className="w-10 shrink-0 px-1 text-right text-muted-foreground/60 select-none">
|
||||
{line.new_line ?? ""}
|
||||
</span>
|
||||
<span
|
||||
<div className="flex justify-end px-2 py-0.5">
|
||||
<button
|
||||
ref={(node) => anchorRef(finding.id, node)}
|
||||
type="button"
|
||||
onClick={() => onFindingClick(finding)}
|
||||
aria-label={`Open finding: ${finding.title}`}
|
||||
className={cn(
|
||||
"w-4 shrink-0 text-center select-none",
|
||||
line.kind === "add" && "text-emerald-500",
|
||||
line.kind === "del" && "text-red-500"
|
||||
"inline-flex items-center gap-1 rounded border border-[var(--ui-border)] bg-[var(--ui-surface)] px-1.5 py-0.5 text-[10px]",
|
||||
style.className
|
||||
)}
|
||||
>
|
||||
{line.kind === "add" ? "+" : line.kind === "del" ? "-" : ""}
|
||||
</span>
|
||||
<span className="pr-3 whitespace-pre">{line.text}</span>
|
||||
{first && (
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => onFindingClick(first)}
|
||||
aria-label={`Open finding: ${first.title}`}
|
||||
className="mr-2 ml-auto shrink-0 self-center"
|
||||
>
|
||||
{(() => {
|
||||
const style = GROUP_STYLES[first.group]
|
||||
const Icon = style.Icon
|
||||
return <Icon className={cn("size-3.5", style.className)} />
|
||||
})()}
|
||||
</button>
|
||||
)}
|
||||
<span className="font-sans">{finding.title}</span>
|
||||
<Icon className="size-3 shrink-0" />
|
||||
</button>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
|
@ -746,7 +674,7 @@ function AnchoredFindingCard({
|
|||
}: {
|
||||
detail: ReviewDetail
|
||||
finding: ReviewFinding
|
||||
anchorEl: HTMLDivElement
|
||||
anchorEl: HTMLElement
|
||||
scrollRef: React.RefObject<HTMLDivElement | null>
|
||||
onClose: () => void
|
||||
}) {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue