From bab53e31cadeb133a92e878be506e2b5174e6b7f Mon Sep 17 00:00:00 2001 From: Renzo F <45401804+rennf93@users.noreply.github.com> Date: Sun, 19 Jul 2026 19:42:24 +0200 Subject: [PATCH] perf(panel): kanban virtualization + row memoization + scorecards batch endpoint (#594) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * perf(panel): kanban virtualization + row memoization + scorecards batch endpoint The audit's remaining phases: the kanban card lists render through @tanstack/react-virtual windows (columns are the dnd drop targets, cards only drag — no sortable conflict) with memoized columns/cards and a stabilized handleAction; the task table's desktop row and mobile card are extracted and memoized (the table itself already client-paginates to 100). Backend: the per-member scorecard N+1 (~20 requests x 3 queries per poll) collapses into GET /dashboard/metrics/members backed by get_all_member_scorecards with grouped rollup/overlay SQL shared with the single-agent path. * test(metrics): shared-DB-safe scorecard tests — unique seeds, delta assertions The two new batch-scorecard tests assumed a private DB: fixed ceo/system slugs collided with other tests' seeds (ix_agents_slug) and a global exactly-one CEO lookup + exact roster count broke in the one-process suite. Unique slugs, subset/disjoint assertions, dead count constant dropped. --------- Co-authored-by: Renn F --- panel/package.json | 1 + panel/pnpm-lock.yaml | 20 + .../components/kanban/core/kanban-board.tsx | 148 ++-- .../components/kanban/core/kanban-card.tsx | 11 +- .../components/kanban/core/kanban-column.tsx | 73 +- .../metrics/__tests__/scorecards-tab.test.tsx | 35 +- .../src/components/metrics/scorecards-tab.tsx | 44 +- panel/src/components/tasks/task-table.tsx | 673 ++++++++++-------- panel/src/hooks/use-observability.ts | 18 + panel/src/lib/api/observability.ts | 14 + roboco/api/routes/dashboard.py | 14 + roboco/services/metrics.py | 198 +++++- tests/integration/test_dashboard_routes.py | 46 ++ tests/integration/test_metrics_scorecards.py | 156 ++++ 14 files changed, 1009 insertions(+), 442 deletions(-) diff --git a/panel/package.json b/panel/package.json index 1cb4e85b..326d52d9 100644 --- a/panel/package.json +++ b/panel/package.json @@ -37,6 +37,7 @@ "@radix-ui/react-tooltip": "^1.2.12", "@tailwindcss/typography": "^0.5.20", "@tanstack/react-query": "^5.101.2", + "@tanstack/react-virtual": "3.14.6", "axios": "^1.16.0", "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", diff --git a/panel/pnpm-lock.yaml b/panel/pnpm-lock.yaml index 85da7f91..5ed21ae8 100644 --- a/panel/pnpm-lock.yaml +++ b/panel/pnpm-lock.yaml @@ -74,6 +74,9 @@ importers: '@tanstack/react-query': specifier: ^5.101.2 version: 5.101.2(react@19.2.7) + '@tanstack/react-virtual': + specifier: 3.14.6 + version: 3.14.6(react-dom@19.2.7(react@19.2.7))(react@19.2.7) axios: specifier: ^1.16.0 version: 1.18.1 @@ -1397,6 +1400,15 @@ packages: peerDependencies: react: ^18 || ^19 + '@tanstack/react-virtual@3.14.6': + resolution: {integrity: sha512-4+Uq8m0/gzO4kMCHUEpTtGX1RnONK0C+g88b2ltwPMWUBiaVarBuWKoPJaz7gj1cKCVRAdyu+U8GcKhwCc2beA==} + peerDependencies: + react: ^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0 + react-dom: ^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0 + + '@tanstack/virtual-core@3.17.4': + resolution: {integrity: sha512-nGm5KteqxasUdThLc2izl6dHUqLv0LQj7Nuyo5gYalTPf/U8a9ermvsl7reT+6ioBW1l8WfpP/mcU338nLXpqw==} + '@testing-library/dom@10.4.1': resolution: {integrity: sha512-o4PXJQidqJl82ckFaXUeoAW+XysPLauYI43Abki5hABd853iMhitooc6znOnczgbTYmEP6U6/y1ZyKAIsvMKGg==} engines: {node: '>=18'} @@ -5009,6 +5021,14 @@ snapshots: '@tanstack/query-core': 5.101.2 react: 19.2.7 + '@tanstack/react-virtual@3.14.6(react-dom@19.2.7(react@19.2.7))(react@19.2.7)': + dependencies: + '@tanstack/virtual-core': 3.17.4 + react: 19.2.7 + react-dom: 19.2.7(react@19.2.7) + + '@tanstack/virtual-core@3.17.4': {} + '@testing-library/dom@10.4.1': dependencies: '@babel/code-frame': 7.29.7 diff --git a/panel/src/components/kanban/core/kanban-board.tsx b/panel/src/components/kanban/core/kanban-board.tsx index bac3ac4d..ba899914 100644 --- a/panel/src/components/kanban/core/kanban-board.tsx +++ b/panel/src/components/kanban/core/kanban-board.tsx @@ -28,7 +28,7 @@ import { useSensor, useSensors, } from "@dnd-kit/core"; -import { useEffect, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { KanbanCard } from "./kanban-card"; import { RequiredNotesDialog } from "@/components/tasks/task-detail/task-action-dialogs"; import { skippedPreconditions } from "./bypass-preconditions"; @@ -104,6 +104,23 @@ export function KanbanBoard({ const updateTask = useUpdateTask(); const [activeTask, setActiveTask] = useState(null); + // Latest tasks snapshot for handleAction below, read via ref instead of a + // closure — React Query hands back a new array reference on every refetch + // even when the underlying rows are unchanged, which would otherwise bust + // handleAction's identity (and, through it, KanbanCard's memoization) every + // 30s regardless of whether anything actually changed. + const tasksRef = useRef([]); + tasksRef.current = tasks || []; + + // useMutation's mutate/mutateAsync are bound once per observer and stay + // referentially stable across renders (query-core binds them in the + // constructor) — pulling them out lets handleAction's useCallback below + // stay stable even though `lifecycle` itself is a fresh object every render. + const claimMutateAsync = lifecycle.claim.mutateAsync; + const startMutateAsync = lifecycle.start.mutateAsync; + const submitQaMutateAsync = lifecycle.submitQa.mutateAsync; + const unblockMutateAsync = lifecycle.unblock.mutateAsync; + const { register, unregister } = usePageRefresh(); useEffect(() => { @@ -127,13 +144,21 @@ export function KanbanBoard({ }), ); - // Group tasks by status - const tasksByStatus = columns.reduce( - (acc, col) => { - acc[col.status] = (tasks || []).filter((t) => t.status === col.status); - return acc; - }, - {} as Record, + // Group tasks by status. Memoized so each column's `tasks` prop keeps a + // stable reference across renders that don't actually change the data — + // required for KanbanColumn/KanbanCard's React.memo to skip re-rendering. + const tasksByStatus = useMemo( + () => + columns.reduce( + (acc, col) => { + acc[col.status] = (tasks || []).filter( + (t) => t.status === col.status, + ); + return acc; + }, + {} as Record, + ), + [tasks, columns], ); const handleDragStart = (event: DragStartEvent) => { @@ -223,57 +248,66 @@ export function KanbanBoard({ } }; - const handleAction = async (action: string, taskId: string) => { - try { - switch (action) { - case "move-forward": - // Determine next action based on current status - const task = tasks?.find((t) => t.id === taskId); - if (!task) return; + const handleAction = useCallback( + async (action: string, taskId: string) => { + try { + switch (action) { + case "move-forward": + // Determine next action based on current status + const task = tasksRef.current.find((t) => t.id === taskId); + if (!task) return; - switch (task.status) { - case TaskStatus.BACKLOG: - // BACKLOG tasks need session before activation - PM only - toast.info( - "Backlog tasks must be activated by PM with a session", - ); - break; - case TaskStatus.PENDING: - await lifecycle.claim.mutateAsync(taskId); - toast.success("Task claimed"); - break; - case TaskStatus.CLAIMED: - await lifecycle.start.mutateAsync(taskId); - toast.success("Task started"); - break; - case TaskStatus.IN_PROGRESS: - await lifecycle.submitQa.mutateAsync({ taskId }); - toast.success("Submitted for QA"); - break; - case TaskStatus.BLOCKED: - await lifecycle.unblock.mutateAsync(taskId); - toast.success("Task unblocked"); - break; - case TaskStatus.AWAITING_QA: - setPendingNotesAction({ kind: "pass-qa", taskId }); - return; // Dialog collects the required note - case TaskStatus.AWAITING_DOCUMENTATION: - setPendingNotesAction({ kind: "complete", taskId }); - return; // Dialog collects the required justification - } - break; - case "pass-qa": - setPendingNotesAction({ kind: "pass-qa", taskId }); - return; // Dialog collects the required note - case "fail-qa": - setPendingNotesAction({ kind: "fail-qa", taskId }); - return; // Dialog collects the required note + switch (task.status) { + case TaskStatus.BACKLOG: + // BACKLOG tasks need session before activation - PM only + toast.info( + "Backlog tasks must be activated by PM with a session", + ); + break; + case TaskStatus.PENDING: + await claimMutateAsync(taskId); + toast.success("Task claimed"); + break; + case TaskStatus.CLAIMED: + await startMutateAsync(taskId); + toast.success("Task started"); + break; + case TaskStatus.IN_PROGRESS: + await submitQaMutateAsync({ taskId }); + toast.success("Submitted for QA"); + break; + case TaskStatus.BLOCKED: + await unblockMutateAsync(taskId); + toast.success("Task unblocked"); + break; + case TaskStatus.AWAITING_QA: + setPendingNotesAction({ kind: "pass-qa", taskId }); + return; // Dialog collects the required note + case TaskStatus.AWAITING_DOCUMENTATION: + setPendingNotesAction({ kind: "complete", taskId }); + return; // Dialog collects the required justification + } + break; + case "pass-qa": + setPendingNotesAction({ kind: "pass-qa", taskId }); + return; // Dialog collects the required note + case "fail-qa": + setPendingNotesAction({ kind: "fail-qa", taskId }); + return; // Dialog collects the required note + } + refetch(); + } catch { + toast.error("Action failed"); } - refetch(); - } catch { - toast.error("Action failed"); - } - }; + }, + [ + claimMutateAsync, + startMutateAsync, + submitQaMutateAsync, + unblockMutateAsync, + refetch, + ], + ); const handleNotesConfirm = async (text: string) => { if (!pendingNotesAction) return; diff --git a/panel/src/components/kanban/core/kanban-card.tsx b/panel/src/components/kanban/core/kanban-card.tsx index 4066886a..04feb1cd 100644 --- a/panel/src/components/kanban/core/kanban-card.tsx +++ b/panel/src/components/kanban/core/kanban-card.tsx @@ -1,6 +1,6 @@ "use client"; -import { useState } from "react"; +import { memo, useState } from "react"; import { Task, TaskStatus } from "@/types"; import { useUpdateTask } from "@/hooks/use-tasks"; import { Card, CardContent } from "@/components/ui/card"; @@ -44,7 +44,12 @@ interface KanbanCardProps { isDragging?: boolean; } -export function KanbanCard({ +// Memoized: a kanban column can mount hundreds of these across all statuses. +// `task` stays referentially stable across React Query refetches for +// unchanged rows (structural sharing), and `onAction` is stabilized by the +// board — so the default shallow prop comparison actually skips re-renders +// instead of every card re-rendering on any board-level state change. +function KanbanCardImpl({ task, onAction, showQaActions, @@ -302,3 +307,5 @@ export function KanbanCard({ ); } + +export const KanbanCard = memo(KanbanCardImpl); diff --git a/panel/src/components/kanban/core/kanban-column.tsx b/panel/src/components/kanban/core/kanban-column.tsx index 72c058d8..252a2437 100644 --- a/panel/src/components/kanban/core/kanban-column.tsx +++ b/panel/src/components/kanban/core/kanban-column.tsx @@ -1,5 +1,6 @@ "use client"; +import { memo, useRef } from "react"; import { Task, TaskStatus } from "@/types"; import { Badge } from "@/components/ui/badge"; import { Skeleton } from "@/components/ui/skeleton"; @@ -12,8 +13,14 @@ import { HelpTip } from "@/components/ui/help-tip"; import { taskStatusDescription } from "@/components/tasks/task-status-badge"; import { KanbanCard } from "./kanban-card"; import { useDroppable } from "@dnd-kit/core"; +import { useVirtualizer } from "@tanstack/react-virtual"; import { cn } from "@/lib/utils"; +// Rough collapsed-card height (title + badges + assign row) used as the +// virtualizer's initial estimate — corrected per-card via measureElement +// once mounted, so a wrong guess only costs one extra frame of scroll jitter. +const ESTIMATED_CARD_HEIGHT = 132; + interface KanbanColumnProps { id: string; title: string; @@ -27,7 +34,11 @@ interface KanbanColumnProps { className?: string; } -export function KanbanColumn({ +// Memoized: KanbanBoard renders one of these per lifecycle-status column, and +// a column's `tasks` slice is stabilized upstream (kanban-board.tsx's +// tasksByStatus useMemo) so an unrelated board re-render (dialogs, drag +// state) skips every column that didn't actually change. +function KanbanColumnImpl({ id: _id, title, status, @@ -42,6 +53,20 @@ export function KanbanColumn({ const { setNodeRef, isOver } = useDroppable({ id: status, }); + const scrollRef = useRef(null); + // A column can carry hundreds of cards (every task in a status, across a + // whole team's history) — window the DOM to roughly what's on screen + // instead of mounting every dnd-kit draggable card at once. Safe with + // dnd-kit here because the drop target is the column itself + // (useDroppable above), not per-card — cards only register useDraggable, + // no sortable reordering depends on every card's DOM node existing. + const rowVirtualizer = useVirtualizer({ + count: tasks.length, + getScrollElement: () => scrollRef.current, + estimateSize: () => ESTIMATED_CARD_HEIGHT, + overscan: 6, + getItemKey: (index) => tasks[index]?.id ?? index, + }); return (
{/* Native overflow scroll: Radix ScrollArea's display:table viewport let cards grow past the column width and clip — a plain div keeps - content constrained to the column. */} -
+ content constrained to the column. Also the virtualizer's scroll + container. */} +
{isLoading ? (
@@ -101,22 +127,37 @@ export function KanbanColumn({ No tasks
) : ( -
- {tasks.map((task) => ( - - ))} +
+ {rowVirtualizer.getVirtualItems().map((virtualRow) => { + const task = tasks[virtualRow.index]; + return ( +
+ +
+ ); + })}
)}
); } + +export const KanbanColumn = memo(KanbanColumnImpl); diff --git a/panel/src/components/metrics/__tests__/scorecards-tab.test.tsx b/panel/src/components/metrics/__tests__/scorecards-tab.test.tsx index 3514cbe2..98ba3292 100644 --- a/panel/src/components/metrics/__tests__/scorecards-tab.test.tsx +++ b/panel/src/components/metrics/__tests__/scorecards-tab.test.tsx @@ -14,7 +14,7 @@ const { mockOrg, mockCeo, mockMember, mockAgents } = vi.hoisted(() => ({ vi.mock("@/hooks/use-observability", () => ({ useOrgScorecard: mockOrg, useCeoScorecard: mockCeo, - useMemberScorecard: mockMember, + useAllMemberScorecards: mockMember, })); vi.mock("@/hooks/use-agents", () => ({ @@ -96,18 +96,21 @@ describe("ScorecardsTabContent", () => { isLoading: false, }); mockMember.mockReturnValue({ - data: member({ - id: "dev1", - name: "be-dev-1", - tasks_completed: 7, - first_pass_yield: 0.8, - active_runtime_hours: 4.2, - qa_pass_rate: 0.9, - escalations: 1, - blocked_others: 2, - utilization: 0.6, - }), + data: [ + member({ + id: "dev1", + name: "be-dev-1", + tasks_completed: 7, + first_pass_yield: 0.8, + active_runtime_hours: 4.2, + qa_pass_rate: 0.9, + escalations: 1, + blocked_others: 2, + utilization: 0.6, + }), + ], isLoading: false, + isError: false, }); mockAgents.mockReturnValue({ data: [ @@ -164,8 +167,10 @@ describe("ScorecardsTabContent", () => { expect( screen.getByText(/failed to load organization metrics/i), ).toBeInTheDocument(); - // The member row shows a failed marker rather than a perpetual skeleton - // (exact lowercase text, distinct from the org card's message). - expect(screen.getByText("failed to load")).toBeInTheDocument(); + // The batched member-scorecard fetch surfaces one table-level banner + // rather than a per-row error (or an endless per-row skeleton). + expect( + screen.getByText(/failed to load member scorecards/i), + ).toBeInTheDocument(); }); }); diff --git a/panel/src/components/metrics/scorecards-tab.tsx b/panel/src/components/metrics/scorecards-tab.tsx index 29593c22..298b9639 100644 --- a/panel/src/components/metrics/scorecards-tab.tsx +++ b/panel/src/components/metrics/scorecards-tab.tsx @@ -1,5 +1,6 @@ "use client"; +import { useMemo } from "react"; import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; import { Skeleton } from "@/components/ui/skeleton"; import { Badge } from "@/components/ui/badge"; @@ -13,12 +14,12 @@ import { TableRow, } from "@/components/ui/table"; import { + useAllMemberScorecards, useCeoScorecard, - useMemberScorecard, useOrgScorecard, } from "@/hooks/use-observability"; import { useAgents } from "@/hooks/use-agents"; -import { AgentRole, type Agent } from "@/types"; +import { AgentRole, type Agent, type MemberScorecard } from "@/types"; function pctOrNa(rate: number | null): string { return rate === null ? "n/a" : (rate * 100).toFixed(0) + "%"; @@ -32,20 +33,18 @@ function hoursOrDash(seconds: number): string { return (seconds / 3600).toFixed(1) + "h"; } -/** One member's row — each row self-fetches its rollup scorecard. */ -function MemberRow({ agent }: { agent: Agent }) { - const { data, isLoading, isError } = useMemberScorecard(agent.id); - if (isError) { - return ( - - {agent.name || agent.slug} - - failed to load - - - ); - } - if (isLoading || !data) { +/** One member's row. Data comes from the batched useAllMemberScorecards + * fetch (one request for the whole table) rather than each row self-fetching + * — ~20 agents used to mean ~20 parallel `/metrics/member/{id}` requests + * (each 3 DB queries) on every table poll. */ +function MemberRow({ + agent, + data, +}: { + agent: Agent; + data: MemberScorecard | undefined; +}) { + if (!data) { return ( {agent.name || agent.slug} @@ -172,6 +171,12 @@ export function ScorecardsTabContent() { const members = (agents ?? []).filter( (a) => a.role !== AgentRole.CEO && a.role !== AgentRole.SYSTEM, ); + const { data: scorecards, isError: scorecardsError } = + useAllMemberScorecards(); + const scorecardById = useMemo( + () => new Map((scorecards ?? []).map((s) => [s.id, s])), + [scorecards], + ); return (
@@ -197,6 +202,11 @@ export function ScorecardsTabContent() { Members + {scorecardsError && ( +

+ Failed to load member scorecards. +

+ )} @@ -245,7 +255,7 @@ export function ScorecardsTabContent() { {members.map((a) => ( - + ))}
diff --git a/panel/src/components/tasks/task-table.tsx b/panel/src/components/tasks/task-table.tsx index 2760ee17..a2f70276 100644 --- a/panel/src/components/tasks/task-table.tsx +++ b/panel/src/components/tasks/task-table.tsx @@ -1,6 +1,6 @@ "use client"; -import { useState, useMemo, useEffect } from "react"; +import { useState, useMemo, useCallback, useEffect, memo } from "react"; import { Task } from "@/types"; import { Card, CardContent } from "@/components/ui/card"; import { Badge } from "@/components/ui/badge"; @@ -262,6 +262,319 @@ function SortableHeader({ ); } +interface TaskRowProps { + node: TaskTreeNode; + isExpanded: boolean; + childCount: number; + projectNames: Record; + projectGitUrls: Record; + productNames: Record; + onToggleExpand: (taskId: string) => void; +} + +// Memoized: a page renders up to 100 of these (PAGE_SIZE_OPTIONS caps there) +// and re-renders on every sort/page/expand change or unrelated parent update +// — memoizing stops an unchanged row's markup from being torn down and +// rebuilt every time. `node` stays referentially stable across React Query +// refetches for unchanged tasks (structural sharing); projectNames/ +// projectGitUrls/productNames/onToggleExpand are stabilized by the caller. +const TaskTableRow = memo(function TaskTableRow({ + node, + isExpanded, + childCount, + projectNames, + projectGitUrls, + productNames, + onToggleExpand, +}: TaskRowProps) { + const task = node.task; + const hasChildren = node.children.length > 0; + + const handleRowClick = (e: React.MouseEvent) => { + // Don't toggle if clicking on interactive elements + const target = e.target as HTMLElement; + if ( + target.closest("a") || + target.closest("button") || + target.closest('[role="button"]') || + target.closest("[data-no-expand]") + ) { + return; + } + if (hasChildren) { + onToggleExpand(task.id); + } + }; + + return ( + 0 && "bg-muted/20", + hasChildren && "cursor-pointer", + )} + onClick={handleRowClick} + > + +
+ {hasChildren ? ( + + + + ) : ( + + )} + +
+ + {task.title} + + {task.batch_id && !task.parent_task_id && ( + + + MegaTask + + + )} + {childCount > 0 && ( + + + {childCount} subtask + {childCount !== 1 ? "s" : ""} + + + )} +
+ +
+ + + + + + + + + {task.team.replace(/_/g, " ")} + + + {task.project_id && projectNames[task.project_id] ? ( + {projectNames[task.project_id]} + ) : task.product_id && productNames[task.product_id] ? ( + + {productNames[task.product_id]}{" "} + (product) + + ) : ( + — + )} + + + + {priorityLabels[task.priority] ?? "P2 - Medium"} + + + + + + {getAgentDisplayName(task.assigned_to)} + + + + + + + {formatDistanceToNow(new Date(task.created_at), { + addSuffix: true, + })} + + + + + + + + ); +}); + +// Mobile card counterpart of TaskTableRow — same memoization rationale. +const TaskTableCard = memo(function TaskTableCard({ + node, + isExpanded, + childCount, + projectNames, + projectGitUrls, + productNames, + onToggleExpand, +}: TaskRowProps) { + const task = node.task; + const hasChildren = node.children.length > 0; + + return ( + +
+
+
+ {hasChildren && ( + + + + )} + + {task.title} + +
+ {(task.batch_id && !task.parent_task_id) || childCount > 0 ? ( +
+ {task.batch_id && !task.parent_task_id && ( + + + MegaTask + + + )} + {childCount > 0 && ( + + + {childCount} subtask + {childCount !== 1 ? "s" : ""} + + + )} +
+ ) : null} +
+ +
+ +
+ + + + + + + + {task.team.replace(/_/g, " ")} + + + {task.project_id && projectNames[task.project_id] + ? projectNames[task.project_id] + : task.product_id && productNames[task.product_id] + ? `${productNames[task.product_id]} (product)` + : "—"} + + + + {priorityLabels[task.priority] ?? "P2 - Medium"} + + + + + + {getAgentDisplayName(task.assigned_to)} + + + + + + + {formatDistanceToNow(new Date(task.created_at), { + addSuffix: true, + })} + + + +
+ + ); +}); + export function TaskTable({ tasks, isLoading, @@ -431,19 +744,25 @@ export function TaskTable({ } }; - const toggleExpand = (taskId: string) => { - const next = new Set(expandedIds); - if (next.has(taskId)) { - next.delete(taskId); - } else { - next.add(taskId); - } - if (onExpandedChange) { - onExpandedChange(next); - } else { - setInternalExpandedIds(next); - } - }; + // Stable across renders that don't touch expand state, so TaskTableRow's + // memoization actually holds on an unrelated re-render (e.g. a sibling + // page-chrome state change) instead of every row rebuilding regardless. + const toggleExpand = useCallback( + (taskId: string) => { + const next = new Set(expandedIds); + if (next.has(taskId)) { + next.delete(taskId); + } else { + next.add(taskId); + } + if (onExpandedChange) { + onExpandedChange(next); + } else { + setInternalExpandedIds(next); + } + }, + [expandedIds, onExpandedChange], + ); const expandAll = () => { const allParentIds = new Set(); @@ -563,171 +882,18 @@ export function TaskTable({ ) : paginatedTasks.length === 0 ? ( ) : ( - paginatedTasks.map((node) => { - const task = node.task; - const hasChildren = node.children.length > 0; - const isExpanded = expandedIds.has(task.id); - const childCount = childrenMap.get(task.id)?.length || 0; - - const handleRowClick = (e: React.MouseEvent) => { - // Don't toggle if clicking on interactive elements - const target = e.target as HTMLElement; - if ( - target.closest("a") || - target.closest("button") || - target.closest('[role="button"]') || - target.closest("[data-no-expand]") - ) { - return; - } - if (hasChildren) { - toggleExpand(task.id); - } - }; - - return ( - 0 && "bg-muted/20", - hasChildren && "cursor-pointer", - )} - onClick={handleRowClick} - > - -
- {hasChildren ? ( - - - - ) : ( - - )} - -
- - {task.title} - - {task.batch_id && !task.parent_task_id && ( - - - MegaTask - - - )} - {childCount > 0 && ( - - - {childCount} subtask - {childCount !== 1 ? "s" : ""} - - - )} -
- -
- - - - - - - - - {task.team.replace(/_/g, " ")} - - - {task.project_id && projectNames[task.project_id] ? ( - {projectNames[task.project_id]} - ) : task.product_id && - productNames[task.product_id] ? ( - - {productNames[task.product_id]}{" "} - (product) - - ) : ( - — - )} - - - - {priorityLabels[task.priority] ?? "P2 - Medium"} - - - - - - {getAgentDisplayName(task.assigned_to)} - - - - - - - {formatDistanceToNow(new Date(task.created_at), { - addSuffix: true, - })} - - - - - - - - ); - }) + paginatedTasks.map((node) => ( + + )) )} @@ -741,143 +907,18 @@ export function TaskTable({ ) : ( - {paginatedTasks.map((node) => { - const task = node.task; - const hasChildren = node.children.length > 0; - const isExpanded = expandedIds.has(task.id); - const childCount = childrenMap.get(task.id)?.length || 0; - - return ( - -
-
-
- {hasChildren && ( - - - - )} - - {task.title} - -
- {(task.batch_id && !task.parent_task_id) || - childCount > 0 ? ( -
- {task.batch_id && !task.parent_task_id && ( - - - MegaTask - - - )} - {childCount > 0 && ( - - - {childCount} subtask - {childCount !== 1 ? "s" : ""} - - - )} -
- ) : null} -
- -
- -
- - - - - - - - - {task.team.replace(/_/g, " ")} - - - - {task.project_id && projectNames[task.project_id] - ? projectNames[task.project_id] - : task.product_id && productNames[task.product_id] - ? `${productNames[task.product_id]} (product)` - : "—"} - - - - {priorityLabels[task.priority] ?? "P2 - Medium"} - - - - - - {getAgentDisplayName(task.assigned_to)} - - - - - - - {formatDistanceToNow(new Date(task.created_at), { - addSuffix: true, - })} - - - -
- - ); - })} + {paginatedTasks.map((node) => ( + + ))} ) } diff --git a/panel/src/hooks/use-observability.ts b/panel/src/hooks/use-observability.ts index 5fc6d841..b7980fd0 100644 --- a/panel/src/hooks/use-observability.ts +++ b/panel/src/hooks/use-observability.ts @@ -31,6 +31,14 @@ export const observabilityKeys = { [...observabilityKeys.all, "scorecard", "ceo", days] as const, memberScorecard: (agentId: string, days: number) => [...observabilityKeys.all, "scorecard", "member", agentId, days] as const, + allMemberScorecards: (days: number, team?: string) => + [ + ...observabilityKeys.all, + "scorecard", + "members", + team ?? "all", + days, + ] as const, orgScorecard: (days: number, team?: string) => [ ...observabilityKeys.all, @@ -115,6 +123,16 @@ export function useMemberScorecard(agentId: string, days = 30) { }); } +/** Every agent's rollup scorecard in one batch — replaces the N+1 pattern of + * calling useMemberScorecard once per row in a member list/table. */ +export function useAllMemberScorecards(days = 30, team?: string) { + return useQuery({ + queryKey: observabilityKeys.allMemberScorecards(days, team), + queryFn: () => observabilityApi.getAllMemberScorecards(days, team), + refetchInterval: SCORECARD_REFETCH_INTERVAL, + }); +} + /** Org-wide (or per-cell) rollup aggregate. */ export function useOrgScorecard(days = 30, team?: string) { return useQuery({ diff --git a/panel/src/lib/api/observability.ts b/panel/src/lib/api/observability.ts index efa0c2fd..4ef7a03e 100644 --- a/panel/src/lib/api/observability.ts +++ b/panel/src/lib/api/observability.ts @@ -167,6 +167,20 @@ export const observabilityApi = { return data; }, + /** Every agent's rollup scorecard in one batch (N+1 fix for the panel's + * Members table) — GET /dashboard/metrics/members?team&days */ + getAllMemberScorecards: async ( + days = 30, + team?: string, + ): Promise => { + if (isMockMode()) return []; + const { data } = await api.get( + "/dashboard/metrics/members", + { params: { days, ...(team ? { team } : {}) } }, + ); + return data; + }, + /** Org / team rollup — GET /dashboard/metrics/org?team&days */ getOrgScorecard: async (days = 30, team?: string): Promise => { if (isMockMode()) return emptyOrg(team ?? null); diff --git a/roboco/api/routes/dashboard.py b/roboco/api/routes/dashboard.py index 2483c82b..1a79aea2 100644 --- a/roboco/api/routes/dashboard.py +++ b/roboco/api/routes/dashboard.py @@ -631,6 +631,20 @@ async def get_member_scorecard( return card.to_dict() +@router.get("/metrics/members") +async def get_all_member_scorecards( + db: DbSession, + team: Team | None = None, + days: int = Query(default=30, ge=1, le=90), +) -> list[dict[str, Any]]: + """Every (non-CEO, non-system) agent's rollup scorecard in one batch — + backs the panel's Members table without an N+1 (one request here instead + of one `/metrics/member/{id}` request per agent on the roster).""" + metrics_service = get_metrics_service(db) + cards = await metrics_service.get_all_member_scorecards(team=team, days=days) + return [card.to_dict() for card in cards] + + @router.get("/metrics/org") async def get_org_scorecard( db: DbSession, diff --git a/roboco/services/metrics.py b/roboco/services/metrics.py index 3fcbb915..0f7ae3c4 100644 --- a/roboco/services/metrics.py +++ b/roboco/services/metrics.py @@ -21,7 +21,7 @@ from roboco.db.tables import ( TaskTable, ) from roboco.foundation.policy.stage_effort import compute_stage_effort -from roboco.models.base import TaskStatus, Team +from roboco.models.base import AgentRole, TaskStatus, Team from roboco.models.metrics import ( CEO_APPROVAL_DECISIONS, CEO_UNBLOCK_DECISIONS, @@ -1117,25 +1117,25 @@ class MetricsService(BaseService): """Guarded ratio → None when the denominator is 0.""" return round(numerator / denominator, 4) if denominator else None - async def get_member_scorecard( - self, agent_id: UUID, days: int = 30 - ) -> MemberScorecard | None: - """Per-member rollup scorecard + live in-flight overlay, or None if no - such agent.""" - agent = ( - await self.session.execute( - select(AgentTable.slug, AgentTable.name).where( - AgentTable.id == agent_id - ) - ) - ).one_or_none() - if agent is None: - return None - slug, name = agent - since_date = (datetime.now(UTC) - timedelta(days=days)).date() - sums, _ = await self._rollup_sums(since_date, agent_slug=slug, team=None) + _EMPTY_OVERLAY: ClassVar[dict[str, float]] = { + "active_runtime_seconds": 0.0, + "turns": 0.0, + "tool_calls": 0.0, + "tokens": 0.0, + "cost_usd": 0.0, + } - overlay = await self._live_inflight_overlay(agent_id) + def _assemble_member_scorecard( + self, + *, + agent_id: UUID, + name: str, + sums: dict[str, float], + overlay: dict[str, float], + ) -> MemberScorecard: + """Shared rollup+overlay -> MemberScorecard builder for both the + single-agent (`get_member_scorecard`) and batch + (`get_all_member_scorecards`) paths — one derivation, no duplication.""" includes_live = any(v for v in overlay.values()) active_runtime = ( sums["active_runtime_seconds"] + overlay["active_runtime_seconds"] @@ -1175,6 +1175,166 @@ class MetricsService(BaseService): includes_live_inflight=includes_live, ) + async def get_member_scorecard( + self, agent_id: UUID, days: int = 30 + ) -> MemberScorecard | None: + """Per-member rollup scorecard + live in-flight overlay, or None if no + such agent.""" + agent = ( + await self.session.execute( + select(AgentTable.slug, AgentTable.name).where( + AgentTable.id == agent_id + ) + ) + ).one_or_none() + if agent is None: + return None + slug, name = agent + since_date = (datetime.now(UTC) - timedelta(days=days)).date() + sums, _ = await self._rollup_sums(since_date, agent_slug=slug, team=None) + overlay = await self._live_inflight_overlay(agent_id) + return self._assemble_member_scorecard( + agent_id=agent_id, name=name, sums=sums, overlay=overlay + ) + + async def _rollup_sums_by_agent( + self, since_date: Any, *, team: Team | None + ) -> dict[str, dict[str, float]]: + """Batch counterpart of `_rollup_sums`: one query grouped by + agent_slug instead of one query per agent filtered to that slug.""" + cols = [ + func.coalesce( + func.sum(getattr(MemberPerformanceDailyTable, name)), 0 + ).label(name) + for name in self._ROLLUP_SUM_COLUMNS + ] + conds: list[Any] = [ + MemberPerformanceDailyTable.member_kind == "agent", + MemberPerformanceDailyTable.date >= since_date, + ] + if team is not None: + conds.append(MemberPerformanceDailyTable.team == team.value) + rows = ( + await self.session.execute( + select(MemberPerformanceDailyTable.agent_slug, *cols) + .where(and_(*conds)) + .group_by(MemberPerformanceDailyTable.agent_slug) + ) + ).all() + return { + row.agent_slug: { + name: float(getattr(row, name) or 0) + for name in self._ROLLUP_SUM_COLUMNS + } + for row in rows + } + + async def _live_inflight_overlay_by_agent( + self, agent_ids: Sequence[UUID] + ) -> dict[UUID, dict[str, float]]: + """Batch counterpart of `_live_inflight_overlay`: one non-terminal-task + lookup + one grouped session-sum query across the whole roster, + instead of a per-agent task lookup + per-agent sum — the scorecard + N+1 (~20 agents meant ~40 extra queries on every panel Members-tab + poll). + """ + if not agent_ids: + return {} + task_rows = ( + await self.session.execute( + select(TaskTable.id, TaskTable.assigned_to).where( + TaskTable.assigned_to.in_(agent_ids), + TaskTable.status.notin_( + [TaskStatus.COMPLETED, TaskStatus.CANCELLED] + ), + ) + ) + ).all() + if not task_rows: + return {} + task_to_agent = {str(task_id): agent_id for task_id, agent_id in task_rows} + + s = AgentSpawnSessionTable + session_rows = ( + await self.session.execute( + select( + s.task_id, + func.coalesce( + func.sum(func.extract("epoch", func.now() - s.started_at)), 0 + ), + func.coalesce(func.sum(s.turns), 0), + func.coalesce(func.sum(s.tool_calls), 0), + func.coalesce( + func.sum( + s.tokens_input + + s.tokens_output + + s.tokens_cache_read + + s.tokens_cache_write + ), + 0, + ), + func.coalesce(func.sum(s.estimated_cost_usd), 0), + ) + .where( + s.task_id.in_(list(task_to_agent.keys())), + s.ended_at.is_(None), + ) + .group_by(s.task_id) + ) + ).all() + + # Each SQL column is already func.coalesce(..., 0)'d, so no Python-side + # `or 0` fallback is needed here (keeps this loop's branch count low). + overlays: dict[UUID, dict[str, float]] = {} + for task_id, runtime, turns, tool_calls, tokens, cost in session_rows: + agent_id = task_to_agent.get(task_id) + if agent_id is None: + continue + bucket = overlays.setdefault(agent_id, dict(self._EMPTY_OVERLAY)) + bucket["active_runtime_seconds"] += float(runtime) + bucket["turns"] += float(turns) + bucket["tool_calls"] += float(tool_calls) + bucket["tokens"] += float(tokens) + bucket["cost_usd"] += float(cost) + return overlays + + async def get_all_member_scorecards( + self, team: Team | None = None, days: int = 30 + ) -> list[MemberScorecard]: + """Every (non-CEO, non-system) agent's rollup scorecard in one batch — + the N+1 fix for the panel's Members table, which used to fire one + `/metrics/member/{id}` request (3 queries) per agent on the roster on + every poll instead of 3 queries total.""" + since_date = (datetime.now(UTC) - timedelta(days=days)).date() + conds: list[Any] = [AgentTable.role.notin_([AgentRole.CEO, AgentRole.SYSTEM])] + if team is not None: + conds.append(AgentTable.team == team) + agents = ( + await self.session.execute( + select(AgentTable.id, AgentTable.slug, AgentTable.name).where( + and_(*conds) + ) + ) + ).all() + if not agents: + return [] + + sums_by_slug = await self._rollup_sums_by_agent(since_date, team=team) + overlay_by_agent = await self._live_inflight_overlay_by_agent( + [agent_id for agent_id, _, _ in agents] + ) + empty_sums = dict.fromkeys(self._ROLLUP_SUM_COLUMNS, 0.0) + + return [ + self._assemble_member_scorecard( + agent_id=agent_id, + name=name, + sums=sums_by_slug.get(slug, empty_sums), + overlay=overlay_by_agent.get(agent_id, self._EMPTY_OVERLAY), + ) + for agent_id, slug, name in agents + ] + async def get_org_scorecard( self, team: Team | None = None, days: int = 30 ) -> OrgScorecard: diff --git a/tests/integration/test_dashboard_routes.py b/tests/integration/test_dashboard_routes.py index 2f53a229..66a76fbf 100644 --- a/tests/integration/test_dashboard_routes.py +++ b/tests/integration/test_dashboard_routes.py @@ -452,6 +452,52 @@ async def test_org_scorecard_endpoint(dashboard_client: AsyncClient) -> None: assert set(body) >= {"member_count", "tasks_completed", "first_pass_yield"} +@pytest.mark.asyncio +async def test_all_member_scorecards_endpoint( + dashboard_client: AsyncClient, db_session: AsyncSession +) -> None: + """The batch scorecard route (N+1 fix) returns one MemberScorecard per + non-CEO/non-system agent — the CEO seeded by the fixture is excluded.""" + dev = AgentTable( + id=uuid4(), + name="be-dev-1", + slug=f"be-dev-{uuid4().hex[:8]}", + role=AgentRole.DEVELOPER, + team=Team.BACKEND, + status=AgentStatus.ACTIVE, + model_config={}, + system_prompt="dev", + capabilities=[], + permissions={}, + metrics={}, + ) + db_session.add(dev) + await db_session.flush() + + resp = await dashboard_client.get("/api/dashboard/metrics/members", headers=_HDR) + assert resp.status_code == HTTPStatus.OK + body = resp.json() + assert isinstance(body, list) + ids = {card["id"] for card in body} + assert str(dev.id) in ids + # The shared per-run DB can hold several CEO rows (other tests seed + # their own); every one of them must be excluded from the batch. + ceo_ids = ( + ( + await db_session.execute( + select(AgentTable.id).where(AgentTable.role == AgentRole.CEO) + ) + ) + .scalars() + .all() + ) + assert ceo_ids + assert ids.isdisjoint({str(cid) for cid in ceo_ids}) + for card in body: + assert card["scope"] == "member" + assert card["member_kind"] == "agent" + + @pytest.mark.asyncio async def test_task_metrics_404_for_missing_task( dashboard_client: AsyncClient, diff --git a/tests/integration/test_metrics_scorecards.py b/tests/integration/test_metrics_scorecards.py index 6eb46fbd..2a38cc26 100644 --- a/tests/integration/test_metrics_scorecards.py +++ b/tests/integration/test_metrics_scorecards.py @@ -293,6 +293,162 @@ async def test_member_scorecard_404_and_guards( assert card.utilization is None +_DEV2_OVERLAY_TURNS = 7 + + +@pytest.mark.asyncio +async def test_all_member_scorecards_matches_single_agent_and_excludes_ceo_system( + svc: MetricsService, db_session: AsyncSession +) -> None: + """get_all_member_scorecards (the N+1 batch fix) must return the exact + same derived numbers get_member_scorecard would for the same agent, plus: + CEO/SYSTEM excluded, and a rollup-less agent still appears zeroed instead + of silently dropped.""" + # Unique slugs throughout: the suite shares one DB per run, so a fixed + # "ceo"/"system" slug collides with other tests' seeds (ix_agents_slug). + dev1 = _agent(AgentRole.DEVELOPER, f"be-dev-{uuid4().hex[:6]}") + dev2 = _agent(AgentRole.QA, f"be-qa-{uuid4().hex[:6]}") + dev3 = _agent(AgentRole.DEVELOPER, f"be-dev-{uuid4().hex[:6]}") # no rollup rows + ceo = _agent(AgentRole.CEO, f"ceo-{uuid4().hex[:6]}") + system = _agent(AgentRole.SYSTEM, f"system-{uuid4().hex[:6]}") + db_session.add_all([dev1, dev2, dev3, ceo, system]) + await db_session.flush() + db_session.add_all( + [ + _daily( + dev1.slug, + tasks_completed=2, + tasks_first_pass=1, + active_runtime_seconds=1800, + turns=6, + qa_reviews_total=3, + qa_reviews_passed=2, + idle_seconds=600, + ), + _daily(dev2.slug, tasks_completed=1, tasks_first_pass=1), + ] + ) + await db_session.flush() + + cards = await svc.get_all_member_scorecards(team=Team.BACKEND, days=30) + by_id = {c.id: c for c in cards} + # Delta assertions, not a global count: the one-process suite shares + # this DB, so other tests' agents may be on the roster too. + for seeded in (dev1, dev2, dev3): + assert str(seeded.id) in by_id + assert str(ceo.id) not in by_id + assert str(system.id) not in by_id + + single = await svc.get_member_scorecard(cast("UUID", dev1.id), days=30) + assert single is not None + batch_dev1 = by_id[str(dev1.id)] + assert batch_dev1.tasks_completed == single.tasks_completed + assert batch_dev1.turns == single.turns + assert batch_dev1.first_pass_yield == single.first_pass_yield + assert batch_dev1.qa_pass_rate == single.qa_pass_rate + assert batch_dev1.utilization == single.utilization + + # A roster member with no rollup rows still appears (zeroed), not dropped. + zeroed = by_id[str(dev3.id)] + assert zeroed.tasks_completed == 0 + assert zeroed.first_pass_yield is None + assert zeroed.utilization is None + + +@pytest.mark.asyncio +async def test_all_member_scorecards_attributes_live_overlay_per_agent( + svc: MetricsService, db_session: AsyncSession +) -> None: + """The batched overlay query must attribute each agent's OPEN spawn + session to that agent alone — not merge every in-flight agent's effort + into one bucket.""" + dev1 = _agent(AgentRole.DEVELOPER, f"be-dev-{uuid4().hex[:6]}") + dev2 = _agent(AgentRole.DEVELOPER, f"be-dev-{uuid4().hex[:6]}") + db_session.add_all([dev1, dev2]) + await db_session.flush() + project = ProjectTable( + id=uuid4(), + name="P", + slug=f"p-{uuid4().hex[:6]}", + git_url="https://example.com/r.git", + assigned_cell=Team.BACKEND, + created_by=dev1.id, + ) + db_session.add(project) + await db_session.flush() + db_session.add_all( + [ + _daily(dev1.slug, tasks_completed=1, active_runtime_seconds=100), + _daily(dev2.slug, tasks_completed=1, active_runtime_seconds=200), + ] + ) + + def _inflight(owner_id: UUID) -> TaskTable: + return TaskTable( + id=uuid4(), + title="t", + description="d", + acceptance_criteria=["ac"], + task_type=TaskType.CODE, + nature=TaskNature.TECHNICAL, + status=TaskStatus.IN_PROGRESS, + team=Team.BACKEND, + project_id=project.id, + created_by=owner_id, + assigned_to=owner_id, + estimated_complexity=Complexity.MEDIUM, + started_at=datetime.now(UTC) - timedelta(hours=1), + ) + + t1 = _inflight(cast("UUID", dev1.id)) + t2 = _inflight(cast("UUID", dev2.id)) + db_session.add_all([t1, t2]) + await db_session.flush() + now = datetime.now(UTC) + db_session.add_all( + [ + AgentSpawnSessionTable( + id=uuid4(), + agent_slug=dev1.slug, + team="backend", + role="developer", + model="claude", + task_id=str(t1.id), + started_at=now - timedelta(seconds=200), + ended_at=None, + turns=_OVERLAY_TURNS, + tool_calls=4, + tokens_input=10, + tokens_output=5, + estimated_cost_usd=0.2, + ), + AgentSpawnSessionTable( + id=uuid4(), + agent_slug=dev2.slug, + team="backend", + role="developer", + model="claude", + task_id=str(t2.id), + started_at=now - timedelta(seconds=500), + ended_at=None, + turns=_DEV2_OVERLAY_TURNS, + tool_calls=9, + tokens_input=20, + tokens_output=8, + estimated_cost_usd=0.5, + ), + ] + ) + await db_session.flush() + + cards = {c.id: c for c in await svc.get_all_member_scorecards(days=30)} + c1, c2 = cards[str(dev1.id)], cards[str(dev2.id)] + assert c1.includes_live_inflight is True + assert c2.includes_live_inflight is True + assert c1.turns == _OVERLAY_TURNS + assert c2.turns == _DEV2_OVERLAY_TURNS + + @pytest.mark.asyncio async def test_org_scorecard_aggregates_members( svc: MetricsService, db_session: AsyncSession