feat: split view, add-to-chat, virtualization + scroll/grouping perf (#1574)

* feat: reviews page split view, add-to-chat, virtualization + perf

- Virtualize the diff (Pierre Virtualizer + worker pool), mirroring the agent
  chat panel, so large PRs window rows instead of materializing every line.
- Split chat and diff into independent scroll containers and make chat
  auto-scroll fully contained, so typing/streaming no longer moves the diff.
- Memoize FileDiffCard with stable callbacks so focusing a finding re-renders
  only the affected card.
- Add a persisted unified/split diff toggle.
- Add highlight-to-chat: select lines (drag / shift-click) + gutter "+" to drop
  a file:line snippet into the chat composer.
- Rebuild sidebar group rows: whole card scrolls to the group (incl. the
  expanded explanation), Read explanation stays a separate toggle, memoized.

* feat: add-to-chat uses attachment pills + selection popup / ⌘L

Replace the raw-snippet injection with a Cursor-style flow:
- Selecting lines shows a floating "Add to Chat ⌘L" popup at the pointer; ⌘L
  adds the current selection without it. Removes the auto-adding gutter "+".
- "Add to chat" now creates a removable attachment pill in the composer (and a
  pill in the sent message bubble) instead of pasting raw text. The code is
  still serialized into the message content so the model receives it as context.

* feat: restore gutter + drag-handle for line selection

Re-enable Pierre's gutter '+' as a click-and-drag line selector (with the
highlight growing as you drag) — the affordance that was lost when the
auto-adding gutter button was removed. It no longer auto-adds: the commit flows
through onLineSelected to the 'Add to Chat' popup / ⌘L.

* fix: live selection highlight while dragging + reposition Add to Chat popup

- Feed onLineSelectionChange into the controlled selection so rows highlight
  live as you drag, not just on release (Pierre only paints the controlled
  selection when the prop updates). Popup now fires on onLineSelectionEnd.
- Anchor the popup's bottom-left to the drag handle (drop horizontal centering)
  so it no longer overlaps the '+' button.

* fix: anchor Add to Chat popup to gutter handle + click-away to unselect

- Position the popup from the gutter '+' handle's rect (in the diff shadow DOM,
  placed on the selection's bottom line) instead of the pointer-release point,
  which landed inconsistently. Falls back to the pointer if not found.
- Clear the line selection (and popup) on any outside pointer-down.

* fix: sidebar-collapse header overlap + PR review comments

- Lift useSidebarLayout to AgentsShell (single source), share collapsed via
  context, and pad the reviews header left when the sidebar is collapsed so the
  fixed collapse toggle no longer overlaps the header content.
- add-to-chat: collect each diff side separately so a selection that spans a
  deletion->addition no longer pastes wrong-file lines (PR comment).
- chat: clear attachments after sending via a suggested prompt, so an attached
  snippet isn't silently resent on the next message (PR comment).

* fix: anchored finding card positions to the right of the diff again

The virtualization refactor moved the card inside the main-width Virtualizer
scroller, so it clamped over the diff. Render it in the outer container as a
viewport-fixed card clamped to window width (right gutter / over the side panel,
like prod) and track the finding as the diff scrolls (rAF-throttled), hiding
when the finding scrolls out of view.
This commit is contained in:
Johannes du Plessis 2026-06-18 16:54:07 -07:00 • committed by GitHub
parent 9db7eab134
commit ecf0898f51
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 1090 additions and 302 deletions

View file

@ -17,12 +17,7 @@ import {
GitPullRequestIcon, GitPullRequestIcon,
SidebarSimpleIcon, SidebarSimpleIcon,
} from "@phosphor-icons/react" } from "@phosphor-icons/react"
import type { import type { FileContents } from "@pierre/diffs/react"
FileContents,
VirtualFileMetrics,
WorkerInitializationRenderOptions,
WorkerPoolOptions,
} from "@pierre/diffs/react"
import type { GitStatus, GitStatusEntry } from "@pierre/trees" import type { GitStatus, GitStatusEntry } from "@pierre/trees"
import type { AgentThread, Message } from "@/lib/agents/types" import type { AgentThread, Message } from "@/lib/agents/types"
@ -30,7 +25,14 @@ import type { ThreadPrDiffFile } from "@/lib/agents/api"
import type { ChangedFileSummaryItem } from "@/components/agents/messages" import type { ChangedFileSummaryItem } from "@/components/agents/messages"
import { useAgentThreadPrDiff } from "@/lib/agents/queries" import { useAgentThreadPrDiff } from "@/lib/agents/queries"
import { buttonVariants } from "@/components/ui/button" import { buttonVariants } from "@/components/ui/button"
import { useDiffOptions } from "@/components/agents/utils/diffUtils" import {
DIFF_VIRTUALIZER_CONFIG,
DIFF_VIRTUAL_METRICS,
DIFF_WORKER_HIGHLIGHTER_OPTIONS,
DIFF_WORKER_POOL_OPTIONS,
fileContentsCacheKey,
useDiffOptions,
} from "@/components/agents/utils/diffUtils"
import { summarizeChangedFiles } from "@/components/agents/ported" import { summarizeChangedFiles } from "@/components/agents/ported"
import { Z } from "@/components/agents/z-index" import { Z } from "@/components/agents/z-index"
import { useIsMobile } from "@/lib/useIsMobile" import { useIsMobile } from "@/lib/useIsMobile"
@ -93,38 +95,6 @@ const PANEL_MIN_WIDTH = 320
// Exported so the chat column can enforce the same floor via min-width. // Exported so the chat column can enforce the same floor via min-width.
export const PANEL_MIN_CHAT_WIDTH = 360 export const PANEL_MIN_CHAT_WIDTH = 360
const DIFF_VIRTUALIZER_CONFIG = {
overscrollSize: 1200,
intersectionObserverMargin: 4800,
}
const DIFF_VIRTUAL_METRICS = {
hunkLineCount: 80,
lineHeight: 18,
diffHeaderHeight: 0,
spacing: 8,
} satisfies Partial<VirtualFileMetrics>
const DIFF_WORKER_POOL_OPTIONS = {
workerFactory: () =>
new Worker(
new URL("@pierre/diffs/worker/worker-portable.js", import.meta.url),
{
type: "module",
}
),
poolSize: 2,
totalASTLRUCacheSize: 120,
} satisfies WorkerPoolOptions
const DIFF_WORKER_HIGHLIGHTER_OPTIONS = {
theme: { light: "pierre-light", dark: "pierre-dark" },
lineDiffType: "word-alt",
maxLineDiffLength: 800,
tokenizeMaxLineLength: 1200,
langs: ["text"],
} satisfies WorkerInitializationRenderOptions
function getPanelMaxWidth(availableWidth?: number): number { function getPanelMaxWidth(availableWidth?: number): number {
if (typeof window === "undefined") return PANEL_DEFAULT_WIDTH if (typeof window === "undefined") return PANEL_DEFAULT_WIDTH
const available = availableWidth ?? window.innerWidth const available = availableWidth ?? window.innerWidth
@ -639,23 +609,6 @@ export function AgentGitPanel({ thread, messages }: AgentGitPanelProps) {
) )
} }
function hashFileContents(contents: string): string {
let hash = 0x811c9dc5
for (let i = 0; i < contents.length; i++) {
hash ^= contents.charCodeAt(i)
hash = Math.imul(hash, 0x01000193)
}
return (hash >>> 0).toString(36)
}
function fileContentsCacheKey(
path: string,
side: "old" | "new",
contents: string
): string {
return `${path}:${side}:${contents.length}:${hashFileContents(contents)}`
}
const FileDiffSection = memo( const FileDiffSection = memo(
function FileDiffSection({ function FileDiffSection({
file, file,

View file

@ -25,6 +25,7 @@ import type { ComponentType, SVGProps } from "react"
import type { SessionUser } from "@/lib/api" import type { SessionUser } from "@/lib/api"
import type { AgentSource, AgentThread } from "@/lib/agents/types" import type { AgentSource, AgentThread } from "@/lib/agents/types"
import type { SidebarLayout } from "@/components/sidebar-layout"
import { SidebarUserMenu } from "@/components/SidebarUserMenu" import { SidebarUserMenu } from "@/components/SidebarUserMenu"
import { import {
ReviewSidebarPanel, ReviewSidebarPanel,
@ -35,6 +36,7 @@ import { Button } from "@/components/ui/button"
import { import {
SidebarCollapseButton, SidebarCollapseButton,
SidebarFrame, SidebarFrame,
SidebarLayoutProvider,
useSidebarLayout, useSidebarLayout,
} from "@/components/sidebar-layout" } from "@/components/sidebar-layout"
import { groupThreads } from "@/lib/agents/api" import { groupThreads } from "@/lib/agents/api"
@ -90,6 +92,7 @@ const PR_STATE_META: Record<
interface AgentsSidebarProps { interface AgentsSidebarProps {
user: SessionUser user: SessionUser
activeThreadId?: string activeThreadId?: string
layout: SidebarLayout
} }
const NAV = [ const NAV = [
@ -98,7 +101,11 @@ const NAV = [
{ to: "/agents/reviews", label: "Reviews", icon: GitPullRequestIcon }, { to: "/agents/reviews", label: "Reviews", icon: GitPullRequestIcon },
] as const ] as const
export function AgentsSidebar({ user, activeThreadId }: AgentsSidebarProps) { export function AgentsSidebar({
user,
activeThreadId,
layout,
}: AgentsSidebarProps) {
const sidebar = useSidebarThreads(RESOLVED_SIDEBAR_LIMIT) const sidebar = useSidebarThreads(RESOLVED_SIDEBAR_LIMIT)
const activeThreads = sidebar.data?.active.items ?? [] const activeThreads = sidebar.data?.active.items ?? []
const resolvedThreads = sidebar.data?.resolved.items ?? [] const resolvedThreads = sidebar.data?.resolved.items ?? []
@ -107,7 +114,6 @@ export function AgentsSidebar({ user, activeThreadId }: AgentsSidebarProps) {
useSeedAgentThreadDetails(visibleThreads, activeThreadId) useSeedAgentThreadDetails(visibleThreads, activeThreadId)
useRunCompletionNotifier(visibleThreads, activeThreadId) useRunCompletionNotifier(visibleThreads, activeThreadId)
const groups = groupThreads(activeThreads) const groups = groupThreads(activeThreads)
const layout = useSidebarLayout()
const reviewSidebar = useReviewSidebarData() const reviewSidebar = useReviewSidebarData()
return ( return (
@ -538,12 +544,19 @@ export function AgentsShell({
activeThreadId?: string activeThreadId?: string
children: React.ReactNode children: React.ReactNode
}) { }) {
const layout = useSidebarLayout()
return ( return (
<ReviewSidebarProvider> <ReviewSidebarProvider>
<SidebarLayoutProvider value={layout}>
<div className="agents-ui flex h-svh overflow-hidden bg-[var(--ui-bg)]"> <div className="agents-ui flex h-svh overflow-hidden bg-[var(--ui-bg)]">
<AgentsSidebar user={user} activeThreadId={activeThreadId} /> <AgentsSidebar
user={user}
activeThreadId={activeThreadId}
layout={layout}
/>
<div className="flex min-w-0 flex-1">{children}</div> <div className="flex min-w-0 flex-1">{children}</div>
</div> </div>
</SidebarLayoutProvider>
</ReviewSidebarProvider> </ReviewSidebarProvider>
) )
} }

View file

@ -1,4 +1,13 @@
import { useCallback, useEffect, useMemo, useRef, useState } from "react" import {
createContext,
useCallback,
useContext,
useEffect,
useLayoutEffect,
useMemo,
useRef,
useState,
} from "react"
import { StreamProvider, useStreamContext } from "@langchain/react" import { StreamProvider, useStreamContext } from "@langchain/react"
import { overrideFetchImplementation } from "@langchain/langgraph-sdk" import { overrideFetchImplementation } from "@langchain/langgraph-sdk"
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query" import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"
@ -6,6 +15,7 @@ import {
ArrowClockwiseIcon, ArrowClockwiseIcon,
ArrowUpIcon, ArrowUpIcon,
CheckIcon, CheckIcon,
CodeIcon,
PlusIcon, PlusIcon,
SparkleIcon, SparkleIcon,
TrashIcon, TrashIcon,
@ -22,6 +32,138 @@ import { Skeleton } from "@/components/ui/skeleton"
import { api, reviewChatApiBase } from "@/lib/api" import { api, reviewChatApiBase } from "@/lib/api"
import { cn } from "@/lib/utils" import { cn } from "@/lib/utils"
// --- Composer bridge ---------------------------------------------------------
//
// Lets the diff column (a separate subtree) attach a code reference to the
// active chat composer. The reference shows as a removable pill; the code is
// only serialized into the message text on send. The chat tab may not be
// mounted when "add to chat" fires, so attachments are stashed until ChatBody
// registers its sink.
export interface ChatAttachment {
id: string
path: string
// e.g. "R35-37" / "L12" — the side+line range shown after the filename.
lineLabel: string
language: string
snippet: string
}
interface ReviewChatComposer {
addAttachment: (attachment: ChatAttachment) => void
registerSink: (fn: ((attachment: ChatAttachment) => void) | null) => void
}
const ReviewChatComposerContext = createContext<ReviewChatComposer | null>(null)
export function ReviewChatComposerProvider({
children,
}: {
children: React.ReactNode
}) {
const sinkRef = useRef<((attachment: ChatAttachment) => void) | null>(null)
const pendingRef = useRef<Array<ChatAttachment>>([])
const value = useMemo<ReviewChatComposer>(
() => ({
addAttachment: (attachment) => {
if (sinkRef.current) sinkRef.current(attachment)
else pendingRef.current.push(attachment)
},
registerSink: (fn) => {
sinkRef.current = fn
if (fn && pendingRef.current.length > 0) {
for (const attachment of pendingRef.current) fn(attachment)
pendingRef.current = []
}
},
}),
[]
)
return (
<ReviewChatComposerContext.Provider value={value}>
{children}
</ReviewChatComposerContext.Provider>
)
}
export function useReviewChatComposer(): ReviewChatComposer | null {
return useContext(ReviewChatComposerContext)
}
function attachmentBasename(path: string): string {
const idx = path.lastIndexOf("/")
return idx === -1 ? path : path.slice(idx + 1)
}
function attachmentPillLabel(attachment: ChatAttachment): string {
return `${attachmentBasename(attachment.path)}:${attachment.lineLabel}`
}
// Serialize attachments as fenced code blocks ahead of the prose so the model
// receives the code as context. The UI renders pills instead (see parse below).
function serializeMessage(text: string, attachments: Array<ChatAttachment>): string {
const blocks = attachments.map(
(a) => `\`${a.path}:${a.lineLabel}\`\n\`\`\`${a.language}\n${a.snippet}\n\`\`\``
)
return [...blocks, text.trim()].filter(Boolean).join("\n\n")
}
interface ParsedAttachment {
label: string
language: string
code: string
}
// Pull the leading attachment blocks (which serializeMessage always writes
// first) back out of a sent message so the bubble can render them as pills.
function parseUserMessage(content: string): {
attachments: Array<ParsedAttachment>
text: string
} {
const attachments: Array<ParsedAttachment> = []
let rest = content
const re = /^`([^`\n]+)`\n```([\w.-]*)\n([\s\S]*?)\n```\n*/
let match = re.exec(rest)
while (match && match.index === 0) {
const loc = match[1] ?? ""
const slash = loc.lastIndexOf("/")
attachments.push({
label: slash === -1 ? loc : loc.slice(slash + 1),
language: match[2] ?? "",
code: match[3] ?? "",
})
rest = rest.slice(match[0].length)
match = re.exec(rest)
}
return { attachments, text: rest.trim() }
}
function AttachmentPill({
label,
onRemove,
}: {
label: string
onRemove?: () => void
}) {
return (
<span className="inline-flex max-w-[200px] items-center gap-1 rounded-md border border-border bg-muted/60 py-0.5 pl-1.5 pr-1 text-[11px] text-foreground">
<CodeIcon className="size-3 shrink-0 text-muted-foreground" />
<span className="truncate font-mono">{label}</span>
{onRemove && (
<IconButton
type="button"
variant="ghost"
size="icon-xs"
aria-label="Remove attachment"
onClick={onRemove}
>
<XIcon />
</IconButton>
)}
</span>
)
}
const dashboardFetch: typeof fetch = (input, init) => const dashboardFetch: typeof fetch = (input, init) =>
fetch(input, { ...init, credentials: "include" }) fetch(input, { ...init, credentials: "include" })
@ -271,25 +413,42 @@ function ChatBody({
onUserSend: (text: string) => void onUserSend: (text: string) => void
expectsHistory: boolean expectsHistory: boolean
}) { }) {
const composer = useReviewChatComposer()
const stream = useStreamContext() const stream = useStreamContext()
const [value, setValue] = useState("") const [value, setValue] = useState("")
const endRef = useRef<HTMLDivElement>(null) const [attachments, setAttachments] = useState<Array<ChatAttachment>>([])
const scrollRef = useRef<HTMLDivElement>(null)
const autoScrollRef = useRef(true)
const prevTopRef = useRef(0)
const messages = stream.messages const messages = stream.messages
const busy = stream.isLoading const busy = stream.isLoading
// True during the one-time getState hydration when switching to / loading an // True during the one-time getState hydration when switching to / loading an
// existing thread, before its messages have arrived. // existing thread, before its messages have arrived.
const hydrating = stream.isThreadLoading const hydrating = stream.isThreadLoading
// Receive "add to chat" attachments from the diff column as composer pills.
useEffect(() => { useEffect(() => {
endRef.current?.scrollIntoView({ behavior: "smooth" }) if (!composer) return
}, [messages]) composer.registerSink((attachment) =>
setAttachments((prev) =>
prev.some((a) => a.id === attachment.id) ? prev : [...prev, attachment]
)
)
return () => composer.registerSink(null)
}, [composer])
const removeAttachment = useCallback((id: string) => {
setAttachments((prev) => prev.filter((a) => a.id !== id))
}, [])
const send = useCallback( const send = useCallback(
(text: string) => { (text: string, atts: Array<ChatAttachment>) => {
const trimmed = text.trim() const trimmed = text.trim()
if (!trimmed || busy) return const first = atts[0]
onUserSend(trimmed) if ((!trimmed && !first) || busy) return
void stream.submit({ messages: [{ type: "human", content: trimmed }] }) const content = serializeMessage(trimmed, atts)
onUserSend(trimmed || (first ? attachmentPillLabel(first) : ""))
void stream.submit({ messages: [{ type: "human", content }] })
}, },
[busy, stream, onUserSend], [busy, stream, onUserSend],
) )
@ -301,44 +460,85 @@ function ChatBody({
}) })
const submitComposer = () => { const submitComposer = () => {
send(value) send(value, attachments)
setValue("") setValue("")
setAttachments([])
} }
// Show the loading placeholder (not the empty/intro state) while an existing // Show the loading placeholder (not the empty/intro state) while an existing
// conversation hydrates, so a chat with messages never flashes its greeting. // conversation hydrates, so a chat with messages never flashes its greeting.
const showEmpty = visible.length === 0 && !busy const showEmpty = visible.length === 0 && !busy
const showLoading = showEmpty && hydrating && expectsHistory const showLoading = showEmpty && hydrating && expectsHistory
const showMessages = !showEmpty && !showLoading
// Auto-scroll is fully contained to the messages list (scrollTop assignment,
// never scrollIntoView) so it can't bubble up and move the diff column. We
// pause auto-scroll when the user scrolls up and resume when near the bottom.
useLayoutEffect(() => {
if (!showMessages) return
const el = scrollRef.current
if (!el || !autoScrollRef.current) return
el.scrollTop = el.scrollHeight
prevTopRef.current = el.scrollTop
}, [showMessages, messages, busy])
useEffect(() => {
if (!showMessages) return
const el = scrollRef.current
if (!el) return
const onScroll = () => {
const top = el.scrollTop
const nearBottom = el.scrollHeight - top - el.clientHeight <= 24
if (top < prevTopRef.current - 1) autoScrollRef.current = false
else if (nearBottom) autoScrollRef.current = true
prevTopRef.current = top
}
el.addEventListener("scroll", onScroll, { passive: true })
return () => el.removeEventListener("scroll", onScroll)
}, [showMessages])
return ( return (
<div className="flex flex-1 flex-col overflow-hidden"> <div className="flex flex-1 flex-col overflow-hidden">
{showLoading ? ( {showLoading ? (
<LoadingState /> <LoadingState />
) : showEmpty ? ( ) : showEmpty ? (
<EmptyState onPick={send} /> <EmptyState
onPick={(prompt) => {
send(prompt, attachments)
setAttachments([])
}}
/>
) : ( ) : (
<div className="flex flex-1 flex-col gap-4 overflow-y-auto p-4"> <div
ref={scrollRef}
className="flex flex-1 flex-col gap-4 overflow-y-auto p-4"
>
{visible.map((message, index) => { {visible.map((message, index) => {
const isUser = messageType(message) === "human" const isUser = messageType(message) === "human"
if (!isUser) {
return ( return (
<div <div key={message.id ?? index} className="flex justify-start">
key={message.id ?? index} <div className="w-full text-[13px] text-foreground">
className={cn("flex", isUser ? "justify-end" : "justify-start")}
>
<div
className={cn(
"text-[13px] text-foreground",
isUser
? "max-w-[85%] rounded-lg bg-muted px-3 py-2"
: "w-full",
)}
>
{isUser ? (
<span className="whitespace-pre-wrap">
{messageText(message.content)}
</span>
) : (
<Markdown content={messageText(message.content)} /> <Markdown content={messageText(message.content)} />
</div>
</div>
)
}
const parsed = parseUserMessage(messageText(message.content))
return (
<div key={message.id ?? index} className="flex justify-end">
<div className="flex max-w-[85%] flex-col items-end gap-1.5">
{parsed.attachments.length > 0 && (
<div className="flex flex-wrap justify-end gap-1">
{parsed.attachments.map((attachment, i) => (
<AttachmentPill key={i} label={attachment.label} />
))}
</div>
)}
{parsed.text && (
<span className="rounded-lg bg-muted px-3 py-2 text-[13px] whitespace-pre-wrap text-foreground">
{parsed.text}
</span>
)} )}
</div> </div>
</div> </div>
@ -351,12 +551,23 @@ function ChatBody({
</div> </div>
</div> </div>
)} )}
<div ref={endRef} />
</div> </div>
)} )}
<div className="p-3"> <div className="p-3">
<div className="flex items-end gap-2 rounded-2xl border border-border bg-background py-1.5 pl-3.5 pr-1.5 transition-colors focus-within:border-ring/60"> <div className="flex flex-col gap-1.5 rounded-2xl border border-border bg-background px-1.5 py-1.5 transition-colors focus-within:border-ring/60">
{attachments.length > 0 && (
<div className="flex flex-wrap gap-1 pl-2 pt-0.5">
{attachments.map((attachment) => (
<AttachmentPill
key={attachment.id}
label={attachmentPillLabel(attachment)}
onRemove={() => removeAttachment(attachment.id)}
/>
))}
</div>
)}
<div className="flex items-end gap-2 pl-2">
<Textarea <Textarea
value={value} value={value}
onChange={(event) => setValue(event.target.value)} onChange={(event) => setValue(event.target.value)}
@ -373,7 +584,7 @@ function ChatBody({
<IconButton <IconButton
type="button" type="button"
onClick={submitComposer} onClick={submitComposer}
disabled={!value.trim() || busy} disabled={(!value.trim() && attachments.length === 0) || busy}
aria-label="Send message" aria-label="Send message"
className="rounded-full" className="rounded-full"
> >
@ -382,6 +593,7 @@ function ChatBody({
</div> </div>
</div> </div>
</div> </div>
</div>
) )
} }

View file

@ -1,4 +1,12 @@
import { createContext, useContext, useEffect, useMemo, useState } from "react" import {
createContext,
memo,
useCallback,
useContext,
useEffect,
useMemo,
useState,
} from "react"
import { import {
FileTree, FileTree,
useFileTree, useFileTree,
@ -192,7 +200,7 @@ function ReviewGroupList({
<ReviewGroupRow <ReviewGroupRow
key={group.index} key={group.index}
group={group} group={group}
onSelect={() => onSelectGroup(group.index)} onSelectGroup={onSelectGroup}
onSelectFile={onSelectFile} onSelectFile={onSelectFile}
/> />
))} ))}
@ -231,29 +239,55 @@ function renderInlineCode(text: string): Array<ReactNode> {
}) })
} }
function ReviewGroupRow({ // The whole card is the scroll-to-group target so clicks anywhere (including
// the expanded explanation body) focus the diff. Nested controls — the file
// links and the "Read explanation" toggle — stop propagation so they keep
// their own behavior. memo'd + memoized string processing so re-renders from
// sibling state don't re-run Markdown/inline-code work.
const ReviewGroupRow = memo(function ReviewGroupRow({
group, group,
onSelect, onSelectGroup,
onSelectFile, onSelectFile,
}: { }: {
group: ReviewSidebarGroup group: ReviewSidebarGroup
onSelect: () => void onSelectGroup: (index: number) => void
onSelectFile: (path: string) => void onSelectFile: (path: string) => void
}) { }) {
const [expanded, setExpanded] = useState(false) const [expanded, setExpanded] = useState(false)
const title = useMemo(() => renderInlineCode(group.title), [group.title])
const summary = useMemo(
() => stripLocationLinks(group.summary),
[group.summary]
)
const selectGroup = useCallback(
() => onSelectGroup(group.index),
[onSelectGroup, group.index]
)
const onKeyDown = useCallback(
(event: React.KeyboardEvent) => {
if (event.key === "Enter" || event.key === " ") {
event.preventDefault()
onSelectGroup(group.index)
}
},
[onSelectGroup, group.index]
)
return ( return (
<div className="px-3 py-3 transition-colors hover:bg-[var(--ui-sidebar-hover)]"> <div
<button role="button"
type="button" tabIndex={0}
onClick={onSelect} onClick={selectGroup}
className="flex w-full items-start gap-2 text-left" onKeyDown={onKeyDown}
className="cursor-pointer px-3 py-3 transition-colors hover:bg-[var(--ui-sidebar-hover)]"
> >
<div className="flex w-full items-start gap-2 text-left">
<span className="mt-0.5 flex size-5 shrink-0 items-center justify-center rounded bg-[var(--ui-panel-2)] text-[11px] font-medium text-[var(--ui-text-dim)]"> <span className="mt-0.5 flex size-5 shrink-0 items-center justify-center rounded bg-[var(--ui-panel-2)] text-[11px] font-medium text-[var(--ui-text-dim)]">
{group.index} {group.index}
</span> </span>
<span className="min-w-0 flex-1"> <span className="min-w-0 flex-1">
<span className="block text-xs leading-5 font-medium text-[var(--ui-text)]"> <span className="block text-xs leading-5 font-medium text-[var(--ui-text)]">
{renderInlineCode(group.title)} {title}
</span> </span>
<span className="mt-1 flex flex-wrap items-center gap-1.5 text-[11px] text-[var(--ui-text-dim)]"> <span className="mt-1 flex flex-wrap items-center gap-1.5 text-[11px] text-[var(--ui-text-dim)]">
<span> <span>
@ -267,7 +301,7 @@ function ReviewGroupRow({
)} )}
</span> </span>
</span> </span>
</button> </div>
{group.files.length > 0 && ( {group.files.length > 0 && (
<div className="mt-2 space-y-0.5 pl-7"> <div className="mt-2 space-y-0.5 pl-7">
@ -277,7 +311,10 @@ function ReviewGroupRow({
<button <button
key={path} key={path}
type="button" type="button"
onClick={() => onSelectFile(path)} onClick={(event) => {
event.stopPropagation()
onSelectFile(path)
}}
title={path} title={path}
className="flex w-full items-baseline gap-1.5 text-left text-[11px] hover:text-[var(--ui-accent)]" className="flex w-full items-baseline gap-1.5 text-left text-[11px] hover:text-[var(--ui-accent)]"
> >
@ -299,7 +336,10 @@ function ReviewGroupRow({
<div className="mt-2"> <div className="mt-2">
<button <button
type="button" type="button"
onClick={() => setExpanded((value) => !value)} onClick={(event) => {
event.stopPropagation()
setExpanded((value) => !value)
}}
className="inline-flex items-center gap-1 text-[11px] font-medium text-[var(--ui-accent)]" className="inline-flex items-center gap-1 text-[11px] font-medium text-[var(--ui-accent)]"
> >
<CaretRightIcon <CaretRightIcon
@ -312,14 +352,14 @@ function ReviewGroupRow({
</button> </button>
{expanded && ( {expanded && (
<div className="mt-1.5"> <div className="mt-1.5">
<Markdown content={stripLocationLinks(group.summary)} /> <Markdown content={summary} />
</div> </div>
)} )}
</div> </div>
)} )}
</div> </div>
) )
} })
function ReviewFileTreeExplorer({ function ReviewFileTreeExplorer({
files, files,

View file

@ -1,7 +1,14 @@
import { useMemo } from "react" import { useMemo } from "react"
import { preloadHighlighter } from "@pierre/diffs" import { preloadHighlighter } from "@pierre/diffs"
import type {
VirtualFileMetrics,
WorkerInitializationRenderOptions,
WorkerPoolOptions,
} from "@pierre/diffs/react"
import { useResolvedTheme } from "@/lib/theme" import { useResolvedTheme } from "@/lib/theme"
export type DiffStyle = "unified" | "split"
export const DIFF_UNSAFE_CSS = ` export const DIFF_UNSAFE_CSS = `
[data-diffs-header], [data-diffs-header],
[data-diff], [data-diff],
@ -71,14 +78,66 @@ export const diffOptions = {
tokenizeMaxLength: 120_000, tokenizeMaxLength: 120_000,
} }
export function useDiffOptions() { export function useDiffOptions(diffStyle: DiffStyle = "unified") {
const resolvedTheme = useResolvedTheme() const resolvedTheme = useResolvedTheme()
return useMemo( return useMemo(
() => ({ ...diffOptions, themeType: resolvedTheme }), () => ({ ...diffOptions, themeType: resolvedTheme, diffStyle }),
[resolvedTheme] [resolvedTheme, diffStyle]
) )
} }
// Shared virtualization + worker-pool config for <Virtualizer>/<MultiFileDiff>.
// Tuned for the agent git panel and the PR reviews page; keep them aligned so
// both viewers window rows and offload highlighting identically.
export const DIFF_VIRTUALIZER_CONFIG = {
overscrollSize: 1200,
intersectionObserverMargin: 4800,
}
export const DIFF_VIRTUAL_METRICS = {
hunkLineCount: 80,
lineHeight: 18,
diffHeaderHeight: 0,
spacing: 8,
} satisfies Partial<VirtualFileMetrics>
export const DIFF_WORKER_POOL_OPTIONS = {
workerFactory: () =>
new Worker(
new URL("@pierre/diffs/worker/worker-portable.js", import.meta.url),
{ type: "module" }
),
poolSize: 2,
totalASTLRUCacheSize: 120,
} satisfies WorkerPoolOptions
export const DIFF_WORKER_HIGHLIGHTER_OPTIONS = {
theme: { light: "pierre-light", dark: "pierre-dark" },
lineDiffType: "word-alt",
maxLineDiffLength: 800,
tokenizeMaxLineLength: 1200,
langs: ["text"],
} satisfies WorkerInitializationRenderOptions
function hashFileContents(contents: string): string {
let hash = 0x811c9dc5
for (let i = 0; i < contents.length; i++) {
hash ^= contents.charCodeAt(i)
hash = Math.imul(hash, 0x01000193)
}
return (hash >>> 0).toString(36)
}
// Stable per-file content key so the worker pool dedupes highlight work across
// re-renders instead of re-tokenizing identical content.
export function fileContentsCacheKey(
path: string,
side: "old" | "new",
contents: string
): string {
return `${path}:${side}:${contents.length}:${hashFileContents(contents)}`
}
let highlighterWarmup: Promise<void> | null = null let highlighterWarmup: Promise<void> | null = null
/** /**

View file

@ -1,4 +1,11 @@
import { useCallback, useEffect, useRef, useState } from "react"; import {
createContext,
useCallback,
useContext,
useEffect,
useRef,
useState,
} from "react";
import { SidebarSimpleIcon } from "@phosphor-icons/react"; import { SidebarSimpleIcon } from "@phosphor-icons/react";
import { useHotkey } from "@/lib/hotkeys"; import { useHotkey } from "@/lib/hotkeys";
@ -55,6 +62,30 @@ export function useSidebarLayout() {
return { width, collapsed, setWidth, setCollapsed, toggle, closeOnMobile }; return { width, collapsed, setWidth, setCollapsed, toggle, closeOnMobile };
} }
export type SidebarLayout = ReturnType<typeof useSidebarLayout>;
// Shares the single sidebar-layout instance with page content so it can react
// to the collapsed state (e.g. clear room for the fixed collapse toggle).
const SidebarLayoutContext = createContext<SidebarLayout | null>(null);
export function SidebarLayoutProvider({
value,
children,
}: {
value: SidebarLayout;
children: React.ReactNode;
}) {
return (
<SidebarLayoutContext.Provider value={value}>
{children}
</SidebarLayoutContext.Provider>
);
}
export function useSidebarCollapsed(): boolean {
return useContext(SidebarLayoutContext)?.collapsed ?? false;
}
interface SidebarFrameProps { interface SidebarFrameProps {
width: number; width: number;
setWidth: (next: number) => void; setWidth: (next: number) => void;

View file

@ -1,6 +1,7 @@
import { Link, Navigate, createFileRoute } from "@tanstack/react-router" import { Link, Navigate, createFileRoute } from "@tanstack/react-router"
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query" import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"
import { import {
memo,
useCallback, useCallback,
useEffect, useEffect,
useLayoutEffect, useLayoutEffect,
@ -20,11 +21,18 @@ import {
FlagIcon, FlagIcon,
GitPullRequestIcon, GitPullRequestIcon,
InfoIcon, InfoIcon,
RowsIcon,
SquareSplitHorizontalIcon,
XCircleIcon, XCircleIcon,
XIcon, XIcon,
} from "@phosphor-icons/react" } from "@phosphor-icons/react"
import { IoLogoGithub } from "react-icons/io5" import { IoLogoGithub } from "react-icons/io5"
import { MultiFileDiff } from "@pierre/diffs/react" import {
MultiFileDiff,
Virtualizer,
WorkerPoolContextProvider,
} from "@pierre/diffs/react"
import type { FileContents } from "@pierre/diffs/react"
import type { DiffLineAnnotation, SelectedLineRange } from "@pierre/diffs" import type { DiffLineAnnotation, SelectedLineRange } from "@pierre/diffs"
import type { import type {
@ -38,10 +46,22 @@ import type {
ReviewSidebarGroup, ReviewSidebarGroup,
ReviewSidebarView, ReviewSidebarView,
} from "@/components/agents/ReviewSidebar" } from "@/components/agents/ReviewSidebar"
import type { ChatAttachment } from "@/components/agents/ReviewChat"
import type { DiffStyle } from "@/components/agents/utils/diffUtils"
import { Markdown } from "@/components/agents/ported" import { Markdown } from "@/components/agents/ported"
import { ReviewChat } from "@/components/agents/ReviewChat"
import { useRegisterReviewSidebar } from "@/components/agents/ReviewSidebar"
import { import {
ReviewChat,
ReviewChatComposerProvider,
useReviewChatComposer,
} from "@/components/agents/ReviewChat"
import { useRegisterReviewSidebar } from "@/components/agents/ReviewSidebar"
import { useSidebarCollapsed } from "@/components/sidebar-layout"
import {
DIFF_VIRTUALIZER_CONFIG,
DIFF_VIRTUAL_METRICS,
DIFF_WORKER_HIGHLIGHTER_OPTIONS,
DIFF_WORKER_POOL_OPTIONS,
fileContentsCacheKey,
useDiffOptions, useDiffOptions,
warmDiffHighlighter, warmDiffHighlighter,
} from "@/components/agents/utils/diffUtils" } from "@/components/agents/utils/diffUtils"
@ -57,6 +77,58 @@ export const Route = createFileRoute("/agents/reviews/$owner/$repo/$number")({
type SideTab = "info" | "chat" type SideTab = "info" | "chat"
const REVIEW_VIEW_STORAGE_KEY = "open-swe.review.view" const REVIEW_VIEW_STORAGE_KEY = "open-swe.review.view"
const REVIEW_DIFF_STYLE_STORAGE_KEY = "open-swe.review.diffStyle"
function readStoredDiffStyle(): DiffStyle {
if (typeof window === "undefined") return "unified"
return window.localStorage.getItem(REVIEW_DIFF_STYLE_STORAGE_KEY) === "split"
? "split"
: "unified"
}
// One attachment for a single-side line range. Deletions resolve against the
// original file, additions against the modified file.
function makeSideAttachment(
file: ReviewDiffFile,
side: "deletions" | "additions",
fromLine: number,
toLine: number
): ChatAttachment {
const source =
side === "deletions" ? file.originalContent : file.modifiedContent
const lines = source.split("\n")
const start = Math.max(1, Math.min(fromLine, toLine))
const end = Math.max(fromLine, toLine)
const snippet = lines.slice(start - 1, end).join("\n")
const sideLabel = side === "deletions" ? "L" : "R"
const lineLabel =
start === end ? `${sideLabel}${start}` : `${sideLabel}${start}-${end}`
const language = file.path.includes(".")
? (file.path.split(".").pop() ?? "")
: ""
return { id: crypto.randomUUID(), path: file.path, lineLabel, language, snippet }
}
// Build chat attachments from the selected range. A range can span from a
// deletion to an addition (side !== endSide) when dragging across a replaced
// block; slicing one file by start..end would paste the wrong lines, so each
// side is collected separately.
function buildSelectionAttachments(
file: ReviewDiffFile,
range: SelectedLineRange
): Array<ChatAttachment> {
const startSide = range.side ?? "additions"
const endSide = range.endSide ?? startSide
if (startSide === endSide) {
return [makeSideAttachment(file, startSide, range.start, range.end)]
}
const deletionLine = startSide === "deletions" ? range.start : range.end
const additionLine = startSide === "additions" ? range.start : range.end
return [
makeSideAttachment(file, "deletions", deletionLine, deletionLine),
makeSideAttachment(file, "additions", additionLine, additionLine),
]
}
interface ResolvedGroup { interface ResolvedGroup {
index: number index: number
@ -127,6 +199,7 @@ function ReviewDetailPage() {
const { owner, repo, number } = Route.useParams() const { owner, repo, number } = Route.useParams()
const prNumber = Number(number) const prNumber = Number(number)
const session = useSession() const session = useSession()
const sidebarCollapsed = useSidebarCollapsed()
const detail = useQuery({ const detail = useQuery({
queryKey: ["review", owner, repo, prNumber], queryKey: ["review", owner, repo, prNumber],
queryFn: () => api.getReview(owner, repo, prNumber), queryFn: () => api.getReview(owner, repo, prNumber),
@ -163,7 +236,13 @@ function ReviewDetailPage() {
return ( return (
<div className="flex min-w-0 flex-1 flex-col overflow-hidden bg-background text-foreground"> <div className="flex min-w-0 flex-1 flex-col overflow-hidden bg-background text-foreground">
<header className="flex h-12 shrink-0 items-center gap-3 border-b border-border px-4 text-xs"> <header
className={cn(
"flex h-12 shrink-0 items-center gap-3 border-b border-border pr-4 text-xs",
// Clear room for the fixed collapse toggle when the sidebar is hidden.
sidebarCollapsed ? "pl-14" : "pl-4"
)}
>
<Link <Link
to="/agents/reviews" to="/agents/reviews"
className="inline-flex items-center gap-1.5 text-muted-foreground hover:text-foreground" className="inline-flex items-center gap-1.5 text-muted-foreground hover:text-foreground"
@ -204,6 +283,13 @@ function ReviewDetailPage() {
) )
} }
const NO_FINDINGS: Array<ReviewFinding> = []
interface UserSelection {
file: string
range: SelectedLineRange
}
function ReviewBody({ function ReviewBody({
detail, detail,
diffFiles, diffFiles,
@ -211,6 +297,23 @@ function ReviewBody({
detail: ReviewDetail detail: ReviewDetail
diffFiles: Array<ReviewDiffFile> | null diffFiles: Array<ReviewDiffFile> | null
}) { }) {
// The composer provider lives inside ReviewBody so it remounts in lockstep
// with the head_sha-keyed body (and the activeId-keyed chat thread).
return (
<ReviewChatComposerProvider>
<ReviewBodyInner detail={detail} diffFiles={diffFiles} />
</ReviewChatComposerProvider>
)
}
function ReviewBodyInner({
detail,
diffFiles,
}: {
detail: ReviewDetail
diffFiles: Array<ReviewDiffFile> | null
}) {
const composer = useReviewChatComposer()
const [sideTab, setSideTab] = useState<SideTab>("info") const [sideTab, setSideTab] = useState<SideTab>("info")
const [selectedFile, setSelectedFile] = useState<string | null>(null) const [selectedFile, setSelectedFile] = useState<string | null>(null)
const fileRefs = useRef<Record<string, HTMLDivElement | null>>({}) const fileRefs = useRef<Record<string, HTMLDivElement | null>>({})
@ -220,13 +323,29 @@ function ReviewBody({
) )
const [focused, setFocused] = useState<ReviewFinding | null>(null) const [focused, setFocused] = useState<ReviewFinding | null>(null)
const [anchorEl, setAnchorEl] = useState<HTMLElement | null>(null) const [anchorEl, setAnchorEl] = useState<HTMLElement | null>(null)
const scrollRef = useRef<HTMLDivElement | null>(null) const [userSelection, setUserSelection] = useState<UserSelection | null>(null)
const [diffScrollEl, setDiffScrollEl] = useState<HTMLDivElement | null>(null)
const groupRefs = useRef<Record<number, HTMLDivElement | null>>({}) const groupRefs = useRef<Record<number, HTMLDivElement | null>>({})
const [diffStyle, setDiffStyleState] = useState<DiffStyle>(() =>
readStoredDiffStyle()
)
const setDiffStyle = useCallback((next: DiffStyle) => {
setDiffStyleState(next)
if (typeof window !== "undefined") {
window.localStorage.setItem(REVIEW_DIFF_STYLE_STORAGE_KEY, next)
}
}, [])
useEffect(() => { useEffect(() => {
void warmDiffHighlighter() void warmDiffHighlighter()
}, []) }, [])
// Latest-value refs so the callbacks below can stay referentially stable
// (so memo(FileDiffCard) actually skips unrelated re-renders) while still
// reading current state.
const focusedRef = useRef(focused)
focusedRef.current = focused
const viewedStorageKey = `open-swe.review.viewed.${detail.owner}/${detail.repo}/${detail.number}.${detail.head_sha}` const viewedStorageKey = `open-swe.review.viewed.${detail.owner}/${detail.repo}/${detail.number}.${detail.head_sha}`
const [viewed, setViewed] = useState<Set<string>>(() => { const [viewed, setViewed] = useState<Set<string>>(() => {
if (typeof window === "undefined") return new Set() if (typeof window === "undefined") return new Set()
@ -237,18 +356,26 @@ function ReviewBody({
return new Set() return new Set()
} }
}) })
const viewedRef = useRef(viewed)
viewedRef.current = viewed
const expandedRef = useRef(expandedFiles)
expandedRef.current = expandedFiles
const toggleViewed = useCallback( const toggleViewed = useCallback(
(path: string) => { (path: string) => {
const becomingViewed = !viewedRef.current.has(path)
setViewed((prev) => { setViewed((prev) => {
const next = new Set(prev) const next = new Set(prev)
if (next.has(path)) next.delete(path) if (becomingViewed) next.add(path)
else next.add(path) else next.delete(path)
window.localStorage.setItem( window.localStorage.setItem(
viewedStorageKey, viewedStorageKey,
JSON.stringify(Array.from(next)) JSON.stringify(Array.from(next))
) )
return next return next
}) })
if (becomingViewed && focusedRef.current?.file === path) setFocused(null)
setExpandedFiles((prev) => ({ ...prev, [path]: !becomingViewed }))
}, },
[viewedStorageKey] [viewedStorageKey]
) )
@ -265,7 +392,6 @@ function ReviewBody({
}) })
const persistRead = useCallback( const persistRead = useCallback(
(next: Set<string>) => { (next: Set<string>) => {
setRead(next)
window.localStorage.setItem( window.localStorage.setItem(
readStorageKey, readStorageKey,
JSON.stringify(Array.from(next)) JSON.stringify(Array.from(next))
@ -275,12 +401,18 @@ function ReviewBody({
) )
const markRead = useCallback( const markRead = useCallback(
(id: string) => { (id: string) => {
persistRead(new Set(read).add(id)) setRead((prev) => {
const next = new Set(prev).add(id)
persistRead(next)
return next
})
}, },
[persistRead, read] [persistRead]
) )
const markAllRead = useCallback(() => { const markAllRead = useCallback(() => {
persistRead(new Set(detail.findings.map((f) => f.id))) const next = new Set(detail.findings.map((f) => f.id))
setRead(next)
persistRead(next)
}, [detail.findings, persistRead]) }, [detail.findings, persistRead])
const findingsByFile = useMemo(() => { const findingsByFile = useMemo(() => {
@ -400,20 +532,124 @@ function ReviewBody({
}) })
}, []) }, [])
const filesByPath = useMemo(
() => new Map((diffFiles ?? []).map((file) => [file.path, file])),
[diffFiles]
)
const filesByPathRef = useRef(filesByPath)
filesByPathRef.current = filesByPath
// The Virtualizer doesn't forward a ref; grab its scroll element (the
// grandparent of this hidden probe, which lives in its content div) so the
// anchored finding card can position against it.
const scrollerProbe = useCallback((node: HTMLDivElement | null) => {
const scroller = node?.parentElement?.parentElement
setDiffScrollEl(scroller instanceof HTMLDivElement ? scroller : null)
}, [])
const registerSection = useCallback(
(path: string, node: HTMLDivElement | null) => {
fileRefs.current[path] = node
},
[]
)
const registerAnchor = useCallback(
(id: string, node: HTMLElement | null) => {
anchorRefs.current[id] = node
},
[]
)
const toggleExpanded = useCallback((path: string) => {
const current = expandedRef.current[path] ?? !viewedRef.current.has(path)
const next = !current
if (!next && focusedRef.current?.file === path) setFocused(null)
setExpandedFiles((prev) => ({ ...prev, [path]: next }))
}, [])
const selectLines = useCallback(
(path: string, range: SelectedLineRange | null) => {
if (range) {
setUserSelection({ file: path, range })
if (focusedRef.current) setFocused(null)
} else {
setUserSelection((prev) => (prev?.file === path ? null : prev))
}
},
[]
)
const addToChat = useCallback(
(path: string, range: SelectedLineRange) => {
const file = filesByPathRef.current.get(path)
if (!file) return
for (const attachment of buildSelectionAttachments(file, range)) {
composer?.addAttachment(attachment)
}
setSideTab("chat")
setUserSelection(null)
},
[composer]
)
// ⌘L / Ctrl+L adds the current line selection to the chat (Cursor-style).
const userSelectionRef = useRef(userSelection)
userSelectionRef.current = userSelection
useEffect(() => {
const onKeyDown = (event: KeyboardEvent) => {
if ((event.metaKey || event.ctrlKey) && event.key.toLowerCase() === "l") {
const sel = userSelectionRef.current
if (sel) {
event.preventDefault()
addToChat(sel.file, sel.range)
}
}
}
window.addEventListener("keydown", onKeyDown)
return () => window.removeEventListener("keydown", onKeyDown)
}, [addToChat])
// Clicking away from the highlighted rows clears the selection. A pointer-down
// that begins a fresh selection clears here first, then the new drag repaints.
// Reads the ref so the listener is registered once (no churn during a drag).
useEffect(() => {
const onPointerDown = (event: PointerEvent) => {
if (!userSelectionRef.current) return
const target = event.target
if (target instanceof Element && target.closest("[data-add-to-chat]"))
return
setUserSelection(null)
}
window.addEventListener("pointerdown", onPointerDown)
return () => window.removeEventListener("pointerdown", onPointerDown)
}, [])
const openFinding = useCallback( const openFinding = useCallback(
(finding: ReviewFinding) => { (finding: ReviewFinding) => {
markRead(finding.id) markRead(finding.id)
setUserSelection(null)
setFocused(finding) setFocused(finding)
if (!isAnchored(finding)) { if (!isAnchored(finding)) {
setAnchorEl(null) setAnchorEl(null)
return return
} }
setExpandedFiles((prev) => ({ ...prev, [finding.file]: true })) setExpandedFiles((prev) => ({ ...prev, [finding.file]: true }))
// The file card header is always mounted, but its diff rows window in/out
// under virtualization — scroll the card into view, then poll a few frames
// for the finding's anchor element before positioning the card.
requestAnimationFrame(() => { requestAnimationFrame(() => {
const node = fileRefs.current[finding.file]?.scrollIntoView({ block: "center" })
anchorRefs.current[finding.id] ?? fileRefs.current[finding.file] let tries = 0
node?.scrollIntoView({ block: "center" }) const tryAnchor = () => {
setAnchorEl(anchorRefs.current[finding.id] ?? null) const anchor = anchorRefs.current[finding.id]
if (anchor) {
setAnchorEl(anchor)
return
}
if (tries++ < 30) requestAnimationFrame(tryAnchor)
else setAnchorEl(null)
}
tryAnchor()
}) })
}, },
[markRead] [markRead]
@ -421,37 +657,32 @@ function ReviewBody({
const closeFinding = useCallback(() => setFocused(null), []) const closeFinding = useCallback(() => setFocused(null), [])
const renderFileCard = (file: ReviewDiffFile) => ( const renderFileCard = (file: ReviewDiffFile) => {
const selectedLines =
focused?.file === file.path && isAnchored(focused)
? findingSelectedRange(focused)
: userSelection?.file === file.path
? userSelection.range
: null
return (
<FileDiffCard <FileDiffCard
key={file.path} key={file.path}
file={file} file={file}
findings={findingsByFile.get(file.path) ?? []} findings={findingsByFile.get(file.path) ?? NO_FINDINGS}
focused={focused} selectedLines={selectedLines}
viewed={viewed.has(file.path)} viewed={viewed.has(file.path)}
onToggleViewed={() => { onToggleViewed={toggleViewed}
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)} expanded={expandedFiles[file.path] ?? !viewed.has(file.path)}
onToggleExpanded={() => { onToggleExpanded={toggleExpanded}
const next = !(expandedFiles[file.path] ?? !viewed.has(file.path))
if (!next && focused?.file === file.path) setFocused(null)
setExpandedFiles((prev) => ({ ...prev, [file.path]: next }))
}}
onFindingClick={openFinding} onFindingClick={openFinding}
sectionRef={(node) => { onSelectLines={selectLines}
fileRefs.current[file.path] = node onAddToChat={addToChat}
}} registerSection={registerSection}
anchorRef={(id, node) => { registerAnchor={registerAnchor}
anchorRefs.current[id] = node diffStyle={diffStyle}
}}
/> />
) )
}
const sidebarData = useMemo( const sidebarData = useMemo(
() => ({ () => ({
@ -498,13 +729,21 @@ function ReviewBody({
} }
}, [focused]) }, [focused])
const isAnchoredCard = focused !== null && isAnchored(focused) && anchorEl
return ( return (
<div <div className="flex min-h-0 flex-1 overflow-hidden">
ref={scrollRef} <main className="relative flex min-h-0 min-w-0 flex-1">
className="relative flex min-h-0 flex-1 overflow-y-auto" <WorkerPoolContextProvider
poolOptions={DIFF_WORKER_POOL_OPTIONS}
highlighterOptions={DIFF_WORKER_HIGHLIGHTER_OPTIONS}
> >
<main className="min-w-0 flex-1"> <Virtualizer
<div className="mx-auto max-w-6xl px-6 py-6"> className="relative min-h-0 flex-1 overflow-y-auto"
contentClassName="mx-auto w-full max-w-6xl px-6 py-6"
config={DIFF_VIRTUALIZER_CONFIG}
>
<div ref={scrollerProbe} aria-hidden className="hidden" />
<PrHeader detail={detail} /> <PrHeader detail={detail} />
<div className="mt-4 rounded-lg border border-border bg-card p-4"> <div className="mt-4 rounded-lg border border-border bg-card p-4">
{detail.pr.body ? ( {detail.pr.body ? (
@ -517,8 +756,9 @@ function ReviewBody({
</div> </div>
<div className="mt-6"> <div className="mt-6">
<div className="mb-2 flex items-center justify-between"> <div className="mb-2 flex items-center justify-between gap-3">
<h2 className="text-sm font-medium">Changes</h2> <h2 className="text-sm font-medium">Changes</h2>
<div className="flex items-center gap-3">
{linesLeft !== null && ( {linesLeft !== null && (
<span className="text-xs text-muted-foreground"> <span className="text-xs text-muted-foreground">
{linesLeft === 0 {linesLeft === 0
@ -526,6 +766,10 @@ function ReviewBody({
: `${linesLeft} lines left`} : `${linesLeft} lines left`}
</span> </span>
)} )}
{diffFiles && diffFiles.length > 0 && (
<DiffStyleToggle value={diffStyle} onChange={setDiffStyle} />
)}
</div>
</div> </div>
{!diffFiles ? ( {!diffFiles ? (
<Skeleton className="h-64 w-full" /> <Skeleton className="h-64 w-full" />
@ -549,10 +793,13 @@ function ReviewBody({
))} ))}
</div> </div>
) : ( ) : (
<div className="space-y-3">{diffFiles.map(renderFileCard)}</div> <div className="space-y-3">
{diffFiles.map(renderFileCard)}
</div>
)} )}
</div> </div>
</div> </Virtualizer>
</WorkerPoolContextProvider>
</main> </main>
<SidePanel <SidePanel
@ -565,17 +812,18 @@ function ReviewBody({
onFindingClick={openFinding} onFindingClick={openFinding}
/> />
{focused && {focused && isAnchoredCard && (
(isAnchored(focused) && anchorEl ? (
<AnchoredFindingCard <AnchoredFindingCard
key={focused.id} key={focused.id}
detail={detail} detail={detail}
finding={focused} finding={focused}
anchorEl={anchorEl} anchorEl={anchorEl}
scrollRef={scrollRef} scrollEl={diffScrollEl}
onClose={closeFinding} onClose={closeFinding}
/> />
) : ( )}
{focused && !isAnchoredCard && (
<div <div
data-finding-card data-finding-card
role="dialog" role="dialog"
@ -588,11 +836,66 @@ function ReviewBody({
onClose={closeFinding} onClose={closeFinding}
/> />
</div> </div>
))} )}
</div> </div>
) )
} }
function DiffStyleToggle({
value,
onChange,
}: {
value: DiffStyle
onChange: (value: DiffStyle) => void
}) {
return (
<div className="flex items-center gap-0.5 rounded-md border border-border p-0.5">
<DiffStyleButton
active={value === "unified"}
label="Unified view"
onClick={() => onChange("unified")}
>
<RowsIcon className="size-3.5" />
</DiffStyleButton>
<DiffStyleButton
active={value === "split"}
label="Split view"
onClick={() => onChange("split")}
>
<SquareSplitHorizontalIcon className="size-3.5" />
</DiffStyleButton>
</div>
)
}
function DiffStyleButton({
active,
label,
onClick,
children,
}: {
active: boolean
label: string
onClick: () => void
children: React.ReactNode
}) {
return (
<button
type="button"
onClick={onClick}
aria-label={label}
aria-pressed={active}
title={label}
className={cn(
"flex size-5 items-center justify-center rounded text-muted-foreground transition-colors",
active ? "bg-muted text-foreground" : "hover:text-foreground"
)}
>
{children}
</button>
)
}
function PrHeader({ detail }: { detail: ReviewDetail }) { function PrHeader({ detail }: { detail: ReviewDetail }) {
const { pr } = detail const { pr } = detail
const stateStyles: Record<string, string> = { const stateStyles: Record<string, string> = {
@ -662,30 +965,43 @@ function GroupHeader({ group }: { group: ResolvedGroup }) {
) )
} }
function FileDiffCard({ const FileDiffCard = memo(function FileDiffCard({
file, file,
findings, findings,
focused, selectedLines,
viewed, viewed,
onToggleViewed, onToggleViewed,
expanded, expanded,
onToggleExpanded, onToggleExpanded,
onFindingClick, onFindingClick,
sectionRef, onSelectLines,
anchorRef, onAddToChat,
registerSection,
registerAnchor,
diffStyle,
}: { }: {
file: ReviewDiffFile file: ReviewDiffFile
findings: Array<ReviewFinding> findings: Array<ReviewFinding>
focused: ReviewFinding | null selectedLines: SelectedLineRange | null
viewed: boolean viewed: boolean
onToggleViewed: () => void onToggleViewed: (path: string) => void
expanded: boolean expanded: boolean
onToggleExpanded: () => void onToggleExpanded: (path: string) => void
onFindingClick: (finding: ReviewFinding) => void onFindingClick: (finding: ReviewFinding) => void
sectionRef: (node: HTMLDivElement | null) => void onSelectLines: (path: string, range: SelectedLineRange | null) => void
anchorRef: (id: string, node: HTMLElement | null) => void onAddToChat: (path: string, range: SelectedLineRange) => void
registerSection: (path: string, node: HTMLDivElement | null) => void
registerAnchor: (id: string, node: HTMLElement | null) => void
diffStyle: DiffStyle
}) { }) {
const diffOptions = useDiffOptions() const diffOptions = useDiffOptions(diffStyle)
const diffWrapperRef = useRef<HTMLDivElement | null>(null)
const lastPointerRef = useRef<{ x: number; y: number } | null>(null)
const [popup, setPopup] = useState<{
range: SelectedLineRange
x: number
y: number
} | null>(null)
const lineAnnotations = useMemo<Array<DiffLineAnnotation<ReviewFinding>>>( const lineAnnotations = useMemo<Array<DiffLineAnnotation<ReviewFinding>>>(
() => () =>
@ -699,12 +1015,87 @@ function FileDiffCard({
[findings] [findings]
) )
const selectedLines = useMemo<SelectedLineRange | null>(() => { // Native line selection plus the visible gutter "+" handle you can click and
if (focused?.file === file.path && isAnchored(focused)) { // drag to select a range. onLineSelectionChange fires on every drag move; we
return findingSelectedRange(focused) // push it into the controlled selection so the rows highlight live as you
// drag (in controlled mode Pierre only paints when the prop updates). The
// "Add to Chat" popup is shown on onLineSelectionEnd (release only); ⌘L
// (handled in ReviewBody) adds without it. onGutterUtilityClick must be
// non-null for Pierre to turn the "+" into a drag selector.
const cardOptions = useMemo(
() => ({
...diffOptions,
enableLineSelection: true,
enableGutterUtility: true,
onGutterUtilityClick: () => undefined,
onLineSelectionChange: (range: SelectedLineRange | null) =>
onSelectLines(file.path, range),
onLineSelectionEnd: (range: SelectedLineRange | null) => {
onSelectLines(file.path, range)
if (!range) {
setPopup(null)
return
} }
return null // Anchor to the gutter "+" handle Pierre places on the selection's
}, [focused, file.path]) // bottom line (inside the diff's shadow DOM) — a stable position,
// unlike the pointer-release point. Fall back to the pointer.
const host = diffWrapperRef.current?.querySelector("diffs-container")
const handle = host?.shadowRoot?.querySelector(
"[data-gutter-utility-slot]"
)
const rect =
handle instanceof HTMLElement ? handle.getBoundingClientRect() : null
const pointer = lastPointerRef.current
if (rect) setPopup({ range, x: rect.left, y: rect.top })
else if (pointer) setPopup({ range, x: pointer.x, y: pointer.y })
else setPopup(null)
},
}),
[diffOptions, file.path, onSelectLines]
)
const addPopupToChat = useCallback(() => {
if (popup) onAddToChat(file.path, popup.range)
setPopup(null)
}, [popup, onAddToChat, file.path])
// Drop the popup once the selection clears (e.g. added via ⌘L, or a finding
// took focus) so it can't add the same range twice.
useEffect(() => {
if (!selectedLines) setPopup(null)
}, [selectedLines])
const oldFile = useMemo<FileContents>(
() => ({
name: file.path,
contents: file.originalContent,
cacheKey: fileContentsCacheKey(file.path, "old", file.originalContent),
}),
[file.path, file.originalContent]
)
const newFile = useMemo<FileContents>(
() => ({
name: file.path,
contents: file.modifiedContent,
cacheKey: fileContentsCacheKey(file.path, "new", file.modifiedContent),
}),
[file.path, file.modifiedContent]
)
const sectionRef = useCallback(
(node: HTMLDivElement | null) => registerSection(file.path, node),
[registerSection, file.path]
)
const renderAnnotation = useCallback(
(annotation: DiffLineAnnotation<ReviewFinding>) => (
<FindingRailMarker
finding={annotation.metadata}
onFindingClick={onFindingClick}
anchorRef={registerAnchor}
/>
),
[onFindingClick, registerAnchor]
)
return ( return (
<div <div
@ -714,7 +1105,7 @@ function FileDiffCard({
<div className="flex items-center gap-2 bg-[var(--ui-panel-2)] 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 <button
type="button" type="button"
onClick={onToggleExpanded} onClick={() => onToggleExpanded(file.path)}
className="inline-flex items-center gap-2 text-left" className="inline-flex items-center gap-2 text-left"
> >
<CaretDownIcon <CaretDownIcon
@ -741,7 +1132,7 @@ function FileDiffCard({
type="button" type="button"
role="checkbox" role="checkbox"
aria-checked={viewed} aria-checked={viewed}
onClick={onToggleViewed} onClick={() => onToggleViewed(file.path)}
className={cn( className={cn(
"flex size-4 items-center justify-center rounded border border-border", "flex size-4 items-center justify-center rounded border border-border",
viewed && "bg-foreground text-background" viewed && "bg-foreground text-background"
@ -757,25 +1148,88 @@ function FileDiffCard({
Binary or large file — diff not shown. Binary or large file — diff not shown.
</div> </div>
) : ( ) : (
<div className="overflow-x-auto bg-[var(--ui-panel)] font-mono text-[11px] leading-5"> <div
ref={diffWrapperRef}
onPointerUpCapture={(event) => {
lastPointerRef.current = { x: event.clientX, y: event.clientY }
}}
className="overflow-x-auto bg-[var(--ui-panel)] font-mono text-[11px] leading-5"
>
<MultiFileDiff<ReviewFinding> <MultiFileDiff<ReviewFinding>
oldFile={{ name: file.path, contents: file.originalContent }} oldFile={oldFile}
newFile={{ name: file.path, contents: file.modifiedContent }} newFile={newFile}
options={diffOptions} options={cardOptions}
metrics={DIFF_VIRTUAL_METRICS}
lineAnnotations={lineAnnotations} lineAnnotations={lineAnnotations}
selectedLines={selectedLines} selectedLines={selectedLines}
renderAnnotation={(annotation) => ( renderAnnotation={renderAnnotation}
<FindingRailMarker />
finding={annotation.metadata} {popup && (
onFindingClick={onFindingClick} <AddToChatPopup
anchorRef={anchorRef} x={popup.x}
y={popup.y}
onAdd={addPopupToChat}
onDismiss={() => setPopup(null)}
/> />
)} )}
/>
</div> </div>
))} ))}
</div> </div>
) )
})
function AddToChatPopup({
x,
y,
onAdd,
onDismiss,
}: {
x: number
y: number
onAdd: () => void
onDismiss: () => void
}) {
// Positioned fixed at the pointer-release point so it escapes the diff's
// overflow clipping. Dismiss on Escape, scroll, or any outside pointer-down.
useEffect(() => {
const onKeyDown = (event: KeyboardEvent) => {
if (event.key === "Escape") onDismiss()
}
const onPointerDown = (event: PointerEvent) => {
const target = event.target
if (target instanceof Element && target.closest("[data-add-to-chat]"))
return
onDismiss()
}
window.addEventListener("keydown", onKeyDown)
window.addEventListener("pointerdown", onPointerDown)
// Capture so it also catches scrolls from the diff scroll container.
window.addEventListener("scroll", onDismiss, true)
return () => {
window.removeEventListener("keydown", onKeyDown)
window.removeEventListener("pointerdown", onPointerDown)
window.removeEventListener("scroll", onDismiss, true)
}
}, [onDismiss])
return (
<div
data-add-to-chat
style={{ position: "fixed", top: y, left: x }}
className="z-50 -translate-y-[calc(100%+4px)] font-sans"
>
<button
type="button"
onClick={onAdd}
className="inline-flex items-center gap-1.5 rounded-md border border-border bg-popover px-2 py-1 text-[11px] font-medium text-popover-foreground shadow-md hover:bg-muted"
>
Add to Chat
<kbd className="rounded border border-border px-1 text-[10px] text-muted-foreground">
⌘L
</kbd>
</button>
</div>
)
} }
function FindingRailMarker({ function FindingRailMarker({
@ -814,56 +1268,81 @@ const FINDING_CARD_GAP = 12
const FINDING_CARD_CLASS = const FINDING_CARD_CLASS =
"flex max-h-[70vh] w-[412px] flex-col overflow-hidden rounded-lg border border-border bg-background shadow-2xl" "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 // Pinned to the right of the diff column (toward the side panel), vertically
// with the diff — no per-scroll JS repositioning, hence no lag. Position is // aligned with the finding's annotation. The diff scroller is only as wide as
// recomputed only when layout shifts (files expand/collapse, resize). // <main>, so the card is viewport-fixed (clamped to the window width) to sit in
// the right gutter rather than overlapping the diff, and tracks the anchor as
// the diff scrolls (rAF-throttled). Hidden while the anchor is out of view.
function AnchoredFindingCard({ function AnchoredFindingCard({
detail, detail,
finding, finding,
anchorEl, anchorEl,
scrollRef, scrollEl,
onClose, onClose,
}: { }: {
detail: ReviewDetail detail: ReviewDetail
finding: ReviewFinding finding: ReviewFinding
anchorEl: HTMLElement anchorEl: HTMLElement
scrollRef: React.RefObject<HTMLDivElement | null> scrollEl: HTMLDivElement | null
onClose: () => void onClose: () => void
}) { }) {
const cardRef = useRef<HTMLDivElement | null>(null) const cardRef = useRef<HTMLDivElement | null>(null)
useLayoutEffect(() => { useLayoutEffect(() => {
const card = cardRef.current const card = cardRef.current
const scroller = scrollRef.current const scroller = scrollEl
if (!card || !scroller) return if (!card || !scroller) return
let frame: number | null = null
const position = () => { const position = () => {
if (!anchorEl.isConnected) return frame = null
const scrollerRect = scroller.getBoundingClientRect() const scrollerRect = scroller.getBoundingClientRect()
const anchorRect = anchorEl.getBoundingClientRect() const anchorRect = anchorEl.getBoundingClientRect()
const top = anchorRect.top - scrollerRect.top + scroller.scrollTop // Hide while the finding's line is scrolled out of the diff viewport.
if (
!anchorEl.isConnected ||
anchorRect.bottom < scrollerRect.top ||
anchorRect.top > scrollerRect.bottom
) {
card.style.visibility = "hidden"
return
}
const left = Math.max( const left = Math.max(
FINDING_CARD_GAP, FINDING_CARD_GAP,
Math.min( Math.min(
anchorRect.right - anchorRect.right + FINDING_CARD_GAP,
scrollerRect.left + window.innerWidth - FINDING_CARD_WIDTH - FINDING_CARD_GAP
scroller.scrollLeft +
FINDING_CARD_GAP,
scroller.clientWidth - FINDING_CARD_WIDTH - FINDING_CARD_GAP
) )
) )
const top = Math.max(
scrollerRect.top + FINDING_CARD_GAP,
Math.min(
anchorRect.top,
scrollerRect.bottom - card.offsetHeight - FINDING_CARD_GAP
)
)
card.style.visibility = "visible"
card.style.top = `${top}px` card.style.top = `${top}px`
card.style.left = `${left}px` card.style.left = `${left}px`
} }
const schedule = () => {
if (frame == null) frame = window.requestAnimationFrame(position)
}
position() position()
const observer = new ResizeObserver(position) scroller.addEventListener("scroll", schedule, { passive: true })
window.addEventListener("resize", schedule)
const observer = new ResizeObserver(schedule)
observer.observe(scroller) observer.observe(scroller)
for (const child of scroller.children) { const content = scroller.firstElementChild
if (child !== card) observer.observe(child) if (content) observer.observe(content)
return () => {
scroller.removeEventListener("scroll", schedule)
window.removeEventListener("resize", schedule)
observer.disconnect()
if (frame != null) window.cancelAnimationFrame(frame)
} }
return () => observer.disconnect() }, [anchorEl, scrollEl])
}, [anchorEl, scrollRef])
return ( return (
<div <div
@ -871,7 +1350,8 @@ function AnchoredFindingCard({
data-finding-card data-finding-card
role="dialog" role="dialog"
aria-label={finding.title} aria-label={finding.title}
className={cn(FINDING_CARD_CLASS, "absolute z-50")} style={{ visibility: "hidden" }}
className={cn(FINDING_CARD_CLASS, "fixed z-50")}
> >
<FindingCardContent detail={detail} finding={finding} onClose={onClose} /> <FindingCardContent detail={detail} finding={finding} onClose={onClose} />
</div> </div>
@ -1122,7 +1602,7 @@ function SidePanel({
<div <div
ref={panelRef} ref={panelRef}
style={{ width }} style={{ width }}
className="sticky top-0 hidden h-full shrink-0 xl:flex" className="relative hidden h-full shrink-0 xl:flex"
> >
<ReviewPanelResizeHandle width={width} onResize={setWidth} /> <ReviewPanelResizeHandle width={width} onResize={setWidth} />
<aside <aside