diff --git a/src/ui/App.tsx b/src/ui/App.tsx index b2f40184..e94d6d19 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -574,6 +574,22 @@ export function App({ [activeAddNoteTarget, review.startUserNote], ); + /** Start editing the active user note and move keyboard focus into it. */ + const editActiveNote = useCallback(() => { + const draft = review.editActiveNote(); + if (draft) { + setFocusArea("note"); + } + }, [review.editActiveNote]); + + /** Start a reply draft for the active review note and move keyboard focus into it. */ + const replyToActiveNote = useCallback(() => { + const draft = review.replyToActiveNote(); + if (draft) { + setFocusArea("note"); + } + }, [review.replyToActiveNote]); + /** Mark the inline draft note textarea as the active keyboard input. */ const focusDraftNote = useCallback(() => { setFocusArea("note"); @@ -685,11 +701,14 @@ export function App({ useAppKeyboardShortcuts({ activeMenuId, activateCurrentMenuItem, + canEditActiveNote: review.activeNoteCanEdit, canRefreshCurrentInput, + canReplyToActiveNote: Boolean(review.activeNoteId), closeHelp, closeMenu, cycleTheme, cancelDraftNote, + editActiveNote, focusArea, focusFilter, moveToAnnotatedHunk, @@ -699,6 +718,7 @@ export function App({ openMenu, pagerMode, requestQuit, + replyToActiveNote, scrollCodeHorizontally, saveDraftNote, scrollDiff, @@ -872,6 +892,7 @@ export function App({ scrollRef={diffScrollRef} selectedFileId={selectedFile?.id} selectedHunkIndex={selectedHunkIndex} + activeNoteId={review.activeNoteId} scrollToNote={review.scrollToNote} draftNote={review.draftNote} draftNoteFocused={focusArea === "note"} @@ -889,7 +910,9 @@ export function App({ theme={activeTheme} width={diffPaneWidth} onActiveAddNoteAffordanceChange={setActiveAddNoteTarget} + onEditActiveNote={editActiveNote} onRemoveUserNote={review.removeUserNote} + onReplyToActiveNote={replyToActiveNote} onSaveDraftNote={saveDraftNote} onStartUserNoteAtHunk={startUserNote} onUpdateDraftNote={review.updateDraftNote} diff --git a/src/ui/components/panes/AgentInlineNote.tsx b/src/ui/components/panes/AgentInlineNote.tsx index ed442cda..be2467b1 100644 --- a/src/ui/components/panes/AgentInlineNote.tsx +++ b/src/ui/components/panes/AgentInlineNote.tsx @@ -122,8 +122,11 @@ export function AgentInlineNote({ layout, noteCount = 1, noteIndex = 0, + active = false, draft, onClose, + onEdit, + onReply, theme, width, }: { @@ -133,6 +136,7 @@ export function AgentInlineNote({ layout: Exclude; noteCount?: number; noteIndex?: number; + active?: boolean; draft?: { body: string; focused: boolean; @@ -143,6 +147,8 @@ export function AgentInlineNote({ onSave: () => void; }; onClose?: () => void; + onEdit?: () => void; + onReply?: () => void; theme: AppTheme; width: number; }) { @@ -192,6 +198,7 @@ export function AgentInlineNote({ const closeText = onClose ? "[x]" : ""; const titleText = `${inlineNoteTitle(annotation, noteIndex, noteCount)} - ${annotationRangeLabel(annotation, file)}`; + const savedDisplayTitleText = active ? `› ${titleText}` : titleText; const splitWidths = splitColumnWidths(width); const canDockRight = layout === "split" && anchorSide === "new" && width >= 84; const canDockLeft = layout === "split" && anchorSide === "old" && width >= 84; @@ -257,7 +264,7 @@ export function AgentInlineNote({ : []), ]; const savedTitleText = fitText( - ` ${titleText} `, + ` ${savedDisplayTitleText} `, Math.max(0, boxWidth - 4 - closeGapWidth - closeWidth), ); const savedTopBorderSuffixWidth = Math.max( @@ -265,7 +272,21 @@ export function AgentInlineNote({ boxWidth - 3 - savedTitleText.length - closeGapWidth - closeWidth, ); const savedTopPrefixWidth = 2 + savedTitleText.length + savedTopBorderSuffixWidth; + const replyButtonText = "[ Reply (r) ]"; + const editButtonText = "[ Edit (e) ]"; + const bottomBorderAction = annotation.editable + ? ` ${replyButtonText} ${editButtonText} ` + : ` ${replyButtonText} `; + const bottomBorderActionFits = active && boxWidth - 2 >= bottomBorderAction.length + 2; + const bottomBorderLeftWidth = bottomBorderActionFits + ? Math.max(1, boxWidth - 2 - bottomBorderAction.length - 1) + : Math.max(0, boxWidth - 2); + const bottomBorderRightWidth = bottomBorderActionFits + ? Math.max(0, boxWidth - 2 - bottomBorderAction.length - bottomBorderLeftWidth) + : 0; const bottomBorder = `╰${"─".repeat(Math.max(0, boxWidth - 2))}╯`; + const savedBorderColor = theme.noteBorder; + const savedHeaderBackground = theme.panel; if (draft) { const draftVisibleLineCount = draftVisibleRows; @@ -504,7 +525,7 @@ export function AgentInlineNote({ {" ".repeat(boxLeft)} - + │ @@ -516,7 +537,7 @@ export function AgentInlineNote({ - + │ @@ -529,36 +550,48 @@ export function AgentInlineNote({ {" ".repeat(boxLeft)} - + - + ╭─ - - {savedTitleText} - - + {active ? ( + + {savedTitleText} + + ) : ( + + {savedTitleText} + + )} + {"─".repeat(savedTopBorderSuffixWidth)} {closeText ? ( - - {" ".repeat(closeGapWidth)} + + {" ".repeat(closeGapWidth)} ) : null} {closeText ? ( - + {closeText} ) : null} - - + + ╮ @@ -574,11 +607,78 @@ export function AgentInlineNote({ {" ".repeat(boxLeft)} - - - {bottomBorder} - - + {bottomBorderActionFits ? ( + + + + ╰ + + + + + {"─".repeat(bottomBorderLeftWidth)} + + + + + {" "} + + + + + {replyButtonText} + + + {annotation.editable ? ( + + + {" "} + + + ) : null} + {annotation.editable ? ( + + + {editButtonText} + + + ) : null} + + + {" "} + + + + + {"─".repeat(bottomBorderRightWidth)} + + + + + ╯ + + + + ) : ( + + + {bottomBorder} + + + )} ); diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index d3bb6950..5ab1f62e 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -25,6 +25,7 @@ import type { DraftReviewNote } from "../../hooks/useReviewController"; import { alwaysShowReviewNote, reviewNoteSource, + visibleReviewNoteId, type VisibleAgentNote, } from "../../lib/agentAnnotations"; import { @@ -178,6 +179,7 @@ export function DiffPane({ scrollRef, selectedFileId, selectedHunkIndex, + activeNoteId = null, scrollToNote = false, draftNote = null, draftNoteFocused = false, @@ -200,7 +202,9 @@ export function DiffPane({ width, cancelCopySelectionRef, onActiveAddNoteAffordanceChange, + onEditActiveNote, onRemoveUserNote, + onReplyToActiveNote, onSaveDraftNote, onStartUserNoteAtHunk, onUpdateDraftNote, @@ -224,6 +228,7 @@ export function DiffPane({ scrollRef: RefObject; selectedFileId?: string; selectedHunkIndex: number; + activeNoteId?: string | null; scrollToNote?: boolean; draftNote?: DraftReviewNote | null; draftNoteFocused?: boolean; @@ -248,7 +253,9 @@ export function DiffPane({ onActiveAddNoteAffordanceChange?: ( affordance: (ActiveAddNoteAffordance & { fileId: string }) | null, ) => void; + onEditActiveNote?: () => void; onRemoveUserNote?: (noteId: string) => void; + onReplyToActiveNote?: () => void; onSaveDraftNote?: () => void; onStartUserNoteAtHunk?: (fileId: string, hunkIndex: number, target?: UserNoteLineTarget) => void; onUpdateDraftNote?: (body: string) => void; @@ -346,20 +353,27 @@ export function DiffPane({ (annotation) => showAgentNotes || alwaysShowReviewNote(annotation), ); const notes: VisibleAgentNote[] = annotations.map((annotation, index) => { + const id = visibleReviewNoteId(file, annotation, index); + const active = id === activeNoteId; const source = reviewNoteSource(annotation); if (source !== "user") { return { - id: `annotation:${file.id}:${annotation.id ?? index}`, + id, + active, annotation, + onReply: active ? onReplyToActiveNote : undefined, }; } return { - id: `annotation:${file.id}:${annotation.id ?? index}`, + id, + active, annotation, source, editable: true, + onEdit: active ? onEditActiveNote : undefined, onRemove: annotation.id ? () => onRemoveUserNote?.(annotation.id!) : undefined, + onReply: active ? onReplyToActiveNote : undefined, }; }); @@ -396,13 +410,16 @@ export function DiffPane({ return next; }, [ + activeNoteId, draftNote, draftNoteFocused, files, onBlurDraftNote, onCancelDraftNote, + onEditActiveNote, onFocusDraftNote, onRemoveUserNote, + onReplyToActiveNote, onSaveDraftNote, onUpdateDraftNote, showAgentNotes, @@ -1215,12 +1232,22 @@ export function DiffPane({ const sectionRelativeHunkTop = selectedEstimatedHunkBounds.top - selectedEstimatedHunkBounds.sectionTop; const sectionRelativeHunkBottom = sectionRelativeHunkTop + selectedEstimatedHunkBounds.height; - const noteRow = geometry.rowBounds.find( - (row) => - row.key.startsWith("inline-note:") && - row.top >= sectionRelativeHunkTop && - row.top < sectionRelativeHunkBottom, - ); + const noteRow = activeNoteId + ? geometry.rowBoundsByStableKey.get(`inline-note:${activeNoteId}`) + : geometry.rowBounds.find( + (row) => + row.key.startsWith("inline-note:") && + row.top >= sectionRelativeHunkTop && + row.top < sectionRelativeHunkBottom, + ); + + if ( + noteRow && + activeNoteId && + (noteRow.top < sectionRelativeHunkTop || noteRow.top >= sectionRelativeHunkBottom) + ) { + return null; + } if (!noteRow) { return null; @@ -1230,7 +1257,7 @@ export function DiffPane({ top: selectedEstimatedHunkBounds.sectionTop + noteRow.top, height: noteRow.height, }; - }, [scrollToNote, sectionGeometry, selectedEstimatedHunkBounds, selectedFileIndex]); + }, [activeNoteId, scrollToNote, sectionGeometry, selectedEstimatedHunkBounds, selectedFileIndex]); const selectedEstimatedHunkTop = selectedEstimatedHunkBounds?.top ?? null; const selectedEstimatedHunkHeight = selectedEstimatedHunkBounds?.height ?? null; const selectedEstimatedHunkStartRowId = selectedEstimatedHunkBounds?.startRowId ?? null; @@ -1524,6 +1551,14 @@ export function DiffPane({ // hunk can request a top offset that is no longer reachable once the viewport hits EOF. // Using the reachable value keeps the reveal logic from fighting later manual scrolling. if (selectedNoteBounds) { + const currentScrollTop = scrollBox.scrollTop ?? 0; + const noteTop = selectedNoteBounds.top; + const noteBottom = selectedNoteBounds.top + selectedNoteBounds.height; + const viewportBottom = currentScrollTop + viewportHeight; + if (noteTop >= currentScrollTop && noteBottom <= viewportBottom) { + return; + } + const revealScrollTop = computeHunkRevealScrollTop({ hunkTop: selectedNoteBounds.top, hunkHeight: selectedNoteBounds.height, @@ -1714,7 +1749,9 @@ export function DiffPane({ headerLabelWidth={headerLabelWidth} headerStatsWidth={headerStatsWidth} layout={layout} - selectedHunkIndex={file.id === selectedFileId ? selectedHunkIndex : -1} + selectedHunkIndex={ + activeNoteId ? -1 : file.id === selectedFileId ? selectedHunkIndex : -1 + } copySelectedRowRanges={copySelectedRowKeysByFile.get(file.id)} copySelectedSide={copySelectionSide} shouldLoadHighlight={highlightPrefetchFileIds.has(file.id)} diff --git a/src/ui/components/ui-components.test.tsx b/src/ui/components/ui-components.test.tsx index 52f70191..a830068b 100644 --- a/src/ui/components/ui-components.test.tsx +++ b/src/ui/components/ui-components.test.tsx @@ -1655,6 +1655,74 @@ describe("UI components", () => { } }); + test("DiffPane scrollToNote leaves an already-visible inline note in place", async () => { + const theme = resolveTheme("midnight", null); + const scrollRef = createRef(); + const beforeLines = Array.from( + { length: 12 }, + (_, index) => `export const line${index + 1} = ${index + 1};`, + ); + const afterLines = [...beforeLines]; + afterLines[3] = "export const line4 = 400;"; + const file = createTestDiffFile( + "visible-note", + "visible-note.ts", + lines(...beforeLines), + lines(...afterLines), + ); + file.agent = { + path: file.path, + annotations: [{ newRange: [4, 4], summary: "Already visible note." }], + }; + let revealNote = () => {}; + + function VisibleNoteHarness() { + const [request, setRequest] = useState({ id: 0, scrollToNote: false }); + revealNote = () => setRequest({ id: 1, scrollToNote: true }); + + return ( + + ); + } + + const setup = await testRender(, { width: 104, height: 12 }); + + try { + await settleDiffPane(setup); + await act(async () => { + scrollRef.current?.scrollTo(2); + }); + await settleDiffPane(setup); + const scrollTopBeforeReveal = scrollRef.current?.scrollTop ?? 0; + expect(setup.captureCharFrame()).toContain("Already visible note."); + + await act(async () => { + revealNote(); + }); + await settleDiffPane(setup); + + expect(scrollRef.current?.scrollTop ?? 0).toBe(scrollTopBeforeReveal); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("AgentCard removes top and bottom padding while keeping the footer inside the frame", async () => { const theme = resolveTheme("midnight", null); const frame = await captureFrame( @@ -1711,6 +1779,80 @@ describe("UI components", () => { expect(lines[4]?.trimStart().startsWith("╰")).toBe(true); }); + test("AgentInlineNote marks the active saved note with a bold title chevron", async () => { + const theme = resolveTheme("midnight", null); + const frame = await captureFrame( + , + 100, + 5, + ); + + const lines = frame.split("\n"); + expect(lines[0]).toContain("╭─ › Agent note - R2–R4"); + expect(lines[0]).not.toContain("ACTIVE"); + expect(lines[2]).not.toContain("›"); + expect(lines[2]).toContain("Summary line"); + expect(lines[4]).toContain("[ Reply (r) ] [ Edit (e) ]"); + }); + + test("AgentInlineNote active action hints are clickable", async () => { + const theme = resolveTheme("midnight", null); + const onReply = mock(() => {}); + const onEdit = mock(() => {}); + const setup = await testRender( + , + { width: 100, height: 5 }, + ); + + try { + await act(async () => { + await setup.renderOnce(); + }); + const lines = setup.captureCharFrame().split("\n"); + const actionY = lines.findIndex((line) => line.includes("[ Reply (r) ]")); + const replyX = lines[actionY]!.indexOf("[ Reply (r) ]") + 1; + const editX = lines[actionY]!.indexOf("[ Edit (e) ]") + 1; + + await act(async () => { + await setup.mockMouse.click(replyX, actionY); + await setup.mockMouse.click(editX, actionY); + await setup.renderOnce(); + }); + + expect(onReply).toHaveBeenCalledTimes(1); + expect(onEdit).toHaveBeenCalledTimes(1); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("AgentInlineNote renders draft notes as an editable composer", async () => { const theme = resolveTheme("midnight", null); const file = createTestDiffFile( diff --git a/src/ui/diff/PierreDiffView.tsx b/src/ui/diff/PierreDiffView.tsx index 15bf6855..e97162e1 100644 --- a/src/ui/diff/PierreDiffView.tsx +++ b/src/ui/diff/PierreDiffView.tsx @@ -304,6 +304,7 @@ export function PierreDiffView({ > diff --git a/src/ui/hooks/useAppKeyboardShortcuts.ts b/src/ui/hooks/useAppKeyboardShortcuts.ts index 85b1ca20..49bca9ad 100644 --- a/src/ui/hooks/useAppKeyboardShortcuts.ts +++ b/src/ui/hooks/useAppKeyboardShortcuts.ts @@ -45,11 +45,14 @@ function isUppercaseGKey(key: KeyEvent) { export interface UseAppKeyboardShortcutsOptions { activeMenuId: MenuId | null; activateCurrentMenuItem: () => void; + canEditActiveNote: boolean; canRefreshCurrentInput: boolean; + canReplyToActiveNote: boolean; closeHelp: () => void; closeMenu: () => void; cycleTheme: () => void; cancelDraftNote: () => void; + editActiveNote: () => void; focusArea: FocusArea; focusFilter: () => void; moveToAnnotatedHunk: (delta: number) => void; @@ -59,6 +62,7 @@ export interface UseAppKeyboardShortcutsOptions { openMenu: (menuId: MenuId) => void; pagerMode: boolean; requestQuit: () => void; + replyToActiveNote: () => void; scrollCodeHorizontally: (delta: number) => void; scrollDiff: (delta: number, unit: ScrollUnit) => void; saveDraftNote: () => void; @@ -82,11 +86,14 @@ export interface UseAppKeyboardShortcutsOptions { export function useAppKeyboardShortcuts({ activeMenuId, activateCurrentMenuItem, + canEditActiveNote, canRefreshCurrentInput, + canReplyToActiveNote, closeHelp, closeMenu, cycleTheme, cancelDraftNote, + editActiveNote, focusArea, focusFilter, moveToAnnotatedHunk, @@ -96,6 +103,7 @@ export function useAppKeyboardShortcuts({ openMenu, pagerMode, requestQuit, + replyToActiveNote, scrollCodeHorizontally, saveDraftNote, scrollDiff, @@ -431,8 +439,22 @@ export function useAppKeyboardShortcuts({ return; } - if ((key.name === "r" || key.sequence === "r") && canRefreshCurrentInput) { - runAndCloseMenu(triggerRefreshCurrentInput); + if (key.name === "r" || key.sequence === "r") { + runAndCloseMenu(() => { + if (canReplyToActiveNote) { + replyToActiveNote(); + return; + } + + if (canRefreshCurrentInput) { + triggerRefreshCurrentInput(); + } + }); + return; + } + + if ((key.name === "e" || key.sequence === "e") && canEditActiveNote) { + runAndCloseMenu(editActiveNote); return; } diff --git a/src/ui/hooks/useReviewController.test.tsx b/src/ui/hooks/useReviewController.test.tsx index 5b99efc9..d0fbf6b8 100644 --- a/src/ui/hooks/useReviewController.test.tsx +++ b/src/ui/hooks/useReviewController.test.tsx @@ -345,6 +345,99 @@ describe("useReviewController", () => { } }); + test("annotated navigation advances note-by-note and tracks the active note", async () => { + const { controllerRef, setup } = await renderReviewController([ + createDiffFile( + "alpha", + "alpha.ts", + "export const alpha = 1;\n", + "export const alpha = 2;\n", + { + path: "alpha.ts", + summary: "file note", + annotations: [ + { id: "first", newRange: [1, 1], summary: "First note" }, + { id: "second", newRange: [1, 1], summary: "Second note" }, + ], + }, + ), + createDiffFile("beta", "beta.ts", "export const beta = 1;\n", "export const beta = 2;\n"), + ]); + + try { + await flush(setup); + + await act(async () => { + expectValue(controllerRef.current).moveToAnnotatedHunk(1); + }); + await flush(setup); + expect(expectValue(controllerRef.current).activeNoteId).toBe("annotation:alpha:first"); + expect(expectValue(controllerRef.current).scrollToNote).toBe(true); + + await act(async () => { + expectValue(controllerRef.current).moveToAnnotatedHunk(1); + }); + await flush(setup); + expect(expectValue(controllerRef.current).activeNoteId).toBe("annotation:alpha:second"); + expect(expectValue(controllerRef.current).selectedHunkIndex).toBe(0); + + await act(async () => { + expectValue(controllerRef.current).moveToHunk(1); + }); + await flush(setup); + expect(expectValue(controllerRef.current).activeNoteId).toBeNull(); + expect(expectValue(controllerRef.current).selectedFile?.path).toBe("beta.ts"); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + + test("replyToActiveNote starts a draft at the active note anchor", async () => { + const { controllerRef, setup } = await renderReviewController([ + createDiffFile( + "alpha", + "alpha.ts", + "export const alpha = 1;\n", + "export const alpha = 2;\n", + { + path: "alpha.ts", + annotations: [{ id: "review", newRange: [1, 1], summary: "Review note" }], + }, + ), + ]); + + try { + await flush(setup); + + await act(async () => { + expectValue(controllerRef.current).moveToAnnotatedHunk(1); + }); + await flush(setup); + expect(expectValue(controllerRef.current).activeNoteId).toBe("annotation:alpha:review"); + + let draft = null as ReturnType; + await act(async () => { + draft = expectValue(controllerRef.current).replyToActiveNote(); + }); + await flush(setup); + + expect(draft).toMatchObject({ fileId: "alpha", hunkIndex: 0, side: "new", line: 1 }); + expect(expectValue(controllerRef.current).draftNote).toMatchObject({ + fileId: "alpha", + hunkIndex: 0, + side: "new", + line: 1, + }); + expect(expectValue(controllerRef.current).activeNoteId).toBeNull(); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("batch live comments validate together and reveal the first applied hunk", async () => { const { controllerRef, setup } = await renderReviewController([createTwoHunkFile()]); @@ -522,6 +615,25 @@ describe("useReviewController", () => { editable: true, }, ]); + expect(expectValue(controllerRef.current).activeNoteCanEdit).toBe(true); + + await act(async () => { + expectValue(controllerRef.current).editActiveNote(); + expectValue(controllerRef.current).updateDraftNote("Please add two regression tests."); + }); + await flush(setup); + + await act(async () => { + expectValue(controllerRef.current).saveDraftNote(); + }); + await flush(setup); + + expect(expectValue(controllerRef.current).userNotesByFileId.alpha).toHaveLength(1); + expect(expectValue(controllerRef.current).reviewNoteSummaries[0]).toMatchObject({ + noteId: savedNoteId, + body: "Please add two regression tests.", + editable: true, + }); await act(async () => { expectValue(controllerRef.current).removeUserNote(savedNoteId); diff --git a/src/ui/hooks/useReviewController.ts b/src/ui/hooks/useReviewController.ts index 0feab177..7a5e377d 100644 --- a/src/ui/hooks/useReviewController.ts +++ b/src/ui/hooks/useReviewController.ts @@ -40,7 +40,12 @@ import type { FileSourceStatus } from "../diff/expandCollapsedRows"; import { selectGapForKeyboardToggle } from "../diff/expandCollapsedRows"; import { trailingCollapsedLines } from "../diff/pierre"; import { findNextHunkCursor } from "../lib/hunks"; -import { reviewNoteSource } from "../lib/agentAnnotations"; +import { + annotationAnchor, + annotationHunkIndex, + reviewNoteSource, + visibleReviewNoteId, +} from "../lib/agentAnnotations"; import { buildReviewState, buildSelectedHunkSummary, @@ -83,6 +88,75 @@ function removeKeys(record: Record, keys: ReadonlySet): Re return changed ? next : record; } +/** Move through note cursors using the active note first, then the current hunk as a fallback. */ +function findNextReviewNoteCursor({ + activeNoteId, + currentFileId, + currentHunkIndex, + cursors, + delta, + streamCursors, +}: { + activeNoteId: string | null; + currentFileId: string | undefined; + currentHunkIndex: number; + cursors: ReviewNoteCursor[]; + delta: number; + streamCursors: { fileId: string; hunkIndex: number }[]; +}) { + if (cursors.length === 0) { + return null; + } + + const activeIndex = activeNoteId + ? cursors.findIndex((cursor) => cursor.noteId === activeNoteId) + : -1; + if (activeIndex >= 0) { + return cursors[Math.min(Math.max(activeIndex + delta, 0), cursors.length - 1)] ?? null; + } + + const currentHunkNoteIndices = cursors + .map((cursor, index) => ({ cursor, index })) + .filter( + ({ cursor }) => cursor.fileId === currentFileId && cursor.hunkIndex === currentHunkIndex, + ) + .map(({ index }) => index); + if (currentHunkNoteIndices.length > 0) { + return ( + cursors[delta >= 0 ? currentHunkNoteIndices[0]! : currentHunkNoteIndices.at(-1)!] ?? null + ); + } + + const currentStreamIndex = streamCursors.findIndex( + (cursor) => cursor.fileId === currentFileId && cursor.hunkIndex === currentHunkIndex, + ); + if (currentStreamIndex < 0) { + return cursors[delta >= 0 ? 0 : cursors.length - 1] ?? null; + } + + const streamIndexByHunk = new Map( + streamCursors.map((cursor, index) => [`${cursor.fileId}\0${cursor.hunkIndex}`, index] as const), + ); + const indexedCursors = cursors + .map((cursor, index) => ({ + index, + streamIndex: streamIndexByHunk.get(`${cursor.fileId}\0${cursor.hunkIndex}`) ?? -1, + })) + .filter(({ streamIndex }) => streamIndex >= 0); + + if (delta >= 0) { + return ( + cursors[ + indexedCursors.find(({ streamIndex }) => streamIndex > currentStreamIndex)?.index ?? + cursors.length - 1 + ] ?? null + ); + } + + const previous = indexedCursors.findLast(({ streamIndex }) => streamIndex < currentStreamIndex); + return cursors[previous?.index ?? 0] ?? null; +} + export interface UserReviewNote extends AgentAnnotation { id: string; source: "user"; @@ -106,6 +180,7 @@ export interface DraftReviewNote { oldRange?: [number, number]; newRange?: [number, number]; body: string; + editingNoteId?: string; } interface SourceLoadRequest { @@ -114,7 +189,15 @@ interface SourceLoadRequest { side: "old" | "new"; } +interface ReviewNoteCursor { + noteId: string; + fileId: string; + hunkIndex: number; + editable: boolean; +} + export interface ReviewSelectionOptions { + activeNoteId?: string | null; alignFileHeaderTop?: boolean; preserveViewport?: boolean; scrollToNote?: boolean; @@ -124,6 +207,8 @@ export interface ReviewController { allFiles: DiffFile[]; expandedGapsByFileId: Record>; filter: string; + activeNoteCanEdit: boolean; + activeNoteId: string | null; draftNote: DraftReviewNote | null; liveCommentCount: number; liveCommentSummaries: SessionLiveCommentSummary[]; @@ -163,6 +248,8 @@ export interface ReviewController { removeLiveComment: (commentId: string) => RemovedCommentResult; cancelDraftNote: () => void; removeUserNote: (noteId: string) => void; + editActiveNote: () => DraftReviewNote | null; + replyToActiveNote: () => DraftReviewNote | null; saveDraftNote: () => UserReviewNote | null; selectFile: (fileId: string, nextHunkIndex?: number, options?: ReviewSelectionOptions) => void; selectHunk: (fileId: string, hunkIndex: number, options?: ReviewSelectionOptions) => void; @@ -183,6 +270,7 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon const [selectedFileTopAlignRequestId, setSelectedFileTopAlignRequestId] = useState(0); const [selectedHunkRevealRequestId, setSelectedHunkRevealRequestId] = useState(0); const [scrollToNote, setScrollToNote] = useState(false); + const [activeNoteId, setActiveNoteId] = useState(null); const [liveCommentsByFileId, setLiveCommentsByFileId] = useState>( {}, ); @@ -233,38 +321,61 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon const deferredFilter = useDeferredValue(filter); - const { - allFiles, - visibleFiles, - sidebarEntries, - selectedFile, - selectedHunk, - hunkCursors, - annotatedHunkCursors, - } = useMemo( - () => - buildReviewState({ + const { allFiles, visibleFiles, sidebarEntries, selectedFile, selectedHunk, hunkCursors } = + useMemo( + () => + buildReviewState({ + files, + liveCommentsByFileId: mergeAnnotationMaps(liveCommentsByFileId, userNotesByFileId), + filterQuery: deferredFilter, + selectedFileId, + selectedHunkIndex, + }), + [ + deferredFilter, files, - liveCommentsByFileId: mergeAnnotationMaps(liveCommentsByFileId, userNotesByFileId), - filterQuery: deferredFilter, + liveCommentsByFileId, selectedFileId, selectedHunkIndex, - }), - [ - deferredFilter, - files, - liveCommentsByFileId, - selectedFileId, - selectedHunkIndex, - userNotesByFileId, - ], + userNotesByFileId, + ], + ); + + const reviewNoteCursors = useMemo( + () => + visibleFiles.flatMap((file) => + (file.agent?.annotations ?? []).flatMap((annotation, index) => { + const hunkIndex = annotationHunkIndex(file, annotation); + return hunkIndex >= 0 + ? [ + { + noteId: visibleReviewNoteId(file, annotation, index), + fileId: file.id, + hunkIndex, + editable: annotation.editable === true, + }, + ] + : []; + }), + ), + [visibleFiles], ); - /** Update the selection and reveal intent together so diff scrolling stays explicit. */ + useEffect(() => { + if (activeNoteId && !reviewNoteCursors.some((cursor) => cursor.noteId === activeNoteId)) { + setActiveNoteId(null); + } + }, [activeNoteId, reviewNoteCursors]); + + const activeNoteCanEdit = + reviewNoteCursors.find((cursor) => cursor.noteId === activeNoteId)?.editable ?? false; + + /** Update the active target, selection, and reveal intent together so diff scrolling stays explicit. */ const selectHunk = useCallback( (fileId: string, hunkIndex: number, options?: ReviewSelectionOptions) => { setSelectedFileId(fileId); setSelectedHunkIndex(hunkIndex); + setActiveNoteId(options?.activeNoteId ?? null); setScrollToNote(Boolean(options?.scrollToNote)); if (options?.alignFileHeaderTop) { @@ -354,23 +465,27 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon [hunkCursors, selectHunk, selectedFile?.id, selectedHunkIndex], ); - /** Move through only hunks that currently have agent notes or live comments. */ + /** Move through notes in review-stream order and make the destination note active. */ const moveToAnnotatedHunk = useCallback( (delta: number) => { - const nextCursor = findNextHunkCursor( - annotatedHunkCursors, - selectedFile?.id, - selectedHunkIndex, + const nextCursor = findNextReviewNoteCursor({ + activeNoteId, + currentFileId: selectedFile?.id, + currentHunkIndex: selectedHunkIndex, + cursors: reviewNoteCursors, delta, - hunkCursors, - ); + streamCursors: hunkCursors, + }); if (!nextCursor) { return; } - selectHunk(nextCursor.fileId, nextCursor.hunkIndex, { scrollToNote: true }); + selectHunk(nextCursor.fileId, nextCursor.hunkIndex, { + activeNoteId: nextCursor.noteId, + scrollToNote: true, + }); }, - [annotatedHunkCursors, hunkCursors, selectHunk, selectedFile?.id, selectedHunkIndex], + [activeNoteId, hunkCursors, reviewNoteCursors, selectHunk, selectedFile?.id, selectedHunkIndex], ); /** Cycle through only the currently visible files that carry annotations. */ @@ -766,6 +881,76 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon [allFiles, selectHunk, selectedFile?.id, selectedHunkIndex], ); + /** Start editing the active user note in the inline composer. */ + const editActiveNote = useCallback((): DraftReviewNote | null => { + if (!activeNoteId) { + return null; + } + + for (const file of allFiles) { + const note = (userNotesByFileId[file.id] ?? []).find( + (candidate) => visibleReviewNoteId(file, candidate, 0) === activeNoteId, + ); + if (!note) { + continue; + } + + const hunkIndex = note.hunkIndex ?? annotationHunkIndex(file, note); + if (hunkIndex < 0) { + return null; + } + + const draft: DraftReviewNote = { + id: `draft:${file.id}:${hunkIndex}:${Date.now()}`, + fileId: file.id, + filePath: file.path, + hunkIndex, + side: note.side, + line: note.line, + oldRange: note.oldRange, + newRange: note.newRange, + body: note.summary, + editingNoteId: note.id, + }; + setDraftNote(draft); + selectHunk(file.id, hunkIndex, { preserveViewport: true }); + return draft; + } + + return null; + }, [activeNoteId, allFiles, selectHunk, userNotesByFileId]); + + /** Start a reply draft at the active note's hunk and line anchor. */ + const replyToActiveNote = useCallback((): DraftReviewNote | null => { + if (!activeNoteId) { + return null; + } + + for (const file of allFiles) { + const annotationIndex = file.agent?.annotations.findIndex( + (annotation, index) => visibleReviewNoteId(file, annotation, index) === activeNoteId, + ); + if (annotationIndex === undefined || annotationIndex < 0) { + continue; + } + + const annotation = file.agent!.annotations[annotationIndex]!; + const hunkIndex = annotationHunkIndex(file, annotation); + if (hunkIndex < 0) { + return null; + } + + const anchor = annotationAnchor(annotation); + return startUserNote( + file.id, + hunkIndex, + anchor ? { side: anchor.side, line: anchor.lineNumber } : undefined, + ); + } + + return null; + }, [activeNoteId, allFiles, startUserNote]); + /** Update the body of the active draft note. */ const updateDraftNote = useCallback((body: string) => { setDraftNote((current) => (current ? { ...current, body } : current)); @@ -788,6 +973,31 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon return null; } + if (draftNote.editingNoteId) { + const existingNote = (userNotesByFileId[draftNote.fileId] ?? []).find( + (note) => note.id === draftNote.editingNoteId, + ); + if (!existingNote) { + setDraftNote(null); + return null; + } + + const updatedNote: UserReviewNote = { + ...existingNote, + summary: body, + updatedAt: new Date().toISOString(), + }; + setUserNotesByFileId((notesByFile) => ({ + ...notesByFile, + [draftNote.fileId]: (notesByFile[draftNote.fileId] ?? []).map((note) => + note.id === draftNote.editingNoteId ? updatedNote : note, + ), + })); + setActiveNoteId(`annotation:${draftNote.fileId}:${draftNote.editingNoteId}`); + setDraftNote(null); + return updatedNote; + } + const savedNote: UserReviewNote = { id: `user:${Date.now()}`, source: "user", @@ -807,9 +1017,10 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon ...notesByFile, [draftNote.fileId]: [...(notesByFile[draftNote.fileId] ?? []), savedNote], })); + setActiveNoteId(`annotation:${draftNote.fileId}:${savedNote.id}`); setDraftNote(null); return savedNote; - }, [draftNote]); + }, [draftNote, userNotesByFileId]); /** Remove one in-memory user note by id. */ const removeUserNote = useCallback( @@ -831,6 +1042,7 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon throw new Error(`No user note matches id ${noteId}.`); } + setActiveNoteId((current) => (current?.endsWith(`:${noteId}`) ? null : current)); setUserNotesByFileId(next); }, [userNotesByFileId], @@ -922,6 +1134,8 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon return { allFiles, + activeNoteCanEdit, + activeNoteId, draftNote, expandedGapsByFileId, filter, @@ -952,9 +1166,11 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon moveToAnnotatedHunk, moveToFile, moveToHunk, + editActiveNote, navigateToLocation, removeLiveComment, removeUserNote, + replyToActiveNote, saveDraftNote, selectFile, selectHunk, diff --git a/src/ui/lib/agentAnnotations.ts b/src/ui/lib/agentAnnotations.ts index 49bcf3cb..f6a5f154 100644 --- a/src/ui/lib/agentAnnotations.ts +++ b/src/ui/lib/agentAnnotations.ts @@ -7,6 +7,7 @@ export interface VisibleAgentNote { id: string; annotation: AgentAnnotation; source?: ReviewNoteSource | "draft"; + active?: boolean; editable?: boolean; draft?: { body: string; @@ -17,7 +18,9 @@ export interface VisibleAgentNote { onInput: (value: string) => void; onSave: () => void; }; + onEdit?: () => void; onRemove?: () => void; + onReply?: () => void; } export interface AnnotationAnchor { @@ -43,13 +46,18 @@ export function alwaysShowReviewNote(annotation: AgentAnnotation) { return reviewNoteSource(annotation) === "user"; } +/** Build the stable UI id used for selecting and rendering one review note. */ +export function visibleReviewNoteId(file: DiffFile, annotation: AgentAnnotation, index: number) { + return `annotation:${file.id}:${annotation.id ?? index}`; +} + /** Check whether two inclusive line ranges overlap. */ function overlap(rangeA: [number, number], rangeB: [number, number]) { return rangeA[0] <= rangeB[1] && rangeB[0] <= rangeA[1]; } /** Check whether an annotation belongs to the visible span of a hunk. */ -function annotationOverlapsHunk(annotation: AgentAnnotation, hunk: Hunk) { +export function annotationOverlapsHunk(annotation: AgentAnnotation, hunk: Hunk) { const hunkRange = hunkLineRange(hunk); if (annotation.newRange && overlap(annotation.newRange, hunkRange.newRange)) { @@ -63,6 +71,12 @@ function annotationOverlapsHunk(annotation: AgentAnnotation, hunk: Hunk) { return false; } +/** Resolve the hunk that owns one annotation, falling back to the first hunk for range-less notes. */ +export function annotationHunkIndex(file: DiffFile, annotation: AgentAnnotation) { + const index = file.metadata.hunks.findIndex((hunk) => annotationOverlapsHunk(annotation, hunk)); + return index >= 0 ? index : file.metadata.hunks.length > 0 ? 0 : -1; +} + /** Return the annotations relevant to the currently selected hunk. */ export function getSelectedAnnotations(file: DiffFile | undefined, hunk: Hunk | undefined) { if (!file?.agent || !hunk) {