Skip to content
Closed
18 changes: 17 additions & 1 deletion packages/core/src/code-review/reviewPrompts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest";
import {
buildAskAboutPrCommentPrompt,
buildBatchedInlineCommentsPrompt,
buildChatAboutPrCommentPrompt,
buildFixPrCommentPrompt,
buildInlineCommentPrompt,
} from "./reviewPrompts";
Expand Down Expand Up @@ -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"),
Expand All @@ -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('<file path="src/a&quot;.ts" />');
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);
});
});
32 changes: 26 additions & 6 deletions packages/core/src/code-review/reviewPrompts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<file path="${escapedPath}" />, line ${line} (${side}):\n\n${thread}`;
}

function formatLineRef(startLine: number, endLine: number): string {
return startLine === endLine
? `line ${startLine}`
Expand Down Expand Up @@ -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 <file path="${escapedPath}" />, line ${line} (${side}):\n\n${thread}`;
const context = formatPrCommentPromptContext(filePath, line, side, comments);
return `Fix this PR review comment on ${context}`;
}

export function buildAskAboutPrCommentPrompt(
Expand All @@ -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 <file path="${escapedPath}" />, 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}`;
}
Original file line number Diff line number Diff line change
@@ -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: () => <span data-testid="file-icon" />,
}));

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<number, PrCommentThread>([
[
1,
{
rootId: 1,
nodeId: "thread-1",
isResolved: false,
filePath: threadPath,
comments: [{ id: 1 }, { id: 2 }] as PrCommentThread["comments"],
},
],
]);

render(
<PatchedFileDiff
file={file}
taskId="task"
options={{}}
collapsed
onToggle={() => {}}
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));
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -63,6 +64,7 @@ export function PatchedFileDiff({
collapsed={collapsed}
onToggle={onToggle}
externalUrl={externalUrl}
commentCount={commentCount}
headerTrailing={headerTrailing}
/>
);
Expand All @@ -78,6 +80,7 @@ export function PatchedFileDiff({
collapsed={collapsed}
onToggle={onToggle}
externalUrl={externalUrl}
commentCount={commentCount}
headerTrailing={headerTrailing}
/>
);
Expand All @@ -95,9 +98,26 @@ export function PatchedFileDiff({
fileDiff={fd}
collapsed={collapsed}
onToggle={onToggle}
commentCount={commentCount}
trailing={headerTrailing}
/>
)}
/>
);
}

function countPrCommentsForFile(
threads: Map<number, PrCommentThread> | undefined,
file: Pick<ChangedFile, "path" | "originalPath">,
): 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;
}
Original file line number Diff line number Diff line change
@@ -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(
<Theme>
<PrCommentThread
taskId="task-1"
prUrl="https://github.com/PostHog/posthog/pull/1"
filePath="src/example.ts"
metadata={makeMetadata()}
/>
</Theme>,
);
}

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<boolean>((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();
});
});
Loading
Loading