From d27afd08962275154633d2f57eb36c2efc6f0596 Mon Sep 17 00:00:00 2001 From: mptyl Date: Tue, 30 Jun 2026 12:48:56 +0200 Subject: [PATCH] fix(gate): read reviewer_select/confirm choice from the choices[] array MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The frontend's reviewer widgets uniformly send the picked option in a `choices` array (SelectWidget/ArtifactGateWidget: `choices: [optionId]`), but the gate's reviewer_select and reviewer_confirm(reject) handlers read `resp.choice` (singular). Result: every single-select gate saw an undefined choice, answered "Nessuna scelta ricevuta", and re-presented forever — the workflow could never pass F1. (reviewer_decide/multiselect already read `resp.choices`, so it worked.) Add a shared selectedChoice(resp) helper reading choices[0] (falling back to the legacy singular choice); both handlers use it. TDD: gate/__tests__/gate_choice.test.js RED->GREEN; full gate suite 28/28. Verified LIVE (Playwright -> real Pi -> GLM 5.2): a single-select F1 answer is now accepted and the workflow advances (clarification 2/4 -> 3/4). The same run also live-verified the F1 hang fix (418187a). Co-Authored-By: Claude Opus 4.8 --- PROJECT_STATE.md | 55 +++++++++++++------ .../gate/__tests__/gate_choice.test.js | 25 +++++++++ harness/.pi/extensions/tht-gate.js | 15 ++++- 3 files changed, 74 insertions(+), 21 deletions(-) create mode 100644 harness/.pi/extensions/gate/__tests__/gate_choice.test.js diff --git a/PROJECT_STATE.md b/PROJECT_STATE.md index 6fe3ce4e..1cb7652b 100644 --- a/PROJECT_STATE.md +++ b/PROJECT_STATE.md @@ -78,7 +78,27 @@ Opens frontend at http://localhost:5173 → backend :8787. - Global user rules (`~/.claude/CLAUDE.md`): think before coding, simplicity first, surgical changes, goal-driven verification. -## Most recent work — F1 reviewer-widget hang fix + multiselect guidance (committed 2026-06-30; authored 2026-06-29) +## Live verification + reviewer_select fix (2026-06-30, afternoon) + +Drove the real stack (Playwright → backend → real Pi → GLM 5.2 → DWH) end-to-end. + +- **F1 hang fix (`418187a`) VERIFIED LIVE.** Answered an F1 reviewer widget; Pi resumed (model + socket reopened) and the gate produced new output — vs the old silent hang. The transition + "silent hang → gate re-presents/advances" proves `ctx.ui.input` now resolves. +- **New bug found + fixed: reviewer_select `choices` vs `choice`.** The gate's `reviewer_select` + (and `reviewer_confirm` reject) read `resp.choice` (singular) but the frontend uniformly sends + `choices: [id]` (array) — so every single-select gate answered "Nessuna scelta ricevuta" and + re-proposed forever (multiselect was fine; it already read `choices`). Fix: a shared + `selectedChoice(resp)` helper (`harness/.pi/extensions/tht-gate.js`) reading the array; both + handlers use it. TDD: `gate/__tests__/gate_choice.test.js` RED→GREEN, full gate suite **28/28**. + VERIFIED LIVE: a single-select answer is now accepted and the workflow advances (2/4 → 3/4). +- **Resume cold-start STALL confirmed (open item #1).** On `/riprendi-sessione`, GLM 5.2 narrates + the bootstrap step then ends the turn without the tool call → Pi idle, unrecoverable from the UI. + Memory: `thothii-resume-cold-start-stall.md`. +- **GLM 5.2 F1 is slow (~3-4 min, ~50+ reads) but works** — looks stuck but isn't; don't hit + "Stop and save" (it `POST /close`s → kills Pi). Memory: `thothii-glm52-f1-slow-not-stuck.md`. + +## Earlier work — F1 reviewer-widget hang fix + multiselect guidance (committed 2026-06-30; authored 2026-06-29) Two fixes, **committed to `main`** (7 files): @@ -108,9 +128,9 @@ Two fixes, **committed to `main`** (7 files): `reviewer_select`. Guidance-only — no new widget (`frontend MultiselectWidget` already exists). -Verified this session: backend `npx vitest run` **67/67 green**; `tsc --noEmit -p .` **OK**; -fake-pi contract `node --test test_fake_pi_contract.mjs` **2/2 green**. Harness pytest and -frontend NOT re-run (untouched by these changes). **NOT verified live** — see open item 4. +Verified at commit time: backend `npx vitest run` **67/67 green**; `tsc --noEmit -p .` **OK**; +fake-pi contract `node --test test_fake_pi_contract.mjs` **2/2 green**. **Verified LIVE +2026-06-30** (see the top "Live verification" section). ## Most recent feature — Session management (MERGED to main @ 2c21e46) Full session management modeled on Claude's UI, all three layers: @@ -127,30 +147,29 @@ Full session management modeled on Claude's UI, all three layers: `docs/superpowers/plans/2026-06-29-session-management.md`. ### ⚠️ Open items / pending gates -1. **Resume cold-start e2e (MANUAL, not yet run).** The resume chain is structurally - complete but unproven live: drive a session to F4, stop it, resume in a fresh process, - confirm it re-enters at F4 (not a new-question kickoff). **Do not advertise the "Resume" - button as working until this passes.** +1. **Resume cold-start STALLS (root cause confirmed 2026-06-30) — NOT fixed.** On + `/riprendi-sessione` GLM 5.2 narrates the bootstrap step ("esamino la sessione…") then ends + the turn without the tool call → Pi idle, unrecoverable from the UI (steer doesn't revive an + ended turn). The new-question path works, so it's resume-specific. Fix direction: harden the + resume kickoff/SKILL so the model chains into the tool call. Memory: + `thothii-resume-cold-start-stall.md`. **Do not advertise "Resume" as working until fixed.** 2. **Full Playwright live-stack verification (MANUAL, not yet run).** 3. **Minor backlog (non-blocking):** explicit id-traversal guard in `delete_session` (today gated by `load_session`); `close_session` could reuse `_save_touched` (DRY); delete-via-kebab integration test skipped (base-ui Menu portal not drivable in jsdom — the dialog itself is unit-tested); a couple of test-file lint nits. -4. **Live-verify the F1 reviewer-widget hang fix (committed 2026-06-30).** The fix is committed - and proven against Pi's source + a now-faithful fake, but the browser round-trip is still - unproven. With VPN up: start the stack, drive an F1 session past the first answer, confirm - the widget unblocks and the model continues. (On 2026-06-29 DWH REST was unreachable with - VPN off, so the live e2e couldn't run; Ollama was reachable and `pi` present.) -5. **Model caveat (likely "struggles to finish the task" symptom).** Settings use - `deepseek-v4-flash` (thinking high) but the harness is tuned for GLM 5.2, and F1 needs - many autonomous tool calls; even with the hang fixed a "flash" model may not complete F1. - Separate config decision — try GLM 5.2 when validating live. +4. **DONE — F1 hang fix live-verified 2026-06-30** (see top section). The live verification + also surfaced + fixed the reviewer_select `choices` mismatch. +5. **Resolved: settings use `zai/glm-5.2`/medium** (not deepseek-flash); GLM 5.2 drives F1 + fine, just slowly (~3-4 min, ~50+ reads). To tell a truly stalled Pi from a merely-slow one, + check its sockets/children. Memory: `thothii-glm52-f1-slow-not-stuck.md`. ## Where design history lives - Specs: `docs/superpowers/specs/` · Plans: `docs/superpowers/plans/` - SDD execution ledger (gitignored scratch): `.superpowers/sdd/progress.md` - Auto-memory index: `~/.claude/projects/-Users-mp-projects-ThothII/memory/MEMORY.md` - (notes on the Pi RPC event vocabulary and the Omics Portal/GSD design system). + (Pi RPC event vocabulary, ui.input id correlation, Omics Portal/GSD design system, + resume cold-start stall, GLM 5.2 F1 slow≠stuck). ## Git `main` tracks `origin` (github.com/mptyl/ThothII). The F1 hang-fix (7 files + this diff --git a/harness/.pi/extensions/gate/__tests__/gate_choice.test.js b/harness/.pi/extensions/gate/__tests__/gate_choice.test.js new file mode 100644 index 00000000..6a0bcc43 --- /dev/null +++ b/harness/.pi/extensions/gate/__tests__/gate_choice.test.js @@ -0,0 +1,25 @@ +const test = require("node:test"); +const assert = require("node:assert"); +const { selectedChoice } = require("../../tht-gate.js"); + +// The frontend's reviewer widgets (SelectWidget, ArtifactGateWidget) always send the +// picked option in a `choices` ARRAY: { id, kind, choices: [optionId] }. The gate's +// select / artifact-gate handlers must read that array — reading a singular `choice` +// yields undefined, so the gate answers "no choice received" and re-presents forever. + +test("selectedChoice reads the frontend's choices[] array (select widget)", () => { + assert.equal(selectedChoice({ id: "u1", kind: "select", choices: ["old75_frozen"] }), "old75_frozen"); +}); + +test("selectedChoice reads choices[] for the artifact-gate reject", () => { + assert.equal(selectedChoice({ id: "u1", kind: "artifact-gate", choices: ["reject"] }), "reject"); +}); + +test("selectedChoice falls back to the legacy singular choice", () => { + assert.equal(selectedChoice({ id: "u1", choice: "a" }), "a"); +}); + +test("selectedChoice returns undefined when nothing is selected", () => { + assert.equal(selectedChoice({ id: "u1", choices: [] }), undefined); + assert.equal(selectedChoice({ id: "u1" }), undefined); +}); diff --git a/harness/.pi/extensions/tht-gate.js b/harness/.pi/extensions/tht-gate.js index 1a159930..5c2b9b17 100644 --- a/harness/.pi/extensions/tht-gate.js +++ b/harness/.pi/extensions/tht-gate.js @@ -207,6 +207,14 @@ function advanceIfReady(ctx, session) { // accepted as a final answer — the widget is re-presented. Real escapes (Back/Exit/Other) // are always present as selectable options in the descriptor, so cancel has no // legitimate meaning. +// The frontend's reviewer widgets always carry the picked option(s) in a `choices` +// ARRAY (SelectWidget/ArtifactGateWidget send `choices: [optionId]`). Single-pick +// handlers read the first element; `choice` (singular) is accepted as a legacy form. +export function selectedChoice(resp) { + if (Array.isArray(resp?.choices)) return resp.choices[0]; + return resp?.choice; +} + export async function emitAndWait(ctx, descriptor) { for (;;) { const value = await ctx.ui.input(JSON.stringify(descriptor), ""); @@ -398,9 +406,10 @@ export default function (pi) { return textResult("Il reviewer vuole tornare indietro."); if (resp.control === "exit") return textResult("Il reviewer vuole uscire."); - const chosen = opts.find((o) => o.id === resp.choice); + const choice = selectedChoice(resp); + const chosen = opts.find((o) => o.id === choice); return textResult( - `Scelta del reviewer: ${chosen ? chosen.label : resp.choice}`, + `Scelta del reviewer: ${chosen ? chosen.label : choice}`, ); }, }); @@ -522,7 +531,7 @@ export default function (pi) { action: { kind: "approve_reject", prompt: "Approvi o rifiuti?" }, }); const resp = await emitAndWait(ctx, widget); - if (resp.control === "freetext" || resp.choice === "reject") { + if (resp.control === "freetext" || selectedChoice(resp) === "reject") { return textResult( `Rifiutato${resp.text ? ` (motivo: ${resp.text})` : ""}: rivedi e riprova.`, );