Skip to content

Commit 0f2dec3

Browse files
authored
feat(diff): implement row windowing for large single-file reviews (#237)
1 parent 40110d8 commit 0f2dec3

7 files changed

Lines changed: 436 additions & 4 deletions

File tree

src/ui/components/panes/DiffPane.tsx

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import { DiffSection } from "./DiffSection";
3737
import { DiffFileHeaderRow } from "./DiffFileHeaderRow";
3838
import { DiffSectionPlaceholder } from "./DiffSectionPlaceholder";
3939
import { VerticalScrollbar, type VerticalScrollbarHandle } from "../scrollbar/VerticalScrollbar";
40+
import type { VisibleBodyBounds } from "../../diff/rowWindowing";
4041
import { prefetchHighlightedDiff } from "../../diff/useHighlightedDiff";
4142

4243
const EMPTY_VISIBLE_AGENT_NOTES: VisibleAgentNote[] = [];
@@ -385,9 +386,9 @@ export function DiffPane({
385386
);
386387

387388
const visibleViewportFileIds = useMemo(() => {
388-
const overscanRows = 8;
389-
const minVisibleY = Math.max(0, scrollViewport.top - overscanRows);
390-
const maxVisibleY = scrollViewport.top + scrollViewport.height + overscanRows;
389+
const overscanTerminalRows = 8;
390+
const minVisibleY = Math.max(0, scrollViewport.top - overscanTerminalRows);
391+
const maxVisibleY = scrollViewport.top + scrollViewport.height + overscanTerminalRows;
391392
return collectIntersectingFileSectionIds(baseFileSectionLayouts, minVisibleY, maxVisibleY);
392393
}, [baseFileSectionLayouts, scrollViewport.height, scrollViewport.top]);
393394

@@ -589,6 +590,56 @@ export function DiffPane({
589590

590591
return next;
591592
}, [adjacentPrefetchFileIds, selectedFileId, visibleViewportFileIds, windowingEnabled]);
593+
const visibleBodyBoundsByFile = useMemo(() => {
594+
const next = new Map<string, VisibleBodyBounds>();
595+
if (scrollViewport.height <= 0) {
596+
return next;
597+
}
598+
599+
const overscanTerminalRows = Math.max(24, scrollViewport.height * 2);
600+
601+
files.forEach((file, index) => {
602+
const sectionLayout = fileSectionLayouts[index];
603+
const geometry = sectionGeometry[index];
604+
if (!sectionLayout || !geometry) {
605+
return;
606+
}
607+
608+
const shouldRenderSection = visibleWindowedFileIds?.has(file.id) ?? true;
609+
if (!shouldRenderSection) {
610+
return;
611+
}
612+
613+
// Convert the absolute review-stream viewport into file-body-local coordinates.
614+
// Example: if the viewport starts at row 2_000 globally and this file body starts at row
615+
// 1_940, then the file-local visible top is 60 rows into this file.
616+
const minTop = scrollViewport.top - sectionLayout.bodyTop - overscanTerminalRows;
617+
const maxBottom =
618+
scrollViewport.top + scrollViewport.height - sectionLayout.bodyTop + overscanTerminalRows;
619+
620+
// Keep the mounted rows bounded to the viewport slice. Selection reveal uses planned hunk
621+
// geometry as its fallback, so mounting an offscreen selected hunk is not necessary and would
622+
// remount very large hunks in full.
623+
624+
// Clamp the requested file-local interval back into the real body extent, then store it as
625+
// { top, height } so the row slicer can rebuild the matching [top, bottom) window later.
626+
const clampedTop = Math.min(geometry.bodyHeight, Math.max(0, minTop));
627+
const clampedBottom = Math.min(geometry.bodyHeight, Math.max(clampedTop, maxBottom));
628+
next.set(file.id, {
629+
top: clampedTop,
630+
height: clampedBottom - clampedTop,
631+
});
632+
});
633+
634+
return next;
635+
}, [
636+
fileSectionLayouts,
637+
files,
638+
scrollViewport.height,
639+
scrollViewport.top,
640+
sectionGeometry,
641+
visibleWindowedFileIds,
642+
]);
592643

593644
const selectedFileIndex = selectedFileId
594645
? files.findIndex((file) => file.id === selectedFileId)
@@ -1085,6 +1136,7 @@ export function DiffPane({
10851136
layout={layout}
10861137
selectedHunkIndex={file.id === selectedFileId ? selectedHunkIndex : -1}
10871138
shouldLoadHighlight={highlightPrefetchFileIds.has(file.id)}
1139+
sectionGeometry={sectionGeometry[index]}
10881140
separatorWidth={separatorWidth}
10891141
showHeader={shouldRenderInStreamFileHeader(index)}
10901142
showSeparator={index > 0}
@@ -1096,6 +1148,7 @@ export function DiffPane({
10961148
visibleAgentNotes={
10971149
visibleAgentNotesByFile.get(file.id) ?? EMPTY_VISIBLE_AGENT_NOTES
10981150
}
1151+
visibleBodyBounds={visibleBodyBoundsByFile.get(file.id)}
10991152
onOpenAgentNotesAtHunk={(hunkIndex) =>
11001153
onOpenAgentNotesAtHunk(file.id, hunkIndex)
11011154
}

src/ui/components/panes/DiffSection.tsx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { memo } from "react";
22
import type { DiffFile, LayoutMode } from "../../../core/types";
33
import { PierreDiffView } from "../../diff/PierreDiffView";
4+
import type { VisibleBodyBounds } from "../../diff/rowWindowing";
5+
import type { DiffSectionGeometry } from "../../lib/diffSectionGeometry";
46
import { getAnnotatedHunkIndices, type VisibleAgentNote } from "../../lib/agentAnnotations";
57
import { diffSectionId } from "../../lib/ids";
68
import { fitText } from "../../lib/text";
@@ -15,6 +17,7 @@ interface DiffSectionProps {
1517
layout: Exclude<LayoutMode, "auto">;
1618
selectedHunkIndex: number;
1719
shouldLoadHighlight: boolean;
20+
sectionGeometry?: DiffSectionGeometry;
1821
separatorWidth: number;
1922
showLineNumbers: boolean;
2023
showHunkHeaders: boolean;
@@ -23,6 +26,7 @@ interface DiffSectionProps {
2326
showSeparator: boolean;
2427
theme: AppTheme;
2528
visibleAgentNotes: VisibleAgentNote[];
29+
visibleBodyBounds?: VisibleBodyBounds;
2630
viewWidth: number;
2731
onOpenAgentNotesAtHunk: (hunkIndex: number) => void;
2832
onSelect: () => void;
@@ -37,6 +41,7 @@ function DiffSectionComponent({
3741
layout,
3842
selectedHunkIndex,
3943
shouldLoadHighlight,
44+
sectionGeometry,
4045
separatorWidth,
4146
showLineNumbers,
4247
showHunkHeaders,
@@ -45,6 +50,7 @@ function DiffSectionComponent({
4550
showSeparator,
4651
theme,
4752
visibleAgentNotes,
53+
visibleBodyBounds,
4854
viewWidth,
4955
onOpenAgentNotesAtHunk,
5056
onSelect,
@@ -98,9 +104,11 @@ function DiffSectionComponent({
98104
visibleAgentNotes={visibleAgentNotes}
99105
onOpenAgentNotesAtHunk={onOpenAgentNotesAtHunk}
100106
selectedHunkIndex={selectedHunkIndex}
107+
sectionGeometry={sectionGeometry}
101108
shouldLoadHighlight={shouldLoadHighlight}
102109
// The parent review stream owns scrolling across files.
103110
scrollable={false}
111+
visibleBodyBounds={visibleBodyBounds}
104112
/>
105113
</box>
106114
);
@@ -117,6 +125,7 @@ export const DiffSection = memo(DiffSectionComponent, (previous, next) => {
117125
previous.layout === next.layout &&
118126
previous.selectedHunkIndex === next.selectedHunkIndex &&
119127
previous.shouldLoadHighlight === next.shouldLoadHighlight &&
128+
previous.sectionGeometry === next.sectionGeometry &&
120129
previous.separatorWidth === next.separatorWidth &&
121130
previous.showLineNumbers === next.showLineNumbers &&
122131
previous.showHunkHeaders === next.showHunkHeaders &&
@@ -125,6 +134,7 @@ export const DiffSection = memo(DiffSectionComponent, (previous, next) => {
125134
previous.showSeparator === next.showSeparator &&
126135
previous.theme === next.theme &&
127136
previous.visibleAgentNotes === next.visibleAgentNotes &&
137+
previous.visibleBodyBounds === next.visibleBodyBounds &&
128138
previous.viewWidth === next.viewWidth
129139
);
130140
});

src/ui/components/ui-components.test.tsx

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,6 +1001,34 @@ describe("UI components", () => {
10011001
}
10021002
});
10031003

1004+
test("DiffPane keeps a distant selected hunk visible when row windowing narrows one file body", async () => {
1005+
const theme = resolveTheme("midnight", null);
1006+
const props = createDiffPaneProps([createWideTwoHunkDiffFile("target", "target.ts")], theme, {
1007+
diffContentWidth: 96,
1008+
headerLabelWidth: 48,
1009+
selectedFileId: "target",
1010+
selectedHunkIndex: 1,
1011+
separatorWidth: 92,
1012+
width: 100,
1013+
});
1014+
const setup = await testRender(<DiffPane {...props} />, {
1015+
width: 104,
1016+
height: 12,
1017+
});
1018+
1019+
try {
1020+
await settleDiffPane(setup);
1021+
const frame = await waitForFrame(setup, (nextFrame) => nextFrame.includes("line60 = 5901"));
1022+
1023+
expect(frame).toContain("line60 = 5901");
1024+
expect(frame).not.toContain("line1 = 1001");
1025+
} finally {
1026+
await act(async () => {
1027+
setup.renderer.destroy();
1028+
});
1029+
}
1030+
});
1031+
10041032
test("DiffPane keeps a selected hunk with inline notes fully visible when it fits", async () => {
10051033
const theme = resolveTheme("midnight", null);
10061034
const file = createViewportSizedBottomHunkDiffFile("target", "target.ts");

src/ui/diff/PierreDiffView.tsx

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,14 @@ import { useMemo } from "react";
22
import type { DiffFile, LayoutMode } from "../../core/types";
33
import { AgentInlineNote, AgentInlineNoteGuideCap } from "../components/panes/AgentInlineNote";
44
import type { VisibleAgentNote } from "../lib/agentAnnotations";
5+
import type { DiffSectionGeometry } from "../lib/diffSectionGeometry";
56
import { reviewRowId } from "../lib/ids";
67
import type { AppTheme } from "../themes";
78
import { findMaxLineNumber } from "./codeColumns";
89
import { buildSplitRows, buildStackRows } from "./pierre";
910
import { plannedReviewRowVisible } from "./plannedReviewRows";
1011
import { buildReviewRenderPlan } from "./reviewRenderPlan";
12+
import { resolveVisiblePlannedRowWindow, type VisibleBodyBounds } from "./rowWindowing";
1113
import { diffMessage, DiffRowView, fitText } from "./renderRows";
1214
import { useHighlightedDiff } from "./useHighlightedDiff";
1315

@@ -28,8 +30,10 @@ export function PierreDiffView({
2830
visibleAgentNotes = EMPTY_VISIBLE_AGENT_NOTES,
2931
width,
3032
selectedHunkIndex,
33+
sectionGeometry,
3134
shouldLoadHighlight = true,
3235
scrollable = true,
36+
visibleBodyBounds,
3337
}: {
3438
annotatedHunkIndices?: Set<number>;
3539
codeHorizontalOffset?: number;
@@ -43,8 +47,10 @@ export function PierreDiffView({
4347
visibleAgentNotes?: VisibleAgentNote[];
4448
width: number;
4549
selectedHunkIndex: number;
50+
sectionGeometry?: DiffSectionGeometry;
4651
shouldLoadHighlight?: boolean;
4752
scrollable?: boolean;
53+
visibleBodyBounds?: VisibleBodyBounds;
4854
}) {
4955
const resolvedHighlighted = useHighlightedDiff({
5056
file,
@@ -74,6 +80,35 @@ export function PierreDiffView({
7480
[file, rows, showHunkHeaders, visibleAgentNotes],
7581
);
7682
const lineNumberDigits = useMemo(() => String(file ? findMaxLineNumber(file) : 1).length, [file]);
83+
const visiblePlannedRowWindow = useMemo(() => {
84+
// Fall back to the full row list unless all three row-windowing inputs are ready:
85+
// - the complete planned row stream for this file
86+
// - measured per-row geometry for that same stream
87+
// - one file-local visible body slice from DiffPane
88+
// The helper relies on those structures staying in lockstep, so any missing input means
89+
// "render everything" instead of risking a mismatched partial slice.
90+
if (!sectionGeometry || !visibleBodyBounds) {
91+
return {
92+
bottomSpacerHeight: 0,
93+
plannedRows,
94+
topSpacerHeight: 0,
95+
};
96+
}
97+
98+
// `visibleBodyBounds` is already relative to this file body, not the whole review stream.
99+
// Example: if DiffPane says "mount rows 120..260 within package-lock.json", this helper keeps
100+
// only the planned rows whose measured bounds overlap that interval.
101+
//
102+
// The return value is not just the sliced rows. It also includes spacer heights for the skipped
103+
// region above and below so the file still occupies its original total body height inside the
104+
// scroll stream. That lets navigation, sticky headers, and reveal math keep using the same
105+
// absolute geometry even though most rows are temporarily unmounted.
106+
return resolveVisiblePlannedRowWindow({
107+
plannedRows,
108+
sectionGeometry,
109+
visibleBodyBounds,
110+
});
111+
}, [plannedRows, sectionGeometry, visibleBodyBounds]);
77112

78113
if (!file) {
79114
return (
@@ -93,7 +128,18 @@ export function PierreDiffView({
93128

94129
const content = (
95130
<box style={{ width: "100%", flexDirection: "column" }}>
96-
{plannedRows.map((plannedRow) => {
131+
{visiblePlannedRowWindow.topSpacerHeight > 0 ? (
132+
// Reserve the skipped height above the mounted slice so the file body keeps its original
133+
// absolute row positions inside the larger review stream.
134+
<box
135+
style={{
136+
width: "100%",
137+
height: visiblePlannedRowWindow.topSpacerHeight,
138+
backgroundColor: theme.panel,
139+
}}
140+
/>
141+
) : null}
142+
{visiblePlannedRowWindow.plannedRows.map((plannedRow) => {
97143
// Mirror the same visibility/id decisions used by the scroll-bound helpers so the mounted
98144
// tree can be measured by hunk later.
99145
const rowId = reviewRowId(plannedRow.key);
@@ -154,6 +200,16 @@ export function PierreDiffView({
154200
</box>
155201
);
156202
})}
203+
{visiblePlannedRowWindow.bottomSpacerHeight > 0 ? (
204+
// Mirror that reservation below the mounted slice so total file-body height stays stable.
205+
<box
206+
style={{
207+
width: "100%",
208+
height: visiblePlannedRowWindow.bottomSpacerHeight,
209+
backgroundColor: theme.panel,
210+
}}
211+
/>
212+
) : null}
157213
</box>
158214
);
159215

0 commit comments

Comments
 (0)