ThothII/docs/superpowers/specs/2026-07-02-reviewer-gate-ux-fixes-design.md

Reviewer gate UX fixes — design

Date: 2026-07-02 · Status: approved (design) · Scope: frontend widgets + harness Pi gate (tht-gate.js, builders.js) + SKILL.md. No tht CLI, phase.py, or workflow.yaml semantic changes.

Problem

Driving a live session surfaced four defects at the reviewer gates. Two user-reported symptoms ("the multiselect has no Exit/Back"; "phase completion only offers Exit and Other-specify, no way forward, and Other is ignored") decompose into four root causes, all verified against source.

1. The multiselect widget renders no Exit/Back/Other controls. MultiselectWidget (frontend/src/widgets/MultiselectWidget.tsx:30-58) is the ONLY pick widget that never renders <ReservedControls> — SelectWidget (SelectWidget.tsx:16) and ArtifactGateWidget (ArtifactGateWidget.tsx:54-57) both do. The backend already sets reserved: ["back","exit","other"] on the multiselect descriptor (builders.js:95) and the reviewer_decide handler already maps control:"back"|"exit"| "freetext" (tht-gate.js:524-532) — the field is just never consumed by the frontend.

2. The phase/artifact gate renders no merito buttons — and "Other" silently approves. reviewer_confirm builds buildArtifactGate({..., action:{kind:"approve_reject"}}) (tht-gate.js:578-584), but buildArtifactGate (builders.js:102-126) never populates an options array. ArtifactGateWidget renders merito buttons only from descriptor.options?.map(...) (ArtifactGateWidget.tsx:44-52) and ignores action entirely → zero Approva/Rifiuta buttons; the reviewer sees only the reserved [Go back][Exit][Other] controls. There is no forward button. Worse, in the handler (tht-gate.js:586-594) the only guards are control:"freetext"/choice:"reject" → reject, control:"back" → back, control:"exit" → exit; control:"other" matches none and falls through to the "approved" branch (:596), advancing the phase — with no text captured. This is the exact "only Exit and Other, no way forward, Other ignored" symptom: "Other" is an accidental approve.

3. "Altro — specifica" never captures text. The working free-text path is an option carrying opens:{widget:"freetext"} (altroOption, builders.js:38-44) routed through LinkageHost — but LinkageHost is wired ONLY in ArtifactGateWidget (ArtifactGateWidget.tsx:17-33); SelectWidget/MultiselectWidget ignore opens. The reserved "Other — specify" button (ReservedControls.tsx:1-6) sends a bare {control:"other"} with no text and no child widget, and the harness maps only control:"freetext" (resolveSelectOutcome, tht-gate.js:245-254; the three handlers) — so "Other" collects nothing and is dropped (select/decide) or mis-approves (confirm, see #2).

4. The empty memory phase (F2) shows a pointless empty checklist. SKILL.md Phase 2 (SKILL.md:162-175) always presents reviewer_decide(allow_empty:true, advance:true), even when tht memory search returned zero candidates. buildMultiselectRequest accepts allowEmpty:true with zero options (builders.js:78-97), so the reviewer sees an empty checklist + a "Select all" over nothing + an enabled Confirm; only after that dead click does advanceIfReady (tht-gate.js:540, :193-204) auto-advance F2 → F3. No "there are no memories" message is ever shown. F2 already auto-advances; the empty widget is the only friction.

Goals

• Give reviewer_decide (multiselect) the same Back/Exit/Other escape hatches as the other pick widgets.
• Make the phase/artifact gate render an explicit forward control ("Approva e continua") and a "Rifiuta" — and stop "Other" from silently approving.
• Make "Altro — specifica" open a text field, carry the text to the model as actionable feedback (recorded verbatim), consistently across pick widgets and the gate.
• When F2 memory is empty, skip the widget: show an info message and auto-advance.

Non-goals

• No new auto-advance phases. F1/F3/F5/F7 keep their reviewer_confirm kind:"phase" gate — each reviews something real (F1 "done clarifying", F3 question.md, F5 the schema-linking readable view per Discipline 7, F7 sql_final.sql). _AUTO_ADVANCE_PHASES stays {2,6} — the deliberate HITL contract (SKILL.md:8-11).
• No change to the advance semantics in workflow.yaml or the auto_advance_eligible rule in phase.py.
• No widget-registry restructure and no new widget kind (info/freetext already exist).

Part 1 — Multiselect reserved controls (frontend only)

MultiselectWidget.tsx: import ReservedControls and render it after the Confirm button, mirroring SelectWidget/ArtifactGateWidget:

<ReservedControls
  reserved={descriptor.reserved}
  onControl={(c) => onRespond({ id: descriptor.id, control: c })}
/>

No backend change — reviewer_decide already emits reserved and handles the responses. Result: options + Select-all + [Go back] [Exit] [Other — specify]. ("Other" is made functional in Part 3.)

Tests — vitest (MultiselectWidget.test.tsx, extend): the three reserved buttons render; clicking each fires onRespond({id, control}) with back/exit/other; the merito Confirm still emits {kind:"multiselect", choices}.

Part 2 — Gate renders explicit merito options; "Other" no longer approves (harness + test)

Backend — derive options from action.kind in buildArtifactGate (builders.js). ArtifactGateWidget is unchanged (it already maps descriptor.options); the fix is to make the builder populate them:

• action.kind:"approve_reject" → options:[{id:"approve", label:"Conferma e prosegui", recommended:true}, {id:"reject", label:"Rifiuta"}]
• action.kind:"confirm" → options:[{id:"approve", label:"Conferma e prosegui", recommended:true}]
• action.kind:"view_only" → options:[] (no merito action; reserved controls only)

The forward option is labelled "Conferma e prosegui" so the "advance to next phase" affordance is unambiguous (the user's core complaint). recommended floats it to the top (the frontend already renders (consigliato) on select; the artifact-gate button list keeps approve first).

Backend — tighten the reviewer_confirm handler (tht-gate.js:585-594): advance ONLY on an explicit approve choice. Replace the permissive fall-through with:

• selectedChoice(resp) === "approve" → run the privileged action (existing kind branches).
• selectedChoice(resp) === "reject" → "Rifiutato: rivedi e riprova."
• control:"freetext" → actionable feedback (Part 3), NOT reject.
• control:"back"/"exit" → unchanged.
• anything else (incl. a stray control:"other" that Part 3 will eliminate) → re-present the widget via the no-limbo loop; never auto-approve.

Tests — node builders.test.js (extend): buildArtifactGate with each action.kind yields the documented options; approve is first and recommended. The ArtifactGateWidget test already injects options:[{id:"approve",...}] — keep it aligned. Handler wiring is L2/live per repo convention; the behavioral guarantee rides on builders.test.js + a deferred live phase-gate check.

Part 3 — "Altro — specifica" captures text and is actionable (frontend + harness)

Frontend — make the reserved "other" control open a freetext child, uniformly. Rather than per-widget opens wiring, centralize: when a reserved control "other" is clicked, collect text via FreetextWidget (reusing the LinkageHost merge shape) and submit {id, control:"freetext", text} — the response the harness already understands (resolveSelectOutcome/handlers map control:"freetext"). Concretely, ReservedControls (or a small wrapper it delegates to) renders an inline FreetextWidget on "other" instead of firing onControl("other") immediately. This fixes "Other" for select, multiselect, and the gate in one place, and the bare control:"other" disappears from the wire.

Harness — treat gate free-text as actionable feedback, not rejection (tht-gate.js:586-589). Split the current combined guard: choice:"reject" → "Rifiutato: rivedi e riprova."; control:"freetext" → return Altro (reviewer): <text>. Valuta e agisci, poi ri-presenta il gate. — matching the wording already used by reviewer_decide/reviewer_select (tht-gate.js:456,:526) and Discipline 10 (record the reviewer's words verbatim in the decision rationale). "Other" is present only where a real widget renders — with Part 4 the empty-memory case shows no widget, so "Other" never appears on a no-op gate (approach A / C).

Tests — vitest: clicking reserved "Other" reveals a text input; submitting sends {control:"freetext", text} (not control:"other"). node: the reviewer_confirm freetext branch returns the actionable "Altro (reviewer): …" string, distinct from the reject string.

Part 4 — Empty memory phase auto-advances with a message (harness + SKILL)

Chosen mechanism (option i): short-circuit the empty case inside reviewer_decide. The model already calls reviewer_decide(allow_empty:true, advance:true) in Phase 2 with zero merito options when memory is empty. In the handler (tht-gate.js:508-546), before emitting the widget: if the merito option list is empty AND allow_empty AND advance, do NOT emit the multiselect — show the reviewer an info notice via ctx.ui.notify("Nessuna memory riutilizzabile per questa domanda — passo alla fase successiva.", "info") (bridged to the client {type:"info"} event by session-bridge.ts:26-27) and call advanceIfReady. Return a textResult reporting the auto-advance. This keeps the harness as the advancer (respects the anti-bypass hook), reuses advanceIfReady (which only advances an auto-eligible phase — F2/F6 with zero substantive decisions), needs no new tool, and leaves the model's Phase-2 call unchanged.

SKILL.md Phase 2 (SKILL.md:162-175): document the short-circuit — when tht memory search returns no candidates, still issue the single reviewer_decide(allow_empty:true, advance:true) with empty merito options; the gate shows the "no memories" info and auto-advances (no separate reviewer_confirm kind:"phase"). The non-empty path is unchanged (selected applied, deselected recorded, closed by the phase gate).

Tests — node: reviewer_decide with empty options + allow_empty:true + advance:true emits an info descriptor and does not emit a multiselect, then calls the advance path (assert via the fake ctx/tht shim used by the gate tests). Non-empty options → unchanged (multiselect emitted, no info). No phase.py change, so no pytest.

Build order & testing

Order: Part 1 → Part 2 → Part 3 → Part 4. Part 1 is a self-contained frontend fix. Part 2 makes the gate usable (its forward button). Part 3 depends on Part 2 (both touch the reviewer_confirm handler + the "other" wire) and removes the bare control:"other". Part 4 is orthogonal but lands last so the SKILL text reflects the finished gate behavior.

TDD where cheap: vitest RED→GREEN on the widget changes (cd frontend && npx vitest run); node RED→GREEN on builders.test.js and the gate handler tests (node --test under harness/.pi/extensions/gate/__tests__). Typecheck the frontend (cd frontend && npx tsc -b). The tht-gate.js glue is L2/live per repo convention; full verification of the phase gate + empty-memory advance through the real stack is deferred (needs VPN — see PROJECT_STATE open items).

Risks

• Gate glue is not unit-tested (repo convention): the reviewer_confirm handler tightening (Part 2) and the empty-memory short-circuit (Part 4) rely on builders.test.js + gate handler tests + a deferred live check. Mitigation: keep handler logic minimal; push option-shaping into the pure builders.js (which IS unit-tested).
• Reserved "other" → freetext centralization (Part 3) changes a shared component (ReservedControls) used by every pick widget. Mitigation: cover all three widgets with a vitest each; keep the non-"other" controls (back/exit) firing immediately as today.
• Empty-memory short-circuit scope: the reviewer_decide short-circuit triggers on any empty+allow_empty+advance call, not F2 by name. This matches intent (only a trivially empty auto-eligible phase advances — advanceIfReady enforces eligibility), but the test must assert a non-empty call is untouched so normal decides never short-circuit.