From f7f313280fa98be0a55795a27aaa89f1dd720426 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Thu, 7 May 2026 17:08:45 -0400 Subject: [PATCH 1/9] fix(diff): cheapen highlight work for large lockfile patches --- CHANGELOG.md | 2 ++ src/ui/diff/highlightPolicy.test.ts | 29 +++++++++++++++++++ src/ui/diff/highlightPolicy.ts | 43 +++++++++++++++++++++++++++++ src/ui/diff/pierre.ts | 17 ++++++++++-- src/ui/diff/useHighlightedDiff.ts | 23 +++++++++++++-- 5 files changed, 109 insertions(+), 5 deletions(-) create mode 100644 src/ui/diff/highlightPolicy.test.ts create mode 100644 src/ui/diff/highlightPolicy.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index c3778684..25db8df5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ All notable user-visible changes to Hunk are documented in this file. ### Fixed +- Kept large dependency lockfile diffs on a cheaper plain-text path so big `package-lock.json`-style patches render faster. + ## [0.11.0-beta.0] - 2026-05-09 ### Added diff --git a/src/ui/diff/highlightPolicy.test.ts b/src/ui/diff/highlightPolicy.test.ts new file mode 100644 index 00000000..cb050cf4 --- /dev/null +++ b/src/ui/diff/highlightPolicy.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, test } from "bun:test"; +import { createTestDiffFile } from "../../../test/helpers/diff-helpers"; +import { resolveDiffHighlightMode } from "./highlightPolicy"; + +describe("resolveDiffHighlightMode", () => { + test("keeps normal source files on full syntax highlighting", () => { + const file = createTestDiffFile({ path: "src/example.ts", language: "typescript" }); + + expect(resolveDiffHighlightMode(file)).toBe("full"); + }); + + test("uses plain-text highlighting for generated dependency manifests", () => { + const file = createTestDiffFile({ path: "pnpm-lock.yaml", language: "yaml" }); + + expect(resolveDiffHighlightMode(file)).toBe("text"); + }); + + test("skips highlight work entirely for very large lockfile diffs", () => { + const file = createTestDiffFile({ path: "package-lock.json", language: "json" }); + + file.metadata = { + ...file.metadata, + deletionLines: Array.from({ length: 2_500 }, (_, index) => `old-${index}`), + additionLines: Array.from({ length: 2_500 }, (_, index) => `new-${index}`), + }; + + expect(resolveDiffHighlightMode(file)).toBe("none"); + }); +}); diff --git a/src/ui/diff/highlightPolicy.ts b/src/ui/diff/highlightPolicy.ts new file mode 100644 index 00000000..e7164cac --- /dev/null +++ b/src/ui/diff/highlightPolicy.ts @@ -0,0 +1,43 @@ +import type { DiffFile } from "../../core/types"; + +export type DiffHighlightMode = "full" | "text" | "none"; + +const GENERATED_DEPENDENCY_FILE_BASENAMES = new Set([ + "bun.lock", + "bun.lockb", + "cargo.lock", + "composer.lock", + "gemfile.lock", + "go.sum", + "npm-shrinkwrap.json", + "package-lock.json", + "pnpm-lock.yaml", + "podfile.lock", + "yarn.lock", +]); +const DISABLED_LOCKFILE_HIGHLIGHT_LINE_THRESHOLD = 2_500; + +/** Return the last path segment so lockfile policies stay path-prefix agnostic. */ +function basename(path: string) { + return path.split("/").filter(Boolean).pop()?.toLowerCase() ?? path.toLowerCase(); +} + +/** Estimate the rendered footprint of one diff using the larger side's line count. */ +function diffLineFootprint(file: DiffFile) { + return Math.max(file.metadata.deletionLines.length, file.metadata.additionLines.length); +} + +/** + * Keep large generated dependency manifests on the cheap rendering path. + * + * Full syntax highlighting adds little review value for lockfiles but can produce thousands of + * extra token spans. Smaller lockfiles still get plain-text diff emphasis, while very large ones + * skip async highlight work entirely so the review stream paints sooner. + */ +export function resolveDiffHighlightMode(file: DiffFile): DiffHighlightMode { + if (!GENERATED_DEPENDENCY_FILE_BASENAMES.has(basename(file.path))) { + return "full"; + } + + return diffLineFootprint(file) >= DISABLED_LOCKFILE_HIGHLIGHT_LINE_THRESHOLD ? "none" : "text"; +} diff --git a/src/ui/diff/pierre.ts b/src/ui/diff/pierre.ts index 7d99b3d2..5862481a 100644 --- a/src/ui/diff/pierre.ts +++ b/src/ui/diff/pierre.ts @@ -10,6 +10,7 @@ import type { DiffFile } from "../../core/types"; import { blendHex, hexColorDistance } from "../lib/color"; import type { AppTheme } from "../themes"; import { expandDiffTabs } from "./codeColumns"; +import { resolveDiffHighlightMode } from "./highlightPolicy"; const PIERRE_THEME = { light: "pierre-light", @@ -515,11 +516,23 @@ export async function loadHighlightedDiff( file: DiffFile, appearance: AppTheme["appearance"] = "dark", ): Promise { + const highlightMode = resolveDiffHighlightMode(file); + if (highlightMode === "none") { + return { + deletionLines: [], + additionLines: [], + } satisfies HighlightedDiffCode; + } + + const highlightLanguage = highlightMode === "text" ? "text" : file.language; + const highlightMetadata = + highlightMode === "text" ? { ...file.metadata, lang: "text" } : file.metadata; + try { - const highlighter = await prepareHighlighter(file.language, appearance); + const highlighter = await prepareHighlighter(highlightLanguage, appearance); return queueHighlightedDiff(() => { const highlighted = renderDiffWithHighlighter( - file.metadata, + highlightMetadata, highlighter, pierreRenderOptions(appearance), ); diff --git a/src/ui/diff/useHighlightedDiff.ts b/src/ui/diff/useHighlightedDiff.ts index 9cac013b..b6efd244 100644 --- a/src/ui/diff/useHighlightedDiff.ts +++ b/src/ui/diff/useHighlightedDiff.ts @@ -1,5 +1,6 @@ import { useLayoutEffect, useState } from "react"; import type { DiffFile } from "../../core/types"; +import { resolveDiffHighlightMode } from "./highlightPolicy"; import { loadHighlightedDiff, type HighlightedDiffCode } from "./pierre"; /** Maximum cached highlight results. Prevents unbounded growth during long watch sessions. */ @@ -136,6 +137,13 @@ export function prefetchHighlightedDiff({ file: DiffFile; appearance: "light" | "dark"; }) { + if (resolveDiffHighlightMode(file) === "none") { + return Promise.resolve({ + deletionLines: [], + additionLines: [], + } satisfies HighlightedDiffCode); + } + return ensureHighlightedDiffLoaded(file, appearance); } @@ -172,12 +180,14 @@ export function useHighlightedDiff({ }) { const [highlighted, setHighlighted] = useState(null); const [highlightedCacheKey, setHighlightedCacheKey] = useState(null); - const appearanceCacheKey = file ? buildCacheKey(appearance, file) : null; + const highlightMode = file ? resolveDiffHighlightMode(file) : null; + const appearanceCacheKey = + file && highlightMode !== "none" ? buildCacheKey(appearance, file) : null; // 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) { + if (!file || !appearanceCacheKey || highlightMode === "none") { setHighlighted(null); setHighlightedCacheKey(null); return; @@ -213,7 +223,14 @@ export function useHighlightedDiff({ return () => { cancelled = true; }; - }, [appearance, appearanceCacheKey, file, highlightedCacheKey, shouldLoadHighlight]); + }, [ + appearance, + appearanceCacheKey, + file, + highlightMode, + highlightedCacheKey, + shouldLoadHighlight, + ]); // Prefer cached highlights during render so revisiting a file can paint immediately. return resolveHighlightedSnapshot({ From be34469a67841ffff20ff66a676c85ebe70c83a5 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Thu, 7 May 2026 17:08:55 -0400 Subject: [PATCH 2/9] feat(diff): add a row-windowing proof of concept --- src/ui/components/panes/DiffPane.tsx | 59 ++++++++++++ src/ui/components/panes/DiffSection.tsx | 10 ++ src/ui/diff/PierreDiffView.tsx | 45 ++++++++- src/ui/diff/rowWindowing.test.ts | 118 ++++++++++++++++++++++++ src/ui/diff/rowWindowing.ts | 100 ++++++++++++++++++++ 5 files changed, 331 insertions(+), 1 deletion(-) create mode 100644 src/ui/diff/rowWindowing.test.ts create mode 100644 src/ui/diff/rowWindowing.ts diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index 88c6088a..b45151fc 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -37,6 +37,7 @@ import { DiffSection } from "./DiffSection"; import { DiffFileHeaderRow } from "./DiffFileHeaderRow"; import { DiffSectionPlaceholder } from "./DiffSectionPlaceholder"; import { VerticalScrollbar, type VerticalScrollbarHandle } from "../scrollbar/VerticalScrollbar"; +import { rowWindowingPocEnabled, type VisibleBodyBounds } from "../../diff/rowWindowing"; import { prefetchHighlightedDiff } from "../../diff/useHighlightedDiff"; const EMPTY_VISIBLE_AGENT_NOTES: VisibleAgentNote[] = []; @@ -589,6 +590,62 @@ export function DiffPane({ return next; }, [adjacentPrefetchFileIds, selectedFileId, visibleViewportFileIds, windowingEnabled]); + const visibleBodyBoundsByFile = useMemo(() => { + const next = new Map(); + if (!rowWindowingPocEnabled() || scrollViewport.height <= 0) { + return next; + } + + const overscanRows = Math.max(24, scrollViewport.height * 2); + + files.forEach((file, index) => { + const sectionLayout = fileSectionLayouts[index]; + const geometry = sectionGeometry[index]; + if (!sectionLayout || !geometry) { + return; + } + + const shouldRenderSection = visibleWindowedFileIds?.has(file.id) ?? true; + if (!shouldRenderSection) { + return; + } + + let minTop = scrollViewport.top - sectionLayout.bodyTop - overscanRows; + let maxBottom = + scrollViewport.top + scrollViewport.height - sectionLayout.bodyTop + overscanRows; + + if (file.id === selectedFileId) { + const selectedHunkBounds = geometry.hunkBounds.get( + Math.max(0, Math.min(selectedHunkIndex, file.metadata.hunks.length - 1)), + ); + if (selectedHunkBounds) { + minTop = Math.min(minTop, selectedHunkBounds.top - overscanRows); + maxBottom = Math.max( + maxBottom, + selectedHunkBounds.top + selectedHunkBounds.height + overscanRows, + ); + } + } + + const clampedTop = Math.max(0, minTop); + const clampedBottom = Math.min(geometry.bodyHeight, Math.max(clampedTop, maxBottom)); + next.set(file.id, { + top: clampedTop, + height: clampedBottom - clampedTop, + }); + }); + + return next; + }, [ + fileSectionLayouts, + files, + scrollViewport.height, + scrollViewport.top, + sectionGeometry, + selectedFileId, + selectedHunkIndex, + visibleWindowedFileIds, + ]); const selectedFileIndex = selectedFileId ? files.findIndex((file) => file.id === selectedFileId) @@ -1085,6 +1142,7 @@ export function DiffPane({ layout={layout} selectedHunkIndex={file.id === selectedFileId ? selectedHunkIndex : -1} shouldLoadHighlight={highlightPrefetchFileIds.has(file.id)} + sectionGeometry={sectionGeometry[index]} separatorWidth={separatorWidth} showHeader={shouldRenderInStreamFileHeader(index)} showSeparator={index > 0} @@ -1096,6 +1154,7 @@ export function DiffPane({ visibleAgentNotes={ visibleAgentNotesByFile.get(file.id) ?? EMPTY_VISIBLE_AGENT_NOTES } + visibleBodyBounds={visibleBodyBoundsByFile.get(file.id)} onOpenAgentNotesAtHunk={(hunkIndex) => onOpenAgentNotesAtHunk(file.id, hunkIndex) } diff --git a/src/ui/components/panes/DiffSection.tsx b/src/ui/components/panes/DiffSection.tsx index 321dc2b8..88c021df 100644 --- a/src/ui/components/panes/DiffSection.tsx +++ b/src/ui/components/panes/DiffSection.tsx @@ -1,6 +1,8 @@ import { memo } from "react"; import type { DiffFile, LayoutMode } from "../../../core/types"; import { PierreDiffView } from "../../diff/PierreDiffView"; +import type { VisibleBodyBounds } from "../../diff/rowWindowing"; +import type { DiffSectionGeometry } from "../../lib/diffSectionGeometry"; import { getAnnotatedHunkIndices, type VisibleAgentNote } from "../../lib/agentAnnotations"; import { diffSectionId } from "../../lib/ids"; import { fitText } from "../../lib/text"; @@ -15,6 +17,7 @@ interface DiffSectionProps { layout: Exclude; selectedHunkIndex: number; shouldLoadHighlight: boolean; + sectionGeometry?: DiffSectionGeometry; separatorWidth: number; showLineNumbers: boolean; showHunkHeaders: boolean; @@ -23,6 +26,7 @@ interface DiffSectionProps { showSeparator: boolean; theme: AppTheme; visibleAgentNotes: VisibleAgentNote[]; + visibleBodyBounds?: VisibleBodyBounds; viewWidth: number; onOpenAgentNotesAtHunk: (hunkIndex: number) => void; onSelect: () => void; @@ -37,6 +41,7 @@ function DiffSectionComponent({ layout, selectedHunkIndex, shouldLoadHighlight, + sectionGeometry, separatorWidth, showLineNumbers, showHunkHeaders, @@ -45,6 +50,7 @@ function DiffSectionComponent({ showSeparator, theme, visibleAgentNotes, + visibleBodyBounds, viewWidth, onOpenAgentNotesAtHunk, onSelect, @@ -98,9 +104,11 @@ function DiffSectionComponent({ visibleAgentNotes={visibleAgentNotes} onOpenAgentNotesAtHunk={onOpenAgentNotesAtHunk} selectedHunkIndex={selectedHunkIndex} + sectionGeometry={sectionGeometry} shouldLoadHighlight={shouldLoadHighlight} // The parent review stream owns scrolling across files. scrollable={false} + visibleBodyBounds={visibleBodyBounds} /> ); @@ -117,6 +125,7 @@ export const DiffSection = memo(DiffSectionComponent, (previous, next) => { previous.layout === next.layout && previous.selectedHunkIndex === next.selectedHunkIndex && previous.shouldLoadHighlight === next.shouldLoadHighlight && + previous.sectionGeometry === next.sectionGeometry && previous.separatorWidth === next.separatorWidth && previous.showLineNumbers === next.showLineNumbers && previous.showHunkHeaders === next.showHunkHeaders && @@ -125,6 +134,7 @@ export const DiffSection = memo(DiffSectionComponent, (previous, next) => { previous.showSeparator === next.showSeparator && previous.theme === next.theme && previous.visibleAgentNotes === next.visibleAgentNotes && + previous.visibleBodyBounds === next.visibleBodyBounds && previous.viewWidth === next.viewWidth ); }); diff --git a/src/ui/diff/PierreDiffView.tsx b/src/ui/diff/PierreDiffView.tsx index 21c394ac..9fe34c4c 100644 --- a/src/ui/diff/PierreDiffView.tsx +++ b/src/ui/diff/PierreDiffView.tsx @@ -2,12 +2,18 @@ import { useMemo } from "react"; import type { DiffFile, LayoutMode } from "../../core/types"; import { AgentInlineNote, AgentInlineNoteGuideCap } from "../components/panes/AgentInlineNote"; import type { VisibleAgentNote } from "../lib/agentAnnotations"; +import type { DiffSectionGeometry } from "../lib/diffSectionGeometry"; import { reviewRowId } from "../lib/ids"; import type { AppTheme } from "../themes"; import { findMaxLineNumber } from "./codeColumns"; import { buildSplitRows, buildStackRows } from "./pierre"; import { plannedReviewRowVisible } from "./plannedReviewRows"; import { buildReviewRenderPlan } from "./reviewRenderPlan"; +import { + resolveVisiblePlannedRowWindow, + rowWindowingPocEnabled, + type VisibleBodyBounds, +} from "./rowWindowing"; import { diffMessage, DiffRowView, fitText } from "./renderRows"; import { useHighlightedDiff } from "./useHighlightedDiff"; @@ -28,8 +34,10 @@ export function PierreDiffView({ visibleAgentNotes = EMPTY_VISIBLE_AGENT_NOTES, width, selectedHunkIndex, + sectionGeometry, shouldLoadHighlight = true, scrollable = true, + visibleBodyBounds, }: { annotatedHunkIndices?: Set; codeHorizontalOffset?: number; @@ -43,8 +51,10 @@ export function PierreDiffView({ visibleAgentNotes?: VisibleAgentNote[]; width: number; selectedHunkIndex: number; + sectionGeometry?: DiffSectionGeometry; shouldLoadHighlight?: boolean; scrollable?: boolean; + visibleBodyBounds?: VisibleBodyBounds; }) { const resolvedHighlighted = useHighlightedDiff({ file, @@ -74,6 +84,21 @@ export function PierreDiffView({ [file, rows, showHunkHeaders, visibleAgentNotes], ); const lineNumberDigits = useMemo(() => String(file ? findMaxLineNumber(file) : 1).length, [file]); + const visiblePlannedRowWindow = useMemo(() => { + if (!sectionGeometry || !visibleBodyBounds || !rowWindowingPocEnabled()) { + return { + bottomSpacerHeight: 0, + plannedRows, + topSpacerHeight: 0, + }; + } + + return resolveVisiblePlannedRowWindow({ + plannedRows, + sectionGeometry, + visibleBodyBounds, + }); + }, [plannedRows, sectionGeometry, visibleBodyBounds]); if (!file) { return ( @@ -93,7 +118,16 @@ export function PierreDiffView({ const content = ( - {plannedRows.map((plannedRow) => { + {visiblePlannedRowWindow.topSpacerHeight > 0 ? ( + + ) : null} + {visiblePlannedRowWindow.plannedRows.map((plannedRow) => { // Mirror the same visibility/id decisions used by the scroll-bound helpers so the mounted // tree can be measured by hunk later. const rowId = reviewRowId(plannedRow.key); @@ -154,6 +188,15 @@ export function PierreDiffView({ ); })} + {visiblePlannedRowWindow.bottomSpacerHeight > 0 ? ( + + ) : null} ); diff --git a/src/ui/diff/rowWindowing.test.ts b/src/ui/diff/rowWindowing.test.ts new file mode 100644 index 00000000..ef1b5296 --- /dev/null +++ b/src/ui/diff/rowWindowing.test.ts @@ -0,0 +1,118 @@ +import { describe, expect, test } from "bun:test"; +import type { DiffSectionGeometry } from "../lib/diffSectionGeometry"; +import type { PlannedReviewRow } from "./reviewRenderPlan"; +import { resolveVisiblePlannedRowWindow } from "./rowWindowing"; + +/** Build one minimal planned row for row-window slicing tests. */ +function createTestPlannedRow(key: string): PlannedReviewRow { + return { + kind: "diff-row", + key, + stableKey: key, + fileId: "file:test", + hunkIndex: 0, + row: { + type: "hunk-header", + key, + fileId: "file:test", + hunkIndex: 0, + text: key, + }, + }; +} + +/** Build one geometry object with explicit row bounds for row-window tests. */ +function createTestSectionGeometry( + rowBounds: Array<{ key: string; top: number; height: number }>, + bodyHeight: number, +): DiffSectionGeometry { + const normalizedRowBounds = rowBounds.map((row) => ({ + ...row, + stableKey: row.key, + stableKeys: [row.key], + })); + + return { + bodyHeight, + hunkAnchorRows: new Map(), + hunkBounds: new Map(), + rowBounds: normalizedRowBounds, + rowBoundsByKey: new Map(normalizedRowBounds.map((row) => [row.key, row])), + rowBoundsByStableKey: new Map(normalizedRowBounds.map((row) => [row.stableKey, row])), + }; +} + +describe("resolveVisiblePlannedRowWindow", () => { + test("returns only rows that intersect the visible body range", () => { + const plannedRows = ["row:0", "row:1", "row:2", "row:3"].map(createTestPlannedRow); + const sectionGeometry = createTestSectionGeometry( + [ + { key: "row:0", top: 0, height: 1 }, + { key: "row:1", top: 1, height: 2 }, + { key: "row:2", top: 3, height: 1 }, + { key: "row:3", top: 4, height: 1 }, + ], + 5, + ); + + const window = resolveVisiblePlannedRowWindow({ + plannedRows, + sectionGeometry, + visibleBodyBounds: { top: 1, height: 3 }, + }); + + expect(window.topSpacerHeight).toBe(1); + expect(window.bottomSpacerHeight).toBe(1); + expect(window.plannedRows.map((row) => row.key)).toEqual(["row:1", "row:2"]); + }); + + test("keeps adjacent zero-height rows attached to the visible slice", () => { + const plannedRows = ["header:hidden", "code:1", "header:hidden:after", "code:2"].map( + createTestPlannedRow, + ); + const sectionGeometry = createTestSectionGeometry( + [ + { key: "header:hidden", top: 0, height: 0 }, + { key: "code:1", top: 0, height: 1 }, + { key: "header:hidden:after", top: 1, height: 0 }, + { key: "code:2", top: 1, height: 1 }, + ], + 2, + ); + + const window = resolveVisiblePlannedRowWindow({ + plannedRows, + sectionGeometry, + visibleBodyBounds: { top: 0, height: 1 }, + }); + + expect(window.topSpacerHeight).toBe(0); + expect(window.bottomSpacerHeight).toBe(1); + expect(window.plannedRows.map((row) => row.key)).toEqual([ + "header:hidden", + "code:1", + "header:hidden:after", + ]); + }); + + test("can collapse a fully offscreen file body into spacer height only", () => { + const plannedRows = ["row:0", "row:1"].map(createTestPlannedRow); + const sectionGeometry = createTestSectionGeometry( + [ + { key: "row:0", top: 0, height: 2 }, + { key: "row:1", top: 2, height: 2 }, + ], + 4, + ); + + const window = resolveVisiblePlannedRowWindow({ + plannedRows, + sectionGeometry, + visibleBodyBounds: { top: 10, height: 2 }, + }); + + expect(window.topSpacerHeight).toBe(0); + expect(window.bottomSpacerHeight).toBe(4); + expect(window.plannedRows).toHaveLength(0); + }); +}); diff --git a/src/ui/diff/rowWindowing.ts b/src/ui/diff/rowWindowing.ts new file mode 100644 index 00000000..cf1421c3 --- /dev/null +++ b/src/ui/diff/rowWindowing.ts @@ -0,0 +1,100 @@ +import type { DiffSectionGeometry } from "../lib/diffSectionGeometry"; +import type { PlannedReviewRow } from "./reviewRenderPlan"; + +const ROW_WINDOWING_POC_ENV = "HUNK_ROW_WINDOWING_POC"; + +export interface VisibleBodyBounds { + top: number; + height: number; +} + +export interface VisiblePlannedRowWindow { + bottomSpacerHeight: number; + plannedRows: PlannedReviewRow[]; + topSpacerHeight: number; +} + +/** Opt-in gate for the row-windowing proof of concept while we validate behavior and gains. */ +export function rowWindowingPocEnabled(env: NodeJS.ProcessEnv = process.env) { + return env[ROW_WINDOWING_POC_ENV] === "1"; +} + +/** + * Slice planned rows down to the visible body range while preserving total section height. + * + * The geometry row bounds come from the same render plan as `plannedRows`, so their array order is + * intentionally aligned and can be sliced by index. + */ +export function resolveVisiblePlannedRowWindow({ + plannedRows, + sectionGeometry, + visibleBodyBounds, +}: { + plannedRows: PlannedReviewRow[]; + sectionGeometry: DiffSectionGeometry; + visibleBodyBounds: VisibleBodyBounds; +}): VisiblePlannedRowWindow { + if (plannedRows.length === 0 || sectionGeometry.rowBounds.length !== plannedRows.length) { + return { + bottomSpacerHeight: 0, + plannedRows, + topSpacerHeight: 0, + }; + } + + const minVisibleTop = Math.max(0, visibleBodyBounds.top); + const maxVisibleBottom = Math.min( + sectionGeometry.bodyHeight, + visibleBodyBounds.top + Math.max(0, visibleBodyBounds.height), + ); + + let firstVisibleIndex = -1; + let lastVisibleIndex = -1; + + for (let index = 0; index < sectionGeometry.rowBounds.length; index += 1) { + const rowBounds = sectionGeometry.rowBounds[index]!; + if (rowBounds.height <= 0) { + continue; + } + + const rowBottom = rowBounds.top + rowBounds.height; + if (rowBottom <= minVisibleTop || rowBounds.top >= maxVisibleBottom) { + continue; + } + + if (firstVisibleIndex < 0) { + firstVisibleIndex = index; + } + lastVisibleIndex = index; + } + + if (firstVisibleIndex < 0 || lastVisibleIndex < 0) { + return { + bottomSpacerHeight: sectionGeometry.bodyHeight, + plannedRows: [], + topSpacerHeight: 0, + }; + } + + let startIndex = firstVisibleIndex; + while (startIndex > 0 && sectionGeometry.rowBounds[startIndex - 1]?.height === 0) { + startIndex -= 1; + } + + let endIndex = lastVisibleIndex + 1; + while (endIndex < plannedRows.length && sectionGeometry.rowBounds[endIndex]?.height === 0) { + endIndex += 1; + } + + const startRowBounds = sectionGeometry.rowBounds[startIndex]!; + const endRowBounds = sectionGeometry.rowBounds[endIndex - 1]!; + + return { + bottomSpacerHeight: Math.max( + 0, + sectionGeometry.bodyHeight - (endRowBounds.top + endRowBounds.height), + ), + plannedRows: plannedRows.slice(startIndex, endIndex), + topSpacerHeight: startRowBounds.top, + }; +} From 1db15c9ded3c8f0a73c16736b36719c818906911 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Thu, 7 May 2026 22:51:05 -0400 Subject: [PATCH 3/9] refactor(diff): focus the perf branch on row windowing --- CHANGELOG.md | 2 -- src/ui/components/panes/DiffPane.tsx | 7 +++++ src/ui/diff/PierreDiffView.tsx | 17 +++++++++++ src/ui/diff/highlightPolicy.test.ts | 29 ------------------- src/ui/diff/highlightPolicy.ts | 43 ---------------------------- src/ui/diff/pierre.ts | 17 ++--------- src/ui/diff/rowWindowing.ts | 15 ++++++++-- src/ui/diff/useHighlightedDiff.ts | 23 ++------------- 8 files changed, 42 insertions(+), 111 deletions(-) delete mode 100644 src/ui/diff/highlightPolicy.test.ts delete mode 100644 src/ui/diff/highlightPolicy.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 25db8df5..c3778684 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,8 +10,6 @@ All notable user-visible changes to Hunk are documented in this file. ### Fixed -- Kept large dependency lockfile diffs on a cheaper plain-text path so big `package-lock.json`-style patches render faster. - ## [0.11.0-beta.0] - 2026-05-09 ### Added diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index b45151fc..0b47cd8c 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -610,6 +610,9 @@ export function DiffPane({ return; } + // Convert the absolute review-stream viewport into file-body-local coordinates. + // Example: if the viewport starts at row 2_000 globally and this file body starts at row + // 1_940, then the file-local visible top is 60 rows into this file. let minTop = scrollViewport.top - sectionLayout.bodyTop - overscanRows; let maxBottom = scrollViewport.top + scrollViewport.height - sectionLayout.bodyTop + overscanRows; @@ -619,6 +622,8 @@ export function DiffPane({ Math.max(0, Math.min(selectedHunkIndex, file.metadata.hunks.length - 1)), ); if (selectedHunkBounds) { + // Always keep the selected hunk inside the mounted slice even if the viewport is a little + // ahead or behind it. That avoids unmounting the active target during navigation settles. minTop = Math.min(minTop, selectedHunkBounds.top - overscanRows); maxBottom = Math.max( maxBottom, @@ -627,6 +632,8 @@ export function DiffPane({ } } + // Clamp the requested file-local interval back into the real body extent, then store it as + // { top, height } so the row slicer can rebuild the matching [top, bottom) window later. const clampedTop = Math.max(0, minTop); const clampedBottom = Math.min(geometry.bodyHeight, Math.max(clampedTop, maxBottom)); next.set(file.id, { diff --git a/src/ui/diff/PierreDiffView.tsx b/src/ui/diff/PierreDiffView.tsx index 9fe34c4c..212744db 100644 --- a/src/ui/diff/PierreDiffView.tsx +++ b/src/ui/diff/PierreDiffView.tsx @@ -85,6 +85,12 @@ export function PierreDiffView({ ); const lineNumberDigits = useMemo(() => String(file ? findMaxLineNumber(file) : 1).length, [file]); const visiblePlannedRowWindow = useMemo(() => { + // Fall back to the full row list unless all three row-windowing inputs are ready: + // - the complete planned row stream for this file + // - measured per-row geometry for that same stream + // - one file-local visible body slice from DiffPane + // The helper relies on those structures staying in lockstep, so any missing input means + // "render everything" instead of risking a mismatched partial slice. if (!sectionGeometry || !visibleBodyBounds || !rowWindowingPocEnabled()) { return { bottomSpacerHeight: 0, @@ -93,6 +99,14 @@ export function PierreDiffView({ }; } + // `visibleBodyBounds` is already relative to this file body, not the whole review stream. + // Example: if DiffPane says "mount rows 120..260 within package-lock.json", this helper keeps + // only the planned rows whose measured bounds overlap that interval. + // + // The return value is not just the sliced rows. It also includes spacer heights for the skipped + // region above and below so the file still occupies its original total body height inside the + // scroll stream. That lets navigation, sticky headers, and reveal math keep using the same + // absolute geometry even though most rows are temporarily unmounted. return resolveVisiblePlannedRowWindow({ plannedRows, sectionGeometry, @@ -119,6 +133,8 @@ export function PierreDiffView({ const content = ( {visiblePlannedRowWindow.topSpacerHeight > 0 ? ( + // Reserve the skipped height above the mounted slice so the file body keeps its original + // absolute row positions inside the larger review stream. 0 ? ( + // Mirror that reservation below the mounted slice so total file-body height stays stable. { - test("keeps normal source files on full syntax highlighting", () => { - const file = createTestDiffFile({ path: "src/example.ts", language: "typescript" }); - - expect(resolveDiffHighlightMode(file)).toBe("full"); - }); - - test("uses plain-text highlighting for generated dependency manifests", () => { - const file = createTestDiffFile({ path: "pnpm-lock.yaml", language: "yaml" }); - - expect(resolveDiffHighlightMode(file)).toBe("text"); - }); - - test("skips highlight work entirely for very large lockfile diffs", () => { - const file = createTestDiffFile({ path: "package-lock.json", language: "json" }); - - file.metadata = { - ...file.metadata, - deletionLines: Array.from({ length: 2_500 }, (_, index) => `old-${index}`), - additionLines: Array.from({ length: 2_500 }, (_, index) => `new-${index}`), - }; - - expect(resolveDiffHighlightMode(file)).toBe("none"); - }); -}); diff --git a/src/ui/diff/highlightPolicy.ts b/src/ui/diff/highlightPolicy.ts deleted file mode 100644 index e7164cac..00000000 --- a/src/ui/diff/highlightPolicy.ts +++ /dev/null @@ -1,43 +0,0 @@ -import type { DiffFile } from "../../core/types"; - -export type DiffHighlightMode = "full" | "text" | "none"; - -const GENERATED_DEPENDENCY_FILE_BASENAMES = new Set([ - "bun.lock", - "bun.lockb", - "cargo.lock", - "composer.lock", - "gemfile.lock", - "go.sum", - "npm-shrinkwrap.json", - "package-lock.json", - "pnpm-lock.yaml", - "podfile.lock", - "yarn.lock", -]); -const DISABLED_LOCKFILE_HIGHLIGHT_LINE_THRESHOLD = 2_500; - -/** Return the last path segment so lockfile policies stay path-prefix agnostic. */ -function basename(path: string) { - return path.split("/").filter(Boolean).pop()?.toLowerCase() ?? path.toLowerCase(); -} - -/** Estimate the rendered footprint of one diff using the larger side's line count. */ -function diffLineFootprint(file: DiffFile) { - return Math.max(file.metadata.deletionLines.length, file.metadata.additionLines.length); -} - -/** - * Keep large generated dependency manifests on the cheap rendering path. - * - * Full syntax highlighting adds little review value for lockfiles but can produce thousands of - * extra token spans. Smaller lockfiles still get plain-text diff emphasis, while very large ones - * skip async highlight work entirely so the review stream paints sooner. - */ -export function resolveDiffHighlightMode(file: DiffFile): DiffHighlightMode { - if (!GENERATED_DEPENDENCY_FILE_BASENAMES.has(basename(file.path))) { - return "full"; - } - - return diffLineFootprint(file) >= DISABLED_LOCKFILE_HIGHLIGHT_LINE_THRESHOLD ? "none" : "text"; -} diff --git a/src/ui/diff/pierre.ts b/src/ui/diff/pierre.ts index 5862481a..7d99b3d2 100644 --- a/src/ui/diff/pierre.ts +++ b/src/ui/diff/pierre.ts @@ -10,7 +10,6 @@ import type { DiffFile } from "../../core/types"; import { blendHex, hexColorDistance } from "../lib/color"; import type { AppTheme } from "../themes"; import { expandDiffTabs } from "./codeColumns"; -import { resolveDiffHighlightMode } from "./highlightPolicy"; const PIERRE_THEME = { light: "pierre-light", @@ -516,23 +515,11 @@ export async function loadHighlightedDiff( file: DiffFile, appearance: AppTheme["appearance"] = "dark", ): Promise { - const highlightMode = resolveDiffHighlightMode(file); - if (highlightMode === "none") { - return { - deletionLines: [], - additionLines: [], - } satisfies HighlightedDiffCode; - } - - const highlightLanguage = highlightMode === "text" ? "text" : file.language; - const highlightMetadata = - highlightMode === "text" ? { ...file.metadata, lang: "text" } : file.metadata; - try { - const highlighter = await prepareHighlighter(highlightLanguage, appearance); + const highlighter = await prepareHighlighter(file.language, appearance); return queueHighlightedDiff(() => { const highlighted = renderDiffWithHighlighter( - highlightMetadata, + file.metadata, highlighter, pierreRenderOptions(appearance), ); diff --git a/src/ui/diff/rowWindowing.ts b/src/ui/diff/rowWindowing.ts index cf1421c3..365c0d29 100644 --- a/src/ui/diff/rowWindowing.ts +++ b/src/ui/diff/rowWindowing.ts @@ -3,6 +3,7 @@ import type { PlannedReviewRow } from "./reviewRenderPlan"; const ROW_WINDOWING_POC_ENV = "HUNK_ROW_WINDOWING_POC"; +/** One visible slice within a file body, measured in file-local row units. */ export interface VisibleBodyBounds { top: number; height: number; @@ -42,6 +43,8 @@ export function resolveVisiblePlannedRowWindow({ }; } + // Convert the requested visible window into one closed-open interval within this file body: + // [minVisibleTop, maxVisibleBottom). Rows above/below that interval become spacer height. const minVisibleTop = Math.max(0, visibleBodyBounds.top); const maxVisibleBottom = Math.min( sectionGeometry.bodyHeight, @@ -58,6 +61,8 @@ export function resolveVisiblePlannedRowWindow({ } const rowBottom = rowBounds.top + rowBounds.height; + // Treat each row as the half-open interval [row.top, row.bottom). If that interval does not + // overlap the visible file-body interval, the row can stay unmounted. if (rowBottom <= minVisibleTop || rowBounds.top >= maxVisibleBottom) { continue; } @@ -77,11 +82,15 @@ export function resolveVisiblePlannedRowWindow({ } let startIndex = firstVisibleIndex; + // Zero-height rows still matter structurally: for example, hidden hunk headers keep anchor ids + // and stable row ordering. If one sits immediately before the visible slice, keep it attached. while (startIndex > 0 && sectionGeometry.rowBounds[startIndex - 1]?.height === 0) { startIndex -= 1; } let endIndex = lastVisibleIndex + 1; + // Do the same on the trailing edge so hidden structural rows continue to travel with the last + // visible rendered row instead of being stranded in the spacer region. while (endIndex < plannedRows.length && sectionGeometry.rowBounds[endIndex]?.height === 0) { endIndex += 1; } @@ -90,11 +99,13 @@ export function resolveVisiblePlannedRowWindow({ const endRowBounds = sectionGeometry.rowBounds[endIndex - 1]!; return { + // The top spacer is exactly the skipped body height before the first mounted row. + topSpacerHeight: startRowBounds.top, + plannedRows: plannedRows.slice(startIndex, endIndex), + // The bottom spacer is the remaining body height after the last mounted row's bottom edge. bottomSpacerHeight: Math.max( 0, sectionGeometry.bodyHeight - (endRowBounds.top + endRowBounds.height), ), - plannedRows: plannedRows.slice(startIndex, endIndex), - topSpacerHeight: startRowBounds.top, }; } diff --git a/src/ui/diff/useHighlightedDiff.ts b/src/ui/diff/useHighlightedDiff.ts index b6efd244..9cac013b 100644 --- a/src/ui/diff/useHighlightedDiff.ts +++ b/src/ui/diff/useHighlightedDiff.ts @@ -1,6 +1,5 @@ import { useLayoutEffect, useState } from "react"; import type { DiffFile } from "../../core/types"; -import { resolveDiffHighlightMode } from "./highlightPolicy"; import { loadHighlightedDiff, type HighlightedDiffCode } from "./pierre"; /** Maximum cached highlight results. Prevents unbounded growth during long watch sessions. */ @@ -137,13 +136,6 @@ export function prefetchHighlightedDiff({ file: DiffFile; appearance: "light" | "dark"; }) { - if (resolveDiffHighlightMode(file) === "none") { - return Promise.resolve({ - deletionLines: [], - additionLines: [], - } satisfies HighlightedDiffCode); - } - return ensureHighlightedDiffLoaded(file, appearance); } @@ -180,14 +172,12 @@ export function useHighlightedDiff({ }) { const [highlighted, setHighlighted] = useState(null); const [highlightedCacheKey, setHighlightedCacheKey] = useState(null); - const highlightMode = file ? resolveDiffHighlightMode(file) : null; - const appearanceCacheKey = - file && highlightMode !== "none" ? buildCacheKey(appearance, file) : null; + const appearanceCacheKey = file ? buildCacheKey(appearance, file) : null; // 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 || highlightMode === "none") { + if (!file || !appearanceCacheKey) { setHighlighted(null); setHighlightedCacheKey(null); return; @@ -223,14 +213,7 @@ export function useHighlightedDiff({ return () => { cancelled = true; }; - }, [ - appearance, - appearanceCacheKey, - file, - highlightMode, - highlightedCacheKey, - shouldLoadHighlight, - ]); + }, [appearance, appearanceCacheKey, file, highlightedCacheKey, shouldLoadHighlight]); // Prefer cached highlights during render so revisiting a file can paint immediately. return resolveHighlightedSnapshot({ From 789262e22c70b0b6bddc99b130cbf15524e9dacb Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Fri, 8 May 2026 13:10:14 -0400 Subject: [PATCH 4/9] test(diff): cover row-windowing navigation flows --- src/ui/components/ui-components.test.tsx | 37 ++++++++++++++++++++ test/pty/ui-integration.test.ts | 43 ++++++++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/src/ui/components/ui-components.test.tsx b/src/ui/components/ui-components.test.tsx index 2fe167a4..9cd431d1 100644 --- a/src/ui/components/ui-components.test.tsx +++ b/src/ui/components/ui-components.test.tsx @@ -1001,6 +1001,43 @@ describe("UI components", () => { } }); + test("DiffPane keeps a distant selected hunk visible when row windowing is enabled", async () => { + const previousRowWindowing = process.env.HUNK_ROW_WINDOWING_POC; + process.env.HUNK_ROW_WINDOWING_POC = "1"; + + const theme = resolveTheme("midnight", null); + const props = createDiffPaneProps([createWideTwoHunkDiffFile("target", "target.ts")], theme, { + diffContentWidth: 96, + headerLabelWidth: 48, + selectedFileId: "target", + selectedHunkIndex: 1, + separatorWidth: 92, + width: 100, + }); + const setup = await testRender(, { + width: 104, + height: 12, + }); + + try { + await settleDiffPane(setup); + const frame = await waitForFrame(setup, (nextFrame) => nextFrame.includes("line60 = 5901")); + + expect(frame).toContain("line60 = 5901"); + expect(frame).not.toContain("line1 = 1001"); + } finally { + if (previousRowWindowing === undefined) { + delete process.env.HUNK_ROW_WINDOWING_POC; + } else { + process.env.HUNK_ROW_WINDOWING_POC = previousRowWindowing; + } + + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("DiffPane keeps a selected hunk with inline notes fully visible when it fits", async () => { const theme = resolveTheme("midnight", null); const file = createViewportSizedBottomHunkDiffFile("target", "target.ts"); diff --git a/test/pty/ui-integration.test.ts b/test/pty/ui-integration.test.ts index d8f6b539..6ab31d1e 100644 --- a/test/pty/ui-integration.test.ts +++ b/test/pty/ui-integration.test.ts @@ -163,6 +163,49 @@ describe("live UI integration", () => { } }); + test("row-windowing PTY sessions can navigate forward and backward between distant hunks", async () => { + const fixture = harness.createMultiHunkFilePair(); + const session = await harness.launchHunk({ + args: ["diff", fixture.before, fixture.after, "--mode", "split"], + cols: 104, + rows: 12, + env: { + HUNK_ROW_WINDOWING_POC: "1", + }, + }); + + try { + const initial = await session.waitForText(/View\s+Navigate\s+Theme\s+Agent\s+Help/, { + timeout: 15_000, + }); + + expect(initial).toContain("line1 = 100"); + expect(initial).not.toContain("line60 = 6000"); + + await session.press("]"); + const secondHunk = await harness.waitForSnapshot( + session, + (text) => text.includes("line60 = 6000") && !text.includes("line1 = 100"), + 5_000, + ); + + expect(secondHunk).toContain("line60 = 6000"); + expect(secondHunk).not.toContain("line1 = 100"); + + await session.press("["); + const firstHunk = await harness.waitForSnapshot( + session, + (text) => text.includes("line1 = 100") && !text.includes("line60 = 6000"), + 5_000, + ); + + expect(firstHunk).toContain("line1 = 100"); + expect(firstHunk).not.toContain("line60 = 6000"); + } finally { + session.close(); + } + }); + test("a short last file does not trap upward scrolling at the bottom edge", async () => { const fixture = harness.createBottomClampedRepoFixture(); const session = await harness.launchHunk({ From 1686be9ae13a444d80fd484238dec601301bb88e Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Fri, 8 May 2026 21:52:35 -0400 Subject: [PATCH 5/9] feat(diff): keep adjacent hunks mounted while windowing --- src/ui/components/panes/DiffPane.tsx | 34 ++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index 0b47cd8c..7e504d7b 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -42,6 +42,7 @@ import { prefetchHighlightedDiff } from "../../diff/useHighlightedDiff"; const EMPTY_VISIBLE_AGENT_NOTES: VisibleAgentNote[] = []; const EMPTY_VISIBLE_AGENT_NOTES_BY_FILE = new Map(); +const ROW_WINDOWING_ADJACENT_HUNK_RADIUS = 1; /** * Clamp one vertical scroll target into the currently reachable review-stream extent. @@ -618,17 +619,30 @@ export function DiffPane({ scrollViewport.top + scrollViewport.height - sectionLayout.bodyTop + overscanRows; if (file.id === selectedFileId) { - const selectedHunkBounds = geometry.hunkBounds.get( - Math.max(0, Math.min(selectedHunkIndex, file.metadata.hunks.length - 1)), + const clampedSelectedHunkIndex = Math.max( + 0, + Math.min(selectedHunkIndex, file.metadata.hunks.length - 1), ); - if (selectedHunkBounds) { - // Always keep the selected hunk inside the mounted slice even if the viewport is a little - // ahead or behind it. That avoids unmounting the active target during navigation settles. - minTop = Math.min(minTop, selectedHunkBounds.top - overscanRows); - maxBottom = Math.max( - maxBottom, - selectedHunkBounds.top + selectedHunkBounds.height + overscanRows, - ); + const firstKeptHunkIndex = Math.max( + 0, + clampedSelectedHunkIndex - ROW_WINDOWING_ADJACENT_HUNK_RADIUS, + ); + const lastKeptHunkIndex = Math.min( + file.metadata.hunks.length - 1, + clampedSelectedHunkIndex + ROW_WINDOWING_ADJACENT_HUNK_RADIUS, + ); + + // Keep a small neighborhood of hunks mounted around the active selection, not just the + // selected hunk itself. That trades a little more mounted work for less churn when the user + // wheels across a hunk boundary or hits `[` / `]` repeatedly through nearby hunks. + for (let hunkIndex = firstKeptHunkIndex; hunkIndex <= lastKeptHunkIndex; hunkIndex += 1) { + const hunkBounds = geometry.hunkBounds.get(hunkIndex); + if (!hunkBounds) { + continue; + } + + minTop = Math.min(minTop, hunkBounds.top - overscanRows); + maxBottom = Math.max(maxBottom, hunkBounds.top + hunkBounds.height + overscanRows); } } From 5db281388949f3eebd55f2bb450122d1244b9908 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Fri, 8 May 2026 22:08:44 -0400 Subject: [PATCH 6/9] revert(diff): drop adjacent-hunk row windowing --- src/ui/components/panes/DiffPane.tsx | 34 ++++++++-------------------- 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index 7e504d7b..0b47cd8c 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -42,7 +42,6 @@ import { prefetchHighlightedDiff } from "../../diff/useHighlightedDiff"; const EMPTY_VISIBLE_AGENT_NOTES: VisibleAgentNote[] = []; const EMPTY_VISIBLE_AGENT_NOTES_BY_FILE = new Map(); -const ROW_WINDOWING_ADJACENT_HUNK_RADIUS = 1; /** * Clamp one vertical scroll target into the currently reachable review-stream extent. @@ -619,30 +618,17 @@ export function DiffPane({ scrollViewport.top + scrollViewport.height - sectionLayout.bodyTop + overscanRows; if (file.id === selectedFileId) { - const clampedSelectedHunkIndex = Math.max( - 0, - Math.min(selectedHunkIndex, file.metadata.hunks.length - 1), + const selectedHunkBounds = geometry.hunkBounds.get( + Math.max(0, Math.min(selectedHunkIndex, file.metadata.hunks.length - 1)), ); - const firstKeptHunkIndex = Math.max( - 0, - clampedSelectedHunkIndex - ROW_WINDOWING_ADJACENT_HUNK_RADIUS, - ); - const lastKeptHunkIndex = Math.min( - file.metadata.hunks.length - 1, - clampedSelectedHunkIndex + ROW_WINDOWING_ADJACENT_HUNK_RADIUS, - ); - - // Keep a small neighborhood of hunks mounted around the active selection, not just the - // selected hunk itself. That trades a little more mounted work for less churn when the user - // wheels across a hunk boundary or hits `[` / `]` repeatedly through nearby hunks. - for (let hunkIndex = firstKeptHunkIndex; hunkIndex <= lastKeptHunkIndex; hunkIndex += 1) { - const hunkBounds = geometry.hunkBounds.get(hunkIndex); - if (!hunkBounds) { - continue; - } - - minTop = Math.min(minTop, hunkBounds.top - overscanRows); - maxBottom = Math.max(maxBottom, hunkBounds.top + hunkBounds.height + overscanRows); + if (selectedHunkBounds) { + // Always keep the selected hunk inside the mounted slice even if the viewport is a little + // ahead or behind it. That avoids unmounting the active target during navigation settles. + minTop = Math.min(minTop, selectedHunkBounds.top - overscanRows); + maxBottom = Math.max( + maxBottom, + selectedHunkBounds.top + selectedHunkBounds.height + overscanRows, + ); } } From 56c7c40f962807fcf88ee842b51b699c415c1e12 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Fri, 8 May 2026 22:10:33 -0400 Subject: [PATCH 7/9] refactor(diff): enable row windowing by default --- src/ui/components/panes/DiffPane.tsx | 4 ++-- src/ui/components/ui-components.test.tsx | 11 +---------- src/ui/diff/PierreDiffView.tsx | 8 ++------ src/ui/diff/rowWindowing.ts | 7 ------- test/pty/ui-integration.test.ts | 5 +---- 5 files changed, 6 insertions(+), 29 deletions(-) diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index 0b47cd8c..72772e3c 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -37,7 +37,7 @@ import { DiffSection } from "./DiffSection"; import { DiffFileHeaderRow } from "./DiffFileHeaderRow"; import { DiffSectionPlaceholder } from "./DiffSectionPlaceholder"; import { VerticalScrollbar, type VerticalScrollbarHandle } from "../scrollbar/VerticalScrollbar"; -import { rowWindowingPocEnabled, type VisibleBodyBounds } from "../../diff/rowWindowing"; +import type { VisibleBodyBounds } from "../../diff/rowWindowing"; import { prefetchHighlightedDiff } from "../../diff/useHighlightedDiff"; const EMPTY_VISIBLE_AGENT_NOTES: VisibleAgentNote[] = []; @@ -592,7 +592,7 @@ export function DiffPane({ }, [adjacentPrefetchFileIds, selectedFileId, visibleViewportFileIds, windowingEnabled]); const visibleBodyBoundsByFile = useMemo(() => { const next = new Map(); - if (!rowWindowingPocEnabled() || scrollViewport.height <= 0) { + if (scrollViewport.height <= 0) { return next; } diff --git a/src/ui/components/ui-components.test.tsx b/src/ui/components/ui-components.test.tsx index 9cd431d1..1dfee062 100644 --- a/src/ui/components/ui-components.test.tsx +++ b/src/ui/components/ui-components.test.tsx @@ -1001,10 +1001,7 @@ describe("UI components", () => { } }); - test("DiffPane keeps a distant selected hunk visible when row windowing is enabled", async () => { - const previousRowWindowing = process.env.HUNK_ROW_WINDOWING_POC; - process.env.HUNK_ROW_WINDOWING_POC = "1"; - + test("DiffPane keeps a distant selected hunk visible when row windowing narrows one file body", async () => { const theme = resolveTheme("midnight", null); const props = createDiffPaneProps([createWideTwoHunkDiffFile("target", "target.ts")], theme, { diffContentWidth: 96, @@ -1026,12 +1023,6 @@ describe("UI components", () => { expect(frame).toContain("line60 = 5901"); expect(frame).not.toContain("line1 = 1001"); } finally { - if (previousRowWindowing === undefined) { - delete process.env.HUNK_ROW_WINDOWING_POC; - } else { - process.env.HUNK_ROW_WINDOWING_POC = previousRowWindowing; - } - await act(async () => { setup.renderer.destroy(); }); diff --git a/src/ui/diff/PierreDiffView.tsx b/src/ui/diff/PierreDiffView.tsx index 212744db..0060525d 100644 --- a/src/ui/diff/PierreDiffView.tsx +++ b/src/ui/diff/PierreDiffView.tsx @@ -9,11 +9,7 @@ import { findMaxLineNumber } from "./codeColumns"; import { buildSplitRows, buildStackRows } from "./pierre"; import { plannedReviewRowVisible } from "./plannedReviewRows"; import { buildReviewRenderPlan } from "./reviewRenderPlan"; -import { - resolveVisiblePlannedRowWindow, - rowWindowingPocEnabled, - type VisibleBodyBounds, -} from "./rowWindowing"; +import { resolveVisiblePlannedRowWindow, type VisibleBodyBounds } from "./rowWindowing"; import { diffMessage, DiffRowView, fitText } from "./renderRows"; import { useHighlightedDiff } from "./useHighlightedDiff"; @@ -91,7 +87,7 @@ export function PierreDiffView({ // - one file-local visible body slice from DiffPane // The helper relies on those structures staying in lockstep, so any missing input means // "render everything" instead of risking a mismatched partial slice. - if (!sectionGeometry || !visibleBodyBounds || !rowWindowingPocEnabled()) { + if (!sectionGeometry || !visibleBodyBounds) { return { bottomSpacerHeight: 0, plannedRows, diff --git a/src/ui/diff/rowWindowing.ts b/src/ui/diff/rowWindowing.ts index 365c0d29..12086cc0 100644 --- a/src/ui/diff/rowWindowing.ts +++ b/src/ui/diff/rowWindowing.ts @@ -1,8 +1,6 @@ import type { DiffSectionGeometry } from "../lib/diffSectionGeometry"; import type { PlannedReviewRow } from "./reviewRenderPlan"; -const ROW_WINDOWING_POC_ENV = "HUNK_ROW_WINDOWING_POC"; - /** One visible slice within a file body, measured in file-local row units. */ export interface VisibleBodyBounds { top: number; @@ -15,11 +13,6 @@ export interface VisiblePlannedRowWindow { topSpacerHeight: number; } -/** Opt-in gate for the row-windowing proof of concept while we validate behavior and gains. */ -export function rowWindowingPocEnabled(env: NodeJS.ProcessEnv = process.env) { - return env[ROW_WINDOWING_POC_ENV] === "1"; -} - /** * Slice planned rows down to the visible body range while preserving total section height. * diff --git a/test/pty/ui-integration.test.ts b/test/pty/ui-integration.test.ts index 6ab31d1e..cd769913 100644 --- a/test/pty/ui-integration.test.ts +++ b/test/pty/ui-integration.test.ts @@ -163,15 +163,12 @@ describe("live UI integration", () => { } }); - test("row-windowing PTY sessions can navigate forward and backward between distant hunks", async () => { + test("PTY sessions can navigate forward and backward between distant hunks in one large file", async () => { const fixture = harness.createMultiHunkFilePair(); const session = await harness.launchHunk({ args: ["diff", fixture.before, fixture.after, "--mode", "split"], cols: 104, rows: 12, - env: { - HUNK_ROW_WINDOWING_POC: "1", - }, }); try { From f0aa2f686c32a2ced27bc3204eea712e731b178b Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Fri, 8 May 2026 22:45:09 -0400 Subject: [PATCH 8/9] fix(diff): bound selected hunk row windowing --- src/ui/components/panes/DiffPane.tsx | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index 72772e3c..5d6d0b52 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -617,24 +617,13 @@ export function DiffPane({ let maxBottom = scrollViewport.top + scrollViewport.height - sectionLayout.bodyTop + overscanRows; - if (file.id === selectedFileId) { - const selectedHunkBounds = geometry.hunkBounds.get( - Math.max(0, Math.min(selectedHunkIndex, file.metadata.hunks.length - 1)), - ); - if (selectedHunkBounds) { - // Always keep the selected hunk inside the mounted slice even if the viewport is a little - // ahead or behind it. That avoids unmounting the active target during navigation settles. - minTop = Math.min(minTop, selectedHunkBounds.top - overscanRows); - maxBottom = Math.max( - maxBottom, - selectedHunkBounds.top + selectedHunkBounds.height + overscanRows, - ); - } - } + // Keep the mounted rows bounded to the viewport slice. Selection reveal uses planned hunk + // geometry as its fallback, so mounting an offscreen selected hunk is not necessary and would + // remount very large hunks in full. // Clamp the requested file-local interval back into the real body extent, then store it as // { top, height } so the row slicer can rebuild the matching [top, bottom) window later. - const clampedTop = Math.max(0, minTop); + const clampedTop = Math.min(geometry.bodyHeight, Math.max(0, minTop)); const clampedBottom = Math.min(geometry.bodyHeight, Math.max(clampedTop, maxBottom)); next.set(file.id, { top: clampedTop, @@ -649,8 +638,6 @@ export function DiffPane({ scrollViewport.height, scrollViewport.top, sectionGeometry, - selectedFileId, - selectedHunkIndex, visibleWindowedFileIds, ]); From 04970a31ae8148121b43e5f92537d85cb959d0fe Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Fri, 8 May 2026 22:47:43 -0400 Subject: [PATCH 9/9] fix(diff): address row windowing review notes --- src/ui/components/panes/DiffPane.tsx | 14 +++++++------- src/ui/diff/rowWindowing.test.ts | 23 ++++++++++++++++++++++- src/ui/diff/rowWindowing.ts | 6 ++++-- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index 5d6d0b52..76bb427c 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -386,9 +386,9 @@ export function DiffPane({ ); const visibleViewportFileIds = useMemo(() => { - const overscanRows = 8; - const minVisibleY = Math.max(0, scrollViewport.top - overscanRows); - const maxVisibleY = scrollViewport.top + scrollViewport.height + overscanRows; + const overscanTerminalRows = 8; + const minVisibleY = Math.max(0, scrollViewport.top - overscanTerminalRows); + const maxVisibleY = scrollViewport.top + scrollViewport.height + overscanTerminalRows; return collectIntersectingFileSectionIds(baseFileSectionLayouts, minVisibleY, maxVisibleY); }, [baseFileSectionLayouts, scrollViewport.height, scrollViewport.top]); @@ -596,7 +596,7 @@ export function DiffPane({ return next; } - const overscanRows = Math.max(24, scrollViewport.height * 2); + const overscanTerminalRows = Math.max(24, scrollViewport.height * 2); files.forEach((file, index) => { const sectionLayout = fileSectionLayouts[index]; @@ -613,9 +613,9 @@ export function DiffPane({ // Convert the absolute review-stream viewport into file-body-local coordinates. // Example: if the viewport starts at row 2_000 globally and this file body starts at row // 1_940, then the file-local visible top is 60 rows into this file. - let minTop = scrollViewport.top - sectionLayout.bodyTop - overscanRows; - let maxBottom = - scrollViewport.top + scrollViewport.height - sectionLayout.bodyTop + overscanRows; + const minTop = scrollViewport.top - sectionLayout.bodyTop - overscanTerminalRows; + const maxBottom = + scrollViewport.top + scrollViewport.height - sectionLayout.bodyTop + overscanTerminalRows; // Keep the mounted rows bounded to the viewport slice. Selection reveal uses planned hunk // geometry as its fallback, so mounting an offscreen selected hunk is not necessary and would diff --git a/src/ui/diff/rowWindowing.test.ts b/src/ui/diff/rowWindowing.test.ts index ef1b5296..3dd47c38 100644 --- a/src/ui/diff/rowWindowing.test.ts +++ b/src/ui/diff/rowWindowing.test.ts @@ -95,7 +95,7 @@ describe("resolveVisiblePlannedRowWindow", () => { ]); }); - test("can collapse a fully offscreen file body into spacer height only", () => { + test("can collapse a fully offscreen file body above the viewport into top spacer height", () => { const plannedRows = ["row:0", "row:1"].map(createTestPlannedRow); const sectionGeometry = createTestSectionGeometry( [ @@ -111,6 +111,27 @@ describe("resolveVisiblePlannedRowWindow", () => { visibleBodyBounds: { top: 10, height: 2 }, }); + expect(window.topSpacerHeight).toBe(4); + expect(window.bottomSpacerHeight).toBe(0); + expect(window.plannedRows).toHaveLength(0); + }); + + test("can collapse a fully offscreen file body below the viewport into bottom spacer height", () => { + const plannedRows = ["row:0", "row:1"].map(createTestPlannedRow); + const sectionGeometry = createTestSectionGeometry( + [ + { key: "row:0", top: 0, height: 2 }, + { key: "row:1", top: 2, height: 2 }, + ], + 4, + ); + + const window = resolveVisiblePlannedRowWindow({ + plannedRows, + sectionGeometry, + visibleBodyBounds: { top: 0, height: 0 }, + }); + expect(window.topSpacerHeight).toBe(0); expect(window.bottomSpacerHeight).toBe(4); expect(window.plannedRows).toHaveLength(0); diff --git a/src/ui/diff/rowWindowing.ts b/src/ui/diff/rowWindowing.ts index 12086cc0..237cbbd2 100644 --- a/src/ui/diff/rowWindowing.ts +++ b/src/ui/diff/rowWindowing.ts @@ -67,10 +67,12 @@ export function resolveVisiblePlannedRowWindow({ } if (firstVisibleIndex < 0 || lastVisibleIndex < 0) { + const topSpacerHeight = Math.min(sectionGeometry.bodyHeight, minVisibleTop); + return { - bottomSpacerHeight: sectionGeometry.bodyHeight, + bottomSpacerHeight: Math.max(0, sectionGeometry.bodyHeight - topSpacerHeight), plannedRows: [], - topSpacerHeight: 0, + topSpacerHeight, }; }