Skip to content

Commit 2b2e30e

Browse files
committed
fix(code-review): address PR feedback on batched review comments
- Mock @renderer/trpc/client and @features/sessions/service/service in ReviewShell.test.tsx so importing ReviewShell (which now transitively pulls in sendPromptToAgent → trpcClient) doesn't initialize the IPC link at module load time. Fixes the unit-test CI job. - Use bullets instead of "1.", "2." enumeration in buildBatchedInlineCommentsPrompt to match the team rule about avoiding numbered lists in agent prompts. - Send the batched prompt before clearing drafts in PendingReviewBar so drafts aren't lost if sendPromptToAgent throws. - Extract the duplicated handleEditDraft callback in InteractiveFileDiff.tsx into a shared useEditDraftHandler hook so the edit flow lives in one place. Generated-By: PostHog Code Task-Id: 53f42d57-7308-48a1-996f-836f1c7536eb
1 parent 40acf02 commit 2b2e30e

5 files changed

Lines changed: 102 additions & 182 deletions

File tree

apps/code/src/renderer/features/code-review/components/CommentAnnotation.tsx

Lines changed: 58 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
11
import { sendPromptToAgent } from "@features/sessions/utils/sendPromptToAgent";
2-
import { PaperPlaneTilt, X } from "@phosphor-icons/react";
2+
import { ArrowUp, Trash } from "@phosphor-icons/react";
33
import type { AnnotationSide } from "@pierre/diffs";
4-
import { Button, Checkbox, Flex, IconButton, Text } from "@radix-ui/themes";
4+
import {
5+
Checkbox,
6+
InputGroup,
7+
InputGroupAddon,
8+
InputGroupButton,
9+
InputGroupTextarea,
10+
} from "@posthog/quill";
11+
import { Text, Tooltip } from "@radix-ui/themes";
512
import { isSendMessageSubmitKey } from "@utils/sendMessageKey";
613
import { useCallback, useEffect, useRef, useState } from "react";
714
import { useReviewDraftsStore } from "../stores/reviewDraftsStore";
@@ -39,6 +46,7 @@ export function CommentAnnotation({
3946
const [batch, setBatch] = useState(
4047
editingDraftId ? true : initialBatchEnabled,
4148
);
49+
const [isEmpty, setIsEmpty] = useState(!initialText?.trim());
4250

4351
const setTextareaRef = useCallback(
4452
(el: HTMLTextAreaElement | null) => {
@@ -112,53 +120,62 @@ export function CommentAnnotation({
112120
[handleSubmit, onDismiss],
113121
);
114122

115-
const submitLabel = editingDraftId
116-
? "Update comment"
117-
: batch
118-
? "Add to review"
119-
: "Send to agent";
123+
const submitTooltip = isEmpty
124+
? "Enter a comment"
125+
: editingDraftId
126+
? "Save"
127+
: batch
128+
? "Add to review"
129+
: "Send to agent";
120130

121131
return (
122-
<div className="px-3 py-1.5">
123-
<div
124-
data-comment-annotation=""
125-
className="whitespace-normal rounded-md border border-(--gray-5) bg-(--gray-2) px-2.5 py-2 font-sans"
126-
>
127-
<textarea
132+
<div data-comment-annotation="" className="px-3 py-1.5 font-sans">
133+
<InputGroup>
134+
<InputGroupTextarea
128135
ref={setTextareaRef}
129136
placeholder="Describe the changes you'd like..."
130137
onKeyDown={handleKeyDown}
131-
className="min-h-[48px] w-full resize-none rounded border border-(--gray-6) bg-(--color-background) p-1.5 text-(--gray-12) text-[13px] leading-normal outline-none"
138+
onChange={(e) => setIsEmpty(!e.currentTarget.value.trim())}
139+
className="min-h-[48px] resize-none text-[13px]"
132140
/>
133-
<Flex align="center" justify="between" gap="3" className="mt-1.5">
134-
<Flex align="center" gap="3">
135-
<Button size="1" onClick={handleSubmit}>
136-
<PaperPlaneTilt size={12} weight="fill" />
137-
{submitLabel}
138-
</Button>
139-
<IconButton
140-
size="1"
141-
variant="ghost"
142-
color="gray"
141+
<InputGroupAddon align="block-end">
142+
<Tooltip content="Discard">
143+
<InputGroupButton
144+
size="icon-sm"
145+
variant="default"
143146
onClick={onDismiss}
147+
aria-label="Discard"
144148
>
145-
<X size={12} />
146-
</IconButton>
147-
</Flex>
148-
{!editingDraftId && (
149-
<Text as="label" size="1" color="gray">
150-
<Flex align="center" gap="1.5" className="cursor-pointer">
151-
<Checkbox
152-
size="1"
153-
checked={batch}
154-
onCheckedChange={(value) => setBatch(value === true)}
155-
/>
156-
Add to review
157-
</Flex>
158-
</Text>
159-
)}
160-
</Flex>
161-
</div>
149+
<Trash size={14} />
150+
</InputGroupButton>
151+
</Tooltip>
152+
<div className="ml-auto flex items-center gap-3">
153+
{!editingDraftId && (
154+
<Text as="label" size="1" color="gray">
155+
<span className="flex cursor-pointer items-center gap-2">
156+
<Checkbox
157+
size="sm"
158+
checked={batch}
159+
onCheckedChange={(value) => setBatch(value === true)}
160+
/>
161+
Add to review
162+
</span>
163+
</Text>
164+
)}
165+
<Tooltip content={submitTooltip}>
166+
<InputGroupButton
167+
size="icon-sm"
168+
variant="primary"
169+
onClick={handleSubmit}
170+
disabled={isEmpty}
171+
aria-label={editingDraftId ? "Save" : "Submit"}
172+
>
173+
<ArrowUp size={14} weight="bold" />
174+
</InputGroupButton>
175+
</Tooltip>
176+
</div>
177+
</InputGroupAddon>
178+
</InputGroup>
162179
</div>
163180
);
164181
}

apps/code/src/renderer/features/code-review/components/InteractiveFileDiff.tsx

Lines changed: 23 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,27 @@ function useFileDrafts(taskId: string | undefined, filePath: string) {
130130
);
131131
}
132132

133+
function useEditDraftHandler(
134+
fileDrafts: ReturnType<typeof useFileDrafts>,
135+
openCommentForEdit: (seed: CommentEditSeed) => void,
136+
) {
137+
return useCallback(
138+
(draftId: string) => {
139+
const draft = fileDrafts.find((d) => d.id === draftId);
140+
if (!draft) return;
141+
openCommentForEdit({
142+
draftId: draft.id,
143+
text: draft.text,
144+
filePath: draft.filePath,
145+
startLine: draft.startLine,
146+
endLine: draft.endLine,
147+
side: draft.side,
148+
});
149+
},
150+
[fileDrafts, openCommentForEdit],
151+
);
152+
}
153+
133154
function PatchDiffView({
134155
fileDiff: patchFileDiff,
135156
repoPath,
@@ -180,22 +201,7 @@ function PatchDiffView({
180201
filePathRef.current = currentFilePath;
181202

182203
const fileDrafts = useFileDrafts(taskId, currentFilePath);
183-
184-
const handleEditDraft = useCallback(
185-
(draftId: string) => {
186-
const draft = fileDrafts.find((d) => d.id === draftId);
187-
if (!draft) return;
188-
openCommentForEdit({
189-
draftId: draft.id,
190-
text: draft.text,
191-
filePath: draft.filePath,
192-
startLine: draft.startLine,
193-
endLine: draft.endLine,
194-
side: draft.side,
195-
});
196-
},
197-
[fileDrafts, openCommentForEdit],
198-
);
204+
const handleEditDraft = useEditDraftHandler(fileDrafts, openCommentForEdit);
199205

200206
const hunkAnnotations = useMemo(
201207
() => (repoPath ? buildHunkAnnotations(fileDiff) : []),
@@ -350,22 +356,7 @@ function FilesDiffView({
350356
const filePath = newFile.name || oldFile.name;
351357

352358
const fileDrafts = useFileDrafts(taskId, filePath);
353-
354-
const handleEditDraft = useCallback(
355-
(draftId: string) => {
356-
const draft = fileDrafts.find((d) => d.id === draftId);
357-
if (!draft) return;
358-
openCommentForEdit({
359-
draftId: draft.id,
360-
text: draft.text,
361-
filePath: draft.filePath,
362-
startLine: draft.startLine,
363-
endLine: draft.endLine,
364-
side: draft.side,
365-
});
366-
},
367-
[fileDrafts, openCommentForEdit],
368-
);
359+
const handleEditDraft = useEditDraftHandler(fileDrafts, openCommentForEdit);
369360

370361
const prAnnotations = useMemo(
371362
() =>
Lines changed: 11 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -1,142 +1,47 @@
11
import { sendPromptToAgent } from "@features/sessions/utils/sendPromptToAgent";
2-
import {
3-
CaretDown,
4-
CaretUp,
5-
PaperPlaneTilt,
6-
Trash,
7-
} from "@phosphor-icons/react";
8-
import { Badge, Button, Flex, IconButton, Text } from "@radix-ui/themes";
9-
import { useState } from "react";
10-
import {
11-
type DraftComment,
12-
useReviewDraftsStore,
13-
} from "../stores/reviewDraftsStore";
14-
import { useReviewNavigationStore } from "../stores/reviewNavigationStore";
2+
import { PaperPlaneTilt } from "@phosphor-icons/react";
3+
import { Button } from "@posthog/quill";
4+
import { Badge, Flex } from "@radix-ui/themes";
5+
import { useReviewDraftsStore } from "../stores/reviewDraftsStore";
156
import { buildBatchedInlineCommentsPrompt } from "../utils/reviewPrompts";
167

178
interface PendingReviewBarProps {
189
taskId: string;
1910
}
2011

21-
function formatLineRef(d: DraftComment): string {
22-
const sideLabel = d.side === "deletions" ? "old" : "new";
23-
return d.startLine === d.endLine
24-
? `L${d.startLine} ${sideLabel}`
25-
: `L${d.startLine}-${d.endLine} ${sideLabel}`;
26-
}
27-
2812
export function PendingReviewBar({ taskId }: PendingReviewBarProps) {
2913
const drafts = useReviewDraftsStore((s) => s.drafts[taskId] ?? []);
3014
const clearDrafts = useReviewDraftsStore((s) => s.clearDrafts);
31-
const removeDraft = useReviewDraftsStore((s) => s.removeDraft);
32-
const requestScrollToFile = useReviewNavigationStore(
33-
(s) => s.requestScrollToFile,
34-
);
35-
36-
const [collapsed, setCollapsed] = useState(false);
3715

3816
if (drafts.length === 0) return null;
3917

4018
const handleSend = () => {
4119
const prompt = buildBatchedInlineCommentsPrompt(drafts);
4220
if (!prompt) return;
43-
clearDrafts(taskId);
4421
sendPromptToAgent(taskId, prompt);
22+
clearDrafts(taskId);
4523
};
4624

47-
const countLabel = `${drafts.length} pending comment${drafts.length === 1 ? "" : "s"}`;
48-
4925
return (
5026
<div className="shrink-0 border-(--gray-5) border-t bg-(--gray-2)">
5127
<Flex align="center" justify="between" gap="3" className="px-3 py-2">
52-
<Flex align="center" gap="2">
53-
<IconButton
54-
size="1"
55-
variant="ghost"
56-
color="gray"
57-
onClick={() => setCollapsed((v) => !v)}
58-
aria-label={
59-
collapsed ? "Expand pending review" : "Collapse pending review"
60-
}
61-
>
62-
{collapsed ? <CaretUp size={12} /> : <CaretDown size={12} />}
63-
</IconButton>
64-
<Badge color="iris" variant="soft" size="1">
65-
Pending review
66-
</Badge>
67-
<Text size="1" color="gray">
68-
{countLabel}
69-
</Text>
70-
</Flex>
28+
<Badge color="iris" variant="soft" size="1">
29+
Pending review · {drafts.length}
30+
</Badge>
7131
<Flex align="center" gap="2">
7232
<Button
73-
size="1"
74-
variant="ghost"
75-
color="gray"
33+
size="sm"
34+
variant="outline"
7635
onClick={() => clearDrafts(taskId)}
7736
>
7837
Discard all
7938
</Button>
80-
<Button size="1" onClick={handleSend}>
39+
<Button size="sm" variant="primary" onClick={handleSend}>
8140
<PaperPlaneTilt size={12} weight="fill" />
8241
Send to agent
8342
</Button>
8443
</Flex>
8544
</Flex>
86-
{!collapsed && (
87-
<div className="max-h-48 overflow-auto border-(--gray-5) border-t">
88-
{drafts.map((d) => (
89-
<Flex
90-
key={d.id}
91-
align="start"
92-
justify="between"
93-
gap="2"
94-
className="border-(--gray-4) border-b px-3 py-1.5 last:border-b-0"
95-
>
96-
<button
97-
type="button"
98-
onClick={() => requestScrollToFile(taskId, d.filePath)}
99-
className="min-w-0 flex-1 cursor-pointer border-0 bg-transparent p-0 text-left"
100-
>
101-
<Flex align="center" gap="2" className="mb-0.5">
102-
<Text
103-
size="1"
104-
weight="medium"
105-
className="truncate text-(--gray-12)"
106-
title={d.filePath}
107-
>
108-
{d.filePath}
109-
</Text>
110-
<Text
111-
size="1"
112-
color="gray"
113-
className="shrink-0 font-mono text-[11px]"
114-
>
115-
{formatLineRef(d)}
116-
</Text>
117-
</Flex>
118-
<Text
119-
as="div"
120-
size="1"
121-
color="gray"
122-
className="line-clamp-2 whitespace-pre-wrap"
123-
>
124-
{d.text}
125-
</Text>
126-
</button>
127-
<IconButton
128-
size="1"
129-
variant="ghost"
130-
color="gray"
131-
onClick={() => removeDraft(taskId, d.id)}
132-
aria-label="Delete draft comment"
133-
>
134-
<Trash size={12} />
135-
</IconButton>
136-
</Flex>
137-
))}
138-
</div>
139-
)}
14045
</div>
14146
);
14247
}

apps/code/src/renderer/features/code-review/components/ReviewShell.test.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ vi.mock("@pierre/diffs/worker/worker.js?worker&url", () => ({ default: "" }));
2424
vi.mock("@components/ui/FileIcon", () => ({
2525
FileIcon: () => <span data-testid="file-icon" />,
2626
}));
27+
vi.mock("@renderer/trpc/client", () => ({
28+
trpcClient: {},
29+
useTRPC: vi.fn(),
30+
}));
31+
vi.mock("@features/sessions/service/service", () => ({
32+
getSessionService: vi.fn(),
33+
}));
2734

2835
import { DeferredDiffPlaceholder, DiffFileHeader } from "./ReviewShell";
2936

apps/code/src/renderer/features/code-review/utils/reviewPrompts.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,15 @@ export function buildBatchedInlineCommentsPrompt(
5353
);
5454
}
5555
const items = drafts
56-
.map((d, idx) => {
56+
.map((d) => {
5757
const lineRef = formatLineRef(d.startLine, d.endLine);
5858
const sideLabel = formatSideLabel(d.side);
5959
const escapedPath = escapeXmlAttr(d.filePath);
6060
const indented = d.text
6161
.split("\n")
62-
.map((line) => ` ${line}`)
62+
.map((line) => ` ${line}`)
6363
.join("\n");
64-
return `${idx + 1}. In file <file path="${escapedPath}" />, ${lineRef} (${sideLabel}):\n${indented}`;
64+
return `- In file <file path="${escapedPath}" />, ${lineRef} (${sideLabel}):\n${indented}`;
6565
})
6666
.join("\n\n");
6767
return `Please address these review comments:\n\n${items}`;

0 commit comments

Comments
 (0)