Skip to content

Commit 1702d55

Browse files
t3dotggclaude
andauthored
fix(web,mobile): bot approvals are visible on the PR summary (#25829)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent bc8d02f commit 1702d55

6 files changed

Lines changed: 91 additions & 7 deletions

File tree

‎apps/mobile/src/features/pull-requests/pullRequestActivityFilter.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,4 +57,19 @@ describe("mobile activity filters", () => {
5757
.activity.reviewThreads,
5858
).toEqual([]);
5959
});
60+
61+
it("keeps bot review verdicts when bot comments are hidden", () => {
62+
const result = filterPullRequestActivity(
63+
{
64+
...activity,
65+
comments: [
66+
comment("approved", bot, "APPROVED"),
67+
comment("changes", bot, "CHANGES_REQUESTED"),
68+
comment("commented", bot, "COMMENTED"),
69+
],
70+
},
71+
DEFAULT_ACTIVITY_FILTER,
72+
);
73+
expect(result.activity.comments.map((entry) => entry.id)).toEqual(["approved", "changes"]);
74+
});
6075
});

‎apps/mobile/src/features/pull-requests/pullRequestActivityFilter.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ function isBot(actor: PullRequestActor | null) {
1818
return actor?.isBot === true;
1919
}
2020

21+
// A bot's approval or change request is a verdict, not chatter.
22+
function isVerdict(reviewState: string | null) {
23+
const state = reviewState?.trim().toUpperCase().replaceAll("-", "_");
24+
return state === "APPROVED" || state === "CHANGES_REQUESTED";
25+
}
26+
2127
function eventCount(activity: PullRequestActivity) {
2228
const discussionIds = new Set(
2329
activity.reviewThreads.flatMap((thread) => thread.comments.map((comment) => comment.id)),
@@ -50,7 +56,7 @@ export function filterPullRequestActivity(activity: PullRequestActivity, filter:
5056
: activity.comments.filter(
5157
(comment) =>
5258
!allDiscussionIds.has(comment.id) &&
53-
(filter.bots || !isBot(comment.author)) &&
59+
(filter.bots || !isBot(comment.author) || isVerdict(comment.reviewState)) &&
5460
(filter.resolved || comment.reviewState?.trim().toLowerCase() !== "dismissed"),
5561
);
5662
const filtered = {

‎apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import {
4949
PullRequestLabelChip,
5050
PullRequestReviewOutcomeBadge,
5151
pullRequestReviewOutcomeLabel,
52+
PullRequestReviewOutcomeAvatarBadge,
5253
pullRequestReviewOutcomeRingClassName,
5354
pullRequestReviewOutcomeStaleLabel,
5455
} from "./pullRequestPresentation";
@@ -689,11 +690,17 @@ export function PullRequestSummaryTab({
689690
variant="avatar"
690691
/>
691692
{entry.outcome ? (
692-
<span className="sr-only">
693-
{entry.stale
694-
? pullRequestReviewOutcomeStaleLabel(entry.outcome)
695-
: pullRequestReviewOutcomeLabel(entry.outcome)}
696-
</span>
693+
<>
694+
<PullRequestReviewOutcomeAvatarBadge
695+
outcome={entry.outcome}
696+
stale={entry.stale}
697+
/>
698+
<span className="sr-only">
699+
{entry.stale
700+
? pullRequestReviewOutcomeStaleLabel(entry.outcome)
701+
: pullRequestReviewOutcomeLabel(entry.outcome)}
702+
</span>
703+
</>
697704
) : null}
698705
</TooltipTrigger>
699706
<TooltipPopup side="bottom">

‎apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,28 @@ describe("review thread conversations", () => {
561561
);
562562
});
563563

564+
it("keeps bot review verdicts when bot comments are hidden", () => {
565+
const review = (id: string, reviewState: string) => ({
566+
...comment(id, 9),
567+
author: bot,
568+
kind: "review" as const,
569+
reviewState,
570+
});
571+
const items = pullRequestConversationItems(
572+
[
573+
review("approved", "APPROVED"),
574+
review("changes", "CHANGES_REQUESTED"),
575+
review("commented", "COMMENTED"),
576+
],
577+
[],
578+
);
579+
expect(
580+
filterPullRequestConversationItems(items, { bots: false, resolved: false }).map(
581+
(item) => item.id,
582+
),
583+
).toEqual(["approved", "changes"]);
584+
});
585+
564586
it("distinguishes unloaded replies from complete threads, even when the host only supplies a cursor", () => {
565587
const timeline = (reviewThread: PullRequestReviewThread) =>
566588
buildPullRequestTimeline({

‎apps/web/src/components/pullRequest/pullRequestDetail.logic.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,11 @@ export function isPullRequestCommentFinished(
621621
);
622622
}
623623

624+
function isPullRequestVerdict(reviewState: string | null): boolean {
625+
const outcome = pullRequestReviewOutcome(reviewState);
626+
return outcome === "approved" || outcome === "changes-requested";
627+
}
628+
624629
export function filterPullRequestConversationItems(
625630
items: ReadonlyArray<PullRequestConversationItem>,
626631
filter: { readonly bots: boolean; readonly resolved: boolean },
@@ -629,7 +634,10 @@ export function filterPullRequestConversationItems(
629634
// Matching any member keeps the finding beside a human reply to a bot.
630635
pullRequestConversationItemComments(item).some(
631636
(comment) =>
632-
(filter.bots || comment.author?.isBot !== true) &&
637+
// A bot's approval or change request is a verdict, not chatter.
638+
(filter.bots ||
639+
comment.author?.isBot !== true ||
640+
isPullRequestVerdict(comment.reviewState)) &&
633641
(filter.resolved ||
634642
!isPullRequestCommentFinished(comment, item.kind === "thread" ? item.thread : undefined)),
635643
),

‎apps/web/src/components/pullRequest/pullRequestPresentation.tsx‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@ const REVIEW_OUTCOME_PRESENTATION = {
279279
ringClassName: "ring-2 ring-emerald-500 dark:ring-emerald-400",
280280
staleRingClassName:
281281
"ring-2 ring-[color-mix(in_srgb,var(--color-emerald-500)_35%,var(--background))] dark:ring-[color-mix(in_srgb,var(--color-emerald-400)_35%,var(--background))]",
282+
fillClassName: "fill-emerald-500 dark:fill-emerald-400",
282283
badgeVariant: "success",
283284
},
284285
"changes-requested": {
@@ -287,6 +288,7 @@ const REVIEW_OUTCOME_PRESENTATION = {
287288
toneClassName: "text-destructive",
288289
ringClassName: "ring-2 ring-destructive",
289290
staleRingClassName: "ring-2 ring-[color-mix(in_srgb,var(--destructive)_35%,var(--background))]",
291+
fillClassName: "fill-destructive",
290292
badgeVariant: "error",
291293
},
292294
dismissed: {
@@ -296,6 +298,7 @@ const REVIEW_OUTCOME_PRESENTATION = {
296298
ringClassName: "ring-2 ring-muted-foreground/60",
297299
staleRingClassName:
298300
"ring-2 ring-[color-mix(in_srgb,var(--contrast-muted-foreground)_30%,var(--background))]",
301+
fillClassName: "fill-muted-foreground",
299302
badgeVariant: "outline",
300303
},
301304
} as const satisfies Record<
@@ -306,6 +309,7 @@ const REVIEW_OUTCOME_PRESENTATION = {
306309
toneClassName: string;
307310
ringClassName: string;
308311
staleRingClassName: string;
312+
fillClassName: string;
309313
badgeVariant: "success" | "error" | "outline";
310314
}
311315
>;
@@ -343,6 +347,28 @@ export function PullRequestReviewOutcomeIcon({
343347
);
344348
}
345349

350+
// Ring color alone blends into colorful bot avatars; the glyph gives each verdict a shape.
351+
export function PullRequestReviewOutcomeAvatarBadge({
352+
outcome,
353+
stale = false,
354+
}: {
355+
outcome: PullRequestReviewOutcome;
356+
stale?: boolean;
357+
}) {
358+
const presentation = REVIEW_OUTCOME_PRESENTATION[outcome];
359+
return (
360+
<presentation.Icon
361+
aria-hidden
362+
strokeWidth={3}
363+
className={cn(
364+
"absolute -right-1 -bottom-1 size-3 text-background",
365+
presentation.fillClassName,
366+
stale && "opacity-50",
367+
)}
368+
/>
369+
);
370+
}
371+
346372
export function pullRequestReviewOutcomeLabel(outcome: PullRequestReviewOutcome): string {
347373
return REVIEW_OUTCOME_PRESENTATION[outcome].label;
348374
}

0 commit comments

Comments
 (0)