From 565e8bb36ad1785e938d6c37931a2f105a616547 Mon Sep 17 00:00:00 2001
From: Ben Vinegar <2153+benvinegar@users.noreply.github.com>
Date: Sat, 17 Oct 2026 20:45:44 -0400
Subject: [PATCH] fix(ui): reuse nowrap geometry across terminal resizes
---
.changeset/tidy-nowrap-geometry.md | 5 +
.../hunk/src/ui/AppHost.responsive.test.tsx | 37 ++++
.../src/ui/diff/diffSectionGeometry.test.ts | 159 +++++++++++++++++-
.../hunk/src/ui/diff/diffSectionGeometry.ts | 13 +-
test/pty/nowrap-resize-integration.test.ts | 82 +++++++++
5 files changed, 281 insertions(+), 15 deletions(-)
create mode 100644 .changeset/tidy-nowrap-geometry.md
create mode 100644 test/pty/nowrap-resize-integration.test.ts
diff --git a/.changeset/tidy-nowrap-geometry.md b/.changeset/tidy-nowrap-geometry.md
new file mode 100644
index 000000000..610f92a6a
--- /dev/null
+++ b/.changeset/tidy-nowrap-geometry.md
@@ -0,0 +1,5 @@
+---
+"hunkdiff": patch
+---
+
+Reduce diff geometry allocation and retention when resizing nonwrapped reviews without inline notes.
diff --git a/packages/hunk/src/ui/AppHost.responsive.test.tsx b/packages/hunk/src/ui/AppHost.responsive.test.tsx
index 7e84c6b7c..630a45bef 100644
--- a/packages/hunk/src/ui/AppHost.responsive.test.tsx
+++ b/packages/hunk/src/ui/AppHost.responsive.test.tsx
@@ -274,6 +274,43 @@ describe("responsive app", () => {
expect(at).toMatch(/alpha = 1;.*alpha = 2;/);
});
+ for (const layout of ["split", "unified"] as const) {
+ test(`fixed ${layout} nowrap resizes preserve stream rows while repainting live columns`, async () => {
+ const bootstrap = createBootstrap(layout);
+ const setup = await testRender(, { width: 140, height: 24 });
+ try {
+ await act(async () => {
+ await setup.renderOnce();
+ });
+ const first = setup.captureCharFrame();
+ for (const width of [100, 150, 110, 140, 100, 140]) {
+ await act(async () => {
+ setup.resize(width, 24);
+ await setup.renderOnce();
+ });
+ const frame = setup.captureCharFrame();
+ expect(frame.indexOf("alpha.ts")).toBeLessThan(frame.indexOf("beta.ts"));
+ expect(frame).toContain("alpha = 2");
+ expect(frame).toContain("betaValue = 1");
+ expect(/▌.*▌/.test(frame)).toBe(layout === "split");
+ // Widths stay below the automatic sidebar cutoff, isolating code-column repainting.
+ const row = frame.split("\n").find((line) => line.includes("alpha = 2"))!;
+ if (layout === "split") {
+ const initialRow = first.split("\n").find((line) => line.includes("alpha = 2"))!;
+ expect(row.indexOf("alpha = 2") - initialRow.indexOf("alpha = 2")).toBe(
+ (width - 140) / 2,
+ );
+ }
+ if (width === 140) expect(frame).toBe(first);
+ }
+ } finally {
+ await act(async () => {
+ setup.renderer.destroy();
+ });
+ }
+ });
+ }
+
test("pager mode stays responsive while hiding app chrome", async () => {
const wide = await captureFrameForBootstrap(createBootstrap("auto", true), 220);
const narrow = await captureFrameForBootstrap(createBootstrap("auto", true), 150);
diff --git a/packages/hunk/src/ui/diff/diffSectionGeometry.test.ts b/packages/hunk/src/ui/diff/diffSectionGeometry.test.ts
index 39f50fd1f..d18340dd1 100644
--- a/packages/hunk/src/ui/diff/diffSectionGeometry.test.ts
+++ b/packages/hunk/src/ui/diff/diffSectionGeometry.test.ts
@@ -138,13 +138,65 @@ describe("measureDiffSectionGeometry", () => {
);
expect(second).toBe(first);
- expect(differentWidth).not.toBe(first);
+ expect(differentWidth).toBe(first);
expect(differentAddNotePolicy).not.toBe(first);
expect(differentTabWidth).not.toBe(first);
expect(differentHunkGap).not.toBe(first);
expect(differentHunkGap.bodyHeight).toBeGreaterThan(first.bodyHeight);
});
+ for (const layout of ["split", "unified"] as const) {
+ test(`${layout} reuses nowrap geometry across zero, tiny, and normal widths with fresh-measurement parity`, () => {
+ const before = Array.from({ length: 100 }, (_, index) => `\tline ${index + 1}\n`).join("");
+ const after = before
+ .replace("line 5\n", "line 5 modified\n")
+ .replace("line 75\n", "line 75 modified\n");
+ const file = createTestDiffFile({ before, after, id: "resize", path: "resize.txt" });
+ const expandedKeys = new Set(["trailing:1"]);
+ const sourceStatus = { kind: "loaded", text: after } as const;
+
+ for (const showHeaders of [false, true]) {
+ /** Measure the same expanded row stream under one fixed rendering policy. */
+ const measure = (input: typeof file, width: number) =>
+ measureDiffSectionGeometry(
+ input,
+ layout,
+ showHeaders,
+ theme,
+ [],
+ width,
+ showHeaders,
+ false,
+ expandedKeys,
+ sourceStatus,
+ !showHeaders,
+ showHeaders ? 4 : 8,
+ 2,
+ );
+ const first = measure(file, 0);
+ const lazyGetter = Object.getOwnPropertyDescriptor(first, "plannedRows")?.get;
+ expect(lazyGetter).toBeFunction();
+ expect(first.hunkBounds.size).toBe(2);
+ expect(first.rowBounds.some((row) => row.expandedGapKey)).toBe(true);
+
+ for (const width of [1, 2, 9, 32, 160, 240, 200, 0, 160]) {
+ const reused = measure(file, width);
+ expect(reused).toBe(first);
+ // A fresh file identity prevents cache reuse from masking width-dependent content.
+ const fresh = measure({ ...file }, width);
+ expect(reused).toEqual(fresh);
+ expect(reused.plannedRows.map((row) => row.key)).toEqual(
+ reused.rowBounds.map((row) => row.key),
+ );
+ for (const bounds of reused.rowBounds) {
+ expect(reused.rowBoundsByKey.get(bounds.key)).toBe(bounds);
+ expect(reused.rowBoundsByStableKey.get(bounds.stableKey)).toBe(bounds);
+ }
+ }
+ }
+ });
+ }
+
test("caches planned rows after the lazy geometry property is first read", () => {
const geometry = measureDiffSectionGeometry(
createTestDiffFile(),
@@ -206,7 +258,7 @@ describe("measureDiffSectionGeometry", () => {
);
});
- test("replaces stale width variants while retaining separate base and note slots", () => {
+ test("reuses the nowrap base while replacing only the bounded note width slot", () => {
const file = createTestDiffFile();
const visibleAgentNotes: VisibleAgentNote[] = [
createVisibleAgentNote(file.metadata.hunks, {
@@ -214,7 +266,7 @@ describe("measureDiffSectionGeometry", () => {
annotation: {
newRange: [1, 1],
rationale: "Keep a note-aware geometry slot.",
- summary: "Explain",
+ summary: "Explain this change with a long inline note. ".repeat(20),
},
}),
];
@@ -232,13 +284,24 @@ describe("measureDiffSectionGeometry", () => {
);
const width101 = measureDiffSectionGeometry(file, "split", true, theme, [], 101, true, false);
- expect(width101).not.toBe(width100);
- expect(measureDiffSectionGeometry(file, "split", true, theme, [], 100, true, false)).not.toBe(
- width100,
+ expect(width101).toBe(width100);
+ expect(measureDiffSectionGeometry(file, "split", true, theme, visibleAgentNotes, 100)).toBe(
+ noteWidth100,
+ );
+ const noteWidth200 = measureDiffSectionGeometry(
+ file,
+ "split",
+ true,
+ theme,
+ visibleAgentNotes,
+ 200,
+ );
+ expect(noteWidth200).not.toBe(noteWidth100);
+ expect(noteWidth200.bodyHeight).toBeLessThan(noteWidth100.bodyHeight);
+ expect(measureDiffSectionGeometry(file, "split", true, theme, [], 200)).toBe(width100);
+ expect(measureDiffSectionGeometry(file, "split", true, theme, visibleAgentNotes, 100)).not.toBe(
+ noteWidth100,
);
- expect(
- measureDiffSectionGeometry(file, "split", true, theme, visibleAgentNotes, 100, true, false),
- ).toBe(noteWidth100);
});
test("accounts for visible inline notes without moving the hunk anchor", () => {
@@ -350,6 +413,84 @@ describe("measureDiffSectionGeometry", () => {
);
});
+ for (const layout of ["split", "unified"] as const) {
+ test(`${layout} replaces bounded wrapped width variants and preserves wrap/layout invalidation`, () => {
+ const file = createTestDiffFile({
+ before: "old\n",
+ after: `${"long code ".repeat(40)}\n`,
+ id: "width-sensitive",
+ path: "long.txt",
+ });
+ const narrow = measureDiffSectionGeometry(file, layout, true, theme, [], 32, true, true);
+ const wide = measureDiffSectionGeometry(file, layout, true, theme, [], 200, true, true);
+ expect(wide).not.toBe(narrow);
+ expect(wide.bodyHeight).toBeLessThan(narrow.bodyHeight);
+ expect(measureDiffSectionGeometry(file, layout, true, theme, [], 200, true, true)).toBe(wide);
+ expect(measureDiffSectionGeometry(file, layout, true, theme, [], 32, true, true)).not.toBe(
+ narrow,
+ );
+ const nowrap = measureDiffSectionGeometry(file, layout, true, theme, [], 32);
+ expect(nowrap).not.toBe(narrow);
+ expect(nowrap.bodyHeight).toBeLessThan(narrow.bodyHeight);
+ expect(
+ measureDiffSectionGeometry(
+ file,
+ layout === "split" ? "unified" : "split",
+ true,
+ theme,
+ [],
+ 32,
+ ),
+ ).not.toBe(nowrap);
+ });
+ }
+
+ test("nowrap expanded geometry invalidates source status and same-length source edits", () => {
+ const before = Array.from({ length: 30 }, (_, index) => `line ${index + 1}\n`).join("");
+ const after = before.replace("line 5\n", "line 5 modified\n");
+ const file = createTestDiffFile({ before, after, id: "source", path: "source.txt" });
+ const expandedKeys = new Set(["trailing:0"]);
+ const statuses = [
+ undefined,
+ { kind: "loading" },
+ { kind: "error" },
+ { kind: "loaded", text: after },
+ { kind: "loaded", text: after.replace("line 20", "edit 20") },
+ ] as const;
+ let previous = measureDiffSectionGeometry(file, "split", true, theme, [], 100);
+ for (const status of statuses) {
+ const geometry = measureDiffSectionGeometry(
+ file,
+ "split",
+ true,
+ theme,
+ [],
+ 120,
+ true,
+ false,
+ expandedKeys,
+ status,
+ );
+ expect(geometry).not.toBe(previous);
+ expect(geometry).toEqual(
+ measureDiffSectionGeometry(
+ { ...file },
+ "split",
+ true,
+ theme,
+ [],
+ 1,
+ true,
+ false,
+ expandedKeys,
+ status,
+ ),
+ );
+ previous = geometry;
+ }
+ expect(JSON.stringify(previous.plannedRows)).toContain("edit 20");
+ });
+
test("returns a one-row placeholder for files with no visible hunks", () => {
const file = createTestDiffFile({
after: "const stable = true;\n",
diff --git a/packages/hunk/src/ui/diff/diffSectionGeometry.ts b/packages/hunk/src/ui/diff/diffSectionGeometry.ts
index 3f70c7757..a71be4bf6 100644
--- a/packages/hunk/src/ui/diff/diffSectionGeometry.ts
+++ b/packages/hunk/src/ui/diff/diffSectionGeometry.ts
@@ -123,8 +123,8 @@ interface SectionGeometryCacheSlots {
// Cache geometry by immutable DiffFile object so hunk navigation can survive parent re-renders
// without rebuilding every file section. Replaced file objects naturally drop from the WeakMap.
-// Keep one no-note and one note-aware variant: large row-bounds arrays from old terminal widths
-// are replaced on resize, while note rendering can still compare against a stable base geometry.
+// Keep one no-note and one note-aware variant. Note-free nowrap geometry survives resizing;
+// width-sensitive variants replace old row-bounds arrays within their own bounded slot.
const SECTION_GEOMETRY_CACHE = new WeakMap();
/** Resolve which bounded per-file cache slot owns one geometry variant. */
@@ -310,9 +310,10 @@ export function measureDiffSectionGeometry(
};
}
- // Width, wrapping, and line-number visibility all affect rendered row heights, so they must
- // participate in the cache key alongside the structural file/layout inputs. Expansion state
- // changes the row stream, so it has to participate too.
+ // Only wrapped code and inline notes have width-sensitive heights. Keep live width for
+ // measurement, but reuse note-free nowrap bounds and lazy rows across terminal resizes.
+ // Preserve structural and rendering policies, including expansion/source state, in the key.
+ const geometryCacheWidth = wrapLines || visibleAgentNotes.length > 0 ? width : 0;
const themeCacheKey = [
theme.id,
theme.syntaxTheme ?? "",
@@ -327,7 +328,7 @@ export function measureDiffSectionGeometry(
const scopeCacheKey = visibleHunkIndexes
? `:scope:${[...visibleHunkIndexes].sort((a, b) => a - b).join(",")}`
: "";
- const cacheKey = `${file.id}:${layout}:${showHunkHeaders ? 1 : 0}:${themeCacheKey}:${width}:${showLineNumbers ? 1 : 0}:${wrapLines ? 1 : 0}:${reserveAddNoteColumn ? 1 : 0}:tabs:${tabWidth}:hunkGap:${hunkGap}${scopeCacheKey}${expansionCacheKey(expandedKeys, sourceStatus)}${notesCacheKey(visibleAgentNotes)}`;
+ const cacheKey = `${file.id}:${layout}:${showHunkHeaders ? 1 : 0}:${themeCacheKey}:${geometryCacheWidth}:${showLineNumbers ? 1 : 0}:${wrapLines ? 1 : 0}:${reserveAddNoteColumn ? 1 : 0}:tabs:${tabWidth}:hunkGap:${hunkGap}${scopeCacheKey}${expansionCacheKey(expandedKeys, sourceStatus)}${notesCacheKey(visibleAgentNotes)}`;
const cacheSlot = sectionGeometryCacheSlot(visibleAgentNotes);
const cached = getCachedSectionGeometry(file, cacheSlot, cacheKey);
if (cached) {
diff --git a/test/pty/nowrap-resize-integration.test.ts b/test/pty/nowrap-resize-integration.test.ts
new file mode 100644
index 000000000..9ea2c0ff1
--- /dev/null
+++ b/test/pty/nowrap-resize-integration.test.ts
@@ -0,0 +1,82 @@
+import { afterEach, expect, setDefaultTimeout, test } from "bun:test";
+import { createPtyHarness } from "./harness";
+
+const harness = createPtyHarness();
+
+/** Allow real terminal startup and repeated resize acknowledgements on slower CI hosts. */
+setDefaultTimeout(30_000);
+
+afterEach(() => {
+ harness.cleanup();
+});
+
+test("fixed split nowrap resizes preserve cross-file navigation and wrapping still reflows", async () => {
+ const fixture = harness.createPinnedHeaderRepoFixture();
+ const session = await harness.launchHunk({
+ args: ["diff", "--mode", "split", "--no-sidebar"],
+ cwd: fixture.dir,
+ cols: 120,
+ rows: 24,
+ });
+
+ try {
+ await harness.waitForSnapshot(session, (text) => text.includes("line01 = 101"), 15_000);
+ await harness.pressAndWaitForSnapshot(
+ session,
+ "]",
+ (text) => text.includes("line17 = 117") && !text.includes("line01 = 101"),
+ );
+
+ for (const cols of [180, 100, 140, 100, 180, 120]) {
+ session.resize({ cols, rows: 24 });
+ // A moved split separator acknowledges the live-width repaint, not a stale old frame.
+ const frame = await harness.waitForSnapshot(session, (text) => {
+ const row = text.split("\n").find((line) => line.includes("line17 = 117"));
+ return Boolean(row && row.lastIndexOf("▌") === Math.floor(cols / 2) - 1);
+ });
+ expect(frame).toContain("second.ts");
+ expect(frame).toContain("line32 = 132");
+ expect(frame).not.toContain("line01 = 101");
+ }
+
+ await harness.pressAndWaitForSnapshot(session, "[", (text) => text.includes("line01 = 101"));
+ await harness.pressAndWaitForSnapshot(
+ session,
+ "]",
+ (text) => text.includes("line17 = 117") && !text.includes("line01 = 101"),
+ );
+
+ await harness.pressAndWaitForSnapshot(session, "[", (text) => text.includes("line01 = 101"));
+ session.resize({ cols: 60, rows: 24 });
+ await harness.waitForSnapshot(session, (text) => {
+ const row = text.split("\n").find((line) => line.includes("line01"));
+ return Boolean(row && row.lastIndexOf("▌") === 29);
+ });
+ const wrapped = await harness.pressAndWaitForSnapshot(
+ session,
+ "w",
+ (text) => text.includes("101;") && !text.includes("line16"),
+ );
+ expect(wrapped).toContain("line01");
+
+ session.resize({ cols: 120, rows: 24 });
+ const wideWrapped = await harness.waitForSnapshot(
+ session,
+ (text) => text.includes("line01 = 101") && text.includes("line16 = 116"),
+ );
+ expect(wideWrapped).toMatch(/▌.*▌/);
+ session.resize({ cols: 60, rows: 24 });
+ await harness.waitForSnapshot(
+ session,
+ (text) => text.includes("101;") && !text.includes("line16"),
+ );
+ const nowrap = await harness.pressAndWaitForSnapshot(
+ session,
+ "w",
+ (text) => !text.includes("101;") && text.includes("line16"),
+ );
+ expect(nowrap).toContain("line01");
+ } finally {
+ session.close();
+ }
+});