fix: keep selected hunks fully visible when they fit

src/ui/components/panes/DiffPane.tsx

@@ -10,6 +10,7 @@ import {
 } from "react";
 import type { DiffFile, LayoutMode } from "../../../core/types";
 import type { VisibleAgentNote } from "../../lib/agentAnnotations";
+import { computeHunkRevealScrollTop } from "../../lib/hunkScroll";
 import { measureDiffSectionMetrics } from "../../lib/sectionHeights";
 import { diffHunkId, diffSectionId } from "../../lib/ids";
 import type { AppTheme } from "../../themes";
@@ -193,15 +194,22 @@ export function DiffPane({
       return next;
     }
 
-    for (const fileId of visibleViewportFileIds) {
+    const fileIdsToMeasure = new Set(visibleViewportFileIds);
+    // Always measure the selected file with its real note rows so hunk navigation can compute
+    // accurate bounds even before the file scrolls into the visible viewport.
+    if (selectedFileId) {
+      fileIdsToMeasure.add(selectedFileId);
+    }
+
+    for (const fileId of fileIdsToMeasure) {
       const visibleNotes = allAgentNotesByFile.get(fileId);
       if (visibleNotes && visibleNotes.length > 0) {
         next.set(fileId, visibleNotes);
       }
     }
 
     return next;
-  }, [allAgentNotesByFile, showAgentNotes, visibleViewportFileIds]);
+  }, [allAgentNotesByFile, selectedFileId, showAgentNotes, visibleViewportFileIds]);
 
   const sectionMetrics = useMemo(
     () =>
@@ -278,43 +286,52 @@ export function DiffPane({
       ? diffHunkId(selectedFile.id, selectedHunkIndex)
       : diffSectionId(selectedFile.id)
     : null;
-  const selectedEstimatedScrollTop = useMemo(() => {
-    if (!selectedFile || selectedFileIndex < 0) {
+  const selectedEstimatedHunkBounds = useMemo(() => {
+    if (!selectedFile || selectedFileIndex < 0 || selectedFile.metadata.hunks.length === 0) {
       return null;
     }
 
-    let top = 0;
+    // Convert the selected hunk's file-local bounds into absolute scrollbox coordinates by adding
+    // the accumulated section chrome and earlier file heights.
+    let sectionTop = 0;
     for (let index = 0; index < selectedFileIndex; index += 1) {
-      top += (index > 0 ? 1 : 0) + 1 + (estimatedBodyHeights[index] ?? 0);
+      sectionTop += (index > 0 ? 1 : 0) + 1 + (estimatedBodyHeights[index] ?? 0);
     }
 
     if (selectedFileIndex > 0) {
-      top += 1;
+      sectionTop += 1;
     }
 
-    top += 1;
+    sectionTop += 1;
 
-    if (selectedFile.metadata.hunks.length > 0) {
-      const clampedHunkIndex = Math.max(
-        0,
-        Math.min(selectedHunkIndex, selectedFile.metadata.hunks.length - 1),
-      );
-      top += sectionMetrics[selectedFileIndex]?.hunkAnchorRows.get(clampedHunkIndex) ?? 0;
+    const clampedHunkIndex = Math.max(
+      0,
+      Math.min(selectedHunkIndex, selectedFile.metadata.hunks.length - 1),
+    );
+    const hunkBounds = sectionMetrics[selectedFileIndex]?.hunkBounds.get(clampedHunkIndex);
+    if (!hunkBounds) {
+      return null;
     }
 
-    return top;
+    return {
+      top: sectionTop + hunkBounds.top,
+      height: hunkBounds.height,
+      startRowId: hunkBounds.startRowId,
+      endRowId: hunkBounds.endRowId,
+    };
   }, [estimatedBodyHeights, sectionMetrics, selectedFile, selectedFileIndex, selectedHunkIndex]);
 
-  // Track the previous selected anchor to detect actual selection changes
+  // Track the previous selected anchor to detect actual selection changes.
   const prevSelectedAnchorIdRef = useRef<string | null>(null);
 
   useLayoutEffect(() => {
-    if (!selectedAnchorId) {
+    if (!selectedAnchorId && !selectedEstimatedHunkBounds) {
       prevSelectedAnchorIdRef.current = null;
       return;
     }
 
-    // Only auto-scroll when the selection actually changes, not when metrics update during scrolling
+    // Only auto-scroll when the selection actually changes, not when metrics update during
+    // scrolling or when the selected section refines its measured bounds.
     const isSelectionChange = prevSelectedAnchorIdRef.current !== selectedAnchorId;
     prevSelectedAnchorIdRef.current = selectedAnchorId;
 
@@ -328,15 +345,43 @@ export function DiffPane({
         return;
       }
 
-      // In the common no-wrap/no-note path we can estimate the selected hunk row and keep it
-      // comfortably below the top edge instead of merely making it barely visible.
-      if (!wrapLines && visibleAgentNotesByFile.size === 0 && selectedEstimatedScrollTop !== null) {
-        const topPaddingRows = Math.max(2, Math.floor(scrollViewport.height * 0.25));
-        scrollBox.scrollTo(Math.max(0, selectedEstimatedScrollTop - topPaddingRows));
+      const viewportHeight = Math.max(scrollViewport.height, scrollBox.viewport.height ?? 0);
+      const preferredTopPadding = Math.max(2, Math.floor(viewportHeight * 0.25));
+
+      if (selectedEstimatedHunkBounds) {
+        const viewportTop = scrollBox.viewport.y;
+        const currentScrollTop = scrollBox.scrollTop;
+        const startRow = scrollBox.content.findDescendantById(
+          selectedEstimatedHunkBounds.startRowId,
+        );
+        const endRow = scrollBox.content.findDescendantById(selectedEstimatedHunkBounds.endRowId);
+
+        // Prefer exact mounted bounds when both edges are available. If only one edge has mounted
+        // so far, fall back to the planned bounds as one atomic estimate instead of mixing sources.
+        const renderedTop = startRow ? currentScrollTop + (startRow.y - viewportTop) : null;
+        const renderedBottom = endRow
+          ? currentScrollTop + (endRow.y + endRow.height - viewportTop)
+          : null;
+        const renderedBoundsReady = renderedTop !== null && renderedBottom !== null;
+        const hunkTop = renderedBoundsReady ? renderedTop : selectedEstimatedHunkBounds.top;
+        const hunkHeight = renderedBoundsReady
+          ? Math.max(0, renderedBottom - renderedTop)
+          : selectedEstimatedHunkBounds.height;
+
+        scrollBox.scrollTo(
+          computeHunkRevealScrollTop({
+            hunkTop,
+            hunkHeight,
+            preferredTopPadding,
+            viewportHeight,
+          }),
+        );
         return;
       }
 
-      scrollBox.scrollChildIntoView(selectedAnchorId);
+      if (selectedAnchorId) {
+        scrollBox.scrollChildIntoView(selectedAnchorId);
+      }
     };
 
     // Run after this pane renders the selected section/hunk, then retry briefly while layout settles.
@@ -346,14 +391,7 @@ export function DiffPane({
     return () => {
       timeouts.forEach((timeout) => clearTimeout(timeout));
     };
-  }, [
-    scrollRef,
-    scrollViewport.height,
-    selectedAnchorId,
-    selectedEstimatedScrollTop,
-    visibleAgentNotesByFile.size,
-    wrapLines,
-  ]);
+  }, [scrollRef, scrollViewport.height, selectedAnchorId, selectedEstimatedHunkBounds]);
 
   // Configure scroll step size to scroll exactly 1 line per step
   useEffect(() => {

src/ui/diff/PierreDiffView.tsx

@@ -2,8 +2,10 @@ 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 { reviewRowId } from "../lib/ids";
 import type { AppTheme } from "../themes";
 import { buildSplitRows, buildStackRows } from "./pierre";
+import { plannedReviewRowVisible } from "./plannedReviewRows";
 import { buildReviewRenderPlan } from "./reviewRenderPlan";
 import { diffMessage, DiffRowView, findMaxLineNumber, fitText } from "./renderRows";
 import { useHighlightedDiff } from "./useHighlightedDiff";
@@ -92,51 +94,63 @@ export function PierreDiffView({
   const content = (
     <box style={{ width: "100%", flexDirection: "column" }}>
       {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);
+        const visible = plannedReviewRowVisible(plannedRow, {
+          showHunkHeaders,
+          layout,
+          width,
+        });
+
+        if (!visible) {
+          return null;
+        }
+
         if (plannedRow.kind === "inline-note") {
           return (
-            <AgentInlineNote
-              key={plannedRow.key}
-              annotation={plannedRow.annotation}
-              anchorSide={plannedRow.anchorSide}
-              layout={layout}
-              noteCount={plannedRow.noteCount}
-              noteIndex={plannedRow.noteIndex}
-              theme={theme}
-              width={width}
-            />
+            <box key={plannedRow.key} id={rowId} style={{ width: "100%", flexDirection: "column" }}>
+              <AgentInlineNote
+                annotation={plannedRow.annotation}
+                anchorSide={plannedRow.anchorSide}
+                layout={layout}
+                noteCount={plannedRow.noteCount}
+                noteIndex={plannedRow.noteIndex}
+                theme={theme}
+                width={width}
+              />
+            </box>
           );
         }
 
         if (plannedRow.kind === "note-guide-cap") {
           return (
-            <AgentInlineNoteGuideCap
-              key={plannedRow.key}
-              side={plannedRow.side}
-              theme={theme}
-              width={width}
-            />
+            <box key={plannedRow.key} id={rowId} style={{ width: "100%", flexDirection: "column" }}>
+              <AgentInlineNoteGuideCap side={plannedRow.side} theme={theme} width={width} />
+            </box>
           );
         }
 
         return (
-          <DiffRowView
-            key={plannedRow.key}
-            row={plannedRow.row}
-            width={width}
-            lineNumberDigits={lineNumberDigits}
-            showLineNumbers={showLineNumbers}
-            showHunkHeaders={showHunkHeaders}
-            wrapLines={wrapLines}
-            theme={theme}
-            selected={plannedRow.row.hunkIndex === selectedHunkIndex}
-            annotated={
-              plannedRow.row.type === "hunk-header" &&
-              annotatedHunkIndices.has(plannedRow.row.hunkIndex)
-            }
-            anchorId={plannedRow.anchorId}
-            noteGuideSide={plannedRow.noteGuideSide}
-            onOpenAgentNotesAtHunk={onOpenAgentNotesAtHunk}
-          />
+          <box key={plannedRow.key} id={rowId} style={{ width: "100%", flexDirection: "column" }}>
+            <DiffRowView
+              row={plannedRow.row}
+              width={width}
+              lineNumberDigits={lineNumberDigits}
+              showLineNumbers={showLineNumbers}
+              showHunkHeaders={showHunkHeaders}
+              wrapLines={wrapLines}
+              theme={theme}
+              selected={plannedRow.row.hunkIndex === selectedHunkIndex}
+              annotated={
+                plannedRow.row.type === "hunk-header" &&
+                annotatedHunkIndices.has(plannedRow.row.hunkIndex)
+              }
+              anchorId={plannedRow.anchorId}
+              noteGuideSide={plannedRow.noteGuideSide}
+              onOpenAgentNotesAtHunk={onOpenAgentNotesAtHunk}
+            />
+          </box>
         );
       })}
     </box>

src/ui/diff/plannedReviewRows.ts

@@ -0,0 +1,114 @@
+import type { LayoutMode } from "../../core/types";
+import { measureAgentInlineNoteHeight } from "../components/panes/AgentInlineNote";
+import { reviewRowId } from "../lib/ids";
+import type { PlannedReviewRow } from "./reviewRenderPlan";
+
+/** Layout inputs needed to turn one planned review row into concrete terminal height. */
+export interface PlannedReviewRowLayoutOptions {
+  showHunkHeaders: boolean;
+  layout: Exclude<LayoutMode, "auto">;
+  width: number;
+}
+
+/**
+ * Visible bounds for one hunk within a file section body.
+ *
+ * The row ids let DiffPane upgrade from planned measurements to exact mounted measurements later.
+ */
+export interface PlannedHunkBounds {
+  top: number;
+  height: number;
+  startRowId: string;
+  endRowId: string;
+}
+
+/** Return whether this planned row should count toward a hunk's own visible extent. */
+function rowContributesToHunkBounds(row: PlannedReviewRow) {
+  // Collapsed gap rows belong between hunks, so they affect total section height but not a hunk's
+  // own visible extent.
+  return !(row.kind === "diff-row" && row.row.type === "collapsed");
+}
+
+/** Measure how many terminal rows one planned review row will occupy once rendered. */
+export function plannedReviewRowHeight(
+  row: PlannedReviewRow,
+  { showHunkHeaders, layout, width }: PlannedReviewRowLayoutOptions,
+) {
+  if (row.kind === "inline-note") {
+    return measureAgentInlineNoteHeight({
+      annotation: row.annotation,
+      anchorSide: row.anchorSide,
+      layout,
+      width,
+    });
+  }
+
+  if (row.kind === "note-guide-cap") {
+    return 1;
+  }
+
+  if (row.row.type === "hunk-header") {
+    return showHunkHeaders ? 1 : 0;
+  }
+
+  return 1;
+}
+
+/** Check whether a planned row will produce any visible output at all. */
+export function plannedReviewRowVisible(
+  row: PlannedReviewRow,
+  options: PlannedReviewRowLayoutOptions,
+) {
+  return plannedReviewRowHeight(row, options) > 0;
+}
+
+/**
+ * Walk one file's planned rows and derive both section metrics and hunk-local bounds.
+ *
+ * `top` is measured in section-body rows, so callers can add the file section offset later.
+ */
+export function measurePlannedHunkBounds(
+  plannedRows: PlannedReviewRow[],
+  options: PlannedReviewRowLayoutOptions,
+) {
+  const hunkAnchorRows = new Map<number, number>();
+  const hunkBounds = new Map<number, PlannedHunkBounds>();
+  let bodyHeight = 0;
+
+  for (const row of plannedRows) {
+    if (row.kind === "diff-row" && row.anchorId && !hunkAnchorRows.has(row.hunkIndex)) {
+      // Track the renderer's anchor row separately from the full hunk bounds so navigation can
+      // still target the same semantic row when headers are hidden.
+      hunkAnchorRows.set(row.hunkIndex, bodyHeight);
+    }
+
+    const rowHeight = plannedReviewRowHeight(row, options);
+
+    if (rowHeight > 0 && rowContributesToHunkBounds(row)) {
+      const rowId = reviewRowId(row.key);
+      const existingBounds = hunkBounds.get(row.hunkIndex);
+
+      if (existingBounds) {
+        // Extend the current hunk through the latest visible row that belongs to it.
+        existingBounds.endRowId = rowId;
+        existingBounds.height += rowHeight;
+      } else {
+        // Seed the first visible row for this hunk; later rows will widen the bounds.
+        hunkBounds.set(row.hunkIndex, {
+          top: bodyHeight,
+          height: rowHeight,
+          startRowId: rowId,
+          endRowId: rowId,
+        });
+      }
+    }
+
+    bodyHeight += rowHeight;
+  }
+
+  return {
+    bodyHeight,
+    hunkAnchorRows,
+    hunkBounds,
+  };
+}