authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-10-09 16:40:10-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-10-09 16:40:10-07:00
log6cd02e98f7edd78b7bde9ceff26c829476dbeb77
tree1db2ddc8dec2022ddfdda8e0acc1abaa18b57644
parente30852213ed3954037091582057f71918918e19f
signature Signed by PGP key B5690EEEBB952194

feat(web): filter PR comments by bots and resolved threads (#17645)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

2 files changed, 205 insertions(+), 269 deletions(-)

apps/web/src/components/pullRequest/PullRequestSummaryTab.test.tsx+89-58
......@@ -9,6 +9,14 @@ vi.mock("~/browser/useOpenLink", () => ({ useOpenLink: () => vi.fn() }));
99vi.mock("./PullRequestMarkdown", () => ({
1010 PullRequestMarkdown: ({ text }: { text: string }) => <p>{text}</p>,
1111}));
12vi.mock("../ui/menu", () => ({
13 Menu: "div",
14 MenuCheckboxItem: "div",
15 MenuGroup: "div",
16 MenuGroupLabel: "div",
17 MenuPopup: "div",
18 MenuTrigger: "div",
19}));
1220vi.mock("../ui/tooltip", () => ({
1321 Tooltip: ({ children }: { children: ReactNode }) => children,
1422 TooltipTrigger: ({ children }: { children: ReactNode }) => children,
......@@ -70,7 +78,20 @@ const detail: PullRequestDetailView = {
7078};
7179
7280let renderer: ReactTestRenderer;
73beforeEach(() => vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true));
81beforeEach(() => {
82 vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true);
83 const store = new Map<string, string>();
84 vi.stubGlobal(
85 "window",
86 Object.assign(new EventTarget(), {
87 localStorage: {
88 getItem: (key: string) => store.get(key) ?? null,
89 setItem: (key: string, value: string) => store.set(key, value),
90 removeItem: (key: string) => store.delete(key),
91 },
92 }),
93 );
94});
7495afterEach(() => {
7596 act(() => renderer?.unmount());
7697 vi.unstubAllGlobals();
......@@ -145,70 +166,80 @@ it("keeps an unsaved description when collapsed and reopened", () => {
145166 expect(renderer.root.findByType("textarea").props.value).toBe("Unsaved description");
146167});
147168
148it("opens bot reports in pages without hiding human comments", () => {
169function texts() {
170 return renderer.root
171 .findAllByType("p")
172 .map((p) => p.children.join(""))
173 .filter((text) => text !== detail.body);
174}
175
176function filterItem(label: string) {
177 return renderer.root.find(
178 (node) => typeof node.props.onCheckedChange === "function" && node.children.includes(label),
179 );
180}
181
182it("hides bot and resolved comments until the filter shows them, and remembers the choice", () => {
183 const comment = (index: number, login: string, body: string, isBot = false) => ({
184 id: `comment-${index}`,
185 kind: "issue-comment" as const,
186 author: { login, name: null, avatarUrl: null, isBot },
187 body,
188 createdAt: `2026-09-01T00:00:${String(index).padStart(2, "0")}Z`,
189 url: null,
190 path: null,
191 reviewState: null,
192 });
193 const resolved = {
194 ...comment(13, "reviewer", "Resolved remark"),
195 kind: "review-comment" as const,
196 };
149197 const value: PullRequestDetailView = {
150198 ...detail,
151 commentCount: 13,
152 comments: Array.from({ length: 13 }, (_, index) => ({
153 id: `comment-${index}`,
154 kind: "issue-comment",
155 author: {
156 login: index === 12 ? "human" : "review-app",
157 name: null,
158 avatarUrl: null,
159 isBot: index !== 12,
199 commentCount: 14,
200 comments: [
201 ...Array.from({ length: 12 }, (_, index) =>
202 comment(index, "review-app", `Bot report ${index}`, true),
203 ),
204 comment(12, "human", "Human comment"),
205 resolved,
206 ],
207 reviewThreads: [
208 {
209 id: "thread",
210 path: "src/index.ts",
211 line: 1,
212 side: "right",
213 isResolved: true,
214 isOutdated: false,
215 comments: [resolved],
160216 },
161 body: index === 12 ? "Human comment" : `Bot report ${index}`,
162 createdAt: `2026-09-01T00:00:${String(index).padStart(2, "0")}Z`,
163 url: null,
164 path: null,
165 reviewState: null,
166 })),
217 ],
167218 };
168219 act(() => {
169220 renderer = create(render(value));
170221 });
171 expect(renderer.root.findAllByType("p").map((p) => p.children.join(""))).toContain(
172 "Human comment",
173 );
174 expect(
175 renderer.root.findAllByType("p").some((p) => p.children.join("").startsWith("Bot report")),
176 ).toBe(false);
177 const group = renderer.root
178 .findAllByType("button")
179 .find((button) => button.props["aria-label"] === "12 bot comments")!;
180 act(() => group.props.onClick({ nativeEvent: {}, preventDefault() {}, stopPropagation() {} }));
222 expect(texts()).toEqual(["Human comment"]);
181223 expect(
182 renderer.root.findAllByType("p").filter((p) => p.children.join("").startsWith("Bot report")),
183 ).toHaveLength(10);
184 expect(renderer.root.findAllByType("p").map((p) => p.children.join(""))).not.toContain(
185 "Bot report 0",
186 );
187 act(() =>
188 renderer.root
189 .findAllByType("button")
190 .find((button) => button.children.includes(" older bot comment"))!
191 .props.onClick(),
192 );
193 expect(
194 renderer.root.findAllByType("p").filter((p) => p.children.join("").startsWith("Bot report")),
195 ).toHaveLength(12);
196 act(() => group.props.onClick({ nativeEvent: {}, preventDefault() {}, stopPropagation() {} }));
197 act(() => group.props.onClick({ nativeEvent: {}, preventDefault() {}, stopPropagation() {} }));
198 expect(renderer.root.findAllByType("p").map((p) => p.children.join(""))).toContain(
199 "Bot report 0",
200 );
201 act(() =>
202 renderer.root
203 .findAllByType("button")
204 .find((button) => button.children.includes(" recent bot comments"))!
205 .props.onClick(),
206 );
207 expect(
208 renderer.root.findAllByType("p").filter((p) => p.children.join("").startsWith("Bot report")),
209 ).toHaveLength(10);
210 act(() => renderer.update(render({ ...value, url: `${value.url}0`, number: 10 })));
224 renderer.root.findByProps({ "aria-label": "Filter comments" }).children.join(""),
225 ).toContain("13 hidden");
226
227 act(() => filterItem("Bot comments").props.onCheckedChange(true));
228 // Bots now share the human comment's pages: the ten most recent, the oldest left behind.
229 expect(texts().filter((text) => text.startsWith("Bot report"))).toHaveLength(9);
230 expect(texts()).toContain("Human comment");
231 expect(texts()).not.toContain("Bot report 0");
232
233 act(() => filterItem("Resolved or dismissed").props.onCheckedChange(true));
211234 expect(
212 renderer.root.findAllByType("p").some((p) => p.children.join("").startsWith("Bot report")),
213 ).toBe(false);
235 renderer.root.findAllByType("button").some((button) => button.children.includes("Resolved")),
236 ).toBe(true);
237
238 // Another pull request, or the same one opened again, keeps the reader's choice.
239 act(() => renderer.unmount());
240 act(() => {
241 renderer = create(render({ ...value, url: `${value.url}0`, number: 10 }));
242 });
243 expect(filterItem("Bot comments").props.checked).toBe(true);
244 expect(texts()).toContain("Bot report 11");
214245});
apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx+116-211
......@@ -12,12 +12,15 @@ import {
1212 ChevronDownIcon,
1313 ChevronRightIcon,
1414 HammerIcon,
15 ListFilterIcon,
1516 TagIcon,
1617 UsersIcon,
1718} from "lucide-react";
19import * as Schema from "effect/Schema";
1820import { useRef, useState, type ReactNode } from "react";
1921
2022import { pullRequestEnvironment } from "~/state/pullRequests";
23import { useLocalStorage } from "~/hooks/useLocalStorage";
2124import { cn } from "~/lib/utils";
2225import { useOpenLink } from "~/browser/useOpenLink";
2326import { formatRelativeTimeLabel } from "~/timestampFormat";
......@@ -25,6 +28,14 @@ import { formatRelativeTimeLabel } from "~/timestampFormat";
2528import { Button } from "../ui/button";
2629import { PullRequestEditButton } from "./PullRequestEditButton";
2730import { Collapsible, CollapsiblePanel, CollapsibleTrigger } from "../ui/collapsible";
31import {
32 Menu,
33 MenuCheckboxItem,
34 MenuGroup,
35 MenuGroupLabel,
36 MenuPopup,
37 MenuTrigger,
38} from "../ui/menu";
2839import { toastManager } from "../ui/toast";
2940import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip";
3041import {
......@@ -349,111 +360,21 @@ function Section({
349360 );
350361}
351362
352function CommentGroup({
353 label,
354 comments,
355 detail,
356 children,
357 onOpenChange,
358}: {
359 label: string;
360 comments: readonly PullRequestComment[];
361 detail: PullRequestDetailView;
362 children: ReactNode;
363 onOpenChange?: (open: boolean) => void;
364}) {
365 const authors = [
366 ...new Map(
367 comments.map((comment) => [reviewerKey(comment.author?.login ?? "ghost"), comment.author]),
368 ).values(),
369 ];
370 const fileCount = new Set(comments.flatMap((comment) => (comment.path ? [comment.path] : [])))
371 .size;
372 const latest = comments.reduce<string | null>(
373 (date, comment) => (date === null || comment.createdAt > date ? comment.createdAt : date),
374 null,
375 );
376 return (
377 <div className="overflow-hidden rounded-lg border border-border bg-background">
378 <Collapsible onOpenChange={onOpenChange}>
379 <div className="flex items-center gap-3 pl-3">
380 <div className="flex shrink-0 -space-x-1.5">
381 {authors.slice(0, 3).map((actor) => (
382 // The ring separates the overlapping faces; it belongs to the stack, not the actor.
383 <span
384 key={actor?.login ?? "ghost"}
385 className="relative flex rounded-full ring-2 ring-background hover:z-10 focus-within:z-10"
386 >
387 <PullRequestActorLabel
388 actor={actor}
389 profileUrl={
390 detail.provider === "github" && actor
391 ? new URL(
392 actor.isBot || actor.login.endsWith("[bot]")
393 ? `/apps/${encodeURIComponent(actor.login.replace(/\[bot\]$/, ""))}`
394 : `/${encodeURIComponent(actor.login)}`,
395 detail.url,
396 ).toString()
397 : null
398 }
399 variant="avatar"
400 />
401 </span>
402 ))}
403 {authors.length > 3 ? (
404 <span className="relative flex size-6 items-center justify-center rounded-full bg-muted text-3xs text-muted-foreground ring-2 ring-background">
405 +{authors.length - 3}
406 </span>
407 ) : null}
408 </div>
409 <CollapsibleTrigger
410 aria-label={label}
411 className="group flex min-w-0 flex-1 items-center gap-3 rounded-md py-3 pr-3 text-left hover:bg-muted/30"
412 >
413 <span className="min-w-0 flex-1 space-y-1">
414 <span className="block text-xs font-medium text-foreground/90">{label}</span>
415 <span className="flex flex-wrap gap-x-1.5 text-2xs text-muted-foreground">
416 <span>
417 {authors.length} {authors.length === 1 ? "author" : "authors"}
418 </span>
419 {fileCount > 0 ? (
420 <span>
421 · {fileCount} {fileCount === 1 ? "file" : "files"}
422 </span>
423 ) : null}
424 {latest ? (
425 <span>
426 · Latest{" "}
427 <Tooltip>
428 <TooltipTrigger render={<time dateTime={latest} />}>
429 {formatRelativeTimeLabel(latest)}
430 </TooltipTrigger>
431 <TooltipPopup>{new Date(latest).toLocaleString()}</TooltipPopup>
432 </Tooltip>
433 </span>
434 ) : null}
435 </span>
436 </span>
437 <ChevronRightIcon
438 aria-hidden
439 className="size-3.5 shrink-0 text-muted-foreground transition-transform group-data-panel-open:rotate-90"
440 />
441 </CollapsibleTrigger>
442 </div>
443 <CollapsiblePanel keepMounted>
444 <div className="border-t border-border/60 px-3 pb-3">{children}</div>
445 </CollapsiblePanel>
446 </Collapsible>
447 </div>
448 );
449}
450
451363/**
452364 * What a first render of the conversation carries. A pull request with two hundred comments is
453365 * two hundred markdown documents, and the ones worth arriving for are the recent ones.
454366 */
455367const COMMENT_PAGE = 10;
456368
369/**
370 * Which kinds of comment the conversation shows. Kept per browser rather than per pull request:
371 * whether a reader wants bot reports and finished threads in front of them is a habit, not a
372 * property of one change.
373 */
374const COMMENT_FILTER_STORAGE_KEY = "t3code:pull-request-comment-filter:v1";
375const CommentFilterSchema = Schema.Struct({ bots: Schema.Boolean, resolved: Schema.Boolean });
376const DEFAULT_COMMENT_FILTER: typeof CommentFilterSchema.Type = { bots: false, resolved: false };
377
457378export function PullRequestSummaryTab({
458379 environmentId,
459380 threadRef,
......@@ -487,10 +408,12 @@ export function PullRequestSummaryTab({
487408 // Keyed by the pull request, so opening another one starts at the end of its conversation
488409 // rather than wherever the last one had been read back to.
489410 const [shown, setShown] = useState({ url: detail.url, count: COMMENT_PAGE });
490 const [openedBotGroup, setOpenedBotGroup] = useState<string | null>(null);
491 const [shownBots, setShownBots] = useState({ url: detail.url, count: COMMENT_PAGE });
492 const shownBotComments = shownBots.url === detail.url ? shownBots.count : COMMENT_PAGE;
493411 const shownComments = shown.url === detail.url ? shown.count : COMMENT_PAGE;
412 const [commentFilter, setCommentFilter] = useLocalStorage(
413 COMMENT_FILTER_STORAGE_KEY,
414 DEFAULT_COMMENT_FILTER,
415 CommentFilterSchema,
416 );
494417 // A comment that already lives on a review thread is that thread: the thread carries the line
495418 // and side the bare comment has lost, and a resolved one is finished work nobody should be
496419 // invited to fix again — the same call the whole-review hand-off makes.
......@@ -500,22 +423,24 @@ export function PullRequestSummaryTab({
500423 ),
501424 );
502425
503 const activeComments: PullRequestComment[] = [];
504 const finishedComments: PullRequestComment[] = [];
505 const botComments: PullRequestComment[] = [];
506 for (const comment of detail.comments) {
507 const finished =
508 threadByCommentId.get(comment.id)?.isResolved ||
509 pullRequestReviewOutcome(comment.reviewState) === "dismissed";
510 const bot = comment.author?.isBot === true || comment.author?.login.endsWith("[bot]");
511 (finished ? finishedComments : bot ? botComments : activeComments).push(comment);
512 }
426 const isFinished = (comment: PullRequestComment) =>
427 threadByCommentId.get(comment.id)?.isResolved === true ||
428 pullRequestReviewOutcome(comment.reviewState) === "dismissed";
429 const isBot = (comment: PullRequestComment) =>
430 comment.author?.isBot === true || comment.author?.login.endsWith("[bot]") === true;
431 const filteredComments = detail.comments.filter(
432 (comment) =>
433 (commentFilter.bots || !isBot(comment)) && (commentFilter.resolved || !isFinished(comment)),
434 );
435 // The one count the filter shows. Per-kind counts would overlap, since a resolved bot comment
436 // is both kinds and needs both shown.
437 const filteredOutCount = detail.comments.length - filteredComments.length;
513438 // Windowed by recency regardless of display order: expanding always reaches further back in
514439 // time, whether the newest comment currently reads first or last.
515 const recentComments = activeComments.slice(Math.max(0, activeComments.length - shownComments));
516 const hiddenCommentCount = activeComments.length - recentComments.length;
517 const recentBotComments = botComments.slice(Math.max(0, botComments.length - shownBotComments));
518 const hiddenBotCommentCount = botComments.length - recentBotComments.length;
440 const recentComments = filteredComments.slice(
441 Math.max(0, filteredComments.length - shownComments),
442 );
443 const hiddenCommentCount = filteredComments.length - recentComments.length;
519444 const [commentOrder, setCommentOrder] = useState<"newest" | "oldest">("newest");
520445 const visibleComments = orderPullRequestComments(recentComments, commentOrder);
521446 const showOldestCommentsButton =
......@@ -638,6 +563,30 @@ export function PullRequestSummaryTab({
638563 const renderComment = (comment: PullRequestComment) => {
639564 const thread = threadByCommentId.get(comment.id);
640565 const body = visibleBody(comment.body);
566 if (isFinished(comment)) {
567 return (
568 <CollapsedComment
569 key={`${detail.url}:${comment.id}`}
570 comment={comment}
571 editing={commentEditing}
572 detail={detail}
573 thread={thread}
574 label={thread?.isResolved ? "Resolved" : "Review dismissed"}
575 body={body}
576 reactionBar={
577 <PullRequestReactionBar
578 className="ml-auto justify-end"
579 reactions={comment.reactions ?? []}
580 canReact={detail.capabilities.reactions === true}
581 subjectId={comment.id}
582 environmentId={environmentId}
583 reference={reference}
584 onRefresh={onRefresh}
585 />
586 }
587 />
588 );
589 }
641590 const outcome = pullRequestReviewOutcome(comment.reviewState);
642591 // An approval is a verdict, not a finding: there is nothing in it to fix.
643592 const finding: PullRequestFinding | null =
......@@ -920,20 +869,51 @@ export function PullRequestSummaryTab({
920869 <Section
921870 title={`Comments (${detail.commentCount})`}
922871 actions={
923 <Button
924 size="xs"
925 variant="ghost-muted"
926 className="shrink-0"
927 aria-label={
928 commentOrder === "newest"
929 ? "Show oldest comments first"
930 : "Show newest comments first"
931 }
932 onClick={() => setCommentOrder((value) => (value === "newest" ? "oldest" : "newest"))}
933 >
934 <ArrowDownUpIcon aria-hidden className="size-3" />
935 {commentOrder === "newest" ? "Newest first" : "Oldest first"}
936 </Button>
872 <div className="flex shrink-0 items-center gap-1">
873 <Menu>
874 <MenuTrigger
875 render={<Button size="xs" variant="ghost-muted" />}
876 aria-label="Filter comments"
877 >
878 <ListFilterIcon aria-hidden className="size-3" />
879 {filteredOutCount > 0 ? `${filteredOutCount} hidden` : "Filter"}
880 </MenuTrigger>
881 <MenuPopup align="end">
882 <MenuGroup>
883 <MenuGroupLabel>Show</MenuGroupLabel>
884 <MenuCheckboxItem
885 checked={commentFilter.bots}
886 closeOnClick={false}
887 onCheckedChange={(bots) => setCommentFilter((value) => ({ ...value, bots }))}
888 >
889 Bot comments
890 </MenuCheckboxItem>
891 <MenuCheckboxItem
892 checked={commentFilter.resolved}
893 closeOnClick={false}
894 onCheckedChange={(resolved) =>
895 setCommentFilter((value) => ({ ...value, resolved }))
896 }
897 >
898 Resolved or dismissed
899 </MenuCheckboxItem>
900 </MenuGroup>
901 </MenuPopup>
902 </Menu>
903 <Button
904 size="xs"
905 variant="ghost-muted"
906 aria-label={
907 commentOrder === "newest"
908 ? "Show oldest comments first"
909 : "Show newest comments first"
910 }
911 onClick={() => setCommentOrder((value) => (value === "newest" ? "oldest" : "newest"))}
912 >
913 <ArrowDownUpIcon aria-hidden className="size-3" />
914 {commentOrder === "newest" ? "Newest first" : "Oldest first"}
915 </Button>
916 </div>
937917 }
938918 >
939919 {activityPending ? (
......@@ -950,6 +930,12 @@ export function PullRequestSummaryTab({
950930 ) : null}
951931 {detail.comments.length === 0 ? (
952932 <p className="py-2 text-xs text-muted-foreground">No comments yet.</p>
933 ) : filteredComments.length === 0 ? (
934 <p className="py-2 text-xs text-muted-foreground">
935 {filteredOutCount === 1
936 ? "The only comment is hidden by the filter."
937 : `All ${filteredOutCount} comments are hidden by the filter.`}
938 </p>
953939 ) : (
954940 <div className="space-y-3">
955941 {commentOrder === "oldest" ? showOldestCommentsButton : null}
......@@ -965,87 +951,6 @@ export function PullRequestSummaryTab({
965951 Show only {COMMENT_PAGE} recent comments
966952 </Button>
967953 ) : null}
968 {botComments.length > 0 ? (
969 <CommentGroup
970 key={`bots:${detail.url}`}
971 label={`${botComments.length} bot comment${botComments.length === 1 ? "" : "s"}`}
972 comments={botComments}
973 detail={detail}
974 onOpenChange={(open) => {
975 if (open) setOpenedBotGroup(detail.url);
976 }}
977 >
978 <div className="space-y-3 pt-2">
979 {openedBotGroup === detail.url
980 ? orderPullRequestComments(recentBotComments, commentOrder).map(
981 renderComment,
982 )
983 : null}
984 {hiddenBotCommentCount > 0 ? (
985 <Button
986 size="sm"
987 variant="outline"
988 className="w-full"
989 onClick={() =>
990 setShownBots({
991 url: detail.url,
992 count: shownBotComments + COMMENT_PAGE,
993 })
994 }
995 >
996 Show {Math.min(hiddenBotCommentCount, COMMENT_PAGE)} older bot comment
997 {hiddenBotCommentCount === 1 ? "" : "s"} ({hiddenBotCommentCount} hidden)
998 </Button>
999 ) : null}
1000 {shownBotComments > COMMENT_PAGE ? (
1001 <Button
1002 size="sm"
1003 variant="ghost"
1004 className="w-full"
1005 onClick={() => setShownBots({ url: detail.url, count: COMMENT_PAGE })}
1006 >
1007 Show only {COMMENT_PAGE} recent bot comments
1008 </Button>
1009 ) : null}
1010 </div>
1011 </CommentGroup>
1012 ) : null}
1013 {finishedComments.length > 0 ? (
1014 <CommentGroup
1015 key={detail.url}
1016 label={`${finishedComments.length} resolved or dismissed comment${finishedComments.length === 1 ? "" : "s"}`}
1017 comments={finishedComments}
1018 detail={detail}
1019 >
1020 <div className="space-y-2 pt-2">
1021 {orderPullRequestComments(finishedComments, commentOrder).map((comment) => {
1022 const thread = threadByCommentId.get(comment.id);
1023 return (
1024 <CollapsedComment
1025 key={comment.id}
1026 comment={comment}
1027 editing={commentEditing}
1028 detail={detail}
1029 thread={thread}
1030 label={thread?.isResolved ? "Resolved" : "Review dismissed"}
1031 body={visibleBody(comment.body)}
1032 reactionBar={
1033 <PullRequestReactionBar
1034 className="ml-auto justify-end"
1035 reactions={comment.reactions ?? []}
1036 canReact={detail.capabilities.reactions === true}
1037 subjectId={comment.id}
1038 environmentId={environmentId}
1039 reference={reference}
1040 onRefresh={onRefresh}
1041 />
1042 }
1043 />
1044 );
1045 })}
1046 </div>
1047 </CommentGroup>
1048 ) : null}
1049954 </div>
1050955 )}
1051956 </>