diff --git a/docs/superpowers/specs/2026-07-03-workflow-ui-fixes-design.md b/docs/superpowers/specs/2026-07-03-workflow-ui-fixes-design.md new file mode 100644 index 00000000..741d998f --- /dev/null +++ b/docs/superpowers/specs/2026-07-03-workflow-ui-fixes-design.md @@ -0,0 +1,188 @@ +# Workflow UI fixes — design + +Date: 2026-07-03 +Status: approved (brainstorming), pending plan + +Five defects surfaced while testing the app. All are UI/contract issues in the +frontend, plus one root-cause fix in the harness gate. No changes to the Pi RPC +wire protocol. + +## Scope + +| # | Problem | Fix location | +|---|---------|--------------| +| 1 | Phase circles don't reflect lifecycle (F1 never turns yellow at start; no explicit green/red on end) | frontend | +| 2 | No total elapsed-time indicator after the 8 circles | frontend | +| 3 | Reviewer artifact gate shows nothing (revised question, schema link, CTEs, final SQL) | frontend (contract fix) | +| 4 | Complex artifacts need a 90% modal, fully formatted, with a Mermaid diagram for schema linking | frontend | +| 5 | Both "Altro - specificare" and "other" appear in dialogs | harness | + +--- + +## Problem 5 — Duplicate "Altro"/"other" (root cause) + +**Cause.** `builders.js` always sets `reserved: ["back","exit","other"]`, which the +frontend renders as the `Other — specify` control. Separately, `reviewer_select` +and `reviewer_decide` strip model-supplied options via `isReserved(o.label)` +(`reserved-labels.mjs`), but `isReserved` does an **exact-string** match against +`ALTRO = "Altro — specifica…"`. The model emits variants (`"Altro - specificare"`, +plain hyphen; `"altro"`; English `"Other — specify"`) that slip through the exact +match, so both the model's free-text option **and** the reserved control appear. + +**Fix.** Make `isReserved(label)` robust in `reserved-labels.mjs`: + +- Normalize: lowercase, strip diacritics (NFD + remove combining marks), collapse + punctuation/whitespace, trim. +- Treat as reserved if the normalized label **starts with** `altro` or `other`, or + equals the normalized canonical back/exit labels (`QUIT_LABEL`, `BACK_LABEL`). + +`stripReserved` and both reviewer tools inherit the fix (they call `isReserved`). +Result: only the reserved `Other — specify` control remains (English, per the UI +convention that chrome/labels stay English). + +**Tests.** Unit test in the harness reserved-labels test: variants +`"Altro - specificare"`, `"altro"`, `"ALTRO — specifica…"`, `"Other — specify"` are +all reserved; a normal merit option (`"procedura"`) is not. + +--- + +## Problem 3 — Artifact gate renders nothing (contract mismatch) + +**Cause.** The harness sends `artifact: { kind, data, version }` (see +`tht-gate.js` `reviewer_confirm` → `buildArtifactGate`, golden +`artifact_gate_F5.json`). The frontend `ArtifactGateWidget` reads +`descriptor.artifact?.content`, which never exists → nothing renders. It is a +`data` vs `content` mismatch, not missing data. + +**Fix.** Rendering reads `artifact.data` and switches on `artifact.kind`. This is +subsumed by the Problem 4 modal: the old `
{artifact.content}
` is +replaced by `ArtifactView` (below). + +--- + +## Problem 4 — 90% modal + ArtifactView + erDiagram + +### ArtifactGateModal + +New `frontend/src/widgets/ArtifactGateModal.tsx`, built on the existing Radix +dialog (`components/ui/dialog.tsx`). + +- Size: `90vw × 90vh`. +- Layout: **top** = scrollable artifact area (`flex-1`, `overflow-auto`) rendering + `ArtifactView`; **bottom** = action bar: the gate `options` buttons + (`Conferma e prosegui` / `Rifiuta`) plus `ReservedControls` (Other/Back/Exit). +- The `option.opens → freetext` linkage keeps working via the existing + `LinkageHost`. +- Auto-opens when `pendingWidget.widget === "artifact-gate"`. The `select` / + `multiselect` / `freetext` gates stay inline as today. +- Routing: `WidgetHost` (or the widget registry) renders artifact-gate through the + modal instead of the inline `ArtifactGateWidget` card. The inline + `ArtifactGateWidget` body is replaced by the modal; its option/linkage logic is + reused. + +### ArtifactView + +New `frontend/src/viewers/ArtifactView.tsx`. Switches on `artifact.kind` and is +**defensive** about `data` (accepts a string or an object, picks common fields, +falls back to formatted JSON): + +| `artifact.kind` | Renderer | +|-----------------|----------| +| `schema_linking` | `SchemaLinkingViewer` (data = linking object) | +| `sql`, `cte_result` | `SqlViewer` (string or `{sql}` / `{ctes[], final}`) | +| `cte_plan` | ordered list of CTE names (`data` = array or `{names[]}`) | +| `question`, `phase` | `MarkdownView` (string or `{markdown}`/`{question}`) | +| default / unknown | pretty-printed JSON in a formatted `
` |
+
+### Schema-linking diagram → Mermaid erDiagram
+
+In `SchemaLinkingViewer.tsx`, replace `buildFlowchart` with `buildErDiagram`
+producing a Mermaid `erDiagram`:
+
+- Entities = promoted **tables**; each table's promoted **columns** (name format
+  `table.column`) become entity attributes.
+- Relationships = `joins` between promoted tables.
+- **Relations are drawn only if `artifact.data.joins` is present.** If the model
+  supplies no joins, tables render without edges. This intervention does **not**
+  change the harness/model to force joins into `schema_linking`.
+- Keep the existing table-view fallback and the oversized-graph cap.
+
+This viewer is shared with `SessionDocumentsPanel`, which improves consistently.
+
+---
+
+## Problem 1 — Phase-circle lifecycle (frontend-optimistic)
+
+`currentPhase` is set only on `ui_request.phase` (`sessionStore.ts`), so before
+F1's first gate (the 3–4 min cold start) no circle is yellow; and "done/green" is
+positional (`i < activeIdx`), so the last phase never greens and there is no
+explicit end signal.
+
+**Fix, in `WorkflowBar.tsx` + the new-question submit path:**
+
+- **Yellow at start.** On **new-question** creation, set `currentPhase = "F1"`
+  immediately (in the submit path — `SteerInput` / store action), so F1 is yellow
+  during cold start. On **resume**, leave `currentPhase` null until the first gate
+  (the phase is unknown until then).
+- **Green on end.** Keep positional green while advancing. Additionally, when the
+  active session is `finalized`, mark **all 8** circles `done` (green).
+- **Red on error.** Unchanged: `phaseError` (set on `info` level `error`) drives
+  the current phase's red state until the next gate.
+- `WorkflowBar` receives `finalized` from `AppShell` (derived from the active
+  session's status).
+
+Positional inference already handles multi-gate phases, auto-advanced phases
+(empty memory F2), and the reviewer `back` control (recomputed each render).
+
+---
+
+## Problem 2 — Total elapsed timer
+
+New `frontend/src/shell/ElapsedTimer.tsx`, rendered after the 8 circles inside the
+workflow strip.
+
+- Anchor = active session's `created_at` (robust to reload/resume).
+- Ticks every second while not `finalized`; freezes at `updated_at − created_at`
+  once `finalized`.
+- Fallback: if `created_at` is not yet available (brand-new session before the
+  sessions-list poll returns it), anchor on a local timestamp captured when the
+  session became active.
+- Format: `Xm Ys`.
+- `AppShell` passes `createdAt` / `finalized` to `WorkflowBar`, which lays out the
+  circles (centered) and the timer (trailing).
+
+---
+
+## Files touched
+
+**Frontend**
+- `shell/WorkflowBar.tsx` — accept `finalized`/`createdAt`; optimistic F1; all-green
+  on finalized; render `ElapsedTimer`.
+- `shell/AppShell.tsx` — pass active session `created_at`/`finalized` to `WorkflowBar`.
+- new-question submit path (`shell/SteerInput.tsx` and/or `store/sessionStore.ts`) —
+  set `currentPhase = "F1"` on new question.
+- new `shell/ElapsedTimer.tsx`.
+- new `widgets/ArtifactGateModal.tsx`; `widgets/index.ts` / `shell/WidgetHost.tsx`
+  route artifact-gate to the modal.
+- new `viewers/ArtifactView.tsx`.
+- `viewers/SchemaLinkingViewer.tsx` (+ `viewers/mermaid.ts` if needed) — erDiagram.
+
+**Harness**
+- `.pi/extensions/reserved-labels.mjs` — normalized `isReserved`.
+- reserved-labels test — variant coverage.
+
+## Testing
+
+- Harness: `node --test` on the reserved-labels test (variant coverage);
+  existing `builders`/gate tests stay green.
+- Frontend: `npx vitest run` — update/keep `ArtifactGateWidget` / f1-loop /
+  sessionStore tests; add tests for `ArtifactView` kind routing, `ElapsedTimer`
+  formatting/freeze, `WorkflowBar` optimistic-F1 + finalized-all-green, and the
+  erDiagram builder. `npx tsc -b` clean.
+
+## Out of scope
+
+- No Pi RPC wire-protocol change; no authoritative phase-lifecycle events from the
+  harness (chosen: frontend-optimistic).
+- No harness/model change to force `joins` into `schema_linking`.
+- Reserved control label stays English (`Other — specify`) per UI convention.