diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index d93f7ef5..fd84f7e9 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -415,11 +415,18 @@ export function DiffPane({ return next; }, [allAgentNotesByFile, selectedFileId, showAgentNotes, visibleViewportFileIds]); + // Measure with the *full* set of agent notes per file, not just the visible-viewport set. + // The visible set is correct for rendering (skip painting cards on off-screen files), but + // using it here makes total content height fluctuate with scroll position: as a file with + // notes leaves the viewport, its measurement shrinks back to the no-notes baseline, which + // shrinks `totalContentHeight`, which tightens `clampReviewScrollTop`'s ceiling, which + // snaps the viewport upward by the height of the off-top note rows. Always include notes + // in geometry for stable bottom-edge clamping. const sectionGeometry = useMemo( () => files.map((file, index) => { - const visibleNotes = visibleAgentNotesByFile.get(file.id) ?? EMPTY_VISIBLE_AGENT_NOTES; - if (visibleNotes.length === 0) { + const notes = allAgentNotesByFile.get(file.id) ?? EMPTY_VISIBLE_AGENT_NOTES; + if (notes.length === 0) { return baseSectionGeometry[index]!; } @@ -428,13 +435,14 @@ export function DiffPane({ layout, showHunkHeaders, theme, - visibleNotes, + notes, diffContentWidth, showLineNumbers, wrapLines, ); }), [ + allAgentNotesByFile, baseSectionGeometry, diffContentWidth, files, @@ -442,7 +450,6 @@ export function DiffPane({ showHunkHeaders, showLineNumbers, theme, - visibleAgentNotesByFile, wrapLines, ], ); diff --git a/src/ui/components/ui-components.test.tsx b/src/ui/components/ui-components.test.tsx index 63074521..dc0f553f 100644 --- a/src/ui/components/ui-components.test.tsx +++ b/src/ui/components/ui-components.test.tsx @@ -856,6 +856,64 @@ describe("UI components", () => { } }); + test("DiffPane keeps bottom scroll stable when offscreen agent notes are windowed out", async () => { + const theme = resolveTheme("midnight", null); + const firstFile = createTallDiffFile("first", "first.ts", 18); + firstFile.agent = { + path: firstFile.path, + summary: "first.ts note", + annotations: [ + { + newRange: [2, 2], + summary: "Offscreen note should still reserve geometry at EOF.", + rationale: + "If measurement drops this note after first.ts leaves the viewport, max scroll shrinks.", + }, + ], + }; + const files = [firstFile, createTallDiffFile("last", "last.ts", 24)]; + const scrollRef = createRef(); + const props = createDiffPaneProps(files, theme, { + diffContentWidth: 88, + headerLabelWidth: 48, + headerStatsWidth: 16, + scrollRef, + selectedFileId: undefined, + separatorWidth: 84, + showAgentNotes: true, + width: 92, + }); + const setup = await testRender(, { + width: 96, + height: 10, + }); + + try { + await settleDiffPane(setup); + + let bottomScrollTop = 0; + await act(async () => { + scrollRef.current?.scrollTo(1_000_000); + bottomScrollTop = scrollRef.current?.scrollTop ?? 0; + }); + expect(bottomScrollTop).toBeGreaterThan(0); + + await settleDiffPane(setup); + expect(scrollRef.current?.scrollTop ?? 0).toBe(bottomScrollTop); + + await act(async () => { + scrollRef.current?.scrollTo(bottomScrollTop + 1); + }); + await settleDiffPane(setup); + + expect(scrollRef.current?.scrollTop ?? 0).toBe(bottomScrollTop); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("DiffPane lets manual scrolling move away from a bottom-clamped file-top alignment", async () => { const theme = resolveTheme("midnight", null); const files = [