diff --git a/src/ui/App.tsx b/src/ui/App.tsx index f06ce7c5..74f751ee 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -718,6 +718,7 @@ export function App({ scrollCodeHorizontally(delta * FAST_CODE_HORIZONTAL_SCROLL_COLUMNS); }} onSelectFile={jumpToFile} + onViewportCenteredHunkChange={review.selectHunk} /> diff --git a/src/ui/AppHost.interactions.test.tsx b/src/ui/AppHost.interactions.test.tsx index a0a3f524..4b3af375 100644 --- a/src/ui/AppHost.interactions.test.tsx +++ b/src/ui/AppHost.interactions.test.tsx @@ -5,7 +5,11 @@ import { describe, expect, mock, test } from "bun:test"; import { testRender } from "@opentui/react/test-utils"; import { act } from "react"; import type { HunkHostClient } from "../mcp/client"; -import type { HunkSessionRegistration, SessionServerMessage } from "../mcp/types"; +import type { + HunkSessionRegistration, + HunkSessionSnapshot, + SessionServerMessage, +} from "../mcp/types"; import type { AppBootstrap, LayoutMode } from "../core/types"; import { createTestGitAppBootstrap } from "../../test/helpers/app-bootstrap"; import { createTestDiffFile as buildTestDiffFile, lines } from "../../test/helpers/diff-helpers"; @@ -41,6 +45,7 @@ function createMockHostClient() { type Bridge = Parameters[0]; let bridge: Bridge = null; + let latestSnapshot: HunkSessionSnapshot | null = null; const registration: HunkSessionRegistration = { sessionId: "session-1", pid: process.pid, @@ -59,9 +64,12 @@ function createMockHostClient() { setBridge: (nextBridge: Bridge) => { bridge = nextBridge; }, - updateSnapshot: () => {}, + updateSnapshot: (snapshot: HunkSessionSnapshot) => { + latestSnapshot = snapshot; + }, } as unknown as HunkHostClient, getBridge: () => bridge, + getLatestSnapshot: () => latestSnapshot, navigateToHunk: async ( input: Extract["input"], ) => { @@ -226,6 +234,40 @@ function createTwoFileHunkBootstrap(): AppBootstrap { }); } +function createMouseScrollSelectionBootstrap(): AppBootstrap { + const firstBeforeLines = createNumberedAssignmentLines(1, 12); + const secondBeforeLines = Array.from( + { length: 90 }, + (_, index) => `export const line${String(index + 13).padStart(2, "0")} = ${index + 13};`, + ); + const secondAfterLines = [...secondBeforeLines]; + + secondAfterLines[0] = "export const line13 = 1300;"; + secondAfterLines[59] = "export const line72 = 7200;"; + secondAfterLines[60] = "export const line73 = 7300;"; + secondAfterLines[61] = "export const line74 = 7400;"; + + return createTestGitAppBootstrap({ + changesetId: "changeset:mouse-scroll-selection", + files: [ + createTestDiffFile( + "first", + "first.ts", + lines(...firstBeforeLines), + lines("export const line01 = 101;", ...createNumberedAssignmentLines(2, 11)), + true, + ), + createTestDiffFile( + "second", + "second.ts", + lines(...secondBeforeLines), + lines(...secondAfterLines), + true, + ), + ], + }); +} + function createCollapsedTopBootstrap(): AppBootstrap { const beforeLines = Array.from( { length: 400 }, @@ -293,6 +335,29 @@ async function waitForFrame( return frame; } +async function waitForSnapshot( + setup: Awaited>, + getSnapshot: () => HunkSessionSnapshot | null, + predicate: (snapshot: HunkSessionSnapshot) => boolean, + attempts = 8, +) { + let snapshot = getSnapshot(); + + for (let attempt = 0; attempt < attempts; attempt += 1) { + if (snapshot && predicate(snapshot)) { + return snapshot; + } + + await act(async () => { + await Bun.sleep(30); + await setup.renderOnce(); + }); + snapshot = getSnapshot(); + } + + return snapshot; +} + function firstVisibleAddedLine(frame: string) { return frame.match(/line\d{2} = 1\d{2}/)?.[0] ?? null; } @@ -1669,6 +1734,55 @@ describe("App interactions", () => { } }); + test("mouse wheel scrolling updates the active file and hunk to the viewport center", async () => { + const { getLatestSnapshot, hostClient } = createMockHostClient(); + const setup = await testRender( + , + { + width: 220, + height: 12, + }, + ); + + try { + await flush(setup); + + expect(getLatestSnapshot()).toMatchObject({ + selectedFilePath: "first.ts", + selectedHunkIndex: 0, + }); + + let snapshot = getLatestSnapshot(); + for (let index = 0; index < 24; index += 1) { + await act(async () => { + await setup.mockMouse.scroll(120, 7, "down"); + }); + await flush(setup); + + snapshot = await waitForSnapshot( + setup, + getLatestSnapshot, + (currentSnapshot) => + currentSnapshot.selectedFilePath === "second.ts" && + currentSnapshot.selectedHunkIndex === 1, + 4, + ); + if (snapshot?.selectedFilePath === "second.ts" && snapshot.selectedHunkIndex === 1) { + break; + } + } + + expect(snapshot).toMatchObject({ + selectedFilePath: "second.ts", + selectedHunkIndex: 1, + }); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("clicking a sidebar file makes that file own the top of the review pane", async () => { const setup = await testRender(, { width: 220, diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index d22c6586..7e16ddcc 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -20,15 +20,19 @@ import { import { buildFileSectionLayouts, buildInStreamFileHeaderHeights, + collectIntersectingFileSectionIds, findHeaderOwningFileSection, shouldRenderInStreamFileHeader, + type FileSectionLayout, } from "../../lib/fileSectionLayout"; import { diffHunkId, diffSectionId } from "../../lib/ids"; +import { findViewportCenteredHunkTarget } from "../../lib/viewportSelection"; import type { AppTheme } from "../../themes"; import { DiffSection } from "./DiffSection"; import { DiffFileHeaderRow } from "./DiffFileHeaderRow"; import { DiffSectionPlaceholder } from "./DiffSectionPlaceholder"; import { VerticalScrollbar, type VerticalScrollbarHandle } from "../scrollbar/VerticalScrollbar"; +import { prefetchHighlightedDiff } from "../../diff/useHighlightedDiff"; const EMPTY_VISIBLE_AGENT_NOTES: VisibleAgentNote[] = []; @@ -125,6 +129,75 @@ function resolveViewportRowAnchorTop( return 0; } +/** Keep syntax-highlight warm for the files immediately adjacent to the current selection. */ +function buildAdjacentPrefetchFileIds(files: DiffFile[], selectedFileId?: string) { + if (!selectedFileId) { + return new Set(); + } + + const selectedIndex = files.findIndex((file) => file.id === selectedFileId); + if (selectedIndex < 0) { + return new Set(); + } + + const next = new Set(); + const previousFile = files[selectedIndex - 1]; + const nextFile = files[selectedIndex + 1]; + + if (previousFile) { + next.add(previousFile.id); + } + + if (nextFile) { + next.add(nextFile.id); + } + + return next; +} + +/** + * Start highlight work before files visibly enter the review stream. + * + * We intentionally include three groups: + * - the selected file, so direct navigation always warms the active target + * - adjacent files, so hunk/file navigation does not wait on a cold highlight + * - files within a larger viewport halo, so wheel/track scrolling sees colorized rows already ready + */ +function buildHighlightPrefetchFileIds({ + adjacentPrefetchFileIds, + fileSectionLayouts, + scrollTop, + viewportHeight, + selectedFileId, +}: { + adjacentPrefetchFileIds: Set; + fileSectionLayouts: FileSectionLayout[]; + scrollTop: number; + viewportHeight: number; + selectedFileId?: string; +}) { + const next = new Set(adjacentPrefetchFileIds); + + if (selectedFileId) { + next.add(selectedFileId); + } + + const clampedViewportHeight = Math.max(1, viewportHeight); + const prefetchRows = Math.max(24, clampedViewportHeight * 3); + const minPrefetchY = Math.max(0, scrollTop - prefetchRows); + const maxPrefetchY = scrollTop + viewportHeight + prefetchRows; + + for (const fileId of collectIntersectingFileSectionIds( + fileSectionLayouts, + minPrefetchY, + maxPrefetchY, + )) { + next.add(fileId); + } + + return next; +} + /** Render the main multi-file review stream. */ export function DiffPane({ codeHorizontalOffset = 0, @@ -150,6 +223,7 @@ export function DiffPane({ onOpenAgentNotesAtHunk, onScrollCodeHorizontally = () => {}, onSelectFile, + onViewportCenteredHunkChange, }: { codeHorizontalOffset?: number; diffContentWidth: number; @@ -174,48 +248,14 @@ export function DiffPane({ onOpenAgentNotesAtHunk: (fileId: string, hunkIndex: number) => void; onScrollCodeHorizontally?: (delta: number) => void; onSelectFile: (fileId: string) => void; + onViewportCenteredHunkChange?: (fileId: string, hunkIndex: number) => void; }) { const renderer = useRenderer(); - const [prefetchAnchorKey, setPrefetchAnchorKey] = useState(null); - const selectedHighlightKey = selectedFileId ? `${theme.appearance}:${selectedFileId}` : null; - - useEffect(() => { - setPrefetchAnchorKey(null); - }, [selectedHighlightKey]); - // Hold background prefetches until the currently selected file has painted once. - const adjacentPrefetchFileIds = useMemo(() => { - if (!selectedHighlightKey || prefetchAnchorKey !== selectedHighlightKey || !selectedFileId) { - return new Set(); - } - - const selectedIndex = files.findIndex((file) => file.id === selectedFileId); - if (selectedIndex < 0) { - return new Set(); - } - - const next = new Set(); - const previousFile = files[selectedIndex - 1]; - const nextFile = files[selectedIndex + 1]; - - if (previousFile) { - next.add(previousFile.id); - } - - if (nextFile) { - next.add(nextFile.id); - } - - return next; - }, [files, prefetchAnchorKey, selectedFileId, selectedHighlightKey]); - - const handleSelectedHighlightReady = useCallback(() => { - if (!selectedHighlightKey) { - return; - } - - setPrefetchAnchorKey((current) => current ?? selectedHighlightKey); - }, [selectedHighlightKey]); + const adjacentPrefetchFileIds = useMemo( + () => buildAdjacentPrefetchFileIds(files, selectedFileId), + [files, selectedFileId], + ); /** Route shifted wheel input into horizontal code-column scrolling without disturbing vertical review scroll. */ const handleMouseScroll = useCallback( @@ -295,7 +335,51 @@ export function DiffPane({ const previousSelectedFileTopAlignRequestIdRef = useRef(selectedFileTopAlignRequestId); const suppressNextSelectionAutoScrollRef = useRef(false); const pendingFileTopAlignFileIdRef = useRef(null); + const mouseScrollSelectionSyncActiveRef = useRef(false); + const mouseScrollSelectionSyncTimeoutRef = useRef | null>(null); + + const armMouseScrollSelectionSync = useCallback(() => { + mouseScrollSelectionSyncActiveRef.current = true; + if (mouseScrollSelectionSyncTimeoutRef.current) { + clearTimeout(mouseScrollSelectionSyncTimeoutRef.current); + } + mouseScrollSelectionSyncTimeoutRef.current = setTimeout(() => { + mouseScrollSelectionSyncActiveRef.current = false; + mouseScrollSelectionSyncTimeoutRef.current = null; + }, 120); + }, []); + + /** + * Combine the existing horizontal-wheel handler with viewport-centered selection sync. + * Horizontal gestures should keep their current behavior without changing the active hunk. + */ + const handleDiffPaneMouseScroll = useCallback( + (event: TuiMouseEvent) => { + const direction = event.scroll?.direction; + const isHorizontalGesture = + direction === "left" || + direction === "right" || + (event.modifiers.shift && (direction === "up" || direction === "down")); + + if (!isHorizontalGesture) { + armMouseScrollSelectionSync(); + } + + handleMouseScroll(event); + }, + [armMouseScrollSelectionSync, handleMouseScroll], + ); + useEffect(() => { + return () => { + if (mouseScrollSelectionSyncTimeoutRef.current) { + clearTimeout(mouseScrollSelectionSyncTimeoutRef.current); + } + }; + }, []); + + // Mirror the imperative OpenTUI scrollbox state into React state so geometry planning, + // windowing, pinned-header ownership, and prefetching can all read the same viewport snapshot. useEffect(() => { const scrollBox = scrollRef.current; if (!scrollBox) { @@ -366,11 +450,7 @@ export function DiffPane({ const overscanRows = 8; const minVisibleY = Math.max(0, scrollViewport.top - overscanRows); const maxVisibleY = scrollViewport.top + scrollViewport.height + overscanRows; - return new Set( - baseFileSectionLayouts - .filter((metric) => metric.sectionBottom >= minVisibleY && metric.sectionTop <= maxVisibleY) - .map((metric) => metric.fileId), - ); + return collectIntersectingFileSectionIds(baseFileSectionLayouts, minVisibleY, maxVisibleY); }, [baseFileSectionLayouts, scrollViewport.height, scrollViewport.top]); const visibleAgentNotesByFile = useMemo(() => { @@ -437,10 +517,91 @@ export function DiffPane({ [estimatedBodyHeights, files, sectionHeaderHeights], ); const totalContentHeight = fileSectionLayouts[fileSectionLayouts.length - 1]?.sectionBottom ?? 0; + + const highlightPrefetchFileIds = useMemo( + () => + buildHighlightPrefetchFileIds({ + adjacentPrefetchFileIds, + fileSectionLayouts, + scrollTop: scrollViewport.top, + viewportHeight: scrollViewport.height, + selectedFileId, + }), + [ + adjacentPrefetchFileIds, + fileSectionLayouts, + scrollViewport.height, + scrollViewport.top, + selectedFileId, + ], + ); + + // Kick off highlight work from viewport planning rather than waiting for the section to mount. + // That avoids the "plain rows first, color later" stutter when a file is about to scroll onscreen. + useEffect(() => { + if (files.length === 0) { + return; + } + + for (const file of files) { + if (!highlightPrefetchFileIds.has(file.id)) { + continue; + } + + void prefetchHighlightedDiff({ + file, + appearance: theme.appearance, + }); + } + }, [files, highlightPrefetchFileIds, theme.appearance]); + // Read the live scroll box position during render so pinned-header ownership flips // immediately after imperative scrolls instead of waiting for the polled viewport snapshot. const effectiveScrollTop = scrollRef.current?.scrollTop ?? scrollViewport.top; + // While the wheel is actively scrolling, selection follows the viewport center instead of + // driving the viewport. That keeps sidebar state in sync without introducing scroll snap-back. + useLayoutEffect(() => { + if ( + !onViewportCenteredHunkChange || + !mouseScrollSelectionSyncActiveRef.current || + files.length === 0 || + scrollViewport.height <= 0 + ) { + return; + } + + const centeredTarget = findViewportCenteredHunkTarget({ + files, + fileSectionLayouts, + sectionGeometry, + scrollTop: scrollViewport.top, + viewportHeight: scrollViewport.height, + }); + if (!centeredTarget) { + return; + } + + if ( + centeredTarget.fileId === selectedFileId && + centeredTarget.hunkIndex === selectedHunkIndex + ) { + return; + } + + suppressNextSelectionAutoScrollRef.current = true; + onViewportCenteredHunkChange(centeredTarget.fileId, centeredTarget.hunkIndex); + }, [ + fileSectionLayouts, + files, + onViewportCenteredHunkChange, + scrollViewport.height, + scrollViewport.top, + sectionGeometry, + selectedFileId, + selectedHunkIndex, + ]); + const pinnedHeaderFile = useMemo(() => { if (files.length === 0) { return null; @@ -702,9 +863,10 @@ export function DiffPane({ useLayoutEffect(() => { const pinnedHeaderFileId = pinnedHeaderFile?.id ?? null; - if (suppressNextSelectionAutoScrollRef.current) { + if (mouseScrollSelectionSyncActiveRef.current || suppressNextSelectionAutoScrollRef.current) { suppressNextSelectionAutoScrollRef.current = false; - // Consume this selection transition so the next render does not re-center the selected hunk. + // While the mouse wheel owns the viewport, selection should follow passively instead of + // snapping the review stream back toward the newly selected hunk. prevSelectedAnchorIdRef.current = selectedAnchorId; prevPinnedHeaderFileIdRef.current = pinnedHeaderFileId; pendingSelectionSettleRef.current = false; @@ -867,13 +1029,13 @@ export function DiffPane({ scrollY={true} viewportCulling={true} focused={pagerMode} + onMouseScroll={handleDiffPaneMouseScroll} rootOptions={{ backgroundColor: theme.panel }} wrapperOptions={{ backgroundColor: theme.panel }} viewportOptions={{ backgroundColor: theme.panel }} contentOptions={{ backgroundColor: theme.panel }} verticalScrollbarOptions={{ visible: false }} horizontalScrollbarOptions={{ visible: false }} - onMouseScroll={handleMouseScroll} > {files.map((file, index) => { const shouldRenderSection = visibleWindowedFileIds?.has(file.id) ?? true; - const shouldPrefetchVisibleHighlight = - Boolean(selectedHighlightKey) && - prefetchAnchorKey === selectedHighlightKey && - visibleViewportFileIds.has(file.id); // Windowing keeps offscreen files cheap: render placeholders with identical // section geometry so scroll math and pinned-header ownership stay stable. @@ -916,14 +1074,7 @@ export function DiffPane({ headerStatsWidth={headerStatsWidth} layout={layout} selectedHunkIndex={file.id === selectedFileId ? selectedHunkIndex : -1} - shouldLoadHighlight={ - file.id === selectedFileId || - adjacentPrefetchFileIds.has(file.id) || - shouldPrefetchVisibleHighlight - } - onHighlightReady={ - file.id === selectedFileId ? handleSelectedHighlightReady : undefined - } + shouldLoadHighlight={highlightPrefetchFileIds.has(file.id)} separatorWidth={separatorWidth} showHeader={shouldRenderInStreamFileHeader(index)} showSeparator={index > 0} diff --git a/src/ui/components/panes/DiffSection.tsx b/src/ui/components/panes/DiffSection.tsx index e92a1799..321dc2b8 100644 --- a/src/ui/components/panes/DiffSection.tsx +++ b/src/ui/components/panes/DiffSection.tsx @@ -15,7 +15,6 @@ interface DiffSectionProps { layout: Exclude; selectedHunkIndex: number; shouldLoadHighlight: boolean; - onHighlightReady?: () => void; separatorWidth: number; showLineNumbers: boolean; showHunkHeaders: boolean; @@ -38,7 +37,6 @@ function DiffSectionComponent({ layout, selectedHunkIndex, shouldLoadHighlight, - onHighlightReady, separatorWidth, showLineNumbers, showHunkHeaders, @@ -99,7 +97,6 @@ function DiffSectionComponent({ annotatedHunkIndices={annotatedHunkIndices} visibleAgentNotes={visibleAgentNotes} onOpenAgentNotesAtHunk={onOpenAgentNotesAtHunk} - onHighlightReady={onHighlightReady} selectedHunkIndex={selectedHunkIndex} shouldLoadHighlight={shouldLoadHighlight} // The parent review stream owns scrolling across files. diff --git a/src/ui/components/ui-components.test.tsx b/src/ui/components/ui-components.test.tsx index ff2f9192..ed865f66 100644 --- a/src/ui/components/ui-components.test.tsx +++ b/src/ui/components/ui-components.test.tsx @@ -65,6 +65,30 @@ function createWindowingFiles(count: number) { ); } +function createHighlightPrefetchWindowFiles() { + return Array.from({ length: 4 }, (_, index) => { + const marker = `prefetchMarker${index + 1}`; + const before = lines( + `export const ${marker} = ${index + 1};`, + ...Array.from( + { length: 8 }, + (_, lineIndex) => + `export function keep${index + 1}_${lineIndex}(value: number) { return value + ${lineIndex}; }`, + ), + ); + const after = lines( + `export const ${marker} = ${index + 100};`, + ...Array.from( + { length: 8 }, + (_, lineIndex) => + `export function keep${index + 1}_${lineIndex}(value: number) { return value * ${lineIndex + 2}; }`, + ), + ); + + return createTestDiffFile(`prefetch-${index + 1}`, `prefetch-${index + 1}.ts`, before, after); + }); +} + function createMultiHunkDiffFile(id: string, path: string) { const before = lines( "export const line1 = 1;", @@ -1734,6 +1758,64 @@ describe("UI components", () => { } }); + test("DiffPane prefetches highlight data for files approaching the viewport before they mount", async () => { + const files = createHighlightPrefetchWindowFiles(); + const theme = resolveTheme("midnight", null); + const setup = await testRender( + , + { width: 100, height: 10 }, + ); + const thirdFileCheck = await testRender( + , + { width: 184, height: 10 }, + ); + + try { + await settleDiffPane(setup); + + const initialFrame = setup.captureCharFrame(); + expect(initialFrame).not.toContain("prefetch-3.ts"); + + let prefetched = false; + for (let iteration = 0; iteration < 400; iteration += 1) { + await act(async () => { + await setup.renderOnce(); + await thirdFileCheck.renderOnce(); + await Bun.sleep(0); + await setup.renderOnce(); + await thirdFileCheck.renderOnce(); + await Bun.sleep(0); + }); + + if (frameHasHighlightedMarker(thirdFileCheck.captureSpans(), "prefetchMarker3")) { + prefetched = true; + break; + } + } + + expect(prefetched).toBe(true); + } finally { + await act(async () => { + thirdFileCheck.renderer.destroy(); + setup.renderer.destroy(); + }); + } + }); + test("App renders the menu bar, multi-file stream, and AI badges", async () => { const bootstrap = createBootstrap(); const frame = await captureFrame(, 280, 24); diff --git a/src/ui/diff/PierreDiffView.tsx b/src/ui/diff/PierreDiffView.tsx index 02e67197..21c394ac 100644 --- a/src/ui/diff/PierreDiffView.tsx +++ b/src/ui/diff/PierreDiffView.tsx @@ -21,7 +21,6 @@ export function PierreDiffView({ file, layout, onOpenAgentNotesAtHunk, - onHighlightReady, showLineNumbers = true, showHunkHeaders = true, wrapLines = false, @@ -37,7 +36,6 @@ export function PierreDiffView({ file: DiffFile | undefined; layout: Exclude; onOpenAgentNotesAtHunk?: (hunkIndex: number) => void; - onHighlightReady?: () => void; showLineNumbers?: boolean; showHunkHeaders?: boolean; wrapLines?: boolean; @@ -51,7 +49,6 @@ export function PierreDiffView({ const resolvedHighlighted = useHighlightedDiff({ file, appearance: theme.appearance, - onHighlightReady, shouldLoadHighlight, }); diff --git a/src/ui/diff/useHighlightedDiff.ts b/src/ui/diff/useHighlightedDiff.ts index c0230a14..e82fcc16 100644 --- a/src/ui/diff/useHighlightedDiff.ts +++ b/src/ui/diff/useHighlightedDiff.ts @@ -1,4 +1,4 @@ -import { useEffect, useLayoutEffect, useMemo, useRef, useState } from "react"; +import { useLayoutEffect, useState } from "react"; import type { DiffFile } from "../../core/types"; import { loadHighlightedDiff, type HighlightedDiffCode } from "./pierre"; @@ -43,49 +43,96 @@ function commitHighlightResult( if (SHARED_HIGHLIGHT_PROMISES.get(cacheKey) !== promise) { return false; } + SHARED_HIGHLIGHT_PROMISES.delete(cacheKey); SHARED_HIGHLIGHTED_DIFF_CACHE.set(cacheKey, result); enforceCacheLimit(); return true; } +/** Start one shared highlight request unless the cache or an in-flight promise already has it. */ +function ensureHighlightedDiffLoaded( + file: DiffFile, + appearance: "light" | "dark", + cacheKey = buildCacheKey(appearance, file), +) { + const cached = SHARED_HIGHLIGHTED_DIFF_CACHE.get(cacheKey); + if (cached) { + return Promise.resolve(cached); + } + + const existing = SHARED_HIGHLIGHT_PROMISES.get(cacheKey); + if (existing) { + return existing; + } + + let pending: Promise; + pending = loadHighlightedDiff(file, appearance) + .then((nextHighlighted) => { + commitHighlightResult(cacheKey, pending, nextHighlighted); + return nextHighlighted; + }) + .catch(() => { + const fallback = { + deletionLines: [], + additionLines: [], + } satisfies HighlightedDiffCode; + commitHighlightResult(cacheKey, pending, fallback); + return fallback; + }); + + SHARED_HIGHLIGHT_PROMISES.set(cacheKey, pending); + return pending; +} + +/** Queue syntax highlighting for one file without mounting its diff rows first. */ +export function prefetchHighlightedDiff({ + file, + appearance, +}: { + file: DiffFile; + appearance: "light" | "dark"; +}) { + return ensureHighlightedDiffLoaded(file, appearance); +} + +/** Read the best already-available highlight result without starting async work during render. */ +function resolveHighlightedSnapshot({ + appearanceCacheKey, + highlighted, + highlightedCacheKey, +}: { + appearanceCacheKey: string | null; + highlighted: HighlightedDiffCode | null; + highlightedCacheKey: string | null; +}) { + if (!appearanceCacheKey) { + return null; + } + + if (highlightedCacheKey === appearanceCacheKey) { + return highlighted; + } + + return SHARED_HIGHLIGHTED_DIFF_CACHE.get(appearanceCacheKey) ?? null; +} + /** Resolve highlighted diff content with shared caching and background prefetch support. */ export function useHighlightedDiff({ file, appearance, - onHighlightReady, shouldLoadHighlight, }: { file: DiffFile | undefined; appearance: "light" | "dark"; - onHighlightReady?: () => void; shouldLoadHighlight?: boolean; }) { const [highlighted, setHighlighted] = useState(null); const [highlightedCacheKey, setHighlightedCacheKey] = useState(null); const appearanceCacheKey = file ? buildCacheKey(appearance, file) : null; - // Selected files load immediately; background prefetch can opt neighboring files in later. - const pendingHighlight = useMemo(() => { - if ( - !shouldLoadHighlight || - !file || - !appearanceCacheKey || - SHARED_HIGHLIGHTED_DIFF_CACHE.has(appearanceCacheKey) - ) { - return null; - } - - const existing = SHARED_HIGHLIGHT_PROMISES.get(appearanceCacheKey); - if (existing) { - return existing; - } - - const pending = loadHighlightedDiff(file, appearance); - SHARED_HIGHLIGHT_PROMISES.set(appearanceCacheKey, pending); - return pending; - }, [appearance, appearanceCacheKey, file, shouldLoadHighlight]); - + // Use a layout effect so a newly available cached result can replace the plain-text fallback + // before the next diff paint whenever possible. That reduces flash/stutter as files enter view. useLayoutEffect(() => { if (!file || !appearanceCacheKey) { setHighlighted(null); @@ -111,63 +158,24 @@ export function useHighlightedDiff({ let cancelled = false; setHighlighted(null); - // Capture the key and promise reference this effect was started for so the - // resolution callback only writes if it is still the active request. - const effectCacheKey = appearanceCacheKey; - const effectPromise = pendingHighlight; - - effectPromise - ?.then((nextHighlighted) => { - if (cancelled) { - return; - } - - if (commitHighlightResult(effectCacheKey, effectPromise, nextHighlighted)) { - setHighlighted(nextHighlighted); - setHighlightedCacheKey(effectCacheKey); - } - }) - .catch(() => { - if (cancelled) { - return; - } - - const fallback = { - deletionLines: [], - additionLines: [], - } satisfies HighlightedDiffCode; - if (commitHighlightResult(effectCacheKey, effectPromise, fallback)) { - setHighlighted(fallback); - setHighlightedCacheKey(effectCacheKey); - } - }); + ensureHighlightedDiffLoaded(file, appearance, appearanceCacheKey).then((nextHighlighted) => { + if (cancelled) { + return; + } + + setHighlighted(nextHighlighted); + setHighlightedCacheKey(appearanceCacheKey); + }); return () => { cancelled = true; }; - }, [appearanceCacheKey, file, highlightedCacheKey, pendingHighlight, shouldLoadHighlight]); + }, [appearance, appearanceCacheKey, file, highlightedCacheKey, shouldLoadHighlight]); // Prefer cached highlights during render so revisiting a file can paint immediately. - const resolvedHighlighted = - appearanceCacheKey && highlightedCacheKey === appearanceCacheKey - ? highlighted - : appearanceCacheKey - ? (SHARED_HIGHLIGHTED_DIFF_CACHE.get(appearanceCacheKey) ?? null) - : null; - const notifiedHighlightKeyRef = useRef(null); - - useEffect(() => { - if (!onHighlightReady || !appearanceCacheKey || !resolvedHighlighted) { - return; - } - - if (notifiedHighlightKeyRef.current === appearanceCacheKey) { - return; - } - - notifiedHighlightKeyRef.current = appearanceCacheKey; - onHighlightReady(); - }, [appearanceCacheKey, onHighlightReady, resolvedHighlighted]); - - return resolvedHighlighted; + return resolveHighlightedSnapshot({ + appearanceCacheKey, + highlighted, + highlightedCacheKey, + }); } diff --git a/src/ui/lib/fileSectionLayout.test.ts b/src/ui/lib/fileSectionLayout.test.ts new file mode 100644 index 00000000..bd0f0041 --- /dev/null +++ b/src/ui/lib/fileSectionLayout.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, test } from "bun:test"; +import { + collectIntersectingFileSectionIds, + findFileSectionAtOffset, + type FileSectionLayout, +} from "./fileSectionLayout"; + +const layouts: FileSectionLayout[] = [ + { + fileId: "alpha", + sectionIndex: 0, + sectionTop: 0, + headerTop: 0, + bodyTop: 0, + bodyHeight: 5, + sectionBottom: 5, + }, + { + fileId: "beta", + sectionIndex: 1, + sectionTop: 5, + headerTop: 6, + bodyTop: 7, + bodyHeight: 4, + sectionBottom: 11, + }, + { + fileId: "gamma", + sectionIndex: 2, + sectionTop: 11, + headerTop: 12, + bodyTop: 13, + bodyHeight: 6, + sectionBottom: 19, + }, +]; + +describe("fileSectionLayout helpers", () => { + test("findFileSectionAtOffset returns the containing section and clamps past the ends", () => { + expect(findFileSectionAtOffset([], 3)).toBeNull(); + expect(findFileSectionAtOffset(layouts, -5)?.fileId).toBe("alpha"); + expect(findFileSectionAtOffset(layouts, 4)?.fileId).toBe("alpha"); + expect(findFileSectionAtOffset(layouts, 5)?.fileId).toBe("beta"); + expect(findFileSectionAtOffset(layouts, 10)?.fileId).toBe("beta"); + expect(findFileSectionAtOffset(layouts, 11)?.fileId).toBe("gamma"); + expect(findFileSectionAtOffset(layouts, 99)?.fileId).toBe("gamma"); + }); + + test("collectIntersectingFileSectionIds returns every file whose section overlaps the range", () => { + expect(Array.from(collectIntersectingFileSectionIds(layouts, 6, 10))).toEqual(["beta"]); + expect(Array.from(collectIntersectingFileSectionIds(layouts, 4, 12))).toEqual([ + "alpha", + "beta", + "gamma", + ]); + expect(Array.from(collectIntersectingFileSectionIds(layouts, 20, 24))).toEqual([]); + }); +}); diff --git a/src/ui/lib/fileSectionLayout.ts b/src/ui/lib/fileSectionLayout.ts index 622049b7..9452450a 100644 --- a/src/ui/lib/fileSectionLayout.ts +++ b/src/ui/lib/fileSectionLayout.ts @@ -60,6 +60,59 @@ export function buildFileSectionLayouts( return layouts; } +/** Find the file section covering one absolute review-stream row. */ +export function findFileSectionAtOffset(fileSectionLayouts: FileSectionLayout[], offset: number) { + if (fileSectionLayouts.length === 0) { + return null; + } + + const firstSection = fileSectionLayouts[0]!; + const lastSection = fileSectionLayouts[fileSectionLayouts.length - 1]!; + + if (offset <= firstSection.sectionTop) { + return firstSection; + } + + if (offset >= lastSection.sectionBottom) { + return lastSection; + } + + let low = 0; + let high = fileSectionLayouts.length - 1; + + while (low <= high) { + const mid = (low + high) >>> 1; + const layout = fileSectionLayouts[mid]!; + + if (offset < layout.sectionTop) { + high = mid - 1; + } else if (offset >= layout.sectionBottom) { + low = mid + 1; + } else { + return layout; + } + } + + return lastSection; +} + +/** Collect every file section that intersects one absolute review-stream range. */ +export function collectIntersectingFileSectionIds( + fileSectionLayouts: FileSectionLayout[], + minY: number, + maxY: number, +) { + const next = new Set(); + + for (const layout of fileSectionLayouts) { + if (layout.sectionBottom >= minY && layout.sectionTop <= maxY) { + next.add(layout.fileId); + } + } + + return next; +} + /** Return the file section that owns the viewport top, switching at each next header row. */ export function findHeaderOwningFileSection( fileSectionLayouts: FileSectionLayout[], diff --git a/src/ui/lib/viewportSelection.test.ts b/src/ui/lib/viewportSelection.test.ts new file mode 100644 index 00000000..c0b6a731 --- /dev/null +++ b/src/ui/lib/viewportSelection.test.ts @@ -0,0 +1,92 @@ +import { describe, expect, test } from "bun:test"; +import { createTestDiffFile, lines } from "../../../test/helpers/diff-helpers"; +import { measureDiffSectionGeometry } from "./diffSectionGeometry"; +import { buildFileSectionLayouts, buildInStreamFileHeaderHeights } from "./fileSectionLayout"; +import { findViewportCenteredHunkTarget } from "./viewportSelection"; +import { resolveTheme } from "../themes"; + +function createWideTwoHunkFile(id: string, path: string, start = 1) { + const beforeLines = Array.from( + { length: 80 }, + (_, index) => `export const line${start + index} = ${start + index};`, + ); + const afterLines = [...beforeLines]; + + afterLines[0] = `export const line${start} = ${start + 1000};`; + afterLines[59] = `export const line${start + 59} = ${start + 5900};`; + + return createTestDiffFile({ + after: lines(...afterLines), + before: lines(...beforeLines), + context: 3, + id, + path, + }); +} + +function scrollTopForCenter(centerOffset: number, viewportHeight: number) { + return Math.max(0, centerOffset - Math.max(0, Math.floor((viewportHeight - 1) / 2))); +} + +describe("findViewportCenteredHunkTarget", () => { + const theme = resolveTheme("midnight", null); + + test("switches the active file when the viewport center enters a later file", () => { + const firstFile = createTestDiffFile({ + after: "export const alpha = 2;\n", + before: "export const alpha = 1;\n", + id: "first", + path: "first.ts", + }); + const secondFile = createWideTwoHunkFile("second", "second.ts", 100); + const files = [firstFile, secondFile]; + const sectionGeometry = files.map((file) => + measureDiffSectionGeometry(file, "split", true, theme, [], 160, true, false), + ); + const fileSectionLayouts = buildFileSectionLayouts( + files, + sectionGeometry.map((geometry) => geometry.bodyHeight), + buildInStreamFileHeaderHeights(files), + ); + const viewportHeight = 7; + const secondFileFirstHunkTop = + fileSectionLayouts[1]!.bodyTop + sectionGeometry[1]!.hunkBounds.get(0)!.top; + + expect( + findViewportCenteredHunkTarget({ + files, + fileSectionLayouts, + sectionGeometry, + scrollTop: scrollTopForCenter(secondFileFirstHunkTop, viewportHeight), + viewportHeight, + }), + ).toEqual({ fileId: "second", hunkIndex: 0 }); + }); + + test("picks the nearest hunk when the viewport center lands in a collapsed gap", () => { + const file = createWideTwoHunkFile("gap", "gap.ts"); + const geometry = measureDiffSectionGeometry(file, "split", true, theme, [], 160, true, false); + const viewportHeight = 7; + const fileSectionLayouts = buildFileSectionLayouts( + [file], + [geometry.bodyHeight], + buildInStreamFileHeaderHeights([file]), + ); + const firstHunk = geometry.hunkBounds.get(0)!; + const secondHunk = geometry.hunkBounds.get(1)!; + const centeredGapOffset = Math.max(firstHunk.top + firstHunk.height, secondHunk.top - 1); + + expect( + findViewportCenteredHunkTarget({ + files: [file], + fileSectionLayouts, + sectionGeometry: [geometry], + scrollTop: scrollTopForCenter( + fileSectionLayouts[0]!.bodyTop + centeredGapOffset, + viewportHeight, + ), + viewportHeight, + }), + ).toEqual({ fileId: "gap", hunkIndex: 1 }); + }); +}); diff --git a/src/ui/lib/viewportSelection.ts b/src/ui/lib/viewportSelection.ts new file mode 100644 index 00000000..53f99f6b --- /dev/null +++ b/src/ui/lib/viewportSelection.ts @@ -0,0 +1,87 @@ +import type { DiffFile } from "../../core/types"; +import type { DiffSectionGeometry } from "./diffSectionGeometry"; +import { findFileSectionAtOffset, type FileSectionLayout } from "./fileSectionLayout"; + +export interface ViewportCenteredHunkTarget { + fileId: string; + hunkIndex: number; +} + +/** Pick the hunk nearest one vertical offset within a file body. */ +function findNearestHunkIndexAtBodyOffset( + sectionGeometry: DiffSectionGeometry | undefined, + bodyOffset: number, + hunkCount: number, +) { + if (!sectionGeometry || hunkCount <= 1 || sectionGeometry.hunkBounds.size === 0) { + return 0; + } + + let nearestHunkIndex = 0; + let nearestDistance = Number.POSITIVE_INFINITY; + + for (let hunkIndex = 0; hunkIndex < hunkCount; hunkIndex += 1) { + const hunkBounds = sectionGeometry.hunkBounds.get(hunkIndex); + if (!hunkBounds) { + continue; + } + + const hunkBottom = hunkBounds.top + hunkBounds.height - 1; + if (bodyOffset >= hunkBounds.top && bodyOffset <= hunkBottom) { + return hunkIndex; + } + + const distance = + bodyOffset < hunkBounds.top ? hunkBounds.top - bodyOffset : bodyOffset - hunkBottom; + + // Favor the later hunk on exact ties so ownership hands off when the midpoint reaches center. + if ( + distance < nearestDistance || + (distance === nearestDistance && hunkIndex > nearestHunkIndex) + ) { + nearestDistance = distance; + nearestHunkIndex = hunkIndex; + } + } + + return nearestHunkIndex; +} + +/** Resolve the file and hunk nearest the current review viewport center. */ +export function findViewportCenteredHunkTarget({ + files, + fileSectionLayouts, + sectionGeometry, + scrollTop, + viewportHeight, +}: { + files: DiffFile[]; + fileSectionLayouts: FileSectionLayout[]; + sectionGeometry: DiffSectionGeometry[]; + scrollTop: number; + viewportHeight: number; +}): ViewportCenteredHunkTarget | null { + if (files.length === 0 || fileSectionLayouts.length === 0) { + return null; + } + + const centerOffset = Math.max(0, scrollTop + Math.max(0, Math.floor((viewportHeight - 1) / 2))); + const centeredSection = findFileSectionAtOffset(fileSectionLayouts, centerOffset); + if (!centeredSection) { + return null; + } + + const centeredFile = files[centeredSection.sectionIndex]; + if (!centeredFile) { + return null; + } + + return { + fileId: centeredFile.id, + hunkIndex: findNearestHunkIndexAtBodyOffset( + sectionGeometry[centeredSection.sectionIndex], + centerOffset - centeredSection.bodyTop, + centeredFile.metadata.hunks.length, + ), + }; +}