From 37b04d504ba44e9fb86edc94b328939c503cbf5a Mon Sep 17 00:00:00 2001 From: Shay Date: Thu, 28 May 2026 08:11:06 -0700 Subject: [PATCH] feat(workbench/evals): polish states, numeric alignment, lane URL state (wave 1d) --- workbench-ui/src/app/evals/EvalMetricGrid.tsx | 184 +++++++++-- workbench-ui/src/app/evals/EvalsRoute.tsx | 312 ++++++++++++------ workbench-ui/src/app/evals/evals.test.tsx | 172 +++++++++- .../CommandPalette.keyboard.test.tsx | 10 +- .../components/primitives/CommandPalette.tsx | 20 +- 5 files changed, 560 insertions(+), 138 deletions(-) diff --git a/workbench-ui/src/app/evals/EvalMetricGrid.tsx b/workbench-ui/src/app/evals/EvalMetricGrid.tsx index 076f34d6..d3497e11 100644 --- a/workbench-ui/src/app/evals/EvalMetricGrid.tsx +++ b/workbench-ui/src/app/evals/EvalMetricGrid.tsx @@ -5,7 +5,7 @@ export function EvalMetricGrid({ }: { metrics: Record; }) { - const sortedKeys = Object.keys(metrics).sort(); + const allKeys = Object.keys(metrics); function formatValue(value: unknown): string { if (typeof value === "boolean") { @@ -34,6 +34,20 @@ export function EvalMetricGrid({ return undefined; } + function isLowerIsBetter(key: string): boolean { + const k = key.toLowerCase(); + return ( + k.includes("latency") || + k.includes("duration") || + k.includes("time") || + k.includes("error") || + k.includes("fail") || + k.includes("fabrication") || + k.includes("divergence") || + k.includes("cost") + ); + } + function renderPassFailBadge(key: string, value: unknown) { const isBool = typeof value === "boolean"; const k = key.toLowerCase(); @@ -65,33 +79,159 @@ export function EvalMetricGrid({ return null; } + // Find pairs + const pairedKeys = new Set(); + const pairs: Array<{ + key: string; + actual: unknown; + target: unknown; + }> = []; + + // 1. Group "passed" and "total" + if (allKeys.includes("passed") && allKeys.includes("total")) { + pairs.push({ + key: "passed", + actual: metrics["passed"], + target: metrics["total"], + }); + pairedKeys.add("passed"); + pairedKeys.add("total"); + } + + // 2. Group "X" and "X_target" or "target_X" + allKeys.forEach((k) => { + if (pairedKeys.has(k)) return; + + if (k.endsWith("_target")) { + const baseKey = k.slice(0, -7); + if (allKeys.includes(baseKey)) { + pairs.push({ + key: baseKey, + actual: metrics[baseKey], + target: metrics[k], + }); + pairedKeys.add(baseKey); + pairedKeys.add(k); + } + } else if (k.startsWith("target_")) { + const baseKey = k.slice(7); + if (allKeys.includes(baseKey)) { + pairs.push({ + key: baseKey, + actual: metrics[baseKey], + target: metrics[k], + }); + pairedKeys.add(baseKey); + pairedKeys.add(k); + } + } + }); + + // 3. Remaining singles + const singles: Array<{ key: string; value: unknown }> = []; + allKeys.forEach((k) => { + if (!pairedKeys.has(k)) { + singles.push({ key: k, value: metrics[k] }); + } + }); + + interface DisplayMetric { + key: string; + isPair: boolean; + actual: unknown; + target?: unknown; + value?: unknown; + } + + const list: DisplayMetric[] = [ + ...pairs.map((p) => ({ key: p.key, isPair: true, actual: p.actual, target: p.target })), + ...singles.map((s) => ({ key: s.key, isPair: false, actual: s.value, value: s.value })), + ]; + + // Sort lexicographically + list.sort((a, b) => a.key.localeCompare(b.key)); + return (
- {sortedKeys.map((key) => { - const val = metrics[key]; + {list.map((item) => { + const key = item.key; const unit = getUnit(key); - const badge = renderPassFailBadge(key, val); - const formatted = formatValue(val); - const isObj = typeof val === "object" && val !== null; - return ( -
-
-
- {key.replaceAll("_", " ")} -
-
- {formatted} - {unit && {unit}} + if (item.isPair) { + const actualVal = item.actual; + const targetVal = item.target; + const badge = renderPassFailBadge(key, actualVal); + + const actualNum = Number(actualVal); + const targetNum = Number(targetVal); + const hasNumericValues = !isNaN(actualNum) && !isNaN(targetNum); + const isFailing = + hasNumericValues && + (isLowerIsBetter(key) ? actualNum > targetNum : actualNum < targetNum); + + const formattedActual = formatValue(actualVal); + const formattedTarget = formatValue(targetVal); + + return ( +
+
+
+ {key.replaceAll("_", " ")} +
+
+ + {formattedActual} + + / + + {formattedTarget} + + {unit && {unit}} +
+ {badge &&
{badge}
}
- {badge &&
{badge}
} -
- ); + ); + } else { + const val = item.value; + const badge = renderPassFailBadge(key, val); + const formatted = formatValue(val); + const isObj = typeof val === "object" && val !== null; + const isNum = typeof val === "number"; + + return ( +
+
+
+ {key.replaceAll("_", " ")} +
+
+ {formatted} + {unit && {unit}} +
+
+ {badge &&
{badge}
} +
+ ); + } })}
); diff --git a/workbench-ui/src/app/evals/EvalsRoute.tsx b/workbench-ui/src/app/evals/EvalsRoute.tsx index 13fc2bca..bdc3dd33 100644 --- a/workbench-ui/src/app/evals/EvalsRoute.tsx +++ b/workbench-ui/src/app/evals/EvalsRoute.tsx @@ -12,10 +12,33 @@ import { LoadingState } from "../../design/components/states/LoadingState"; import type { EvalRunResult } from "../../types/api"; import { WorkbenchApiError } from "../../api/client"; +import { useState, useEffect } from "react"; +import { useSearchParams } from "react-router-dom"; +import { useEvalLanes, useEvalLane } from "../../api/queries"; +import { EvalLaneCard } from "./EvalLaneCard"; +import { EvalRunButton } from "./EvalRunButton"; +import { EvalMetricGrid } from "./EvalMetricGrid"; +import { EvalFailureViewer } from "./EvalFailureViewer"; +import { EvalArtifactLink } from "./EvalArtifactLink"; +import { EmptyState } from "../../design/components/states/EmptyState"; +import { ErrorState } from "../../design/components/states/ErrorState"; +import { LoadingState } from "../../design/components/states/LoadingState"; +import type { EvalRunResult } from "../../types/api"; +import { WorkbenchApiError } from "../../api/client"; +import { useQueryClient } from "@tanstack/react-query"; + export function EvalsRoute() { const { data: lanes, isLoading, isError, error } = useEvalLanes(); const [searchParams, setSearchParams] = useSearchParams(); const selectedLaneName = searchParams.get("lane") || ""; + const queryClient = useQueryClient(); + + const { + data: lastRunResult, + isLoading: isLaneLoading, + isError: isLaneError, + error: laneError, + } = useEvalLane(selectedLaneName); // Maintain per-lane run states (pending, result, error) const [runStates, setRunStates] = useState< @@ -29,6 +52,13 @@ export function EvalsRoute() { > >({}); + // Auto-select the first lane on load if no selection is in the URL + useEffect(() => { + if (!selectedLaneName && lanes && lanes.length > 0) { + setSearchParams({ lane: lanes[0].lane }, { replace: true }); + } + }, [selectedLaneName, lanes, setSearchParams]); + if (isLoading) { return ; } @@ -46,6 +76,178 @@ export function EvalsRoute() { const selectedLane = lanes?.find((l) => l.lane === selectedLaneName) || null; const currentRunState = selectedLaneName ? runStates[selectedLaneName] : null; + const isNotFoundError = + laneError instanceof WorkbenchApiError && laneError.code === "not_found"; + + function renderHeaderAndForm(lane: typeof selectedLane) { + if (!lane) return null; + return ( + <> + {/* Header info */} +
+
+

+ {lane.lane} +

+ {lane.description && ( +

+ {lane.description} +

+ )} +
+
+ + {/* Run Button configuration if read-only */} + {lane.read_only ? ( + { + setRunStates((prev) => ({ + ...prev, + [lane.lane]: { isPending: true }, + })); + }} + onRunSuccess={(result) => { + setRunStates((prev) => ({ + ...prev, + [lane.lane]: { isPending: false, result }, + })); + queryClient.invalidateQueries({ queryKey: ["api", "eval", lane.lane] }); + }} + onRunError={(err) => { + setRunStates((prev) => ({ + ...prev, + [lane.lane]: { isPending: false, error: err }, + })); + }} + /> + ) : ( +
+ ⚠️ API runs disabled for write-active lane. Use local CLI to execute this lane: + + core eval --lane {lane.lane} + +
+ )} + + ); + } + + const renderRightPane = () => { + if (!selectedLaneName) { + return ( + + ); + } + + if (!selectedLane) { + return ( + + ); + } + + // 1. Loading state for fetching the lane's last run details + if (isLaneLoading && !lastRunResult) { + return ; + } + + // 2. Error state for fetching the lane details (excluding not_found) + if (isLaneError && !isNotFoundError) { + return ( + + ); + } + + // 3. Current run execution state + if (currentRunState?.isPending) { + return ( +
+ {renderHeaderAndForm(selectedLane)} + +
+ ); + } + + if (currentRunState?.error) { + return ( +
+ {renderHeaderAndForm(selectedLane)} + +
+ ); + } + + // 4. Success or Empty state + const result = currentRunState?.result || (lastRunResult as EvalRunResult | undefined); + if (result && result.metrics) { + return ( +
+ {renderHeaderAndForm(selectedLane)} +
+ + Status: + + + {result.passed ? "Passed" : "Failed"} + + {result.source_digest && ( + + )} +
+ +
+

Metrics

+ +
+ + +
+ ); + } + + return ( +
+ {renderHeaderAndForm(selectedLane)} + +
+ ); + }; return (
@@ -73,115 +275,7 @@ export function EvalsRoute() { {/* Right Pane: Results / Form */}
- {selectedLane ? ( - <> - {/* Header info */} -
-
-

- {selectedLane.lane} -

- {selectedLane.description && ( -

- {selectedLane.description} -

- )} -
-
- - {/* Run Button configuration if read-only */} - {selectedLane.read_only ? ( - { - setRunStates((prev) => ({ - ...prev, - [selectedLane.lane]: { isPending: true }, - })); - }} - onRunSuccess={(result) => { - setRunStates((prev) => ({ - ...prev, - [selectedLane.lane]: { isPending: false, result }, - })); - }} - onRunError={(err) => { - setRunStates((prev) => ({ - ...prev, - [selectedLane.lane]: { isPending: false, error: err }, - })); - }} - /> - ) : ( -
- ⚠️ API runs disabled for write-active lane. Use local CLI to execute this lane: - - core eval --lane {selectedLane.lane} - -
- )} - - {/* Result display */} - {currentRunState?.isPending ? ( - - ) : currentRunState?.error ? ( - - ) : currentRunState?.result ? ( -
-
- - Status: - - - {currentRunState.result.passed ? "Passed" : "Failed"} - - {currentRunState.result.source_digest && ( - - )} -
- -
-

Metrics

- -
- - -
- ) : ( - - )} - - ) : ( - - )} + {renderRightPane()}
); diff --git a/workbench-ui/src/app/evals/evals.test.tsx b/workbench-ui/src/app/evals/evals.test.tsx index d1b155d0..5e90d79c 100644 --- a/workbench-ui/src/app/evals/evals.test.tsx +++ b/workbench-ui/src/app/evals/evals.test.tsx @@ -16,11 +16,13 @@ vi.mock("../../api/queries", async (importOriginal) => { return { ...actual, useEvalLanes: vi.fn(), + useEvalLane: vi.fn(), useEvalRun: vi.fn(), }; }); -import { useEvalLanes, useEvalRun } from "../../api/queries"; +import { useEvalLanes, useEvalLane, useEvalRun } from "../../api/queries"; +import { CommandPalette } from "../../design/components/primitives/CommandPalette"; const mockLanes: EvalLaneSummary[] = [ { lane: "contemplation_quality", versions: ["v1", "v2"], read_only: true, description: "Contemplation checks" }, @@ -49,6 +51,27 @@ describe("W-030 Component Tests", () => { beforeEach(() => { vi.resetAllMocks(); + + vi.mocked(useEvalLanes).mockReturnValue({ + data: mockLanes, + isLoading: false, + isError: false, + error: null, + } as any); + vi.mocked(useEvalLane).mockReturnValue({ + data: undefined, + isLoading: false, + isError: true, + error: new WorkbenchApiError("not_found", "No run history found"), + } as any); + vi.mocked(useEvalRun).mockReturnValue({ + mutate: vi.fn(), + isPending: false, + isError: false, + isSuccess: false, + error: null, + } as any); + fetchMock.mockImplementation((url: any) => { const urlStr = typeof url === "string" ? url : String(url?.url || url || ""); if (urlStr.endsWith("/evals")) { @@ -162,10 +185,45 @@ describe("W-030 Component Tests", () => { // Verify lexicographical order expect(cards1).toEqual(cards2); + expect(cards1.length).toBe(3); expect(cards1[0]).toContain("accuracy"); expect(cards1[1]).toContain("latency ms"); expect(cards1[2]).toContain("passed"); - expect(cards1[3]).toContain("total"); + expect(cards1[2]).toContain("9/10"); + }); + + it("renders actual / target, emphasizes actual, and applies failure color token when target is not met", () => { + const metrics = { + passed: 4, + total: 5, + latency_ms: 150, + latency_ms_target: 100, // failing because actual > target + accuracy: 0.9, + accuracy_target: 0.95, // failing because actual < target + }; + + const { container } = render(); + + const cards = container.querySelectorAll('[data-testid="metric-card"]'); + expect(cards.length).toBe(3); + + const passedCard = Array.from(cards).find(c => c.textContent?.includes("passed")); + expect(passedCard).toBeDefined(); + expect(passedCard?.querySelector(".text-\\[var\\(--color-state-contradicted\\)\\]")).toBeInTheDocument(); + expect(passedCard?.textContent).toContain("4"); + expect(passedCard?.textContent).toContain("5"); + + const latencyCard = Array.from(cards).find(c => c.textContent?.includes("latency ms")); + expect(latencyCard).toBeDefined(); + expect(latencyCard?.querySelector(".text-\\[var\\(--color-state-contradicted\\)\\]")).toBeInTheDocument(); + expect(latencyCard?.textContent).toContain("150"); + expect(latencyCard?.textContent).toContain("100"); + + const accuracyCard = Array.from(cards).find(c => c.textContent?.includes("accuracy")); + expect(accuracyCard).toBeDefined(); + expect(accuracyCard?.querySelector(".text-\\[var\\(--color-state-contradicted\\)\\]")).toBeInTheDocument(); + expect(accuracyCard?.textContent).toContain("0.9"); + expect(accuracyCard?.textContent).toContain("0.95"); }); }); @@ -295,5 +353,115 @@ describe("W-030 Component Tests", () => { const res = await runEvalLane({ lane: "contemplation_quality", split: "holdout" }); expect(res).toEqual(mockResult); }); + + it("renders loading state when lane list is loading", () => { + vi.mocked(useEvalLanes).mockReturnValue({ + data: null, + isLoading: true, + isError: false, + error: null, + } as any); + + const client = makeClient(); + render( + + + + + + ); + + expect(screen.getByText("Loading eval lanes...")).toBeInTheDocument(); + }); + + it("renders empty state when there are no lanes", () => { + vi.mocked(useEvalLanes).mockReturnValue({ + data: [], + isLoading: false, + isError: false, + error: null, + } as any); + + const client = makeClient(); + render( + + + + + + ); + + expect(screen.getByText("No eval lanes discovered.")).toBeInTheDocument(); + }); + + it("renders loading and error states for the selected lane detail view", () => { + vi.mocked(useEvalLanes).mockReturnValue({ + data: mockLanes, + isLoading: false, + isError: false, + error: null, + } as any); + + // Mock useEvalLane to return loading + vi.mocked(useEvalLane).mockReturnValue({ + data: undefined, + isLoading: true, + isError: false, + error: null, + } as any); + + const client = makeClient(); + const { rerender } = render( + + + + + + ); + + expect(screen.getByText("Loading eval lane details...")).toBeInTheDocument(); + + // Mock useEvalLane to return error (other than not_found) + vi.mocked(useEvalLane).mockReturnValue({ + data: undefined, + isLoading: false, + isError: true, + error: new WorkbenchApiError("read_error", "Disk read error"), + } as any); + + rerender( + + + + + + ); + + expect(screen.getByText("Disk read error")).toBeInTheDocument(); + }); + + it("CommandPalette registers dynamic command entries driven by useEvalLanes", () => { + vi.mocked(useEvalLanes).mockReturnValue({ + data: mockLanes, + isLoading: false, + isError: false, + error: null, + } as any); + + const client = makeClient(); + render( + + + + + + ); + + // Verify static commands + expect(screen.getByRole("button", { name: "Open Chat" })).toBeInTheDocument(); + // Verify dynamic commands + expect(screen.getByRole("button", { name: "Open eval lane contemplation_quality" })).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Open eval lane unsafe_lane" })).toBeInTheDocument(); + }); }); }); diff --git a/workbench-ui/src/design/components/primitives/CommandPalette.keyboard.test.tsx b/workbench-ui/src/design/components/primitives/CommandPalette.keyboard.test.tsx index 36e558c9..a9082d5e 100644 --- a/workbench-ui/src/design/components/primitives/CommandPalette.keyboard.test.tsx +++ b/workbench-ui/src/design/components/primitives/CommandPalette.keyboard.test.tsx @@ -1,10 +1,18 @@ import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { MemoryRouter } from "react-router-dom"; import { useState } from "react"; import { CommandPalette } from "./CommandPalette"; +vi.mock("../../../api/queries", () => ({ + useEvalLanes: () => ({ + data: [], + isLoading: false, + isError: false, + }), +})); + function PaletteHarness({ initialOpen = false }: { initialOpen?: boolean }) { const [open, setOpen] = useState(initialOpen); return ( diff --git a/workbench-ui/src/design/components/primitives/CommandPalette.tsx b/workbench-ui/src/design/components/primitives/CommandPalette.tsx index f2420694..34465680 100644 --- a/workbench-ui/src/design/components/primitives/CommandPalette.tsx +++ b/workbench-ui/src/design/components/primitives/CommandPalette.tsx @@ -2,13 +2,14 @@ import * as Dialog from "@radix-ui/react-dialog"; import { Search } from "lucide-react"; import { useRef, useState, useEffect, useCallback } from "react"; import { useNavigate, useInRouterContext } from "react-router-dom"; +import { useEvalLanes } from "../../../api/queries"; interface Command { name: string; path: string; } -const COMMANDS: Command[] = [ +const STATIC_COMMANDS: Command[] = [ { name: "Open Chat", path: "/chat" }, { name: "Open Proposals", path: "/proposals" }, { name: "Open Evals", path: "/evals" }, @@ -20,6 +21,15 @@ function RouterCommandPalette(props: { onOpenChange: (open: boolean) => void; }) { const navigate = useNavigate(); + const { data: lanes } = useEvalLanes(); + + const dynamicCommands = (lanes || []).map((lane) => ({ + name: `Open eval lane ${lane.lane}`, + path: `/evals?lane=${lane.lane}`, + })); + + const commands = [...STATIC_COMMANDS, ...dynamicCommands]; + const activate = useCallback( (cmd: Command) => { navigate(cmd.path); @@ -27,7 +37,7 @@ function RouterCommandPalette(props: { }, [navigate, props], ); - return ; + return ; } // Fallback for design-system preview (no Router). @@ -41,23 +51,25 @@ function FallbackCommandPalette(props: { }, [props], ); - return ; + return ; } function CommandPaletteContent({ open, onOpenChange, onActivate, + commands, }: { open: boolean; onOpenChange: (open: boolean) => void; onActivate: (cmd: Command) => void; + commands: Command[]; }) { const [query, setQuery] = useState(""); const [focusedIndex, setFocusedIndex] = useState(0); const inputRef = useRef(null); - const filtered = COMMANDS.filter((cmd) => + const filtered = commands.filter((cmd) => cmd.name.toLowerCase().includes(query.toLowerCase()), );