From d5242cb011c28f6df4d7991a3f440ab814281c8d Mon Sep 17 00:00:00 2001 From: Rassl Date: Wed, 2 Sep 2026 18:58:45 +0400 Subject: [PATCH] feat: reviewer filter, decided sort and analytics popover on reviews page - decided views (Approved/Dismissed/Failed/All) gain a 'Recently decided' sort and an Admin/Workflow/System/Other reviewer filter; both reset when returning to Pending, and the node-type chips follow the filter. - new bar-chart button opens a 7-day analytics popover: totals, per-day stacked approved/dismissed/failed bars and a reviewer breakdown, scoped to the active action chip via the new /v2/reviews/stats endpoint. - AnchoredPopover learns align='end' so a far-right trigger opens its surface leftwards instead of past the viewport edge. --- src/app/admin/reviews/page.tsx | 48 +++- src/components/admin/review-stats-popover.tsx | 201 ++++++++++++++++ src/components/ui/anchored-popover.tsx | 14 +- src/lib/__tests__/reviews.test.tsx | 216 ++++++++++++++++++ src/lib/graph-api.ts | 107 ++++++++- 5 files changed, 576 insertions(+), 10 deletions(-) create mode 100644 src/components/admin/review-stats-popover.tsx diff --git a/src/app/admin/reviews/page.tsx b/src/app/admin/reviews/page.tsx index bae2ef3..25af2b4 100644 --- a/src/app/admin/reviews/page.tsx +++ b/src/app/admin/reviews/page.tsx @@ -9,8 +9,9 @@ import type { LucideIcon } from "lucide-react" import { useReviewStore } from "@/stores/review-store" import { useSchemaStore } from "@/stores/schema-store" import { approveReview, dismissReview, listReviews, getReviewNodeTypeCounts, triggerMergeWorkflow } from "@/lib/graph-api" -import type { Review, ReviewStatus } from "@/lib/graph-api" +import type { DeciderCategory, Review, ReviewStatus } from "@/lib/graph-api" import { ReviewRow, getApproveVerb } from "@/components/admin/review-row" +import { ReviewStatsPopover } from "@/components/admin/review-stats-popover" import { Button } from "@/components/ui/button" import { Checkbox } from "@/components/ui/checkbox" import { SelectCustom } from "@/components/ui/select-custom" @@ -43,6 +44,18 @@ const SORT_OPTIONS = [ { label: "Highest priority", value: "priority" }, ] +// decided_at is null on pending rows, so this sort only makes sense once the +// view can contain decided reviews. +const DECIDED_SORT_OPTION = { label: "Recently decided", value: "decided_at" } + +const DECIDER_OPTIONS: { label: string; value: DeciderCategory | "" }[] = [ + { label: "All reviewers", value: "" }, + { label: "Admin", value: "admin" }, + { label: "Workflow", value: "workflow" }, + { label: "System", value: "system" }, + { label: "Other", value: "other" }, +] + const PAGE_SIZE = 50 function SkeletonRows() { @@ -80,6 +93,13 @@ export default function ReviewsPage() { const [statusFilter, setStatusFilter] = useState("pending") const [actionFilter, setActionFilter] = useState("") const [sort, setSort] = useState("created_at") + const [deciderFilter, setDeciderFilter] = useState("") + + // The decided-only controls live on every tab but Pending. + const onDecidedView = statusFilter !== "pending" + const sortOptions = onDecidedView + ? [...SORT_OPTIONS, DECIDED_SORT_OPTION] + : SORT_OPTIONS const [searchQuery, setSearchQuery] = useState("") const debouncedSearch = useDebounce(searchQuery, 300) @@ -124,6 +144,7 @@ export default function ReviewsPage() { limit: PAGE_SIZE, search: debouncedSearch || undefined, node_type: nodeTypeFilter || undefined, + decider: deciderFilter || undefined, }, ctrl.signal ) @@ -151,7 +172,7 @@ export default function ReviewsPage() { if (!options?.silent) setLoading(false) } }, - [statusFilter, actionFilter, sort, debouncedSearch, nodeTypeFilter] + [statusFilter, actionFilter, sort, debouncedSearch, nodeTypeFilter, deciderFilter] ) useEffect(() => { @@ -167,12 +188,13 @@ export default function ReviewsPage() { status: statusFilter || undefined, action_name: actionFilter || undefined, search: debouncedSearch || undefined, + decider: deciderFilter || undefined, }) setNodeTypeCounts(res.counts) setTruncatedCounts(res.truncated) } catch {} }, 300) - }, [statusFilter, actionFilter, debouncedSearch]) + }, [statusFilter, actionFilter, debouncedSearch, deciderFilter]) useEffect(() => { fetchNodeTypeCounts() @@ -401,7 +423,13 @@ export default function ReviewsPage() { + + setOpen(false)} + matchWidth={false} + align="end" + maxHeight={420} + className="w-[300px] rounded-lg border border-border bg-popover p-3 shadow-lg" + > +
+
+ + Decisions — last {WINDOW_DAYS} days + + {actionName && ( + {actionName} + )} +
+ + {error ? ( +

+ Failed to load analytics +

+ ) : !stats && loading ? ( +
+ {Array.from({ length: 4 }).map((_, i) => ( +
+ ))} +
+ ) : stats ? ( +
+ {/* Totals */} +

+ + {stats.totals.total} + {" "} + decided ·{" "} + {stats.totals.approved} approved ·{" "} + {stats.totals.dismissed} dismissed + {stats.totals.failed > 0 && ( + <> + {" "}· {stats.totals.failed} failed + + )} +

+ + {/* Per-day stacked bars */} +
+ {stats.days.map((d, i) => ( +
+ + {weekdayLabel(d.day, i, stats.days.length)} + +
+ {d.total > 0 && ( +
+ {d.approved > 0 && ( +
+ )} + {d.dismissed > 0 && ( +
+ )} + {d.failed > 0 && ( +
+ )} +
+ )} +
+ + {d.total} + +
+ ))} +
+ + {/* Reviewer breakdown */} +
+

+ By reviewer +

+
+ {DECIDER_ORDER.filter((k) => stats.totals.deciders[k] > 0).map( + (k) => ( + + {DECIDER_LABELS[k]} + + {stats.totals.deciders[k]} + + + ) + )} + {stats.totals.total === 0 && ( + + No decisions in this window + + )} +
+
+
+ ) : null} +
+ +
+ ) +} diff --git a/src/components/ui/anchored-popover.tsx b/src/components/ui/anchored-popover.tsx index 9157f62..15ef80b 100644 --- a/src/components/ui/anchored-popover.tsx +++ b/src/components/ui/anchored-popover.tsx @@ -17,10 +17,15 @@ interface AnchoredPopoverProps { // Upper bound on the surface height so a long list stays a tidy, scrollable // popover instead of stretching to fill the viewport. maxHeight?: number + // Which edge of the anchor the popover lines up with. "start" (default) + // pins the left edges together; "end" pins the right edges — use it for + // triggers near the right viewport edge so the surface grows leftwards. + align?: "start" | "end" } interface Position { - left: number + left?: number + right?: number width?: number top?: number bottom?: number @@ -50,6 +55,7 @@ export function AnchoredPopover({ matchWidth = true, gap = 6, maxHeight = DEFAULT_MAX_HEIGHT, + align = "start", }: AnchoredPopoverProps) { const popoverRef = React.useRef(null) const [mounted, setMounted] = React.useState(false) @@ -67,14 +73,15 @@ export function AnchoredPopover({ const placeBelow = spaceBelow >= PREFERRED_MIN || spaceBelow >= spaceAbove const available = placeBelow ? spaceBelow : spaceAbove setPos({ - left: r.left, + left: align === "end" ? undefined : r.left, + right: align === "end" ? window.innerWidth - r.right : undefined, width: matchWidth ? r.width : undefined, top: placeBelow ? r.bottom + gap : undefined, bottom: placeBelow ? undefined : window.innerHeight - r.top + gap, // Cap to a tidy size, but never exceed the room actually available. maxHeight: Math.min(maxHeight, Math.max(120, available)), }) - }, [anchorRef, gap, matchWidth, maxHeight]) + }, [anchorRef, gap, matchWidth, maxHeight, align]) // Position on open and keep it pinned as the page scrolls/resizes. Capture // scroll so we also catch scrolling inside ancestor containers (the modal). @@ -113,6 +120,7 @@ export function AnchoredPopover({ style={{ position: "fixed", left: pos.left, + right: pos.right, top: pos.top, bottom: pos.bottom, width: pos.width, diff --git a/src/lib/__tests__/reviews.test.tsx b/src/lib/__tests__/reviews.test.tsx index e936dd6..89afef1 100644 --- a/src/lib/__tests__/reviews.test.tsx +++ b/src/lib/__tests__/reviews.test.tsx @@ -12,6 +12,7 @@ const { mockGetReviewNodeTypeCounts, mockTriggerMergeWorkflow, mockGetLatestStakworkRun, + mockGetReviewStats, } = vi.hoisted(() => ({ mockApproveReview: vi.fn(), mockDismissReview: vi.fn(), @@ -19,6 +20,7 @@ const { mockGetReviewNodeTypeCounts: vi.fn(), mockTriggerMergeWorkflow: vi.fn(), mockGetLatestStakworkRun: vi.fn(), + mockGetReviewStats: vi.fn(), })) vi.mock("@/lib/graph-api", async (importOriginal) => { @@ -31,6 +33,7 @@ vi.mock("@/lib/graph-api", async (importOriginal) => { getReviewNodeTypeCounts: (...args: unknown[]) => mockGetReviewNodeTypeCounts(...args), triggerMergeWorkflow: (...args: unknown[]) => mockTriggerMergeWorkflow(...args), getLatestStakworkRun: (...args: unknown[]) => mockGetLatestStakworkRun(...args), + getReviewStats: (...args: unknown[]) => mockGetReviewStats(...args), } }) @@ -2245,3 +2248,216 @@ describe("Toolkit non-admin", () => { expect(queryByLabelText("Reviews")).toBeNull() }) }) + +// ── deciderCategory ────────────────────────────────────────────────────────── + +describe("deciderCategory", () => { + it("classifies decided_by values into reviewer categories", async () => { + const { deciderCategory } = await import("@/lib/graph-api") + expect(deciderCategory("admin")).toBe("admin") + // historical boltwall-stamped pubkey (66 hex chars) + expect(deciderCategory("02723956fa52318a" + "a".repeat(50))).toBe("admin") + expect(deciderCategory("stakwork")).toBe("workflow") + expect(deciderCategory("stakwork-job")).toBe("workflow") + expect(deciderCategory("stawork-job")).toBe("workflow") + expect(deciderCategory("system")).toBe("system") + expect(deciderCategory("system:migration_121")).toBe("system") + expect(deciderCategory("")).toBe("other") + expect(deciderCategory(undefined)).toBe("other") + expect(deciderCategory("someone-else")).toBe("other") + }) +}) + +// ── ReviewStatsPopover ─────────────────────────────────────────────────────── + +describe("ReviewStatsPopover", () => { + beforeEach(() => { + vi.clearAllMocks() + mockGetReviewStats.mockResolvedValue({ + days: [ + { + day: "2026-09-01", + total: 3, + approved: 2, + dismissed: 1, + failed: 0, + deciders: { admin: 1, workflow: 2, system: 0, other: 0 }, + }, + { + day: "2026-09-02", + total: 5, + approved: 4, + dismissed: 0, + failed: 1, + deciders: { admin: 4, workflow: 1, system: 0, other: 0 }, + }, + ], + totals: { + total: 8, + approved: 6, + dismissed: 1, + failed: 1, + deciders: { admin: 5, workflow: 3, system: 0, other: 0 }, + }, + window_days: 7, + since: 0, + }) + }) + + it("fetches and shows totals plus reviewer breakdown when opened", async () => { + const user = userEvent.setup() + const { ReviewStatsPopover } = await import( + "@/components/admin/review-stats-popover" + ) + render() + + expect(mockGetReviewStats).not.toHaveBeenCalled() + await user.click(screen.getByTestId("review-stats-btn")) + + await waitFor(() => { + expect(screen.getByTestId("review-stats-panel")).toBeTruthy() + }) + expect(mockGetReviewStats).toHaveBeenCalledWith( + expect.objectContaining({ days: 7, action_name: "merge_nodes" }) + ) + await waitFor(() => { + expect(screen.getByText("8")).toBeTruthy() + expect(screen.getByText("6 approved")).toBeTruthy() + expect(screen.getByText("1 dismissed")).toBeTruthy() + expect(screen.getByText("1 failed")).toBeTruthy() + expect(screen.getByText("Admin")).toBeTruthy() + expect(screen.getByText("Workflow")).toBeTruthy() + }) + // zero-count reviewer chips are hidden + expect(screen.queryByText("System")).toBeNull() + }) + + it("shows an error message when the stats fetch fails", async () => { + mockGetReviewStats.mockRejectedValue(new Error("boom")) + const user = userEvent.setup() + const { ReviewStatsPopover } = await import( + "@/components/admin/review-stats-popover" + ) + render() + + await user.click(screen.getByTestId("review-stats-btn")) + await waitFor(() => { + expect(screen.getByText("Failed to load analytics")).toBeTruthy() + }) + }) +}) + +// ── ReviewsPage decided-view controls ──────────────────────────────────────── + +describe("ReviewsPage decided-view controls", () => { + beforeEach(async () => { + vi.resetModules() + vi.clearAllMocks() + mockListReviews.mockResolvedValue({ reviews: [], total: 0, skip: 0, limit: 20 }) + mockGetReviewNodeTypeCounts.mockResolvedValue({ counts: {}, truncated: false }) + }) + + async function renderPage() { + vi.doMock("@/stores/review-store", () => ({ + useReviewStore: () => ({ pendingCount: 0, setPendingCount: vi.fn() }), + })) + vi.doMock("@/stores/schema-store", () => ({ + useSchemaStore: (sel: (s: { schemas: never[] }) => unknown) => + sel({ schemas: [] }), + })) + vi.doMock("@/components/admin/review-row", () => ({ + ReviewRow: ({ review }: { review: Review }) => ( +
{review.rationale}
+ ), + getApproveVerb: (action: string) => action, + })) + // Render selects as native so options are clickable in the test + vi.doMock("@/components/ui/select-custom", () => ({ + SelectCustom: ({ + value, + onChange, + options, + }: { + value: string + onChange: (v: string) => void + options: { label: string; value: string }[] + }) => ( + + ), + })) + vi.doMock("@/components/ui/checkbox", () => ({ + Checkbox: () => , + })) + const { default: ReviewsPage } = await import("@/app/admin/reviews/page") + return render() + } + + it("hides reviewer filter and decided sort on the Pending tab", async () => { + await renderPage() + await waitFor(() => expect(mockListReviews).toHaveBeenCalled()) + expect(screen.queryByText("All reviewers")).toBeNull() + expect(screen.queryByText("Recently decided")).toBeNull() + }) + + it("shows both controls on the Approved tab and passes the decider filter", async () => { + const user = userEvent.setup() + await renderPage() + await waitFor(() => expect(mockListReviews).toHaveBeenCalled()) + + await user.click(screen.getByRole("button", { name: "Approved" })) + await waitFor(() => { + expect(screen.getByText("All reviewers")).toBeTruthy() + expect(screen.getByText("Recently decided")).toBeTruthy() + }) + + const deciderSelect = screen + .getByText("All reviewers") + .closest("select") as HTMLSelectElement + await user.selectOptions(deciderSelect, "workflow") + await waitFor(() => { + expect(mockListReviews).toHaveBeenCalledWith( + expect.objectContaining({ status: "approved", decider: "workflow" }), + expect.anything() + ) + }) + }) + + it("clears the decider filter and decided sort when returning to Pending", async () => { + const user = userEvent.setup() + await renderPage() + await waitFor(() => expect(mockListReviews).toHaveBeenCalled()) + + await user.click(screen.getByRole("button", { name: "Approved" })) + const sortSelect = (await screen.findByText("Recently decided")).closest( + "select" + ) as HTMLSelectElement + await user.selectOptions(sortSelect, "decided_at") + await waitFor(() => { + expect(mockListReviews).toHaveBeenCalledWith( + expect.objectContaining({ sort: "decided_at" }), + expect.anything() + ) + }) + + // The pending-badge refresh also calls listReviews (limit: 1, no sort), so + // assert on the page fetch's shape rather than whichever call landed last. + mockListReviews.mockClear() + await user.click(screen.getByRole("button", { name: "Pending" })) + await waitFor(() => { + expect(mockListReviews).toHaveBeenCalledWith( + expect.objectContaining({ status: "pending", sort: "created_at" }), + expect.anything() + ) + }) + const decidedSortCalls = mockListReviews.mock.calls.filter( + ([params]) => params?.sort === "decided_at" || params?.decider !== undefined + ) + expect(decidedSortCalls).toHaveLength(0) + }) +}) diff --git a/src/lib/graph-api.ts b/src/lib/graph-api.ts index a0f2a46..842a307 100644 --- a/src/lib/graph-api.ts +++ b/src/lib/graph-api.ts @@ -1027,8 +1027,22 @@ function getMockReviewsStore(): Review[] { return _mockReviewsStore } +// Reviewer categories for the decided-by filter and the analytics popover. +// Mirrors the backend's DECIDER_PREDICATES: 'admin' includes the historical +// boltwall-stamped pubkeys, 'workflow' the Stakwork job labels (typos +// included), 'system' the reconciliation/migration tombstones. +export type DeciderCategory = "admin" | "workflow" | "system" | "other" + +export function deciderCategory(decidedBy?: string | null): DeciderCategory { + const value = (decidedBy ?? "").trim() + if (value === "admin" || /^[0-9a-f]{64,66}$/i.test(value)) return "admin" + if (value.includes("stakwork") || value.includes("stawork")) return "workflow" + if (value.startsWith("system")) return "system" + return "other" +} + export async function listReviews( - params?: { status?: ReviewStatus; type?: string; action_name?: string; run_ref_id?: string; sort?: string; skip?: number; limit?: number; search?: string; node_type?: string }, + params?: { status?: ReviewStatus; type?: string; action_name?: string; run_ref_id?: string; sort?: string; skip?: number; limit?: number; search?: string; node_type?: string; decider?: DeciderCategory }, signal?: AbortSignal ): Promise { if (isMocksEnabled()) { @@ -1038,6 +1052,7 @@ export async function listReviews( if (params?.type) filtered = filtered.filter((r) => r.type === params.type) if (params?.action_name) filtered = filtered.filter((r) => r.action_name === params.action_name) if (params?.run_ref_id) filtered = filtered.filter((r) => r.run_ref_id === params.run_ref_id) + if (params?.decider) filtered = filtered.filter((r) => deciderCategory(r.decided_by) === params.decider) if (params?.search) { const q = params.search.toLowerCase() filtered = filtered.filter( @@ -1056,6 +1071,9 @@ export async function listReviews( const sort = params?.sort ?? "created_at" if (sort === "priority") { filtered.sort((a, b) => b.priority - a.priority) + } else if (sort === "decided_at") { + const decidedMs = (r: Review) => (r.decided_at ? new Date(r.decided_at).getTime() : 0) + filtered.sort((a, b) => decidedMs(b) - decidedMs(a)) } else { filtered.sort((a, b) => new Date(b.created_at).getTime() - new Date(a.created_at).getTime()) } @@ -1079,11 +1097,12 @@ export async function listReviews( if (params?.limit !== undefined) qs.set("limit", String(params.limit)) if (params?.search) qs.set("search", params.search) if (params?.node_type) qs.set("node_type", params.node_type) + if (params?.decider) qs.set("decider", params.decider) return api.get(`/v2/reviews?${qs}`, undefined, signal) } export async function getReviewNodeTypeCounts( - params?: { status?: ReviewStatus; action_name?: string; search?: string }, + params?: { status?: ReviewStatus; action_name?: string; search?: string; decider?: DeciderCategory }, signal?: AbortSignal ): Promise<{ counts: Record; truncated: boolean }> { if (isMocksEnabled()) { @@ -1091,6 +1110,7 @@ export async function getReviewNodeTypeCounts( let filtered = [...store] if (params?.status) filtered = filtered.filter((r) => r.status === params.status) if (params?.action_name) filtered = filtered.filter((r) => r.action_name === params.action_name) + if (params?.decider) filtered = filtered.filter((r) => deciderCategory(r.decided_by) === params.decider) if (params?.search) { const q = params.search.toLowerCase() filtered = filtered.filter( @@ -1121,6 +1141,7 @@ export async function getReviewNodeTypeCounts( if (params?.status) qs.set("status", params.status) if (params?.action_name) qs.set("action_name", params.action_name) if (params?.search) qs.set("search", params.search) + if (params?.decider) qs.set("decider", params.decider) return api.get<{ counts: Record; truncated: boolean }>( `/v2/reviews/node_type_counts?${qs}`, undefined, @@ -1128,6 +1149,88 @@ export async function getReviewNodeTypeCounts( ) } +export interface ReviewStatsBucket { + total: number + approved: number + dismissed: number + failed: number + deciders: Record +} + +export interface ReviewStatsDay extends ReviewStatsBucket { + // ISO date (local to the requested tz offset), e.g. "2026-09-02" + day: string +} + +export interface ReviewStatsResponse { + days: ReviewStatsDay[] + totals: ReviewStatsBucket + window_days: number + since: number +} + +function emptyStatsBucket(): ReviewStatsBucket { + return { + total: 0, + approved: 0, + dismissed: 0, + failed: 0, + deciders: { admin: 0, workflow: 0, system: 0, other: 0 }, + } +} + +/** + * Per-day decision counts for the reviews analytics popover. Days are bucketed + * on the browser's local midnight — the backend takes our UTC offset so its + * "today" matches the operator's. + */ +export async function getReviewStats( + params?: { days?: number; action_name?: string }, + signal?: AbortSignal +): Promise { + const days = params?.days ?? 7 + if (isMocksEnabled()) { + const store = getMockReviewsStore() + const dayKey = (iso: string) => { + const d = new Date(iso) + return `${d.getFullYear()}-${String(d.getMonth() + 1).padStart(2, "0")}-${String(d.getDate()).padStart(2, "0")}` + } + const start = new Date() + start.setHours(0, 0, 0, 0) + start.setDate(start.getDate() - (days - 1)) + const dayBuckets: ReviewStatsDay[] = Array.from({ length: days }, (_, i) => { + const d = new Date(start) + d.setDate(start.getDate() + i) + return { day: dayKey(d.toISOString()), ...emptyStatsBucket() } + }) + const byDay = new Map(dayBuckets.map((b) => [b.day, b])) + const totals = emptyStatsBucket() + for (const review of store) { + if (review.status === "pending" || !review.decided_at) continue + if (params?.action_name && review.action_name !== params.action_name) continue + const bucket = byDay.get(dayKey(review.decided_at)) + if (!bucket) continue + const category = deciderCategory(review.decided_by) + for (const b of [bucket, totals]) { + b.total += 1 + if (review.status === "approved" || review.status === "dismissed" || review.status === "failed") { + b[review.status] += 1 + } + b.deciders[category] += 1 + } + } + return { days: dayBuckets, totals, window_days: days, since: start.getTime() / 1000 } + } + + const qs = new URLSearchParams() + qs.set("days", String(days)) + // JS getTimezoneOffset() is minutes *behind* UTC (UTC+4 → -240), the API + // wants minutes ahead, hence the negation. + qs.set("tz_offset_minutes", String(-new Date().getTimezoneOffset())) + if (params?.action_name) qs.set("action_name", params.action_name) + return api.get(`/v2/reviews/stats?${qs}`, undefined, signal) +} + /** * Fetch the property table proposed for a scratchpad_entry review's new type. *