Skip to content

Commit e467dad

Browse files
HonaHona
authored
fix(app): restore review line comments (anomalyco#33481)
1 parent 985189c commit e467dad

5 files changed

Lines changed: 169 additions & 26 deletions

File tree

‎packages/app/e2e/regression/review-line-comment.spec.ts‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
import { expect, test, type Page } from "@playwright/test"
2+
import { base64Encode } from "@opencode-ai/core/util/encode"
3+
import { mockOpenCodeServer } from "../utils/mock-server"
4+
import { expectAppVisible, expectSessionTitle } from "../utils/waits"
5+
6+
const directory = "C:/OpenCode/ReviewLineCommentRegression"
7+
const sessionID = "ses_review_line_comment_regression"
8+
const title = "Review line comment regression"
9+
10+
test.beforeEach(async ({ page }) => {
11+
await openReview(page)
12+
})
13+
14+
test("opens the comment editor when code is clicked", async ({ page }) => {
15+
const review = page.locator('[data-component="session-review"]')
16+
const line = review.getByText("export const value = 'after'", { exact: true })
17+
await expectAppVisible(line)
18+
await line.click()
19+
20+
await expect(review.getByRole("textbox")).toBeVisible()
21+
})
22+
23+
test("opens the comment editor when a line number is clicked", async ({ page }) => {
24+
const review = page.locator('[data-component="session-review"]')
25+
const lineNumber = review.locator('[data-column-number="1"]').last()
26+
await expectAppVisible(lineNumber)
27+
await lineNumber.click()
28+
29+
await expect(review.getByRole("textbox")).toBeVisible()
30+
})
31+
32+
test("opens the comment editor for a line number range", async ({ page }) => {
33+
const review = page.locator('[data-component="session-review"]')
34+
const start = review.locator('[data-column-number="1"]').last()
35+
const end = review.locator('[data-column-number="3"]').last()
36+
await expectAppVisible(start)
37+
await expectAppVisible(end)
38+
39+
const from = await start.boundingBox()
40+
const to = await end.boundingBox()
41+
if (!from || !to) throw new Error("Missing line number bounds")
42+
await page.mouse.move(from.x + from.width / 2, from.y + from.height / 2)
43+
await page.mouse.down()
44+
await page.mouse.move(to.x + to.width / 2, to.y + to.height / 2)
45+
await page.mouse.up()
46+
47+
await expect(review.getByRole("textbox")).toBeVisible()
48+
})
49+
50+
test("shows a comment button when a line number is hovered", async ({ page }) => {
51+
const review = page.locator('[data-component="session-review"]')
52+
const lineNumber = review.locator('[data-column-number="1"]').last()
53+
await expectAppVisible(lineNumber)
54+
55+
const comment = review.getByRole("button", { name: "Comment", exact: true })
56+
await expect(async () => {
57+
await page.mouse.move(0, 0)
58+
await lineNumber.hover()
59+
await expect(comment).toBeVisible({ timeout: 500 })
60+
}).toPass()
61+
await comment.click()
62+
await expect(review.getByRole("textbox")).toBeVisible()
63+
})
64+
65+
test("stages a submitted line comment in the prompt context", async ({ page }) => {
66+
const requests: string[] = []
67+
page.on("request", (request) => {
68+
if (request.method() !== "GET") requests.push(`${request.method()} ${new URL(request.url()).pathname}`)
69+
})
70+
71+
const review = page.locator('[data-component="session-review"]')
72+
await review.getByText("export const value = 'after'", { exact: true }).click()
73+
await review.getByRole("textbox").fill("Use the existing value instead")
74+
await review.locator('[data-slot="line-comment-action"][data-variant="primary"]').click()
75+
76+
await expect(review.getByText("Use the existing value instead", { exact: true })).toBeVisible()
77+
const context = page.getByText("Use the existing value instead", { exact: true }).last()
78+
await expect(context).toBeVisible()
79+
await expect(context.locator("..")).toContainText("review.ts:2")
80+
expect(requests).toEqual([])
81+
})
82+
83+
async function openReview(page: Page) {
84+
await page.setViewportSize({ width: 700, height: 900 })
85+
await mockOpenCodeServer(page, {
86+
directory,
87+
project: {
88+
id: "proj_review_line_comment_regression",
89+
worktree: directory,
90+
vcs: "git",
91+
name: "review-line-comment-regression",
92+
time: { created: 1700000000000, updated: 1700000000000 },
93+
sandboxes: [],
94+
},
95+
provider: { all: [], connected: [], default: {} },
96+
sessions: [
97+
{
98+
id: sessionID,
99+
slug: "review-line-comment-regression",
100+
projectID: "proj_review_line_comment_regression",
101+
directory,
102+
title,
103+
version: "dev",
104+
time: { created: 1700000000000, updated: 1700000000000 },
105+
},
106+
],
107+
vcsDiff: [
108+
{
109+
file: "src/review.ts",
110+
additions: 1,
111+
deletions: 1,
112+
status: "modified",
113+
patch:
114+
"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",
115+
},
116+
],
117+
pageMessages: () => ({
118+
items: [
119+
{
120+
info: {
121+
id: "msg_review_line_comment_regression",
122+
sessionID,
123+
role: "user",
124+
time: { created: 1700000000000 },
125+
summary: { diffs: [] },
126+
agent: "build",
127+
model: { providerID: "opencode", modelID: "test" },
128+
},
129+
parts: [
130+
{
131+
id: "prt_review_line_comment_regression",
132+
sessionID,
133+
messageID: "msg_review_line_comment_regression",
134+
type: "text",
135+
text: "Review this change.",
136+
},
137+
],
138+
},
139+
],
140+
}),
141+
})
142+
143+
await page.goto(`/${base64Encode(directory)}/session/${sessionID}`)
144+
await expectSessionTitle(page, title)
145+
const diffResponse = page.waitForResponse((response) => new URL(response.url()).pathname === "/vcs/diff")
146+
await page.getByRole("tab", { name: "Changes" }).click()
147+
expect(await (await diffResponse).json()).toHaveLength(1)
148+
149+
const review = page.locator('[data-component="session-review"]')
150+
await expectAppVisible(review)
151+
await review
152+
.getByRole("heading", { name: /review\.ts/ })
153+
.getByRole("button")
154+
.first()
155+
.click()
156+
}

‎packages/app/e2e/utils/mock-server.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ export interface MockServerConfig {
1818
project: unknown
1919
sessions: ({ id: string } & Record<string, unknown>)[]
2020
pageMessages: (sessionId: string, limit: number, before?: string) => { items: unknown[]; cursor?: string }
21+
vcsDiff?: unknown[]
2122
messageDelay?: number
2223
onMessages?: (input: { sessionID: string; before?: string; phase: "start" | "end" }) => void
2324
events?: () => unknown[]
@@ -52,6 +53,7 @@ export async function mockOpenCodeServer(page: Page, config: MockServerConfig) {
5253
const path = url.pathname
5354
if (path === "/global/event" || path === "/event") return sse(route, config.events?.(), config.eventRetry)
5455
if (path === "/global/health") return json(route, { healthy: true })
56+
if (path === "/vcs/diff" && config.vcsDiff) return json(route, config.vcsDiff)
5557
if (emptyObject.has(path)) return json(route, {})
5658
if (emptyList.has(path)) return json(route, [])
5759
if (path in staticRoutes) return json(route, staticRoutes[path])

‎packages/app/src/pages/session/file-tabs.tsx‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -405,19 +405,18 @@ export function FileTabContent(props: { tab: string }) {
405405
cacheKey: cacheKey(),
406406
}}
407407
enableLineSelection
408-
enableHoverUtility
408+
enableGutterUtility
409409
selectedLines={activeSelection()}
410410
commentedLines={commentedLines()}
411411
onRendered={() => {
412412
scrollSync.queueRestore()
413413
}}
414414
annotations={commentsUi.annotations()}
415415
renderAnnotation={commentsUi.renderAnnotation}
416-
renderHoverUtility={commentsUi.renderHoverUtility}
416+
renderGutterUtility={commentsUi.renderGutterUtility}
417417
onLineSelected={(range: SelectedLineRange | null) => {
418418
commentsUi.onLineSelected(range)
419419
}}
420-
onLineNumberSelectionEnd={commentsUi.onLineNumberSelectionEnd}
421420
onLineSelectionEnd={(range: SelectedLineRange | null) => {
422421
commentsUi.onLineSelectionEnd(range)
423422
}}

‎packages/ui/src/components/line-comment-annotations.tsx‎

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -288,12 +288,6 @@ export function createLineCommentState<T>(props: LineCommentStateProps<T>) {
288288
setSelected(next)
289289
}
290290

291-
const finishSelection = (range: SelectedLineRange) => {
292-
closeComment()
293-
setSelected(range)
294-
cancelDraft()
295-
}
296-
297291
return {
298292
draft,
299293
setDraft,
@@ -310,7 +304,6 @@ export function createLineCommentState<T>(props: LineCommentStateProps<T>) {
310304
openEditor,
311305
hoverComment,
312306
cancelDraft,
313-
finishSelection,
314307
select: setSelected,
315308
reset,
316309
}
@@ -322,21 +315,19 @@ export function createLineCommentController<T extends LineCommentShape>(
322315
note: ReturnType<typeof createLineCommentState<string>>
323316
annotations: Accessor<DiffLineAnnotation<LineCommentAnnotationMeta<T>>[]>
324317
renderAnnotation: ReturnType<typeof createManagedLineCommentAnnotationRenderer<T>>["renderAnnotation"]
325-
renderHoverUtility: ReturnType<typeof createLineCommentHoverRenderer>
318+
renderGutterUtility: ReturnType<typeof createLineCommentGutterRenderer>
326319
onLineSelected: (range: SelectedLineRange | null) => void
327320
onLineSelectionEnd: (range: SelectedLineRange | null) => void
328-
onLineNumberSelectionEnd: (range: SelectedLineRange | null) => void
329321
}
330322
export function createLineCommentController<T extends LineCommentShape>(
331323
props: LineCommentControllerProps<T>,
332324
): {
333325
note: ReturnType<typeof createLineCommentState<string>>
334326
annotations: Accessor<LineCommentAnnotation<T>[]>
335327
renderAnnotation: ReturnType<typeof createManagedLineCommentAnnotationRenderer<T>>["renderAnnotation"]
336-
renderHoverUtility: ReturnType<typeof createLineCommentHoverRenderer>
328+
renderGutterUtility: ReturnType<typeof createLineCommentGutterRenderer>
337329
onLineSelected: (range: SelectedLineRange | null) => void
338330
onLineSelectionEnd: (range: SelectedLineRange | null) => void
339-
onLineNumberSelectionEnd: (range: SelectedLineRange | null) => void
340331
}
341332
export function createLineCommentController<T extends LineCommentShape>(
342333
props: LineCommentControllerProps<T> | LineCommentControllerWithSideProps<T>,
@@ -426,7 +417,7 @@ export function createLineCommentController<T extends LineCommentShape>(
426417
}),
427418
})
428419

429-
const renderHoverUtility = createLineCommentHoverRenderer({
420+
const renderGutterUtility = createLineCommentGutterRenderer({
430421
label: props.label,
431422
getSelectedRange: () => {
432423
if (note.opened()) return null
@@ -452,22 +443,16 @@ export function createLineCommentController<T extends LineCommentShape>(
452443
return
453444
}
454445

455-
note.finishSelection(range)
456-
}
457-
458-
const onLineNumberSelectionEnd = (range: SelectedLineRange | null) => {
459-
if (!range) return
460446
note.openDraft(range)
461447
}
462448

463449
return {
464450
note,
465451
annotations,
466452
renderAnnotation,
467-
renderHoverUtility,
453+
renderGutterUtility,
468454
onLineSelected,
469455
onLineSelectionEnd,
470-
onLineNumberSelectionEnd,
471456
}
472457
}
473458

@@ -569,7 +554,7 @@ export function createManagedLineCommentAnnotationRenderer<T>(props: {
569554
}
570555
}
571556

572-
export function createLineCommentHoverRenderer(props: {
557+
export function createLineCommentGutterRenderer(props: {
573558
label: string
574559
getSelectedRange: Accessor<SelectedLineRange | null>
575560
onOpenDraft: (range: SelectedLineRange) => void

‎packages/ui/src/components/session-review.tsx‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -620,13 +620,14 @@ export const SessionReview = (props: SessionReviewProps) => {
620620
props.onDiffRendered?.()
621621
}}
622622
enableLineSelection={props.onLineComment != null}
623-
enableHoverUtility={props.onLineComment != null}
623+
enableGutterUtility={props.onLineComment != null}
624624
onLineSelected={handleLineSelected}
625625
onLineSelectionEnd={handleLineSelectionEnd}
626-
onLineNumberSelectionEnd={commentsUi.onLineNumberSelectionEnd}
627626
annotations={commentsUi.annotations()}
628627
renderAnnotation={commentsUi.renderAnnotation}
629-
renderHoverUtility={props.onLineComment ? commentsUi.renderHoverUtility : undefined}
628+
renderGutterUtility={
629+
props.onLineComment ? commentsUi.renderGutterUtility : undefined
630+
}
630631
selectedLines={selectedLines()}
631632
commentedLines={commentedLines()}
632633
media={{

0 commit comments

Comments
 (0)