jb/paseo
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

Keep large diff reviews responsive (#3422)

Browse Source 
* perf(app): bound large diff row mounting

Keep the existing vertical and horizontal scroll owners so virtualization does not change gesture or scrolling behavior. Reserve exact offscreen row heights while mounting only the visible window, and retain the full renderer when wrapping or editing reviews requires dynamic geometry.

* perf(app): keep large diff reviews responsive

Opening an inline editor changed the whole review action object, reconciled large syntax-token trees, and disabled row virtualization. Keep the expensive token subtree stable and preserve deterministic row geometry while the editor is open.\n\nBound server-side highlighting for pathological single lines and remove the unused client highlighter so the actual diff pipeline owns the limit.

* test(app): check mounted diff typography

Row virtualization can unmount a fixed diff row after scrolling. Assert the configured font size against a currently mounted row so the test continues to verify rendered typography without depending on window position.

* test(app): stabilize Mermaid streaming coverage
This commit is contained in:
Mohamed Boudra
2026-08-16 15:38:12 +08:00
committed by GitHub
parent 4748aad103
commit 7f8a485a77
15 changed files with 755 additions and 930 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/app/src/git/diff-pane.tsx
+212 -57
View File
@@ -4,6 +4,7 @@ import {
useEffect,
useMemo,
useRef,
useSyncExternalStore,
memo,
type ReactElement,
type ReactNode,
@@ -43,6 +44,12 @@ import {
} from "lucide-react-native";
import { type ParsedDiffFile, type DiffLine, type HighlightToken } from "@/git/use-diff-query";
import { buildDiffFlatItems, sumHeightsBefore, type DiffFlatItem } from "@/git/diff-flat-items";
import {
createDiffRowWindow,
createDiffViewport,
type DiffRowWindow,
type DiffViewport,
} from "@/git/diff-virtualization";
import { buildDiffTree, collectDirPaths, compressSingleChildChains } from "@/git/diff-tree";
import { DiffFolderRow } from "@/git/diff-folder-row";
import {
@@ -104,6 +111,7 @@ import { collectAllTabs, useWorkspaceLayoutStore } from "@/stores/workspace-layo
import { buildWorkspaceTabPersistenceKey } from "@/workspace-tabs/model";
import { buildWorkspaceExplorerStateKey } from "@/hooks/use-file-explorer-actions";
import {
compactHighlightTokens,
formatDiffContentText,
formatDiffGutterText,
hasVisibleDiffTokens,
@@ -180,7 +188,7 @@ function HighlightedToken({ token }: { token: HighlightToken }) {
return <Text style={syntaxTokenStyleFor(token.style)}>{token.text}</Text>;
}
function HighlightedText({
const HighlightedText = memo(function HighlightedText({
tokens,
textMetricsStyle,
wrapLines = false,
@@ -196,10 +204,14 @@ function HighlightedText({
[textMetricsStyle, wrapLines],
);
const keyedTokens = useMemo(
() => tokens.map((token, index) => ({ key: `${index}-${token.text}`, token })),
[tokens],
);
const keyedTokens = useMemo(() => {
let characterOffset = 0;
return compactHighlightTokens(tokens).map((token) => {
const keyedToken = { key: `${characterOffset}-${token.style ?? "base"}`, token };
characterOffset += token.text.length;
return keyedToken;
});
}, [tokens]);
return (
<Text style={containerStyle} testID={testID}>
@@ -208,7 +220,7 @@ function HighlightedText({
))}
</Text>
);
}
});
interface DiffFileSectionProps {
file: ParsedDiffFile;
@@ -804,8 +816,53 @@ function InlineReviewRow({
);
}
const DIFF_ROW_OVERSCAN = 900;
const FULL_DIFF_VIEWPORT = { top: 0, height: Number.MAX_SAFE_INTEGER };
const subscribeToNothing = () => () => {};
const getFullDiffViewport = () => FULL_DIFF_VIEWPORT;
function useDiffRowWindow({
viewport,
bodyOffset,
rowHeights,
enabled,
}: {
viewport?: DiffViewport;
bodyOffset: number;
rowHeights: readonly number[];
enabled: boolean;
}): DiffRowWindow {
const viewportSnapshot = useSyncExternalStore(
viewport?.subscribe ?? subscribeToNothing,
viewport?.getSnapshot ?? getFullDiffViewport,
viewport?.getSnapshot ?? getFullDiffViewport,
);
return useMemo(() => {
if (!enabled) {
return createDiffRowWindow(rowHeights, {
viewportTop: 0,
viewportHeight: Number.MAX_SAFE_INTEGER,
overscan: 0,
});
}
return createDiffRowWindow(rowHeights, {
viewportTop: viewportSnapshot.top - bodyOffset - BORDER_WIDTH[1],
viewportHeight: viewportSnapshot.height,
overscan: DIFF_ROW_OVERSCAN,
});
}, [bodyOffset, enabled, rowHeights, viewportSnapshot.height, viewportSnapshot.top]);
}
function DiffRowSpacer({ height }: { height: number }) {
if (height <= 0) {
return null;
}
return <View style={inlineUnistylesStyle({ height })} />;
}
function SplitDiffColumn({
rows,
rowWindow,
side,
gutterWidth,
wrapLines,
@@ -814,6 +871,7 @@ function SplitDiffColumn({
showDivider = false,
}: {
rows: SplitDiffRow[];
rowWindow: DiffRowWindow;
side: "left" | "right";
gutterWidth: number;
wrapLines: boolean;
@@ -844,12 +902,20 @@ function SplitDiffColumn({
[textMetricsStyle],
);
const keyedRows = useMemo(() => rows.map((row, i) => ({ key: `row-${i}`, row })), [rows]);
const keyedRows = useMemo(
() =>
rows.slice(rowWindow.startIndex, rowWindow.endIndex).map((row, index) => ({
key: `row-${rowWindow.startIndex + index}`,
row,
})),
[rowWindow.endIndex, rowWindow.startIndex, rows],
);
if (wrapLines) {
return (
<View style={wrapCellStyle}>
<View style={styles.linesContainer}>
<DiffRowSpacer height={rowWindow.beforeHeight} />
{keyedRows.map(({ key, row }) => {
if (row.kind === "header") {
return (
@@ -882,6 +948,7 @@ function SplitDiffColumn({
</View>
);
})}
<DiffRowSpacer height={rowWindow.afterHeight} />
</View>
</View>
);
@@ -890,6 +957,7 @@ function SplitDiffColumn({
return (
<View style={rowCellStyle}>
<View style={styles.gutterColumn}>
<DiffRowSpacer height={rowWindow.beforeHeight} />
{keyedRows.map(({ key, row }) => {
if (row.kind === "header") {
return (
@@ -931,6 +999,7 @@ function SplitDiffColumn({
</View>
);
})}
<DiffRowSpacer height={rowWindow.afterHeight} />
</View>
<DiffScroll
scrollViewWidth={scrollWidth}
@@ -939,6 +1008,7 @@ function SplitDiffColumn({
contentContainerStyle={styles.diffContentInner}
>
<View style={linesContainerRowStyle}>
<DiffRowSpacer height={rowWindow.beforeHeight} />
{keyedRows.map(({ key, row }) => {
if (row.kind === "header") {
return (
@@ -974,6 +1044,7 @@ function SplitDiffColumn({
</View>
);
})}
<DiffRowSpacer height={rowWindow.afterHeight} />
</View>
</DiffScroll>
</View>
@@ -1255,6 +1326,8 @@ const DiffFileHeader = memo(function DiffFileHeader({
export function DiffFileBody({
file,
viewport,
bodyOffset = 0,
layout,
wrapLines,
codeFontSize,
@@ -1264,6 +1337,8 @@ export function DiffFileBody({
testID,
}: {
file: ParsedDiffFile;
viewport?: DiffViewport;
bodyOffset?: number;
layout: "unified" | "split";
wrapLines: boolean;
codeFontSize: number;
@@ -1276,6 +1351,50 @@ export function DiffFileBody({
const [bodyWidth, setBodyWidth] = useState(0);
const [hoveredReviewTargetKey, setHoveredReviewTargetKey] = useState<string | null>(null);
const { t } = useTranslation();
const canRenderRows = file.status !== "too_large" && file.status !== "binary";
const splitRows = useMemo(
() => (canRenderRows && layout === "split" ? getCachedSplitDiffRows(file) : []),
[canRenderRows, file, layout],
);
const unifiedLines = useMemo(
() => (canRenderRows && layout === "unified" ? getCachedUnifiedDiffLines(file) : []),
[canRenderRows, file, layout],
);
const rowHeights = useMemo(() => {
const lineHeight = getNumericLineHeight(textMetricsStyle) ?? Math.round(codeFontSize * 1.5);
if (layout === "split") {
return splitRows.map((row) => {
if (row.kind === "header") {
return lineHeight;
}
return (
lineHeight +
(getSplitInlineReviewThreadState({
left: row.left?.reviewTarget,
right: row.right?.reviewTarget,
reviewActions,
})?.height ?? 0)
);
});
}
return unifiedLines.map(
({ reviewTarget }) =>
lineHeight + (getInlineReviewThreadState({ reviewTarget, reviewActions })?.height ?? 0),
);
}, [codeFontSize, layout, reviewActions, splitRows, textMetricsStyle, unifiedLines]);
const rowWindow = useDiffRowWindow({
viewport,
bodyOffset,
rowHeights,
enabled: Boolean(viewport) && canRenderRows && !wrapLines,
});
const visibleUnifiedLines = useMemo(
() =>
unifiedLines
.slice(rowWindow.startIndex, rowWindow.endIndex)
.map((entry, index) => ({ entry, index: rowWindow.startIndex + index })),
[rowWindow.endIndex, rowWindow.startIndex, unifiedLines],
);
const handleLayout = useCallback(
(event: LayoutChangeEvent) => {
@@ -1324,11 +1443,11 @@ export function DiffFileBody({
const gutterWidth = lineNumberGutterWidth(maxLineNo, codeFontSize);
if (layout === "split") {
const rows = buildSplitDiffRows(file);
return (
<View style={[styles.diffContent, styles.splitRow]} dataSet={CODE_SURFACE_DATASET}>
<SplitDiffColumn
rows={rows}
rows={splitRows}
rowWindow={rowWindow}
side="left"
gutterWidth={gutterWidth}
wrapLines={wrapLines}
@@ -1336,7 +1455,8 @@ export function DiffFileBody({
reviewActions={reviewActions}
/>
<SplitDiffColumn
rows={rows}
rows={splitRows}
rowWindow={rowWindow}
side="right"
gutterWidth={gutterWidth}
wrapLines={wrapLines}
@@ -1348,30 +1468,32 @@ export function DiffFileBody({
);
}
const computedLines = buildUnifiedDiffLines(file);
if (wrapLines) {
return (
<View style={styles.diffContent} dataSet={CODE_SURFACE_DATASET}>
<View style={styles.linesContainer}>
{computedLines.map(({ line, lineNumber, key, reviewTarget }, index) => (
<View key={key} testID={`diff-wrapped-row-${index}`}>
<DiffLineView
line={line}
lineNumber={lineNumber}
gutterWidth={gutterWidth}
wrapLines={wrapLines}
textMetricsStyle={textMetricsStyle}
reviewTarget={reviewTarget}
reviewActions={reviewActions}
/>
<InlineReviewRow
reviewTarget={reviewTarget}
reviewActions={reviewActions}
gutterWidth={gutterWidth}
/>
</View>
))}
<DiffRowSpacer height={rowWindow.beforeHeight} />
{visibleUnifiedLines.map(
({ entry: { line, lineNumber, key, reviewTarget }, index }) => (
<View key={key} testID={`diff-wrapped-row-${index}`}>
<DiffLineView
line={line}
lineNumber={lineNumber}
gutterWidth={gutterWidth}
wrapLines={wrapLines}
textMetricsStyle={textMetricsStyle}
reviewTarget={reviewTarget}
reviewActions={reviewActions}
/>
<InlineReviewRow
reviewTarget={reviewTarget}
reviewActions={reviewActions}
gutterWidth={gutterWidth}
/>
</View>
),
)}
<DiffRowSpacer height={rowWindow.afterHeight} />
</View>
</View>
);
@@ -1382,28 +1504,33 @@ export function DiffFileBody({
return (
<View style={[styles.diffContent, styles.diffContentRow]} dataSet={CODE_SURFACE_DATASET}>
<View style={styles.gutterColumn}>
{computedLines.map(({ line, lineNumber, key, reviewTarget }, index) => (
<View key={key} testID={`diff-gutter-row-${index}`}>
<DiffGutterCell
lineNumber={lineNumber}
type={line.type}
gutterWidth={gutterWidth}
textMetricsStyle={textMetricsStyle}
reviewTarget={reviewTarget}
reviewActions={reviewActions}
isLineHovered={
reviewTarget?.key !== undefined && hoveredReviewTargetKey === reviewTarget.key
}
textTestID={`diff-gutter-text-${index}`}
actionTestID={`diff-gutter-action-${index}`}
/>
<InlineReviewGutterSpacer
reviewTarget={reviewTarget}
reviewActions={reviewActions}
gutterWidth={gutterWidth}
/>
</View>
))}
<DiffRowSpacer height={rowWindow.beforeHeight} />
{visibleUnifiedLines.map(
({ entry: { line, lineNumber, key, reviewTarget }, index }) => (
<View key={key} testID={`diff-gutter-row-${index}`}>
<DiffGutterCell
lineNumber={lineNumber}
type={line.type}
gutterWidth={gutterWidth}
textMetricsStyle={textMetricsStyle}
reviewTarget={reviewTarget}
reviewActions={reviewActions}
isLineHovered={
reviewTarget?.key !== undefined &&
hoveredReviewTargetKey === reviewTarget.key
}
textTestID={`diff-gutter-text-${index}`}
actionTestID={`diff-gutter-action-${index}`}
/>
<InlineReviewGutterSpacer
reviewTarget={reviewTarget}
reviewActions={reviewActions}
gutterWidth={gutterWidth}
/>
</View>
),
)}
<DiffRowSpacer height={rowWindow.afterHeight} />
</View>
<DiffScroll
scrollViewWidth={scrollViewWidth}
@@ -1412,7 +1539,8 @@ export function DiffFileBody({
contentContainerStyle={styles.diffContentInner}
>
<View style={linesContainerRowStyle}>
{computedLines.map(({ line, key, reviewTarget }, index) => (
<DiffRowSpacer height={rowWindow.beforeHeight} />
{visibleUnifiedLines.map(({ entry: { line, key, reviewTarget }, index }) => (
<View key={key} testID={`diff-code-row-${index}`}>
<DiffTextLine
line={line}
@@ -1432,6 +1560,7 @@ export function DiffFileBody({
/>
</View>
))}
<DiffRowSpacer height={rowWindow.afterHeight} />
</View>
</DiffScroll>
</View>
@@ -1828,7 +1957,9 @@ const EMPTY_PATH_LIST: string[] = [];
interface DiffFileMetrics {
contentLength: number;
splitRows?: SplitDiffRow[];
splitLineCount?: number;
unifiedLines?: ReturnType<typeof buildUnifiedDiffLines>;
unifiedLineCount: number;
}
@@ -1855,11 +1986,23 @@ function getDiffFileMetrics(file: ParsedDiffFile): DiffFileMetrics {
function getSplitDiffLineCount(file: ParsedDiffFile): number {
const metrics = getDiffFileMetrics(file);
if (metrics.splitLineCount === undefined) {
metrics.splitLineCount = buildSplitDiffRows(file).length;
metrics.splitLineCount = getCachedSplitDiffRows(file).length;
}
return metrics.splitLineCount;
}
function getCachedSplitDiffRows(file: ParsedDiffFile): SplitDiffRow[] {
const metrics = getDiffFileMetrics(file);
metrics.splitRows ??= buildSplitDiffRows(file);
return metrics.splitRows;
}
function getCachedUnifiedDiffLines(file: ParsedDiffFile): ReturnType<typeof buildUnifiedDiffLines> {
const metrics = getDiffFileMetrics(file);
metrics.unifiedLines ??= buildUnifiedDiffLines(file);
return metrics.unifiedLines;
}
function computeEmptyMessage(
hideWhitespace: boolean,
diffMode: "uncommitted" | "base",
@@ -2062,6 +2205,8 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi
[allFolderPathSet, collapsedFolders],
);
const diffListRef = useRef<FlatList<DiffFlatItem>>(null);
const diffViewport = useMemo(() => createDiffViewport(), []);
useEffect(() => () => diffViewport.dispose(), [diffViewport]);
const scrollbar = useOverlayFlatListScrollbar(diffListRef, { enabled: !isCompact });
const { onLayout: updateScrollbarLayout, onScroll: updateScrollbarOffset } = scrollbar;
const consumedFocusRequestRef = useRef<string | null>(null);
@@ -2216,9 +2361,13 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi
const handleDiffListScroll = useCallback(
(event: NativeSyntheticEvent<NativeScrollEvent>) => {
diffListScrollOffsetRef.current = event.nativeEvent.contentOffset.y;
diffViewport.update({
top: diffListScrollOffsetRef.current,
height: diffListViewportHeightRef.current,
});
updateScrollbarOffset(event);
},
[updateScrollbarOffset],
[diffViewport, updateScrollbarOffset],
);
const handleDiffListLayout = useCallback(
@@ -2228,9 +2377,10 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi
return;
}
diffListViewportHeightRef.current = height;
diffViewport.update({ top: diffListScrollOffsetRef.current, height });
updateScrollbarLayout(event);
},
[updateScrollbarLayout],
[diffViewport, updateScrollbarLayout],
);
const computeItemOffset = useCallback(
@@ -2387,7 +2537,7 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi
);
const renderFlatItem = useCallback(
({ item }: { item: DiffFlatItem }) => {
({ item, index }: { item: DiffFlatItem; index: number }) => {
if (item.type === "folder") {
return (
<DiffFolderRow
@@ -2441,6 +2591,8 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi
return (
<DiffFileBody
file={item.file}
viewport={diffViewport}
bodyOffset={sumHeightsBefore(flatItems, index, getFlatItemHeight)}
layout={layout}
wrapLines={wrapLines}
codeFontSize={codeFontSize}
@@ -2453,6 +2605,9 @@ export function SharedDiffView({ files, displayPreferences, mode }: SharedDiffVi
},
[
codeFontSize,
diffViewport,
flatItems,
getFlatItemHeight,
handleBodyHeightChange,
handleFolderRowHeightChange,
handleHeaderHeightChange,
packages/app/src/git/diff-virtualization.test.ts
+54
View File
@@ -0,0 +1,54 @@
import { describe, expect, it } from "vitest";
import { createDiffRowWindow } from "./diff-virtualization";
describe("createDiffRowWindow", () => {
it("renders only the viewport and overscan while preserving the full height", () => {
const rows = Array.from({ length: 1_200 }, () => 18);
const window = createDiffRowWindow(rows, {
viewportTop: 0,
viewportHeight: 900,
overscan: 900,
});
expect(window.startIndex).toBe(0);
expect(window.endIndex).toBe(100);
expect(window.beforeHeight).toBe(0);
expect(window.afterHeight).toBe(19_800);
expect(window.beforeHeight + window.renderedHeight + window.afterHeight).toBe(21_600);
});
it("finds a deep variable-height window without replaying earlier rows", () => {
const rows = [18, 18, 90, 18, 150, 18, 18];
const window = createDiffRowWindow(rows, {
viewportTop: 120,
viewportHeight: 60,
overscan: 0,
});
expect(window).toEqual({
startIndex: 2,
endIndex: 5,
beforeHeight: 36,
renderedHeight: 258,
afterHeight: 36,
totalHeight: 330,
});
});
it("returns an empty row window when an overscanned body is still offscreen", () => {
const window = createDiffRowWindow([18, 18, 18], {
viewportTop: -2_000,
viewportHeight: 800,
overscan: 600,
});
expect(window).toEqual({
startIndex: 0,
endIndex: 0,
beforeHeight: 0,
renderedHeight: 0,
afterHeight: 54,
totalHeight: 54,
});
});
});
packages/app/src/git/diff-virtualization.ts
+143
View File
@@ -0,0 +1,143 @@
export interface DiffViewportWindow {
viewportTop: number;
viewportHeight: number;
overscan: number;
}
export interface DiffRowWindow {
startIndex: number;
endIndex: number;
beforeHeight: number;
renderedHeight: number;
afterHeight: number;
totalHeight: number;
}
export interface DiffViewportSnapshot {
top: number;
height: number;
}
export interface DiffViewport {
getSnapshot: () => DiffViewportSnapshot;
subscribe: (listener: () => void) => () => void;
update: (snapshot: DiffViewportSnapshot) => void;
dispose: () => void;
}
/** Coalesce native/web scroll events so diff bodies select rows at most once per frame. */
export function createDiffViewport(): DiffViewport {
let snapshot: DiffViewportSnapshot = { top: 0, height: 0 };
let pending = snapshot;
let frame: number | null = null;
const listeners = new Set<() => void>();
const flush = () => {
frame = null;
if (pending.top === snapshot.top && pending.height === snapshot.height) {
return;
}
snapshot = pending;
for (const listener of listeners) {
listener();
}
};
return {
getSnapshot: () => snapshot,
subscribe(listener) {
listeners.add(listener);
return () => listeners.delete(listener);
},
update(nextSnapshot) {
pending = nextSnapshot;
frame ??= requestAnimationFrame(flush);
},
dispose() {
if (frame !== null) {
cancelAnimationFrame(frame);
}
frame = null;
listeners.clear();
},
};
}
/**
* Select the contiguous diff rows that intersect a viewport while reserving
* exact spacer heights for everything outside it.
*/
export function createDiffRowWindow(
rowHeights: readonly number[],
viewport: DiffViewportWindow,
): DiffRowWindow {
const offsets = Array.from({ length: rowHeights.length + 1 }, () => 0);
offsets[0] = 0;
for (let index = 0; index < rowHeights.length; index += 1) {
offsets[index + 1] = offsets[index] + rowHeights[index];
}
const totalHeight = offsets[rowHeights.length];
const visibleTop = viewport.viewportTop - viewport.overscan;
const visibleBottom = viewport.viewportTop + viewport.viewportHeight + viewport.overscan;
if (visibleBottom <= 0) {
return emptyWindow(0, 0, totalHeight);
}
if (visibleTop >= totalHeight) {
return emptyWindow(rowHeights.length, totalHeight, 0);
}
const startIndex = firstIndexWhoseEndExceeds(offsets, Math.max(0, visibleTop));
const endIndex = firstIndexWhoseStartReaches(offsets, Math.min(totalHeight, visibleBottom));
const beforeHeight = offsets[startIndex];
const renderedHeight = offsets[endIndex] - beforeHeight;
return {
startIndex,
endIndex,
beforeHeight,
renderedHeight,
afterHeight: totalHeight - offsets[endIndex],
totalHeight,
};
}
function emptyWindow(index: number, beforeHeight: number, afterHeight: number): DiffRowWindow {
return {
startIndex: index,
endIndex: index,
beforeHeight,
renderedHeight: 0,
afterHeight,
totalHeight: beforeHeight + afterHeight,
};
}
function firstIndexWhoseEndExceeds(offsets: readonly number[], target: number): number {
let low = 1;
let high = offsets.length;
while (low < high) {
const middle = Math.floor((low + high) / 2);
if ((offsets[middle] ?? Number.POSITIVE_INFINITY) > target) {
high = middle;
} else {
low = middle + 1;
}
}
return Math.min(offsets.length - 1, low - 1);
}
function firstIndexWhoseStartReaches(offsets: readonly number[], target: number): number {
let low = 0;
let high = offsets.length - 1;
while (low < high) {
const middle = Math.floor((low + high) / 2);
if (offsets[middle] >= target) {
high = middle;
} else {
low = middle + 1;
}
}
return low;
}