diff --git a/.changeset/agent-attention-marks.md b/.changeset/agent-attention-marks.md new file mode 100644 index 000000000..c8cfbb40e --- /dev/null +++ b/.changeset/agent-attention-marks.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": minor +--- + +Agents can now light up exact character ranges in a live review with `hunk session highlight add` / `clear` (five contrast-guaranteed tones, painted through the same pipeline as extension line highlights), and `hunk session navigate` line targets now land the viewport on the exact line instead of just its hunk. diff --git a/docs/extension-architecture.md b/docs/extension-architecture.md index 1c35a7169..5f6d28bcc 100644 --- a/docs/extension-architecture.md +++ b/docs/extension-architecture.md @@ -127,6 +127,18 @@ keeps highlights out of `buildDiffSectionRowPlan`, its caches, and every geometry measurement: a highlight change is a repaint, never a re-plan. The static pager never runs extension code, so highlights are interactive-only. +Agent attention marks (`hunk session highlight add` / `clear`) join this same +pipeline rather than growing a second one: `useReviewController.ts` validates +each daemon-pushed mark with the same `validate.ts` contract and caps, holds +them per file, and `src/ui/highlights/merge.ts` appends them after extension +marks in the one map `DiffPane` paints from — so agent marks share paint, +contrast, and geometry guarantees, and win where ranges overlap. Unlike +extension marks, nothing re-derives agent marks after a reload, so +`src/ui/highlights/reconcile.ts` carries them across a document replacement only +for files whose `contentIdentity` is unchanged — those still show the same +characters — and drops the rest. Line-target `session navigate` reuses the same +`revealLine` landing policy `ctx.navigation.revealLine` gets. + `src/ui/fileViews/mode.ts` owns file-view mode activation, validity, and callback containment. The presentation controller stores the active mode and funnels all exit paths through one teardown, including re-entrant handoffs. diff --git a/skills/hunk-review/SKILL.md b/skills/hunk-review/SKILL.md index fa7ecb0f2..01c7472d5 100644 --- a/skills/hunk-review/SKILL.md +++ b/skills/hunk-review/SKILL.md @@ -1,6 +1,6 @@ --- name: hunk-review -description: Interacts with live Hunk diff review sessions via CLI. Inspects review focus, navigates files and hunks, reloads session contents, and adds inline review comments. Use when the user has a Hunk session running or wants to review diffs interactively. +description: Interacts with live Hunk diff review sessions via CLI. Inspects review focus, navigates files, hunks, and exact lines, reloads session contents, adds inline review comments, and paints attention marks on character ranges. Use when the user has a Hunk session running or wants to review diffs interactively. --- # Hunk Review @@ -21,6 +21,7 @@ If no session exists, ask the user to launch Hunk in their terminal first. 7. hunk session reload -- # swap contents if needed 8. hunk session comment add ... # leave one review note 9. hunk session comment apply ... # apply many agent notes in one stdin batch +10. hunk session highlight add ... # light up the exact range you are explaining ``` ## Session selection @@ -78,6 +79,7 @@ hunk session navigate --repo . --prev-comment - `--hunk ` is 1-based - `--new-line` / `--old-line` are 1-based line numbers on that diff side +- A line target lands the user's viewport on that exact line (falling back to its hunk when the line is inside a collapsed region); `--hunk` lands on the hunk - Use either `--next-comment` or `--prev-comment`, not both ### Reload @@ -132,6 +134,30 @@ printf '%s\n' '{"comments":[{"filePath":"README.md","newLine":103,"summary":"Tig - `comment list` and `comment clear` accept optional `--file` - Quote `--summary` and `--rationale` defensively in the shell +### Attention marks + +Highlights paint character ranges inside the diff lines the user is looking at — use them to light up the exact expression you are explaining while you narrate. + +```bash +hunk session highlight add ( | --repo ) --file (--old-line | --new-line ) --start --end [--tone ] [--focus] [--json] +hunk session highlight clear ( | --repo ) [--file ] [--json] +``` + +Examples: + +```bash +hunk session highlight add --repo . --file src/App.tsx --new-line 42 --start 6 --end 19 +hunk session highlight add --repo . --file src/App.tsx --new-line 42 --start 6 --end 19 --tone warning --focus +hunk session highlight clear --repo . +``` + +- `highlight add` requires `--file`, exactly one of `--old-line` or `--new-line`, and the `--start` / `--end` offsets +- `--start` is a 0-based inclusive offset into the line's text and `--end` is exclusive, counted in UTF-16 code units — the same `[start, end)` range extensions use +- Tones: `match` (default), `info`, `warning`, `error`; `current` renders as reverse video and is best reserved for the one range under discussion +- Pass `--focus` to also land the viewport on the marked line +- Marks survive scrolling, navigation, and reloads that leave the marked file's content unchanged; a reload that changes that file drops its marks, and `highlight clear` removes them explicitly (optionally per `--file`) +- Marks are visual only — pair them with a `comment add` when the explanation should persist as a note + ### Experimental rich markup notes (STML) Only use STML when `hunk session context --json` lists `stml` in `experimentalFeatures`. The user opts into that experience by launching the review with `--experimental`; do not ask a normal session to render markup. @@ -166,6 +192,7 @@ Guidelines: - Work in the order that tells the clearest story, not necessarily file order - Navigate before commenting so the user sees the code you're discussing +- Use `highlight add --focus` to steer the user's eyes to the exact expression while you explain it, and `highlight clear` before moving to the next topic - Use `comment apply` for agent-generated batches and `comment add` for one-off notes - Use `--focus` sparingly when the note itself should actively steer the review - Keep comments focused: intent, structure, risks, or follow-ups @@ -181,5 +208,7 @@ Guidelines: - **"Pass --stdin to read batch comments from stdin JSON."** -- `comment apply` only reads its batch payload from stdin. - **"Specify exactly one navigation target"** -- pick one of `--hunk`, `--old-line`, or `--new-line`. - **"Specify exactly one comment target"** -- pass `comment add` one of `--old-line` or `--new-line`. +- **"Specify exactly one highlight target"** -- pass `highlight add` one of `--old-line` or `--new-line`. +- **"Highlight --end must be greater than --start"** -- offsets are `[start, end)` UTF-16 code units into the line text; end is exclusive. - **"Specify either --next-comment or --prev-comment, not both."** -- choose one comment-navigation direction. - **"Could not read the raw diff for ..."** -- the session reloaded or closed while `--include-patch` was reading it. Re-run `review`; drop `--include-patch` if you only need file and hunk structure. diff --git a/src/core/cli.test.ts b/src/core/cli.test.ts index 39a70af36..5b7ad5ca2 100644 --- a/src/core/cli.test.ts +++ b/src/core/cli.test.ts @@ -948,6 +948,138 @@ describe("parseCli", () => { }); }); + test("parses session highlight add with defaults", async () => { + const parsed = await parseCli([ + "bun", + "hunk", + "session", + "highlight", + "add", + "session-1", + "--file", + "src/App.tsx", + "--new-line", + "42", + "--start", + "0", + "--end", + "13", + ]); + + expect(parsed).toEqual({ + kind: "session", + action: "highlight-add", + selector: { sessionId: "session-1" }, + filePath: "src/App.tsx", + side: "new", + line: 42, + start: 0, + end: 13, + reveal: false, + output: "text", + }); + }); + + test("parses session highlight add with tone, old side, and focus", async () => { + const parsed = await parseCli([ + "bun", + "hunk", + "session", + "highlight", + "add", + "--repo", + "/tmp/repo", + "--file", + "src/App.tsx", + "--old-line", + "7", + "--start", + "6", + "--end", + "19", + "--tone", + "warning", + "--focus", + "--json", + ]); + + expect(parsed).toEqual({ + kind: "session", + action: "highlight-add", + selector: { repoRoot: resolve("/tmp/repo") }, + filePath: "src/App.tsx", + side: "old", + line: 7, + start: 6, + end: 19, + tone: "warning", + reveal: true, + output: "json", + }); + }); + + test("rejects session highlight add with an empty range, bad tone, or missing target", async () => { + const base = [ + "bun", + "hunk", + "session", + "highlight", + "add", + "session-1", + "--file", + "src/App.tsx", + ]; + + await expect( + parseCli([...base, "--new-line", "42", "--start", "5", "--end", "5"]), + ).rejects.toThrow("Highlight --end must be greater than --start"); + await expect( + parseCli([...base, "--new-line", "42", "--start", "0", "--end", "4", "--tone", "loud"]), + ).rejects.toThrow("Highlight tone must be one of match, current, info, warning, error."); + await expect(parseCli([...base, "--start", "0", "--end", "4"])).rejects.toThrow( + "Specify exactly one highlight target: --old-line or --new-line .", + ); + await expect( + parseCli([...base, "--new-line", "42", "--start", "-1", "--end", "4"]), + ).rejects.toThrow(); + }); + + test("parses session highlight clear globally and per file", async () => { + expect(await parseCli(["bun", "hunk", "session", "highlight", "clear", "session-1"])).toEqual({ + kind: "session", + action: "highlight-clear", + selector: { sessionId: "session-1" }, + output: "text", + }); + + expect( + await parseCli([ + "bun", + "hunk", + "session", + "highlight", + "clear", + "--repo", + "/tmp/repo", + "--file", + "src/App.tsx", + "--json", + ]), + ).toEqual({ + kind: "session", + action: "highlight-clear", + selector: { repoRoot: resolve("/tmp/repo") }, + filePath: "src/App.tsx", + output: "json", + }); + }); + + test("rejects unknown session highlight subcommands", async () => { + await expect(parseCli(["bun", "hunk", "session", "highlight", "paint"])).rejects.toThrow( + "Supported highlight subcommands are add and clear.", + ); + }); + test("rejects session commands without an explicit target", async () => { await expect(parseCli(["bun", "hunk", "session", "get"])).rejects.toThrow( "Specify one live Hunk session with or --repo .", diff --git a/src/core/cli.ts b/src/core/cli.ts index e4ccca782..c166eceff 100644 --- a/src/core/cli.ts +++ b/src/core/cli.ts @@ -26,15 +26,20 @@ import { type SessionCommandOptions, COMMENT_DIRECTION_CONSTRAINT, COMMENT_TARGET_CONSTRAINT, + HIGHLIGHT_TARGET_CONSTRAINT, + HIGHLIGHT_TONES, + isHighlightTone, NAVIGATE_TARGET_CONSTRAINT, optionKeyFromFlag, SESSION_AGENT_COMMANDS, SESSION_AGENT_COMMAND_LIST, SESSION_COMMENT_COMMAND_LIST, + SESSION_HIGHLIGHT_COMMAND_LIST, } from "../session/agent/surface"; import { COMMENT_APPLY_STDIN_MESSAGE, constraintViolationMessage, + HIGHLIGHT_RANGE_MESSAGE, RELOAD_SEPARATOR_MESSAGE, } from "../session/agent/errors"; import { DEFAULT_TAB_WIDTH, parseTabWidth } from "./tabWidth"; @@ -263,6 +268,20 @@ function parsePositiveInt(value: string) { return parsed; } +/** Parse one required non-negative integer CLI value, accepting 0. */ +function parseNonNegativeInt(value: string) { + if (!/^(0|[1-9]\d*)$/.test(value)) { + throw new Error(`Invalid non-negative integer: ${value}`); + } + + const parsed = Number(value); + if (!Number.isSafeInteger(parsed)) { + throw new Error(`Invalid non-negative integer: ${value}`); + } + + return parsed; +} + /** Read one paired positive/negative boolean flag directly from raw argv. */ function resolveBooleanFlag(argv: string[], enabledFlag: string, disabledFlag: string) { let resolved: boolean | undefined; @@ -880,6 +899,8 @@ function buildSessionCommand(spec: AgentCommandSpec) { : command.option.bind(command); if (option.parse === "positiveInt") { register(option.flag, option.description, parsePositiveInt); + } else if (option.parse === "nonNegativeInt") { + register(option.flag, option.description, parseNonNegativeInt); } else { register(option.flag, option.description); } @@ -1294,6 +1315,93 @@ async function parseSessionCommand(tokens: string[]): Promise { throw new Error("Supported comment subcommands are add, apply, list, rm, and clear."); } + if (subcommand === "highlight") { + const [highlightSubcommand, ...highlightRest] = rest; + if (!highlightSubcommand || highlightSubcommand === "--help" || highlightSubcommand === "-h") { + return { + kind: "help", + text: ["Usage:", ...sessionUsageLines(SESSION_HIGHLIGHT_COMMAND_LIST)].join("\n") + "\n", + }; + } + + if (highlightSubcommand === "add") { + const command = buildSessionCommand(SESSION_AGENT_COMMANDS["highlight-add"]); + + let parsedSessionId: string | undefined; + let parsedOptions: SessionCommandOptions<"highlight-add"> = { + file: "", + start: 0, + end: 0, + }; + + command.action( + (sessionId: string | undefined, options: SessionCommandOptions<"highlight-add">) => { + parsedSessionId = sessionId; + parsedOptions = options; + }, + ); + + if (highlightRest.includes("--help") || highlightRest.includes("-h")) { + return sessionCommandHelpText(command, SESSION_AGENT_COMMANDS["highlight-add"]); + } + + await parseStandaloneCommand(command, highlightRest); + + enforceConstraint(HIGHLIGHT_TARGET_CONSTRAINT, parsedOptions); + if (parsedOptions.end <= parsedOptions.start) { + throw new Error(HIGHLIGHT_RANGE_MESSAGE); + } + const tone = parsedOptions.tone; + if (tone !== undefined && !isHighlightTone(tone)) { + throw new Error(`Highlight tone must be one of ${HIGHLIGHT_TONES.join(", ")}.`); + } + + return { + kind: "session", + action: "highlight-add", + output: resolveJsonOutput(parsedOptions), + selector: resolveExplicitSessionSelector(parsedSessionId, parsedOptions.repo), + filePath: parsedOptions.file, + side: parsedOptions.oldLine !== undefined ? "old" : "new", + line: parsedOptions.oldLine ?? parsedOptions.newLine ?? 0, + start: parsedOptions.start, + end: parsedOptions.end, + ...(tone !== undefined && isHighlightTone(tone) ? { tone } : {}), + reveal: parsedOptions.focus ?? false, + }; + } + + if (highlightSubcommand === "clear") { + const command = buildSessionCommand(SESSION_AGENT_COMMANDS["highlight-clear"]); + + let parsedSessionId: string | undefined; + let parsedOptions: SessionCommandOptions<"highlight-clear"> = {}; + + command.action( + (sessionId: string | undefined, options: SessionCommandOptions<"highlight-clear">) => { + parsedSessionId = sessionId; + parsedOptions = options; + }, + ); + + if (highlightRest.includes("--help") || highlightRest.includes("-h")) { + return sessionCommandHelpText(command, SESSION_AGENT_COMMANDS["highlight-clear"]); + } + + await parseStandaloneCommand(command, highlightRest); + + return { + kind: "session", + action: "highlight-clear", + output: resolveJsonOutput(parsedOptions), + selector: resolveExplicitSessionSelector(parsedSessionId, parsedOptions.repo), + filePath: parsedOptions.file, + }; + } + + throw new Error("Supported highlight subcommands are add and clear."); + } + throw new Error(`Unknown session command: ${subcommand}`); } diff --git a/src/core/types.ts b/src/core/types.ts index f19510db4..af8c16054 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -278,6 +278,30 @@ export interface SessionCommentClearCommandInput { confirmed: boolean; } +export interface SessionHighlightAddCommandInput { + kind: "session"; + action: "highlight-add"; + output: SessionCommandOutput; + selector: SessionSelectorInput; + filePath: string; + side: "old" | "new"; + line: number; + /** 0-based inclusive UTF-16 code-unit offset into the line's raw text. */ + start: number; + /** Exclusive end offset; must exceed `start`. */ + end: number; + tone?: "match" | "current" | "info" | "warning" | "error"; + reveal: boolean; +} + +export interface SessionHighlightClearCommandInput { + kind: "session"; + action: "highlight-clear"; + output: SessionCommandOutput; + selector: SessionSelectorInput; + filePath?: string; +} + export type SessionCommandInput = | SessionListCommandInput | SessionGetCommandInput @@ -288,7 +312,9 @@ export type SessionCommandInput = | SessionCommentApplyCommandInput | SessionCommentListCommandInput | SessionCommentRemoveCommandInput - | SessionCommentClearCommandInput; + | SessionCommentClearCommandInput + | SessionHighlightAddCommandInput + | SessionHighlightClearCommandInput; /** * Review requests extend the published input views rather than restating them, diff --git a/src/hunk-review/skillDocument.ts b/src/hunk-review/skillDocument.ts index 424b1a835..fdfa9013a 100644 --- a/src/hunk-review/skillDocument.ts +++ b/src/hunk-review/skillDocument.ts @@ -32,7 +32,7 @@ function navigateExamples(kind: "absolute" | "relative") { const FRONTMATTER = [ "---", "name: hunk-review", - "description: Interacts with live Hunk diff review sessions via CLI. Inspects review focus, navigates files and hunks, reloads session contents, and adds inline review comments. Use when the user has a Hunk session running or wants to review diffs interactively.", + "description: Interacts with live Hunk diff review sessions via CLI. Inspects review focus, navigates files, hunks, and exact lines, reloads session contents, adds inline review comments, and paints attention marks on character ranges. Use when the user has a Hunk session running or wants to review diffs interactively.", "---", ]; @@ -57,6 +57,7 @@ const WORKFLOW = [ "7. hunk session reload -- # swap contents if needed", "8. hunk session comment add ... # leave one review note", "9. hunk session comment apply ... # apply many agent notes in one stdin batch", + "10. hunk session highlight add ... # light up the exact range you are explaining", "```", ]; @@ -103,6 +104,7 @@ const NAVIGATE_SECTION = [ "", "- `--hunk ` is 1-based", "- `--new-line` / `--old-line` are 1-based line numbers on that diff side", + "- A line target lands the user's viewport on that exact line (falling back to its hunk when the line is inside a collapsed region); `--hunk` lands on the hunk", "- Use either `--next-comment` or `--prev-comment`, not both", ]; @@ -154,6 +156,28 @@ const COMMENTS_SECTION = [ "- Quote `--summary` and `--rationale` defensively in the shell", ]; +const HIGHLIGHTS_SECTION = [ + "### Attention marks", + "", + "Highlights paint character ranges inside the diff lines the user is looking at — use them to light up the exact expression you are explaining while you narrate.", + "", + ...bashFence(synopsisLines(commands["highlight-add"], commands["highlight-clear"])), + "", + "Examples:", + "", + ...bashFence([ + ...(commands["highlight-add"].examples ?? []), + ...(commands["highlight-clear"].examples ?? []), + ]), + "", + "- `highlight add` requires `--file`, exactly one of `--old-line` or `--new-line`, and the `--start` / `--end` offsets", + "- `--start` is a 0-based inclusive offset into the line's text and `--end` is exclusive, counted in UTF-16 code units — the same `[start, end)` range extensions use", + "- Tones: `match` (default), `info`, `warning`, `error`; `current` renders as reverse video and is best reserved for the one range under discussion", + "- Pass `--focus` to also land the viewport on the marked line", + "- Marks survive scrolling, navigation, and reloads that leave the marked file's content unchanged; a reload that changes that file drops its marks, and `highlight clear` removes them explicitly (optionally per `--file`)", + "- Marks are visual only — pair them with a `comment add` when the explanation should persist as a note", +]; + const STML_SECTION = [ "### Experimental rich markup notes (STML)", "", @@ -191,6 +215,7 @@ const GUIDING_SECTION = [ "", "- Work in the order that tells the clearest story, not necessarily file order", "- Navigate before commenting so the user sees the code you're discussing", + "- Use `highlight add --focus` to steer the user's eyes to the exact expression while you explain it, and `highlight clear` before moving to the next topic", "- Use `comment apply` for agent-generated batches and `comment add` for one-off notes", "- Use `--focus` sparingly when the note itself should actively steer the review", "- Keep comments focused: intent, structure, risks, or follow-ups", @@ -217,6 +242,7 @@ export function renderHunkReviewSkill() { NAVIGATE_SECTION, RELOAD_SECTION, COMMENTS_SECTION, + HIGHLIGHTS_SECTION, STML_SECTION, NEW_FILES_SECTION, GUIDING_SECTION, diff --git a/src/session/agent/cliClient.test.ts b/src/session/agent/cliClient.test.ts index 40351fc09..f3f026dbe 100644 --- a/src/session/agent/cliClient.test.ts +++ b/src/session/agent/cliClient.test.ts @@ -18,10 +18,12 @@ import { import { createHttpHunkSessionCliClient, formatClearCommentsOutput, + formatClearHighlightsOutput, formatCommentApplyOutput, formatCommentListOutput, formatCommentOutput, formatContextOutput, + formatHighlightOutput, formatListOutput, formatNavigationOutput, formatReloadOutput, @@ -91,6 +93,27 @@ describe("HTTP Hunk session CLI client", () => { filePath: "src/app.ts", }, }, + "highlight-add": { + result: { + fileId: "file-1", + filePath: "src/app.ts", + hunkIndex: 0, + side: "new" as const, + line: 12, + start: 2, + end: 9, + tone: "warning" as const, + fileMarkCount: 1, + revealed: "line" as const, + }, + }, + "highlight-clear": { + result: { + removedCount: 2, + remainingCount: 0, + filePath: "src/app.ts", + }, + }, }; globalThis.fetch = (async (input, init) => { @@ -202,6 +225,30 @@ describe("HTTP Hunk session CLI client", () => { output: "json", }), ).toMatchObject({ removedCount: 1 }); + expect( + await client.addHighlight({ + kind: "session", + action: "highlight-add", + selector, + filePath: "src/app.ts", + side: "new", + line: 12, + start: 2, + end: 9, + tone: "warning", + reveal: true, + output: "json", + }), + ).toMatchObject({ fileMarkCount: 1, revealed: "line" }); + expect( + await client.clearHighlights({ + kind: "session", + action: "highlight-clear", + selector, + filePath: "src/app.ts", + output: "json", + }), + ).toMatchObject({ removedCount: 2 }); expect(requests).toEqual([ { action: "list" }, @@ -243,6 +290,18 @@ describe("HTTP Hunk session CLI client", () => { { action: "comment-list", selector, filePath: "src/app.ts" }, { action: "comment-rm", selector, commentId: "comment-1" }, { action: "comment-clear", selector, filePath: "src/app.ts" }, + { + action: "highlight-add", + selector, + filePath: "src/app.ts", + side: "new", + line: 12, + start: 2, + end: 9, + tone: "warning", + reveal: true, + }, + { action: "highlight-clear", selector, filePath: "src/app.ts" }, ]); }); @@ -576,4 +635,72 @@ describe("Hunk session CLI formatters", () => { }), ).toBe("Cleared 5 live comments from session session-1. Remaining comments: 0.\n"); }); + + test("highlight formatters describe marks, reveals, and line-exact navigation", () => { + expect( + formatNavigationOutput(selector, { + fileId: "file-1", + filePath: "src/app.ts", + hunkIndex: 1, + revealed: "line", + side: "new", + line: 42, + }), + ).toBe("Revealed src/app.ts:42 (new) in hunk 2 of session session-1.\n"); + // A hunk fallback reads as the classic focus message, not a false line claim. + expect( + formatNavigationOutput(selector, { + fileId: "file-1", + filePath: "src/app.ts", + hunkIndex: 1, + revealed: "hunk", + side: "new", + line: 42, + }), + ).toBe("Focused src/app.ts hunk 2 in session session-1.\n"); + + expect( + formatHighlightOutput(selector, { + fileId: "file-1", + filePath: "src/app.ts", + hunkIndex: 0, + side: "new", + line: 12, + start: 2, + end: 9, + tone: "warning", + fileMarkCount: 3, + revealed: "line", + }), + ).toBe( + "Marked src/app.ts:12 (new) [2, 9) as warning in session session-1 and revealed its line. File marks: 3.\n", + ); + expect( + formatHighlightOutput(selector, { + fileId: "file-1", + filePath: "src/app.ts", + hunkIndex: 0, + side: "old", + line: 7, + start: 0, + end: 4, + tone: "match", + fileMarkCount: 1, + }), + ).toBe("Marked src/app.ts:7 (old) [0, 4) as match in session session-1. File marks: 1.\n"); + + expect( + formatClearHighlightsOutput(selector, { + removedCount: 2, + remainingCount: 1, + filePath: "src/app.ts", + }), + ).toBe("Cleared 2 attention marks from src/app.ts in session session-1. Remaining marks: 1.\n"); + expect( + formatClearHighlightsOutput(selector, { + removedCount: 4, + remainingCount: 0, + }), + ).toBe("Cleared 4 attention marks from session session-1. Remaining marks: 0.\n"); + }); }); diff --git a/src/session/agent/cliClient.ts b/src/session/agent/cliClient.ts index 7f1748b63..998971b8a 100644 --- a/src/session/agent/cliClient.ts +++ b/src/session/agent/cliClient.ts @@ -14,7 +14,9 @@ import { import type { AppliedCommentBatchResult, AppliedCommentResult, + AppliedHighlightResult, ClearedCommentsResult, + ClearedHighlightsResult, ListedSession, NavigatedSelectionResult, ReloadedSessionResult, @@ -30,6 +32,8 @@ import type { SessionCommentClearCommandInput, SessionCommentListCommandInput, SessionCommentRemoveCommandInput, + SessionHighlightAddCommandInput, + SessionHighlightClearCommandInput, SessionNavigateCommandInput, SessionReloadCommandInput, SessionReviewCommandInput, @@ -52,6 +56,8 @@ export interface HunkSessionCliClient { ): Promise>; removeComment(input: SessionCommentRemoveCommandInput): Promise; clearComments(input: SessionCommentClearCommandInput): Promise; + addHighlight(input: SessionHighlightAddCommandInput): Promise; + clearHighlights(input: SessionHighlightClearCommandInput): Promise; } async function extractResponseError(response: Response) { @@ -210,6 +216,32 @@ class HttpHunkSessionCliClient implements HunkSessionCliClient { }) ).result; } + + async addHighlight(input: SessionHighlightAddCommandInput) { + return ( + await this.request<{ result: AppliedHighlightResult }>({ + action: "highlight-add", + selector: input.selector, + filePath: input.filePath, + side: input.side, + line: input.line, + start: input.start, + end: input.end, + tone: input.tone, + reveal: input.reveal, + }) + ).result; + } + + async clearHighlights(input: SessionHighlightClearCommandInput) { + return ( + await this.request<{ result: ClearedHighlightsResult }>({ + action: "highlight-clear", + selector: input.selector, + filePath: input.filePath, + }) + ).result; + } } /** Create the concrete Hunk session CLI client that speaks to the broker-backed HTTP API. */ @@ -426,6 +458,10 @@ export function formatNavigationOutput( selector: SessionSelectorInput, result: NavigatedSelectionResult, ) { + if (result.revealed === "line" && result.line !== undefined) { + return `Revealed ${formatSessionPath(result.filePath)}:${result.line} (${result.side}) in hunk ${result.hunkIndex + 1} of ${formatSessionSelector(selector)}.\n`; + } + return `Focused ${formatSessionPath(result.filePath)} hunk ${result.hunkIndex + 1} in ${formatSessionSelector(selector)}.\n`; } @@ -518,6 +554,45 @@ export function formatNoteListOutput( .join("\n\n")}\n`; } +/** + * Report one applied attention mark, including whether the review moved to it. + * + * The running mark count is part of the answer because marks accumulate per + * file: an agent that keeps marking needs to see its own total without asking. + */ +export function formatHighlightOutput( + selector: SessionSelectorInput, + result: AppliedHighlightResult, +) { + const reveal = + result.revealed === "line" + ? " and revealed its line" + : result.revealed === "hunk" + ? " and revealed its hunk" + : ""; + return ( + `Marked ${formatSessionPath(result.filePath)}:${result.line} (${result.side}) ` + + `[${result.start}, ${result.end}) as ${result.tone} in ${formatSessionSelector(selector)}${reveal}. ` + + `File marks: ${result.fileMarkCount}.\n` + ); +} + +/** + * Report how many attention marks were cleared, and from what scope. + * + * Clearing is addressed either to one file or to the whole session, so the + * scope is named back to the caller rather than assumed. + */ +export function formatClearHighlightsOutput( + selector: SessionSelectorInput, + result: ClearedHighlightsResult, +) { + const scope = result.filePath + ? `${formatSessionPath(result.filePath)} in ${formatSessionSelector(selector)}` + : formatSessionSelector(selector); + return `Cleared ${result.removedCount} attention marks from ${scope}. Remaining marks: ${result.remainingCount}.\n`; +} + export function formatClearCommentsOutput( selector: SessionSelectorInput, result: ClearedCommentsResult, diff --git a/src/session/agent/commands.test.ts b/src/session/agent/commands.test.ts index 3618f6810..44b6c7297 100644 --- a/src/session/agent/commands.test.ts +++ b/src/session/agent/commands.test.ts @@ -71,6 +71,8 @@ function createClient(overrides: Partial): HunkDaemonCliCli "comment-list", "comment-rm", "comment-clear", + "highlight-add", + "highlight-clear", ], }), listSessions: async () => [], @@ -121,6 +123,21 @@ function createClient(overrides: Partial): HunkDaemonCliCli removedCount: 0, remainingCommentCount: 0, }), + addHighlight: async () => ({ + fileId: "file-1", + filePath: "README.md", + hunkIndex: 0, + side: "new", + line: 1, + start: 0, + end: 4, + tone: "match", + fileMarkCount: 1, + }), + clearHighlights: async () => ({ + removedCount: 0, + remainingCount: 0, + }), ...overrides, }; } @@ -1049,6 +1066,76 @@ describe("session command compatibility checks", () => { expect(calls).toEqual(["navigate", "comment-list", "comment-rm", "comment-clear"]); }); + + test("routes highlight actions through the daemon and formats text output", async () => { + const selector: SessionSelectorInput = { sessionId: "session-1" }; + const calls: string[] = []; + + setSessionCommandTestHooks({ + createClient: () => + createClient({ + addHighlight: async (input) => { + calls.push("highlight-add"); + expect(input.selector).toEqual(selector); + expect(input.filePath).toBe("README.md"); + expect(input.side).toBe("new"); + expect(input.line).toBe(2); + expect(input.start).toBe(4); + expect(input.end).toBe(11); + expect(input.tone).toBe("info"); + expect(input.reveal).toBe(true); + return { + fileId: "file-1", + filePath: "README.md", + hunkIndex: 0, + side: "new", + line: 2, + start: 4, + end: 11, + tone: "info", + fileMarkCount: 1, + revealed: "line", + }; + }, + clearHighlights: async (input) => { + calls.push("highlight-clear"); + expect(input.selector).toEqual(selector); + expect(input.filePath).toBeUndefined(); + return { removedCount: 1, remainingCount: 0 }; + }, + }), + resolveDaemonAvailability: async () => true, + }); + + expect( + await runSessionCommand({ + kind: "session", + action: "highlight-add", + selector, + filePath: "README.md", + side: "new", + line: 2, + start: 4, + end: 11, + tone: "info", + reveal: true, + output: "text", + } satisfies SessionCommandInput), + ).toBe( + "Marked README.md:2 (new) [4, 11) as info in session session-1 and revealed its line. File marks: 1.\n", + ); + + expect( + await runSessionCommand({ + kind: "session", + action: "highlight-clear", + selector, + output: "text", + } satisfies SessionCommandInput), + ).toBe("Cleared 1 attention marks from session session-1. Remaining marks: 0.\n"); + + expect(calls).toEqual(["highlight-add", "highlight-clear"]); + }); }); describe("session list includes terminal metadata", () => { diff --git a/src/session/agent/commands.ts b/src/session/agent/commands.ts index 9a994f07b..6e8011e59 100644 --- a/src/session/agent/commands.ts +++ b/src/session/agent/commands.ts @@ -17,10 +17,12 @@ import { matchesSessionSelector, normalizeSessionSelector } from "@hunk/session- import { createHttpHunkSessionCliClient, formatClearCommentsOutput, + formatClearHighlightsOutput, formatCommentApplyOutput, formatCommentListOutput, formatCommentOutput, formatContextOutput, + formatHighlightOutput, formatListOutput, formatNavigationOutput, formatNoteListOutput, @@ -46,6 +48,8 @@ const REQUIRED_ACTION_BY_COMMAND: Record + formatHighlightOutput(input.selector, result), + ); + } + case "highlight-clear": { + const result = await client.clearHighlights({ + ...input, + selector: normalizedSelector!, + }); + return renderOutput(input.output, { result }, () => + formatClearHighlightsOutput(input.selector, result), + ); + } } } diff --git a/src/session/agent/errors.test.ts b/src/session/agent/errors.test.ts index 53989f5ae..4f1d8e32a 100644 --- a/src/session/agent/errors.test.ts +++ b/src/session/agent/errors.test.ts @@ -5,6 +5,7 @@ import { agentErrorQuotePrefix, COMMENT_APPLY_STDIN_MESSAGE, constraintViolationMessage, + HIGHLIGHT_RANGE_MESSAGE, NO_ACTIVE_SESSIONS_MESSAGE, noDiffFileMatchesMessage, RELOAD_SEPARATOR_MESSAGE, @@ -13,6 +14,7 @@ import { import { COMMENT_DIRECTION_CONSTRAINT, COMMENT_TARGET_CONSTRAINT, + HIGHLIGHT_TARGET_CONSTRAINT, NAVIGATE_TARGET_CONSTRAINT, } from "./surface"; @@ -66,6 +68,8 @@ describe("agent error messages", () => { COMMENT_APPLY_STDIN_MESSAGE, constraintViolationMessage(NAVIGATE_TARGET_CONSTRAINT), constraintViolationMessage(COMMENT_TARGET_CONSTRAINT), + constraintViolationMessage(HIGHLIGHT_TARGET_CONSTRAINT), + HIGHLIGHT_RANGE_MESSAGE, constraintViolationMessage(COMMENT_DIRECTION_CONSTRAINT), reviewResourceUnavailableMessage("src/App.tsx"), ]; diff --git a/src/session/agent/errors.ts b/src/session/agent/errors.ts index b41b37901..dcba9dad4 100644 --- a/src/session/agent/errors.ts +++ b/src/session/agent/errors.ts @@ -33,6 +33,10 @@ export const RELOAD_SEPARATOR_MESSAGE = /** `comment apply` invoked without opting into the stdin JSON batch. */ export const COMMENT_APPLY_STDIN_MESSAGE = "Pass --stdin to read batch comments from stdin JSON."; +/** `highlight add` invoked with an empty or inverted character range. */ +export const HIGHLIGHT_RANGE_MESSAGE = + "Highlight --end must be greater than --start; the range is [start, end) with an exclusive end."; + /** The daemon is reachable but no live Hunk session has registered with it. */ export const NO_ACTIVE_SESSIONS_MESSAGE = "No active Hunk sessions are registered with the daemon. Open Hunk and wait for it to connect."; @@ -104,6 +108,14 @@ export const AGENT_ERROR_DOCS: AgentErrorDoc[] = [ quote: "Specify exactly one comment target", remedy: "pass `comment add` one of `--old-line` or `--new-line`.", }, + { + quote: "Specify exactly one highlight target", + remedy: "pass `highlight add` one of `--old-line` or `--new-line`.", + }, + { + quote: "Highlight --end must be greater than --start", + remedy: "offsets are `[start, end)` UTF-16 code units into the line text; end is exclusive.", + }, { quote: "Specify either --next-comment or --prev-comment, not both.", remedy: "choose one comment-navigation direction.", diff --git a/src/session/agent/surface.test.ts b/src/session/agent/surface.test.ts index 0aef17f67..23c5b1534 100644 --- a/src/session/agent/surface.test.ts +++ b/src/session/agent/surface.test.ts @@ -1,11 +1,13 @@ import { describe, expect, test } from "bun:test"; import { agentOptionFlagName, + isHighlightTone, optionKeyFromFlag, type AgentCommandOption, SESSION_AGENT_COMMAND_LIST, SESSION_AGENT_COMMANDS, SESSION_COMMENT_COMMAND_LIST, + SESSION_HIGHLIGHT_COMMAND_LIST, type SessionCommandOptions, } from "./surface"; @@ -22,6 +24,10 @@ describe("session agent command surface", () => { "session comment rm", "session comment clear", ]); + expect(SESSION_HIGHLIGHT_COMMAND_LIST.map((spec) => spec.name)).toEqual([ + "session highlight add", + "session highlight clear", + ]); }); test("declares each option flag once per command", () => { @@ -104,4 +110,21 @@ describe("session agent command surface", () => { } } }); + + test("parses highlight offsets as 0-based start and positive exclusive end", () => { + const options: readonly AgentCommandOption[] = SESSION_AGENT_COMMANDS["highlight-add"].options; + const parseByFlag = new Map( + options.map((option) => [agentOptionFlagName(option), option.parse]), + ); + // `--start` accepts 0 because offsets are 0-based; `--end` is exclusive so it starts at 1. + expect(parseByFlag.get("--start")).toBe("nonNegativeInt"); + expect(parseByFlag.get("--end")).toBe("positiveInt"); + }); + + test("recognizes exactly the five shared highlight tones", () => { + for (const tone of ["match", "current", "info", "warning", "error"]) { + expect(isHighlightTone(tone)).toBe(true); + } + expect(isHighlightTone("loud")).toBe(false); + }); }); diff --git a/src/session/agent/surface.ts b/src/session/agent/surface.ts index 261fdbf09..2ea475543 100644 --- a/src/session/agent/surface.ts +++ b/src/session/agent/surface.ts @@ -15,8 +15,8 @@ export interface AgentCommandOption { readonly flag: string; /** Description shared by `--help` output and generated docs. */ readonly description: string; - /** Parse the option value as a 1-based positive integer. */ - readonly parse?: "positiveInt"; + /** Parse the option value as a 1-based positive or 0-based non-negative integer. */ + readonly parse?: "positiveInt" | "nonNegativeInt"; /** Register with Commander as a required option. */ readonly required?: boolean; } @@ -60,7 +60,7 @@ type FlagBody = Flag extends `--${infer Name} ${string}` /** The parsed value Commander produces for one option: boolean flag, string value, or number. */ type OptionValue