Skip to content

Commit 7f320bd

Browse files
Valentin Grünerclaude
andcommitted
fix(webapp): name the reviewed work's own source
A review run compared its artifact kind against `PULL_REQUEST`, `ISSUE` and `CONVERSATION_THREAD`. Those are the Java constants' names; `ArtifactKind` serialises through `@JsonValue` under a grammar that excludes upper case, so the wire only ever carries `scm.pull_request`, `scm.issue`, `chat.conversation_thread` and `docs.document`. Every branch was dead: a Slack thread and an Outline document both took the GitHub mark and the GitHub label, the channel name was dropped, and a run without a title read "Chat.conversation thread on GitHub". The shared registry in `lib/artifact-kinds` already owned these ids, and the filter on the same page already used them — only the card had been left behind. It now takes its glyph and its words from there, and the provider mark comes from a table covering all four providers rather than defaulting to GitHub. `docs.document` becomes filterable in the same move, and the source sentence under a standing loses its third private copy of the same vocabulary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MAQxdMw579siMh1wXKEEh4
1 parent bf1ae49 commit 7f320bd

10 files changed

Lines changed: 105 additions & 513 deletions

webapp/src/components/profile/PracticeGroupDetailPage.tsx

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -35,58 +35,65 @@ import {
3535
PopoverTrigger,
3636
} from "@/components/ui/popover";
3737
import { Skeleton } from "@/components/ui/skeleton";
38+
import { ARTIFACT_KIND_VALUES, artifactKindPluralLabel } from "@/lib/artifact-kinds";
3839
import { cn } from "@/lib/utils";
40+
import { PRACTICE_GROUP_STANDING_BADGE } from "./practice-group-standing-presentation";
3941
import { PracticeNextStepCallout } from "./PracticeNextStepCallout";
4042
import { PracticeTrendChip } from "./PracticeTrendChip";
41-
import { PRACTICE_GROUP_STANDING_BADGE } from "./practice-group-standing-presentation";
42-
import { ReviewRunTimeline } from "./ReviewRunTimeline";
4343
import type { FeedbackUsefulness, ObservationDetailState } from "./review-runs";
44+
import { ReviewRunTimeline } from "./ReviewRunTimeline";
4445
import { SEVERITY_ORDER, SEVERITY_PRESENTATION, type SeverityKey } from "./severity-presentation";
4546

4647
type PracticeStandingKey = NonNullable<PracticeStanding["standing"]> | "UNMEASURED";
4748

49+
/**
50+
* The node's glyph and colour. Its **words** come from the standing registry, so a practice node and
51+
* the group badge above it can never name the same value differently — they used to disagree on
52+
* three of five. `UNMEASURED` is the one entry the registry cannot own: it is not a standing the
53+
* server reports but the client's word for a practice it sent no standing for at all.
54+
*/
4855
const STANDING_NODE: Record<
4956
PracticeStandingKey,
50-
{ Icon: typeof CircleCheckIcon; circleClass: string; textClass: string; label: string }
57+
{ Icon: typeof CircleCheckIcon; circleClass: string; textClass: string }
5158
> = {
5259
DEVELOPING: {
5360
Icon: CircleAlertIcon,
5461
circleClass: "border-destructive/50 text-destructive",
5562
textClass: "text-destructive",
56-
label: "Needs attention",
5763
},
5864
MIXED: {
5965
Icon: CircleMinusIcon,
6066
circleClass: "border-warning/60 text-warning",
6167
textClass: "text-warning",
62-
label: "Mixed feedback",
6368
},
6469
STRENGTH: {
6570
Icon: CircleCheckIcon,
6671
circleClass: "border-success/60 text-success",
6772
textClass: "text-success",
68-
label: "Going well",
6973
},
7074
NOT_OBSERVED: {
7175
Icon: CircleDashedIcon,
7276
circleClass: "border-border text-muted-foreground",
7377
textClass: "text-muted-foreground",
74-
label: "Not observed yet",
7578
},
7679
NO_OPPORTUNITY: {
7780
Icon: CircleDashedIcon,
7881
circleClass: "border-border text-muted-foreground",
7982
textClass: "text-muted-foreground",
80-
label: "No occasion yet",
8183
},
8284
UNMEASURED: {
8385
Icon: CircleDashedIcon,
8486
circleClass: "border-border text-muted-foreground",
8587
textClass: "text-muted-foreground",
86-
label: "Not measured yet",
8788
},
8889
};
8990

91+
function standingNodeLabel(standing: PracticeStandingKey): string {
92+
return standing === "UNMEASURED"
93+
? "Not measured yet"
94+
: PRACTICE_GROUP_STANDING_BADGE[standing].label;
95+
}
96+
9097
export type ReviewRunSource = string;
9198

9299
export interface ReviewRunFilters {
@@ -233,11 +240,10 @@ export function PracticeGroupDetailPage({
233240
const hasAnyFeedNarrowing = activeFilterCount > 0 || selectedPractice !== undefined;
234241
const hasReviewRuns = (reviewRuns?.length ?? 0) > 0;
235242

236-
const sourceOptions: { value: ReviewRunSource; label: string }[] = [
237-
{ value: "scm.pull_request", label: "Pull requests" },
238-
{ value: "scm.issue", label: "Issues" },
239-
{ value: "chat.conversation_thread", label: "Slack conversations" },
240-
];
243+
// Every kind the build can name, so a document review is filterable the day the server sends one.
244+
const sourceOptions: { value: ReviewRunSource; label: string }[] = ARTIFACT_KIND_VALUES.map(
245+
(kind) => ({ value: kind, label: artifactKindPluralLabel(kind) }),
246+
);
241247

242248
const toggleSource = (source: ReviewRunSource, checked: boolean) => {
243249
const sources = checked
@@ -384,7 +390,9 @@ export function PracticeGroupDetailPage({
384390
{practice.name}
385391
</span>
386392
<div className="relative z-20 flex flex-wrap items-center gap-x-2 gap-y-1 text-sm leading-5">
387-
<span className={node.textClass}>{node.label}</span>
393+
<span className={node.textClass}>
394+
{standingNodeLabel(practiceStanding)}
395+
</span>
388396
{practiceTrend && (
389397
<PracticeTrendChip
390398
direction={practiceTrend.direction}

webapp/src/components/profile/PracticeGroupStandingCard.tsx

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,15 @@ import { Badge } from "@/components/ui/badge";
77
import { Button } from "@/components/ui/button";
88
import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card";
99
import { Skeleton } from "@/components/ui/skeleton";
10+
import { artifactKindCountLabel } from "@/lib/artifact-kinds";
1011
import { cn } from "@/lib/utils";
12+
import { PRACTICE_GROUP_STANDING_BADGE } from "./practice-group-standing-presentation";
1113
import {
1214
PracticeGroupStandingRing,
1315
STANDING_LEGEND,
1416
summarizePracticeStandings,
1517
} from "./PracticeGroupStandingRing";
1618
import { PracticeTrendChip } from "./PracticeTrendChip";
17-
import {
18-
PRACTICE_GROUP_SOURCE_META,
19-
PRACTICE_GROUP_STANDING_BADGE,
20-
} from "./practice-group-standing-presentation";
2119

2220
const COLLAPSED_GROUP_COUNT = 3;
2321
const STANDING_PRIORITY: Record<PracticeGroupStanding["standing"], number> = {
@@ -38,12 +36,6 @@ export interface PracticeGroupStandingSectionProps {
3836
practicesByGroup?: Record<string, PracticeStanding[] | undefined>;
3937
}
4038

41-
function sourceLabel(workKind: string, count: number): string {
42-
const metadata = PRACTICE_GROUP_SOURCE_META[workKind];
43-
if (!metadata) return `${count} ${workKind}`;
44-
return `${count} ${count === 1 ? metadata.singular : metadata.plural}`;
45-
}
46-
4739
export function PracticeGroupStandingCard({
4840
groups,
4941
standings,
@@ -162,7 +154,7 @@ export function PracticeGroupStandingCard({
162154
<p className="text-xs text-muted-foreground">
163155
Based on{" "}
164156
{groupStanding.sources
165-
.map(({ workKind, count }) => sourceLabel(workKind, count))
157+
.map(({ workKind, count }) => artifactKindCountLabel(workKind, count))
166158
.join(", ")}
167159
</p>
168160
)}

webapp/src/components/profile/PracticeTrendPanel.stories.tsx

Lines changed: 0 additions & 102 deletions
This file was deleted.

webapp/src/components/profile/PracticeTrendPanel.test.tsx

Lines changed: 0 additions & 122 deletions
This file was deleted.

0 commit comments

Comments
 (0)