Skip to content

Commit 0cae5d3

Browse files
committed
fix(code-review): stabilize comment filtering
Generated-By: PostHog Code Task-Id: 199c1147-fca6-415b-9ea1-00a45d9b7267
1 parent e1091a8 commit 0cae5d3

8 files changed

Lines changed: 83 additions & 35 deletions

File tree

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
} from "../commentFileFilter";
1414
import { useReviewNavigationStore } from "../reviewNavigationStore";
1515
import { PatchedFileDiff } from "./PatchedFileDiff";
16-
import { buildItemIndex, ReviewShell, useReviewState } from "./ReviewShell";
16+
import { ReviewShell, useReviewState } from "./ReviewShell";
1717
import { changedFileSignature } from "./reviewItemBuilders";
1818

1919
interface CloudReviewPageProps {
@@ -35,12 +35,15 @@ export function CloudReviewPage({ task }: CloudReviewPageProps) {
3535
toolCalls,
3636
isLoading,
3737
} = useCloudChangedFiles(taskId, task, isReviewOpen);
38-
const { commentThreads } = usePrDetails(prUrl, {
39-
includeComments: isReviewOpen,
38+
const { commentThreads, commentsLoading } = usePrDetails(prUrl, {
39+
includeComments: isReviewOpen && showReviewComments,
4040
});
4141
const commentedFilePaths = useMemo(
42-
() => (prUrl ? getCommentedFilePaths(commentThreads) : undefined),
43-
[commentThreads, prUrl],
42+
() =>
43+
prUrl && !commentsLoading
44+
? getCommentedFilePaths(commentThreads)
45+
: undefined,
46+
[commentThreads, commentsLoading, prUrl],
4447
);
4548

4649
const allPaths = useMemo(() => reviewFiles.map((f) => f.path), [reviewFiles]);
@@ -117,8 +120,6 @@ export function CloudReviewPage({ task }: CloudReviewPageProps) {
117120
toolCallFallbacks,
118121
]);
119122

120-
const itemIndexByFilePath = useMemo(() => buildItemIndex(items), [items]);
121-
122123
if (!prUrl && !effectiveBranch && reviewFiles.length === 0) {
123124
if (isRunActive) {
124125
return (
@@ -152,7 +153,6 @@ export function CloudReviewPage({ task }: CloudReviewPageProps) {
152153
onUncollapseFile={uncollapseFile}
153154
onCollapseFiles={collapseFiles}
154155
items={items}
155-
itemIndexByFilePath={itemIndexByFilePath}
156156
commentedFilePaths={commentedFilePaths?.all}
157157
unresolvedCommentedFilePaths={commentedFilePaths?.unresolved}
158158
currentSignatures={currentSignatures}

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

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import { useEffectiveDiffSource } from "../hooks/useEffectiveDiffSource";
2727
import { useReviewDiffs } from "../hooks/useReviewDiffs";
2828
import { useReviewNavigationStore } from "../reviewNavigationStore";
2929
import type { DiffOptions } from "../types";
30-
import { buildItemIndex, ReviewShell, useReviewState } from "./ReviewShell";
30+
import { ReviewShell, useReviewState } from "./ReviewShell";
3131
import {
3232
buildPatchReviewItems,
3333
buildRemoteReviewItems,
@@ -108,15 +108,18 @@ export function ReviewPage({ task }: ReviewPageProps) {
108108
} = useEffectiveDiffSource(taskId);
109109

110110
const showReviewComments = useDiffViewerStore((s) => s.showReviewComments);
111-
const { commentThreads } = usePrDetails(prUrl, {
112-
includeComments: isReviewOpen,
111+
const { commentThreads, commentsLoading } = usePrDetails(prUrl, {
112+
includeComments: isReviewOpen && showReviewComments,
113113
});
114114
const effectiveCommentThreads = showReviewComments
115115
? commentThreads
116116
: undefined;
117117
const commentedFilePaths = useMemo(
118-
() => (prUrl ? getCommentedFilePaths(commentThreads) : undefined),
119-
[commentThreads, prUrl],
118+
() =>
119+
prUrl && !commentsLoading
120+
? getCommentedFilePaths(commentThreads)
121+
: undefined,
122+
[commentThreads, commentsLoading, prUrl],
120123
);
121124

122125
const isLocalActive = isReviewOpen && effectiveSource === "local";
@@ -425,8 +428,6 @@ function LocalReviewContent({
425428
unstagedParsedFiles,
426429
]);
427430

428-
const itemIndexByFilePath = useMemo(() => buildItemIndex(items), [items]);
429-
430431
return (
431432
<ReviewShell
432433
task={task}
@@ -447,7 +448,6 @@ function LocalReviewContent({
447448
prSourceAvailable={prSourceAvailable}
448449
defaultBranch={defaultBranch}
449450
items={items}
450-
itemIndexByFilePath={itemIndexByFilePath}
451451
commentedFilePaths={commentedFilePaths}
452452
unresolvedCommentedFilePaths={unresolvedCommentedFilePaths}
453453
currentSignatures={currentSignatures}
@@ -543,8 +543,6 @@ function RemoteReviewPage({
543543
taskId,
544544
],
545545
);
546-
const itemIndexByFilePath = useMemo(() => buildItemIndex(items), [items]);
547-
548546
return (
549547
<ReviewShell
550548
task={task}
@@ -564,7 +562,6 @@ function RemoteReviewPage({
564562
prSourceAvailable={prSourceAvailable}
565563
defaultBranch={defaultBranch}
566564
items={items}
567-
itemIndexByFilePath={itemIndexByFilePath}
568565
commentedFilePaths={commentedFilePaths}
569566
unresolvedCommentedFilePaths={unresolvedCommentedFilePaths}
570567
currentSignatures={currentSignatures}

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

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import {
1515
} from "react";
1616
import { VList, type VListHandle } from "virtua";
1717
import {
18-
type CommentFileFilter,
1918
deriveCommentFileFilterState,
2019
getEmptyReviewMessage,
2120
type ReviewListItem,
@@ -126,7 +125,6 @@ export function ReviewShell({
126125
isLoading,
127126
isEmpty,
128127
items,
129-
itemIndexByFilePath,
130128
commentedFilePaths,
131129
unresolvedCommentedFilePaths,
132130
currentSignatures,
@@ -151,7 +149,12 @@ export function ReviewShell({
151149
const lastActiveRef = useRef<string | null>(null);
152150
const pendingNavigationRef = useRef<string | null>(null);
153151
const navigationFrameRef = useRef<number | null>(null);
154-
const [commentFilter, setCommentFilter] = useState<CommentFileFilter>("none");
152+
const commentFilter = useReviewNavigationStore(
153+
(state) => state.commentFileFilters[taskId] ?? "none",
154+
);
155+
const setCommentFileFilter = useReviewNavigationStore(
156+
(state) => state.setCommentFileFilter,
157+
);
155158
const {
156159
activeFilter: activeCommentFilter,
157160
visibleItems,
@@ -265,15 +268,7 @@ export function ReviewShell({
265268
useEffect(() => {
266269
if (!scrollRequest) return;
267270
const targetIndex = visibleItemIndexByFilePath.get(scrollRequest);
268-
if (targetIndex === undefined) {
269-
if (
270-
activeCommentFilter !== "none" &&
271-
itemIndexByFilePath.has(scrollRequest)
272-
) {
273-
setCommentFilter("none");
274-
}
275-
return;
276-
}
271+
if (targetIndex === undefined) return;
277272

278273
const currentSignature = currentSignatures.get(scrollRequest);
279274
const viewed =
@@ -313,11 +308,9 @@ export function ReviewShell({
313308
}, [
314309
clearScrollRequest,
315310
currentSignatures,
316-
itemIndexByFilePath,
317311
onUncollapseFile,
318312
scrollRequest,
319313
setActiveFilePath,
320-
activeCommentFilter,
321314
taskId,
322315
visibleItemIndexByFilePath,
323316
viewedRecord,
@@ -417,7 +410,7 @@ export function ReviewShell({
417410
commentFilter={activeCommentFilter}
418411
onCommentFilterChange={
419412
commentedFilePaths && unresolvedCommentedFilePaths
420-
? setCommentFilter
413+
? (filter) => setCommentFileFilter(taskId, filter)
421414
: undefined
422415
}
423416
linesAdded={linesAdded}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { beforeEach, describe, expect, it } from "vitest";
2+
import { useReviewNavigationStore } from "./reviewNavigationStore";
3+
4+
describe("reviewNavigationStore", () => {
5+
beforeEach(() => {
6+
useReviewNavigationStore.setState({
7+
activeFilePaths: {},
8+
scrollRequests: {},
9+
reviewModes: {},
10+
commentFileFilters: {},
11+
});
12+
});
13+
14+
it("clears the comment filter when navigating to a file", () => {
15+
const store = useReviewNavigationStore.getState();
16+
store.setCommentFileFilter("task-1", "unresolved");
17+
18+
store.requestScrollToFile("task-1", "src/example.ts");
19+
20+
const state = useReviewNavigationStore.getState();
21+
expect(state.scrollRequests["task-1"]).toBe("src/example.ts");
22+
expect(state.commentFileFilters["task-1"]).toBe("none");
23+
});
24+
});

packages/ui/src/features/code-review/reviewNavigationStore.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import { create } from "zustand";
2+
import type { CommentFileFilter } from "./commentFileFilter";
23

34
export type ReviewMode = "closed" | "split" | "expanded";
45

56
interface ReviewNavigationStoreState {
67
activeFilePaths: Record<string, string | null>;
78
scrollRequests: Record<string, string | null>;
89
reviewModes: Record<string, ReviewMode>;
10+
commentFileFilters: Record<string, CommentFileFilter>;
911
}
1012

1113
interface ReviewNavigationStoreActions {
@@ -14,6 +16,7 @@ interface ReviewNavigationStoreActions {
1416
clearScrollRequest: (taskId: string) => void;
1517
clearTask: (taskId: string) => void;
1618
setReviewMode: (taskId: string, mode: ReviewMode) => void;
19+
setCommentFileFilter: (taskId: string, filter: CommentFileFilter) => void;
1720
getReviewMode: (taskId: string) => ReviewMode;
1821
}
1922

@@ -25,6 +28,7 @@ export const useReviewNavigationStore = create<ReviewNavigationStore>()(
2528
activeFilePaths: {},
2629
scrollRequests: {},
2730
reviewModes: {},
31+
commentFileFilters: {},
2832

2933
setActiveFilePath: (taskId, path) =>
3034
set((state) => ({
@@ -34,6 +38,10 @@ export const useReviewNavigationStore = create<ReviewNavigationStore>()(
3438
requestScrollToFile: (taskId, path) =>
3539
set((state) => ({
3640
scrollRequests: { ...state.scrollRequests, [taskId]: path },
41+
commentFileFilters: {
42+
...state.commentFileFilters,
43+
[taskId]: "none",
44+
},
3745
})),
3846

3947
clearScrollRequest: (taskId) =>
@@ -45,13 +53,25 @@ export const useReviewNavigationStore = create<ReviewNavigationStore>()(
4553
set((state) => ({
4654
activeFilePaths: { ...state.activeFilePaths, [taskId]: null },
4755
scrollRequests: { ...state.scrollRequests, [taskId]: null },
56+
commentFileFilters: {
57+
...state.commentFileFilters,
58+
[taskId]: "none",
59+
},
4860
})),
4961

5062
setReviewMode: (taskId, mode) =>
5163
set((state) => ({
5264
reviewModes: { ...state.reviewModes, [taskId]: mode },
5365
})),
5466

67+
setCommentFileFilter: (taskId, filter) =>
68+
set((state) => ({
69+
commentFileFilters: {
70+
...state.commentFileFilters,
71+
[taskId]: filter,
72+
},
73+
})),
74+
5575
getReviewMode: (taskId) => get().reviewModes[taskId] ?? "closed",
5676
}),
5777
);

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,4 +280,18 @@ describe("commented file filtering", () => {
280280
expect(state.commentedFileCount).toBe(2);
281281
expect(state.unresolvedCommentedFileCount).toBe(1);
282282
});
283+
284+
it("keeps all files visible while comment paths are loading", () => {
285+
const items: ReviewListItem[] = [
286+
{ key: "a.ts", filePaths: ["a.ts"], node: <span>A</span> },
287+
];
288+
289+
const state = deriveCommentFileFilterState({
290+
items,
291+
requestedFilter: "unresolved",
292+
});
293+
294+
expect(state.activeFilter).toBe("none");
295+
expect(state.visibleItems).toBe(items);
296+
});
283297
});

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,6 @@ export interface ReviewShellProps {
209209
isLoading: boolean;
210210
isEmpty: boolean;
211211
items: ReviewListItem[];
212-
itemIndexByFilePath: Map<string, number>;
213212
commentedFilePaths?: ReadonlySet<string>;
214213
unresolvedCommentedFilePaths?: ReadonlySet<string>;
215214
currentSignatures: Map<string, string>;

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,5 +85,6 @@ export function usePrDetails(
8585
isLoading: metaQuery.isLoading,
8686
},
8787
commentThreads,
88+
commentsLoading: commentsQuery.isLoading,
8889
};
8990
}

0 commit comments

Comments
 (0)