feat: inline PR comments in the reviews UI (#1600)

* feat: inline PR comments in the reviews UI

Click the diff gutter "+" on a line to open an inline comment composer
(rendered like the finding card via a Pierre annotation); submitting
posts a real inline PR review comment as the signed-in user through a
new POST /reviews/{owner}/{repo}/{number}/comments. The "+" press-drag →
"Add to Chat" selection path is unchanged.

* feat: GitHub-parity comment box, PR comments dropdown, collapse nav

- Comment composer now mirrors GitHub's box: Write/Preview tabs (markdown
  rendered via the existing Markdown component) and a markdown toolbar
  (heading, bold, italic, quote, code, link, bulleted/numbered/task list).
- Surface other people's inline PR comments in a Devin-style dropdown in the
  review header (search + link to the thread on GitHub). New
  GET /reviews/{owner}/{repo}/{number}/comments lists them and flags the
  reviewer's own (marker-bearing) comments so they're filtered out.
- Collapse the global nav by default on a review detail page, restoring the
  prior preference on leave.

* feat: bigger comment-toolbar icons; open dropdown comments inline

- Enlarge the markdown toolbar glyphs (Phosphor) in the comment composer —
  they were rendering at 10px.
- Clicking a comment in the PR comments dropdown now opens it inline in the
  diff as a read-only finding-style card (InlineComment), scrolling its line
  into view, instead of navigating to GitHub. Falls back to GitHub when the
  comment's file/line isn't in the current diff.

* fix: drive "Add to Chat" from native text selection

The gutter "+" is now comment-only; wiring its click to the composer
conflicted with its old double-duty as the drag-to-select handle, which
broke selection → "Add to Chat". Switch to Devin's model: disable Pierre's
interactive line selection and instead map a native text highlight in the
diff to a line range (via the data-line / data-line-type attributes Pierre
stamps on each line, read from the diff's open shadow root) to show the
"Add to Chat" popup. ⌘L and the existing attachment/popup path are unchanged.

* feat: gutter "+" drag selects a range for multi-line comments

Re-enable Pierre's gutter line selection so dragging the "+" down the
gutter comments across a range (click still comments on a single line);
onLineSelectionEnd routes the range to the composer. Native code-text
selection still drives "Add to Chat" — Pierre only line-selects from the
gutter, and onLineSelectionEnd bails when a native text selection is
present, so a code highlight never opens the composer.

* fix: keep the range highlighted while its comment composer is open

Previously opening the composer cleared the selection, so the lines being
commented on lost their highlight. Drive the controlled selection from the
open comment draft's range so the rows stay highlighted until the composer
is closed.

* fix: address PR review — paginate comments, fall back for outdated ones

- list_review_comments now pages through all PR review comments (bounded by
  _MAX_REVIEW_COMMENT_PAGES) instead of returning only the first 100, so older
  comments still show in the dropdown.
- Surface GitHub's outdated flag (position == null) as is_outdated; opening such
  a comment (or one whose line isn't in the diff) now opens it on GitHub instead
  of silently rendering nothing, plus a timeout fallback if the annotation never
  mounts (e.g. collapsed context).
This commit is contained in:
Johannes du Plessis 2026-06-23 16:01:46 -07:00 • committed by GitHub
parent ca9280d25c
commit 9370a8c7f4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 1235 additions and 67 deletions

View file

@ -10,6 +10,7 @@ from __future__ import annotations
import ipaddress
import logging
import re
import socket
from collections.abc import Awaitable, Callable
from typing import Any, Literal
@ -55,6 +56,43 @@ async def _github_get(
return response.json()
def _github_error_message(response: httpx.Response) -> str:
"""Best-effort extraction of GitHub's error message for surfacing to the UI."""
fallback = f"GitHub request failed ({response.status_code})"
try:
data = response.json()
except ValueError:
return fallback
if not isinstance(data, dict):
return fallback
message = data.get("message")
message_str = message if isinstance(message, str) else ""
errors = data.get("errors")
detail_parts: list[str] = []
if isinstance(errors, list):
for err in errors:
if isinstance(err, dict) and isinstance(err.get("message"), str):
detail_parts.append(err["message"])
detail = "; ".join(detail_parts)
if message_str and detail:
return f"{message_str}: {detail}"
return message_str or detail or fallback
async def _github_post(path: str, token: str, *, json: dict[str, Any]) -> Any:
async with httpx.AsyncClient(timeout=_GITHUB_TIMEOUT) as client:
response = await client.post(
f"{_GITHUB_API}{path}", headers=github_headers(token), json=json
)
if response.status_code >= 400:
message = _github_error_message(response)
logger.warning("GitHub POST %s failed: %s %s", path, response.status_code, message)
# Pass 4xx through verbatim (422 = line not in diff, 403 = perms); collapse
# 5xx to a 502 so a GitHub outage doesn't masquerade as a client error.
raise HTTPException(response.status_code if response.status_code < 500 else 502, message)
return response.json()
def reviewer_thread_id(owner: str, repo: str, pr_number: int) -> str:
import uuid
@ -364,6 +402,114 @@ async def get_pr_head_sha(owner: str, repo: str, pr_number: int) -> str:
return sha if isinstance(sha, str) else ""
async def create_review_comment(
owner: str,
repo: str,
pr_number: int,
*,
token: str,
path: str,
line: int,
side: Literal["LEFT", "RIGHT"],
body: str,
start_line: int | None = None,
start_side: Literal["LEFT", "RIGHT"] | None = None,
) -> dict[str, Any]:
"""Post a single inline review comment to a PR using the caller's token.
Unlike the reviewer agent (which batches comments into one review via the App
token), this posts a standalone comment immediately, authored by the signed-in
user. ``commit_id`` is the PR's live head SHA. GitHub errors surface verbatim so
the UI can explain a 422 (line not part of the diff) or 403 (missing permission).
"""
head_sha = await get_pr_head_sha(owner, repo, pr_number)
if not head_sha:
raise HTTPException(502, "could not resolve PR head commit")
payload: dict[str, Any] = {
"body": body,
"commit_id": head_sha,
"path": path,
"line": line,
"side": side,
}
# GitHub forbids multi-line ranges that span sides; only add the range start
# when it is a distinct earlier line on the same side.
if start_line is not None and start_line != line:
payload["start_line"] = start_line
payload["start_side"] = start_side or side
return await _github_post(
f"/repos/{owner}/{repo}/pulls/{pr_number}/comments", token, json=payload
)
_HTML_COMMENT_RE = re.compile(r"<!--.*?-->", re.DOTALL)
# Inline comments the reviewer posts carry this hidden marker (see reviewer_publish).
_OPEN_SWE_COMMENT_RE = re.compile(r"<!--\s*open-swe-review-comment\b")
_REVIEW_COMMENTS_PER_PAGE = 100
# Bound the fetch so a pathological PR can't trigger unbounded paging (~2000 comments).
_MAX_REVIEW_COMMENT_PAGES = 20
def _clean_comment_body(body: str) -> str:
return _HTML_COMMENT_RE.sub("", body).strip()
def _normalize_review_comment(item: dict[str, Any]) -> dict[str, Any]:
body = item.get("body") if isinstance(item.get("body"), str) else ""
user = item.get("user") if isinstance(item.get("user"), dict) else {}
line = item.get("line")
if not isinstance(line, int):
original = item.get("original_line")
line = original if isinstance(original, int) else None
return {
"id": item.get("id"),
"author": user.get("login") if isinstance(user.get("login"), str) else "",
"author_avatar_url": (
user.get("avatar_url") if isinstance(user.get("avatar_url"), str) else ""
),
"path": item.get("path") if isinstance(item.get("path"), str) else "",
"line": line,
"side": item.get("side") if item.get("side") in ("LEFT", "RIGHT") else "RIGHT",
"body": _clean_comment_body(body),
"html_url": item.get("html_url") if isinstance(item.get("html_url"), str) else "",
"created_at": item.get("created_at") if isinstance(item.get("created_at"), str) else "",
"is_open_swe": bool(_OPEN_SWE_COMMENT_RE.search(body)),
# GitHub nulls `position` when the line no longer appears in the current
# diff — i.e. the comment is outdated and can't be rendered inline.
"is_outdated": not isinstance(item.get("position"), int),
}
async def list_review_comments(owner: str, repo: str, pr_number: int) -> dict[str, Any]:
"""List inline review comments on a PR (newest first), normalized for the UI.
Surfaces every inline comment on the PR — including humans' — not just the
reviewer's findings. ``is_open_swe`` flags the reviewer's own (marker-bearing)
comments so the UI can separate them from other people's. Pages through the
full list (bounded by ``_MAX_REVIEW_COMMENT_PAGES``) so older comments aren't
silently dropped.
"""
token = await _require_app_token()
comments: list[dict[str, Any]] = []
for page in range(1, _MAX_REVIEW_COMMENT_PAGES + 1):
raw = await _github_get(
f"/repos/{owner}/{repo}/pulls/{pr_number}/comments",
token,
params={
"per_page": _REVIEW_COMMENTS_PER_PAGE,
"page": page,
"sort": "created",
"direction": "desc",
},
)
if not isinstance(raw, list) or not raw:
break
comments.extend(_normalize_review_comment(item) for item in raw if isinstance(item, dict))
if len(raw) < _REVIEW_COMMENTS_PER_PAGE:
break
return {"comments": comments}
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()

View file

@ -5,7 +5,7 @@ from __future__ import annotations
import hmac
import logging
import os
from typing import Any
from typing import Any, Literal
import httpx
from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Request
@ -83,8 +83,10 @@ from .repo_snapshots import (
update_repo_snapshot,
)
from .review_api import (
create_review_comment,
get_review,
get_review_diff,
list_review_comments,
list_reviews,
proxy_pr_image,
trigger_re_review,
@ -1070,6 +1072,57 @@ async def api_re_review(
return await trigger_re_review(owner, repo, pr_number, session["sub"])
class ReviewCommentCreate(BaseModel):
path: str
line: int
side: Literal["LEFT", "RIGHT"]
body: str
start_line: int | None = None
start_side: Literal["LEFT", "RIGHT"] | None = None
@router.get("/reviews/{owner}/{repo}/{pr_number}/comments")
async def api_list_review_comments(
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 list_review_comments(owner, repo, pr_number)
@router.post("/reviews/{owner}/{repo}/{pr_number}/comments")
async def api_create_review_comment(
owner: str,
repo: str,
pr_number: int,
comment: ReviewCommentCreate,
session: dict[str, Any] = _SESSION_DEP,
) -> dict[str, Any]:
await require_repo_access_for_user(session["sub"], f"{owner}/{repo}")
body = comment.body.strip()
if not body:
raise HTTPException(422, "comment body is required")
# Post as the signed-in user (their user-to-server token), so the comment is
# attributed to them rather than the Open SWE app.
token = await get_valid_access_token(session["sub"])
if not token:
raise HTTPException(401, "GitHub re-auth required")
return await create_review_comment(
owner,
repo,
pr_number,
token=token,
path=comment.path,
line=comment.line,
side=comment.side,
body=body,
start_line=comment.start_line,
start_side=comment.start_side,
)
# --- PR chat (sandbox-less ``chat`` graph) -----------------------------------
# The frontend points a LangGraph StreamProvider at the base
# ``/reviews/{owner}/{repo}/{pr_number}/chat``; the SDK then issues the

View file

@ -0,0 +1,167 @@
import { useEffect, useMemo, useRef, useState } from "react"
import { useQuery } from "@tanstack/react-query"
import { ChatCircleIcon, MagnifyingGlassIcon } from "@phosphor-icons/react"
import type { PrReviewComment } from "@/lib/api"
import { api } from "@/lib/api"
import { cn } from "@/lib/utils"
function basename(path: string): string {
const idx = path.lastIndexOf("/")
return idx === -1 ? path : path.slice(idx + 1)
}
// Devin-style dropdown surfacing inline PR comments left by people (the
// reviewer's own findings already render inline + in the side panel, so they're
// filtered out). Each entry links to the comment thread on GitHub.
export function ReviewCommentsMenu({
owner,
repo,
number,
onSelect,
}: {
owner: string
repo: string
number: number
onSelect: (comment: PrReviewComment) => void
}) {
const [open, setOpen] = useState(false)
const [query, setQuery] = useState("")
const wrapperRef = useRef<HTMLDivElement | null>(null)
const comments = useQuery({
queryKey: ["reviewComments", owner, repo, number],
queryFn: () => api.listReviewComments(owner, repo, number),
enabled: Number.isFinite(number),
staleTime: 30_000,
})
const otherComments = useMemo(
() => (comments.data?.comments ?? []).filter((c) => !c.is_open_swe),
[comments.data]
)
const filtered = useMemo(() => {
const q = query.trim().toLowerCase()
if (!q) return otherComments
return otherComments.filter((c) =>
`${c.author} ${c.path} ${c.body}`.toLowerCase().includes(q)
)
}, [otherComments, query])
useEffect(() => {
if (!open) return
const onPointerDown = (event: PointerEvent) => {
if (
event.target instanceof Node &&
!wrapperRef.current?.contains(event.target)
) {
setOpen(false)
}
}
const onKeyDown = (event: KeyboardEvent) => {
if (event.key === "Escape") setOpen(false)
}
window.addEventListener("pointerdown", onPointerDown)
window.addEventListener("keydown", onKeyDown)
return () => {
window.removeEventListener("pointerdown", onPointerDown)
window.removeEventListener("keydown", onKeyDown)
}
}, [open])
const count = otherComments.length
return (
<div ref={wrapperRef} className="relative">
<button
type="button"
onClick={() => setOpen((value) => !value)}
aria-label="PR comments"
aria-expanded={open}
className={cn(
"inline-flex items-center gap-1.5 rounded-md border border-border px-2 py-1 text-xs text-muted-foreground hover:text-foreground",
open && "text-foreground"
)}
>
<ChatCircleIcon className="size-3.5" />
<span>Comments</span>
{count > 0 && (
<span className="rounded bg-muted px-1 text-[10px] font-medium text-foreground">
{count}
</span>
)}
</button>
{open && (
<div className="absolute top-full right-0 z-50 mt-1 w-96 overflow-hidden rounded-md border border-border bg-popover text-popover-foreground shadow-md">
<div className="flex items-center gap-1.5 border-b border-border px-2 py-1.5">
<MagnifyingGlassIcon className="size-3.5 shrink-0 text-muted-foreground" />
<input
value={query}
onChange={(event) => setQuery(event.target.value)}
placeholder="Search comments"
className="w-full bg-transparent text-xs outline-none placeholder:text-muted-foreground"
/>
</div>
<div className="max-h-96 overflow-y-auto">
{comments.isLoading ? (
<p className="px-3 py-4 text-center text-xs text-muted-foreground">
Loading…
</p>
) : comments.isError ? (
<p className="px-3 py-4 text-center text-xs text-destructive">
Failed to load comments
</p>
) : filtered.length === 0 ? (
<p className="px-3 py-4 text-center text-xs text-muted-foreground">
{otherComments.length === 0
? "No comments yet"
: "No matching comments"}
</p>
) : (
<ul className="divide-y divide-border">
{filtered.map((comment) => (
<li key={comment.id}>
<button
type="button"
onClick={() => {
onSelect(comment)
setOpen(false)
}}
className="flex w-full gap-2 px-3 py-2 text-left hover:bg-muted/50"
>
{comment.author_avatar_url ? (
<img
src={comment.author_avatar_url}
alt=""
className="mt-0.5 size-4 shrink-0 rounded-full"
/>
) : (
<span className="mt-0.5 size-4 shrink-0 rounded-full bg-muted" />
)}
<div className="min-w-0 flex-1">
<div className="flex items-center gap-1.5 text-[11px]">
<span className="font-medium text-foreground">
{comment.author}
</span>
{comment.path && (
<span className="truncate font-mono text-muted-foreground">
{basename(comment.path)}
{comment.line !== null ? `:${comment.line}` : ""}
</span>
)}
</div>
<p className="mt-0.5 line-clamp-2 text-xs text-muted-foreground">
{comment.body}
</p>
</div>
</button>
</li>
))}
</ul>
)}
</div>
</div>
)}
</div>
)
}

File diff suppressed because it is too large Load diff

View file

@ -86,6 +86,12 @@ export function useSidebarCollapsed(): boolean {
return useContext(SidebarLayoutContext)?.collapsed ?? false;
}
// Full sidebar controls (collapse/expand) for pages that want to drive the nav,
// e.g. the review detail page collapsing it by default. Null outside the provider.
export function useSidebarControls(): SidebarLayout | null {
return useContext(SidebarLayoutContext);
}
interface SidebarFrameProps {
width: number;
setWidth: (next: number) => void;

View file

@ -388,6 +388,39 @@ export interface ReviewFinding {
group: FindingGroup
}
export interface ReviewCommentCreate {
path: string
line: number
side: "LEFT" | "RIGHT"
body: string
start_line?: number | null
start_side?: "LEFT" | "RIGHT" | null
}
export interface ReviewCommentResult {
id: number
html_url: string
}
export interface PrReviewComment {
id: number
author: string
author_avatar_url: string
path: string
line: number | null
side: "LEFT" | "RIGHT"
body: string
html_url: string
created_at: string
is_open_swe: boolean
// Outdated: the line no longer appears in the current diff, so it can't render inline.
is_outdated: boolean
}
export interface ReviewCommentsPayload {
comments: Array<PrReviewComment>
}
export interface ReviewCounts {
open: number
resolved: number
@ -742,6 +775,20 @@ export const api = {
`/reviews/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/${number}/re-review`,
{ method: "POST" }
),
createReviewComment: (
owner: string,
repo: string,
number: number,
body: ReviewCommentCreate
) =>
request<ReviewCommentResult>(
`/reviews/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/${number}/comments`,
{ method: "POST", body: JSON.stringify(body) }
),
listReviewComments: (owner: string, repo: string, number: number) =>
request<ReviewCommentsPayload>(
`/reviews/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/${number}/comments`
),
getReviewerEval: () => request<ReviewerEvalStatus>("/admin/evals/reviewer"),
logout: () => request<void>("/auth/logout", { method: "POST" }),
}

View file

@ -1,10 +1,12 @@
import { Link, Navigate, createFileRoute } from "@tanstack/react-router"
import { useQuery, useQueryClient } from "@tanstack/react-query"
import { useEffect, useRef } from "react"
import { useCallback, useEffect, useRef, useState } from "react"
import { ArrowLeftIcon, GitPullRequestIcon } from "@phosphor-icons/react"
import type { PrReviewComment } from "@/lib/api"
import { ReviewCommentsMenu } from "@/components/agents/ReviewCommentsMenu"
import { ReviewMainBody } from "@/components/agents/ReviewMainBody"
import { useSidebarCollapsed } from "@/components/sidebar-layout"
import { useSidebarControls } from "@/components/sidebar-layout"
import { Skeleton } from "@/components/ui/skeleton"
import { api } from "@/lib/api"
import { useSession } from "@/lib/session"
@ -18,7 +20,24 @@ function ReviewDetailPage() {
const { owner, repo, number } = Route.useParams()
const prNumber = Number(number)
const session = useSession()
const sidebarCollapsed = useSidebarCollapsed()
const sidebar = useSidebarControls()
const sidebarCollapsed = sidebar?.collapsed ?? false
// A comment picked from the dropdown, shown inline in the diff (not GitHub).
const [activeComment, setActiveComment] = useState<PrReviewComment | null>(
null
)
const closeActiveComment = useCallback(() => setActiveComment(null), [])
// Collapse the global nav by default while viewing a review (roomy diff),
// restoring the prior preference on leave. Runs once for the page's lifetime.
const sidebarRef = useRef(sidebar)
sidebarRef.current = sidebar
useEffect(() => {
const controls = sidebarRef.current
if (!controls || controls.collapsed) return
controls.setCollapsed(true)
return () => controls.setCollapsed(false)
}, [])
const detail = useQuery({
queryKey: ["review", owner, repo, prNumber],
queryFn: () => api.getReview(owner, repo, prNumber),
@ -80,6 +99,16 @@ function ReviewDetailPage() {
{detail.data ? ` ${detail.data.pr.title}` : ""}
</span>
</span>
{Number.isFinite(prNumber) && (
<div className="ml-auto shrink-0">
<ReviewCommentsMenu
owner={owner}
repo={repo}
number={prNumber}
onSelect={setActiveComment}
/>
</div>
)}
</header>
{detail.error ? (
@ -96,6 +125,8 @@ function ReviewDetailPage() {
key={detail.data.head_sha}
detail={detail.data}
diffFiles={diff.data?.files ?? null}
openComment={activeComment}
onCloseOpenComment={closeActiveComment}
/>
)}
</div>