From 9c2a090829a1b2a9870914411830479a44c2c375 Mon Sep 17 00:00:00 2001 From: Oliver Browne Date: Thu, 4 Jun 2026 12:14:45 +0300 Subject: [PATCH 1/4] feat(inbox): edit suggested reviewers from the report detail pane Surfaces UI for the suggested-reviewers PUT API (PostHog#59357) in the inbox detail pane. Adds a SuggestedReviewersEditor with an "Add" picker (org members, filtered locally over the cached available-reviewers list) and per-row removal. Each add/remove persists immediately via a full-replacement PUT with optimistic cache patching. - New client method updateSignalReportArtefact + write-entry type - useUpdateSuggestedReviewers optimistic mutation hook - Section renders whenever a suggested_reviewers artefact exists so an emptied list keeps the Add entry point; gated out when absent - Two new analytics action types Generated-By: PostHog Code Task-Id: f961c44f-0c0b-4bff-ab85-b533bf1856ce --- .../src/renderer/api/posthogClient.test.ts | 124 ++++++ apps/code/src/renderer/api/posthogClient.ts | 40 ++ .../components/detail/ReportDetailPane.tsx | 134 +----- .../detail/SuggestedReviewersEditor.tsx | 404 ++++++++++++++++++ .../inbox/hooks/useInboxReports.test.tsx | 151 +++++++ .../features/inbox/hooks/useInboxReports.ts | 65 +++ apps/code/src/shared/types.ts | 11 + apps/code/src/shared/types/analytics.ts | 2 + 8 files changed, 813 insertions(+), 118 deletions(-) create mode 100644 apps/code/src/renderer/features/inbox/components/detail/SuggestedReviewersEditor.tsx create mode 100644 apps/code/src/renderer/features/inbox/hooks/useInboxReports.test.tsx diff --git a/apps/code/src/renderer/api/posthogClient.test.ts b/apps/code/src/renderer/api/posthogClient.test.ts index 2e0f299643..4fd603f8f7 100644 --- a/apps/code/src/renderer/api/posthogClient.test.ts +++ b/apps/code/src/renderer/api/posthogClient.test.ts @@ -385,4 +385,128 @@ describe("PostHogAPIClient", () => { expect(result.length).toBe(50); }); }); + + describe("updateSignalReportArtefact", () => { + const ARTEFACT_PATH = + "/api/projects/123/signals/reports/report-1/artefacts/art-1/"; + + function makeClient(fetch: ReturnType) { + const client = new PostHogAPIClient( + "http://localhost:8000", + async () => "token", + async () => "token", + 123, + ); + ( + client as unknown as { + api: { baseUrl: string; fetcher: { fetch: typeof fetch } }; + } + ).api = { baseUrl: "http://localhost:8000", fetcher: { fetch } }; + return client; + } + + it("PUTs the full-replacement content and returns the parsed artefact", async () => { + const fetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ + id: "art-1", + type: "suggested_reviewers", + created_at: "2024-01-01T00:00:00Z", + content: [ + { + github_login: "octocat", + github_name: "The Octocat", + relevant_commits: [], + user: null, + }, + ], + }), + }); + const client = makeClient(fetch); + + const result = await client.updateSignalReportArtefact( + "report-1", + "art-1", + [{ github_login: "octocat" }, { user_uuid: "uuid-1" }], + ); + + expect(result.type).toBe("suggested_reviewers"); + expect(result.content).toEqual([ + { + github_login: "octocat", + github_name: "The Octocat", + relevant_commits: [], + user: null, + }, + ]); + expect(fetch).toHaveBeenCalledWith( + expect.objectContaining({ + method: "put", + path: ARTEFACT_PATH, + overrides: { + body: JSON.stringify({ + content: [{ github_login: "octocat" }, { user_uuid: "uuid-1" }], + }), + }, + }), + ); + }); + + it("sends an empty content array when clearing reviewers", async () => { + const fetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ + id: "art-1", + type: "suggested_reviewers", + created_at: "2024-01-01T00:00:00Z", + content: [], + }), + }); + const client = makeClient(fetch); + + const result = await client.updateSignalReportArtefact( + "report-1", + "art-1", + [], + ); + + expect(result.content).toEqual([]); + expect(fetch).toHaveBeenCalledWith( + expect.objectContaining({ + method: "put", + overrides: { body: JSON.stringify({ content: [] }) }, + }), + ); + }); + + it("throws with the server message on a non-ok response", async () => { + const fetch = vi.fn().mockResolvedValue({ + ok: false, + text: async () => + '{"error":"Only suggested_reviewers artefacts may be modified via this endpoint."}', + }); + const client = makeClient(fetch); + + await expect( + client.updateSignalReportArtefact("report-1", "art-1", []), + ).rejects.toThrow("Only suggested_reviewers"); + }); + + it("throws when the response is not a suggested_reviewers artefact", async () => { + const fetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ + id: "art-1", + type: "dismissal", + created_at: "2024-01-01T00:00:00Z", + content: { reason: "noise", note: "" }, + }), + }); + const client = makeClient(fetch); + + await expect( + client.updateSignalReportArtefact("report-1", "art-1", []), + ).rejects.toThrow("Unexpected response"); + }); + }); }); diff --git a/apps/code/src/renderer/api/posthogClient.ts b/apps/code/src/renderer/api/posthogClient.ts index 150b501901..e778fc451f 100644 --- a/apps/code/src/renderer/api/posthogClient.ts +++ b/apps/code/src/renderer/api/posthogClient.ts @@ -28,6 +28,7 @@ import type { SlackChannelsQueryParams, SlackChannelsResponse, SuggestedReviewersArtefact, + SuggestedReviewerWriteEntry, Task, TaskRun, } from "@shared/types"; @@ -2122,6 +2123,45 @@ export class PostHogAPIClient { return (await response.json()) as SignalReport; } + /** + * Replace the content of a `suggested_reviewers` artefact (full replacement). + * The server canonicalizes each entry to a lowercase `github_login` and preserves + * `relevant_commits` / `github_name` for logins that survive the replace. + */ + async updateSignalReportArtefact( + reportId: string, + artefactId: string, + content: SuggestedReviewerWriteEntry[], + ): Promise { + const teamId = await this.getTeamId(); + const url = new URL( + `${this.api.baseUrl}/api/projects/${teamId}/signals/reports/${reportId}/artefacts/${artefactId}/`, + ); + const path = `/api/projects/${teamId}/signals/reports/${reportId}/artefacts/${artefactId}/`; + + const response = await this.api.fetcher.fetch({ + method: "put", + url, + path, + overrides: { + body: JSON.stringify({ content }), + }, + }); + + if (!response.ok) { + const errorText = await response.text(); + throw new Error(errorText || "Failed to update suggested reviewers"); + } + + const parsed = normalizeSignalReportArtefact(await response.json()); + if (!parsed || parsed.type !== "suggested_reviewers") { + throw new Error("Unexpected response updating suggested reviewers"); + } + // `SignalReportArtefact.type` is a plain string, so the guard above can't + // statically narrow the union — the runtime check makes this cast safe. + return parsed as SuggestedReviewersArtefact; + } + async deleteSignalReport(reportId: string): Promise<{ status: "deletion_started" | "already_running"; report_id: string; diff --git a/apps/code/src/renderer/features/inbox/components/detail/ReportDetailPane.tsx b/apps/code/src/renderer/features/inbox/components/detail/ReportDetailPane.tsx index 94e4c8c1fd..d85624482b 100644 --- a/apps/code/src/renderer/features/inbox/components/detail/ReportDetailPane.tsx +++ b/apps/code/src/renderer/features/inbox/components/detail/ReportDetailPane.tsx @@ -1,4 +1,3 @@ -import { Badge } from "@components/ui/Badge"; import { Button } from "@components/ui/Button"; import { useInboxReportArtefacts, @@ -12,12 +11,9 @@ import { useAuthenticatedQuery } from "@hooks/useAuthenticatedQuery"; import { useDetectedCloudRepository } from "@hooks/useDetectedCloudRepository"; import { useMeQuery } from "@hooks/useMeQuery"; import { - ArrowSquareOutIcon, CaretDownIcon, CaretRightIcon, ChatCircleIcon, - EyeIcon, - InfoIcon, LinkSimpleIcon, Plus, ThumbsDownIcon, @@ -46,7 +42,6 @@ import type { SignalFindingArtefact, SignalReport, SignalReportTask, - SuggestedReviewer, SuggestedReviewersArtefact, Task, } from "@shared/types"; @@ -72,15 +67,9 @@ import { SignalReportStatusBadge } from "../utils/SignalReportStatusBadge"; import { SignalReportSummaryMarkdown } from "../utils/SignalReportSummaryMarkdown"; import { ReportTaskLogs } from "./ReportTaskLogs"; import { SignalCard } from "./SignalCard"; +import { SuggestedReviewersEditor } from "./SuggestedReviewersEditor"; import type { SignalInteractionAction } from "./signalInteractionContext"; -function isSuggestedReviewerRowMe( - reviewer: SuggestedReviewer, - meUuid: string | undefined, -): boolean { - return !!reviewer.user?.uuid && !!meUuid && meUuid === reviewer.user.uuid; -} - const REPOSITORY_SOURCE_RELATIONSHIPS: SignalReportTask["relationship"][] = [ "repo_selection", "research", @@ -208,16 +197,14 @@ export function ReportDetailPane({ }); const allArtefacts = artefactsQuery.data?.results ?? []; - const suggestedReviewers = useMemo(() => { - const reviewerArtefact = allArtefacts.find( - (a): a is SuggestedReviewersArtefact => a.type === "suggested_reviewers", - ); - const reviewers = reviewerArtefact?.content ?? []; - if (!me?.uuid) return reviewers; - const meIndex = reviewers.findIndex((r) => r.user?.uuid === me.uuid); - if (meIndex <= 0) return reviewers; - return [reviewers[meIndex], ...reviewers.filter((_, i) => i !== meIndex)]; - }, [allArtefacts, me?.uuid]); + const reviewerArtefact = useMemo( + () => + allArtefacts.find( + (a): a is SuggestedReviewersArtefact => + a.type === "suggested_reviewers", + ) ?? null, + [allArtefacts], + ); const signalFindings = useMemo(() => { const map = new Map(); @@ -769,102 +756,13 @@ export function ReportDetailPane({ )} {/* ── Suggested reviewers ─────────────────────────────── */} - {suggestedReviewers.length > 0 && ( - - - Suggested reviewers - - - {suggestedReviewers.map((reviewer) => { - const isMe = isSuggestedReviewerRowMe(reviewer, me?.uuid); - return ( - - e.currentTarget.classList.add("loaded")} - /> - - {reviewer.user?.first_name ?? - reviewer.github_name ?? - reviewer.github_login} - - {isMe && ( - - - - - - )} - - fireDetailAction("click_suggested_reviewer") - } - > - @{reviewer.github_login} - - - {reviewer.relevant_commits.length > 0 && ( - - {reviewer.relevant_commits.map((commit, i) => ( - - {i > 0 && ", "} - - - Why was I assigned? - - - {commit.reason} - - - ) : ( - commit.reason || undefined - ) - } - > - - - {commit.sha.slice(0, 7)} - - {isMe && commit.reason && ( - - )} - - - - ))} - - )} - - ); - })} - - + {reviewerArtefact && ( + )} {/* ── Signals ─────────────────────────────────────────── */} diff --git a/apps/code/src/renderer/features/inbox/components/detail/SuggestedReviewersEditor.tsx b/apps/code/src/renderer/features/inbox/components/detail/SuggestedReviewersEditor.tsx new file mode 100644 index 0000000000..fd7914a3e8 --- /dev/null +++ b/apps/code/src/renderer/features/inbox/components/detail/SuggestedReviewersEditor.tsx @@ -0,0 +1,404 @@ +import { Badge } from "@components/ui/Badge"; +import { useOptionalAuthenticatedClient } from "@features/auth/hooks/authClient"; +import { useCurrentUser } from "@features/auth/hooks/authQueries"; +import { + useInboxAvailableSuggestedReviewers, + useUpdateSuggestedReviewers, +} from "@features/inbox/hooks/useInboxReports"; +import { + buildSuggestedReviewerFilterOptions, + getSuggestedReviewerDisplayName, +} from "@features/inbox/utils/suggestedReviewerFilters"; +import { + ArrowSquareOutIcon, + Check, + EyeIcon, + InfoIcon, + MagnifyingGlass, + Plus, + XIcon, +} from "@phosphor-icons/react"; +import { Box, Flex, Popover, Spinner, Text, Tooltip } from "@radix-ui/themes"; +import type { + AvailableSuggestedReviewer, + SuggestedReviewer, + SuggestedReviewersArtefact, + SuggestedReviewerWriteEntry, +} from "@shared/types"; +import { useDeferredValue, useMemo, useState } from "react"; + +type ReviewerAction = + | "click_suggested_reviewer" + | "add_suggested_reviewer" + | "remove_suggested_reviewer"; + +interface SuggestedReviewersEditorProps { + reportId: string; + artefact: SuggestedReviewersArtefact; + meUuid: string | undefined; + fireAction: (action: ReviewerAction) => void; +} + +function isRowMe( + reviewer: SuggestedReviewer, + meUuid: string | undefined, +): boolean { + return !!reviewer.user?.uuid && !!meUuid && meUuid === reviewer.user.uuid; +} + +/** A reviewer in the artefact matches an org member by user uuid or (case-insensitive) login. */ +function reviewerMatchesAvailable( + reviewer: SuggestedReviewer, + available: AvailableSuggestedReviewer, +): boolean { + if (reviewer.user?.uuid && reviewer.user.uuid === available.uuid) { + return true; + } + return ( + !!reviewer.github_login && + !!available.github_login && + reviewer.github_login.toLowerCase() === available.github_login.toLowerCase() + ); +} + +/** + * Build the write-shape payload from a read-shape list. Kept reviewers are sent by + * `github_login` (the server preserves their commits/name); an entry that only has a + * resolved user (e.g. one optimistically added before the refetch fills in its login) + * falls back to `user_uuid`. Entries with neither are dropped. + */ +function toWriteContent( + reviewers: SuggestedReviewer[], +): SuggestedReviewerWriteEntry[] { + return reviewers + .map((reviewer): SuggestedReviewerWriteEntry | null => { + if (reviewer.github_login) { + return { github_login: reviewer.github_login }; + } + if (reviewer.user?.uuid) { + return { user_uuid: reviewer.user.uuid }; + } + return null; + }) + .filter((entry): entry is SuggestedReviewerWriteEntry => entry !== null); +} + +export function SuggestedReviewersEditor({ + reportId, + artefact, + meUuid, + fireAction, +}: SuggestedReviewersEditorProps) { + const client = useOptionalAuthenticatedClient(); + const [addOpen, setAddOpen] = useState(false); + const [reviewerQuery, setReviewerQuery] = useState(""); + const deferredReviewerQuery = useDeferredValue(reviewerQuery); + + const { mutate: updateReviewers, isPending } = + useUpdateSuggestedReviewers(reportId); + + // Reviewers in their original (agent-ranked) order — used to build write payloads. + const reviewers = artefact.content; + + // Display order: pin the current user first, otherwise preserve agent ranking. + const displayReviewers = useMemo(() => { + if (!meUuid) return reviewers; + const meIndex = reviewers.findIndex((r) => r.user?.uuid === meUuid); + if (meIndex <= 0) return reviewers; + return [reviewers[meIndex], ...reviewers.filter((_, i) => i !== meIndex)]; + }, [reviewers, meUuid]); + + const { data: currentUser } = useCurrentUser({ client, enabled: !!client }); + // Fetch the full base list (no `query`) so it's served from the cached + // `inboxAvailableSuggestedReviewersStore` and we filter locally — avoids a + // server round-trip on every keystroke. + const { data: availableReviewers, isFetching } = + useInboxAvailableSuggestedReviewers({ + enabled: !!client && addOpen, + }); + + // Org members that can be added. The `available_reviewers` endpoint only returns + // members with a linked GitHub identity, so every option is addable via its uuid — + // note the endpoint does not include `github_login` in its payload. + const addableOptions = useMemo(() => { + const options = buildSuggestedReviewerFilterOptions( + availableReviewers?.results ?? [], + currentUser, + ); + const q = deferredReviewerQuery.trim().toLowerCase(); + if (!q) return options; + return options.filter( + (option) => + option.name.toLowerCase().includes(q) || + option.email.toLowerCase().includes(q) || + option.github_login.toLowerCase().includes(q), + ); + }, [availableReviewers?.results, currentUser, deferredReviewerQuery]); + + const removeReviewer = (target: SuggestedReviewer) => { + const next = reviewers.filter((r) => r !== target); + fireAction("remove_suggested_reviewer"); + updateReviewers({ + artefactId: artefact.id, + content: toWriteContent(next), + optimisticReviewers: next, + }); + }; + + const toggleReviewer = (option: AvailableSuggestedReviewer) => { + const existing = reviewers.find((r) => reviewerMatchesAvailable(r, option)); + if (existing) { + removeReviewer(existing); + return; + } + + const optimisticEntry: SuggestedReviewer = { + github_login: option.github_login, + github_name: option.name || null, + relevant_commits: [], + user: { + id: 0, + uuid: option.uuid, + email: option.email, + first_name: option.name, + last_name: "", + }, + }; + const next = [...reviewers, optimisticEntry]; + fireAction("add_suggested_reviewer"); + updateReviewers({ + artefactId: artefact.id, + // Keep existing by login, add the new one by uuid (server canonicalizes). + content: [...toWriteContent(reviewers), { user_uuid: option.uuid }], + optimisticReviewers: next, + }); + }; + + return ( + + + Suggested reviewers + {isPending && } + + { + setAddOpen(next); + if (!next) setReviewerQuery(""); + }} + > + + + + + + + + setReviewerQuery(e.target.value)} + className="min-w-0 flex-1 bg-transparent text-[12px] text-gray-12 outline-none placeholder:text-gray-9" + /> + + + + {isFetching && addableOptions.length === 0 ? ( + + + + ) : addableOptions.length === 0 ? ( + + No users found. + + ) : ( + + {addableOptions.map((option) => { + const isAssigned = reviewers.some((r) => + reviewerMatchesAvailable(r, option), + ); + const displayName = + getSuggestedReviewerDisplayName(option); + return ( + + ); + })} + + )} + + + + + + + {displayReviewers.length === 0 ? ( + + No reviewers assigned. Use “Add” to suggest one. + + ) : ( + + {displayReviewers.map((reviewer) => { + const isMe = isRowMe(reviewer, meUuid); + return ( + + {reviewer.github_login ? ( + e.currentTarget.classList.add("loaded")} + /> + ) : null} + + {reviewer.user?.first_name ?? + reviewer.github_name ?? + reviewer.github_login} + + {isMe && ( + + + + + + )} + {reviewer.github_login ? ( + fireAction("click_suggested_reviewer")} + > + @{reviewer.github_login} + + + ) : null} + {reviewer.relevant_commits.length > 0 && ( + + {reviewer.relevant_commits.map((commit, i) => ( + + {i > 0 && ", "} + + + Why was I assigned? + + + {commit.reason} + + + ) : ( + commit.reason || undefined + ) + } + > + + + {commit.sha.slice(0, 7)} + + {isMe && commit.reason && ( + + )} + + + + ))} + + )} + + + ); + })} + + )} + + ); +} diff --git a/apps/code/src/renderer/features/inbox/hooks/useInboxReports.test.tsx b/apps/code/src/renderer/features/inbox/hooks/useInboxReports.test.tsx new file mode 100644 index 0000000000..64f27d4962 --- /dev/null +++ b/apps/code/src/renderer/features/inbox/hooks/useInboxReports.test.tsx @@ -0,0 +1,151 @@ +import type { + SignalReportArtefactsResponse, + SuggestedReviewer, + SuggestedReviewersArtefact, +} from "@shared/types"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { renderHook } from "@testing-library/react"; +import { act, type ReactNode } from "react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mockUpdateArtefact = vi.hoisted(() => vi.fn()); +const mockClient = vi.hoisted(() => ({ + updateSignalReportArtefact: mockUpdateArtefact, +})); + +vi.mock("@features/auth/hooks/authClient", () => ({ + useOptionalAuthenticatedClient: () => mockClient, +})); + +vi.mock("@renderer/trpc/client", () => ({ + trpcClient: {}, +})); + +vi.mock("@utils/logger", () => ({ + logger: { + scope: () => ({ + info: vi.fn(), + debug: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }), + }, +})); + +import { reportKeys, useUpdateSuggestedReviewers } from "./useInboxReports"; + +const REPORT_ID = "report-1"; +const ARTEFACT_ID = "art-1"; + +function reviewer(login: string, uuid?: string): SuggestedReviewer { + return { + github_login: login, + github_name: login, + relevant_commits: [], + user: uuid + ? { + id: 1, + uuid, + email: `${login}@x.io`, + first_name: login, + last_name: "", + } + : null, + }; +} + +function artefact(content: SuggestedReviewer[]): SuggestedReviewersArtefact { + return { + id: ARTEFACT_ID, + type: "suggested_reviewers", + created_at: "2024-01-01T00:00:00Z", + content, + }; +} + +function renderUpdateHook() { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + const wrapper = ({ children }: { children: ReactNode }) => ( + {children} + ); + const result = renderHook(() => useUpdateSuggestedReviewers(REPORT_ID), { + wrapper, + }); + return { ...result, queryClient }; +} + +describe("useUpdateSuggestedReviewers", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("optimistically patches the suggested_reviewers artefact in the cache", async () => { + mockUpdateArtefact.mockResolvedValue(artefact([reviewer("octocat")])); + const { result, queryClient } = renderUpdateHook(); + + const key = reportKeys.artefacts(REPORT_ID); + queryClient.setQueryData(key, { + results: [artefact([reviewer("octocat"), reviewer("hubot")])], + count: 1, + }); + + const next = [reviewer("octocat")]; + await act(async () => { + await result.current.mutateAsync({ + artefactId: ARTEFACT_ID, + content: [{ github_login: "octocat" }], + optimisticReviewers: next, + }); + }); + + expect(mockUpdateArtefact).toHaveBeenCalledWith(REPORT_ID, ARTEFACT_ID, [ + { github_login: "octocat" }, + ]); + + const cached = queryClient.getQueryData(key); + const cachedArtefact = cached?.results.find((a) => a.id === ARTEFACT_ID) as + | SuggestedReviewersArtefact + | undefined; + expect(cachedArtefact?.content.map((r) => r.github_login)).toEqual([ + "octocat", + ]); + }); + + it("rolls back the cache when the request fails", async () => { + const failure = new Error("boom"); + mockUpdateArtefact.mockRejectedValue(failure); + const { result, queryClient } = renderUpdateHook(); + + const key = reportKeys.artefacts(REPORT_ID); + const original = [reviewer("octocat"), reviewer("hubot")]; + queryClient.setQueryData(key, { + results: [artefact(original)], + count: 1, + }); + + let caught: unknown; + await act(async () => { + try { + await result.current.mutateAsync({ + artefactId: ARTEFACT_ID, + content: [{ github_login: "octocat" }], + optimisticReviewers: [reviewer("octocat")], + }); + } catch (error) { + caught = error; + } + }); + expect(caught).toBe(failure); + + const cached = queryClient.getQueryData(key); + const cachedArtefact = cached?.results.find((a) => a.id === ARTEFACT_ID) as + | SuggestedReviewersArtefact + | undefined; + expect(cachedArtefact?.content.map((r) => r.github_login)).toEqual([ + "octocat", + "hubot", + ]); + }); +}); diff --git a/apps/code/src/renderer/features/inbox/hooks/useInboxReports.ts b/apps/code/src/renderer/features/inbox/hooks/useInboxReports.ts index 8ba010385c..021c1b46db 100644 --- a/apps/code/src/renderer/features/inbox/hooks/useInboxReports.ts +++ b/apps/code/src/renderer/features/inbox/hooks/useInboxReports.ts @@ -4,6 +4,7 @@ import { } from "@features/auth/hooks/authQueries"; import { useInboxAvailableSuggestedReviewersStore } from "@features/inbox/stores/inboxAvailableSuggestedReviewersStore"; import { useAuthenticatedInfiniteQuery } from "@hooks/useAuthenticatedInfiniteQuery"; +import { useAuthenticatedMutation } from "@hooks/useAuthenticatedMutation"; import { useAuthenticatedQuery } from "@hooks/useAuthenticatedQuery"; import type { AvailableSuggestedReviewersResponse, @@ -13,7 +14,10 @@ import type { SignalReportSignalsResponse, SignalReportsQueryParams, SignalReportsResponse, + SuggestedReviewersArtefact, + SuggestedReviewerWriteEntry, } from "@shared/types"; +import { useQueryClient } from "@tanstack/react-query"; import { useEffect, useMemo } from "react"; const REPORTS_PAGE_SIZE = 100; @@ -209,3 +213,64 @@ export function useInboxReportSignals( { enabled: !!reportId && (options?.enabled ?? true) }, ); } + +interface UpdateSuggestedReviewersVariables { + artefactId: string; + /** Full-replacement payload sent to the server. */ + content: SuggestedReviewerWriteEntry[]; + /** Read-shape list used to optimistically patch the cache for immediate-apply UI. */ + optimisticReviewers: SuggestedReviewersArtefact["content"]; +} + +/** + * Persists a full replacement of a report's `suggested_reviewers` artefact and optimistically + * patches the cached artefacts so the detail pane reflects the change instantly (immediate apply). + */ +export function useUpdateSuggestedReviewers(reportId: string) { + const queryClient = useQueryClient(); + const queryKey = reportKeys.artefacts(reportId); + + return useAuthenticatedMutation< + SuggestedReviewersArtefact, + Error, + UpdateSuggestedReviewersVariables + >( + (client, { artefactId, content }) => + client.updateSignalReportArtefact(reportId, artefactId, content), + { + onMutate: async ({ artefactId, optimisticReviewers }) => { + await queryClient.cancelQueries({ queryKey }); + const previous = + queryClient.getQueryData(queryKey); + + if (previous) { + queryClient.setQueryData(queryKey, { + ...previous, + results: previous.results.map((artefact) => + artefact.id === artefactId && + artefact.type === "suggested_reviewers" + ? ({ + ...artefact, + content: optimisticReviewers, + } as SuggestedReviewersArtefact) + : artefact, + ), + }); + } + + return { previous }; + }, + onError: (_error, _variables, context) => { + const previous = ( + context as { previous?: SignalReportArtefactsResponse } + )?.previous; + if (previous) { + queryClient.setQueryData(queryKey, previous); + } + }, + onSettled: () => { + queryClient.invalidateQueries({ queryKey }); + }, + }, + ); +} diff --git a/apps/code/src/shared/types.ts b/apps/code/src/shared/types.ts index 53d4f54f34..4d2250f879 100644 --- a/apps/code/src/shared/types.ts +++ b/apps/code/src/shared/types.ts @@ -418,6 +418,17 @@ export interface SuggestedReviewer { user: SuggestedReviewerUser | null; } +/** + * A single entry in the PUT body that replaces a `suggested_reviewers` artefact's content. + * Reviewers are identified by `github_login`, `user_uuid`, or both — the server canonicalizes + * to a lowercase `github_login` (with `user_uuid` winning when both are supplied). + */ +export interface SuggestedReviewerWriteEntry { + github_login?: string; + user_uuid?: string; + github_name?: string; +} + interface MatchedSignalMetadata { parent_signal_id: string; match_query: string; diff --git a/apps/code/src/shared/types/analytics.ts b/apps/code/src/shared/types/analytics.ts index b029dfb24b..5379d9b9fa 100644 --- a/apps/code/src/shared/types/analytics.ts +++ b/apps/code/src/shared/types/analytics.ts @@ -503,6 +503,8 @@ export type InboxReportActionType = | "view_signal_external" | "expand_why" | "click_suggested_reviewer" + | "add_suggested_reviewer" + | "remove_suggested_reviewer" | "expand_task_section" | "play_session_recording"; From 72aaeb15ccc093cf39a5bed91d8a4f9399844e64 Mon Sep 17 00:00:00 2001 From: Oliver Browne Date: Thu, 4 Jun 2026 12:18:02 +0300 Subject: [PATCH 2/4] refactor(inbox): polish suggested-reviewers editor after review - Toast on PUT failure to match sibling mutation hooks (silent rollback was inconsistent with useCreatePrReport / bulk actions) - Fix picker spinner gate: show "No users found" rather than a spinner when a non-matching filter coincides with a background refetch (gate on base-list presence, not filtered length) Generated-By: PostHog Code Task-Id: f961c44f-0c0b-4bff-ab85-b533bf1856ce --- .../inbox/components/detail/SuggestedReviewersEditor.tsx | 2 +- .../code/src/renderer/features/inbox/hooks/useInboxReports.ts | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/code/src/renderer/features/inbox/components/detail/SuggestedReviewersEditor.tsx b/apps/code/src/renderer/features/inbox/components/detail/SuggestedReviewersEditor.tsx index fd7914a3e8..c875d10c94 100644 --- a/apps/code/src/renderer/features/inbox/components/detail/SuggestedReviewersEditor.tsx +++ b/apps/code/src/renderer/features/inbox/components/detail/SuggestedReviewersEditor.tsx @@ -223,7 +223,7 @@ export function SuggestedReviewersEditor({ - {isFetching && addableOptions.length === 0 ? ( + {isFetching && !availableReviewers?.results?.length ? ( diff --git a/apps/code/src/renderer/features/inbox/hooks/useInboxReports.ts b/apps/code/src/renderer/features/inbox/hooks/useInboxReports.ts index 021c1b46db..ed0ae58fc4 100644 --- a/apps/code/src/renderer/features/inbox/hooks/useInboxReports.ts +++ b/apps/code/src/renderer/features/inbox/hooks/useInboxReports.ts @@ -19,6 +19,7 @@ import type { } from "@shared/types"; import { useQueryClient } from "@tanstack/react-query"; import { useEffect, useMemo } from "react"; +import { toast } from "sonner"; const REPORTS_PAGE_SIZE = 100; @@ -260,13 +261,14 @@ export function useUpdateSuggestedReviewers(reportId: string) { return { previous }; }, - onError: (_error, _variables, context) => { + onError: (error, _variables, context) => { const previous = ( context as { previous?: SignalReportArtefactsResponse } )?.previous; if (previous) { queryClient.setQueryData(queryKey, previous); } + toast.error(error.message || "Failed to update suggested reviewers"); }, onSettled: () => { queryClient.invalidateQueries({ queryKey }); From 1d3f74e99d58a39ab327f5f13e8dabc877667932 Mon Sep 17 00:00:00 2001 From: Oliver Browne Date: Thu, 4 Jun 2026 12:25:42 +0300 Subject: [PATCH 3/4] test(inbox): parameterise updateSignalReportArtefact success cases Collapse the two structurally-identical success-path cases into an it.each block per the project's parameterised-test preference (Greptile review). Generated-By: PostHog Code Task-Id: f961c44f-0c0b-4bff-ab85-b533bf1856ce --- .../src/renderer/api/posthogClient.test.ts | 73 ++++++------------- 1 file changed, 23 insertions(+), 50 deletions(-) diff --git a/apps/code/src/renderer/api/posthogClient.test.ts b/apps/code/src/renderer/api/posthogClient.test.ts index 4fd603f8f7..19d4d04347 100644 --- a/apps/code/src/renderer/api/posthogClient.test.ts +++ b/apps/code/src/renderer/api/posthogClient.test.ts @@ -405,21 +405,32 @@ describe("PostHogAPIClient", () => { return client; } - it("PUTs the full-replacement content and returns the parsed artefact", async () => { + const OCTOCAT_REVIEWER = { + github_login: "octocat", + github_name: "The Octocat", + relevant_commits: [], + user: null, + }; + + it.each([ + { + name: "PUTs the full-replacement content and returns the parsed artefact", + input: [{ github_login: "octocat" }, { user_uuid: "uuid-1" }], + responseContent: [OCTOCAT_REVIEWER], + }, + { + name: "sends an empty content array when clearing reviewers", + input: [], + responseContent: [], + }, + ])("$name", async ({ input, responseContent }) => { const fetch = vi.fn().mockResolvedValue({ ok: true, json: async () => ({ id: "art-1", type: "suggested_reviewers", created_at: "2024-01-01T00:00:00Z", - content: [ - { - github_login: "octocat", - github_name: "The Octocat", - relevant_commits: [], - user: null, - }, - ], + content: responseContent, }), }); const client = makeClient(fetch); @@ -427,54 +438,16 @@ describe("PostHogAPIClient", () => { const result = await client.updateSignalReportArtefact( "report-1", "art-1", - [{ github_login: "octocat" }, { user_uuid: "uuid-1" }], + input, ); expect(result.type).toBe("suggested_reviewers"); - expect(result.content).toEqual([ - { - github_login: "octocat", - github_name: "The Octocat", - relevant_commits: [], - user: null, - }, - ]); + expect(result.content).toEqual(responseContent); expect(fetch).toHaveBeenCalledWith( expect.objectContaining({ method: "put", path: ARTEFACT_PATH, - overrides: { - body: JSON.stringify({ - content: [{ github_login: "octocat" }, { user_uuid: "uuid-1" }], - }), - }, - }), - ); - }); - - it("sends an empty content array when clearing reviewers", async () => { - const fetch = vi.fn().mockResolvedValue({ - ok: true, - json: async () => ({ - id: "art-1", - type: "suggested_reviewers", - created_at: "2024-01-01T00:00:00Z", - content: [], - }), - }); - const client = makeClient(fetch); - - const result = await client.updateSignalReportArtefact( - "report-1", - "art-1", - [], - ); - - expect(result.content).toEqual([]); - expect(fetch).toHaveBeenCalledWith( - expect.objectContaining({ - method: "put", - overrides: { body: JSON.stringify({ content: [] }) }, + overrides: { body: JSON.stringify({ content: input }) }, }), ); }); From c0802a3810ce64452ff37385880e3396ab79d1fc Mon Sep 17 00:00:00 2001 From: Oliver Browne Date: Thu, 4 Jun 2026 12:38:16 +0300 Subject: [PATCH 4/4] feat(inbox): record reviewer identity on suggested-reviewer events The add/remove/click suggested-reviewer actions already fire the shared INBOX_REPORT_ACTION event (with report_id). Attach the target reviewer's github_login and user uuid so the events capture which reviewer changed, following the existing optional-extras pattern (signal_id, why_field). Generated-By: PostHog Code Task-Id: f961c44f-0c0b-4bff-ab85-b533bf1856ce --- .../detail/SuggestedReviewersEditor.tsx | 24 +++++++++++++++---- apps/code/src/shared/types/analytics.ts | 4 ++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/apps/code/src/renderer/features/inbox/components/detail/SuggestedReviewersEditor.tsx b/apps/code/src/renderer/features/inbox/components/detail/SuggestedReviewersEditor.tsx index c875d10c94..299b8955c0 100644 --- a/apps/code/src/renderer/features/inbox/components/detail/SuggestedReviewersEditor.tsx +++ b/apps/code/src/renderer/features/inbox/components/detail/SuggestedReviewersEditor.tsx @@ -32,11 +32,17 @@ type ReviewerAction = | "add_suggested_reviewer" | "remove_suggested_reviewer"; +/** Identity of the reviewer the action targeted, attached to the analytics event. */ +interface ReviewerActionExtra { + suggested_reviewer_login?: string; + suggested_reviewer_uuid?: string; +} + interface SuggestedReviewersEditorProps { reportId: string; artefact: SuggestedReviewersArtefact; meUuid: string | undefined; - fireAction: (action: ReviewerAction) => void; + fireAction: (action: ReviewerAction, extra?: ReviewerActionExtra) => void; } function isRowMe( @@ -137,7 +143,10 @@ export function SuggestedReviewersEditor({ const removeReviewer = (target: SuggestedReviewer) => { const next = reviewers.filter((r) => r !== target); - fireAction("remove_suggested_reviewer"); + fireAction("remove_suggested_reviewer", { + suggested_reviewer_login: target.github_login || undefined, + suggested_reviewer_uuid: target.user?.uuid, + }); updateReviewers({ artefactId: artefact.id, content: toWriteContent(next), @@ -165,7 +174,10 @@ export function SuggestedReviewersEditor({ }, }; const next = [...reviewers, optimisticEntry]; - fireAction("add_suggested_reviewer"); + fireAction("add_suggested_reviewer", { + suggested_reviewer_login: option.github_login || undefined, + suggested_reviewer_uuid: option.uuid, + }); updateReviewers({ artefactId: artefact.id, // Keep existing by login, add the new one by uuid (server canonicalizes). @@ -337,7 +349,11 @@ export function SuggestedReviewersEditor({ target="_blank" rel="noreferrer" className="inline-flex items-center gap-0.5 text-[11px] text-gray-9 hover:text-gray-11" - onClick={() => fireAction("click_suggested_reviewer")} + onClick={() => + fireAction("click_suggested_reviewer", { + suggested_reviewer_login: reviewer.github_login, + }) + } > @{reviewer.github_login} diff --git a/apps/code/src/shared/types/analytics.ts b/apps/code/src/shared/types/analytics.ts index 5379d9b9fa..df312790a5 100644 --- a/apps/code/src/shared/types/analytics.ts +++ b/apps/code/src/shared/types/analytics.ts @@ -609,6 +609,10 @@ export interface InboxReportActionProperties { signal_section?: "relevant_code" | "data_queried"; why_field?: "priority" | "actionability"; task_section?: "research" | "implementation"; + // Identity of the reviewer added/removed/clicked, for the suggested-reviewer + // actions. Login is absent when the org-member lookup doesn't expose one. + suggested_reviewer_login?: string; + suggested_reviewer_uuid?: string; // True when the user submitted Discuss with a first question via the popover. has_question?: boolean; // The first question text the user typed before hitting Discuss. Truncated to