Skip to content
This repository was archived by the owner on Aug 6, 2026. It is now read-only.

Commit ac76f5f

Browse files
authored
fix(code-review): show PR comment counts on files (#3433)
1 parent 7806f6f commit ac76f5f

5 files changed

Lines changed: 146 additions & 14 deletions

File tree

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import type { FileDiffMetadata } from "@pierre/diffs";
2+
import type { PrCommentThread } from "@posthog/core/code-review/types";
3+
import type { ChangedFile } from "@posthog/shared/domain-types";
4+
import { render, screen } from "@testing-library/react";
5+
import type { ReactNode } from "react";
6+
import { describe, expect, it, vi } from "vitest";
7+
8+
vi.mock("../../../primitives/FileIcon", () => ({
9+
FileIcon: () => <span data-testid="file-icon" />,
10+
}));
11+
12+
vi.mock("./InteractiveFileDiff", () => ({
13+
InteractiveFileDiff: ({
14+
fileDiff,
15+
renderCustomHeader,
16+
}: {
17+
fileDiff: FileDiffMetadata;
18+
renderCustomHeader: (fileDiff: FileDiffMetadata) => ReactNode;
19+
}) => renderCustomHeader(fileDiff),
20+
}));
21+
22+
import { PatchedFileDiff } from "./PatchedFileDiff";
23+
24+
const patch = `diff --git a/src/reviewed.ts b/src/reviewed.ts
25+
index 1111111..2222222 100644
26+
--- a/src/reviewed.ts
27+
+++ b/src/reviewed.ts
28+
@@ -1 +1 @@
29+
-before
30+
+after`;
31+
32+
describe.each([
33+
[
34+
"regular",
35+
{
36+
path: "src/reviewed.ts",
37+
originalPath: "src/original.ts",
38+
patch,
39+
},
40+
],
41+
["binary", { path: "assets/reviewed.png", patch: null }],
42+
["unavailable", { path: "src/unavailable.ts", patch: null }],
43+
] as const)("PatchedFileDiff %s header", (_kind, fileInput) => {
44+
it("renders metadata before line change stats", () => {
45+
const file = {
46+
...fileInput,
47+
linesAdded: 2,
48+
linesRemoved: 1,
49+
} as ChangedFile;
50+
const threadPath = file.originalPath ?? file.path;
51+
const commentThreads = new Map<number, PrCommentThread>([
52+
[
53+
1,
54+
{
55+
rootId: 1,
56+
nodeId: "thread-1",
57+
isResolved: false,
58+
filePath: threadPath,
59+
comments: [{ id: 1 }, { id: 2 }] as PrCommentThread["comments"],
60+
},
61+
],
62+
]);
63+
64+
render(
65+
<PatchedFileDiff
66+
file={file}
67+
taskId="task"
68+
options={{}}
69+
collapsed
70+
onToggle={() => {}}
71+
commentThreads={commentThreads}
72+
/>,
73+
);
74+
75+
const header = screen.getByRole("button");
76+
const text = header.textContent ?? "";
77+
const additions = _kind === "regular" ? "+1" : "+2";
78+
79+
expect(screen.getByTitle("2 comments")).toBeInTheDocument();
80+
expect(text.indexOf("2 comments")).toBeLessThan(text.indexOf(additions));
81+
});
82+
});

packages/ui/src/features/code-review/components/PatchedFileDiff.tsx

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ export function PatchedFileDiff({
4949
}
5050
return null;
5151
}, [fileDiff, fallback, file.path]);
52+
const commentCount = countPrCommentsForFile(commentThreads, file);
5253

5354
// Branch/PR diffs have no reliable local working-tree file to preview (the
5455
// checkout may be on a different ref, and GitHub omits binary patches), so
@@ -63,6 +64,7 @@ export function PatchedFileDiff({
6364
collapsed={collapsed}
6465
onToggle={onToggle}
6566
externalUrl={externalUrl}
67+
commentCount={commentCount}
6668
headerTrailing={headerTrailing}
6769
/>
6870
);
@@ -78,6 +80,7 @@ export function PatchedFileDiff({
7880
collapsed={collapsed}
7981
onToggle={onToggle}
8082
externalUrl={externalUrl}
83+
commentCount={commentCount}
8184
headerTrailing={headerTrailing}
8285
/>
8386
);
@@ -95,9 +98,26 @@ export function PatchedFileDiff({
9598
fileDiff={fd}
9699
collapsed={collapsed}
97100
onToggle={onToggle}
101+
commentCount={commentCount}
98102
trailing={headerTrailing}
99103
/>
100104
)}
101105
/>
102106
);
103107
}
108+
109+
function countPrCommentsForFile(
110+
threads: Map<number, PrCommentThread> | undefined,
111+
file: Pick<ChangedFile, "path" | "originalPath">,
112+
): number {
113+
let count = 0;
114+
for (const thread of threads?.values() ?? []) {
115+
if (
116+
thread.filePath === file.path ||
117+
(file.originalPath != null && thread.filePath === file.originalPath)
118+
) {
119+
count += thread.comments.length;
120+
}
121+
}
122+
return count;
123+
}

packages/ui/src/features/code-review/reviewShellParts.test.tsx

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,13 @@ function findSpan(
3636
return found;
3737
}
3838

39-
function renderHeader(path: string) {
39+
function renderHeader(path: string, commentCount?: number) {
4040
const diff = render(
4141
<DiffFileHeader
4242
fileDiff={makeFileDiff(path)}
4343
collapsed={false}
4444
onToggle={() => {}}
45+
commentCount={commentCount}
4546
/>,
4647
);
4748
const deferred = render(
@@ -52,6 +53,7 @@ function renderHeader(path: string) {
5253
reason="line-limit"
5354
collapsed={false}
5455
onToggle={() => {}}
56+
commentCount={commentCount}
5557
/>,
5658
);
5759
return { diff, deferred };
@@ -96,4 +98,12 @@ describe.each([
9698
expect(dirSpan.parentElement).toBe(fileSpan.parentElement);
9799
expect(dirSpan.parentElement?.classList.contains("flex")).toBe(true);
98100
});
101+
102+
it("renders metadata before line changes", () => {
103+
const rendered = renderHeader("src/ReviewShell.tsx", 2)[which];
104+
const text = rendered.container.querySelector("button")?.textContent ?? "";
105+
const additions = which === "diff" ? "+3" : "+10";
106+
107+
expect(text.indexOf("2 comments")).toBeLessThan(text.indexOf(additions));
108+
});
99109
});

packages/ui/src/features/code-review/reviewShellParts.tsx

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import {
22
ArrowCounterClockwise,
33
ArrowSquareOut,
44
CaretDown,
5+
ChatCircle,
56
Minus,
67
Plus,
78
} from "@phosphor-icons/react";
@@ -13,6 +14,7 @@ import {
1314
splitFilePath,
1415
sumHunkStats,
1516
} from "@posthog/core/code-review/reviewShellGeometry";
17+
import { Badge } from "@posthog/quill";
1618
import type { ChangedFile, Task } from "@posthog/shared/domain-types";
1719
import { type ReactNode, useCallback, useMemo, useState } from "react";
1820
import { FileIcon } from "../../primitives/FileIcon";
@@ -139,6 +141,7 @@ export function FileHeaderRow({
139141
deletions,
140142
collapsed,
141143
onToggle,
144+
commentCount,
142145
trailing,
143146
}: {
144147
dirPath: string;
@@ -147,6 +150,7 @@ export function FileHeaderRow({
147150
deletions: number;
148151
collapsed: boolean;
149152
onToggle: () => void;
153+
commentCount?: number;
150154
trailing?: ReactNode;
151155
}) {
152156
return (
@@ -176,6 +180,9 @@ export function FileHeaderRow({
176180
{dirPath}
177181
</span>
178182
</span>
183+
{commentCount != null && commentCount > 0 && (
184+
<PrCommentCountBadge count={commentCount} />
185+
)}
179186
<span className="font-mono text-[10px]">
180187
{additions > 0 && (
181188
<span className="mr-[2px] text-(--green-9)">+{additions}</span>
@@ -210,6 +217,7 @@ export function DiffFileHeader({
210217
onDiscard,
211218
onStage,
212219
staged,
220+
commentCount,
213221
trailing,
214222
}: {
215223
fileDiff: FileDiffMetadata;
@@ -219,6 +227,7 @@ export function DiffFileHeader({
219227
onDiscard?: () => void;
220228
onStage?: () => void;
221229
staged?: boolean;
230+
commentCount?: number;
222231
/** Extra controls rendered after the action buttons (e.g. a "Viewed" toggle). */
223232
trailing?: ReactNode;
224233
}) {
@@ -237,6 +246,7 @@ export function DiffFileHeader({
237246
deletions={deletions}
238247
collapsed={collapsed}
239248
onToggle={onToggle}
249+
commentCount={commentCount}
240250
trailing={
241251
(onStage || onDiscard || onOpenFile || trailing) && (
242252
<span className="ml-auto inline-flex items-center gap-[2px]">
@@ -299,6 +309,7 @@ export function DeferredDiffPlaceholder({
299309
onToggle,
300310
onShow,
301311
externalUrl,
312+
commentCount,
302313
headerTrailing,
303314
}: {
304315
filePath: string;
@@ -309,6 +320,7 @@ export function DeferredDiffPlaceholder({
309320
onToggle: () => void;
310321
onShow?: () => void;
311322
externalUrl?: string;
323+
commentCount?: number;
312324
/** Extra controls in the header row (e.g. a "Viewed" toggle). */
313325
headerTrailing?: ReactNode;
314326
}) {
@@ -323,6 +335,7 @@ export function DeferredDiffPlaceholder({
323335
deletions={linesRemoved}
324336
collapsed={collapsed}
325337
onToggle={onToggle}
338+
commentCount={commentCount}
326339
trailing={
327340
headerTrailing && (
328341
<span className="ml-auto inline-flex items-center">
@@ -369,3 +382,18 @@ export function DeferredDiffPlaceholder({
369382
</div>
370383
);
371384
}
385+
386+
function PrCommentCountBadge({ count }: { count: number }) {
387+
const label = `${count} comment${count === 1 ? "" : "s"}`;
388+
return (
389+
<Badge
390+
variant="default"
391+
title={label}
392+
className="shrink-0 gap-[3px] border-(--gray-7) bg-(--gray-3) text-[11px] text-gray-12 tabular-nums"
393+
>
394+
<ChatCircle size={12} weight="fill" />
395+
{count}
396+
<span className="sr-only"> comment{count === 1 ? "" : "s"}</span>
397+
</Badge>
398+
);
399+
}

packages/ui/src/features/git-interaction/usePrDetails.ts

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,10 @@ interface UsePrDetailsOptions {
88
includeComments?: boolean;
99
}
1010

11-
function threadsToMap(threads: PrReviewThread[]): Map<number, PrCommentThread> {
12-
const map = new Map<number, PrCommentThread>();
13-
for (const thread of threads) {
14-
map.set(thread.rootId, {
15-
rootId: thread.rootId,
16-
nodeId: thread.nodeId,
17-
isResolved: thread.isResolved,
18-
comments: thread.comments,
19-
filePath: thread.filePath,
20-
});
21-
}
22-
return map;
11+
function mapPrCommentThreads(
12+
threads: PrReviewThread[],
13+
): Map<number, PrCommentThread> {
14+
return new Map(threads.map((thread) => [thread.rootId, thread]));
2315
}
2416

2517
export interface PrStateDetails {
@@ -80,7 +72,7 @@ export function usePrDetails(
8072
});
8173

8274
const commentThreads = useMemo(
83-
() => threadsToMap(commentsQuery.data ?? []),
75+
() => mapPrCommentThreads(commentsQuery.data ?? []),
8476
[commentsQuery.data],
8577
);
8678

0 commit comments

Comments
 (0)