From bd38893fca1181763d5b11693465f71e737f6d9e Mon Sep 17 00:00:00 2001 From: Alejandro Bernal Date: Thu, 7 May 2026 16:23:12 -0500 Subject: [PATCH 1/2] fix(ui): use full agent-note set for section geometry measurement The visible-viewport agent-note set is correct for rendering (skip painting cards on off-screen files) but using it for section measurement made total content height fluctuate with scroll position: as a file with notes left the viewport its geometry shrank back to the no-notes baseline, which shrank totalContentHeight, which tightened clampReviewScrollTop's ceiling and snapped the viewport upward by the height of the off-top note rows. Always include notes in geometry for stable bottom-edge clamping; rendering keeps using visibleAgentNotesByFile. Fixes #234. --- src/ui/components/panes/DiffPane.tsx | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) 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, ], ); From abf906ce010b83026da85660979d569bc1ff1df0 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Sat, 9 May 2026 20:43:21 -0400 Subject: [PATCH 2/2] test(ui): cover bottom scroll with offscreen notes --- src/ui/components/ui-components.test.tsx | 58 ++++++++++++++++++++++++ 1 file changed, 58 insertions(+) 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 = [