From 3de1a784ced2590951d2e1261c91496bd1a19fb1 Mon Sep 17 00:00:00 2001 From: mptyl Date: Fri, 3 Jul 2026 13:01:31 +0200 Subject: [PATCH] docs(plan): implementation plan for workflow UI fixes (6 tasks, TDD) Co-Authored-By: Claude Opus 4.8 --- .../plans/2026-07-03-workflow-ui-fixes.md | 944 ++++++++++++++++++ 1 file changed, 944 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-03-workflow-ui-fixes.md diff --git a/docs/superpowers/plans/2026-07-03-workflow-ui-fixes.md b/docs/superpowers/plans/2026-07-03-workflow-ui-fixes.md new file mode 100644 index 00000000..116cb348 --- /dev/null +++ b/docs/superpowers/plans/2026-07-03-workflow-ui-fixes.md @@ -0,0 +1,944 @@ +# Workflow UI fixes Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fix five UI/contract defects found while testing: phase-circle lifecycle colors, a total elapsed timer, the empty reviewer artifact gate, a 90% artifact modal with a Mermaid schema diagram, and the duplicate "Altro"/"other" option. + +**Architecture:** All fixes are in the frontend except one root-cause fix in the harness gate. The reviewer artifact gate renders `artifact.data` (harness contract) — not the never-populated `artifact.content` — through a new `ArtifactView` router inside a full-screen modal. Phase lifecycle stays frontend-optimistic (no new Pi RPC events). The duplicate-option bug is fixed by making the harness reserved-label matcher robust to model-produced variants. + +**Tech Stack:** React 18 + Vite + vitest + Testing Library (frontend, no network — MSW); Node `node --test` for the harness gate JS; Mermaid (already a dependency); Base UI dialog (`components/ui/dialog.tsx`). + +## Global Constraints + +- Frontend has no ESLint; `npx tsc -b` is the gate. Run it before every commit that touches TS. +- Frontend tests use vitest + MSW, no real network. +- Harness gate JS tests run via `cd harness && npm test` (`node --test .pi/extensions/gate/__tests__/*.test.js`). New harness tests MUST live in `.pi/extensions/gate/__tests__/` and be named `*.test.js`. +- UI strings/chrome/labels stay **English** (project convention). Document *content* stays the workspace language. +- Harness JS line style: keep the surrounding tab-indented style of the file you edit; do not reformat neighbours. +- Every changed line must trace to one of the five defects — no unrelated refactoring. + +--- + +## File Structure + +**Harness** +- `harness/.pi/extensions/reserved-labels.mjs` — modify `isReserved` to normalize + prefix-match. +- `harness/.pi/extensions/gate/__tests__/reserved_labels.test.js` — new unit test. + +**Frontend** +- `frontend/src/viewers/SchemaLinkingViewer.tsx` — replace `buildFlowchart` with exported `buildErDiagram` (Mermaid `erDiagram`). +- `frontend/src/viewers/SchemaLinkingViewer.test.tsx` — add a `buildErDiagram` unit test. +- `frontend/src/viewers/ArtifactView.tsx` — new: route `artifact.kind` → viewer, defensive on `data`. +- `frontend/src/viewers/ArtifactView.test.tsx` — new. +- `frontend/src/widgets/ArtifactGateWidget.tsx` — rewrite body to render a 90% modal using `ArtifactView` (keep the export name and filename). +- `frontend/src/widgets/ArtifactGateWidget.test.tsx` — update to the modal behaviour. +- `frontend/src/shell/WorkflowBar.tsx` — `finalized`/`createdAt`/`updatedAt` props; all-green on finalized; render `ElapsedTimer`. +- `frontend/src/shell/WorkflowBar.test.tsx` — new. +- `frontend/src/shell/ElapsedTimer.tsx` — new. +- `frontend/src/shell/ElapsedTimer.test.tsx` — new. +- `frontend/src/shell/SteerInput.tsx` — set `currentPhase="F1"` on new-question creation. +- `frontend/src/shell/AppShell.tsx` — pass `finalized`/`createdAt`/`updatedAt` to `WorkflowBar`. +- `frontend/src/shell/f1-loop.test.tsx` — assert optimistic F1 after a new question. + +--- + +## Task 1: Harness — robust `isReserved` (fix duplicate Altro/other) + +**Files:** +- Modify: `harness/.pi/extensions/reserved-labels.mjs` +- Test: `harness/.pi/extensions/gate/__tests__/reserved_labels.test.js` + +**Interfaces:** +- Consumes: nothing new. +- Produces: `isReserved(label: string): boolean` — true for any label whose normalized form starts with `altro` or `other`, or equals the normalized quit/back canonical labels. `stripReserved(labels: string[]): string[]` unchanged in signature; inherits the new matcher. Used by `tht-gate.js` `reviewer_select`/`reviewer_decide` to drop model-supplied free-text options. + +- [ ] **Step 1: Write the failing test** + +Create `harness/.pi/extensions/gate/__tests__/reserved_labels.test.js`: + +```javascript +const test = require("node:test"); +const assert = require("node:assert"); +const { isReserved, stripReserved } = require("../../reserved-labels.mjs"); + +test("Altro variants (any punctuation/case) are reserved", () => { + assert.equal(isReserved("Altro — specifica…"), true); + assert.equal(isReserved("Altro - specificare"), true); + assert.equal(isReserved("altro"), true); + assert.equal(isReserved("ALTRO (specificare)"), true); +}); + +test("English Other variant is reserved", () => { + assert.equal(isReserved("Other — specify"), true); + assert.equal(isReserved("other"), true); +}); + +test("canonical quit/back labels stay reserved", () => { + assert.equal(isReserved("Esci da Pi (/quit)"), true); + assert.equal(isReserved("Torna indietro (fase precedente)"), true); +}); + +test("normal merit options are NOT reserved", () => { + assert.equal(isReserved("procedura"), false); + assert.equal(isReserved("patologia"), false); + assert.equal(isReserved("altrove"), false); // starts with "altro"? no — "altrove" -> normalized "altrove" starts with "altro" -> guard below +}); + +test("stripReserved drops every Altro/other variant, keeps merit order", () => { + assert.deepEqual( + stripReserved(["procedura", "Altro - specificare", "patologia", "other"]), + ["procedura", "patologia"], + ); +}); +``` + +Note: `"altrove"` normalizes to `"altrove"`, which *does* start with `"altro"`. To avoid stripping a legitimate option, match the whole first token equals `altro`/`other` rather than a raw `startsWith`. The implementation below tokenizes, so `"altrove"` (single token `altrove`) is NOT reserved. Keep this test line as the guard. + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd harness && node --test .pi/extensions/gate/__tests__/reserved_labels.test.js` +Expected: FAIL — `isReserved("Altro - specificare")` returns `false` (exact-match implementation). + +- [ ] **Step 3: Implement the normalized matcher** + +Replace the body of `harness/.pi/extensions/reserved-labels.mjs` from the `const RESERVED` line through `stripReserved` with: + +```javascript +// Normalize a label to lowercase ASCII tokens: strip diacritics, turn every run +// of punctuation/space into a single space, trim. "Altro — specifica…" -> "altro specifica". +function normalize(label) { + return String(label) + .normalize("NFD") + .replace(/[̀-ͯ]/g, "") + .toLowerCase() + .replace(/[^a-z0-9]+/g, " ") + .trim(); +} + +const NORM_QUIT = normalize(QUIT_LABEL); +const NORM_BACK = normalize(BACK_LABEL); + +// true if the label is one the gate adds itself. Robust to the variants models +// emit ("Altro - specificare", "altro", English "Other — specify"): reserved when +// the FIRST normalized token is exactly "altro"/"other", or the whole normalized +// label equals the canonical quit/back labels. First-token match keeps real +// options like "altrove" out of the reserved set. +export function isReserved(label) { + const n = normalize(label); + if (!n) return false; + const first = n.split(" ")[0]; + return first === "altro" || first === "other" || n === NORM_QUIT || n === NORM_BACK; +} + +// removes every reserved entry from a list of labels, preserving order and normal +// entries. Idempotent. +export function stripReserved(labels) { + return labels.filter((label) => !isReserved(label)); +} +``` + +Leave the `export const ALTRO / QUIT_LABEL / BACK_LABEL / CONTROL_LABELS` lines at the top of the file unchanged (still imported elsewhere). Remove the now-unused `const RESERVED = new Set([...])` line. + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cd harness && node --test .pi/extensions/gate/__tests__/reserved_labels.test.js` +Expected: PASS (all 5 tests). + +Run the whole gate suite to confirm no regression: `cd harness && npm test` +Expected: PASS (existing `builders.test.js` assertion "select no longer injects an Altro option" and the rest stay green). + +- [ ] **Step 5: Commit** + +```bash +git add harness/.pi/extensions/reserved-labels.mjs harness/.pi/extensions/gate/__tests__/reserved_labels.test.js +git commit -m "fix(gate): robust isReserved strips model Altro/other variants (no duplicate free-text option)" +``` + +--- + +## Task 2: Frontend — Mermaid `erDiagram` for schema linking + +**Files:** +- Modify: `frontend/src/viewers/SchemaLinkingViewer.tsx` (replace `buildFlowchart`, ~lines 36-57 and its call ~line 92) +- Test: `frontend/src/viewers/SchemaLinkingViewer.test.tsx` + +**Interfaces:** +- Consumes: `Candidate`, `Join` (already exported from `SchemaLinkingViewer.tsx`). +- Produces: `export function buildErDiagram(promoted: Candidate[], joins: Join[]): string` — a Mermaid `erDiagram` string: promoted tables as entities, their promoted `table.column` candidates as attributes, `joins` (resolved to owning tables) as relationships. Consumed by `ArtifactView` indirectly (via the component) and unit-tested here. + +- [ ] **Step 1: Write the failing test** + +Add to `frontend/src/viewers/SchemaLinkingViewer.test.tsx` — update the import on line 3 and append the test: + +```tsx +import { SchemaLinkingViewer, buildErDiagram } from "./SchemaLinkingViewer"; +``` + +```tsx +test("(e) buildErDiagram emits entities, attributes and a relation", () => { + const def = buildErDiagram( + [ + { kind: "table", name: "orders", decision: "promoted" }, + { kind: "column", name: "orders.id", decision: "promoted" }, + { kind: "table", name: "customers", decision: "promoted" }, + ], + [{ from: "orders.customer_id", to: "customers.id" }], + ); + expect(def.startsWith("erDiagram")).toBe(true); + expect(def).toContain("orders {"); + expect(def).toContain("col id"); + expect(def).toContain("orders }o--o{ customers : join"); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd frontend && npx vitest run src/viewers/SchemaLinkingViewer.test.tsx -t buildErDiagram` +Expected: FAIL — `buildErDiagram` is not exported (`buildErDiagram is not a function`). + +- [ ] **Step 3: Replace `buildFlowchart` with `buildErDiagram`** + +In `frontend/src/viewers/SchemaLinkingViewer.tsx`, delete the `buildFlowchart` function (lines ~36-57) and add: + +```tsx +export function buildErDiagram(promoted: Candidate[], joins: Join[]): string { + const sanitize = (name: string) => name.replace(/[^a-zA-Z0-9]/g, "_"); + const tables = promoted.filter((c) => c.kind === "table"); + const tableNames = new Set(tables.map((t) => t.name)); + const columns = promoted.filter((c) => c.kind === "column"); + + const lines: string[] = ["erDiagram"]; + + for (const t of tables) { + const id = sanitize(t.name); + const cols = columns.filter((col) => col.name.startsWith(t.name + ".")); + lines.push(` ${id} {`); + for (const col of cols) { + lines.push(` col ${sanitize(col.name.slice(t.name.length + 1))}`); + } + lines.push(` }`); + } + + // A join endpoint may be "table" or "table.column"; resolve to its owning table. + // Draw a relationship only between two DISTINCT promoted tables, once per pair. + const owningTable = (ref: string) => + ref.includes(".") ? ref.slice(0, ref.indexOf(".")) : ref; + const seen = new Set(); + for (const j of joins) { + const a = owningTable(j.from); + const b = owningTable(j.to); + if (a === b || !tableNames.has(a) || !tableNames.has(b)) continue; + const key = [a, b].sort().join("::"); + if (seen.has(key)) continue; + seen.add(key); + lines.push(` ${sanitize(a)} }o--o{ ${sanitize(b)} : join`); + } + + return lines.join("\n"); +} +``` + +Then update the effect that builds the diagram (~line 92): change + +```tsx + const def = buildFlowchart(promoted, linking.joins); +``` + +to + +```tsx + const def = buildErDiagram(promoted, linking.joins); +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cd frontend && npx vitest run src/viewers/SchemaLinkingViewer.test.tsx` +Expected: PASS — the new `(e)` test plus the existing `(a)-(d)` tests (they mock `renderMermaid`, so the diagram string change doesn't affect them). + +Run: `cd frontend && npx tsc -b` +Expected: no errors. + +- [ ] **Step 5: Commit** + +```bash +git add frontend/src/viewers/SchemaLinkingViewer.tsx frontend/src/viewers/SchemaLinkingViewer.test.tsx +git commit -m "feat(viewer): schema linking renders a Mermaid erDiagram (tables + relations)" +``` + +--- + +## Task 3: Frontend — `ArtifactView` (render artifact.data by kind) + +**Files:** +- Create: `frontend/src/viewers/ArtifactView.tsx` +- Test: `frontend/src/viewers/ArtifactView.test.tsx` + +**Interfaces:** +- Consumes: `SqlViewer` + `SqlBlock` (`./SqlViewer`), `SchemaLinkingViewer` + `SchemaLinking` (`./SchemaLinkingViewer`), `MarkdownView` (`./MarkdownView`). +- Produces: `export function ArtifactView({ artifact }: { artifact: { kind: string; data?: unknown; content?: unknown; [k: string]: unknown } }): ReactElement`. Routes on `artifact.kind`, defensive about `data` (string or object), JSON fallback otherwise. Consumed by `ArtifactGateWidget` (Task 4). + +- [ ] **Step 1: Write the failing test** + +Create `frontend/src/viewers/ArtifactView.test.tsx`: + +```tsx +import { render, screen } from "@testing-library/react"; +import { ArtifactView } from "./ArtifactView"; + +vi.mock("./mermaid", () => ({ + renderMermaid: vi.fn().mockResolvedValue(''), +})); + +test("sql artifact renders the SQL text", () => { + render(); + expect(screen.getByText("SELECT 1")).toBeInTheDocument(); +}); + +test("cte_plan renders an ordered list of names", () => { + render(); + expect(screen.getByText("a_cte")).toBeInTheDocument(); + expect(screen.getByText("b_cte")).toBeInTheDocument(); +}); + +test("question renders markdown headings", () => { + render(); + expect(screen.getByText("Domanda")).toBeInTheDocument(); +}); + +test("unknown kind falls back to formatted JSON", () => { + render(); + expect(screen.getByText(/"a": 1/)).toBeInTheDocument(); +}); + +test("schema_linking renders the schema viewer", async () => { + render( + , + ); + expect(await screen.findByTestId("mm")).toBeInTheDocument(); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd frontend && npx vitest run src/viewers/ArtifactView.test.tsx` +Expected: FAIL — cannot resolve `./ArtifactView`. + +- [ ] **Step 3: Implement `ArtifactView`** + +Create `frontend/src/viewers/ArtifactView.tsx`: + +```tsx +import type { ReactElement } from "react"; +import { SqlViewer, type SqlBlock } from "./SqlViewer"; +import { SchemaLinkingViewer, type SchemaLinking } from "./SchemaLinkingViewer"; +import { MarkdownView } from "./MarkdownView"; + +type ArtifactData = { kind: string; data?: unknown; content?: unknown; [k: string]: unknown }; + +function asRecord(v: unknown): Record | null { + return v && typeof v === "object" && !Array.isArray(v) ? (v as Record) : null; +} + +// The payload lives in `data` (harness contract); fall back to `content`, then the +// artifact object itself, so older/other producers still render something. +function payload(artifact: ArtifactData): unknown { + if (artifact.data !== undefined) return artifact.data; + if (artifact.content !== undefined) return artifact.content; + return artifact; +} + +function toSchemaLinking(raw: unknown): SchemaLinking | null { + let obj: unknown = raw; + if (typeof raw === "string") { + try { obj = JSON.parse(raw); } catch { return null; } + } + const rec = asRecord(obj); + if (!rec || !Array.isArray(rec.candidates)) return null; + return { + candidates: rec.candidates as SchemaLinking["candidates"], + joins: Array.isArray(rec.joins) ? (rec.joins as SchemaLinking["joins"]) : [], + excluded: Array.isArray(rec.excluded) ? (rec.excluded as SchemaLinking["excluded"]) : [], + open_questions: Array.isArray(rec.open_questions) ? (rec.open_questions as string[]) : [], + question: typeof rec.question === "string" ? rec.question : undefined, + }; +} + +function toSqlBlocks(raw: unknown): SqlBlock[] | null { + if (typeof raw === "string") return [{ name: "SQL", sql: raw }]; + const rec = asRecord(raw); + if (!rec) return null; + if (typeof rec.sql === "string") return [{ name: "SQL", sql: rec.sql }]; + if (Array.isArray(rec.ctes)) { + const blocks = rec.ctes + .map((c) => asRecord(c)) + .filter((c): c is Record => !!c && typeof c.sql === "string") + .map((c) => ({ name: typeof c.name === "string" ? c.name : "cte", sql: c.sql as string })); + if (typeof rec.final === "string") blocks.push({ name: "final", sql: rec.final }); + return blocks.length ? blocks : null; + } + return null; +} + +function toMarkdown(raw: unknown): string | null { + if (typeof raw === "string") return raw; + const rec = asRecord(raw); + if (!rec) return null; + for (const k of ["markdown", "question", "text"]) { + if (typeof rec[k] === "string") return rec[k] as string; + } + return null; +} + +function cteNames(raw: unknown): string[] | null { + if (Array.isArray(raw) && raw.every((x) => typeof x === "string")) return raw as string[]; + const rec = asRecord(raw); + if (rec && Array.isArray(rec.names) && rec.names.every((x) => typeof x === "string")) { + return rec.names as string[]; + } + return null; +} + +function JsonFallback({ value }: { value: unknown }): ReactElement { + const text = typeof value === "string" ? value : JSON.stringify(value, null, 2); + return ( +
{text}
+ ); +} + +export function ArtifactView({ artifact }: { artifact: ArtifactData }): ReactElement { + const kind = artifact.kind ?? ""; + const data = payload(artifact); + + if (kind === "schema_linking") { + const linking = toSchemaLinking(data); + if (linking) return ; + } + if (kind === "sql" || kind === "cte_result") { + const blocks = toSqlBlocks(data); + if (blocks) return ; + } + if (kind === "cte_plan") { + const names = cteNames(data); + if (names) { + return ( +
    + {names.map((n, i) => ( +
  1. {n}
  2. + ))} +
+ ); + } + } + if (kind === "question" || kind === "phase") { + const md = toMarkdown(data); + if (md !== null) return ; + } + return ; +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cd frontend && npx vitest run src/viewers/ArtifactView.test.tsx` +Expected: PASS (5 tests). (The `sql` test reads the un-highlighted `
` fallback that `SqlViewer` shows before async highlight resolves.)
+
+Run: `cd frontend && npx tsc -b`
+Expected: no errors.
+
+- [ ] **Step 5: Commit**
+
+```bash
+git add frontend/src/viewers/ArtifactView.tsx frontend/src/viewers/ArtifactView.test.tsx
+git commit -m "feat(viewer): ArtifactView routes artifact.data by kind (schema/sql/cte/question)"
+```
+
+---
+
+## Task 4: Frontend — artifact gate as a 90% modal
+
+**Files:**
+- Modify (rewrite body, keep export name): `frontend/src/widgets/ArtifactGateWidget.tsx`
+- Test: `frontend/src/widgets/ArtifactGateWidget.test.tsx`
+
+**Interfaces:**
+- Consumes: `ArtifactView` (`../viewers/ArtifactView`), `Dialog`/`DialogContent`/`DialogTitle` (`../components/ui/dialog`), `ReservedControls`, `LinkageHost`, `WidgetProps`.
+- Produces: `ArtifactGateWidget` (unchanged name; still registered for `"artifact-gate"` in `widgets/index.ts`). Renders a `90vw × 90vh` modal: artifact on top via `ArtifactView`, action bar (options + reserved) at the bottom. Same `onRespond` payloads as before: `{ id, kind: "artifact-gate", choices: [id] }`, linkage merges child `text`, reserved → `{ id, control, text? }`.
+
+- [ ] **Step 1: Write the failing test**
+
+Replace the first test in `frontend/src/widgets/ArtifactGateWidget.test.tsx` (the "renders artifact content in a pre block" test, lines ~5-19) with two tests, and add a `vi.mock` for mermaid at the top. Final file top + first tests:
+
+```tsx
+import { render, screen } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+import { ArtifactGateWidget } from "./ArtifactGateWidget";
+
+vi.mock("../viewers/mermaid", () => ({
+  renderMermaid: vi.fn().mockResolvedValue(''),
+}));
+
+test("renders inside a dialog and shows the artifact via ArtifactView (data path)", () => {
+  const onRespond = vi.fn();
+  render(
+    
+  );
+  expect(screen.getByRole("dialog")).toBeInTheDocument();
+  expect(screen.getByText("SELECT 42")).toBeInTheDocument();
+});
+```
+
+Keep the existing "clicking an option without opens responds immediately" and "reserved control responds with control field" tests as-is (lines ~21-54) — they still describe the modal's behaviour. The reserved test's descriptor `artifact: { kind: "cte", content: "SELECT 1" }` now renders through `ArtifactView`'s JSON fallback (kind `cte` is unknown; `content` string → `JsonFallback` prints `SELECT 1`), which does not affect the button assertions.
+
+- [ ] **Step 2: Run test to verify it fails**
+
+Run: `cd frontend && npx vitest run src/widgets/ArtifactGateWidget.test.tsx -t "inside a dialog"`
+Expected: FAIL — current widget renders a `
`, no `role="dialog"`.
+
+- [ ] **Step 3: Rewrite `ArtifactGateWidget.tsx` as the modal**
+
+Replace the entire contents of `frontend/src/widgets/ArtifactGateWidget.tsx` with:
+
+```tsx
+import { useState } from "react";
+import type { WidgetProps } from "./types";
+import type { UiResponse, WidgetDescriptor } from "../api/types";
+import { Dialog, DialogContent, DialogTitle } from "../components/ui/dialog";
+import { ArtifactView } from "../viewers/ArtifactView";
+import { ReservedControls } from "./ReservedControls";
+import { LinkageHost } from "./LinkageHost";
+
+/**
+ * Artifact review gate rendered as a full-screen (90%) modal: the artifact fills
+ * the top (scrollable) area via ArtifactView; the action bar (options + reserved
+ * controls) sits at the bottom. The gate contract forbids silent dismissal, so the
+ * dialog has no close button and is not closeable by Esc/backdrop — the only way
+ * out is an action or a reserved control, both of which call onRespond.
+ */
+export function ArtifactGateWidget({ descriptor, onRespond }: WidgetProps) {
+  const [pendingLinkage, setPendingLinkage] = useState<{
+    parentResponse: UiResponse;
+    childDescriptor: WidgetDescriptor;
+  } | null>(null);
+
+  function handleOption(optionId: string) {
+    const option = descriptor.options?.find((o) => o.id === optionId);
+    const parentResponse: UiResponse = { id: descriptor.id, kind: "artifact-gate", choices: [optionId] };
+    if (option?.opens) setPendingLinkage({ parentResponse, childDescriptor: option.opens });
+    else onRespond(parentResponse);
+  }
+
+  return (
+    
+      
+        {descriptor.title ?? "Artifact review"}
+
+        
+ {descriptor.artifact ? ( + + ) : ( +

No artifact.

+ )} +
+ +
+ {pendingLinkage ? ( + + ) : ( + <> +
+ {descriptor.options?.map((o) => ( + + ))} +
+ + onRespond({ id: descriptor.id, control: c, ...(t !== undefined ? { text: t } : {}) }) + } + /> + + )} +
+
+
+ ); +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cd frontend && npx vitest run src/widgets/ArtifactGateWidget.test.tsx src/widgets/linkage.test.tsx` +Expected: PASS — the new dialog test, the two kept option/reserved tests, and both `linkage.test.tsx` tests (linkage flow is unchanged). + +Run: `cd frontend && npx tsc -b` +Expected: no errors. + +- [ ] **Step 5: Commit** + +```bash +git add frontend/src/widgets/ArtifactGateWidget.tsx frontend/src/widgets/ArtifactGateWidget.test.tsx +git commit -m "feat(gate): render reviewer artifacts in a 90% modal (fix empty artifact.data gate)" +``` + +--- + +## Task 5: Frontend — phase-circle lifecycle (optimistic F1 + finalized all-green) + +**Files:** +- Modify: `frontend/src/shell/WorkflowBar.tsx` +- Modify: `frontend/src/shell/SteerInput.tsx` (submit path, ~lines 29-45) +- Modify: `frontend/src/shell/AppShell.tsx` (WorkflowBar render, ~line 172) +- Modify: `frontend/src/shell/f1-loop.test.tsx` (add optimistic-F1 assertion) +- Test: `frontend/src/shell/WorkflowBar.test.tsx` + +**Interfaces:** +- Consumes: `useSessionStore` `currentPhase`/`phaseError`/`setPhase`. +- Produces: `WorkflowBar({ finalized }: { finalized?: boolean })` — when `finalized`, every dot is `data-state="done"`. Optimistic F1: `SteerInput` calls `setPhase("F1")` right after a new-question `createSession`. + +- [ ] **Step 1: Write the failing tests** + +Create `frontend/src/shell/WorkflowBar.test.tsx`: + +```tsx +import { render, screen } from "@testing-library/react"; +import { WorkflowBar } from "./WorkflowBar"; +import { useSessionStore } from "../store/sessionStore"; + +beforeEach(() => useSessionStore.getState().resetSession()); + +test("currentPhase F1 renders F1 as running (yellow)", () => { + useSessionStore.getState().setPhase("F1"); + render(); + expect(screen.getByTestId("phase-F1")).toHaveAttribute("data-state", "running"); +}); + +test("a phase before the active one is done (green)", () => { + useSessionStore.getState().setPhase("F3"); + render(); + expect(screen.getByTestId("phase-F1")).toHaveAttribute("data-state", "done"); + expect(screen.getByTestId("phase-F3")).toHaveAttribute("data-state", "running"); + expect(screen.getByTestId("phase-F5")).toHaveAttribute("data-state", "pending"); +}); + +test("finalized marks all phases done (green)", () => { + useSessionStore.getState().setPhase("F8"); + render(); + for (const id of ["F1", "F4", "F8"]) { + expect(screen.getByTestId(`phase-${id}`)).toHaveAttribute("data-state", "done"); + } +}); +``` + +Also add, in `frontend/src/shell/f1-loop.test.tsx`, an optimistic-F1 assertion right after the SSE connects (after line 39 `await waitFor(() => expect(FakeEventSource.instances).toHaveLength(1));`): + +```tsx + // Optimistic lifecycle: a brand-new question paints F1 immediately (before any gate). + await waitFor(() => expect(useSessionStore.getState().currentPhase).toBe("F1")); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cd frontend && npx vitest run src/shell/WorkflowBar.test.tsx src/shell/f1-loop.test.tsx` +Expected: FAIL — `finalized` prop has no effect yet; `currentPhase` stays `null` after a new question. + +- [ ] **Step 3: Implement the lifecycle changes** + +In `frontend/src/shell/WorkflowBar.tsx`, change the signature and the state computation: + +```tsx +export function WorkflowBar({ finalized = false }: { finalized?: boolean }) { + const currentPhase = useSessionStore((s) => s.currentPhase); + const phaseError = useSessionStore((s) => s.phaseError); + const activeIdx = PHASES.findIndex((p) => p.id === currentPhase); +``` + +Inside the `PHASES.map`, replace the `state` and `connectorDone` computations with: + +```tsx + const isActive = currentPhase === p.id; + const isDone = activeIdx > -1 && i < activeIdx; + const state: DotState = finalized + ? "done" + : isActive + ? phaseError === p.id + ? "error" + : "running" + : isDone + ? "done" + : "pending"; + const connectorDone = finalized || (activeIdx > -1 && i <= activeIdx); +``` + +In `frontend/src/shell/SteerInput.tsx`, add `setPhase` from the store and call it on new-question creation. Change the store hook (line ~27) and the `else` branch of `submit` (lines ~36-39): + +```tsx + const setLastUserEntry = useSessionStore((s) => s.setLastUserEntry); + const setPhase = useSessionStore((s) => s.setPhase); +``` + +```tsx + } else { + const { id } = await createSession({ question: trimmed }); + setPhase("F1"); // optimistic: paint F1 yellow during the cold start, before the first gate + onSessionCreated?.(id); + } +``` + +In `frontend/src/shell/AppShell.tsx`, derive `finalized` and pass it. Add near the other derived values (after line ~54 `const refresh = ...` or beside the `working` computation ~line 134): + +```tsx + const activeSession = sessions.find((s) => s.id === activeSessionId) ?? null; + const finalized = activeSession?.status === "finalized"; +``` + +Then change the `` render (line ~172) to: + +```tsx + +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cd frontend && npx vitest run src/shell/WorkflowBar.test.tsx src/shell/f1-loop.test.tsx` +Expected: PASS. + +Run: `cd frontend && npx tsc -b` +Expected: no errors. + +- [ ] **Step 5: Commit** + +```bash +git add frontend/src/shell/WorkflowBar.tsx frontend/src/shell/SteerInput.tsx frontend/src/shell/AppShell.tsx frontend/src/shell/WorkflowBar.test.tsx frontend/src/shell/f1-loop.test.tsx +git commit -m "feat(workflow-bar): optimistic F1 at start; all-green when session finalized" +``` + +--- + +## Task 6: Frontend — total elapsed timer + +**Files:** +- Create: `frontend/src/shell/ElapsedTimer.tsx` +- Test: `frontend/src/shell/ElapsedTimer.test.tsx` +- Modify: `frontend/src/shell/WorkflowBar.tsx` (render the timer after the dots) +- Modify: `frontend/src/shell/AppShell.tsx` (pass `createdAt`/`updatedAt`, with a local fallback) + +**Interfaces:** +- Consumes: nothing new. +- Produces: `export function ElapsedTimer({ startedAt, stoppedAt }: { startedAt: string | null; stoppedAt?: string | null }): ReactElement | null` — ticks every second from `startedAt` until `stoppedAt` is set (then frozen); renders `null` when `startedAt` is null. Format `Xm Ys`. `WorkflowBar` gains `createdAt`/`updatedAt` props and renders `` after the phase dots. + +- [ ] **Step 1: Write the failing test** + +Create `frontend/src/shell/ElapsedTimer.test.tsx`: + +```tsx +import { render, screen } from "@testing-library/react"; +import { ElapsedTimer } from "./ElapsedTimer"; + +test("renders nothing without a start time", () => { + const { container } = render(); + expect(container).toBeEmptyDOMElement(); +}); + +test("frozen elapsed when stoppedAt is set (2m 5s)", () => { + render( + , + ); + expect(screen.getByText("2m 5s")).toBeInTheDocument(); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd frontend && npx vitest run src/shell/ElapsedTimer.test.tsx` +Expected: FAIL — cannot resolve `./ElapsedTimer`. + +- [ ] **Step 3: Implement `ElapsedTimer` and render it in `WorkflowBar`** + +Create `frontend/src/shell/ElapsedTimer.tsx`: + +```tsx +import { useEffect, useState } from "react"; + +function fmt(ms: number): string { + const total = Math.max(0, Math.floor(ms / 1000)); + return `${Math.floor(total / 60)}m ${total % 60}s`; +} + +/** + * Total process time, anchored on the session's created_at (robust to reload and + * resume). Ticks every second while running; freezes at stoppedAt once the session + * is finalized. Renders nothing until a start time is known. + */ +export function ElapsedTimer({ + startedAt, + stoppedAt, +}: { + startedAt: string | null; + stoppedAt?: string | null; +}) { + const [now, setNow] = useState(() => Date.now()); + + useEffect(() => { + if (!startedAt || stoppedAt) return; + const t = setInterval(() => setNow(Date.now()), 1000); + return () => clearInterval(t); + }, [startedAt, stoppedAt]); + + if (!startedAt) return null; + const start = new Date(startedAt).getTime(); + const end = stoppedAt ? new Date(stoppedAt).getTime() : now; + return ( + + {fmt(end - start)} + + ); +} +``` + +In `frontend/src/shell/WorkflowBar.tsx`: import the timer and extend the props, then wrap the returned `