From ecf0898f51d06bac1fa97c9f2bca76e9fda4cf79 Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Thu, 18 Jun 2026 16:54:07 -0700 Subject: [PATCH] feat: split view, add-to-chat, virtualization + scroll/grouping perf (#1574) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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. --- ui/src/components/agents/AgentGitPanel.tsx | 65 +- ui/src/components/agents/AgentsSidebar.tsx | 25 +- ui/src/components/agents/ReviewChat.tsx | 318 +++++-- ui/src/components/agents/ReviewSidebar.tsx | 74 +- ui/src/components/agents/utils/diffUtils.ts | 65 +- ui/src/components/sidebar-layout.tsx | 33 +- .../agents/reviews/$owner.$repo.$number.tsx | 812 ++++++++++++++---- 7 files changed, 1090 insertions(+), 302 deletions(-) diff --git a/ui/src/components/agents/AgentGitPanel.tsx b/ui/src/components/agents/AgentGitPanel.tsx index 2d53c290..ccbe69a1 100644 --- a/ui/src/components/agents/AgentGitPanel.tsx +++ b/ui/src/components/agents/AgentGitPanel.tsx @@ -17,12 +17,7 @@ import { GitPullRequestIcon, SidebarSimpleIcon, } from "@phosphor-icons/react" -import type { - FileContents, - VirtualFileMetrics, - WorkerInitializationRenderOptions, - WorkerPoolOptions, -} from "@pierre/diffs/react" +import type { FileContents } from "@pierre/diffs/react" import type { GitStatus, GitStatusEntry } from "@pierre/trees" 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 { useAgentThreadPrDiff } from "@/lib/agents/queries" 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 { Z } from "@/components/agents/z-index" 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. 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 - -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 { if (typeof window === "undefined") return PANEL_DEFAULT_WIDTH 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( function FileDiffSection({ file, diff --git a/ui/src/components/agents/AgentsSidebar.tsx b/ui/src/components/agents/AgentsSidebar.tsx index 9da0504f..fecc7407 100644 --- a/ui/src/components/agents/AgentsSidebar.tsx +++ b/ui/src/components/agents/AgentsSidebar.tsx @@ -25,6 +25,7 @@ import type { ComponentType, SVGProps } from "react" import type { SessionUser } from "@/lib/api" import type { AgentSource, AgentThread } from "@/lib/agents/types" +import type { SidebarLayout } from "@/components/sidebar-layout" import { SidebarUserMenu } from "@/components/SidebarUserMenu" import { ReviewSidebarPanel, @@ -35,6 +36,7 @@ import { Button } from "@/components/ui/button" import { SidebarCollapseButton, SidebarFrame, + SidebarLayoutProvider, useSidebarLayout, } from "@/components/sidebar-layout" import { groupThreads } from "@/lib/agents/api" @@ -90,6 +92,7 @@ const PR_STATE_META: Record< interface AgentsSidebarProps { user: SessionUser activeThreadId?: string + layout: SidebarLayout } const NAV = [ @@ -98,7 +101,11 @@ const NAV = [ { to: "/agents/reviews", label: "Reviews", icon: GitPullRequestIcon }, ] as const -export function AgentsSidebar({ user, activeThreadId }: AgentsSidebarProps) { +export function AgentsSidebar({ + user, + activeThreadId, + layout, +}: AgentsSidebarProps) { const sidebar = useSidebarThreads(RESOLVED_SIDEBAR_LIMIT) const activeThreads = sidebar.data?.active.items ?? [] const resolvedThreads = sidebar.data?.resolved.items ?? [] @@ -107,7 +114,6 @@ export function AgentsSidebar({ user, activeThreadId }: AgentsSidebarProps) { useSeedAgentThreadDetails(visibleThreads, activeThreadId) useRunCompletionNotifier(visibleThreads, activeThreadId) const groups = groupThreads(activeThreads) - const layout = useSidebarLayout() const reviewSidebar = useReviewSidebarData() return ( @@ -538,12 +544,19 @@ export function AgentsShell({ activeThreadId?: string children: React.ReactNode }) { + const layout = useSidebarLayout() return ( -
- -
{children}
-
+ +
+ +
{children}
+
+ ) } diff --git a/ui/src/components/agents/ReviewChat.tsx b/ui/src/components/agents/ReviewChat.tsx index f2389434..55150fc2 100644 --- a/ui/src/components/agents/ReviewChat.tsx +++ b/ui/src/components/agents/ReviewChat.tsx @@ -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 { overrideFetchImplementation } from "@langchain/langgraph-sdk" import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query" @@ -6,6 +15,7 @@ import { ArrowClockwiseIcon, ArrowUpIcon, CheckIcon, + CodeIcon, PlusIcon, SparkleIcon, TrashIcon, @@ -22,6 +32,138 @@ import { Skeleton } from "@/components/ui/skeleton" import { api, reviewChatApiBase } from "@/lib/api" 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(null) + +export function ReviewChatComposerProvider({ + children, +}: { + children: React.ReactNode +}) { + const sinkRef = useRef<((attachment: ChatAttachment) => void) | null>(null) + const pendingRef = useRef>([]) + const value = useMemo( + () => ({ + 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 ( + + {children} + + ) +} + +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): 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 + text: string +} { + const attachments: Array = [] + 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 ( + + + {label} + {onRemove && ( + + + + )} + + ) +} + const dashboardFetch: typeof fetch = (input, init) => fetch(input, { ...init, credentials: "include" }) @@ -271,25 +413,42 @@ function ChatBody({ onUserSend: (text: string) => void expectsHistory: boolean }) { + const composer = useReviewChatComposer() const stream = useStreamContext() const [value, setValue] = useState("") - const endRef = useRef(null) + const [attachments, setAttachments] = useState>([]) + const scrollRef = useRef(null) + const autoScrollRef = useRef(true) + const prevTopRef = useRef(0) const messages = stream.messages const busy = stream.isLoading // True during the one-time getState hydration when switching to / loading an // existing thread, before its messages have arrived. const hydrating = stream.isThreadLoading + // Receive "add to chat" attachments from the diff column as composer pills. useEffect(() => { - endRef.current?.scrollIntoView({ behavior: "smooth" }) - }, [messages]) + if (!composer) return + 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( - (text: string) => { + (text: string, atts: Array) => { const trimmed = text.trim() - if (!trimmed || busy) return - onUserSend(trimmed) - void stream.submit({ messages: [{ type: "human", content: trimmed }] }) + const first = atts[0] + if ((!trimmed && !first) || busy) return + const content = serializeMessage(trimmed, atts) + onUserSend(trimmed || (first ? attachmentPillLabel(first) : "")) + void stream.submit({ messages: [{ type: "human", content }] }) }, [busy, stream, onUserSend], ) @@ -301,44 +460,85 @@ function ChatBody({ }) const submitComposer = () => { - send(value) + send(value, attachments) setValue("") + setAttachments([]) } // Show the loading placeholder (not the empty/intro state) while an existing // conversation hydrates, so a chat with messages never flashes its greeting. const showEmpty = visible.length === 0 && !busy 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 (
{showLoading ? ( ) : showEmpty ? ( - + { + send(prompt, attachments) + setAttachments([]) + }} + /> ) : ( -
+
{visible.map((message, index) => { const isUser = messageType(message) === "human" - return ( -
-
- {isUser ? ( - - {messageText(message.content)} - - ) : ( + if (!isUser) { + return ( +
+
+
+
+ ) + } + const parsed = parseUserMessage(messageText(message.content)) + return ( +
+
+ {parsed.attachments.length > 0 && ( +
+ {parsed.attachments.map((attachment, i) => ( + + ))} +
+ )} + {parsed.text && ( + + {parsed.text} + )}
@@ -351,34 +551,46 @@ function ChatBody({
)} -
)}
-
-