Skip to content

Support hunk-targeted live comments #161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
benvinegar wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Support hunk-targeted live comments #161

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
676e827
Support hunk-targeted live comments
benvinegar Apr 3, 2026
0d92723
Prefer additions when resolving hunk comments
benvinegar Apr 3, 2026
ba1a169
Add bulk live-comment apply for agent reviews (#162)
benvinegar Apr 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ Use it to:
- inspect the current review context
- jump to a file, hunk, or line
- reload the current window with a different `diff` or `show` command
- add, list, and remove inline comments
- add, list, and remove inline comments (by hunk or by line)

Most users only need `hunk session ...`. Use `hunk mcp serve` only for manual startup or debugging of the local daemon.

Expand All @@ -175,6 +175,7 @@ hunk session reload --repo . -- diff
hunk session reload --repo /path/to/worktree -- diff
hunk session reload --session-path /path/to/live-window --source /path/to/other-checkout -- diff
hunk session reload --repo . -- show HEAD~1 -- README.md
hunk session comment add --repo . --file README.md --hunk 2 --summary "Explain this hunk"
hunk session comment add --repo . --file README.md --new-line 103 --summary "Tighten this wording"
hunk session comment list --repo .
hunk session comment rm --repo . <comment-id>
Expand Down
4 changes: 3 additions & 1 deletion skills/hunk-review/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,15 @@ hunk session reload --session-path /path/to/live-window --source /path/to/other-
### Comments

```bash
hunk session comment add --repo . --file README.md --hunk 2 --summary "Explain the hunk" [--rationale "..."] [--author "agent"] [--no-reveal]
hunk session comment add --repo . --file README.md --new-line 103 --summary "Tighten this wording" [--rationale "..."] [--author "agent"] [--no-reveal]
hunk session comment list --repo . [--file README.md]
hunk session comment rm --repo . <comment-id>
hunk session comment clear --repo . --yes [--file README.md]
```

- `comment add` requires `--file`, `--summary`, and exactly one of `--old-line` or `--new-line`
- `comment add` requires `--file`, `--summary`, and exactly one of `--hunk`, `--old-line`, or `--new-line`
- Prefer `--hunk <n>` when you want to annotate the whole diff hunk instead of picking a single line manually
- `comment add` reveals the note by default; pass `--no-reveal` to keep the current focus
- `comment list` and `comment clear` accept optional `--file`
- Quote `--summary` and `--rationale` defensively in the shell
Expand Down
22 changes: 17 additions & 5 deletions src/core/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ async function parseSessionCommand(tokens: string[]): Promise<ParsedCliInput> {
" hunk session navigate (<session-id> | --repo <path>) (--next-comment | --prev-comment)",
" hunk session reload (<session-id> | --repo <path> | --session-path <path>) [--source <path>] -- diff [ref] [-- <pathspec...>]",
" hunk session reload (<session-id> | --repo <path> | --session-path <path>) [--source <path>] -- show [ref] [-- <pathspec...>]",
" hunk session comment add (<session-id> | --repo <path>) --file <path> (--old-line <n> | --new-line <n>) --summary <text>",
" hunk session comment add (<session-id> | --repo <path>) --file <path> (--hunk <n> | --old-line <n> | --new-line <n>) --summary <text>",
" hunk session comment list (<session-id> | --repo <path>)",
" hunk session comment rm (<session-id> | --repo <path>) <comment-id>",
" hunk session comment clear (<session-id> | --repo <path>) --yes",
Expand Down Expand Up @@ -728,7 +728,7 @@ async function parseSessionCommand(tokens: string[]): Promise<ParsedCliInput> {
text:
[
"Usage:",
" hunk session comment add (<session-id> | --repo <path>) --file <path> (--old-line <n> | --new-line <n>) --summary <text>",
" hunk session comment add (<session-id> | --repo <path>) --file <path> (--hunk <n> | --old-line <n> | --new-line <n>) --summary <text>",
" hunk session comment list (<session-id> | --repo <path>) [--file <path>]",
" hunk session comment rm (<session-id> | --repo <path>) <comment-id>",
" hunk session comment clear (<session-id> | --repo <path>) [--file <path>] --yes",
Expand All @@ -743,6 +743,7 @@ async function parseSessionCommand(tokens: string[]): Promise<ParsedCliInput> {
.requiredOption("--file <path>", "diff file path as shown by Hunk")
.requiredOption("--summary <text>", "short review note")
.option("--repo <path>", "target the live session whose repo root matches this path")
.option("--hunk <n>", "1-based hunk number within the file", parsePositiveInt)
.option("--old-line <n>", "1-based line number on the old side", parsePositiveInt)
.option("--new-line <n>", "1-based line number on the new side", parsePositiveInt)
.option("--rationale <text>", "optional longer explanation")
Expand All @@ -756,6 +757,7 @@ async function parseSessionCommand(tokens: string[]): Promise<ParsedCliInput> {
repo?: string;
file: string;
summary: string;
hunk?: number;
oldLine?: number;
newLine?: number;
rationale?: string;
Expand All @@ -774,6 +776,7 @@ async function parseSessionCommand(tokens: string[]): Promise<ParsedCliInput> {
repo?: string;
file: string;
summary: string;
hunk?: number;
oldLine?: number;
newLine?: number;
rationale?: string;
Expand All @@ -794,11 +797,14 @@ async function parseSessionCommand(tokens: string[]): Promise<ParsedCliInput> {
await parseStandaloneCommand(command, commentRest);

const selectors = [
parsedOptions.hunk !== undefined,
parsedOptions.oldLine !== undefined,
parsedOptions.newLine !== undefined,
].filter(Boolean);
if (selectors.length !== 1) {
throw new Error("Specify exactly one comment target: --old-line <n> or --new-line <n>.");
throw new Error(
"Specify exactly one comment target: --hunk <n>, --old-line <n>, or --new-line <n>.",
);
}

return {
Expand All @@ -807,8 +813,14 @@ async function parseSessionCommand(tokens: string[]): Promise<ParsedCliInput> {
output: resolveJsonOutput(parsedOptions),
selector: resolveExplicitSessionSelector(parsedSessionId, parsedOptions.repo),
filePath: parsedOptions.file,
side: parsedOptions.oldLine !== undefined ? "old" : "new",
line: parsedOptions.oldLine ?? parsedOptions.newLine ?? 0,
hunkNumber: parsedOptions.hunk,
side:
parsedOptions.oldLine !== undefined
? "old"
: parsedOptions.newLine !== undefined
? "new"
: undefined,
line: parsedOptions.oldLine ?? parsedOptions.newLine,
summary: parsedOptions.summary,
rationale: parsedOptions.rationale,
author: parsedOptions.author,
Expand Down
74 changes: 73 additions & 1 deletion src/core/liveComments.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@ import type { Hunk } from "@pierre/diffs";
import type { DiffFile } from "./types";
import type { CommentToolInput, DiffSide, LiveComment } from "../mcp/types";

export interface ResolvedCommentTarget {
hunkIndex: number;
side: DiffSide;
line: number;
}

/** Compute the inclusive old/new line spans touched by one hunk. */
export function hunkLineRange(hunk: Hunk) {
const newEnd = Math.max(
Expand Down Expand Up @@ -33,9 +39,75 @@ export function findHunkIndexForLine(file: DiffFile, side: DiffSide, line: numbe
});
}

/** Pick one stable anchor line inside a hunk, preferring new-side changes when present. */
export function firstCommentTargetForHunk(hunk: Hunk): Omit<ResolvedCommentTarget, "hunkIndex"> {
let deletionLineNumber = hunk.deletionStart;
let additionLineNumber = hunk.additionStart;

for (const content of hunk.hunkContent) {
if (content.type === "context") {
deletionLineNumber += content.lines;
additionLineNumber += content.lines;
continue;
}

if (content.additions > 0) {
return {
side: "new",
line: additionLineNumber,
};
}

if (content.deletions > 0) {
return {
side: "old",
line: deletionLineNumber,
};
}
}

const fallbackRange = hunkLineRange(hunk);
return hunk.additionLines > 0
? { side: "new", line: fallbackRange.newRange[0] }
: { side: "old", line: fallbackRange.oldRange[0] };
}
Comment on lines +43 to +81
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 "prefer new" bias not fully enforced across separate chunks

The function's stated goal is to prefer the new (addition) side when present, but that preference only applies when the first non-context chunk already contains additions. If hunkContent has a pure-deletion block first followed by a pure-addition block later, the loop returns "old" at the deletion block and never reaches the addition block.

// example hunkContent sequence that triggers the issue:
// { type: "change", deletions: 2, additions: 0 }   ← loop returns "old" here
// { type: "change", deletions: 0, additions: 3 }   ← never checked

The fallback below the loop correctly inspects hunk.additionLines > 0 to prefer "new", but that fallback is only reachable when all chunks have zero additions and zero deletions — essentially impossible for a real diff hunk, so it will never fire in this situation.

A simple fix is to do a two-pass scan: first sweep for the earliest addition to get the "new" preference, then fall back to the earliest deletion, before reaching the hunkLineRange fallback.

Copy link
Copy Markdown
Member Author

@benvinegar benvinegar Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I changed firstCommentTargetForHunk() to keep scanning after deletion-only chunks, remember the first deletion as a fallback, and still prefer the earliest addition when one appears later in the same hunk. I also added a regression test for that mixed deletion/addition shape.

This comment was generated by Pi using OpenAI o3


/** Resolve a line-based or hunk-based live-comment target against one visible diff file. */
export function resolveCommentTarget(
file: DiffFile,
input: CommentToolInput,
): ResolvedCommentTarget {
if (input.hunkIndex !== undefined) {
const hunk = file.metadata.hunks[input.hunkIndex];
if (!hunk) {
throw new Error(`No diff hunk ${input.hunkIndex + 1} exists in ${input.filePath}.`);
}

return {
hunkIndex: input.hunkIndex,
...firstCommentTargetForHunk(hunk),
};
}

if (!input.side || input.line === undefined) {
throw new Error("comment requires either hunkIndex or both side and line.");
}

const hunkIndex = findHunkIndexForLine(file, input.side, input.line);
if (hunkIndex < 0) {
throw new Error(`No ${input.side} diff hunk in ${input.filePath} covers line ${input.line}.`);
}

return {
hunkIndex,
side: input.side,
line: input.line,
};
}

/** Convert one incoming MCP comment command into a live annotation. */
export function buildLiveComment(
input: CommentToolInput,
input: CommentToolInput & { side: DiffSide; line: number },
commentId: string,
createdAt: string,
hunkIndex: number,
Expand Down
5 changes: 3 additions & 2 deletions src/core/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,9 @@ export interface SessionCommentAddCommandInput {
output: SessionCommandOutput;
selector: SessionSelectorInput;
filePath: string;
side: "old" | "new";
line: number;
hunkNumber?: number;
side?: "old" | "new";
line?: number;
summary: string;
rationale?: string;
author?: string;
Expand Down
11 changes: 10 additions & 1 deletion src/mcp/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,19 @@ async function handleSessionApiRequest(state: HunkDaemonState, request: Request)
}),
};
break;
case "comment-add":
case "comment-add": {
if (
input.hunkNumber === undefined &&
(input.side === undefined || input.line === undefined)
) {
throw new Error("comment-add requires either hunkNumber or both side and line.");
}

response = {
result: await state.sendComment({
...input.selector,
filePath: input.filePath,
hunkIndex: input.hunkNumber !== undefined ? input.hunkNumber - 1 : undefined,
side: input.side,
line: input.line,
summary: input.summary,
Expand All @@ -136,6 +144,7 @@ async function handleSessionApiRequest(state: HunkDaemonState, request: Request)
}),
};
break;
}
case "comment-list":
response = {
comments: state.listComments(input.selector, { filePath: input.filePath }),
Expand Down
5 changes: 3 additions & 2 deletions src/mcp/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,9 @@ export interface HunkSessionSnapshot {

export interface CommentToolInput extends SessionTargetInput {
filePath: string;
side: DiffSide;
line: number;
hunkIndex?: number;
side?: DiffSide;
line?: number;
summary: string;
rationale?: string;
reveal?: boolean;
Expand Down
1 change: 1 addition & 0 deletions src/session/commands.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ class HttpHunkDaemonCliClient implements HunkDaemonCliClient {
action: "comment-add",
selector: input.selector,
filePath: input.filePath,
hunkNumber: input.hunkNumber,
side: input.side,
line: input.line,
summary: input.summary,
Expand Down
5 changes: 3 additions & 2 deletions src/session/protocol.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,9 @@ export type SessionDaemonRequest =
action: "comment-add";
selector: SessionCommentAddCommandInput["selector"];
filePath: string;
side: "old" | "new";
line: number;
hunkNumber?: number;
side?: "old" | "new";
line?: number;
summary: string;
rationale?: string;
author?: string;
Expand Down
24 changes: 12 additions & 12 deletions src/ui/hooks/useHunkSessionBridge.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
findDiffFileByPath,
findHunkIndexForLine,
hunkLineRange,
resolveCommentTarget,
} from "../../core/liveComments";
import { HunkHostClient } from "../../mcp/client";
import { filterReviewFiles, mergeFileAnnotationsByFileId } from "../lib/files";
Expand Down Expand Up @@ -157,19 +158,18 @@ export function useHunkSessionBridge({
throw new Error(`No visible diff file matches ${message.input.filePath}.`);
}

const hunkIndex = findHunkIndexForLine(file, message.input.side, message.input.line);
if (hunkIndex < 0) {
throw new Error(
`No ${message.input.side} diff hunk in ${message.input.filePath} covers line ${message.input.line}.`,
);
}
const target = resolveCommentTarget(file, message.input);

const commentId = `mcp:${message.requestId}`;
const liveComment = buildLiveComment(
message.input,
{
...message.input,
side: target.side,
line: target.line,
},
commentId,
new Date().toISOString(),
hunkIndex,
target.hunkIndex,
);

setLiveCommentsByFileId((current) => ({
Expand All @@ -178,17 +178,17 @@ export function useHunkSessionBridge({
}));

if (message.input.reveal ?? true) {
jumpToFile(file.id, hunkIndex);
jumpToFile(file.id, target.hunkIndex);
openAgentNotes();
}

return {
commentId,
fileId: file.id,
filePath: file.path,
hunkIndex,
side: message.input.side,
line: message.input.line,
hunkIndex: target.hunkIndex,
side: target.side,
line: target.line,
};
},
[files, jumpToFile, openAgentNotes],
Expand Down
54 changes: 54 additions & 0 deletions test/cli.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,39 @@ describe("parseCli", () => {
});
});

test("parses session comment add by hunk number", async () => {
const parsed = await parseCli([
"bun",
"hunk",
"session",
"comment",
"add",
"session-1",
"--file",
"README.md",
"--hunk",
"2",
"--summary",
"Anchor this note to the whole hunk",
"--json",
]);

expect(parsed).toEqual({
kind: "session",
action: "comment-add",
selector: { sessionId: "session-1" },
filePath: "README.md",
hunkNumber: 2,
side: undefined,
line: undefined,
summary: "Anchor this note to the whole hunk",
rationale: undefined,
author: undefined,
reveal: true,
output: "json",
});
});

test("parses session comment list with file filter", async () => {
const parsed = await parseCli([
"bun",
Expand Down Expand Up @@ -491,6 +524,27 @@ describe("parseCli", () => {
).rejects.toThrow("Specify exactly one navigation target");
});

test("rejects session comment add with multiple target selectors", async () => {
await expect(
parseCli([
"bun",
"hunk",
"session",
"comment",
"add",
"session-1",
"--file",
"README.md",
"--hunk",
"2",
"--new-line",
"103",
"--summary",
"Too many targets",
]),
).rejects.toThrow("Specify exactly one comment target");
});

test("rejects session comment clear without confirmation", async () => {
await expect(
parseCli(["bun", "hunk", "session", "comment", "clear", "session-1"]),
Expand Down
Loading
Loading