fix: keep selected hunks fully visible when they fit

[benvinegar]

Summary

• keep hunk navigation focused on the selected hunk's full bounds instead of only its anchor row
• derive hunk bounds from the review render plan and upgrade to exact rendered row bounds when mounted
• add regressions for plain, wrapped, and inline-note hunks plus a pure scroll-policy test

Testing

• bun run typecheck
• bun test

[greptile-apps]

Greptile Summary

This PR improves hunk navigation in the diff pane so the full visible extent of a selected hunk is kept on screen, rather than just snapping its anchor row into view. It does this by deriving hunk bounds (top + height + boundary row IDs) from the same render plan used by PierreDiffView, then upgrading to exact rendered coordinates once the rows are mounted.

Key changes:

• plannedReviewRows.ts (new): extracts row-height measurement and introduces measurePlannedHunkBounds, which computes per-hunk top/height/startRowId/endRowId from the planned row list, skipping collapsed gap rows.
• hunkScroll.ts (new): pure computeHunkRevealScrollTop function that applies a preferred top-padding while guaranteeing the full hunk fits in the viewport when it is smaller than the viewport.
• PierreDiffView.tsx: wraps every planned row in a stable id-bearing <box>, enabling findDescendantById lookups at scroll time; invisible rows (zero-height hunk headers) are now filtered to null instead of rendered at zero height.
• DiffPane.tsx: replaces the old anchor-row scroll estimate with selectedEstimatedHunkBounds; uses live rendered coordinates when the start/end rows are mounted, with estimated bounds as a fallback; also ensures the selected file's notes are always included in the section metrics even before the file scrolls into the viewport.
• Three new integration tests cover plain, wrapped-line, and inline-note hunk scenarios.

Confidence Score: 4/5

• Safe to merge with minor follow-up; logic is correct and well-tested, with one style and one edge-case concern worth addressing.
• The core algorithm in computeHunkRevealScrollTop is pure and thoroughly unit-tested. The render-plan-based bounds calculation correctly handles notes, wrapped lines, and collapsed gap rows. The partial-render mixed measurement (live top + estimated height) is a minor correctness gap that the retry loop mitigates in practice, but it could cause a one-frame misalignment for tall wrapped-line hunks. The duplicate DiffSectionHunkBounds/PlannedHunkBounds type is a maintainability concern but not a runtime risk.
• src/ui/components/panes/DiffPane.tsx (partial-render fallback), src/ui/lib/sectionHeights.ts (duplicate type)

Important Files Changed

Filename	Overview
src/ui/lib/hunkScroll.ts	New pure utility that computes the scroll target to keep a hunk fully visible; logic is straightforward and well-tested.
src/ui/diff/plannedReviewRows.ts	New module extracting row-height measurement and hunk-bounds calculation from sectionHeights.ts; introduces PlannedHunkBounds which is structurally duplicated by DiffSectionHunkBounds in sectionHeights.ts.
src/ui/lib/sectionHeights.ts	Delegates row measurement to plannedReviewRows and adds hunkBounds to DiffSectionMetrics; DiffSectionHunkBounds is a redundant duplicate of PlannedHunkBounds.
src/ui/components/panes/DiffPane.tsx	Core scroll logic upgraded from anchor-row point to full hunk bounds; mixed live/estimated measurement in partial-render case may occasionally yield a slightly inaccurate first scroll target.
src/ui/diff/PierreDiffView.tsx	Each planned row is now wrapped in an id-carrying box, enabling findDescendantById lookups; invisible rows (zero-height hunk headers) are filtered to null.
src/ui/lib/ids.ts	Adds reviewRowId helper to build stable IDs for presentational rows; straightforward and consistent with existing id helpers.
test/ui-lib.test.ts	Adds unit tests for hunkBounds fields and computeHunkRevealScrollTop; test expectations are verified correct by manual calculation.
test/ui-components.test.tsx	Adds three regression tests covering plain, wrapped, and inline-note hunk visibility; good coverage of the new scroll policy.

Sequence Diagram

sequenceDiagram
    participant DiffPane
    participant sectionHeights
    participant plannedReviewRows
    participant PierreDiffView
    participant hunkScroll

    DiffPane->>sectionHeights: measureDiffSectionMetrics(file, layout, ...)
    sectionHeights->>plannedReviewRows: measurePlannedHunkBounds(plannedRows, options)
    plannedReviewRows-->>sectionHeights: { bodyHeight, hunkAnchorRows, hunkBounds }
    sectionHeights-->>DiffPane: DiffSectionMetrics (with hunkBounds)

    DiffPane->>DiffPane: selectedEstimatedHunkBounds = hunkBounds.get(clampedHunkIndex) + sectionTop

    DiffPane->>PierreDiffView: render(plannedRows with id-wrapped boxes)
    PierreDiffView-->>DiffPane: mounted DOM with reviewRowId on each box

    DiffPane->>DiffPane: useLayoutEffect fires
    DiffPane->>DiffPane: findDescendantById(startRowId) → renderedTop
    DiffPane->>DiffPane: findDescendantById(endRowId) → renderedBottom
    DiffPane->>hunkScroll: computeHunkRevealScrollTop({ hunkTop, hunkHeight, preferredTopPadding, viewportHeight })
    hunkScroll-->>DiffPane: scrollTop (keeps full hunk visible)
    DiffPane->>DiffPane: scrollBox.scrollTo(scrollTop)

Loading

_{Reviews (1): Last reviewed commit: "fix: keep selected hunks fully visible w..." | Re-trigger Greptile}

[greptile-apps]

Duplicate interface definition

DiffSectionHunkBounds defined here (lines 8–13) is structurally identical to PlannedHunkBounds exported from src/ui/diff/plannedReviewRows.ts (lines 12–17). Both have the exact same four fields (top, height, startRowId, endRowId). TypeScript's structural typing makes the assignment work silently today, but these two types can drift independently in the future.

Consider re-exporting PlannedHunkBounds from plannedReviewRows.ts and using it directly in the DiffSectionMetrics interface here, removing DiffSectionHunkBounds entirely:

import type { PlannedHunkBounds } from "../diff/plannedReviewRows";

export interface DiffSectionMetrics {
  bodyHeight: number;
  hunkAnchorRows: Map<number, number>;
  hunkBounds: Map<number, PlannedHunkBounds>;
}

[benvinegar]

Good catch. I removed the duplicate DiffSectionHunkBounds interface and now use PlannedHunkBounds directly from plannedReviewRows.ts so the bounds shape stays defined in one place.

_{Responded by pi using unknown-model.}

[greptile-apps]

Partial-render fallback uses estimated height

When startRow is found in the rendered tree but endRow is not (or vice versa), hunkHeight falls back silently to selectedEstimatedHunkBounds.height:

const hunkHeight =
  renderedTop !== null && renderedBottom !== null
    ? Math.max(0, renderedBottom - renderedTop)
    : selectedEstimatedHunkBounds.height;

If only one boundary is resolved (e.g. the start row has already rendered but the end row hasn't been mounted yet during the retry window), hunkTop is the live rendered position but hunkHeight is the estimated height from the plan. For a hunk whose last row contains wrapped lines or multi-row inline notes whose actual height differs from the estimate, this mixed measurement can produce a scroll target that still cuts off the bottom of the hunk on the first settle pass, until the next retry corrects it.

Consider using the estimated bounds (selectedEstimatedHunkBounds.top + selectedEstimatedHunkBounds.height) as an atomic unit — either use both live values or neither — to avoid combining a live hunkTop with a stale hunkHeight.

[benvinegar]

Agreed. I changed the fallback to use the rendered bounds only when both boundaries are mounted; otherwise it falls back to the planned top and height together so we do not mix a live top with an estimated height.

_{Responded by pi using unknown-model.}