From 8303c3c51ef1c7321fa107bbffbbbb6b639e5e2d Mon Sep 17 00:00:00 2001 From: Shay Date: Sun, 14 Jun 2026 13:31:43 -0700 Subject: [PATCH] fix(workbench): DAG natural-height fit for dense graphs + consistent click-to-copy feedback Two UX wrinkles surfaced while exercising the CORE-Logos Studio: - DAG viewport: tall, few-layer graphs (e.g. the 408x1890, 2-layer/55-node alignment fan) were squished by viewBox-meet into an unreadable sliver. Render at natural height (scale ~1) inside a bounded, scrollable box; small graphs are unaffected (>= the caller height). Golden layout unchanged (layoutDag untouched). - Click-to-copy: new shared useCopyToClipboard hook gives every copy affordance a tooltip + transient confirmation. StatusFooter SHA now confirms 'Copied' (was a silent copy that read as 'does nothing'); 'Read Only' is labeled a non-interactive status (not a toggle); checkpoint-revision gains an explanatory tooltip. DigestBadge + MetadataTable copy buttons route through the hook + gain titles. Full workbench-ui vitest: 520 passed (+4), clean exit; tsc + build clean. --- workbench-ui/src/app/StatusFooter.test.tsx | 79 +++++++++++++++++++ workbench-ui/src/app/StatusFooter.tsx | 14 ++-- .../src/design/components/Dag/Dag.test.tsx | 21 +++++ .../src/design/components/Dag/Dag.tsx | 19 ++++- .../components/DigestBadge/DigestBadge.tsx | 15 +--- .../MetadataTable/MetadataTable.tsx | 16 ++-- .../src/design/hooks/useCopyToClipboard.ts | 25 ++++++ 7 files changed, 161 insertions(+), 28 deletions(-) create mode 100644 workbench-ui/src/app/StatusFooter.test.tsx create mode 100644 workbench-ui/src/design/hooks/useCopyToClipboard.ts diff --git a/workbench-ui/src/app/StatusFooter.test.tsx b/workbench-ui/src/app/StatusFooter.test.tsx new file mode 100644 index 00000000..a76e3f0c --- /dev/null +++ b/workbench-ui/src/app/StatusFooter.test.tsx @@ -0,0 +1,79 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { QueryClientProvider } from "@tanstack/react-query"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { createTestQueryClient } from "../test/createTestQueryClient"; +import type { RuntimeStatus } from "../types/api"; + +vi.mock("../api/queries", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useRuntimeStatus: vi.fn() }; +}); + +import { useRuntimeStatus } from "../api/queries"; +import { StatusFooter } from "./StatusFooter"; + +const status: RuntimeStatus = { + backend: "numpy", + git_revision: "5474a152057d9999", + engine_state_present: false, + checkpoint_revision: "unknown", + revision_warning: false, + active_session_id: null, + mutation_mode: "read_only", +}; + +function renderFooter() { + return render( + + + , + ); +} + +beforeEach(() => { + vi.mocked(useRuntimeStatus).mockReturnValue({ + data: status, + isError: false, + } as unknown as ReturnType); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("StatusFooter", () => { + it("labels Read Only as a non-interactive status, not a toggle", () => { + renderFooter(); + const chip = screen.getByText("Read Only"); + expect(chip).toHaveAttribute( + "title", + "Runtime mutation mode — read-only by design (status, not a toggle)", + ); + expect(chip.tagName).toBe("SPAN"); // status display, never a button + }); + + it("copies the full SHA and shows a transient confirmation on click", async () => { + // userEvent.setup() installs its own clipboard stub; read it back to verify + // the full SHA was copied (not just the short display form). + const user = userEvent.setup(); + renderFooter(); + + const sha = screen.getByTestId("git-revision"); + expect(sha).toHaveAttribute("title", "Copy full git revision SHA"); + expect(sha).toHaveTextContent("5474a152"); // short SHA before copy + + await user.click(sha); + + // The confirmation the silent-copy version lacked. + expect(await screen.findByText("Copied")).toBeInTheDocument(); + expect(await navigator.clipboard.readText()).toBe("5474a152057d9999"); + }); + + it("gives the checkpoint-revision toggle an explanatory tooltip", () => { + renderFooter(); + const checkpoint = screen.getByTestId("checkpoint-revision"); + expect(checkpoint.getAttribute("title")).toMatch(/checkpoint revision/i); + expect(checkpoint.getAttribute("title")).toMatch(/unknown/); + }); +}); diff --git a/workbench-ui/src/app/StatusFooter.tsx b/workbench-ui/src/app/StatusFooter.tsx index 3e21270a..021341df 100644 --- a/workbench-ui/src/app/StatusFooter.tsx +++ b/workbench-ui/src/app/StatusFooter.tsx @@ -1,11 +1,13 @@ import { useState } from "react"; import { useIsMutating } from "@tanstack/react-query"; import { useRuntimeStatus } from "../api/queries"; +import { useCopyToClipboard } from "../design/hooks/useCopyToClipboard"; export function StatusFooter() { const { data, isError } = useRuntimeStatus(); const chatTurnsPending = useIsMutating({ mutationKey: ["chat-turn"] }) > 0; const [revisionExpanded, setRevisionExpanded] = useState(false); + const { copied: shaCopied, copy: copySha } = useCopyToClipboard(); if (isError) { return ( @@ -29,7 +31,8 @@ export function StatusFooter() { visibleMutationMode === "read_only" ? ( Read Only @@ -54,12 +57,12 @@ export function StatusFooter() {
@@ -73,6 +76,7 @@ export function StatusFooter() { ].join(" ")} onClick={() => setRevisionExpanded((v) => !v)} aria-expanded={revisionExpanded} + title={`Engine checkpoint revision (${checkpoint_revision}) — click to ${revisionExpanded ? "hide" : "show"} details`} aria-label={`checkpoint revision: ${checkpoint_revision}${revision_warning ? " (warning)" : ""}`} data-testid="checkpoint-revision" data-warning={revision_warning ? "true" : undefined} diff --git a/workbench-ui/src/design/components/Dag/Dag.test.tsx b/workbench-ui/src/design/components/Dag/Dag.test.tsx index d4078266..70d23a83 100644 --- a/workbench-ui/src/design/components/Dag/Dag.test.tsx +++ b/workbench-ui/src/design/components/Dag/Dag.test.tsx @@ -69,4 +69,25 @@ describe("Dag", () => { expect(screen.getByTestId("dag-viewer")).toBeInTheDocument(); }); + + it("renders a tall, few-layer graph at natural height inside a bounded scroll box", () => { + // 12 sources -> 12 sinks: 2 layers, 24 rows — a tall sliver that the old + // fixed-320 viewport squished. The SVG should grow to the layout's natural + // height (legible, scale ~1), and the wrapper bounds + scrolls it. + const wideNodes = Array.from({ length: 24 }, (_, i) => ({ id: `n${i}` })); + const fanEdges = Array.from({ length: 12 }, (_, i) => ({ + from: `n${i}`, + to: `n${i + 12}`, + })); + const layout = layoutDag(wideNodes, fanEdges); + expect(layout.height).toBeGreaterThan(320); // precondition: genuinely tall + + render(); + + const svg = screen.getByRole("img", { name: "Tall fan DAG" }); + expect(svg).toHaveAttribute("height", String(layout.height)); + const viewport = screen.getByTestId("dag-viewport"); + expect(viewport.style.maxHeight).toBe("560px"); + expect(viewport.className).toContain("overflow-auto"); + }); }); diff --git a/workbench-ui/src/design/components/Dag/Dag.tsx b/workbench-ui/src/design/components/Dag/Dag.tsx index 35303f5e..ab9ea191 100644 --- a/workbench-ui/src/design/components/Dag/Dag.tsx +++ b/workbench-ui/src/design/components/Dag/Dag.tsx @@ -12,6 +12,13 @@ export interface DagViewerProps { showInspector?: boolean; } +// Tall, few-layer graphs (e.g. a 2-layer alignment fan of 50+ nodes) must not +// be squished into a fixed short viewport by viewBox-meet — that produces an +// unreadable sliver. Render at natural height (scale ~1) inside a bounded, +// scrollable box instead. Small graphs (layout.height < the caller height) are +// unaffected. +const MAX_VIEWPORT_HEIGHT = 560; + function clippedLabel(label: string) { return label.length > 22 ? `${label.slice(0, 19)}...` : label; } @@ -31,6 +38,10 @@ export function DagViewer({ showInspector = true, }: DagViewerProps) { const layout = useMemo(() => layoutDag(nodes, edges), [nodes, edges]); + // Grow the SVG to the graph's natural height so dense graphs render legibly; + // the wrapper bounds + scrolls it. Never shrinks below the caller's height. + const svgHeight = Math.max(height, layout.height); + const viewportMaxHeight = Math.max(height, MAX_VIEWPORT_HEIGHT); const [scale, setScale] = useState(1); const [offset, setOffset] = useState({ x: 0, y: 0 }); const [internalSelectedId, setInternalSelectedId] = useState( @@ -98,11 +109,16 @@ export function DagViewer({
+
{ @@ -186,6 +202,7 @@ export function DagViewer({ })} +
{showInspector ? (
diff --git a/workbench-ui/src/design/components/DigestBadge/DigestBadge.tsx b/workbench-ui/src/design/components/DigestBadge/DigestBadge.tsx index 5d468531..6ba692e5 100644 --- a/workbench-ui/src/design/components/DigestBadge/DigestBadge.tsx +++ b/workbench-ui/src/design/components/DigestBadge/DigestBadge.tsx @@ -1,6 +1,4 @@ -import { useState } from "react"; -import { copyText } from "../../lib"; -import { useManagedTimeout } from "../../hooks/useManagedTimeout"; +import { useCopyToClipboard } from "../../hooks/useCopyToClipboard"; export interface DigestBadgeProps { digest: string; @@ -40,8 +38,7 @@ export function DigestBadge({ // Wave R hash display standard: 12 visible chars + copy, everywhere. truncate = 12, }: DigestBadgeProps) { - const [copied, setCopied] = useState(false); - const scheduleReset = useManagedTimeout(); + const { copied, copy } = useCopyToClipboard(); const display = digest.length > truncate ? `${digest.slice(0, truncate)}...` @@ -52,6 +49,7 @@ export function DigestBadge({ return (