From c29c12a0a6c3d3161fb44db8db22594bd09c0dbe Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Sun, 10 May 2026 17:16:57 -0400 Subject: [PATCH] feat: add session user review notes --- CHANGELOG.md | 2 + src/core/cli.ts | 128 ++++++- src/core/types.ts | 51 ++- src/hunk-session/bridge.ts | 7 + src/hunk-session/cli.ts | 86 +++++ src/hunk-session/projections.test.ts | 64 ++++ src/hunk-session/projections.ts | 29 +- src/hunk-session/sessionRegistration.ts | 2 + src/hunk-session/types.ts | 33 +- src/hunk-session/wire.ts | 47 +++ src/session-broker/brokerServer.test.ts | 5 +- src/session-broker/brokerServer.ts | 47 ++- src/session/commands.ts | 40 +++ src/session/protocol.ts | 32 +- src/ui/App.tsx | 37 +- src/ui/AppHost.interactions.test.tsx | 4 +- src/ui/components/panes/AgentInlineNote.tsx | 275 ++++++++++++++- src/ui/components/panes/DiffPane.tsx | 97 ++++-- src/ui/components/panes/DiffSection.tsx | 3 + src/ui/components/ui-components.test.tsx | 115 ++++++- src/ui/diff/PierreDiffView.tsx | 18 +- src/ui/diff/renderRows.tsx | 364 ++++++++++++++------ src/ui/diff/reviewRenderPlan.ts | 2 + src/ui/hooks/useAppKeyboardShortcuts.ts | 40 ++- src/ui/hooks/useHunkSessionBridge.ts | 13 + src/ui/hooks/useReviewController.test.tsx | 79 +++++ src/ui/hooks/useReviewController.ts | 266 +++++++++++++- src/ui/lib/agentAnnotations.test.ts | 20 +- src/ui/lib/agentAnnotations.ts | 47 ++- src/ui/lib/reviewState.ts | 10 +- test/pty/ui-integration.test.ts | 25 ++ 31 files changed, 1796 insertions(+), 192 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa36456d..3bfe5714 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ All notable user-visible changes to Hunk are documented in this file. ### Added +- Added session-persistent user-authored inline notes with `i` to draft/save notes and `hunk session note ...` commands for agent readback. + ### Changed ### Fixed diff --git a/src/core/cli.ts b/src/core/cli.ts index 0b94564c..4d49bc0f 100644 --- a/src/core/cli.ts +++ b/src/core/cli.ts @@ -8,6 +8,7 @@ import type { LayoutMode, PagerCommandInput, ParsedCliInput, + ReviewNoteSource, SessionCommentApplyItemInput, } from "./types"; import { resolveBundledHunkReviewSkillPath } from "./paths"; @@ -596,8 +597,8 @@ async function parseSessionCommand(tokens: string[]): Promise { " hunk session get --repo ", " hunk session context ", " hunk session context --repo ", - " hunk session review [--include-patch]", - " hunk session review --repo [--include-patch]", + " hunk session review [--include-patch] [--include-notes]", + " hunk session review --repo [--include-patch] [--include-notes]", " 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] [-- ]", @@ -607,6 +608,9 @@ async function parseSessionCommand(tokens: string[]): Promise { " hunk session comment list ( | --repo )", " hunk session comment rm ( | --repo ) ", " hunk session comment clear ( | --repo ) --yes", + " hunk session note list ( | --repo ) [--source ]", + " hunk session note get ( | --repo ) ", + " hunk session note rm ( | --repo ) ", ].join("\n") + "\n", }; } @@ -647,19 +651,23 @@ async function parseSessionCommand(tokens: string[]): Promise { .option("--json", "emit structured JSON"); if (subcommand === "review") { - command.option( - "--include-patch", - "include raw unified diff text for each file in review output", - ); + command + .option("--include-patch", "include raw unified diff text for each file in review output") + .option("--include-notes", "include live review notes in review output"); } let parsedSessionId: string | undefined; - let parsedOptions: { repo?: string; includePatch?: boolean; json?: boolean } = {}; + let parsedOptions: { + repo?: string; + includePatch?: boolean; + includeNotes?: boolean; + json?: boolean; + } = {}; command.action( ( sessionId: string | undefined, - options: { repo?: string; includePatch?: boolean; json?: boolean }, + options: { repo?: string; includePatch?: boolean; includeNotes?: boolean; json?: boolean }, ) => { parsedSessionId = sessionId; parsedOptions = options; @@ -678,6 +686,7 @@ async function parseSessionCommand(tokens: string[]): Promise { output: resolveJsonOutput(parsedOptions), selector: resolveExplicitSessionSelector(parsedSessionId, parsedOptions.repo), includePatch: parsedOptions.includePatch ?? false, + includeNotes: parsedOptions.includeNotes ?? false, }; } @@ -1162,6 +1171,109 @@ async function parseSessionCommand(tokens: string[]): Promise { throw new Error("Supported comment subcommands are add, apply, list, rm, and clear."); } + if (subcommand === "note") { + const [noteSubcommand, ...noteRest] = rest; + if (!noteSubcommand || noteSubcommand === "--help" || noteSubcommand === "-h") { + return { + kind: "help", + text: + [ + "Usage:", + " hunk session note list ( | --repo ) [--file ] [--source ]", + " hunk session note get ( | --repo ) ", + " hunk session note rm ( | --repo ) ", + ].join("\n") + "\n", + }; + } + + if (noteSubcommand === "list") { + const command = new Command("session note list") + .description("list inline review notes") + .argument("[sessionId]") + .option("--repo ", "target the live session whose repo root matches this path") + .option("--file ", "filter notes to one diff file") + .option("--source ", "filter to ai, agent, or user notes") + .option("--json", "emit structured JSON"); + + let parsedSessionId: string | undefined; + let parsedOptions: { repo?: string; file?: string; source?: string; json?: boolean } = {}; + command.action((sessionId: string | undefined, options) => { + parsedSessionId = sessionId; + parsedOptions = options; + }); + + if (noteRest.includes("--help") || noteRest.includes("-h")) { + return { kind: "help", text: `${command.helpInformation().trimEnd()}\n` }; + } + + await parseStandaloneCommand(command, noteRest); + if ( + parsedOptions.source !== undefined && + parsedOptions.source !== "ai" && + parsedOptions.source !== "agent" && + parsedOptions.source !== "user" + ) { + throw new Error("Note source must be one of ai, agent, or user."); + } + + return { + kind: "session", + action: "note-list", + output: resolveJsonOutput(parsedOptions), + selector: resolveExplicitSessionSelector(parsedSessionId, parsedOptions.repo), + filePath: parsedOptions.file, + source: parsedOptions.source as ReviewNoteSource | undefined, + }; + } + + if (noteSubcommand === "get" || noteSubcommand === "rm") { + const command = new Command(`session note ${noteSubcommand}`) + .description( + noteSubcommand === "get" + ? "show one inline review note" + : "remove one user-authored inline review note", + ) + .argument("[sessionIdOrNoteId]") + .argument("[noteId]") + .option("--repo ", "target the live session whose repo root matches this path") + .option("--json", "emit structured JSON"); + + let parsedSessionIdOrNoteId: string | undefined; + let parsedNoteId: string | undefined; + let parsedOptions: { repo?: string; json?: boolean } = {}; + command.action( + (sessionIdOrNoteId: string | undefined, noteId: string | undefined, options) => { + parsedSessionIdOrNoteId = sessionIdOrNoteId; + parsedNoteId = noteId; + parsedOptions = options; + }, + ); + + if (noteRest.includes("--help") || noteRest.includes("-h")) { + return { kind: "help", text: `${command.helpInformation().trimEnd()}\n` }; + } + + await parseStandaloneCommand(command, noteRest); + const resolvedNoteId = parsedOptions.repo + ? (parsedNoteId ?? parsedSessionIdOrNoteId) + : parsedNoteId; + const resolvedSessionId = parsedOptions.repo ? undefined : parsedSessionIdOrNoteId; + if (!resolvedNoteId) { + throw new Error(`Pass a note id to session note ${noteSubcommand}.`); + } + + return { + kind: "session", + action: noteSubcommand === "get" ? "note-get" : "note-rm", + output: resolveJsonOutput(parsedOptions), + selector: resolveExplicitSessionSelector(resolvedSessionId, parsedOptions.repo), + noteId: resolvedNoteId, + }; + } + + throw new Error("Supported note subcommands are list, get, and rm."); + } + throw new Error(`Unknown session command: ${subcommand}`); } diff --git a/src/core/types.ts b/src/core/types.ts index b79fa395..f1b33b2a 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -3,6 +3,23 @@ import type { FileDiffMetadata } from "@pierre/diffs"; export type LayoutMode = "auto" | "split" | "stack"; export type VcsMode = "git" | "jj"; +export type ReviewNoteSource = "ai" | "agent" | "user"; + +export interface ReviewNote { + id: string; + source: ReviewNoteSource; + filePath: string; + hunkIndex?: number; + oldRange?: [number, number]; + newRange?: [number, number]; + body: string; + title?: string; + author?: string; + createdAt: string; + updatedAt?: string; + editable: boolean; +} + export interface AgentAnnotation { id?: string; oldRange?: [number, number]; @@ -12,8 +29,11 @@ export interface AgentAnnotation { tags?: string[]; confidence?: "low" | "medium" | "high"; source?: string; + title?: string; author?: string; createdAt?: string; + updatedAt?: string; + editable?: boolean; } export interface AgentFileContext { @@ -119,6 +139,7 @@ export interface SessionReviewCommandInput { output: SessionCommandOutput; selector: SessionSelectorInput; includePatch: boolean; + includeNotes?: boolean; } export interface SessionNavigateCommandInput { @@ -200,6 +221,31 @@ export interface SessionCommentClearCommandInput { confirmed: boolean; } +export interface SessionNoteListCommandInput { + kind: "session"; + action: "note-list"; + output: SessionCommandOutput; + selector: SessionSelectorInput; + filePath?: string; + source?: ReviewNoteSource; +} + +export interface SessionNoteGetCommandInput { + kind: "session"; + action: "note-get"; + output: SessionCommandOutput; + selector: SessionSelectorInput; + noteId: string; +} + +export interface SessionNoteRemoveCommandInput { + kind: "session"; + action: "note-rm"; + output: SessionCommandOutput; + selector: SessionSelectorInput; + noteId: string; +} + export type SessionCommandInput = | SessionListCommandInput | SessionGetCommandInput @@ -210,7 +256,10 @@ export type SessionCommandInput = | SessionCommentApplyCommandInput | SessionCommentListCommandInput | SessionCommentRemoveCommandInput - | SessionCommentClearCommandInput; + | SessionCommentClearCommandInput + | SessionNoteListCommandInput + | SessionNoteGetCommandInput + | SessionNoteRemoveCommandInput; export interface VcsCommandInput { kind: "vcs"; diff --git a/src/hunk-session/bridge.ts b/src/hunk-session/bridge.ts index 17ff9984..cebdd3f5 100644 --- a/src/hunk-session/bridge.ts +++ b/src/hunk-session/bridge.ts @@ -7,6 +7,7 @@ import type { NavigatedSelectionResult, ReloadedSessionResult, RemovedCommentResult, + RemovedUserNoteResult, } from "./types"; export interface HunkSessionBridgeHandlers { @@ -33,6 +34,7 @@ export interface HunkSessionBridgeHandlers { options?: { sourcePath?: string }, ) => Promise; removeLiveComment: (commentId: string) => RemovedCommentResult; + removeUserNote?: (noteId: string) => RemovedUserNoteResult; } /** Build the app-facing bridge handler the generic broker client calls into for Hunk commands. */ @@ -72,6 +74,11 @@ export function createHunkSessionBridge(handlers: HunkSessionBridgeHandlers) { }); case "remove_comment": return handlers.removeLiveComment(message.input.commentId); + case "remove_user_note": + if (!handlers.removeUserNote) { + throw new Error("This Hunk session cannot remove user notes."); + } + return handlers.removeUserNote(message.input.noteId); case "clear_comments": return handlers.clearLiveComments(message.input.filePath); } diff --git a/src/hunk-session/cli.ts b/src/hunk-session/cli.ts index b647b7a0..9d2b7ad7 100644 --- a/src/hunk-session/cli.ts +++ b/src/hunk-session/cli.ts @@ -14,9 +14,11 @@ import type { NavigatedSelectionResult, ReloadedSessionResult, RemovedCommentResult, + RemovedUserNoteResult, SelectedSessionContext, SessionLiveCommentSummary, SessionReview, + SessionReviewNoteSummary, } from "./types"; import type { SessionCommentAddCommandInput, @@ -25,6 +27,9 @@ import type { SessionCommentListCommandInput, SessionCommentRemoveCommandInput, SessionNavigateCommandInput, + SessionNoteGetCommandInput, + SessionNoteListCommandInput, + SessionNoteRemoveCommandInput, SessionReloadCommandInput, SessionReviewCommandInput, SessionSelectorInput, @@ -44,6 +49,9 @@ export interface HunkSessionCliClient { listComments(input: SessionCommentListCommandInput): Promise; removeComment(input: SessionCommentRemoveCommandInput): Promise; clearComments(input: SessionCommentClearCommandInput): Promise; + listNotes?: (input: SessionNoteListCommandInput) => Promise; + getNote?: (input: SessionNoteGetCommandInput) => Promise; + removeNote?: (input: SessionNoteRemoveCommandInput) => Promise; } async function extractResponseError(response: Response) { @@ -102,6 +110,7 @@ class HttpHunkSessionCliClient implements HunkSessionCliClient { action: "review", selector: input.selector, includePatch: input.includePatch, + includeNotes: input.includeNotes, }) ).review; } @@ -187,6 +196,37 @@ class HttpHunkSessionCliClient implements HunkSessionCliClient { }) ).result; } + + async listNotes(input: SessionNoteListCommandInput) { + return ( + await this.request<{ notes: SessionReviewNoteSummary[] }>({ + action: "note-list", + selector: input.selector, + filePath: input.filePath, + source: input.source, + }) + ).notes; + } + + async getNote(input: SessionNoteGetCommandInput) { + return ( + await this.request<{ note: SessionReviewNoteSummary }>({ + action: "note-get", + selector: input.selector, + noteId: input.noteId, + }) + ).note; + } + + async removeNote(input: SessionNoteRemoveCommandInput) { + return ( + await this.request<{ result: RemovedUserNoteResult }>({ + action: "note-rm", + selector: input.selector, + noteId: input.noteId, + }) + ).result; + } } /** Create the concrete Hunk session CLI client that speaks to the broker-backed HTTP API. */ @@ -350,6 +390,15 @@ export function formatReviewOutput(review: SessionReview) { `Selected hunk: ${hunkNumber}`, `Agent notes visible: ${review.showAgentNotes ? "yes" : "no"}`, `Live comments: ${review.liveCommentCount}`, + `Review notes: ${review.reviewNoteCount ?? review.reviewNotes?.length ?? 0}`, + ...(review.reviewNotes + ? [ + "Notes:", + ...review.reviewNotes.map( + (note) => ` - ${note.noteId} [${note.source}] ${note.filePath}: ${note.body}`, + ), + ] + : []), "Files:", ...review.files.flatMap((file) => [ ` - ${file.path} (+${file.additions} -${file.deletions}, hunks: ${file.hunkCount})`, @@ -422,6 +471,43 @@ export function formatRemoveCommentOutput( return `Removed live comment ${result.commentId} from ${describeSessionSelector(selector)}. Remaining comments: ${result.remainingCommentCount}.\n`; } +export function formatNoteListOutput( + selector: SessionSelectorInput, + notes: SessionReviewNoteSummary[], +) { + if (notes.length === 0) { + return `No review notes for ${describeSessionSelector(selector)}.\n`; + } + + return `${notes + .map((note) => + [ + `${note.noteId} ${note.filePath} [${note.source}]`, + ...(note.hunkIndex !== undefined ? [` hunk: ${note.hunkIndex + 1}`] : []), + ` body: ${note.body}`, + ...(note.author ? [` author: ${note.author}`] : []), + ].join("\n"), + ) + .join("\n\n")}\n`; +} + +export function formatNoteGetOutput(note: SessionReviewNoteSummary) { + return `${[ + `${note.noteId} ${note.filePath} [${note.source}]`, + ...(note.hunkIndex !== undefined ? [`hunk: ${note.hunkIndex + 1}`] : []), + `body: ${note.body}`, + ...(note.author ? [`author: ${note.author}`] : []), + "", + ].join("\n")}`; +} + +export function formatRemoveNoteOutput( + selector: SessionSelectorInput, + result: RemovedUserNoteResult, +) { + return `Removed user note ${result.noteId} from ${describeSessionSelector(selector)}. Remaining notes: ${result.remainingNoteCount}.\n`; +} + export function formatClearCommentsOutput( selector: SessionSelectorInput, result: ClearedCommentsResult, diff --git a/src/hunk-session/projections.test.ts b/src/hunk-session/projections.test.ts index de0cf72a..289ec90a 100644 --- a/src/hunk-session/projections.test.ts +++ b/src/hunk-session/projections.test.ts @@ -8,7 +8,9 @@ import { buildHunkSessionReview, buildListedHunkSession, buildSelectedHunkSessionContext, + getHunkSessionNote, listHunkSessionComments, + listHunkSessionNotes, } from "./projections"; function createEntry() { @@ -71,6 +73,31 @@ describe("hunk session projections", () => { expect(withPatch.files[0]).toEqual(expect.objectContaining({ patch: "@@ -1,1 +1,1 @@" })); }); + test("buildHunkSessionReview can include live review notes on demand", () => { + const entry = { + registration: createTestSessionRegistration(), + snapshot: createTestSessionSnapshot({ + reviewNoteCount: 1, + reviewNotes: [ + { + noteId: "user:1", + source: "user", + filePath: "src/example.ts", + body: "Please cover this case.", + author: "user", + createdAt: "2026-05-10T00:00:00.000Z", + editable: true, + }, + ], + }), + }; + + expect(buildHunkSessionReview(entry).reviewNotes).toBeUndefined(); + expect(buildHunkSessionReview(entry, { includeNotes: true }).reviewNotes).toEqual([ + expect.objectContaining({ noteId: "user:1", source: "user" }), + ]); + }); + test("listHunkSessionComments returns live comments and honors file filters", () => { const session = buildListedHunkSession({ registration: createTestSessionRegistration(), @@ -93,4 +120,41 @@ describe("hunk session projections", () => { expect.objectContaining({ commentId: "comment-1" }), ]); }); + + test("listHunkSessionNotes filters by file and source and getHunkSessionNote finds one id", () => { + const session = buildListedHunkSession({ + registration: createTestSessionRegistration(), + snapshot: createTestSessionSnapshot({ + reviewNoteCount: 2, + reviewNotes: [ + { + noteId: "user:1", + source: "user", + filePath: "src/example.ts", + body: "Human note", + createdAt: "2026-05-10T00:00:00.000Z", + editable: true, + }, + { + noteId: "agent:1", + source: "agent", + filePath: "src/other.ts", + body: "Agent note", + createdAt: "2026-05-10T00:00:00.000Z", + editable: false, + }, + ], + }), + }); + + expect(listHunkSessionNotes(session, { source: "user" })).toEqual([ + expect.objectContaining({ noteId: "user:1" }), + ]); + expect(listHunkSessionNotes(session, { filePath: "src/other.ts" })).toEqual([ + expect.objectContaining({ noteId: "agent:1" }), + ]); + expect(getHunkSessionNote(session, "user:1")).toEqual( + expect.objectContaining({ body: "Human note" }), + ); + }); }); diff --git a/src/hunk-session/projections.ts b/src/hunk-session/projections.ts index 2da4f777..302ecf3b 100644 --- a/src/hunk-session/projections.ts +++ b/src/hunk-session/projections.ts @@ -6,6 +6,7 @@ import type { SessionFileSummary, SessionLiveCommentSummary, SessionReview, + SessionReviewNoteSummary, SessionReviewFile, } from "./types"; @@ -107,7 +108,7 @@ export function buildSelectedHunkSessionContext(session: ListedSession): Selecte /** Project one raw broker entry into the Hunk review export used by `hunk session review`. */ export function buildHunkSessionReview( entry: HunkSessionEntryLike, - options: { includePatch?: boolean } = {}, + options: { includePatch?: boolean; includeNotes?: boolean } = {}, ): SessionReview { const selectedFile = findSelectedReviewFile(entry); const includePatch = options.includePatch ?? false; @@ -125,6 +126,9 @@ export function buildHunkSessionReview( : null, showAgentNotes: entry.snapshot.state.showAgentNotes, liveCommentCount: entry.snapshot.state.liveCommentCount, + reviewNoteCount: + entry.snapshot.state.reviewNoteCount ?? entry.snapshot.state.reviewNotes?.length ?? 0, + reviewNotes: options.includeNotes ? (entry.snapshot.state.reviewNotes ?? []) : undefined, files: entry.registration.info.files.map((file) => serializeReviewFile(file, includePatch)), }; } @@ -142,3 +146,26 @@ export function listHunkSessionComments( (comment) => comment.filePath === filter.filePath, ); } + +/** Return review notes for one Hunk session, optionally filtered to a file and source. */ +export function listHunkSessionNotes( + session: ListedSession, + filter: { filePath?: string; source?: SessionReviewNoteSummary["source"] } = {}, +): SessionReviewNoteSummary[] { + return (session.snapshot.state.reviewNotes ?? []).filter((note) => { + if (filter.filePath && note.filePath !== filter.filePath) { + return false; + } + + if (filter.source && note.source !== filter.source) { + return false; + } + + return true; + }); +} + +/** Find one review note in a Hunk session snapshot by id. */ +export function getHunkSessionNote(session: ListedSession, noteId: string) { + return (session.snapshot.state.reviewNotes ?? []).find((note) => note.noteId === noteId) ?? null; +} diff --git a/src/hunk-session/sessionRegistration.ts b/src/hunk-session/sessionRegistration.ts index 44549fe8..958d93cc 100644 --- a/src/hunk-session/sessionRegistration.ts +++ b/src/hunk-session/sessionRegistration.ts @@ -107,6 +107,8 @@ export function createInitialSessionSnapshot(bootstrap: AppBootstrap): HunkSessi showAgentNotes: bootstrap.initialShowAgentNotes ?? false, liveCommentCount: 0, liveComments: [], + reviewNoteCount: 0, + reviewNotes: [], }, }; } diff --git a/src/hunk-session/types.ts b/src/hunk-session/types.ts index 8d3ae1e7..b86d870c 100644 --- a/src/hunk-session/types.ts +++ b/src/hunk-session/types.ts @@ -1,4 +1,4 @@ -import type { AgentAnnotation, CliInput } from "../core/types"; +import type { AgentAnnotation, CliInput, ReviewNoteSource } from "../core/types"; import type { SessionBrokerClient } from "../session-broker/brokerClient"; import type { SessionClientMessage, @@ -56,6 +56,8 @@ export interface HunkSessionState { showAgentNotes: boolean; liveCommentCount: number; liveComments: SessionLiveCommentSummary[]; + reviewNoteCount?: number; + reviewNotes?: SessionReviewNoteSummary[]; } export type HunkSessionRegistration = SessionRegistration; @@ -103,6 +105,10 @@ export interface RemoveCommentToolInput extends SessionTargetInput { commentId: string; } +export interface RemoveUserNoteToolInput extends SessionTargetInput { + noteId: string; +} + export interface ClearCommentsToolInput extends SessionTargetInput { filePath?: string; } @@ -130,6 +136,21 @@ export interface SessionLiveCommentSummary { createdAt: string; } +export interface SessionReviewNoteSummary { + noteId: string; + source: ReviewNoteSource; + filePath: string; + hunkIndex?: number; + oldRange?: [number, number]; + newRange?: [number, number]; + body: string; + title?: string; + author?: string; + createdAt: string; + updatedAt?: string; + editable: boolean; +} + export interface AppliedCommentResult { commentId: string; fileId: string; @@ -156,6 +177,12 @@ export interface RemovedCommentResult { remainingCommentCount: number; } +export interface RemovedUserNoteResult { + noteId: string; + removed: boolean; + remainingNoteCount: number; +} + export interface ClearedCommentsResult { removedCount: number; remainingCommentCount: number; @@ -211,6 +238,8 @@ export interface SessionReview { selectedHunk: SessionReviewHunk | null; showAgentNotes: boolean; liveCommentCount: number; + reviewNoteCount?: number; + reviewNotes?: SessionReviewNoteSummary[]; files: SessionReviewFile[]; } @@ -219,6 +248,7 @@ export type HunkSessionCommandResult = | AppliedCommentBatchResult | NavigatedSelectionResult | RemovedCommentResult + | RemovedUserNoteResult | ClearedCommentsResult | ReloadedSessionResult; @@ -241,4 +271,5 @@ export type HunkSessionServerMessage = | SessionServerMessage<"navigate_to_hunk", NavigateToHunkToolInput> | SessionServerMessage<"reload_session", ReloadSessionToolInput> | SessionServerMessage<"remove_comment", RemoveCommentToolInput> + | SessionServerMessage<"remove_user_note", RemoveUserNoteToolInput> | SessionServerMessage<"clear_comments", ClearCommentsToolInput>; diff --git a/src/hunk-session/wire.ts b/src/hunk-session/wire.ts index 184abf28..e25f20d8 100644 --- a/src/hunk-session/wire.ts +++ b/src/hunk-session/wire.ts @@ -9,6 +9,7 @@ import type { HunkSessionInfo, HunkSessionState, SessionLiveCommentSummary, + SessionReviewNoteSummary, SessionReviewFile, SessionReviewHunk, } from "./types"; @@ -138,6 +139,47 @@ function parseSessionLiveCommentSummary(value: unknown): SessionLiveCommentSumma }; } +/** Parse one review note summary from the app-owned snapshot payload. */ +function parseSessionReviewNoteSummary(value: unknown): SessionReviewNoteSummary | null { + const record = brokerWireParsers.asRecord(value); + if (!record) { + return null; + } + + const noteId = brokerWireParsers.parseRequiredString(record.noteId); + const filePath = brokerWireParsers.parseRequiredString(record.filePath); + const body = brokerWireParsers.parseRequiredString(record.body); + const createdAt = brokerWireParsers.parseRequiredString(record.createdAt); + const source = + record.source === "ai" || record.source === "agent" || record.source === "user" + ? record.source + : null; + if ( + noteId === null || + filePath === null || + body === null || + createdAt === null || + source === null + ) { + return null; + } + + return { + noteId, + source, + filePath, + hunkIndex: brokerWireParsers.parseNonNegativeInt(record.hunkIndex) ?? undefined, + oldRange: parseOptionalRange(record.oldRange), + newRange: parseOptionalRange(record.newRange), + body, + title: brokerWireParsers.parseOptionalString(record.title), + author: brokerWireParsers.parseOptionalString(record.author), + createdAt, + updatedAt: brokerWireParsers.parseOptionalString(record.updatedAt), + editable: typeof record.editable === "boolean" ? record.editable : source === "user", + }; +} + /** Parse the app-owned registration info embedded inside one broker registration envelope. */ function parseHunkSessionInfo(value: unknown): HunkSessionInfo | null { const record = brokerWireParsers.asRecord(value); @@ -181,6 +223,9 @@ function parseHunkSessionState(value: unknown): HunkSessionState | null { const liveComments = record.liveComments .map(parseSessionLiveCommentSummary) .filter((comment): comment is SessionLiveCommentSummary => comment !== null); + const reviewNotes = (Array.isArray(record.reviewNotes) ? record.reviewNotes : []) + .map(parseSessionReviewNoteSummary) + .filter((note): note is SessionReviewNoteSummary => note !== null); return { selectedFileId: brokerWireParsers.parseOptionalString(record.selectedFileId), @@ -191,6 +236,8 @@ function parseHunkSessionState(value: unknown): HunkSessionState | null { showAgentNotes, liveCommentCount: liveComments.length, liveComments, + reviewNoteCount: reviewNotes.length, + reviewNotes, }; } diff --git a/src/session-broker/brokerServer.test.ts b/src/session-broker/brokerServer.test.ts index e262802c..708d6fb8 100644 --- a/src/session-broker/brokerServer.test.ts +++ b/src/session-broker/brokerServer.test.ts @@ -213,7 +213,7 @@ describe("Hunk session daemon server", () => { expect(capabilities.status).toBe(200); await expect(capabilities.json()).resolves.toMatchObject({ version: 1, - daemonVersion: 2, + daemonVersion: 3, actions: [ "list", "get", @@ -226,6 +226,9 @@ describe("Hunk session daemon server", () => { "comment-list", "comment-rm", "comment-clear", + "note-list", + "note-get", + "note-rm", ], }); diff --git a/src/session-broker/brokerServer.ts b/src/session-broker/brokerServer.ts index fa3a600e..fb3d0eba 100644 --- a/src/session-broker/brokerServer.ts +++ b/src/session-broker/brokerServer.ts @@ -21,7 +21,9 @@ import type { NavigatedSelectionResult, ReloadedSessionResult, RemovedCommentResult, + RemovedUserNoteResult, } from "../hunk-session/types"; +import { getHunkSessionNote, listHunkSessionNotes } from "../hunk-session/projections"; import { HUNK_SESSION_API_PATH, HUNK_SESSION_API_VERSION, @@ -49,6 +51,9 @@ const SUPPORTED_SESSION_ACTIONS: SessionDaemonAction[] = [ "comment-list", "comment-rm", "comment-clear", + "note-list", + "note-get", + "note-rm", ]; export interface ServeSessionBrokerDaemonOptions { @@ -115,11 +120,16 @@ async function handleSessionApiRequest(state: HunkSessionBrokerState, request: R case "context": response = { context: state.getSelectedContext(input.selector) }; break; - case "review": - response = { - review: state.getSessionReview(input.selector, { includePatch: input.includePatch }), - }; + case "review": { + const review = state.getSessionReview(input.selector, { includePatch: input.includePatch }); + if (input.includeNotes) { + const session = state.getSession(input.selector); + review.reviewNotes = session.snapshot.state.reviewNotes ?? []; + review.reviewNoteCount = review.reviewNotes.length; + } + response = { review }; break; + } case "navigate": { if ( !input.commentDirection && @@ -234,6 +244,35 @@ async function handleSessionApiRequest(state: HunkSessionBrokerState, request: R }), }; break; + case "note-list": + response = { + notes: listHunkSessionNotes(state.getSession(input.selector), { + filePath: input.filePath, + source: input.source, + }), + }; + break; + case "note-get": { + const note = getHunkSessionNote(state.getSession(input.selector), input.noteId); + if (!note) { + throw new Error(`No review note matches id ${input.noteId}.`); + } + response = { note }; + break; + } + case "note-rm": + response = { + result: await state.dispatchCommand({ + selector: input.selector, + command: "remove_user_note", + input: { + ...input.selector, + noteId: input.noteId, + }, + timeoutMessage: "Timed out waiting for the session to remove the requested note.", + }), + }; + break; default: throw new Error("Unknown session API action."); } diff --git a/src/session/commands.ts b/src/session/commands.ts index b76d0c94..c4590706 100644 --- a/src/session/commands.ts +++ b/src/session/commands.ts @@ -21,8 +21,11 @@ import { formatContextOutput, formatListOutput, formatNavigationOutput, + formatNoteGetOutput, + formatNoteListOutput, formatReloadOutput, formatRemoveCommentOutput, + formatRemoveNoteOutput, formatReviewOutput, formatSessionOutput, stringifyJson, @@ -43,6 +46,9 @@ const REQUIRED_ACTION_BY_COMMAND: Record + formatNoteListOutput(input.selector, notes), + ); + } + case "note-get": { + if (!client.getNote) { + throw new Error("The active Hunk session client does not support note-get."); + } + const note = await client.getNote({ + ...input, + selector: normalizedSelector!, + }); + return renderOutput(input.output, { note }, () => formatNoteGetOutput(note)); + } + case "note-rm": { + if (!client.removeNote) { + throw new Error("The active Hunk session client does not support note-rm."); + } + const result = await client.removeNote({ + ...input, + selector: normalizedSelector!, + }); + return renderOutput(input.output, { result }, () => + formatRemoveNoteOutput(input.selector, result), + ); + } } } diff --git a/src/session/protocol.ts b/src/session/protocol.ts index 2814cd55..0057a41b 100644 --- a/src/session/protocol.ts +++ b/src/session/protocol.ts @@ -5,6 +5,9 @@ import type { SessionCommentListCommandInput, SessionCommentRemoveCommandInput, SessionNavigateCommandInput, + SessionNoteGetCommandInput, + SessionNoteListCommandInput, + SessionNoteRemoveCommandInput, SessionReloadCommandInput, SessionReviewCommandInput, SessionSelectorInput, @@ -17,9 +20,11 @@ import type { NavigatedSelectionResult, ReloadedSessionResult, RemovedCommentResult, + RemovedUserNoteResult, SelectedSessionContext, SessionLiveCommentSummary, SessionReview, + SessionReviewNoteSummary, } from "../hunk-session/types"; export const HUNK_SESSION_API_PATH = "/session-api"; @@ -30,7 +35,7 @@ export const HUNK_SESSION_API_VERSION = 1; * Version daemon/session compatibility separately from the HTTP action surface so newer Hunk * builds can refresh an older daemon even when it still exposes the same API endpoints. */ -export const HUNK_SESSION_DAEMON_VERSION = 2; +export const HUNK_SESSION_DAEMON_VERSION = 3; export type SessionDaemonAction = | "list" @@ -43,7 +48,10 @@ export type SessionDaemonAction = | "comment-apply" | "comment-list" | "comment-rm" - | "comment-clear"; + | "comment-clear" + | "note-list" + | "note-get" + | "note-rm"; export interface SessionDaemonCapabilities { version: number; @@ -67,6 +75,7 @@ export type SessionDaemonRequest = action: "review"; selector: SessionSelectorInput; includePatch: SessionReviewCommandInput["includePatch"]; + includeNotes: SessionReviewCommandInput["includeNotes"]; } | { action: "navigate"; @@ -114,6 +123,22 @@ export type SessionDaemonRequest = action: "comment-clear"; selector: SessionCommentClearCommandInput["selector"]; filePath?: string; + } + | { + action: "note-list"; + selector: SessionNoteListCommandInput["selector"]; + filePath?: string; + source?: SessionNoteListCommandInput["source"]; + } + | { + action: "note-get"; + selector: SessionNoteGetCommandInput["selector"]; + noteId: string; + } + | { + action: "note-rm"; + selector: SessionNoteRemoveCommandInput["selector"]; + noteId: string; }; export type SessionDaemonResponse = @@ -126,5 +151,8 @@ export type SessionDaemonResponse = | { result: AppliedCommentResult } | { result: AppliedCommentBatchResult } | { comments: SessionLiveCommentSummary[] } + | { notes: SessionReviewNoteSummary[] } + | { note: SessionReviewNoteSummary } | { result: RemovedCommentResult } + | { result: RemovedUserNoteResult } | { result: ClearedCommentsResult }; diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 56b71523..87f4858d 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -28,7 +28,7 @@ import { resolveResponsiveLayout } from "./lib/responsive"; import { resizeSidebarWidth } from "./lib/sidebar"; import { resolveTheme, THEMES } from "./themes"; -type FocusArea = "files" | "filter"; +type FocusArea = "files" | "filter" | "note"; const FAST_CODE_HORIZONTAL_SCROLL_COLUMNS = 8; @@ -150,6 +150,9 @@ export function App({ openAgentNotes, reloadSession: onReloadSession, removeLiveComment: review.removeLiveComment, + removeUserNote: review.removeUserNote, + reviewNoteCount: review.reviewNoteCount, + reviewNoteSummaries: review.reviewNoteSummaries, selectedFile, selectedHunk: review.selectedHunk, selectedHunkIndex, @@ -460,6 +463,29 @@ export function App({ setFocusArea((current) => (current === "files" ? "filter" : "files")); }, []); + /** Start a user-authored inline note and move keyboard focus into it. */ + const startUserNote = useCallback( + (fileId?: string, hunkIndex?: number) => { + const draft = review.startUserNote(fileId, hunkIndex); + if (draft) { + setFocusArea("note"); + } + }, + [review.startUserNote], + ); + + /** Save the active draft note and return focus to review navigation. */ + const saveDraftNote = useCallback(() => { + review.saveDraftNote(); + setFocusArea("files"); + }, [review.saveDraftNote]); + + /** Cancel the active draft note and return focus to review navigation. */ + const cancelDraftNote = useCallback(() => { + review.cancelDraftNote(); + setFocusArea("files"); + }, [review.cancelDraftNote]); + /** Cycle through the available built-in themes. */ const cycleTheme = useCallback(() => { const currentIndex = THEMES.findIndex((theme) => theme.id === activeTheme.id); @@ -545,6 +571,7 @@ export function App({ closeHelp, closeMenu, cycleTheme, + cancelDraftNote, focusArea, focusFilter, moveToAnnotatedHunk, @@ -554,9 +581,11 @@ export function App({ pagerMode, requestQuit, scrollCodeHorizontally, + saveDraftNote, scrollDiff, selectLayoutMode, showHelp, + startUserNote: () => startUserNote(), switchMenu, toggleAgentNotes, toggleFocusArea, @@ -709,6 +738,7 @@ export function App({ selectedFileId={selectedFile?.id} selectedHunkIndex={selectedHunkIndex} scrollToNote={review.scrollToNote} + draftNote={review.draftNote} separatorWidth={diffSeparatorWidth} showAgentNotes={showAgentNotes} showLineNumbers={showLineNumbers} @@ -722,6 +752,11 @@ export function App({ theme={activeTheme} width={diffPaneWidth} onOpenAgentNotesAtHunk={openAgentNotesAtHunk} + onRemoveUserNote={review.removeUserNote} + onSaveDraftNote={saveDraftNote} + onStartUserNoteAtHunk={startUserNote} + onUpdateDraftNote={review.updateDraftNote} + onCancelDraftNote={cancelDraftNote} onScrollCodeHorizontally={(delta) => { scrollCodeHorizontally(delta * FAST_CODE_HORIZONTAL_SCROLL_COLUMNS); }} diff --git a/src/ui/AppHost.interactions.test.tsx b/src/ui/AppHost.interactions.test.tsx index 82dc9e45..b12e8d2d 100644 --- a/src/ui/AppHost.interactions.test.tsx +++ b/src/ui/AppHost.interactions.test.tsx @@ -990,12 +990,12 @@ describe("App interactions", () => { await flush(setup); const frame = setup.captureCharFrame(); - expect(frame).toContain("AI note · ▶ new 2"); + expect(frame).toContain("AI note · prefs.ts R2"); expect(frame).toContain("Annotation for prefs.ts"); expect(frame).toContain("Why prefs.ts changed"); expect(frame).not.toContain("@@ -1,1 +1,2 @@"); expect(frame).not.toContain("1 - export const message"); - expect(frame.indexOf("AI note · ▶ new 2")).toBeLessThan( + expect(frame.indexOf("AI note · prefs.ts R2")).toBeLessThan( frame.indexOf("export const added = true;"), ); } finally { diff --git a/src/ui/components/panes/AgentInlineNote.tsx b/src/ui/components/panes/AgentInlineNote.tsx index a14be446..dff5e9d9 100644 --- a/src/ui/components/panes/AgentInlineNote.tsx +++ b/src/ui/components/panes/AgentInlineNote.tsx @@ -1,11 +1,18 @@ -import type { AgentAnnotation, LayoutMode } from "../../../core/types"; +import type { AgentAnnotation, DiffFile, LayoutMode } from "../../../core/types"; +import { isEscapeKey } from "../../lib/keyboard"; import { wrapText } from "../../lib/agentPopover"; -import { annotationRangeLabel } from "../../lib/agentAnnotations"; +import { annotationRangeLabel, reviewNoteSource } from "../../lib/agentAnnotations"; import { fitText, padText } from "../../lib/text"; import type { AppTheme } from "../../themes"; -function inlineNoteTitle(noteIndex: number, noteCount: number) { - return noteCount > 1 ? `AI note ${noteIndex + 1}/${noteCount}` : "AI note"; +function inlineNoteTitle(annotation: AgentAnnotation, noteIndex: number, noteCount: number) { + if (annotation.source === "user-draft") { + return "Draft note"; + } + + const source = reviewNoteSource(annotation); + const label = source === "user" ? "Your note" : source === "agent" ? "Agent note" : "AI note"; + return noteCount > 1 ? `${label} ${noteIndex + 1}/${noteCount}` : label; } interface AgentInlineNoteLine { @@ -48,16 +55,25 @@ export function measureAgentInlineNoteHeight({ const boxWidth = clamp(preferredDockWidth, 28, Math.max(28, width - 4)); const innerWidth = Math.max(1, boxWidth - 2); const bodyWidth = innerWidth; + const contentWidth = Math.max(1, bodyWidth - 2); const lines: AgentInlineNoteLine[] = [ - ...wrapText(annotation.summary, bodyWidth).map((text) => ({ kind: "summary" as const, text })), + ...wrapText(annotation.summary, contentWidth).map((text) => ({ + kind: "summary" as const, + text, + })), ...(annotation.rationale - ? wrapText(annotation.rationale, bodyWidth).map((text) => ({ + ? wrapText(annotation.rationale, contentWidth).map((text) => ({ kind: "rationale" as const, text, })) : []), ]; + if (annotation.source === "user-draft") { + // Title cap + connector + five-line body + button footer. + return 11; + } + // top border + title row + body lines + bottom border return 3 + lines.length; } @@ -66,24 +82,35 @@ export function measureAgentInlineNoteHeight({ export function AgentInlineNote({ annotation, anchorSide, + file, layout, noteCount = 1, noteIndex = 0, + draft, onClose, theme, width, }: { annotation: AgentAnnotation; anchorSide?: "old" | "new"; + file?: DiffFile; layout: Exclude; noteCount?: number; noteIndex?: number; + draft?: { + body: string; + focused: boolean; + onCancel: () => void; + onInput: (value: string) => void; + onSave: () => void; + }; onClose?: () => void; theme: AppTheme; width: number; }) { const closeText = onClose ? "[x]" : ""; - const titleText = `${inlineNoteTitle(noteIndex, noteCount)} · ${annotationRangeLabel(annotation)}`; + const titleSeparator = annotation.source === "user-draft" ? " - " : " · "; + const titleText = `${inlineNoteTitle(annotation, noteIndex, noteCount)}${titleSeparator}${annotationRangeLabel(annotation, file)}`; const splitWidths = splitColumnWidths(width); const canDockRight = layout === "split" && anchorSide === "new" && width >= 84; const canDockLeft = layout === "split" && anchorSide === "old" && width >= 84; @@ -99,12 +126,19 @@ export function AgentInlineNote({ ? 0 : Math.min(4, Math.max(0, width - boxWidth)); const innerWidth = Math.max(1, boxWidth - 2); - const titleWidth = Math.max(1, innerWidth - (closeText ? closeText.length + 1 : 0)); + const titleSidePadding = 1; + const closeGapWidth = closeText ? 1 : 0; + const closeWidth = closeText.length; + const titleWidth = Math.max(1, innerWidth - titleSidePadding * 2 - closeGapWidth - closeWidth); const bodyWidth = innerWidth; + const contentWidth = Math.max(1, bodyWidth - 2); const lines: AgentInlineNoteLine[] = [ - ...wrapText(annotation.summary, bodyWidth).map((text) => ({ kind: "summary" as const, text })), + ...wrapText(annotation.summary, contentWidth).map((text) => ({ + kind: "summary" as const, + text, + })), ...(annotation.rationale - ? wrapText(annotation.rationale, bodyWidth).map((text) => ({ + ? wrapText(annotation.rationale, contentWidth).map((text) => ({ kind: "rationale" as const, text, })) @@ -118,6 +152,200 @@ export function AgentInlineNote({ ? `├${"─".repeat(Math.max(0, boxWidth - 2))}┘` : `└${"─".repeat(Math.max(0, boxWidth - 2))}┘`; + if (draft) { + const draftBodyRows = 5; + const draftTitleBoxWidth = clamp(titleText.length + 4, 16, boxWidth); + const draftInnerWidth = Math.max(1, boxWidth - 2); + const draftContentWidth = Math.max(1, draftInnerWidth - 2); + const connectorRightWidth = Math.max(0, boxWidth - draftTitleBoxWidth - 1); + const saveInnerWidth = 6; + const cancelInnerWidth = 8; + const footerRemainderWidth = Math.max( + 0, + boxWidth - (1 + saveInnerWidth + 1 + cancelInnerWidth + 1 + 1), + ); + const footerWidth = 1 + saveInnerWidth + 1 + cancelInnerWidth + 1; + const draftTopBorder = `┌${"─".repeat(Math.max(0, draftTitleBoxWidth - 2))}┐`; + const draftConnector = `├${"─".repeat(Math.max(0, draftTitleBoxWidth - 2))}┴${"─".repeat(connectorRightWidth)}┐`; + const draftActionBorder = `├${"─".repeat(saveInnerWidth)}┬${"─".repeat(cancelInnerWidth)}┬${"─".repeat(footerRemainderWidth)}┘`; + const draftButtonBottom = `└${"─".repeat(saveInnerWidth)}┴${"─".repeat(cancelInnerWidth)}┘`; + + return ( + + + + {" ".repeat(boxLeft)} + + + + {draftTopBorder} + + + + + + + {" ".repeat(boxLeft)} + + + + │ + + + + + {padText(fitText(` ${titleText} `, draftTitleBoxWidth - 2), draftTitleBoxWidth - 2)} + + + + + │ + + + + + + + {" ".repeat(boxLeft)} + + + + {draftConnector} + + + + + {Array.from({ length: draftBodyRows }, (_, rowIndex) => ( + + + {" ".repeat(boxLeft)} + + + + │ + + + + + + + {rowIndex === 0 ? ( + { + if (isEscapeKey(key)) { + key.preventDefault(); + key.stopPropagation(); + draft.onCancel(); + return; + } + + if (key.ctrl && (key.name === "s" || key.sequence === "\u0013")) { + key.preventDefault(); + key.stopPropagation(); + draft.onSave(); + } + }} + /> + ) : ( + {" ".repeat(draftContentWidth)} + )} + + + + + + + │ + + + + ))} + + + + {" ".repeat(boxLeft)} + + + + {draftActionBorder} + + + + + + + {" ".repeat(boxLeft)} + + + + │ + + + + + {padText(" Save", saveInnerWidth)} + + + + + │ + + + + + {padText(" Cancel", cancelInnerWidth)} + + + + + │ + + + + + + + {" ".repeat(boxLeft)} + + + + {draftButtonBottom} + + + + + ); + } + return ( @@ -140,19 +368,32 @@ export function AgentInlineNote({ │ + + {" ".repeat(titleSidePadding)} + {padText(fitText(titleText, titleWidth), titleWidth)} + {closeText ? ( + + {" ".repeat(closeGapWidth)} + + ) : null} {closeText ? ( - {` ${closeText}`} + + {closeText} + ) : null} + + {" ".repeat(titleSidePadding)} + │ @@ -173,11 +414,17 @@ export function AgentInlineNote({ │ - + + + + - {padText(line.text, bodyWidth)} + {padText(line.text, contentWidth)} + + + │ diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index dec8fd1c..0d7d255f 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -9,8 +9,13 @@ import { useState, type RefObject, } from "react"; -import type { DiffFile, LayoutMode } from "../../../core/types"; -import type { VisibleAgentNote } from "../../lib/agentAnnotations"; +import type { AgentAnnotation, DiffFile, LayoutMode } from "../../../core/types"; +import type { DraftReviewNote } from "../../hooks/useReviewController"; +import { + alwaysShowReviewNote, + reviewNoteSource, + type VisibleAgentNote, +} from "../../lib/agentAnnotations"; import { computeHunkRevealScrollTop } from "../../lib/hunkScroll"; import { measureDiffSectionGeometry, @@ -41,7 +46,6 @@ import type { VisibleBodyBounds } from "../../diff/rowWindowing"; import { prefetchHighlightedDiff } from "../../diff/useHighlightedDiff"; const EMPTY_VISIBLE_AGENT_NOTES: VisibleAgentNote[] = []; -const EMPTY_VISIBLE_AGENT_NOTES_BY_FILE = new Map(); /** * Clamp one vertical scroll target into the currently reachable review-stream extent. @@ -137,6 +141,7 @@ export function DiffPane({ selectedFileId, selectedHunkIndex, scrollToNote = false, + draftNote = null, separatorWidth, pagerMode = false, showAgentNotes, @@ -151,6 +156,11 @@ export function DiffPane({ theme, width, onOpenAgentNotesAtHunk, + onRemoveUserNote, + onSaveDraftNote, + onStartUserNoteAtHunk, + onUpdateDraftNote, + onCancelDraftNote, onScrollCodeHorizontally = () => {}, onSelectFile, onViewportCenteredHunkChange, @@ -165,6 +175,7 @@ export function DiffPane({ selectedFileId?: string; selectedHunkIndex: number; scrollToNote?: boolean; + draftNote?: DraftReviewNote | null; separatorWidth: number; pagerMode?: boolean; showAgentNotes: boolean; @@ -179,6 +190,11 @@ export function DiffPane({ theme: AppTheme; width: number; onOpenAgentNotesAtHunk: (fileId: string, hunkIndex: number) => void; + onRemoveUserNote?: (noteId: string) => void; + onSaveDraftNote?: () => void; + onStartUserNoteAtHunk?: (fileId: string, hunkIndex: number) => void; + onUpdateDraftNote?: (body: string) => void; + onCancelDraftNote?: () => void; onScrollCodeHorizontally?: (delta: number) => void; onSelectFile: (fileId: string) => void; onViewportCenteredHunkChange?: (fileId: string, hunkIndex: number) => void; @@ -249,27 +265,67 @@ export function DiffPane({ const allAgentNotesByFile = useMemo(() => { const next = new Map(); - if (!showAgentNotes) { - return next; - } - files.forEach((file) => { - const annotations = file.agent?.annotations ?? []; - if (annotations.length === 0) { - return; - } + const annotations = (file.agent?.annotations ?? []).filter( + (annotation) => showAgentNotes || alwaysShowReviewNote(annotation), + ); + const notes: VisibleAgentNote[] = annotations.map((annotation, index) => { + const source = reviewNoteSource(annotation); + if (source !== "user") { + return { + id: `annotation:${file.id}:${annotation.id ?? index}`, + annotation, + }; + } - next.set( - file.id, - annotations.map((annotation, index) => ({ + return { id: `annotation:${file.id}:${annotation.id ?? index}`, annotation, - })), - ); + source, + editable: true, + onRemove: annotation.id ? () => onRemoveUserNote?.(annotation.id!) : undefined, + }; + }); + + if (draftNote?.fileId === file.id) { + const draftAnnotation: AgentAnnotation = { + id: draftNote.id, + source: "user-draft", + summary: draftNote.body || " ", + oldRange: draftNote.oldRange, + newRange: draftNote.newRange, + editable: true, + }; + notes.push({ + id: draftNote.id, + annotation: draftAnnotation, + source: "draft", + editable: true, + draft: { + body: draftNote.body, + focused: true, + onCancel: onCancelDraftNote ?? (() => {}), + onInput: onUpdateDraftNote ?? (() => {}), + onSave: onSaveDraftNote ?? (() => {}), + }, + }); + } + + if (notes.length > 0) { + next.set(file.id, notes); + } }); return next; - }, [files, showAgentNotes]); + }, [ + draftNote, + files, + onCancelDraftNote, + onRemoveUserNote, + onSaveDraftNote, + onUpdateDraftNote, + showAgentNotes, + ]); // Keep exact row rendering for wrapped lines and the selected file's visible notes; // other files can still use placeholders and viewport windowing. @@ -421,10 +477,6 @@ export function DiffPane({ const visibleAgentNotesByFile = useMemo(() => { const next = new Map(); - if (!showAgentNotes) { - return EMPTY_VISIBLE_AGENT_NOTES_BY_FILE; - } - const fileIdsToMeasure = new Set(visibleViewportFileIds); // Always measure the selected file with its real note rows so hunk navigation can compute // accurate bounds even before the file scrolls into the visible viewport. @@ -1185,6 +1237,9 @@ export function DiffPane({ onOpenAgentNotesAtHunk={(hunkIndex) => onOpenAgentNotesAtHunk(file.id, hunkIndex) } + onStartUserNoteAtHunk={(hunkIndex) => + onStartUserNoteAtHunk?.(file.id, hunkIndex) + } onSelect={() => onSelectFile(file.id)} /> ); diff --git a/src/ui/components/panes/DiffSection.tsx b/src/ui/components/panes/DiffSection.tsx index 88c021df..6f43f29a 100644 --- a/src/ui/components/panes/DiffSection.tsx +++ b/src/ui/components/panes/DiffSection.tsx @@ -29,6 +29,7 @@ interface DiffSectionProps { visibleBodyBounds?: VisibleBodyBounds; viewWidth: number; onOpenAgentNotesAtHunk: (hunkIndex: number) => void; + onStartUserNoteAtHunk?: (hunkIndex: number) => void; onSelect: () => void; } @@ -53,6 +54,7 @@ function DiffSectionComponent({ visibleBodyBounds, viewWidth, onOpenAgentNotesAtHunk, + onStartUserNoteAtHunk, onSelect, }: DiffSectionProps) { const annotatedHunkIndices = getAnnotatedHunkIndices(file); @@ -103,6 +105,7 @@ function DiffSectionComponent({ annotatedHunkIndices={annotatedHunkIndices} visibleAgentNotes={visibleAgentNotes} onOpenAgentNotesAtHunk={onOpenAgentNotesAtHunk} + onStartUserNoteAtHunk={onStartUserNoteAtHunk} selectedHunkIndex={selectedHunkIndex} sectionGeometry={sectionGeometry} shouldLoadHighlight={shouldLoadHighlight} diff --git a/src/ui/components/ui-components.test.tsx b/src/ui/components/ui-components.test.tsx index 668a3f15..bffc6e3b 100644 --- a/src/ui/components/ui-components.test.tsx +++ b/src/ui/components/ui-components.test.tsx @@ -1,4 +1,4 @@ -import { describe, expect, test } from "bun:test"; +import { describe, expect, mock, test } from "bun:test"; import type { ScrollBoxRenderable } from "@opentui/core"; import { testRender } from "@opentui/react/test-utils"; import { act, createRef, useEffect, useState, type ReactNode } from "react"; @@ -21,6 +21,7 @@ const { MenuDropdown } = await import("./chrome/MenuDropdown"); const { StatusBar } = await import("./chrome/StatusBar"); const { DiffSectionPlaceholder } = await import("./panes/DiffSectionPlaceholder"); const { PierreDiffView } = await import("../diff/PierreDiffView"); +const { DiffRowView } = await import("../diff/renderRows"); function createTestDiffFile( id: string, @@ -499,6 +500,65 @@ describe("UI components", () => { expect(frame.indexOf("alpha.ts")).toBeLessThan(frame.indexOf("beta.ts")); }); + test("DiffRowView renders a clickable add-note affordance for a hovered diff row", async () => { + const theme = resolveTheme("midnight", null); + const startUserNote = mock(() => undefined); + const setup = await testRender( + , + { width: 80, height: 3 }, + ); + + try { + await act(async () => { + await setup.renderOnce(); + }); + const frame = setup.captureCharFrame(); + expect(frame).toContain("[+]"); + const addNoteY = frame.split("\n").findIndex((line) => line.includes("[+]")); + const addNoteX = frame.split("\n")[addNoteY]?.indexOf("[+]") ?? -1; + expect(addNoteY).toBeGreaterThanOrEqual(0); + expect(addNoteX).toBeGreaterThanOrEqual(0); + + await act(async () => { + await setup.mockMouse.click(4, addNoteY); + }); + expect(startUserNote).not.toHaveBeenCalled(); + + await act(async () => { + await setup.mockMouse.click(addNoteX + 1, addNoteY); + }); + expect(startUserNote).toHaveBeenCalledWith(0); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("DiffPane scrolls a later selected file into view in the windowed path", async () => { const files = createWindowingFiles(6); const theme = resolveTheme("midnight", null); @@ -1278,13 +1338,54 @@ describe("UI components", () => { const lines = frame.split("\n"); expect(lines[0]?.trimStart().startsWith("┌")).toBe(true); - expect(lines[1]).toContain("AI note · ▶ new 2-4"); + expect(lines[1]).toContain("AI note · R2–R4"); expect(lines[1]).toContain("[x]"); expect(lines[2]).toContain("Summary line"); expect(lines[3]).toContain("Rationale line."); expect(lines[4]?.trimStart().startsWith("└")).toBe(true); }); + test("AgentInlineNote renders draft notes as an editable composer", async () => { + const theme = resolveTheme("midnight", null); + const file = createTestDiffFile( + "draft", + "src/core/cli.ts", + "export const value = 1;\n", + "export const value = 2;\n", + ); + const frame = await captureFrame( + {}, + onInput: () => {}, + onSave: () => {}, + }} + file={file} + anchorSide="new" + layout="split" + theme={theme} + width={96} + />, + 100, + 12, + ); + + const lines = frame.split("\n"); + expect(lines[0]).toContain("┌"); + expect(lines[1]).toContain("│ Draft note - src/core/cli.ts R611 │"); + expect(lines[2]).toContain("┴"); + expect(frame).toContain("│ my comment. I think we should think"); + expect(frame).toContain("│ Save │ Cancel │"); + expect(frame).toContain("└──────┴────────┘"); + }); + test("DiffPane renders all visible hunk notes across the review stream", async () => { const bootstrap = createBootstrap(); bootstrap.changeset.files[1]!.agent = { @@ -1327,13 +1428,13 @@ describe("UI components", () => { 28, ); - expect(frame).toContain("AI note · ▶ new 2"); + expect(frame).toContain("AI note · alpha.ts R2"); expect(frame).toContain("Annotation for alpha.ts"); expect(frame).toContain("Why alpha.ts changed"); - expect(frame.indexOf("AI note · ▶ new 2")).toBeLessThan( + expect(frame.indexOf("AI note · alpha.ts R2")).toBeLessThan( frame.indexOf("2 + export const add = true;"), ); - expect(frame).toContain("AI note · ▶ new 1"); + expect(frame).toContain("AI note · beta.ts R1"); expect(frame).toContain("Annotation for beta.ts"); expect(frame).toContain("Why beta.ts changed"); expect(frame).not.toContain("alpha.ts note"); @@ -1938,11 +2039,11 @@ describe("UI components", () => { ); expect(frame).not.toContain("@@ -1,1 +1,2 @@"); - expect(frame).toContain("AI note · hunk"); + expect(frame).toContain("AI note · note-fallback.ts hunk"); expect(frame).toContain("Ungrounded note"); expect(frame).toContain("Falls back to the first visible"); expect(frame).toContain("row."); - expect(frame.indexOf("AI note · hunk")).toBeLessThan( + expect(frame.indexOf("AI note · note-fallback.ts hunk")).toBeLessThan( frame.indexOf("1 - export const value = 1;"), ); }); diff --git a/src/ui/diff/PierreDiffView.tsx b/src/ui/diff/PierreDiffView.tsx index 0060525d..e562a7b6 100644 --- a/src/ui/diff/PierreDiffView.tsx +++ b/src/ui/diff/PierreDiffView.tsx @@ -1,4 +1,4 @@ -import { useMemo } from "react"; +import { useMemo, useState } from "react"; import type { DiffFile, LayoutMode } from "../../core/types"; import { AgentInlineNote, AgentInlineNoteGuideCap } from "../components/panes/AgentInlineNote"; import type { VisibleAgentNote } from "../lib/agentAnnotations"; @@ -23,6 +23,7 @@ export function PierreDiffView({ file, layout, onOpenAgentNotesAtHunk, + onStartUserNoteAtHunk, showLineNumbers = true, showHunkHeaders = true, wrapLines = false, @@ -40,6 +41,7 @@ export function PierreDiffView({ file: DiffFile | undefined; layout: Exclude; onOpenAgentNotesAtHunk?: (hunkIndex: number) => void; + onStartUserNoteAtHunk?: (hunkIndex: number) => void; showLineNumbers?: boolean; showHunkHeaders?: boolean; wrapLines?: boolean; @@ -52,6 +54,7 @@ export function PierreDiffView({ scrollable?: boolean; visibleBodyBounds?: VisibleBodyBounds; }) { + const [hoveredRowKey, setHoveredRowKey] = useState(null); const resolvedHighlighted = useHighlightedDiff({ file, appearance: theme.appearance, @@ -159,9 +162,12 @@ export function PierreDiffView({ @@ -178,7 +184,12 @@ export function PierreDiffView({ } return ( - + setHoveredRowKey(plannedRow.key)} + > ); diff --git a/src/ui/diff/renderRows.tsx b/src/ui/diff/renderRows.tsx index 3576e074..78e91991 100644 --- a/src/ui/diff/renderRows.tsx +++ b/src/ui/diff/renderRows.tsx @@ -91,6 +91,7 @@ function sliceSpansWindow(spans: RenderSpan[], offset: number, width: number) { } const marker = diffRailMarker; +const addNoteBadgeText = "[+]"; /** Render a fixed-width inline span sequence for one diff cell. */ function renderInlineSpans( @@ -454,16 +455,34 @@ function renderHeaderRow( selected: boolean, annotated: boolean, anchorId?: string, + showAddNoteBadge = false, + onHoverRow?: (rowKey: string) => void, onOpenAgentNotesAtHunk?: (hunkIndex: number) => void, + onStartUserNoteAtHunk?: (hunkIndex: number) => void, ) { - const badgeText = annotated ? "[AI]" : ""; - const badgeWidth = annotated ? badgeText.length + 1 : 0; + const badges = [ + annotated + ? { + key: "agent", + text: "[AI]", + onClick: () => onOpenAgentNotesAtHunk?.(row.hunkIndex), + } + : null, + showAddNoteBadge + ? { + key: "user-note", + text: "[+]", + onClick: () => onStartUserNoteAtHunk?.(row.hunkIndex), + } + : null, + ].filter((badge): badge is { key: string; text: string; onClick: () => void } => Boolean(badge)); + const badgeWidth = badges.reduce((total, badge) => total + badge.text.length + 1, 0); const label = row.type === "collapsed" ? fitText(`··· ${row.text} ···`, Math.max(0, width - 1 - badgeWidth)) : fitText(row.text, Math.max(0, width - 1 - badgeWidth)); - if (!annotated) { + if (badges.length === 0) { return ( onHoverRow?.(row.key)} > onHoverRow?.(row.key)} > @@ -519,12 +540,35 @@ function renderHeaderRow( - onOpenAgentNotesAtHunk?.(row.hunkIndex)} - > - {` ${badgeText}`} - + {badges.map((badge) => ( + + {` ${badge.text}`} + + ))} + + ); +} + +/** Render the hover-only add-note target as a separate clickable hit area. */ +function renderAddNoteButton( + key: string, + theme: AppTheme, + hunkIndex: number, + onStartUserNoteAtHunk?: (hunkIndex: number) => void, +) { + return ( + onStartUserNoteAtHunk?.(hunkIndex)} + > + + {addNoteBadgeText} + ); } @@ -607,7 +651,10 @@ function renderRow( annotated: boolean, anchorId?: string, noteGuideSide?: "old" | "new", + showAddNoteBadge = false, + onHoverRow?: (rowKey: string) => void, onOpenAgentNotesAtHunk?: (hunkIndex: number) => void, + onStartUserNoteAtHunk?: (hunkIndex: number) => void, ) { let baseRow: ReactNode; @@ -619,19 +666,34 @@ function renderRow( selected, annotated, anchorId, + showAddNoteBadge, + onHoverRow, onOpenAgentNotesAtHunk, + onStartUserNoteAtHunk, ); } else if (row.type === "hunk-header") { baseRow = showHunkHeaders - ? renderHeaderRow(row, width, theme, selected, annotated, anchorId, onOpenAgentNotesAtHunk) + ? renderHeaderRow( + row, + width, + theme, + selected, + annotated, + anchorId, + showAddNoteBadge, + onHoverRow, + onOpenAgentNotesAtHunk, + onStartUserNoteAtHunk, + ) : null; } else if (row.type === "split-line") { const guideOnOldSide = noteGuideSide === "old"; const guideOnNewSide = noteGuideSide === "new"; - // Reserve fixed columns for the diff rails and center separator slot. + // Reserve fixed columns for the diff rails, center separator slot, and hover affordance. + const addBadgeWidth = showAddNoteBadge ? addNoteBadgeText.length : 0; const { leftWidth, rightWidth } = resolveSplitPaneWidths(width); - const rightRenderWidth = Math.max(0, rightWidth - (guideOnNewSide ? 1 : 0)); + const rightRenderWidth = Math.max(0, rightWidth - (guideOnNewSide ? 1 : 0) - addBadgeWidth); const leftPrefix = { text: guideOnOldSide ? "│" : marker(), fg: guideOnOldSide ? theme.noteBorder : splitLeftRailColor(row.left.kind, theme, selected), @@ -645,34 +707,53 @@ function renderRow( if (!wrapLines) { baseRow = ( - - - {renderSplitCell( - row.left, - leftWidth, - lineNumberDigits, - showLineNumbers, - theme, - `${row.key}:left`, - codeHorizontalOffset, - leftPrefix, - )} - {renderSplitCell( - row.right, - rightRenderWidth, - lineNumberDigits, - showLineNumbers, - theme, - `${row.key}:right`, - codeHorizontalOffset, - rightPrefix, - )} - {guideOnNewSide ? ( - - │ - - ) : null} - + onHoverRow?.(row.key)} + > + + + {renderSplitCell( + row.left, + leftWidth, + lineNumberDigits, + showLineNumbers, + theme, + `${row.key}:left`, + codeHorizontalOffset, + leftPrefix, + )} + {renderSplitCell( + row.right, + rightRenderWidth, + lineNumberDigits, + showLineNumbers, + theme, + `${row.key}:right`, + codeHorizontalOffset, + rightPrefix, + )} + {guideOnNewSide ? ( + + │ + + ) : null} + + + {showAddNoteBadge + ? renderAddNoteButton( + `${row.key}:add-note`, + theme, + row.hunkIndex, + onStartUserNoteAtHunk, + ) + : null} ); } else { @@ -714,31 +795,52 @@ function renderRow( spans: [], }; + const showBadgeOnLine = showAddNoteBadge && index === 0; + return ( - - - {renderWrappedSplitCellLine( - leftLine, - leftLayout.palette, - leftContentWidth, - theme, - `${row.key}:left:${index}`, - leftPrefix, - )} - {renderWrappedSplitCellLine( - rightLine, - rightLayout.palette, - rightContentWidth, - theme, - `${row.key}:right:${index}`, - rightPrefix, - )} - {guideOnNewSide ? ( - - │ - - ) : null} - + onHoverRow?.(row.key)} + > + + + {renderWrappedSplitCellLine( + leftLine, + leftLayout.palette, + leftContentWidth, + theme, + `${row.key}:left:${index}`, + leftPrefix, + )} + {renderWrappedSplitCellLine( + rightLine, + rightLayout.palette, + rightContentWidth, + theme, + `${row.key}:right:${index}`, + rightPrefix, + )} + {guideOnNewSide ? ( + + │ + + ) : null} + + + {showBadgeOnLine + ? renderAddNoteButton( + `${row.key}:add-note:${index}`, + theme, + row.hunkIndex, + onStartUserNoteAtHunk, + ) + : null} ); })} @@ -748,7 +850,8 @@ function renderRow( } else if (row.type === "stack-line") { const guideOnOldSide = noteGuideSide === "old"; const guideOnNewSide = noteGuideSide === "new"; - const contentWidth = Math.max(0, width - (guideOnNewSide ? 1 : 0)); + const addBadgeWidth = showAddNoteBadge ? addNoteBadgeText.length : 0; + const contentWidth = Math.max(0, width - (guideOnNewSide ? 1 : 0) - addBadgeWidth); const prefix = { text: guideOnOldSide ? "│" : marker(), fg: guideOnOldSide ? theme.noteBorder : stackRailColor(row.cell.kind, theme, selected), @@ -757,24 +860,43 @@ function renderRow( if (!wrapLines) { baseRow = ( - - - {renderStackCell( - row.cell, - contentWidth, - lineNumberDigits, - showLineNumbers, - theme, - `${row.key}:stack`, - codeHorizontalOffset, - prefix, - )} - {guideOnNewSide ? ( - - │ - - ) : null} - + onHoverRow?.(row.key)} + > + + + {renderStackCell( + row.cell, + contentWidth, + lineNumberDigits, + showLineNumbers, + theme, + `${row.key}:stack`, + codeHorizontalOffset, + prefix, + )} + {guideOnNewSide ? ( + + │ + + ) : null} + + + {showAddNoteBadge + ? renderAddNoteButton( + `${row.key}:add-note`, + theme, + row.hunkIndex, + onStartUserNoteAtHunk, + ) + : null} ); } else { @@ -793,25 +915,48 @@ function renderRow( baseRow = ( - {layout.lines.map((line, index) => ( - - - {renderWrappedStackCellLine( - line, - layout.palette, - wrappedContentWidth, - theme, - `${row.key}:stack:${index}`, - prefix, - )} - {guideOnNewSide ? ( - - │ - - ) : null} - - - ))} + {layout.lines.map((line, index) => { + const showBadgeOnLine = showAddNoteBadge && index === 0; + + return ( + onHoverRow?.(row.key)} + > + + + {renderWrappedStackCellLine( + line, + layout.palette, + wrappedContentWidth, + theme, + `${row.key}:stack:${index}`, + prefix, + )} + {guideOnNewSide ? ( + + │ + + ) : null} + + + {showBadgeOnLine + ? renderAddNoteButton( + `${row.key}:add-note:${index}`, + theme, + row.hunkIndex, + onStartUserNoteAtHunk, + ) + : null} + + ); + })} ); } @@ -839,7 +984,10 @@ interface DiffRowViewProps { annotated: boolean; anchorId?: string; noteGuideSide?: "old" | "new"; + showAddNoteBadge?: boolean; + onHoverRow?: (rowKey: string) => void; onOpenAgentNotesAtHunk?: (hunkIndex: number) => void; + onStartUserNoteAtHunk?: (hunkIndex: number) => void; } /** Render one diff row, memoized to avoid unnecessary rerenders. */ @@ -857,7 +1005,10 @@ export const DiffRowView = memo( annotated, anchorId, noteGuideSide, + showAddNoteBadge, + onHoverRow, onOpenAgentNotesAtHunk, + onStartUserNoteAtHunk, }: DiffRowViewProps) { return renderRow( row, @@ -872,7 +1023,10 @@ export const DiffRowView = memo( annotated, anchorId, noteGuideSide, + showAddNoteBadge, + onHoverRow, onOpenAgentNotesAtHunk, + onStartUserNoteAtHunk, ); }, (previous, next) => { @@ -888,7 +1042,11 @@ export const DiffRowView = memo( previous.selected === next.selected && previous.annotated === next.annotated && previous.anchorId === next.anchorId && - previous.noteGuideSide === next.noteGuideSide + previous.noteGuideSide === next.noteGuideSide && + previous.showAddNoteBadge === next.showAddNoteBadge && + previous.onHoverRow === next.onHoverRow && + previous.onOpenAgentNotesAtHunk === next.onOpenAgentNotesAtHunk && + previous.onStartUserNoteAtHunk === next.onStartUserNoteAtHunk ); }, ); diff --git a/src/ui/diff/reviewRenderPlan.ts b/src/ui/diff/reviewRenderPlan.ts index ba2e89ed..77f9eaf2 100644 --- a/src/ui/diff/reviewRenderPlan.ts +++ b/src/ui/diff/reviewRenderPlan.ts @@ -39,6 +39,7 @@ export type PlannedReviewRow = hunkIndex: number; annotationId: string; annotation: AgentAnnotation; + note: VisibleAgentNote; anchorSide?: "old" | "new"; noteCount: number; noteIndex: number; @@ -363,6 +364,7 @@ export function buildReviewRenderPlan({ hunkIndex: placement.hunkIndex, annotationId: placement.note.id, annotation: placement.note.annotation, + note: placement.note, anchorSide: placement.anchorSide, noteCount: placement.noteCount, noteIndex: placement.noteIndex, diff --git a/src/ui/hooks/useAppKeyboardShortcuts.ts b/src/ui/hooks/useAppKeyboardShortcuts.ts index f2349d9e..001b64fb 100644 --- a/src/ui/hooks/useAppKeyboardShortcuts.ts +++ b/src/ui/hooks/useAppKeyboardShortcuts.ts @@ -14,7 +14,7 @@ import { isStepUpKey, } from "../lib/keyboard"; -type FocusArea = "files" | "filter"; +type FocusArea = "files" | "filter" | "note"; type ScrollUnit = "step" | "viewport" | "content" | "half"; const FAST_CODE_HORIZONTAL_SCROLL_COLUMNS = 8; @@ -26,6 +26,7 @@ export interface UseAppKeyboardShortcutsOptions { closeHelp: () => void; closeMenu: () => void; cycleTheme: () => void; + cancelDraftNote: () => void; focusArea: FocusArea; focusFilter: () => void; moveToAnnotatedHunk: (delta: number) => void; @@ -36,8 +37,10 @@ export interface UseAppKeyboardShortcutsOptions { requestQuit: () => void; scrollCodeHorizontally: (delta: number) => void; scrollDiff: (delta: number, unit: ScrollUnit) => void; + saveDraftNote: () => void; selectLayoutMode: (mode: LayoutMode) => void; showHelp: boolean; + startUserNote: () => void; switchMenu: (delta: number) => void; toggleAgentNotes: () => void; toggleFocusArea: () => void; @@ -57,6 +60,7 @@ export function useAppKeyboardShortcuts({ closeHelp, closeMenu, cycleTheme, + cancelDraftNote, focusArea, focusFilter, moveToAnnotatedHunk, @@ -66,9 +70,11 @@ export function useAppKeyboardShortcuts({ pagerMode, requestQuit, scrollCodeHorizontally, + saveDraftNote, scrollDiff, selectLayoutMode, showHelp, + startUserNote, switchMenu, toggleAgentNotes, toggleFocusArea, @@ -225,17 +231,32 @@ export function useAppKeyboardShortcuts({ return false; }; - const handleFilterShortcut = (key: KeyEvent) => { - if (focusAreaRef.current !== "filter") { + const handleFocusedInputShortcut = (key: KeyEvent) => { + if (focusAreaRef.current === "filter") { + if (key.name === "tab") { + toggleFocusArea(); + return true; + } + + // Let the focused input own filter editing and escape handling. + return true; + } + + if (focusAreaRef.current !== "note") { return false; } - if (key.name === "tab") { - toggleFocusArea(); + if (isEscapeKey(key)) { + cancelDraftNote(); return true; } - // Let the focused input own filter editing and escape handling. + if (key.ctrl && (key.name === "s" || key.sequence === "\u0013")) { + saveDraftNote(); + return true; + } + + // Let the focused inline note input own text editing. return true; }; @@ -266,6 +287,11 @@ export function useAppKeyboardShortcuts({ return; } + if (key.name?.toLowerCase() === "i" || key.sequence?.toLowerCase() === "i") { + runAndCloseMenu(startUserNote); + return; + } + if (isPageDownKey(key)) { scrollDiff(1, "viewport"); return; @@ -404,7 +430,7 @@ export function useAppKeyboardShortcuts({ return; } - if (handleFilterShortcut(key)) { + if (handleFocusedInputShortcut(key)) { return; } diff --git a/src/ui/hooks/useHunkSessionBridge.ts b/src/ui/hooks/useHunkSessionBridge.ts index 248d9313..cab71c97 100644 --- a/src/ui/hooks/useHunkSessionBridge.ts +++ b/src/ui/hooks/useHunkSessionBridge.ts @@ -6,6 +6,7 @@ import type { HunkSessionBrokerClient, ReloadedSessionResult, SessionLiveCommentSummary, + SessionReviewNoteSummary, } from "../../hunk-session/types"; import type { ReviewController } from "./useReviewController"; @@ -21,6 +22,9 @@ export function useHunkSessionBridge({ openAgentNotes, reloadSession, removeLiveComment, + removeUserNote, + reviewNoteCount, + reviewNoteSummaries, selectedFile, selectedHunk, selectedHunkIndex, @@ -39,6 +43,9 @@ export function useHunkSessionBridge({ options?: { resetApp?: boolean; sourcePath?: string }, ) => Promise; removeLiveComment: ReviewController["removeLiveComment"]; + removeUserNote: ReviewController["removeUserNote"]; + reviewNoteCount: number; + reviewNoteSummaries: SessionReviewNoteSummary[]; selectedFile: DiffFile | undefined; selectedHunk: DiffFile["metadata"]["hunks"][number] | undefined; selectedHunkIndex: number; @@ -54,6 +61,7 @@ export function useHunkSessionBridge({ openAgentNotes, reloadSession: (nextInput, options) => reloadSession(nextInput, { ...options }), removeLiveComment, + removeUserNote, }), [ addLiveComment, @@ -63,6 +71,7 @@ export function useHunkSessionBridge({ openAgentNotes, reloadSession, removeLiveComment, + removeUserNote, ], ); @@ -92,12 +101,16 @@ export function useHunkSessionBridge({ showAgentNotes, liveCommentCount, liveComments: liveCommentSummaries, + reviewNoteCount, + reviewNotes: reviewNoteSummaries, }, }); }, [ hostClient, liveCommentCount, liveCommentSummaries, + reviewNoteCount, + reviewNoteSummaries, selectedFile?.id, selectedFile?.path, selectedHunk, diff --git a/src/ui/hooks/useReviewController.test.tsx b/src/ui/hooks/useReviewController.test.tsx index d0885f59..20b2b1d8 100644 --- a/src/ui/hooks/useReviewController.test.tsx +++ b/src/ui/hooks/useReviewController.test.tsx @@ -363,4 +363,83 @@ describe("useReviewController", () => { }); } }); + + test("user note drafts can be saved, edited, removed, and exposed as review notes", async () => { + const controllerRef: { current: ReviewController | null } = { current: null }; + const setup = await testRender( + { + controllerRef.current = nextController; + }} + />, + { width: 80, height: 4 }, + ); + + try { + await flush(setup); + + await act(async () => { + expectValue(controllerRef.current).startUserNote(); + expectValue(controllerRef.current).updateDraftNote("Please add a regression test."); + }); + await flush(setup); + + let savedNoteId = ""; + await act(async () => { + const saved = expectValue(controllerRef.current).saveDraftNote(); + savedNoteId = saved?.id ?? ""; + }); + await flush(setup); + + expect(savedNoteId).toStartWith("user:"); + expect(expectValue(controllerRef.current).userNotesByFileId.alpha).toHaveLength(1); + expect(expectValue(controllerRef.current).reviewNoteSummaries).toMatchObject([ + { + noteId: savedNoteId, + source: "user", + filePath: "alpha.ts", + body: "Please add a regression test.", + editable: true, + }, + ]); + + await act(async () => { + expectValue(controllerRef.current).editUserNote(savedNoteId); + }); + await flush(setup); + await act(async () => { + 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).reviewNoteSummaries[0]).toMatchObject({ + noteId: savedNoteId, + body: "Please add two regression tests.", + }); + + await act(async () => { + expectValue(controllerRef.current).removeUserNote(savedNoteId); + }); + await flush(setup); + + expect(expectValue(controllerRef.current).userNotesByFileId.alpha).toBeUndefined(); + expect(expectValue(controllerRef.current).reviewNoteSummaries).toEqual([]); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); }); diff --git a/src/ui/hooks/useReviewController.ts b/src/ui/hooks/useReviewController.ts index cd2508dc..05b44a56 100644 --- a/src/ui/hooks/useReviewController.ts +++ b/src/ui/hooks/useReviewController.ts @@ -17,9 +17,10 @@ import { import { buildLiveComment, findDiffFileByPath, + firstCommentTargetForHunk, resolveCommentTarget, } from "../../core/liveComments"; -import type { DiffFile } from "../../core/types"; +import type { AgentAnnotation, DiffFile } from "../../core/types"; import type { AppliedCommentBatchResult, AppliedCommentResult, @@ -30,7 +31,9 @@ import type { NavigateToHunkToolInput, NavigatedSelectionResult, RemovedCommentResult, + RemovedUserNoteResult, SessionLiveCommentSummary, + SessionReviewNoteSummary, } from "../../hunk-session/types"; import { findNextHunkCursor } from "../lib/hunks"; import { @@ -46,6 +49,48 @@ function clamp(value: number, min: number, max: number) { return Math.min(Math.max(value, min), max); } +/** Merge file-id keyed annotation maps without losing their concrete item types. */ +function mergeAnnotationMaps( + first: Record, + second: Record, +): Record> { + const next: Record> = {}; + for (const [fileId, annotations] of Object.entries(first)) { + next[fileId] = [...annotations]; + } + for (const [fileId, annotations] of Object.entries(second)) { + next[fileId] = [...(next[fileId] ?? []), ...annotations]; + } + return next; +} + +export interface UserReviewNote extends AgentAnnotation { + id: string; + source: "user"; + filePath: string; + hunkIndex: number; + side: "old" | "new"; + line: number; + summary: string; + author: string; + createdAt: string; + updatedAt?: string; + editable: true; +} + +export interface DraftReviewNote { + id: string; + fileId: string; + filePath: string; + hunkIndex: number; + side: "old" | "new"; + line: number; + oldRange?: [number, number]; + newRange?: [number, number]; + body: string; + editingNoteId?: string; +} + export interface ReviewSelectionOptions { alignFileHeaderTop?: boolean; preserveViewport?: boolean; @@ -55,9 +100,13 @@ export interface ReviewSelectionOptions { export interface ReviewController { allFiles: DiffFile[]; filter: string; + draftNote: DraftReviewNote | null; liveCommentCount: number; liveCommentSummaries: SessionLiveCommentSummary[]; liveCommentsByFileId: Record; + reviewNoteCount: number; + reviewNoteSummaries: SessionReviewNoteSummary[]; + userNotesByFileId: Record; moveToAnnotatedFile: (delta: number) => void; moveToAnnotatedHunk: (delta: number) => void; moveToHunk: (delta: number) => void; @@ -84,9 +133,15 @@ export interface ReviewController { clearLiveComments: (filePath?: string) => ClearedCommentsResult; navigateToLocation: (input: NavigateToHunkToolInput) => NavigatedSelectionResult; removeLiveComment: (commentId: string) => RemovedCommentResult; + cancelDraftNote: () => void; + editUserNote: (noteId: string) => void; + removeUserNote: (noteId: string) => RemovedUserNoteResult; + saveDraftNote: () => UserReviewNote | null; selectFile: (fileId: string, nextHunkIndex?: number, options?: ReviewSelectionOptions) => void; selectHunk: (fileId: string, hunkIndex: number, options?: ReviewSelectionOptions) => void; + startUserNote: (fileId?: string, hunkIndex?: number) => DraftReviewNote | null; setFilter: (value: string) => void; + updateDraftNote: (body: string) => void; } /** Own the shared review stream state used by both the UI and session bridge. */ @@ -100,6 +155,8 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon const [liveCommentsByFileId, setLiveCommentsByFileId] = useState>( {}, ); + const [userNotesByFileId, setUserNotesByFileId] = useState>({}); + const [draftNote, setDraftNote] = useState(null); const deferredFilter = useDeferredValue(filter); const { @@ -114,12 +171,19 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon () => buildReviewState({ files, - liveCommentsByFileId, + liveCommentsByFileId: mergeAnnotationMaps(liveCommentsByFileId, userNotesByFileId), filterQuery: deferredFilter, selectedFileId, selectedHunkIndex, }), - [deferredFilter, files, liveCommentsByFileId, selectedFileId, selectedHunkIndex], + [ + deferredFilter, + files, + liveCommentsByFileId, + selectedFileId, + selectedHunkIndex, + userNotesByFileId, + ], ); /** Update the selection and reveal intent together so diff scrolling stays explicit. */ @@ -458,12 +522,198 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon [allFiles, liveCommentsByFileId], ); + /** Start a human-authored draft note at the selected or requested hunk. */ + const startUserNote = useCallback( + (fileId = selectedFile?.id, hunkIndex = selectedHunkIndex): DraftReviewNote | null => { + const file = allFiles.find((candidate) => candidate.id === fileId); + const hunk = file?.metadata.hunks[hunkIndex]; + if (!file || !hunk) { + return null; + } + + const target = firstCommentTargetForHunk(hunk); + const draft: DraftReviewNote = { + id: `draft:${file.id}:${hunkIndex}:${Date.now()}`, + fileId: file.id, + filePath: file.path, + hunkIndex, + side: target.side, + line: target.line, + oldRange: target.side === "old" ? [target.line, target.line] : undefined, + newRange: target.side === "new" ? [target.line, target.line] : undefined, + body: "", + }; + setDraftNote(draft); + selectHunk(file.id, hunkIndex, { scrollToNote: true }); + return draft; + }, + [allFiles, selectHunk, selectedFile?.id, selectedHunkIndex], + ); + + /** Update the body of the active draft note. */ + const updateDraftNote = useCallback((body: string) => { + setDraftNote((current) => (current ? { ...current, body } : current)); + }, []); + + /** Discard the active human note draft. */ + const cancelDraftNote = useCallback(() => { + setDraftNote(null); + }, []); + + /** Persist the active draft into the in-memory user note collection. */ + const saveDraftNote = useCallback((): UserReviewNote | null => { + if (!draftNote) { + return null; + } + + const body = draftNote.body.trim(); + if (!body) { + setDraftNote(null); + return null; + } + + const existingNote = (userNotesByFileId[draftNote.fileId] ?? []).find( + (note) => note.id === draftNote.editingNoteId, + ); + const now = new Date().toISOString(); + const savedNote: UserReviewNote = { + id: draftNote.editingNoteId ?? `user:${Date.now()}`, + source: "user", + filePath: draftNote.filePath, + hunkIndex: draftNote.hunkIndex, + side: draftNote.side, + line: draftNote.line, + oldRange: draftNote.oldRange, + newRange: draftNote.newRange, + summary: body, + author: existingNote?.author ?? "user", + createdAt: existingNote?.createdAt ?? now, + updatedAt: existingNote ? now : undefined, + editable: true, + }; + + setUserNotesByFileId((notesByFile) => { + const existing = notesByFile[draftNote.fileId] ?? []; + const withoutEdited = draftNote.editingNoteId + ? existing.filter((note) => note.id !== draftNote.editingNoteId) + : existing; + return { + ...notesByFile, + [draftNote.fileId]: [...withoutEdited, savedNote], + }; + }); + setDraftNote(null); + return savedNote; + }, [draftNote, userNotesByFileId]); + + /** Reopen an existing user note as the active inline draft. */ + const editUserNote = useCallback( + (noteId: string) => { + for (const [fileId, notes] of Object.entries(userNotesByFileId)) { + const note = notes.find((candidate) => candidate.id === noteId); + if (!note) { + continue; + } + + setDraftNote({ + id: `draft:${note.id}`, + fileId, + filePath: note.filePath, + hunkIndex: note.hunkIndex, + side: note.side, + line: note.line, + oldRange: note.oldRange, + newRange: note.newRange, + body: note.summary, + editingNoteId: note.id, + }); + selectHunk(fileId, note.hunkIndex, { scrollToNote: true }); + return; + } + + throw new Error(`No user note matches id ${noteId}.`); + }, + [selectHunk, userNotesByFileId], + ); + + /** Remove one in-memory user note by id. */ + const removeUserNote = useCallback( + (noteId: string): RemovedUserNoteResult => { + let removed = false; + let remainingNoteCount = 0; + const next: Record = {}; + + for (const [fileId, notes] of Object.entries(userNotesByFileId)) { + const filtered = notes.filter((note) => note.id !== noteId); + if (filtered.length !== notes.length) { + removed = true; + } + if (filtered.length > 0) { + next[fileId] = filtered; + remainingNoteCount += filtered.length; + } + } + + if (!removed) { + throw new Error(`No user note matches id ${noteId}.`); + } + + setUserNotesByFileId(next); + setDraftNote((current) => (current?.editingNoteId === noteId ? null : current)); + return { noteId, removed: true, remainingNoteCount }; + }, + [userNotesByFileId], + ); + /** Count all currently tracked live comments, including ones hidden by the active filter. */ const liveCommentCount = useMemo( () => Object.values(liveCommentsByFileId).reduce((sum, comments) => sum + comments.length, 0), [liveCommentsByFileId], ); + /** Format current inline notes for daemon snapshots without exposing UI-only objects. */ + const reviewNoteSummaries = useMemo(() => { + const noteSummaries: SessionReviewNoteSummary[] = []; + + files.forEach((file) => { + (liveCommentsByFileId[file.id] ?? []).forEach((comment) => { + noteSummaries.push({ + noteId: comment.id, + source: "agent", + filePath: file.path, + hunkIndex: comment.hunkIndex, + oldRange: comment.oldRange, + newRange: comment.newRange, + body: [comment.summary, comment.rationale].filter(Boolean).join("\n\n"), + author: comment.author, + createdAt: comment.createdAt, + editable: false, + }); + }); + + (userNotesByFileId[file.id] ?? []).forEach((note) => { + noteSummaries.push({ + noteId: note.id, + source: "user", + filePath: file.path, + hunkIndex: note.hunkIndex, + oldRange: note.oldRange, + newRange: note.newRange, + body: note.summary, + author: note.author, + createdAt: note.createdAt, + updatedAt: note.updatedAt, + editable: true, + }); + }); + }); + + return noteSummaries; + }, [files, liveCommentsByFileId, userNotesByFileId]); + + /** Count all currently tracked review notes, including AI, agent, and user notes. */ + const reviewNoteCount = reviewNoteSummaries.length; + /** Format current live comments for daemon snapshots without exposing merged UI-only objects. */ const liveCommentSummaries = useMemo( () => @@ -485,10 +735,14 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon return { allFiles, + draftNote, filter, liveCommentCount, liveCommentSummaries, liveCommentsByFileId, + reviewNoteCount, + reviewNoteSummaries, + userNotesByFileId, scrollToNote, selectedFile, selectedFileId, @@ -501,14 +755,20 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon addLiveComment, addLiveCommentBatch, clearFilter, + cancelDraftNote, clearLiveComments, + editUserNote, moveToAnnotatedFile, moveToAnnotatedHunk, moveToHunk, navigateToLocation, removeLiveComment, + removeUserNote, + saveDraftNote, selectFile, selectHunk, + startUserNote, setFilter, + updateDraftNote, }; } diff --git a/src/ui/lib/agentAnnotations.test.ts b/src/ui/lib/agentAnnotations.test.ts index cd3c7837..fdc3414b 100644 --- a/src/ui/lib/agentAnnotations.test.ts +++ b/src/ui/lib/agentAnnotations.test.ts @@ -1,7 +1,11 @@ import { describe, expect, test } from "bun:test"; import { createTestDiffFile, lines } from "../../../test/helpers/diff-helpers"; import { buildLiveComment, resolveCommentTarget } from "../../core/liveComments"; -import { getAnnotatedHunkIndices, getSelectedAnnotations } from "./agentAnnotations"; +import { + annotationRangeLabel, + getAnnotatedHunkIndices, + getSelectedAnnotations, +} from "./agentAnnotations"; function createContextHeavyHunkFile() { const beforeLines = Array.from({ length: 25 }, (_, i) => `line${i + 1}`); @@ -18,6 +22,20 @@ function createContextHeavyHunkFile() { } describe("agent annotations", () => { + test("formats inline note locations with GitHub-style file and side anchors", () => { + const file = createContextHeavyHunkFile(); + + expect(annotationRangeLabel({ summary: "Added", newRange: [142, 142] }, file)).toBe( + "src/sparse.ts R142", + ); + expect(annotationRangeLabel({ summary: "Removed", oldRange: [88, 91] }, file)).toBe( + "src/sparse.ts L88–L91", + ); + expect( + annotationRangeLabel({ summary: "Changed", oldRange: [10, 11], newRange: [20, 21] }, file), + ).toBe("src/sparse.ts L10–L11 → R20–R21"); + }); + test("keeps hunk-number comments visible when anchored after leading context", () => { const file = createContextHeavyHunkFile(); const hunk = file.metadata.hunks[0]!; diff --git a/src/ui/lib/agentAnnotations.ts b/src/ui/lib/agentAnnotations.ts index 7ae51c0e..92526d74 100644 --- a/src/ui/lib/agentAnnotations.ts +++ b/src/ui/lib/agentAnnotations.ts @@ -1,11 +1,21 @@ import type { Hunk } from "@pierre/diffs"; -import type { AgentAnnotation, DiffFile } from "../../core/types"; +import type { AgentAnnotation, DiffFile, ReviewNoteSource } from "../../core/types"; import { hunkLineRange } from "../../core/liveComments"; import { fileLabel } from "./files"; export interface VisibleAgentNote { id: string; annotation: AgentAnnotation; + source?: ReviewNoteSource | "draft"; + editable?: boolean; + draft?: { + body: string; + focused: boolean; + onCancel: () => void; + onInput: (value: string) => void; + onSave: () => void; + }; + onRemove?: () => void; } export interface AnnotationAnchor { @@ -13,6 +23,24 @@ export interface AnnotationAnchor { lineNumber: number; } +/** Resolve the user-facing source for one inline note annotation. */ +export function reviewNoteSource(annotation: AgentAnnotation): ReviewNoteSource { + if (annotation.source === "user") { + return "user"; + } + + if (annotation.source === "mcp" || annotation.source === "agent") { + return "agent"; + } + + return "ai"; +} + +/** Return whether a note should remain visible when the AI note layer is hidden. */ +export function alwaysShowReviewNote(annotation: AgentAnnotation) { + return reviewNoteSource(annotation) === "user"; +} + /** Check whether two inclusive line ranges overlap. */ function overlap(rangeA: [number, number], rangeB: [number, number]) { return rangeA[0] <= rangeB[1] && rangeB[0] <= rangeA[1]; @@ -82,19 +110,26 @@ export function annotationAnchor(annotation: AgentAnnotation): AnnotationAnchor return null; } -/** Build a concise side-aware range label for inline note rows. */ -export function annotationRangeLabel(annotation: AgentAnnotation) { +function formatGithubStyleRange(prefix: "L" | "R", range: [number, number]) { + return range[0] === range[1] + ? `${prefix}${range[0]}` + : `${prefix}${range[0]}–${prefix}${range[1]}`; +} + +/** Build a concise GitHub-style file-and-line label for inline note rows. */ +export function annotationRangeLabel(annotation: AgentAnnotation, file?: DiffFile) { const locationParts: string[] = []; if (annotation.oldRange) { - locationParts.push(`◀ old ${formatRange(annotation.oldRange)}`); + locationParts.push(formatGithubStyleRange("L", annotation.oldRange)); } if (annotation.newRange) { - locationParts.push(`▶ new ${formatRange(annotation.newRange)}`); + locationParts.push(formatGithubStyleRange("R", annotation.newRange)); } - return locationParts.join(" · ") || "hunk"; + const location = locationParts.join(" → ") || "hunk"; + return file ? `${fileLabel(file)} ${location}` : location; } /** Build the compact file-and-lines label shown on a framed agent note card. */ diff --git a/src/ui/lib/reviewState.ts b/src/ui/lib/reviewState.ts index c5a647ba..f52b53a2 100644 --- a/src/ui/lib/reviewState.ts +++ b/src/ui/lib/reviewState.ts @@ -7,12 +7,8 @@ * tested without React state in the loop. */ import { findDiffFileByPath, findHunkIndexForLine, hunkLineRange } from "../../core/liveComments"; -import type { DiffFile } from "../../core/types"; -import type { - LiveComment, - NavigateToHunkToolInput, - SelectedHunkSummary, -} from "../../hunk-session/types"; +import type { AgentAnnotation, DiffFile } from "../../core/types"; +import type { NavigateToHunkToolInput, SelectedHunkSummary } from "../../hunk-session/types"; import { buildSidebarEntries, filterReviewFiles, @@ -28,7 +24,7 @@ import { export interface BuildReviewStateOptions { files: DiffFile[]; - liveCommentsByFileId: Record; + liveCommentsByFileId: Record; filterQuery: string; selectedFileId: string; selectedHunkIndex: number; diff --git a/test/pty/ui-integration.test.ts b/test/pty/ui-integration.test.ts index cd769913..2a1f709f 100644 --- a/test/pty/ui-integration.test.ts +++ b/test/pty/ui-integration.test.ts @@ -92,6 +92,31 @@ describe("live UI integration", () => { } }); + test("user notes can be drafted and saved inline in a real PTY", async () => { + const fixture = harness.createLongWrapFilePair(); + const session = await harness.launchHunk({ + args: ["diff", fixture.before, fixture.after, "--mode", "split"], + cols: 120, + rows: 20, + }); + + try { + await session.waitForText(/View\s+Navigate\s+Theme\s+Agent\s+Help/, { + timeout: 15_000, + }); + + await session.press("i"); + await session.waitForText(/Draft note/, { timeout: 5_000 }); + await session.type("Please cover this edge case."); + await session.press("enter"); + + const savedNote = await session.waitForText(/Your note/, { timeout: 5_000 }); + expect(savedNote).toContain("Please cover this edge case."); + } finally { + session.close(); + } + }); + test("real hunk navigation jumps to later hunks in the review stream", async () => { const fixture = harness.createMultiHunkFilePair(); const session = await harness.launchHunk({