diff --git a/packages/core/src/code-review/reviewPrompts.test.ts b/packages/core/src/code-review/reviewPrompts.test.ts index 376940691f..c25e0a9ede 100644 --- a/packages/core/src/code-review/reviewPrompts.test.ts +++ b/packages/core/src/code-review/reviewPrompts.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest"; import { buildAskAboutPrCommentPrompt, buildBatchedInlineCommentsPrompt, + buildChatAboutPrCommentPrompt, buildFixPrCommentPrompt, buildInlineCommentPrompt, } from "./reviewPrompts"; @@ -65,7 +66,7 @@ describe("buildBatchedInlineCommentsPrompt", () => { }); }); -describe("buildFixPrCommentPrompt / buildAskAboutPrCommentPrompt", () => { +describe("PR comment prompts", () => { it("includes the thread body and side", () => { const out = buildFixPrCommentPrompt("a.ts", 4, "new", [ makeComment("please rename"), @@ -81,4 +82,19 @@ describe("buildFixPrCommentPrompt / buildAskAboutPrCommentPrompt", () => { ]); expect(out).toContain("Do not make any changes"); }); + + it("chat prompt includes the thread and custom message", () => { + const out = buildChatAboutPrCommentPrompt( + 'src/a".ts', + 8, + "new", + [makeComment("consider extracting this", "reviewer")], + "Is there already a helper for this?", + ); + expect(out).toContain(''); + expect(out).toContain("line 8 (new)"); + expect(out).toContain("@reviewer"); + expect(out).toContain("consider extracting this"); + expect(out.endsWith("Is there already a helper for this?")).toBe(true); + }); }); diff --git a/packages/core/src/code-review/reviewPrompts.ts b/packages/core/src/code-review/reviewPrompts.ts index 688aa79d4a..a5c7570d1b 100644 --- a/packages/core/src/code-review/reviewPrompts.ts +++ b/packages/core/src/code-review/reviewPrompts.ts @@ -15,6 +15,17 @@ function formatThreadForPrompt(comments: PrReviewComment[]): string { return comments.map((c) => `@${c.user.login}:\n> ${c.body}`).join("\n\n"); } +function formatPrCommentPromptContext( + filePath: string, + line: number, + side: "old" | "new", + comments: PrReviewComment[], +): string { + const escapedPath = escapeXmlAttr(filePath); + const thread = formatThreadForPrompt(comments); + return `, line ${line} (${side}):\n\n${thread}`; +} + function formatLineRef(startLine: number, endLine: number): string { return startLine === endLine ? `line ${startLine}` @@ -85,9 +96,8 @@ export function buildFixPrCommentPrompt( side: "old" | "new", comments: PrReviewComment[], ): string { - const escapedPath = escapeXmlAttr(filePath); - const thread = formatThreadForPrompt(comments); - return `Fix this PR review comment on , line ${line} (${side}):\n\n${thread}`; + const context = formatPrCommentPromptContext(filePath, line, side, comments); + return `Fix this PR review comment on ${context}`; } export function buildAskAboutPrCommentPrompt( @@ -96,7 +106,17 @@ export function buildAskAboutPrCommentPrompt( side: "old" | "new", comments: PrReviewComment[], ): string { - const escapedPath = escapeXmlAttr(filePath); - const thread = formatThreadForPrompt(comments); - return `Help me understand this PR review comment on , line ${line} (${side}):\n\n${thread}\n\nWhat is this comment asking for and how should I address it? Do not make any changes, your job is simply to chat with me about this comment. If I need further changes, I'll ask.`; + const context = formatPrCommentPromptContext(filePath, line, side, comments); + return `Help me understand this PR review comment on ${context}\n\nWhat is this comment asking for and how should I address it? Do not make any changes, your job is simply to chat with me about this comment. If I need further changes, I'll ask.`; +} + +export function buildChatAboutPrCommentPrompt( + filePath: string, + line: number, + side: "old" | "new", + comments: PrReviewComment[], + message: string, +): string { + const context = formatPrCommentPromptContext(filePath, line, side, comments); + return `Regarding this PR review comment on ${context}\n\n${message}`; } diff --git a/packages/ui/src/features/code-review/components/PatchedFileDiff.test.tsx b/packages/ui/src/features/code-review/components/PatchedFileDiff.test.tsx new file mode 100644 index 0000000000..bc2fac4499 --- /dev/null +++ b/packages/ui/src/features/code-review/components/PatchedFileDiff.test.tsx @@ -0,0 +1,82 @@ +import type { FileDiffMetadata } from "@pierre/diffs"; +import type { PrCommentThread } from "@posthog/core/code-review/types"; +import type { ChangedFile } from "@posthog/shared/domain-types"; +import { render, screen } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { describe, expect, it, vi } from "vitest"; + +vi.mock("../../../primitives/FileIcon", () => ({ + FileIcon: () => , +})); + +vi.mock("./InteractiveFileDiff", () => ({ + InteractiveFileDiff: ({ + fileDiff, + renderCustomHeader, + }: { + fileDiff: FileDiffMetadata; + renderCustomHeader: (fileDiff: FileDiffMetadata) => ReactNode; + }) => renderCustomHeader(fileDiff), +})); + +import { PatchedFileDiff } from "./PatchedFileDiff"; + +const patch = `diff --git a/src/reviewed.ts b/src/reviewed.ts +index 1111111..2222222 100644 +--- a/src/reviewed.ts ++++ b/src/reviewed.ts +@@ -1 +1 @@ +-before ++after`; + +describe.each([ + [ + "regular", + { + path: "src/reviewed.ts", + originalPath: "src/original.ts", + patch, + }, + ], + ["binary", { path: "assets/reviewed.png", patch: null }], + ["unavailable", { path: "src/unavailable.ts", patch: null }], +] as const)("PatchedFileDiff %s header", (_kind, fileInput) => { + it("renders metadata before line change stats", () => { + const file = { + ...fileInput, + linesAdded: 2, + linesRemoved: 1, + } as ChangedFile; + const threadPath = file.originalPath ?? file.path; + const commentThreads = new Map([ + [ + 1, + { + rootId: 1, + nodeId: "thread-1", + isResolved: false, + filePath: threadPath, + comments: [{ id: 1 }, { id: 2 }] as PrCommentThread["comments"], + }, + ], + ]); + + render( + {}} + commentThreads={commentThreads} + />, + ); + + const header = screen.getByRole("button"); + const text = header.textContent ?? ""; + const additions = _kind === "regular" ? "+1" : "+2"; + + expect(screen.getByTitle("2 comments")).toBeInTheDocument(); + expect(text.indexOf("2 comments")).toBeLessThan(text.indexOf(additions)); + }); +}); diff --git a/packages/ui/src/features/code-review/components/PatchedFileDiff.tsx b/packages/ui/src/features/code-review/components/PatchedFileDiff.tsx index e648537e1d..93b987636a 100644 --- a/packages/ui/src/features/code-review/components/PatchedFileDiff.tsx +++ b/packages/ui/src/features/code-review/components/PatchedFileDiff.tsx @@ -49,6 +49,7 @@ export function PatchedFileDiff({ } return null; }, [fileDiff, fallback, file.path]); + const commentCount = countPrCommentsForFile(commentThreads, file); // Branch/PR diffs have no reliable local working-tree file to preview (the // checkout may be on a different ref, and GitHub omits binary patches), so @@ -63,6 +64,7 @@ export function PatchedFileDiff({ collapsed={collapsed} onToggle={onToggle} externalUrl={externalUrl} + commentCount={commentCount} headerTrailing={headerTrailing} /> ); @@ -78,6 +80,7 @@ export function PatchedFileDiff({ collapsed={collapsed} onToggle={onToggle} externalUrl={externalUrl} + commentCount={commentCount} headerTrailing={headerTrailing} /> ); @@ -95,9 +98,26 @@ export function PatchedFileDiff({ fileDiff={fd} collapsed={collapsed} onToggle={onToggle} + commentCount={commentCount} trailing={headerTrailing} /> )} /> ); } + +function countPrCommentsForFile( + threads: Map | undefined, + file: Pick, +): number { + let count = 0; + for (const thread of threads?.values() ?? []) { + if ( + thread.filePath === file.path || + (file.originalPath != null && thread.filePath === file.originalPath) + ) { + count += thread.comments.length; + } + } + return count; +} diff --git a/packages/ui/src/features/code-review/components/PrCommentThread.test.tsx b/packages/ui/src/features/code-review/components/PrCommentThread.test.tsx new file mode 100644 index 0000000000..7f14fe2ee9 --- /dev/null +++ b/packages/ui/src/features/code-review/components/PrCommentThread.test.tsx @@ -0,0 +1,154 @@ +import type { PrReviewComment } from "@posthog/shared"; +import { Theme } from "@radix-ui/themes"; +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { PrCommentMetadata } from "../types"; +import { PrCommentThread } from "./PrCommentThread"; + +const { reply, resolve, sendPromptToAgent } = vi.hoisted(() => ({ + reply: vi.fn(), + resolve: vi.fn(), + sendPromptToAgent: vi.fn(), +})); + +vi.mock("../hooks/usePrCommentActions", () => ({ + usePrCommentActions: () => ({ reply, resolve }), +})); + +vi.mock("../../sessions/sendPromptToAgent", () => ({ sendPromptToAgent })); + +function makeComment(): PrReviewComment { + return { + id: 42, + body: "Could this use the shared helper?", + created_at: "2026-07-14T12:00:00Z", + user: { + login: "reviewer", + avatar_url: "", + }, + } as PrReviewComment; +} + +function makeMetadata(): PrCommentMetadata { + return { + kind: "pr-comment", + threadId: 42, + nodeId: "thread-node", + isResolved: false, + comments: [makeComment()], + isOutdated: false, + isFileLevel: false, + startLine: 8, + endLine: 8, + side: "additions", + }; +} + +function renderThread() { + return render( + + + , + ); +} + +describe("PrCommentThread", () => { + beforeEach(() => { + vi.clearAllMocks(); + reply.mockResolvedValue(true); + resolve.mockResolvedValue(true); + sendPromptToAgent.mockResolvedValue(true); + }); + + it("sends a custom chat message with the review context", async () => { + const user = userEvent.setup(); + renderThread(); + + await user.click(screen.getByRole("button", { name: "Chat" })); + await user.type( + screen.getByPlaceholderText("Ask the agent about this comment..."), + "Check whether this helper already exists", + ); + await user.click(screen.getByRole("button", { name: "Send" })); + + expect(sendPromptToAgent).toHaveBeenCalledWith( + "task-1", + expect.stringContaining("Could this use the shared helper?"), + ); + expect(sendPromptToAgent).toHaveBeenCalledWith( + "task-1", + expect.stringContaining("Check whether this helper already exists"), + ); + await waitFor(() => + expect( + screen.queryByPlaceholderText("Ask the agent about this comment..."), + ).not.toBeInTheDocument(), + ); + }); + + it("keeps the custom message available when sending fails", async () => { + sendPromptToAgent.mockResolvedValue(false); + const user = userEvent.setup(); + renderThread(); + + await user.click(screen.getByRole("button", { name: "Chat" })); + const textarea = screen.getByPlaceholderText( + "Ask the agent about this comment...", + ); + await user.type(textarea, "Keep this draft"); + await user.click(screen.getByRole("button", { name: "Send" })); + + await waitFor(() => + expect( + screen.getByPlaceholderText("Ask the agent about this comment..."), + ).toHaveValue("Keep this draft"), + ); + }); + + it("keeps a reply composer opened while a chat message is sending", async () => { + let resolveSend: ((success: boolean) => void) | undefined; + sendPromptToAgent.mockReturnValue( + new Promise((resolve) => { + resolveSend = resolve; + }), + ); + const user = userEvent.setup(); + renderThread(); + + await user.click(screen.getByRole("button", { name: "Chat" })); + await user.type( + screen.getByPlaceholderText("Ask the agent about this comment..."), + "Check this", + ); + await user.click(screen.getByRole("button", { name: "Send" })); + await user.click(screen.getByRole("button", { name: "Close composer" })); + await user.click(screen.getByRole("button", { name: "Reply" })); + await user.type(screen.getByPlaceholderText("Write a reply..."), "Keep me"); + + resolveSend?.(true); + + await waitFor(() => + expect(screen.getByPlaceholderText("Write a reply...")).toHaveValue( + "Keep me", + ), + ); + }); + + it("still posts replies without sending them to chat", async () => { + const user = userEvent.setup(); + renderThread(); + + await user.click(screen.getByRole("button", { name: "Reply" })); + await user.type(screen.getByPlaceholderText("Write a reply..."), "Done"); + await user.click(screen.getByRole("button", { name: "Reply" })); + + expect(reply).toHaveBeenCalledWith(42, "Done"); + expect(sendPromptToAgent).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/ui/src/features/code-review/components/PrCommentThread.tsx b/packages/ui/src/features/code-review/components/PrCommentThread.tsx index dfc10acec3..3addfc943c 100644 --- a/packages/ui/src/features/code-review/components/PrCommentThread.tsx +++ b/packages/ui/src/features/code-review/components/PrCommentThread.tsx @@ -12,6 +12,7 @@ import { } from "@phosphor-icons/react"; import { buildAskAboutPrCommentPrompt, + buildChatAboutPrCommentPrompt, buildFixPrCommentPrompt, } from "@posthog/core/code-review/reviewPrompts"; import { Button } from "@posthog/quill"; @@ -40,6 +41,7 @@ const ghRehypePlugins: PluggableList = [ ]; const MAX_COMMENT_HEIGHT = 120; +type ComposerMode = "reply" | "chat"; /** Strip markdown noise to a single-line preview for the collapsed header. */ function toPreview(body: string): string { @@ -61,11 +63,12 @@ interface ThreadActionBarProps { comments: PrReviewComment[]; isResolved: boolean; onResolveToggle: () => void; - showReplyBox: boolean; + composerMode: ComposerMode | null; pendingReply: string | null; - onShowReplyBox: () => void; - onHideReplyBox: () => void; - onSubmitReply: () => void; + isSendingChat: boolean; + onShowComposer: (mode: ComposerMode) => void; + onHideComposer: () => void; + onSubmitComposer: () => void; onKeyDown: (e: React.KeyboardEvent) => void; textareaRefCallback: (el: HTMLTextAreaElement | null) => void; } @@ -79,20 +82,26 @@ function ThreadActionBar({ comments, isResolved, onResolveToggle, - showReplyBox, + composerMode, pendingReply, - onShowReplyBox, - onHideReplyBox, - onSubmitReply, + isSendingChat, + onShowComposer, + onHideComposer, + onSubmitComposer, onKeyDown, textareaRefCallback, }: ThreadActionBarProps) { - if (showReplyBox) { + if (composerMode) { + const isReply = composerMode === "reply"; + const isSending = isSendingChat || (isReply && !!pendingReply); + const submitLabel = isSending ? "Sending..." : isReply ? "Reply" : "Send"; return (