docs(spec): design for workflow UI fixes (phase circles, timer, artifact modal, Altro dedup)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 4.8
@@ -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 `<pre>{artifact.content}</pre>` 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 `<pre>` |
|
||||
|
||||
### 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.
|
||||
Reference in New Issue
Block a user