diff --git a/packages/app/e2e/regression/review-line-comment.spec.ts b/packages/app/e2e/regression/review-line-comment.spec.ts new file mode 100644 index 000000000000..eac90c7eb130 --- /dev/null +++ b/packages/app/e2e/regression/review-line-comment.spec.ts @@ -0,0 +1,156 @@ +import { expect, test, type Page } from "@playwright/test" +import { base64Encode } from "@opencode-ai/core/util/encode" +import { mockOpenCodeServer } from "../utils/mock-server" +import { expectAppVisible, expectSessionTitle } from "../utils/waits" + +const directory = "C:/OpenCode/ReviewLineCommentRegression" +const sessionID = "ses_review_line_comment_regression" +const title = "Review line comment regression" + +test.beforeEach(async ({ page }) => { + await openReview(page) +}) + +test("opens the comment editor when code is clicked", async ({ page }) => { + const review = page.locator('[data-component="session-review"]') + const line = review.getByText("export const value = 'after'", { exact: true }) + await expectAppVisible(line) + await line.click() + + await expect(review.getByRole("textbox")).toBeVisible() +}) + +test("opens the comment editor when a line number is clicked", async ({ page }) => { + const review = page.locator('[data-component="session-review"]') + const lineNumber = review.locator('[data-column-number="1"]').last() + await expectAppVisible(lineNumber) + await lineNumber.click() + + await expect(review.getByRole("textbox")).toBeVisible() +}) + +test("opens the comment editor for a line number range", async ({ page }) => { + const review = page.locator('[data-component="session-review"]') + const start = review.locator('[data-column-number="1"]').last() + const end = review.locator('[data-column-number="3"]').last() + await expectAppVisible(start) + await expectAppVisible(end) + + const from = await start.boundingBox() + const to = await end.boundingBox() + if (!from || !to) throw new Error("Missing line number bounds") + await page.mouse.move(from.x + from.width / 2, from.y + from.height / 2) + await page.mouse.down() + await page.mouse.move(to.x + to.width / 2, to.y + to.height / 2) + await page.mouse.up() + + await expect(review.getByRole("textbox")).toBeVisible() +}) + +test("shows a comment button when a line number is hovered", async ({ page }) => { + const review = page.locator('[data-component="session-review"]') + const lineNumber = review.locator('[data-column-number="1"]').last() + await expectAppVisible(lineNumber) + + const comment = review.getByRole("button", { name: "Comment", exact: true }) + await expect(async () => { + await page.mouse.move(0, 0) + await lineNumber.hover() + await expect(comment).toBeVisible({ timeout: 500 }) + }).toPass() + await comment.click() + await expect(review.getByRole("textbox")).toBeVisible() +}) + +test("stages a submitted line comment in the prompt context", async ({ page }) => { + const requests: string[] = [] + page.on("request", (request) => { + if (request.method() !== "GET") requests.push(`${request.method()} ${new URL(request.url()).pathname}`) + }) + + const review = page.locator('[data-component="session-review"]') + await review.getByText("export const value = 'after'", { exact: true }).click() + await review.getByRole("textbox").fill("Use the existing value instead") + await review.locator('[data-slot="line-comment-action"][data-variant="primary"]').click() + + await expect(review.getByText("Use the existing value instead", { exact: true })).toBeVisible() + const context = page.getByText("Use the existing value instead", { exact: true }).last() + await expect(context).toBeVisible() + await expect(context.locator("..")).toContainText("review.ts:2") + expect(requests).toEqual([]) +}) + +async function openReview(page: Page) { + await page.setViewportSize({ width: 700, height: 900 }) + await mockOpenCodeServer(page, { + directory, + project: { + id: "proj_review_line_comment_regression", + worktree: directory, + vcs: "git", + name: "review-line-comment-regression", + time: { created: 1700000000000, updated: 1700000000000 }, + sandboxes: [], + }, + provider: { all: [], connected: [], default: {} }, + sessions: [ + { + id: sessionID, + slug: "review-line-comment-regression", + projectID: "proj_review_line_comment_regression", + directory, + title, + version: "dev", + time: { created: 1700000000000, updated: 1700000000000 }, + }, + ], + vcsDiff: [ + { + file: "src/review.ts", + additions: 1, + deletions: 1, + status: "modified", + patch: + "diff --git a/src/review.ts b/src/review.ts\n--- a/src/review.ts\n+++ b/src/review.ts\n@@ -1,3 +1,3 @@\n export const first = 1\n-export const value = 'before'\n+export const value = 'after'\n export const last = 3\n", + }, + ], + pageMessages: () => ({ + items: [ + { + info: { + id: "msg_review_line_comment_regression", + sessionID, + role: "user", + time: { created: 1700000000000 }, + summary: { diffs: [] }, + agent: "build", + model: { providerID: "opencode", modelID: "test" }, + }, + parts: [ + { + id: "prt_review_line_comment_regression", + sessionID, + messageID: "msg_review_line_comment_regression", + type: "text", + text: "Review this change.", + }, + ], + }, + ], + }), + }) + + await page.goto(`/${base64Encode(directory)}/session/${sessionID}`) + await expectSessionTitle(page, title) + const diffResponse = page.waitForResponse((response) => new URL(response.url()).pathname === "/vcs/diff") + await page.getByRole("tab", { name: "Changes" }).click() + expect(await (await diffResponse).json()).toHaveLength(1) + + const review = page.locator('[data-component="session-review"]') + await expectAppVisible(review) + await review + .getByRole("heading", { name: /review\.ts/ }) + .getByRole("button") + .first() + .click() +} diff --git a/packages/app/e2e/utils/mock-server.ts b/packages/app/e2e/utils/mock-server.ts index cf0c95243148..248459db962f 100644 --- a/packages/app/e2e/utils/mock-server.ts +++ b/packages/app/e2e/utils/mock-server.ts @@ -18,6 +18,7 @@ export interface MockServerConfig { project: unknown sessions: ({ id: string } & Record)[] pageMessages: (sessionId: string, limit: number, before?: string) => { items: unknown[]; cursor?: string } + vcsDiff?: unknown[] messageDelay?: number onMessages?: (input: { sessionID: string; before?: string; phase: "start" | "end" }) => void events?: () => unknown[] @@ -52,6 +53,7 @@ export async function mockOpenCodeServer(page: Page, config: MockServerConfig) { const path = url.pathname if (path === "/global/event" || path === "/event") return sse(route, config.events?.(), config.eventRetry) if (path === "/global/health") return json(route, { healthy: true }) + if (path === "/vcs/diff" && config.vcsDiff) return json(route, config.vcsDiff) if (emptyObject.has(path)) return json(route, {}) if (emptyList.has(path)) return json(route, []) if (path in staticRoutes) return json(route, staticRoutes[path]) diff --git a/packages/app/src/pages/session/file-tabs.tsx b/packages/app/src/pages/session/file-tabs.tsx index 364760eab0bc..06d6637b3766 100644 --- a/packages/app/src/pages/session/file-tabs.tsx +++ b/packages/app/src/pages/session/file-tabs.tsx @@ -405,7 +405,7 @@ export function FileTabContent(props: { tab: string }) { cacheKey: cacheKey(), }} enableLineSelection - enableHoverUtility + enableGutterUtility selectedLines={activeSelection()} commentedLines={commentedLines()} onRendered={() => { @@ -413,11 +413,10 @@ export function FileTabContent(props: { tab: string }) { }} annotations={commentsUi.annotations()} renderAnnotation={commentsUi.renderAnnotation} - renderHoverUtility={commentsUi.renderHoverUtility} + renderGutterUtility={commentsUi.renderGutterUtility} onLineSelected={(range: SelectedLineRange | null) => { commentsUi.onLineSelected(range) }} - onLineNumberSelectionEnd={commentsUi.onLineNumberSelectionEnd} onLineSelectionEnd={(range: SelectedLineRange | null) => { commentsUi.onLineSelectionEnd(range) }} diff --git a/packages/ui/src/components/line-comment-annotations.tsx b/packages/ui/src/components/line-comment-annotations.tsx index f0286a36a926..c8fe70c758de 100644 --- a/packages/ui/src/components/line-comment-annotations.tsx +++ b/packages/ui/src/components/line-comment-annotations.tsx @@ -288,12 +288,6 @@ export function createLineCommentState(props: LineCommentStateProps) { setSelected(next) } - const finishSelection = (range: SelectedLineRange) => { - closeComment() - setSelected(range) - cancelDraft() - } - return { draft, setDraft, @@ -310,7 +304,6 @@ export function createLineCommentState(props: LineCommentStateProps) { openEditor, hoverComment, cancelDraft, - finishSelection, select: setSelected, reset, } @@ -322,10 +315,9 @@ export function createLineCommentController( note: ReturnType> annotations: Accessor>[]> renderAnnotation: ReturnType>["renderAnnotation"] - renderHoverUtility: ReturnType + renderGutterUtility: ReturnType onLineSelected: (range: SelectedLineRange | null) => void onLineSelectionEnd: (range: SelectedLineRange | null) => void - onLineNumberSelectionEnd: (range: SelectedLineRange | null) => void } export function createLineCommentController( props: LineCommentControllerProps, @@ -333,10 +325,9 @@ export function createLineCommentController( note: ReturnType> annotations: Accessor[]> renderAnnotation: ReturnType>["renderAnnotation"] - renderHoverUtility: ReturnType + renderGutterUtility: ReturnType onLineSelected: (range: SelectedLineRange | null) => void onLineSelectionEnd: (range: SelectedLineRange | null) => void - onLineNumberSelectionEnd: (range: SelectedLineRange | null) => void } export function createLineCommentController( props: LineCommentControllerProps | LineCommentControllerWithSideProps, @@ -426,7 +417,7 @@ export function createLineCommentController( }), }) - const renderHoverUtility = createLineCommentHoverRenderer({ + const renderGutterUtility = createLineCommentGutterRenderer({ label: props.label, getSelectedRange: () => { if (note.opened()) return null @@ -452,11 +443,6 @@ export function createLineCommentController( return } - note.finishSelection(range) - } - - const onLineNumberSelectionEnd = (range: SelectedLineRange | null) => { - if (!range) return note.openDraft(range) } @@ -464,10 +450,9 @@ export function createLineCommentController( note, annotations, renderAnnotation, - renderHoverUtility, + renderGutterUtility, onLineSelected, onLineSelectionEnd, - onLineNumberSelectionEnd, } } @@ -569,7 +554,7 @@ export function createManagedLineCommentAnnotationRenderer(props: { } } -export function createLineCommentHoverRenderer(props: { +export function createLineCommentGutterRenderer(props: { label: string getSelectedRange: Accessor onOpenDraft: (range: SelectedLineRange) => void diff --git a/packages/ui/src/components/session-review.tsx b/packages/ui/src/components/session-review.tsx index ebdd5d939caf..fdfbff04f233 100644 --- a/packages/ui/src/components/session-review.tsx +++ b/packages/ui/src/components/session-review.tsx @@ -620,13 +620,14 @@ export const SessionReview = (props: SessionReviewProps) => { props.onDiffRendered?.() }} enableLineSelection={props.onLineComment != null} - enableHoverUtility={props.onLineComment != null} + enableGutterUtility={props.onLineComment != null} onLineSelected={handleLineSelected} onLineSelectionEnd={handleLineSelectionEnd} - onLineNumberSelectionEnd={commentsUi.onLineNumberSelectionEnd} annotations={commentsUi.annotations()} renderAnnotation={commentsUi.renderAnnotation} - renderHoverUtility={props.onLineComment ? commentsUi.renderHoverUtility : undefined} + renderGutterUtility={ + props.onLineComment ? commentsUi.renderGutterUtility : undefined + } selectedLines={selectedLines()} commentedLines={commentedLines()} media={{