fix(app): restore review line comments (#33481)
This commit is contained in:
Luke Parker
GitHub
parent
597f47b486
commit
d111812389
5 changed files with 169 additions and 26 deletions
156
packages/app/e2e/regression/review-line-comment.spec.ts
Normal file
156
packages/app/e2e/regression/review-line-comment.spec.ts
Normal file
|
|
@ -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()
|
||||
}
|
||||
|
|
@ -18,6 +18,7 @@ export interface MockServerConfig {
|
|||
project: unknown
|
||||
sessions: ({ id: string } & Record<string, unknown>)[]
|
||||
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])
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}}
|
||||
|
|
|
|||
|
|
@ -288,12 +288,6 @@ export function createLineCommentState<T>(props: LineCommentStateProps<T>) {
|
|||
setSelected(next)
|
||||
}
|
||||
|
||||
const finishSelection = (range: SelectedLineRange) => {
|
||||
closeComment()
|
||||
setSelected(range)
|
||||
cancelDraft()
|
||||
}
|
||||
|
||||
return {
|
||||
draft,
|
||||
setDraft,
|
||||
|
|
@ -310,7 +304,6 @@ export function createLineCommentState<T>(props: LineCommentStateProps<T>) {
|
|||
openEditor,
|
||||
hoverComment,
|
||||
cancelDraft,
|
||||
finishSelection,
|
||||
select: setSelected,
|
||||
reset,
|
||||
}
|
||||
|
|
@ -322,10 +315,9 @@ export function createLineCommentController<T extends LineCommentShape>(
|
|||
note: ReturnType<typeof createLineCommentState<string>>
|
||||
annotations: Accessor<DiffLineAnnotation<LineCommentAnnotationMeta<T>>[]>
|
||||
renderAnnotation: ReturnType<typeof createManagedLineCommentAnnotationRenderer<T>>["renderAnnotation"]
|
||||
renderHoverUtility: ReturnType<typeof createLineCommentHoverRenderer>
|
||||
renderGutterUtility: ReturnType<typeof createLineCommentGutterRenderer>
|
||||
onLineSelected: (range: SelectedLineRange | null) => void
|
||||
onLineSelectionEnd: (range: SelectedLineRange | null) => void
|
||||
onLineNumberSelectionEnd: (range: SelectedLineRange | null) => void
|
||||
}
|
||||
export function createLineCommentController<T extends LineCommentShape>(
|
||||
props: LineCommentControllerProps<T>,
|
||||
|
|
@ -333,10 +325,9 @@ export function createLineCommentController<T extends LineCommentShape>(
|
|||
note: ReturnType<typeof createLineCommentState<string>>
|
||||
annotations: Accessor<LineCommentAnnotation<T>[]>
|
||||
renderAnnotation: ReturnType<typeof createManagedLineCommentAnnotationRenderer<T>>["renderAnnotation"]
|
||||
renderHoverUtility: ReturnType<typeof createLineCommentHoverRenderer>
|
||||
renderGutterUtility: ReturnType<typeof createLineCommentGutterRenderer>
|
||||
onLineSelected: (range: SelectedLineRange | null) => void
|
||||
onLineSelectionEnd: (range: SelectedLineRange | null) => void
|
||||
onLineNumberSelectionEnd: (range: SelectedLineRange | null) => void
|
||||
}
|
||||
export function createLineCommentController<T extends LineCommentShape>(
|
||||
props: LineCommentControllerProps<T> | LineCommentControllerWithSideProps<T>,
|
||||
|
|
@ -426,7 +417,7 @@ export function createLineCommentController<T extends LineCommentShape>(
|
|||
}),
|
||||
})
|
||||
|
||||
const renderHoverUtility = createLineCommentHoverRenderer({
|
||||
const renderGutterUtility = createLineCommentGutterRenderer({
|
||||
label: props.label,
|
||||
getSelectedRange: () => {
|
||||
if (note.opened()) return null
|
||||
|
|
@ -452,11 +443,6 @@ export function createLineCommentController<T extends LineCommentShape>(
|
|||
return
|
||||
}
|
||||
|
||||
note.finishSelection(range)
|
||||
}
|
||||
|
||||
const onLineNumberSelectionEnd = (range: SelectedLineRange | null) => {
|
||||
if (!range) return
|
||||
note.openDraft(range)
|
||||
}
|
||||
|
||||
|
|
@ -464,10 +450,9 @@ export function createLineCommentController<T extends LineCommentShape>(
|
|||
note,
|
||||
annotations,
|
||||
renderAnnotation,
|
||||
renderHoverUtility,
|
||||
renderGutterUtility,
|
||||
onLineSelected,
|
||||
onLineSelectionEnd,
|
||||
onLineNumberSelectionEnd,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -569,7 +554,7 @@ export function createManagedLineCommentAnnotationRenderer<T>(props: {
|
|||
}
|
||||
}
|
||||
|
||||
export function createLineCommentHoverRenderer(props: {
|
||||
export function createLineCommentGutterRenderer(props: {
|
||||
label: string
|
||||
getSelectedRange: Accessor<SelectedLineRange | null>
|
||||
onOpenDraft: (range: SelectedLineRange) => void
|
||||
|
|
|
|||
|
|
@ -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={{
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue