diff --git a/CHANGELOG.md b/CHANGELOG.md index b531f9d80..695e2e214 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Review comments, handed to the session as one prompt.** A diff could give + the session a *reference* — a file, a line range — but never an instruction, + so anything you actually wanted changed had to be typed out by hand while + looking straight at it. Right-clicking a selection, in the Review dock or in a + pull request's Files changed, now also offers *Comment on lines*: a box opens + between the lines themselves, and the note is held against them instead of + being written out. Comments collect at the foot of the panel in the order you + read the diff, each removable on its own, + and one *Send* hands the whole batch over as a single prompt — pasted into the + session unsent, so the last word before the agent starts is still yours. They + outlive leaving the tab but not a restart: a line number written before an + edit points somewhere else after it. - **Every worktree gets a dev-server port of its own.** Two worktrees of one project both ran `pnpm dev` and both wanted the same port, so the second one lost — and the setup script that had just installed the dependencies had no diff --git a/frontend/src/components/diff/CommentBatch.tsx b/frontend/src/components/diff/CommentBatch.tsx new file mode 100644 index 000000000..273ea9600 --- /dev/null +++ b/frontend/src/components/diff/CommentBatch.tsx @@ -0,0 +1,88 @@ +import { Send, X } from "lucide-react" +import { useSyncExternalStore } from "react" +import { toast } from "sonner" +import { IconAction } from "@/components/common/IconAction" +import { Button } from "@/components/ui/button" +import { splitPath } from "@/lib/git/lang-badge" +import { + clearReviewComments, + composeReviewComments, + removeReviewComment, + reviewComments, + subscribeReviewComments, +} from "@/lib/review-comments" + +interface CommentBatchProps { + /** What is under review: the checkout's path, or the pull request's URL. */ + target: string + /** Write into the session's terminal; false when no session took it. */ + onInject: (text: string) => boolean +} + +// CommentBatch is the strip at the foot of a review panel: the comments written +// so far, and the one action that hands them all to the session as a single +// prompt. Nothing at all until the first comment is written. +export function CommentBatch({ target, onInject }: CommentBatchProps) { + const comments = useSyncExternalStore(subscribeReviewComments, () => reviewComments(target)) + + if (comments.length === 0) { + return null + } + + // Comments are the one thing here the user typed, so they survive a failed + // send: only a write the session took clears them. + const send = () => { + if (!onInject(composeReviewComments(comments))) { + toast.error("Open a session to send these comments to") + return + } + clearReviewComments(target) + } + + return ( +
+ +
+ + {comments.length} {comments.length === 1 ? "comment" : "comments"} + + + + + +
+
+ ) +} diff --git a/frontend/src/components/diff/FileDiff.tsx b/frontend/src/components/diff/FileDiff.tsx index d727f5098..ca18e1efb 100644 --- a/frontend/src/components/diff/FileDiff.tsx +++ b/frontend/src/components/diff/FileDiff.tsx @@ -32,6 +32,9 @@ const MOUNT_MARGIN = "600px 0px" interface FileDiffProps { file: DiffFile onInject: (text: string) => void + /** Hold a comment on these lines for the panel's batch. Absent = no comment + * action, the shape every surface had before review comments existed. */ + onComment?: (path: string, lines: string, text: string) => void /** Ask the panel to confirm and revert this file's changes. Omitted for a * read-only diff (a PR's changes), where discarding makes no sense. */ onDiscard?: () => void @@ -46,7 +49,15 @@ interface FileDiffProps { // The card must not clip overflow — a clipping ancestor would break the // sticky header. -export function FileDiff({ file, onInject, onDiscard, bulk, viewed, onViewed }: FileDiffProps) { +export function FileDiff({ + file, + onInject, + onComment, + onDiscard, + bulk, + viewed, + onViewed, +}: FileDiffProps) { const doc = useMemo(() => buildFileDoc(file), [file]) const [expanded, setExpanded] = useState(!file.binary && doc.lineMeta.length <= LARGE_FILE_LINES) // The nonce guard skips the initial mount so each file keeps its own @@ -127,7 +138,7 @@ export function FileDiff({ file, onInject, onDiscard, bulk, viewed, onViewed }: (file.binary ? (

Binary file

) : ( - + ))} ) @@ -137,6 +148,7 @@ interface DiffBodyProps { doc: FileDoc path: string onInject: (text: string) => void + onComment?: (path: string, lines: string, text: string) => void } // A panel holds one CodeMirror view per expanded file, and a wide diff (a PR @@ -144,7 +156,7 @@ interface DiffBodyProps { // page. LazyDiffBody defers each one until its card comes near the viewport; // once built, the editor stays, so scrolling back keeps its selection and // highlighting. Collapsing the file still destroys it. -function LazyDiffBody({ doc, path, onInject }: DiffBodyProps) { +function LazyDiffBody({ doc, path, onInject, onComment }: DiffBodyProps) { const placeholder = useRef(null) const [mounted, setMounted] = useState(false) @@ -166,7 +178,7 @@ function LazyDiffBody({ doc, path, onInject }: DiffBodyProps) { }, []) if (mounted) { - return + return } // The placeholder has to stand in for the file's height: zero-height cards // would all sit in the viewport at once and nothing would be deferred. @@ -178,15 +190,21 @@ function LazyDiffBody({ doc, path, onInject }: DiffBodyProps) { // // A diff's document is not the file: the selection lands on doc lines, which // newLineRange maps back to the line numbers the agent needs to be told. -function DiffBody({ doc, path, onInject }: DiffBodyProps) { - const { containerRef, getSelectedDocLines } = useDiffEditor(doc, path) - const [lineRef, setLineRef] = useState(null) +function DiffBody({ doc, path, onInject, onComment }: DiffBodyProps) { + const { containerRef, getSelectedDocLines, openComment } = useDiffEditor( + doc, + path, + onComment && ((lines, text) => onComment(path, lines, text)), + ) + // Both halves of the resolved selection: the file lines an inject or a + // comment is filed against, and the doc line the composer hangs under. + const [selection, setSelection] = useState<{ lines: string; docLine: number } | null>(null) return ( { if (!open) { @@ -194,9 +212,15 @@ function DiffBody({ doc, path, onInject }: DiffBodyProps) { } const selected = getSelectedDocLines() const range = selected ? newLineRange(doc.lineMeta, selected.from, selected.to) : null - setLineRef(range && formatLineRef(range)) + setSelection( + range && selected ? { lines: formatLineRef(range), docLine: selected.to } : null, + ) }} onInject={onInject} + onComment={ + onComment && + (() => selection && openComment({ line: selection.docLine, lines: selection.lines })) + } /> ) } diff --git a/frontend/src/components/diff/InjectMenu.tsx b/frontend/src/components/diff/InjectMenu.tsx index 9433e9d5a..e38455b7c 100644 --- a/frontend/src/components/diff/InjectMenu.tsx +++ b/frontend/src/components/diff/InjectMenu.tsx @@ -16,11 +16,16 @@ interface InjectMenuProps { /** Resolve the selection; fired as the menu opens, not on every change. */ onOpenChange: (open: boolean) => void onInject: (text: string) => void + /** Start a comment on the selected lines. Absent on a surface that is not a + * review — the dock's file preview — which offers the references only. */ + onComment?: (lines: string) => void } // The right-click menu over a code view — a file's diff, or the read-only // preview in the dock — that writes a reference into the session's terminal. -// The whole file, or just the lines under the selection. +// The whole file, or just the lines under the selection. On a review surface it +// also opens a comment on those lines, which is held for the batch rather than +// written out on its own. // // The isolate wrapper keeps CodeMirror's high-z-index gutter from painting over // the sticky header of the card above it. @@ -30,6 +35,7 @@ export function InjectMenu({ lineRef, onOpenChange, onInject, + onComment, }: InjectMenuProps) { return ( @@ -42,6 +48,14 @@ export function InjectMenu({ > {lineRef === null ? "Inject lines" : `Inject lines ${lineRef}`} + {onComment && ( + lineRef && onComment(lineRef)} + > + {lineRef === null ? "Comment on lines…" : `Comment on lines ${lineRef}…`} + + )} ) diff --git a/frontend/src/components/diff/ReviewPanel.tsx b/frontend/src/components/diff/ReviewPanel.tsx index 50af7b81a..1d419eaf9 100644 --- a/frontend/src/components/diff/ReviewPanel.tsx +++ b/frontend/src/components/diff/ReviewPanel.tsx @@ -2,18 +2,21 @@ import { useCallback, useEffect, useState } from "react" import { toast } from "sonner" import { Notice } from "@/components/common/Notice" import { discardTargets, parseDiff, type DiffFile } from "@/lib/git/diff" +import { addReviewComment } from "@/lib/review-comments" import { ProjectService } from "@/lib/rpc" import { useActiveSession } from "@/lib/session/use-active-session" import { useGitStatus } from "@/lib/git/use-git-status" import { useInject } from "@/lib/use-inject" import { errorText } from "@/lib/utils" +import { CommentBatch } from "./CommentBatch" import type { DiffBulk } from "./diff-bulk" import { DiscardDialog } from "./DiscardDialog" import { FileDiff } from "./FileDiff" // ReviewPanel is the Review tab's body: the active session's uncommitted diff, // one collapsible file at a time. Context-menu actions write file/line -// references into the session's PTY, mirroring the footer's attach-file button. +// references into the session's PTY, mirroring the footer's attach-file button, +// or hold a comment on the selection for the batch at the panel's foot. // It follows the active session — a worktree session reviews its checkout, not // the project root. The dock (RightDock) owns the surrounding chrome: width, // full screen, the tab bar and the close button. @@ -65,14 +68,18 @@ export function ReviewPanel({ bulk }: { bulk: DiffBulk }) { } return ( -
- +
+
+ addReviewComment(path, file, lines, text)} + onDiscard={setPendingDiscard} + bulk={bulk} + /> +
+ setPendingDiscard(null)} @@ -86,11 +93,12 @@ interface PanelBodyProps { files: DiffFile[] | null failed: boolean onInject: (text: string) => void + onComment: (path: string, lines: string, text: string) => void onDiscard: (file: DiffFile) => void bulk: DiffBulk } -function PanelBody({ files, failed, onInject, onDiscard, bulk }: PanelBodyProps) { +function PanelBody({ files, failed, onInject, onComment, onDiscard, bulk }: PanelBodyProps) { if (failed) { return Not a git repository } @@ -107,6 +115,7 @@ function PanelBody({ files, failed, onInject, onDiscard, bulk }: PanelBodyProps) key={file.newPath} file={file} onInject={onInject} + onComment={onComment} onDiscard={() => onDiscard(file)} bulk={bulk} /> diff --git a/frontend/src/components/diff/useDiffEditor.ts b/frontend/src/components/diff/useDiffEditor.ts index f9a967d98..b83479298 100644 --- a/frontend/src/components/diff/useDiffEditor.ts +++ b/frontend/src/components/diff/useDiffEditor.ts @@ -1,6 +1,7 @@ -import { useMemo } from "react" +import { useMemo, useRef } from "react" import type { RefObject } from "react" import { buildLineDecorations, diffGutter, type DocLineSelection } from "@/lib/codemirror" +import { commentComposer, setCommentDraft, type CommentDraft } from "@/lib/codemirror-comment" import { useCodeMirrorView } from "@/lib/use-codemirror-view" import type { FileDoc } from "@/lib/git/diff" @@ -8,21 +9,43 @@ export interface DiffEditor { containerRef: RefObject /** 1-based doc line span of the current selection, or null when empty. */ getSelectedDocLines: () => DocLineSelection | null + /** Open the inline comment composer under a draft's line. */ + openComment: (draft: CommentDraft) => void } // useDiffEditor is the diff card's read-only CodeMirror view: the shared editor -// plus the two extensions that make it a diff — the +/- gutter and the per-line -// background. Its selection lands on *doc* lines, which are the diff's own -// interleaving of added and deleted, not the file's numbering; mapping them -// back is newLineRange's job at the call site. -export function useDiffEditor(doc: FileDoc, filename: string): DiffEditor { +// plus the extensions that make it a diff — the +/- gutter, the per-line +// background, and the comment composer that opens between the lines. +// +// Its selection lands on *doc* lines, which are the diff's own interleaving of +// added and deleted, not the file's numbering; mapping them back is +// newLineRange's job at the call site. +export function useDiffEditor( + doc: FileDoc, + filename: string, + onComment?: (lines: string, text: string) => void, +): DiffEditor { + // The extensions are built once per document, so the composer cannot capture + // this render's handler; it reads the current one off the ref instead. + const submit = useRef(onComment) + submit.current = onComment + const source = useMemo( () => ({ text: doc.text, - extensions: [diffGutter(doc.lineMeta), buildLineDecorations(doc.lineMeta)], + extensions: [ + diffGutter(doc.lineMeta), + buildLineDecorations(doc.lineMeta), + commentComposer((lines, text) => submit.current?.(lines, text)), + ], }), [doc], ) - const { containerRef, getSelectedLines } = useCodeMirrorView(source, filename) - return { containerRef, getSelectedDocLines: getSelectedLines } + const { containerRef, getSelectedLines, dispatchEffects } = useCodeMirrorView(source, filename) + + return { + containerRef, + getSelectedDocLines: getSelectedLines, + openComment: (draft) => dispatchEffects([setCommentDraft.of(draft)]), + } } diff --git a/frontend/src/components/pulls/PullRequestView.tsx b/frontend/src/components/pulls/PullRequestView.tsx index e30ab99ce..0c3ec695b 100644 --- a/frontend/src/components/pulls/PullRequestView.tsx +++ b/frontend/src/components/pulls/PullRequestView.tsx @@ -50,7 +50,8 @@ interface PullRequestViewProps { onRefresh: () => void /** The merge landed; the screen decides what follows (a toast, the worktree). */ onMerged: () => void - onInject: (text: string) => void + /** Write into the session's terminal; false when no session took it. */ + onInject: (text: string) => boolean } // PullRequestView is one pull request in full: the header (title, the actions diff --git a/frontend/src/components/pulls/PullsFiles.tsx b/frontend/src/components/pulls/PullsFiles.tsx index a2571d419..5f0714598 100644 --- a/frontend/src/components/pulls/PullsFiles.tsx +++ b/frontend/src/components/pulls/PullsFiles.tsx @@ -2,6 +2,7 @@ import { PanelLeftClose, PanelLeftOpen } from "lucide-react" import { useMemo, useRef, useState, useSyncExternalStore } from "react" import { IconAction } from "@/components/common/IconAction" import { Notice } from "@/components/common/Notice" +import { CommentBatch } from "@/components/diff/CommentBatch" import { CollapseAllAction, useDiffBulk } from "@/components/diff/diff-bulk" import { FileDiff } from "@/components/diff/FileDiff" import { DiffStat } from "@/components/DiffStat" @@ -17,6 +18,7 @@ import { viewedFiles, } from "@/lib/pulls/pull-request-viewed" import { usePullRequestDiff } from "@/lib/pulls/use-pull-request-diff" +import { addReviewComment } from "@/lib/review-comments" import { usePanelVisible } from "@/lib/use-panel-visible" // The file tree is a navigator, not the review itself: hiding it hands the @@ -75,13 +77,15 @@ interface PullsFilesProps { /** Identity of the pull request being reviewed (its URL) — what the Viewed * ticks are keyed by, so two PRs never share them. */ pullRequest: string - onInject: (text: string) => void + /** Write into the session's terminal; false when no session took it. */ + onInject: (text: string) => boolean } // PullsFiles is the "Files changed" tab of the Pulls screen: a changed-files // tree on the left (click to jump) beside the PR's diff, rendered with the same // FileDiff cards as the Review dock — read-only, no discard. Inject still works, -// so a PR file can be referenced into the session's terminal. Each file can be +// so a PR file can be referenced into the session's terminal, and a comment on +// the selection joins the batch this tab sends as one prompt. Each file can be // ticked off as viewed, which folds it away and counts toward the header total. export function PullsFiles({ path, number, head, pullRequest, onInject }: PullsFilesProps) { const { files, error } = usePullRequestDiff(path, head, number) @@ -172,6 +176,9 @@ export function PullsFiles({ path, number, head, pullRequest, onInject }: PullsF + addReviewComment(pullRequest, path, lines, text) + } bulk={bulk} viewed={isViewed(viewed, file.newPath, fingerprints.get(file.newPath) ?? "")} onViewed={(next) => @@ -182,6 +189,7 @@ export function PullsFiles({ path, number, head, pullRequest, onInject }: PullsF ))}
+ ) diff --git a/frontend/src/lib/codemirror-comment.ts b/frontend/src/lib/codemirror-comment.ts new file mode 100644 index 000000000..62512612a --- /dev/null +++ b/frontend/src/lib/codemirror-comment.ts @@ -0,0 +1,120 @@ +// The inline comment composer inside a diff: the box that opens under the lines +// a review comment is about, so the note is written where it is aimed rather +// than in a dialog over the middle of the screen. +// +// A CodeMirror block widget, not a React tree: the editor owns this DOM, and +// mounting a portal into a widget would tie React's lifecycle to the view's. +// The widget is created once per draft (eq keeps it alive across unrelated +// state changes) — recreating it would throw away what was typed. + +import { StateEffect, StateField, type Extension } from "@codemirror/state" +import { Decoration, EditorView, WidgetType, type DecorationSet } from "@codemirror/view" + +export interface CommentDraft { + /** Doc line the composer hangs under — the last line of the selection. */ + line: number + /** The new-file range the comment is filed against: "19" or "12-30". */ + lines: string +} + +/** Open the composer on a draft, or close it with null. */ +export const setCommentDraft = StateEffect.define() + +const ROOT_CLASS = "flex flex-col gap-1 px-2 py-1.5 font-sans" +const LABEL_CLASS = "font-mono text-[0.6875rem] text-muted-foreground" +const FIELD_CLASS = + "min-h-14 w-full resize-y rounded-md border border-input bg-transparent px-2 py-1 text-xs outline-none transition-[color,box-shadow] placeholder:text-muted-foreground focus-visible:border-ring focus-visible:ring-3 focus-visible:ring-ring/50 dark:bg-input/30" +const HINT_CLASS = "text-right text-[0.625rem] text-muted-foreground" + +class ComposerWidget extends WidgetType { + constructor( + readonly draft: CommentDraft, + readonly submit: (lines: string, text: string) => void, + ) { + super() + } + + eq(other: ComposerWidget): boolean { + return other.draft.line === this.draft.line && other.draft.lines === this.draft.lines + } + + toDOM(view: EditorView): HTMLElement { + const root = document.createElement("div") + // The editor's content is contenteditable; the composer must not be, or + // typing in it would run through CodeMirror's own input handling. + root.contentEditable = "false" + root.className = ROOT_CLASS + + const label = document.createElement("div") + label.className = LABEL_CLASS + label.textContent = `comment · ${this.draft.lines}` + + const field = document.createElement("textarea") + field.className = FIELD_CLASS + field.rows = 2 + field.placeholder = "What should change here?" + + const hint = document.createElement("div") + hint.className = HINT_CLASS + hint.textContent = "Esc to cancel · ⏎ to add · ⇧⏎ for a new line" + + const close = () => view.dispatch({ effects: setCommentDraft.of(null) }) + + field.addEventListener("keydown", (event) => { + // Every key stops here: the diff's own shortcuts must not read what is + // being typed into the note. + event.stopPropagation() + if (event.key === "Escape") { + event.preventDefault() + close() + view.focus() + return + } + if (event.key === "Enter" && !event.shiftKey) { + event.preventDefault() + const text = field.value.trim() + if (text !== "") { + this.submit(this.draft.lines, text) + } + close() + } + }) + + root.append(label, field, hint) + // The node is in the document only after CodeMirror inserts it. + requestAnimationFrame(() => field.focus()) + return root + } + + ignoreEvent(): boolean { + return true + } +} + +// commentComposer holds at most one open composer per editor — one file's diff. +// Its submit callback is captured once, so callers hand over a stable function +// that reads the current handler rather than a new closure per render. +export function commentComposer(submit: (lines: string, text: string) => void): Extension { + return StateField.define({ + create: () => Decoration.none, + update(decorations, tr) { + for (const effect of tr.effects) { + if (!effect.is(setCommentDraft)) { + continue + } + if (!effect.value) { + return Decoration.none + } + const line = tr.state.doc.line(Math.min(effect.value.line, tr.state.doc.lines)) + const widget = Decoration.widget({ + widget: new ComposerWidget(effect.value, submit), + block: true, + side: 1, + }) + return Decoration.set([widget.range(line.to)]) + } + return decorations.map(tr.changes) + }, + provide: (field) => EditorView.decorations.from(field), + }) +} diff --git a/frontend/src/lib/review-comments.test.ts b/frontend/src/lib/review-comments.test.ts new file mode 100644 index 000000000..5a232bf38 --- /dev/null +++ b/frontend/src/lib/review-comments.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it, vi } from "vitest" +import { + addReviewComment, + clearReviewComments, + composeReviewComments, + removeReviewComment, + reviewComments, + subscribeReviewComments, +} from "./review-comments" + +// The store is module-level, so every test writes under its own target. +const target = (name: string) => `/tmp/review/${name}` + +describe("review-comments", () => { + it("starts with nothing pending", () => { + expect(reviewComments(target("empty"))).toEqual([]) + }) + + it("keeps comments in the order they were written", () => { + const t = target("order") + addReviewComment(t, "a.ts", "12-30", "leaks the listener") + addReviewComment(t, "b.ts", "4", "rename this") + expect(reviewComments(t).map((c) => c.path)).toEqual(["a.ts", "b.ts"]) + }) + + it("keeps reviews apart", () => { + addReviewComment(target("one"), "a.ts", "1", "note") + expect(reviewComments(target("two"))).toEqual([]) + }) + + it("drops blank text and trims the rest", () => { + const t = target("blank") + addReviewComment(t, "a.ts", "1", " \n ") + expect(reviewComments(t)).toEqual([]) + addReviewComment(t, "a.ts", "1", " note ") + expect(reviewComments(t)[0].text).toBe("note") + }) + + it("removes one comment and clears the rest", () => { + const t = target("remove") + addReviewComment(t, "a.ts", "1", "first") + addReviewComment(t, "b.ts", "2", "second") + removeReviewComment(t, reviewComments(t)[0].id) + expect(reviewComments(t).map((c) => c.text)).toEqual(["second"]) + clearReviewComments(t) + expect(reviewComments(t)).toEqual([]) + }) + + it("notifies subscribers on a change, and never on a no-op", () => { + const t = target("notify") + const listener = vi.fn() + const unsubscribe = subscribeReviewComments(listener) + addReviewComment(t, "a.ts", "1", "note") + expect(listener).toHaveBeenCalledTimes(1) + removeReviewComment(t, "no-such-id") + clearReviewComments(target("never-written")) + expect(listener).toHaveBeenCalledTimes(1) + unsubscribe() + addReviewComment(t, "b.ts", "2", "another") + expect(listener).toHaveBeenCalledTimes(1) + }) + + it("composes one paste holding every comment, anchored like an inject", () => { + const t = target("compose") + addReviewComment(t, "src/a.ts", "12-30", "leaks the listener") + addReviewComment(t, "src/b.tsx", "44", "rename to X") + const payload = composeReviewComments(reviewComments(t)) + expect(payload).toBe( + "\x1b[200~Review comments:\n\n" + + "- src/a.ts:12-30 — leaks the listener\n" + + "- src/b.tsx:44 — rename to X\x1b[201~", + ) + }) + + it("keeps a multi-line comment as one list item", () => { + const t = target("multiline") + addReviewComment(t, "a.ts", "1", "first\nsecond") + expect(composeReviewComments(reviewComments(t))).toContain("- a.ts:1 — first\n second") + }) +}) diff --git a/frontend/src/lib/review-comments.ts b/frontend/src/lib/review-comments.ts new file mode 100644 index 000000000..7d6a6e567 --- /dev/null +++ b/frontend/src/lib/review-comments.ts @@ -0,0 +1,102 @@ +// The line comments written while reading a review, held until they go to the +// session as one prompt. A comment is a place plus what should change there: +// the file, the lines the selection covered, and the note. +// +// Keyed by what is under review — the checkout's path for the working diff, the +// pull request's URL for a PR — so the dock and the Pulls screen never mix their +// notes, and leaving a tab does not drop them. +// +// In memory only, like the Viewed ticks next door: a comment anchors to line +// numbers of the diff as it was rendered, and nothing tracks the file moving +// underneath. Outliving a restart would mean outliving every edit made in +// between, pointing at lines that have since moved. +// +// Module-level store + a subscribe/getSnapshot pair for useSyncExternalStore, +// matching the app's no-state-library pattern. Each snapshot is an array that is +// replaced, never mutated, so subscribers keep a stable reference until +// something actually changes. + +export interface ReviewComment { + /** Identity for the list and for removal; carries no meaning of its own. */ + id: string + path: string + /** The selection's new-file lines, git-style: "19" or "12-30". */ + lines: string + text: string +} + +const byTarget = new Map() +const listeners = new Set<() => void>() + +const empty: readonly ReviewComment[] = [] + +let nextId = 1 + +function notify(): void { + for (const listener of listeners) { + listener() + } +} + +/** One review's pending comments, in the order they were written. */ +export function reviewComments(target: string): readonly ReviewComment[] { + return byTarget.get(target) ?? empty +} + +/** Append a comment. Blank text is not a comment, and is dropped. */ +export function addReviewComment(target: string, path: string, lines: string, text: string): void { + const trimmed = text.trim() + if (trimmed === "") { + return + } + const comment: ReviewComment = { id: String(nextId++), path, lines, text: trimmed } + byTarget.set(target, [...reviewComments(target), comment]) + notify() +} + +export function removeReviewComment(target: string, id: string): void { + const current = reviewComments(target) + const next = current.filter((comment) => comment.id !== id) + if (next.length === current.length) { + return + } + byTarget.set(target, next) + notify() +} + +export function clearReviewComments(target: string): void { + if (!byTarget.delete(target)) { + return + } + notify() +} + +export function subscribeReviewComments(listener: () => void): () => void { + listeners.add(listener) + return () => { + listeners.delete(listener) + } +} + +// Bracketed paste. The block is multi-line, and a newline written straight into +// a TUI's prompt is a send — one per line, so the agent would answer the first +// comment while the rest were still being typed. Wrapping the batch the way +// xterm wraps text pasted by hand lands it in the prompt as a single paste, and +// unsent: the user presses Enter, exactly as they do after an inject. +const PASTE_START = "\x1b[200~" +const PASTE_END = "\x1b[201~" + +/** + * The batch as one terminal write: the comments as a markdown list, each + * anchored the way an inject writes a line reference, wrapped as a paste. + */ +export function composeReviewComments(list: readonly ReviewComment[]): string { + const items = list.map((c) => `- ${c.path}:${c.lines} — ${continuation(c.text)}`) + return `${PASTE_START}Review comments:\n\n${items.join("\n")}${PASTE_END}` +} + +// A comment written across several lines stays one list item: its own newlines +// are indented under the bullet instead of starting a new one. +function continuation(text: string): string { + return text.split("\n").join("\n ") +} diff --git a/frontend/src/lib/use-codemirror-view.ts b/frontend/src/lib/use-codemirror-view.ts index 59aee1587..bead76f12 100644 --- a/frontend/src/lib/use-codemirror-view.ts +++ b/frontend/src/lib/use-codemirror-view.ts @@ -1,6 +1,6 @@ import { useEffect, useRef } from "react" import type { RefObject } from "react" -import type { Extension } from "@codemirror/state" +import type { Extension, StateEffect } from "@codemirror/state" import { EditorState } from "@codemirror/state" import { EditorView } from "@codemirror/view" import { loadLanguage, readOnlyCodeExtensions, selectedDocLines } from "@/lib/codemirror" @@ -20,6 +20,9 @@ export interface CodeMirrorView { containerRef: RefObject /** 1-based doc line span of the current selection, or null when empty. */ getSelectedLines: () => DocLineSelection | null + /** Drive an extension from outside the editor — the seam the inline comment + * composer opens through. A no-op once the view is gone. */ + dispatchEffects: (effects: StateEffect[]) => void } // useCodeMirrorView owns one read-only CodeMirror view for its container: @@ -57,5 +60,9 @@ export function useCodeMirrorView(source: CodeMirrorSource, filename: string): C const getSelectedLines = (): DocLineSelection | null => viewRef.current ? selectedDocLines(viewRef.current) : null - return { containerRef, getSelectedLines } + const dispatchEffects = (effects: StateEffect[]): void => { + viewRef.current?.dispatch({ effects }) + } + + return { containerRef, getSelectedLines, dispatchEffects } } diff --git a/frontend/src/lib/use-inject.ts b/frontend/src/lib/use-inject.ts index 9771aa8bb..1294e140f 100644 --- a/frontend/src/lib/use-inject.ts +++ b/frontend/src/lib/use-inject.ts @@ -4,13 +4,16 @@ import { Terminal as TerminalService } from "./rpc" // useInject writes text straight into a session's PTY — the file and line // references the review panel, the file browser and the pull request screen all // hand to the agent working there. A session-less surface gets a no-op rather -// than a guard at every call site. -export function useInject(sessionId: string): (text: string) => void { +// than a guard at every call site, and the false it answers with is for the +// callers whose text would otherwise be lost (a batch of review comments). +export function useInject(sessionId: string): (text: string) => boolean { return useCallback( (text: string) => { - if (sessionId) { - void TerminalService.Write(sessionId, text) + if (!sessionId) { + return false } + void TerminalService.Write(sessionId, text) + return true }, [sessionId], )