diff --git a/src/core/cli.ts b/src/core/cli.ts index cdcb0031..a1f764fe 100644 --- a/src/core/cli.ts +++ b/src/core/cli.ts @@ -501,6 +501,7 @@ async function parseSessionCommand(tokens: string[]): Promise { " hunk session context ", " hunk session context --repo ", " hunk session navigate ( | --repo ) --file (--hunk | --old-line | --new-line )", + " hunk session navigate ( | --repo ) (--next-comment | --prev-comment)", " hunk session reload ( | --repo | --session-path ) [--source ] -- diff [ref] [-- ]", " hunk session reload ( | --repo | --session-path ) [--source ] -- show [ref] [-- ]", " hunk session comment add ( | --repo ) --file (--old-line | --new-line ) --summary ", @@ -569,32 +570,38 @@ async function parseSessionCommand(tokens: string[]): Promise { const command = new Command("session navigate") .description("move a live Hunk session to one diff hunk") .argument("[sessionId]") - .requiredOption("--file ", "diff file path as shown by Hunk") + .option("--file ", "diff file path as shown by Hunk") .option("--repo ", "target the live session whose repo root matches this path") .option("--hunk ", "1-based hunk number within the file", parsePositiveInt) .option("--old-line ", "1-based line number on the old side", parsePositiveInt) .option("--new-line ", "1-based line number on the new side", parsePositiveInt) + .option("--next-comment", "jump to the next annotated hunk") + .option("--prev-comment", "jump to the previous annotated hunk") .option("--json", "emit structured JSON"); let parsedSessionId: string | undefined; let parsedOptions: { repo?: string; - file: string; + file?: string; hunk?: number; oldLine?: number; newLine?: number; + nextComment?: boolean; + prevComment?: boolean; json?: boolean; - } = { file: "" }; + } = {}; command.action( ( sessionId: string | undefined, options: { repo?: string; - file: string; + file?: string; hunk?: number; oldLine?: number; newLine?: number; + nextComment?: boolean; + prevComment?: boolean; json?: boolean; }, ) => { @@ -609,6 +616,28 @@ async function parseSessionCommand(tokens: string[]): Promise { await parseStandaloneCommand(command, rest); + /** Relative comment navigation mode. */ + if (parsedOptions.nextComment || parsedOptions.prevComment) { + if (parsedOptions.nextComment && parsedOptions.prevComment) { + throw new Error("Specify either --next-comment or --prev-comment, not both."); + } + + return { + kind: "session", + action: "navigate", + output: resolveJsonOutput(parsedOptions), + selector: resolveExplicitSessionSelector(parsedSessionId, parsedOptions.repo), + commentDirection: parsedOptions.nextComment ? "next" : "prev", + } as const; + } + + /** Absolute navigation mode requires --file and a target. */ + if (!parsedOptions.file) { + throw new Error( + "Specify --file with a navigation target, or use --next-comment / --prev-comment.", + ); + } + const selectors = [ parsedOptions.hunk !== undefined, parsedOptions.oldLine !== undefined, diff --git a/src/core/types.ts b/src/core/types.ts index 280da6d9..87ac290c 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -112,10 +112,11 @@ export interface SessionNavigateCommandInput { action: "navigate"; output: SessionCommandOutput; selector: SessionSelectorInput; - filePath: string; + filePath?: string; hunkNumber?: number; side?: "old" | "new"; line?: number; + commentDirection?: "next" | "prev"; } export interface SessionReloadCommandInput { diff --git a/src/mcp/server.ts b/src/mcp/server.ts index a2909fad..b8b02e61 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -94,6 +94,7 @@ async function handleSessionApiRequest(state: HunkDaemonState, request: Request) break; case "navigate": { if ( + !input.commentDirection && input.hunkNumber === undefined && (input.side === undefined || input.line === undefined) ) { @@ -107,6 +108,7 @@ async function handleSessionApiRequest(state: HunkDaemonState, request: Request) hunkIndex: input.hunkNumber !== undefined ? input.hunkNumber - 1 : undefined, side: input.side, line: input.line, + commentDirection: input.commentDirection, }), }; break; diff --git a/src/mcp/types.ts b/src/mcp/types.ts index 01f13cf7..59afd2a6 100644 --- a/src/mcp/types.ts +++ b/src/mcp/types.ts @@ -79,10 +79,11 @@ export interface NavigateToFileToolInput extends SessionTargetInput { } export interface NavigateToHunkToolInput extends SessionTargetInput { - filePath: string; + filePath?: string; hunkIndex?: number; side?: DiffSide; line?: number; + commentDirection?: "next" | "prev"; } export interface ReloadSessionToolInput extends SessionTargetInput { diff --git a/src/session/commands.ts b/src/session/commands.ts index ac4c8c3c..2ea14b5d 100644 --- a/src/session/commands.ts +++ b/src/session/commands.ts @@ -154,6 +154,7 @@ class HttpHunkDaemonCliClient implements HunkDaemonCliClient { hunkNumber: input.hunkNumber, side: input.side, line: input.line, + commentDirection: input.commentDirection, }) ).result; } diff --git a/src/session/protocol.ts b/src/session/protocol.ts index 73ac2aea..0d77dc88 100644 --- a/src/session/protocol.ts +++ b/src/session/protocol.ts @@ -53,10 +53,11 @@ export type SessionDaemonRequest = | { action: "navigate"; selector: SessionNavigateCommandInput["selector"]; - filePath: string; + filePath?: string; hunkNumber?: number; side?: "old" | "new"; line?: number; + commentDirection?: "next" | "prev"; } | { action: "reload"; diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 4e5f3f05..682a890f 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -35,8 +35,8 @@ import { PaneDivider } from "./components/panes/PaneDivider"; import { useHunkSessionBridge } from "./hooks/useHunkSessionBridge"; import { useMenuController } from "./hooks/useMenuController"; import { buildAppMenus } from "./lib/appMenus"; -import { buildSidebarEntries } from "./lib/files"; -import { buildHunkCursors, findNextHunkCursor } from "./lib/hunks"; +import { buildSidebarEntries, filterReviewFiles, mergeFileAnnotationsByFileId } from "./lib/files"; +import { buildAnnotatedHunkCursors, buildHunkCursors, findNextHunkCursor } from "./lib/hunks"; import { fileRowId } from "./lib/ids"; import { resolveResponsiveLayout } from "./lib/responsive"; import { resizeSidebarWidth } from "./lib/sidebar"; @@ -126,6 +126,7 @@ function AppShell({ const [resizeStartWidth, setResizeStartWidth] = useState(null); const [selectedFileId, setSelectedFileId] = useState(bootstrap.changeset.files[0]?.id ?? ""); const [selectedHunkIndex, setSelectedHunkIndex] = useState(0); + const [scrollToNote, setScrollToNote] = useState(false); const deferredFilter = useDeferredValue(filter); const pagerMode = Boolean(bootstrap.input.options.pager); @@ -135,6 +136,14 @@ function AppShell({ filesScrollRef.current?.scrollChildIntoView(fileRowId(fileId)); setSelectedFileId(fileId); setSelectedHunkIndex(nextHunkIndex); + setScrollToNote(false); + }, []); + + const jumpToAnnotatedHunk = useCallback((fileId: string, nextHunkIndex = 0) => { + filesScrollRef.current?.scrollChildIntoView(fileRowId(fileId)); + setSelectedFileId(fileId); + setSelectedHunkIndex(nextHunkIndex); + setScrollToNote(true); }, []); const openAgentNotes = useCallback(() => { @@ -147,7 +156,9 @@ function AppShell({ const { liveCommentsByFileId } = useHunkSessionBridge({ currentHunk: baseSelectedFile?.metadata.hunks[selectedHunkIndex], files: bootstrap.changeset.files, + filterQuery: deferredFilter, hostClient, + jumpToAnnotatedHunk, jumpToFile, openAgentNotes, reloadSession: onReloadSession, @@ -157,42 +168,21 @@ function AppShell({ }); const allFiles = useMemo( - () => - bootstrap.changeset.files.map((file) => { - const liveComments = liveCommentsByFileId[file.id]; - if (!liveComments || liveComments.length === 0) { - return file; - } - - return { - ...file, - agent: { - path: file.path, - summary: file.agent?.summary, - annotations: [...(file.agent?.annotations ?? []), ...liveComments], - }, - }; - }), + () => mergeFileAnnotationsByFileId(bootstrap.changeset.files, liveCommentsByFileId), [bootstrap.changeset.files, liveCommentsByFileId], ); - const filteredFiles = allFiles.filter((file) => { - if (!deferredFilter.trim()) { - return true; - } - - const haystack = [file.path, file.previousPath, file.agent?.summary] - .filter(Boolean) - .join(" ") - .toLowerCase(); - return haystack.includes(deferredFilter.trim().toLowerCase()); - }); + const filteredFiles = useMemo( + () => filterReviewFiles(allFiles, deferredFilter), + [allFiles, deferredFilter], + ); const selectedFile = filteredFiles.find((file) => file.id === selectedFileId) ?? allFiles.find((file) => file.id === selectedFileId) ?? filteredFiles[0]; const hunkCursors = buildHunkCursors(filteredFiles); + const annotatedHunkCursors = buildAnnotatedHunkCursors(filteredFiles); const bodyPadding = pagerMode ? 0 : BODY_PADDING; const bodyWidth = Math.max(0, terminal.width - bodyPadding); @@ -284,6 +274,22 @@ function AppShell({ filesScrollRef.current?.scrollChildIntoView(fileRowId(nextCursor.fileId)); setSelectedFileId(nextCursor.fileId); setSelectedHunkIndex(nextCursor.hunkIndex); + setScrollToNote(false); + }; + + /** Move the review focus to the next or previous annotated hunk. */ + const moveAnnotatedHunk = (delta: number) => { + const nextCursor = findNextHunkCursor( + annotatedHunkCursors, + selectedFile?.id, + selectedHunkIndex, + delta, + ); + if (!nextCursor) { + return; + } + + jumpToAnnotatedHunk(nextCursor.fileId, nextCursor.hunkIndex); }; /** Scroll the main review pane by line steps, viewport fractions, or whole-content jumps. */ @@ -478,6 +484,7 @@ function AppShell({ focusFilter: () => setFocusArea("filter"), layoutMode, moveAnnotatedFile, + moveAnnotatedHunk, moveHunk, refreshCurrentInput: triggerRefreshCurrentInput, requestQuit, @@ -502,6 +509,7 @@ function AppShell({ canRefreshCurrentInput, layoutMode, moveAnnotatedFile, + moveAnnotatedHunk, moveHunk, requestQuit, triggerRefreshCurrentInput, @@ -877,6 +885,18 @@ function AppShell({ closeMenu(); return; } + + if (key.sequence === "{") { + moveAnnotatedHunk(-1); + closeMenu(); + return; + } + + if (key.sequence === "}") { + moveAnnotatedHunk(1); + closeMenu(); + return; + } }); return ( @@ -960,6 +980,7 @@ function AppShell({ scrollRef={diffScrollRef} selectedFileId={selectedFile?.id} selectedHunkIndex={selectedHunkIndex} + scrollToNote={scrollToNote} separatorWidth={diffSeparatorWidth} showAgentNotes={showAgentNotes} showLineNumbers={showLineNumbers} diff --git a/src/ui/components/chrome/HelpDialog.tsx b/src/ui/components/chrome/HelpDialog.tsx index bff0bfc5..10c72d52 100644 --- a/src/ui/components/chrome/HelpDialog.tsx +++ b/src/ui/components/chrome/HelpDialog.tsx @@ -26,6 +26,7 @@ export function HelpDialog({ ["Shift+Space", "page up (alt)"], ["d / u", "half page down / up"], ["[ / ]", "previous / next hunk"], + ["{ / }", "previous / next comment"], ["Home / End", "jump to top / bottom"], ], }, diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index 5cb28660..03b83d8e 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -129,6 +129,7 @@ export function DiffPane({ scrollRef, selectedFileId, selectedHunkIndex, + scrollToNote = false, separatorWidth, pagerMode = false, showAgentNotes, @@ -149,6 +150,7 @@ export function DiffPane({ scrollRef: RefObject; selectedFileId?: string; selectedHunkIndex: number; + scrollToNote?: boolean; separatorWidth: number; pagerMode?: boolean; showAgentNotes: boolean; @@ -439,9 +441,41 @@ export function DiffPane({ height: hunkBounds.height, startRowId: hunkBounds.startRowId, endRowId: hunkBounds.endRowId, + sectionTop, }; }, [estimatedBodyHeights, sectionMetrics, selectedFile, selectedFileIndex, selectedHunkIndex]); + /** Absolute scroll offset and height of the first inline note in the selected hunk, if any. */ + const selectedNoteBounds = useMemo(() => { + if (!scrollToNote || !selectedEstimatedHunkBounds || selectedFileIndex < 0) { + return null; + } + + const metrics = sectionMetrics[selectedFileIndex]; + if (!metrics) { + return null; + } + + const sectionRelativeHunkTop = + selectedEstimatedHunkBounds.top - selectedEstimatedHunkBounds.sectionTop; + const sectionRelativeHunkBottom = sectionRelativeHunkTop + selectedEstimatedHunkBounds.height; + const noteRow = metrics.rowMetrics.find( + (row) => + row.key.startsWith("inline-note:") && + row.offset >= sectionRelativeHunkTop && + row.offset < sectionRelativeHunkBottom, + ); + + if (!noteRow) { + return null; + } + + return { + top: selectedEstimatedHunkBounds.sectionTop + noteRow.offset, + height: noteRow.height, + }; + }, [scrollToNote, sectionMetrics, selectedEstimatedHunkBounds, selectedFileIndex]); + // Track the previous selected anchor to detect actual selection changes. const prevSelectedAnchorIdRef = useRef(null); @@ -520,6 +554,21 @@ export function DiffPane({ const viewportHeight = Math.max(scrollViewport.height, scrollBox.viewport.height ?? 0); const preferredTopPadding = Math.max(2, Math.floor(viewportHeight * 0.25)); + // When navigating comment-to-comment, scroll the inline note card near the viewport top + // instead of positioning the entire hunk. Uses the same reveal function so the padding + // behavior matches regular hunk navigation. + if (selectedNoteBounds) { + scrollBox.scrollTo( + computeHunkRevealScrollTop({ + hunkTop: selectedNoteBounds.top, + hunkHeight: selectedNoteBounds.height, + preferredTopPadding, + viewportHeight, + }), + ); + return; + } + if (selectedEstimatedHunkBounds) { const viewportTop = scrollBox.viewport.y; const currentScrollTop = scrollBox.scrollTop; @@ -564,7 +613,13 @@ export function DiffPane({ return () => { timeouts.forEach((timeout) => clearTimeout(timeout)); }; - }, [scrollRef, scrollViewport.height, selectedAnchorId, selectedEstimatedHunkBounds]); + }, [ + scrollRef, + scrollViewport.height, + selectedAnchorId, + selectedEstimatedHunkBounds, + selectedNoteBounds, + ]); // Configure scroll step size to scroll exactly 1 line per step useEffect(() => { diff --git a/src/ui/hooks/useHunkSessionBridge.ts b/src/ui/hooks/useHunkSessionBridge.ts index e802dc3e..e97deabb 100644 --- a/src/ui/hooks/useHunkSessionBridge.ts +++ b/src/ui/hooks/useHunkSessionBridge.ts @@ -7,6 +7,8 @@ import { hunkLineRange, } from "../../core/liveComments"; import { HunkHostClient } from "../../mcp/client"; +import { filterReviewFiles, mergeFileAnnotationsByFileId } from "../lib/files"; +import { buildAnnotatedHunkCursors, findNextHunkCursor } from "../lib/hunks"; import type { LiveComment, ReloadedSessionResult, @@ -18,7 +20,9 @@ import type { export function useHunkSessionBridge({ currentHunk, files, + filterQuery, hostClient, + jumpToAnnotatedHunk, jumpToFile, openAgentNotes, reloadSession, @@ -28,7 +32,9 @@ export function useHunkSessionBridge({ }: { currentHunk: DiffFile["metadata"]["hunks"][number] | undefined; files: DiffFile[]; + filterQuery: string; hostClient?: HunkHostClient; + jumpToAnnotatedHunk: (fileId: string, nextHunkIndex?: number) => void; jumpToFile: (fileId: string, nextHunkIndex?: number) => void; openAgentNotes: () => void; reloadSession: ( @@ -56,8 +62,57 @@ export function useHunkSessionBridge({ }; }, []); + /** Merge live comments into the raw file list so annotation queries see both sources. */ + const filesWithLiveComments = useCallback( + () => mergeFileAnnotationsByFileId(files, liveCommentsByFileIdRef.current), + [files], + ); + + /** Apply the active filter to the merged file list so keyboard and CLI comment navigation agree. */ + const visibleFilesWithLiveComments = useCallback( + () => filterReviewFiles(filesWithLiveComments(), filterQuery), + [filesWithLiveComments, filterQuery], + ); + const navigateToHunkSelection = useCallback( async (message: Extract) => { + /** Handle relative comment navigation (--next-comment / --prev-comment). */ + if (message.input.commentDirection) { + const delta = message.input.commentDirection === "next" ? 1 : -1; + const visibleFiles = visibleFilesWithLiveComments(); + const annotatedCursors = buildAnnotatedHunkCursors(visibleFiles); + const nextCursor = findNextHunkCursor( + annotatedCursors, + selectedFile?.id, + selectedHunkIndex, + delta, + ); + + if (!nextCursor) { + throw new Error("No annotated hunks found in the current review."); + } + + const targetFile = visibleFiles.find((file) => file.id === nextCursor.fileId); + if (!targetFile) { + throw new Error("Resolved annotated hunk references an unknown file."); + } + + jumpToAnnotatedHunk(targetFile.id, nextCursor.hunkIndex); + return { + fileId: targetFile.id, + filePath: targetFile.path, + hunkIndex: nextCursor.hunkIndex, + selectedHunk: buildSelectedHunkSummary(targetFile, nextCursor.hunkIndex), + }; + } + + /** Handle absolute navigation by file + hunk/line target. */ + if (!message.input.filePath) { + throw new Error( + "navigate requires --file when not using --next-comment or --prev-comment.", + ); + } + const file = findDiffFileByPath(files, message.input.filePath); if (!file) { throw new Error(`No visible diff file matches ${message.input.filePath}.`); @@ -84,7 +139,15 @@ export function useHunkSessionBridge({ selectedHunk: buildSelectedHunkSummary(file, hunkIndex), }; }, - [buildSelectedHunkSummary, files, jumpToFile], + [ + buildSelectedHunkSummary, + files, + jumpToAnnotatedHunk, + jumpToFile, + selectedFile?.id, + selectedHunkIndex, + visibleFilesWithLiveComments, + ], ); const applyIncomingComment = useCallback( diff --git a/src/ui/lib/appMenus.ts b/src/ui/lib/appMenus.ts index cf92ca98..b38901cb 100644 --- a/src/ui/lib/appMenus.ts +++ b/src/ui/lib/appMenus.ts @@ -8,6 +8,7 @@ export interface BuildAppMenusOptions { focusFilter: () => void; layoutMode: LayoutMode; moveAnnotatedFile: (delta: number) => void; + moveAnnotatedHunk: (delta: number) => void; moveHunk: (delta: number) => void; refreshCurrentInput: () => void; requestQuit: () => void; @@ -35,6 +36,7 @@ export function buildAppMenus({ focusFilter, layoutMode, moveAnnotatedFile, + moveAnnotatedHunk, moveHunk, refreshCurrentInput, requestQuit, @@ -171,6 +173,19 @@ export function buildAppMenus({ action: () => moveHunk(1), }, { kind: "separator" }, + { + kind: "item", + label: "Previous comment", + hint: "{", + action: () => moveAnnotatedHunk(-1), + }, + { + kind: "item", + label: "Next comment", + hint: "}", + action: () => moveAnnotatedHunk(1), + }, + { kind: "separator" }, { kind: "item", label: "Focus filter", diff --git a/src/ui/lib/files.ts b/src/ui/lib/files.ts index 2ae0da6d..16de6df8 100644 --- a/src/ui/lib/files.ts +++ b/src/ui/lib/files.ts @@ -1,5 +1,5 @@ import { basename, dirname } from "node:path/posix"; -import type { DiffFile } from "../../core/types"; +import type { AgentAnnotation, DiffFile } from "../../core/types"; export interface FileListEntry { kind: "file"; @@ -28,7 +28,45 @@ function sidebarFileName(file: DiffFile) { return previousName === nextName ? nextName : `${previousName} -> ${nextName}`; } -/** Group sidebar rows by their current parent folder while preserving file order. */ +/** Merge one file-id keyed annotation map into the review stream file list. */ +export function mergeFileAnnotationsByFileId( + files: DiffFile[], + annotationsByFileId: Record, +): DiffFile[] { + return files.map((file) => { + const annotations = annotationsByFileId[file.id]; + if (!annotations || annotations.length === 0) { + return file; + } + + return { + ...file, + agent: { + path: file.path, + summary: file.agent?.summary, + annotations: [...(file.agent?.annotations ?? []), ...annotations], + }, + }; + }); +} + +/** Apply the shell's file filter query to the visible review stream. */ +export function filterReviewFiles(files: DiffFile[], query: string): DiffFile[] { + const trimmedQuery = query.trim().toLowerCase(); + if (!trimmedQuery) { + return files; + } + + return files.filter((file) => { + const haystack = [file.path, file.previousPath, file.agent?.summary] + .filter(Boolean) + .join(" ") + .toLowerCase(); + return haystack.includes(trimmedQuery); + }); +} + +/** Build the grouped sidebar entries while preserving the review stream order. */ export function buildSidebarEntries(files: DiffFile[]): SidebarEntry[] { const entries: SidebarEntry[] = []; let activeGroup: string | null = null; diff --git a/src/ui/lib/hunks.ts b/src/ui/lib/hunks.ts index bf45b1d8..ba7e89fa 100644 --- a/src/ui/lib/hunks.ts +++ b/src/ui/lib/hunks.ts @@ -1,4 +1,5 @@ import type { DiffFile } from "../../core/types"; +import { getAnnotatedHunkIndices } from "./agentAnnotations"; export interface HunkCursor { fileId: string; @@ -12,6 +13,16 @@ export function buildHunkCursors(files: DiffFile[]): HunkCursor[] { ); } +/** Flatten only the annotated hunks into a cursor list for comment navigation. */ +export function buildAnnotatedHunkCursors(files: DiffFile[]): HunkCursor[] { + return files.flatMap((file) => { + const annotated = getAnnotatedHunkIndices(file); + return file.metadata.hunks + .map((_, hunkIndex) => ({ fileId: file.id, hunkIndex })) + .filter((cursor) => annotated.has(cursor.hunkIndex)); + }); +} + /** Move forward or backward through the review-stream hunk cursor list. */ export function findNextHunkCursor( cursors: HunkCursor[], diff --git a/test/app-interactions.test.tsx b/test/app-interactions.test.tsx index ed9db068..df27b6fb 100644 --- a/test/app-interactions.test.tsx +++ b/test/app-interactions.test.tsx @@ -5,6 +5,8 @@ import { describe, expect, mock, test } from "bun:test"; import { testRender } from "@opentui/react/test-utils"; import { parseDiffFromFile } from "@pierre/diffs"; import { act } from "react"; +import type { HunkHostClient } from "../src/mcp/client"; +import type { HunkSessionRegistration, SessionServerMessage } from "../src/mcp/types"; import type { AppBootstrap, DiffFile, LayoutMode } from "../src/core/types"; const { loadAppBootstrap } = await import("../src/core/loaders"); @@ -66,6 +68,48 @@ function createDiffFile( }; } +function createMockHostClient() { + type Bridge = Parameters[0]; + + let bridge: Bridge = null; + const registration: HunkSessionRegistration = { + sessionId: "session-1", + pid: process.pid, + cwd: process.cwd(), + repoRoot: process.cwd(), + inputKind: "git", + title: "repo working tree", + sourceLabel: "repo", + launchedAt: "2026-03-24T00:00:00.000Z", + files: [], + }; + return { + hostClient: { + getRegistration: () => registration, + replaceSession: () => {}, + setBridge: (nextBridge: Bridge) => { + bridge = nextBridge; + }, + updateSnapshot: () => {}, + } as unknown as HunkHostClient, + getBridge: () => bridge, + navigateToHunk: async ( + input: Extract["input"], + ) => { + if (!bridge) { + throw new Error("Expected App to register a bridge before running the test command."); + } + + return bridge.navigateToHunk({ + type: "command", + requestId: "test-request", + command: "navigate_to_hunk", + input, + }); + }, + }; +} + function createBootstrap(initialMode: LayoutMode = "split", pager = false): AppBootstrap { return { input: { @@ -192,6 +236,60 @@ function createLineScrollBootstrap(pager = false): AppBootstrap { }; } +/** Build a two-hunk fixture with a deep inline note for CLI comment-navigation scroll tests. */ +function createDeepNoteBootstrap(): AppBootstrap { + const beforeLines = Array.from( + { length: 80 }, + (_, index) => `export const line${index + 1} = ${index + 1};`, + ); + const afterLines = [...beforeLines]; + + afterLines[0] = "export const line1 = 100;"; + afterLines[59] = "export const line60 = 6000;"; + afterLines[60] = "export const line61 = 6100;"; + afterLines[61] = "export const line62 = 6200;"; + afterLines[62] = "export const line63 = 6300;"; + afterLines[63] = "export const line64 = 6400;"; + afterLines[64] = "export const line65 = 6500;"; + + const file = createDiffFile( + "deep-note", + "deep-note.ts", + `${beforeLines.join("\n")}\n`, + `${afterLines.join("\n")}\n`, + ); + file.agent = { + path: file.path, + summary: "file note", + annotations: [ + { + newRange: [62, 62], + summary: "Note anchored on second hunk.", + }, + ], + }; + + return { + input: { + kind: "git", + staged: false, + options: { + mode: "split", + agentNotes: true, + }, + }, + changeset: { + id: "changeset:app-deep-note", + sourceLabel: "repo", + title: "repo working tree", + files: [file], + }, + initialMode: "split", + initialTheme: "midnight", + initialShowAgentNotes: true, + }; +} + /** Build a long-line fixture that is tall enough to verify viewport-anchor restoration. */ function createWrapScrollBootstrap(): AppBootstrap { const before = @@ -1070,6 +1168,93 @@ describe("App interactions", () => { } }); + test("CLI comment navigation respects the active file filter", async () => { + const { hostClient, navigateToHunk } = createMockHostClient(); + const setup = await testRender(, { + width: 240, + height: 24, + }); + + try { + await flush(setup); + + await act(async () => { + await setup.mockInput.pressTab(); + }); + await flush(setup); + await act(async () => { + await setup.mockInput.typeText("beta"); + }); + await flush(setup); + + let frame = setup.captureCharFrame(); + expect(frame).toContain("filter:"); + expect(frame).toContain("beta"); + expect(frame).toContain("betaValue"); + expect(frame).not.toContain("add = true"); + + let navigationError: unknown; + await act(async () => { + try { + await navigateToHunk({ commentDirection: "next" }); + } catch (error) { + navigationError = error; + } + }); + + expect(navigationError).toBeInstanceOf(Error); + expect((navigationError as Error).message).toContain( + "No annotated hunks found in the current review.", + ); + + await flush(setup); + frame = setup.captureCharFrame(); + expect(frame).toContain("betaValue"); + expect(frame).not.toContain("add = true"); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + + test("CLI comment navigation scrolls the inline note into view", async () => { + const { hostClient, navigateToHunk } = createMockHostClient(); + const setup = await testRender( + , + { + width: 104, + height: 18, + }, + ); + + try { + await flush(setup); + + let frame = setup.captureCharFrame(); + expect(frame).not.toContain("Note anchored on second hunk."); + + let result: Awaited> | undefined; + await act(async () => { + result = await navigateToHunk({ commentDirection: "next" }); + }); + + expect(result).toMatchObject({ + filePath: "deep-note.ts", + hunkIndex: 1, + }); + + frame = await waitForFrame(setup, (currentFrame) => + currentFrame.includes("Note anchored on second hunk."), + ); + expect(frame).toContain("Note anchored on second hunk."); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("menu navigation wraps across the first and last top-level menus", async () => { const setup = await testRender(, { width: 220, diff --git a/test/cli.test.ts b/test/cli.test.ts index 4b51deac..761baca9 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -399,6 +399,66 @@ describe("parseCli", () => { ); }); + test("parses session navigate with --next-comment", async () => { + const parsed = await parseCli([ + "bun", + "hunk", + "session", + "navigate", + "--repo", + "/tmp/repo", + "--next-comment", + ]); + + expect(parsed).toEqual({ + kind: "session", + action: "navigate", + selector: { repoRoot: "/tmp/repo" }, + commentDirection: "next", + output: "text", + }); + }); + + test("parses session navigate with --prev-comment", async () => { + const parsed = await parseCli([ + "bun", + "hunk", + "session", + "navigate", + "session-1", + "--prev-comment", + "--json", + ]); + + expect(parsed).toEqual({ + kind: "session", + action: "navigate", + selector: { sessionId: "session-1" }, + commentDirection: "prev", + output: "json", + }); + }); + + test("rejects session navigate with both --next-comment and --prev-comment", async () => { + await expect( + parseCli([ + "bun", + "hunk", + "session", + "navigate", + "session-1", + "--next-comment", + "--prev-comment", + ]), + ).rejects.toThrow("Specify either --next-comment or --prev-comment, not both."); + }); + + test("rejects session navigate without --file when not using comment direction", async () => { + await expect( + parseCli(["bun", "hunk", "session", "navigate", "session-1", "--hunk", "1"]), + ).rejects.toThrow("Specify --file"); + }); + test("rejects session navigation with multiple target selectors", async () => { await expect( parseCli([ diff --git a/test/hunk-navigation.test.ts b/test/hunk-navigation.test.ts index 8541fec6..221526df 100644 --- a/test/hunk-navigation.test.ts +++ b/test/hunk-navigation.test.ts @@ -1,5 +1,37 @@ import { describe, expect, test } from "bun:test"; -import { findNextHunkCursor, type HunkCursor } from "../src/ui/lib/hunks"; +import { parseDiffFromFile } from "@pierre/diffs"; +import type { DiffFile } from "../src/core/types"; +import { + buildAnnotatedHunkCursors, + findNextHunkCursor, + type HunkCursor, +} from "../src/ui/lib/hunks"; + +/** Build a minimal DiffFile with real Pierre-parsed hunks and optional annotations. */ +function createTestFile( + id: string, + path: string, + before: string, + after: string, + annotations: DiffFile["agent"], +): DiffFile { + const metadata = parseDiffFromFile( + { name: path, contents: before, cacheKey: `${id}:before` }, + { name: path, contents: after, cacheKey: `${id}:after` }, + { context: 3 }, + true, + ); + + return { + id, + path, + patch: "", + language: "typescript", + stats: { additions: 0, deletions: 0 }, + metadata, + agent: annotations, + }; +} describe("hunk navigation", () => { const cursors: HunkCursor[] = [ @@ -28,3 +60,117 @@ describe("hunk navigation", () => { expect(findNextHunkCursor(cursors, undefined, 0, -1)).toEqual({ fileId: "beta", hunkIndex: 0 }); }); }); + +describe("annotated hunk navigation", () => { + // Two-hunk file: lines 1-10 change in hunk 0, lines 20-30 change in hunk 1. + const beforeA = + "line1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9\nline10\n" + + "gap11\ngap12\ngap13\ngap14\ngap15\ngap16\ngap17\ngap18\ngap19\n" + + "line20\nline21\nline22\nline23\nline24\nline25\nline26\nline27\nline28\nline29\nline30\n"; + const afterA = + "CHANGED1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9\nline10\n" + + "gap11\ngap12\ngap13\ngap14\ngap15\ngap16\ngap17\ngap18\ngap19\n" + + "CHANGED20\nline21\nline22\nline23\nline24\nline25\nline26\nline27\nline28\nline29\nline30\n"; + + // Single-hunk file: one change at line 1. + const beforeB = "old\n"; + const afterB = "new\n"; + + test("only includes hunks that have overlapping annotations", () => { + // Hunk 0 new range is [1,1], hunk 1 new range is [17,17]. + // Annotate only hunk 1 in file alpha, and hunk 0 in file beta. + const fileA = createTestFile("alpha", "alpha.ts", beforeA, afterA, { + path: "alpha.ts", + annotations: [{ newRange: [17, 17], summary: "Note on hunk 1" }], + }); + const fileB = createTestFile("beta", "beta.ts", beforeB, afterB, { + path: "beta.ts", + annotations: [{ newRange: [1, 1], summary: "Note on beta" }], + }); + + expect(fileA.metadata.hunks.length).toBe(2); + const annotatedCursors = buildAnnotatedHunkCursors([fileA, fileB]); + + // Alpha hunk 0 (line 1) has no annotation, so it should be skipped. + expect(annotatedCursors).toEqual([ + { fileId: "alpha", hunkIndex: 1 }, + { fileId: "beta", hunkIndex: 0 }, + ]); + }); + + test("returns an empty list when no files have annotations", () => { + const fileA = createTestFile("alpha", "alpha.ts", beforeA, afterA, null); + const fileB = createTestFile("beta", "beta.ts", beforeB, afterB, null); + + expect(buildAnnotatedHunkCursors([fileA, fileB])).toEqual([]); + }); + + test("skips files with agent context but no matching annotations", () => { + // Annotation range doesn't overlap any hunk (line 10 is in the gap between hunks). + const fileA = createTestFile("alpha", "alpha.ts", beforeA, afterA, { + path: "alpha.ts", + annotations: [{ newRange: [10, 10], summary: "Note in gap, no hunk overlap" }], + }); + + expect(buildAnnotatedHunkCursors([fileA])).toEqual([]); + }); + + test("navigates forward and backward through annotated cursors only", () => { + // Annotate only hunk 1 (new range [17,17]) in alpha, and hunk 0 in beta. + const fileA = createTestFile("alpha", "alpha.ts", beforeA, afterA, { + path: "alpha.ts", + annotations: [{ newRange: [17, 17], summary: "Note on hunk 1 only" }], + }); + const fileB = createTestFile("beta", "beta.ts", beforeB, afterB, { + path: "beta.ts", + annotations: [{ newRange: [1, 1], summary: "Note on beta" }], + }); + + const annotatedCursors = buildAnnotatedHunkCursors([fileA, fileB]); + + // Forward from alpha hunk 1 → beta hunk 0 + expect(findNextHunkCursor(annotatedCursors, "alpha", 1, 1)).toEqual({ + fileId: "beta", + hunkIndex: 0, + }); + + // Backward from beta hunk 0 → alpha hunk 1 + expect(findNextHunkCursor(annotatedCursors, "beta", 0, -1)).toEqual({ + fileId: "alpha", + hunkIndex: 1, + }); + + // Clamps at ends + expect(findNextHunkCursor(annotatedCursors, "alpha", 1, -1)).toEqual({ + fileId: "alpha", + hunkIndex: 1, + }); + expect(findNextHunkCursor(annotatedCursors, "beta", 0, 1)).toEqual({ + fileId: "beta", + hunkIndex: 0, + }); + }); + + test("jumps from an unannotated hunk to the nearest annotated one", () => { + // Only hunk 1 (new range [17,17]) is annotated; hunk 0 is not. + const fileA = createTestFile("alpha", "alpha.ts", beforeA, afterA, { + path: "alpha.ts", + annotations: [{ newRange: [17, 17], summary: "Note on hunk 1 only" }], + }); + + const annotatedCursors = buildAnnotatedHunkCursors([fileA]); + + // Current position is alpha hunk 0, which is not in the annotated list. + // Forward should land on the first annotated cursor. + expect(findNextHunkCursor(annotatedCursors, "alpha", 0, 1)).toEqual({ + fileId: "alpha", + hunkIndex: 1, + }); + + // Backward from an unknown position should land on the last annotated cursor. + expect(findNextHunkCursor(annotatedCursors, "alpha", 0, -1)).toEqual({ + fileId: "alpha", + hunkIndex: 1, + }); + }); +}); diff --git a/test/ui-components.test.tsx b/test/ui-components.test.tsx index a7dab435..a31e64d9 100644 --- a/test/ui-components.test.tsx +++ b/test/ui-components.test.tsx @@ -590,6 +590,104 @@ describe("UI components", () => { } }); + test("DiffPane scrollToNote positions the inline note near the viewport top instead of the hunk top", async () => { + const theme = resolveTheme("midnight", null); + + // Build a file with two distant hunks so the second hunk is far below the first when scrolled + // to the hunk top. The annotation anchors on the second hunk. + const beforeLines = Array.from( + { length: 80 }, + (_, index) => `export const line${index + 1} = ${index + 1};`, + ); + const afterLines = [...beforeLines]; + // Hunk 0: change at line 1 + afterLines[0] = "export const line1 = 100;"; + // Hunk 1: changes at lines 60-65 to make a multi-line hunk + afterLines[59] = "export const line60 = 6000;"; + afterLines[60] = "export const line61 = 6100;"; + afterLines[61] = "export const line62 = 6200;"; + afterLines[62] = "export const line63 = 6300;"; + afterLines[63] = "export const line64 = 6400;"; + afterLines[64] = "export const line65 = 6500;"; + + const file = createDiffFile( + "deep-note", + "deep-note.ts", + lines(...beforeLines), + lines(...afterLines), + ); + file.agent = { + path: file.path, + summary: "file note", + annotations: [ + { + newRange: [63, 63], + summary: "Note anchored on second hunk.", + }, + ], + }; + + // Without scrollToNote: hunk top (context before line 60) is near viewport top, + // but the note card (anchored at line 63) may be below the visible area. + const propsWithoutFlag = createDiffPaneProps([file], theme, { + diffContentWidth: 96, + headerLabelWidth: 48, + selectedFileId: "deep-note", + selectedHunkIndex: 1, + separatorWidth: 92, + showAgentNotes: true, + showHunkHeaders: true, + width: 100, + }); + const setupWithout = await testRender(, { + width: 104, + height: 12, + }); + + try { + await settleDiffPane(setupWithout); + const frameWithout = setupWithout.captureCharFrame(); + + // Hunk context (lines near 57-59) should be visible at the top. + expect(frameWithout).toContain("line57"); + // Note card should NOT be visible — it's below the 12-row viewport. + expect(frameWithout).not.toContain("Note anchored on second hunk."); + } finally { + await act(async () => { + setupWithout.renderer.destroy(); + }); + } + + // With scrollToNote: note card should be near the viewport top. + const propsWithFlag = createDiffPaneProps([file], theme, { + diffContentWidth: 96, + headerLabelWidth: 48, + selectedFileId: "deep-note", + selectedHunkIndex: 1, + scrollToNote: true, + separatorWidth: 92, + showAgentNotes: true, + showHunkHeaders: true, + width: 100, + }); + const setupWith = await testRender(, { + width: 104, + height: 12, + }); + + try { + await settleDiffPane(setupWith); + const frameWith = setupWith.captureCharFrame(); + + // Note should be visible. + expect(frameWith).toContain("Note anchored on second hunk."); + } finally { + await act(async () => { + setupWith.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( @@ -841,13 +939,13 @@ describe("UI components", () => { const frame = await captureFrame( {}} />, 76, - 28, + 29, ); const expectedRows = [ @@ -860,6 +958,7 @@ describe("UI components", () => { "Shift+Space page up (alt)", "d / u half page down / up", "[ / ] previous / next hunk", + "{ / } previous / next comment", "Home / End jump to top / bottom", "View", "1 / 2 / 0 split / stack / auto", diff --git a/test/ui-lib.test.ts b/test/ui-lib.test.ts index be8020c3..95905ace 100644 --- a/test/ui-lib.test.ts +++ b/test/ui-lib.test.ts @@ -102,6 +102,7 @@ describe("ui helpers", () => { focusFilter: () => {}, layoutMode: "stack", moveAnnotatedFile: () => {}, + moveAnnotatedHunk: () => {}, moveHunk: () => {}, refreshCurrentInput: () => {}, requestQuit: () => {},