diff --git a/agent/dashboard/review_api.py b/agent/dashboard/review_api.py index a6672961..fb51fd70 100644 --- a/agent/dashboard/review_api.py +++ b/agent/dashboard/review_api.py @@ -170,6 +170,7 @@ def _thread_review_summary(thread: dict[str, Any]) -> dict[str, Any] | None: "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 "", + "author": pr.get("author") if isinstance(pr.get("author"), str) else "", "head_sha": metadata.get("head_sha") or "", "watch": bool(metadata.get("watch")), "status": _run_status(thread, metadata), @@ -179,27 +180,35 @@ def _thread_review_summary(thread: dict[str, Any]) -> dict[str, Any] | None: async def list_reviews( - limit: int = 100, + limit: int = 20, *, + offset: int = 0, + author: str | None = None, is_accessible: Callable[[dict[str, Any]], Awaitable[bool]] | None = None, page_size: int = 100, max_scan: int = 1000, -) -> list[dict[str, Any]]: +) -> tuple[list[dict[str, Any]], bool]: """List review summaries, newest first. + Returns ``(summaries, has_more)`` where the summaries are the page at + ``offset`` (counted in accessible, filter-matching records) and + ``has_more`` says whether at least one more record exists past it. + 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. + until enough 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. ``author`` filters on the PR author's + GitHub login stored in thread metadata. """ client = langgraph_client() + needed = offset + limit + 1 summaries: list[dict[str, Any]] = [] - offset = 0 - while len(summaries) < limit and offset < max_scan: + scan_offset = 0 + while len(summaries) < needed and scan_offset < max_scan: threads = await client.threads.search( metadata={"kind": REVIEWER_THREAD_KIND}, limit=page_size, - offset=offset, + offset=scan_offset, sort_by="updated_at", sort_order="desc", ) @@ -211,15 +220,18 @@ async def list_reviews( summary = _thread_review_summary(thread) if not summary: continue + if author is not None and summary["author"] != author: + continue if is_accessible is not None and not await is_accessible(summary): continue summaries.append(summary) - if len(summaries) >= limit: + if len(summaries) >= needed: break if len(threads) < page_size: break - offset += page_size - return summaries + scan_offset += page_size + page = summaries[offset : offset + limit] + return page, len(summaries) > offset + limit def _user_ref(value: Any) -> dict[str, Any] | None: diff --git a/agent/dashboard/routes.py b/agent/dashboard/routes.py index 423ddcce..4369218d 100644 --- a/agent/dashboard/routes.py +++ b/agent/dashboard/routes.py @@ -709,10 +709,15 @@ async def api_list_review_styles( return out +REVIEWS_PAGE_SIZE = 20 + + @router.get("/reviews") async def api_list_reviews( + page: int = 0, + mine: bool = True, session: dict[str, Any] = _SESSION_DEP, -) -> list[dict[str, Any]]: +) -> dict[str, Any]: login = session["sub"] access_cache: dict[str, bool] = {} @@ -728,7 +733,14 @@ async def api_list_reviews( access_cache[full_name] = False return access_cache[full_name] - return await list_reviews(is_accessible=is_accessible) + page = max(page, 0) + reviews, has_more = await list_reviews( + REVIEWS_PAGE_SIZE, + offset=page * REVIEWS_PAGE_SIZE, + author=login if mine else None, + is_accessible=is_accessible, + ) + return {"reviews": reviews, "page": page, "has_more": has_more} @router.get("/reviews/{owner}/{repo}/{pr_number}") diff --git a/agent/reviewer_findings.py b/agent/reviewer_findings.py index d8cfa59b..7adc23b2 100644 --- a/agent/reviewer_findings.py +++ b/agent/reviewer_findings.py @@ -171,6 +171,7 @@ class ReviewerPRMeta(TypedDict, total=False): title: str head_ref: str base_ref: str + author: str class ReviewerSlackThread(TypedDict, total=False): diff --git a/agent/webapp.py b/agent/webapp.py index be42640a..4fe23385 100644 --- a/agent/webapp.py +++ b/agent/webapp.py @@ -1865,6 +1865,7 @@ async def trigger_pr_review_from_ref( "title": pr_title, "head_ref": branch_name, "base_ref": base_ref, + "author": (pr_metadata.get("user") or {}).get("login", ""), } slack_thread_meta: ReviewerSlackThread | None = None if slack_channel_id and slack_thread_ts: @@ -2020,6 +2021,7 @@ async def _dispatch_first_review_from_pr_payload(payload: dict[str, Any], *, sou "title": pr_title, "head_ref": branch_name, "base_ref": base_ref, + "author": (pull_request.get("user") or {}).get("login", ""), } last_reviewed_sha = "" if payload.get("action") == "ready_for_review": @@ -2496,6 +2498,7 @@ async def process_github_push_event(payload: dict[str, Any]) -> None: "title": pr_title, "head_ref": head_ref, "base_ref": base_ref, + "author": (pr.get("user") or {}).get("login", ""), } await set_reviewer_thread_metadata(thread_id, pr=pr_meta, watch=True, head_sha=head_sha) diff --git a/ui/src/components/agents/ReviewSidebar.tsx b/ui/src/components/agents/ReviewSidebar.tsx index d04f37bb..db89e669 100644 --- a/ui/src/components/agents/ReviewSidebar.tsx +++ b/ui/src/components/agents/ReviewSidebar.tsx @@ -122,7 +122,9 @@ function ReviewFileTreeExplorer({ { height: "100%", ...treeThemeStyle(), - "--trees-theme-sidebar-bg": "transparent", + // Must stay opaque: the tree's truncation marker ("…") paints + // this color behind itself to hide the overflowing filename. + "--trees-theme-sidebar-bg": "var(--ui-sidebar)", } as React.CSSProperties } /> diff --git a/ui/src/lib/api.ts b/ui/src/lib/api.ts index 33d6c6f0..003c6d69 100644 --- a/ui/src/lib/api.ts +++ b/ui/src/lib/api.ts @@ -311,6 +311,7 @@ export interface ReviewSummary { url: string; head_ref: string; base_ref: string; + author: string; head_sha: string; watch: boolean; status: "running" | "error" | "idle"; @@ -319,6 +320,12 @@ export interface ReviewSummary { full_name?: string; } +export interface ReviewListPayload { + reviews: Array; + page: number; + has_more: boolean; +} + export interface ReviewUserRef { login: string; avatar_url?: string | null; @@ -470,7 +477,8 @@ export const api = { `/admin/user-mappings/${encodeURIComponent(github_login)}`, { method: "DELETE" }, ), - listReviews: () => request>("/reviews"), + listReviews: (page: number, mine: boolean) => + request(`/reviews?page=${page}&mine=${mine}`), getReview: (owner: string, repo: string, number: number) => request( `/reviews/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/${number}`, diff --git a/ui/src/routes/agents/reviews/$owner.$repo.$number.tsx b/ui/src/routes/agents/reviews/$owner.$repo.$number.tsx index 41c34a6d..2eeb32a7 100644 --- a/ui/src/routes/agents/reviews/$owner.$repo.$number.tsx +++ b/ui/src/routes/agents/reviews/$owner.$repo.$number.tsx @@ -1,6 +1,13 @@ 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 { + useCallback, + useEffect, + useLayoutEffect, + useMemo, + useRef, + useState, +} from "react" import { ArrowClockwiseIcon, ArrowLeftIcon, @@ -213,9 +220,8 @@ function ReviewBody({ {} ) const [focused, setFocused] = useState(null) - const [cardPos, setCardPos] = useState<{ top: number; left: number | null }>( - { top: 96, left: null } - ) + const [anchorEl, setAnchorEl] = useState(null) + const scrollRef = useRef(null) const viewedStorageKey = `open-swe.review.viewed.${detail.owner}/${detail.repo}/${detail.number}.${detail.head_sha}` const [viewed, setViewed] = useState>(() => { @@ -307,7 +313,7 @@ function ReviewBody({ markRead(finding.id) setFocused(finding) if (!isAnchored(finding)) { - setCardPos({ top: 96, left: null }) + setAnchorEl(null) return } setExpandedFiles((prev) => ({ ...prev, [finding.file]: true })) @@ -315,20 +321,7 @@ function ReviewBody({ 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 }) - } - }) + setAnchorEl(anchorRefs.current[finding.id] ?? null) }) }, [markRead] @@ -353,13 +346,26 @@ function ReviewBody({ const onKeyDown = (event: KeyboardEvent) => { if (event.key === "Escape") setFocused(null) } + const onPointerDown = (event: PointerEvent) => { + const target = event.target + if (target instanceof Element && target.closest("[data-finding-card]")) + return + setFocused(null) + } window.addEventListener("keydown", onKeyDown) - return () => window.removeEventListener("keydown", onKeyDown) + window.addEventListener("pointerdown", onPointerDown) + return () => { + window.removeEventListener("keydown", onKeyDown) + window.removeEventListener("pointerdown", onPointerDown) + } }, [focused]) return ( -
-
+
+
@@ -398,18 +404,27 @@ function ReviewBody({ findings={findingsByFile.get(file.path) ?? []} focused={focused} viewed={viewed.has(file.path)} - onToggleViewed={() => toggleViewed(file.path)} + onToggleViewed={() => { + const collapses = + !viewed.has(file.path) && + expandedFiles[file.path] === undefined + if (collapses && focused?.file === file.path) + setFocused(null) + toggleViewed(file.path) + }} expanded={ expandedFiles[file.path] ?? !viewed.has(file.path) } - onToggleExpanded={() => + onToggleExpanded={() => { + const next = !( + expandedFiles[file.path] ?? !viewed.has(file.path) + ) + if (!next && focused?.file === file.path) setFocused(null) setExpandedFiles((prev) => ({ ...prev, - [file.path]: !( - prev[file.path] ?? !viewed.has(file.path) - ), + [file.path]: next, })) - } + }} onFindingClick={openFinding} sectionRef={(node) => { fileRefs.current[file.path] = node @@ -435,15 +450,30 @@ function ReviewBody({ onFindingClick={openFinding} /> - {focused && ( - - )} + {focused && + (isAnchored(focused) && anchorEl ? ( + + ) : ( +
+ +
+ ))}
) } @@ -698,17 +728,83 @@ function DiffLineRow({ ) } -function FindingFloatingCard({ +const FINDING_CARD_WIDTH = 412 +const FINDING_CARD_GAP = 12 + +const FINDING_CARD_CLASS = + "flex max-h-[70vh] w-[412px] flex-col overflow-hidden rounded-lg border border-border bg-background shadow-2xl" + +// Positioned absolutely inside the scroll container so it scrolls natively +// with the diff — no per-scroll JS repositioning, hence no lag. Position is +// recomputed only when layout shifts (files expand/collapse, resize). +function AnchoredFindingCard({ + detail, + finding, + anchorEl, + scrollRef, + onClose, +}: { + detail: ReviewDetail + finding: ReviewFinding + anchorEl: HTMLDivElement + scrollRef: React.RefObject + onClose: () => void +}) { + const cardRef = useRef(null) + + useLayoutEffect(() => { + const card = cardRef.current + const scroller = scrollRef.current + if (!card || !scroller) return + + const position = () => { + if (!anchorEl.isConnected) return + const scrollerRect = scroller.getBoundingClientRect() + const anchorRect = anchorEl.getBoundingClientRect() + const top = anchorRect.top - scrollerRect.top + scroller.scrollTop + const left = Math.max( + FINDING_CARD_GAP, + Math.min( + anchorRect.right - + scrollerRect.left + + scroller.scrollLeft + + FINDING_CARD_GAP, + scroller.clientWidth - FINDING_CARD_WIDTH - FINDING_CARD_GAP + ) + ) + card.style.top = `${top}px` + card.style.left = `${left}px` + } + + position() + const observer = new ResizeObserver(position) + observer.observe(scroller) + for (const child of scroller.children) { + if (child !== card) observer.observe(child) + } + return () => observer.disconnect() + }, [anchorEl, scrollRef]) + + return ( +
+ +
+ ) +} + +function FindingCardContent({ detail, finding, - top, - left, onClose, }: { detail: ReviewDetail finding: ReviewFinding - top: number - left: number | null onClose: () => void }) { const [copied, setCopied] = useState(false) @@ -737,15 +833,7 @@ function FindingFloatingCard({ } return ( -
+ <>
@@ -797,7 +885,7 @@ function FindingFloatingCard({ )}
-
+ ) } @@ -844,7 +932,7 @@ function SidePanel({ return (