fix(ui): keep copy-selection rows lazy during navigation

.changeset/lazy-navigation-copy-rows.md

@@ -0,0 +1,5 @@
+---
+"hunkdiff": patch
+---
+
+Reduce memory use in large reviews by keeping copy-selection row content lazy during keyboard navigation.

benchmarks/README.md

@@ -72,7 +72,7 @@ bun run bench:competitors
 - `huge-stream.ts` — opt-in huge tier (`--include-huge` or `HUNK_BENCH_INCLUDE_HUGE=1`): cold first frame, scroll-tick and hunk-navigation latency, and memory ceilings on ~1k files / 300k+ diff lines plus one giant ~50k-line file.
 - `large-stream-profile.ts` — optional local profiler for the main pure planning stages behind the large split-stream benchmark.
 - `memory.ts` — optional local RSS/heap profiler after fixture loading, planning, first frame, and next-hunk navigation.
-- `geometry-memory.ts` — optional local retained-memory profiler for all-files section geometry, including JSC-native heap metrics and giant-file lazy planned-row materialization latency used by first copy selection.
+- `geometry-memory.ts` — optional local retained-memory profiler for all-files section geometry, including JSC-native heap metrics, navigation projection, and giant-file first-copy latency. Copy-content heap growth is measured from the post-navigation sample; ordinary navigation should not materialize lazy copy-selection rows.
 - `navigation-memory.ts` — optional local retained-memory profiler for repeated hunk navigation through a mounted review stream.
 - `resize-memory.ts` — optional local retained-memory profiler for repeated terminal-width changes through a mounted review stream; this targets geometry-cache retention across resize variants.
 - `competitors.ts` — optional local informational comparisons against `git diff --no-ext-diff`, `delta`, `difftastic`, and `diff-so-fancy` when installed.

benchmarks/geometry-memory.ts

@@ -3,6 +3,8 @@ import { heapStats } from "bun:jsc";
 import { performance } from "node:perf_hooks";
 import { measureDiffSectionGeometry } from "../packages/hunk/src/ui/diff/diffSectionGeometry";
 import { resolveTheme } from "../packages/hunk/src/ui/themes";
+import { buildLineCursors } from "../packages/hunk/src/ui/lib/lineCursors";
+import { buildReviewVerticalStops } from "../packages/hunk/src/ui/lib/reviewVerticalStops";
 import {
   createGiantSingleDiffFile,
   createLargeSplitStreamBootstrap,
@@ -139,8 +141,18 @@ const geometryMs = performance.now() - geometryStart;
 const afterGeometry = sampleMemory(options);
 printMemory("after_geometry", afterGeometry);
 
-// Materialize the lazy planned-row streams as an upper-bound diagnostic for copy-selection style
-// consumers. Normal review scrolling/navigation should not pay this retained-memory cost.
+// Exercise real navigation before first copy so eager row-content materialization appears in
+// retained heap rather than hiding in the copy-only upper bound. Keep both projections live.
+const navigationStart = performance.now();
+const cursors = buildLineCursors(bootstrap.changeset.files, geometries);
+const stops = buildReviewVerticalStops(bootstrap.changeset.files, geometries, cursors);
+const navigationMs = performance.now() - navigationStart;
+const afterNavigation = sampleMemory(options);
+printMemory("after_navigation", afterNavigation);
+console.log(`METRIC navigation_ms=${navigationMs.toFixed(2)}`);
+console.log(`METRIC navigation_stops=${stops.length}`);
+
+// Materialize lazy row streams as an upper-bound diagnostic for copy-selection consumers.
 const materializeStart = performance.now();
 for (const geometry of geometries) {
   materializedPlannedRows += geometry.plannedRows.length;
@@ -162,12 +174,12 @@ console.log(`METRIC materialize_planned_rows_ms=${materializeMs.toFixed(2)}`);
 console.log(`METRIC materialized_planned_rows=${materializedPlannedRows}`);
 console.log(
   `METRIC materialized_planned_rows_jsc_heap_size_growth_bytes=${
-    afterMaterializedRows.jscHeapSizeBytes - afterGeometry.jscHeapSizeBytes
+    afterMaterializedRows.jscHeapSizeBytes - afterNavigation.jscHeapSizeBytes
   }`,
 );
 console.log(
   `METRIC materialized_planned_rows_rss_growth_bytes=${
-    afterMaterializedRows.rssBytes - afterGeometry.rssBytes
+    afterMaterializedRows.rssBytes - afterNavigation.rssBytes
   }`,
 );
 console.log(`METRIC files=${options.fileCount}`);

packages/hunk/src/ui/diff/diffSectionGeometry.ts

@@ -28,6 +28,8 @@ export interface DiffSectionRowBounds extends VerticalBounds {
   stableKeys: string[];
   /** Exact collapsed gap that produced this synthesized source row. */
   expandedGapKey?: string;
+  /** Selectable semantic note at this row; drafts and decoration-only cards omit it. */
+  noteTarget?: { noteId: string; hunkIndex: number };
 }
 
 /**
@@ -67,7 +69,7 @@ function notesCacheKey(visibleAgentNotes: VisibleAgentNote[]) {
   }
 
   return `:notes:${visibleAgentNotes
-    .map(({ annotation, actions, thread }) => {
+    .map(({ annotation, actions, thread, source }) => {
       const key = JSON.stringify({
         author: annotation.author,
         id: annotation.id,
@@ -77,6 +79,9 @@ function notesCacheKey(visibleAgentNotes: VisibleAgentNote[]) {
         source: annotation.source,
         summary: annotation.summary,
         title: annotation.title,
+        // Bounds also carry keyboard note targets, including whether this card is selectable.
+        noteId: thread?.noteId,
+        noteSource: source,
         parentId: thread?.parentId,
         threadDepth: thread?.depth,
         actions: actions
@@ -374,8 +379,13 @@ export function measureDiffSectionGeometry(
       row.stableKey,
       ...(row.kind === "diff-row" ? (row.stableAliasKeys ?? []) : []),
     ];
-    const rowBoundsEntry = {
+    const noteId =
+      row.kind === "inline-note" && row.note.source !== "draft"
+        ? row.note.thread?.noteId
+        : undefined;
+    const rowBoundsEntry: DiffSectionRowBounds = {
       key: row.key,
+      ...(noteId ? { noteTarget: { noteId, hunkIndex: row.hunkIndex } } : {}),
       stableKey: row.stableKey,
       stableKeys,
       ...(row.kind === "diff-row" &&

packages/hunk/src/ui/lib/reviewVerticalStops.test.ts

@@ -21,6 +21,26 @@ function stopLabel(stop: ReturnType<typeof buildReviewVerticalStops>[number]) {
 }
 
 describe("review vertical stops", () => {
+  for (const layout of ["split", "unified"] as const) {
+    test(`${layout} note-free navigation leaves copy-selection rows lazy`, () => {
+      const file = createTestDiffFile({
+        before: lines("one", "two"),
+        after: lines("ONE", "two"),
+        context: 1,
+      });
+      const geometry = measureDiffSectionGeometry(file, layout, true, theme, [], 80);
+      const cursors = buildLineCursors([file], [geometry]);
+      Object.defineProperty(geometry, "plannedRows", {
+        get() {
+          throw new Error("Navigation must not materialize copy-selection rows");
+        },
+      });
+
+      const stops = buildReviewVerticalStops([file], [geometry], cursors);
+      expect(stops).toEqual(cursors.map((cursor) => ({ kind: "line", cursor })));
+    });
+  }
+
   test("places semantic roots and replies after their rendered source line", () => {
     const file = createTestDiffFile({
       id: "alpha",
@@ -53,6 +73,11 @@ describe("review vertical stops", () => {
     ];
     const geometry = measureDiffSectionGeometry(file, "unified", true, theme, notes, 80);
     const cursors = buildLineCursors([file], [geometry]);
+    Object.defineProperty(geometry, "plannedRows", {
+      get() {
+        throw new Error("Navigation must not materialize copy-selection rows");
+      },
+    });
     const stops = buildReviewVerticalStops([file], [geometry], cursors);
     const labels = stops.map(stopLabel);
     const changedLine = labels.indexOf("line:new:3");
@@ -77,6 +102,45 @@ describe("review vertical stops", () => {
     expect(findNextReviewNoteStop(stops, stops[changedLine + 3]!, -1)?.noteId).toBe("reply");
   });
 
+  for (const layout of ["split", "unified"] as const) {
+    test(`${layout} remeasures cached note targets when semantic identity or draft status changes`, () => {
+      const file = createTestDiffFile({
+        before: lines("one", "two"),
+        after: lines("ONE", "two"),
+        context: 1,
+      });
+      const annotation = {
+        id: "stable-card",
+        source: "user",
+        summary: "Same card",
+        newRange: [1, 1] as [number, number],
+      };
+      const variants = [
+        { source: "user" as const, noteId: "first-note", expected: ["first-note"] },
+        { source: "user" as const, noteId: "replacement-note", expected: ["replacement-note"] },
+        { source: "draft" as const, noteId: "replacement-note", expected: [] },
+        { source: "user" as const, noteId: "replacement-note", expected: ["replacement-note"] },
+      ];
+      for (const { source, noteId, expected } of variants) {
+        const note = createVisibleAgentNote(file.metadata.hunks, {
+          id: "stable-card",
+          annotation,
+          source,
+          thread: { noteId, depth: 0 },
+        });
+        const geometry = measureDiffSectionGeometry(file, layout, true, theme, [note], 80);
+        const stops = buildReviewVerticalStops(
+          [file],
+          [geometry],
+          buildLineCursors([file], [geometry]),
+        );
+        expect(stops.filter((stop) => stop.kind === "note").map((stop) => stop.noteId)).toEqual(
+          expected,
+        );
+      }
+    });
+  }
+
   test("uses alternate file-view inline-note rows as selectable stops", () => {
     const file = createTestDiffFile({
       id: "alpha",

packages/hunk/src/ui/lib/reviewVerticalStops.ts

@@ -42,24 +42,23 @@ export function buildReviewVerticalStops(
     const geometry = sectionGeometry[sectionIndex];
     if (!geometry) return [];
 
-    const plannedRows = geometry.fileViewRows ?? geometry.plannedRows;
-    const plannedRowByStableKey = new Map(plannedRows.map((row) => [row.stableKey, row] as const));
+    // Alternate views already own a mounted row plan. Diff geometry carries just the measured
+    // note target, so keyboard navigation never materializes its lazy copy-selection content.
+    const fileViewRowByStableKey = geometry.fileViewRows
+      ? new Map(geometry.fileViewRows.map((row) => [row.stableKey, row] as const))
+      : undefined;
     const stops: ReviewVerticalStop[] = [];
 
     for (const bounds of geometry.rowBounds) {
-      const plannedRow = plannedRowByStableKey.get(bounds.stableKey);
-      if (plannedRow?.kind === "inline-note") {
-        const noteId =
-          plannedRow.note.source === "draft" ? undefined : plannedRow.note.thread?.noteId;
-        if (noteId) {
-          stops.push({
-            kind: "note",
-            fileId: file.id,
-            hunkIndex: plannedRow.hunkIndex,
-            noteId,
-            stableKey: bounds.stableKey,
-          });
-        }
+      const fileViewRow = fileViewRowByStableKey?.get(bounds.stableKey);
+      const target =
+        fileViewRow?.kind === "inline-note"
+          ? fileViewRow.note.source !== "draft" && fileViewRow.note.thread?.noteId
+            ? { noteId: fileViewRow.note.thread.noteId, hunkIndex: fileViewRow.hunkIndex }
+            : undefined
+          : bounds.noteTarget;
+      if (target) {
+        stops.push({ kind: "note", fileId: file.id, ...target, stableKey: bounds.stableKey });
         continue;
       }
 

test/pty/notes.test.ts

@@ -392,7 +392,7 @@ describe("PTY notes", () => {
     });
   }
 
-  test("saved notes are visibly active and keyboard-navigable without hover", async () => {
+  test("saved notes remain active and keyboard-navigable after live resizes without hover", async () => {
     const fixture = harness.createLongWrapFilePair();
     const session = await harness.launchHunk({
       args: ["diff", "--files", fixture.before, fixture.after, "--mode", "unified"],
@@ -427,6 +427,17 @@ describe("PTY notes", () => {
       expect(savedActive).toContain("E edit");
       expect(savedActive).toContain("D delete");
 
+      const noteBorderEnd = savedActive
+        .split("\n")
+        .find((line) => line.includes("● Your note"))!.length;
+      for (const cols of [80, 120, 100]) {
+        session.resize({ cols, rows: 30 });
+        await harness.waitForSnapshot(session, (text) => {
+          const border = text.split("\n").find((line) => line.includes("● Your note"));
+          return border?.length === noteBorderEnd + cols - 100;
+        });
+      }
+
       await session.press("k");
       const firstActive = await harness.pressAndWaitForSnapshot(
         session,