From cef9ae4368a390d9aeb1b31725289e9c6cb2765c Mon Sep 17 00:00:00 2001 From: mptyl Date: Tue, 30 Jun 2026 18:01:50 +0200 Subject: [PATCH] feat(harness): single-select answers auto-confirm (reviewer_select persists) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F — reviewer_select options may now carry a `decision` payload {type, subject, detail?, rationale?} plus an optional `advance`. Picking such an option IS the confirmation: the gate persists it directly (tht decision add) and optionally advances, with no redundant reviewer_decide/reviewer_confirm follow-up gate. Options without a payload stay ask-only; back/exit/Other never persist. Pure logic extracted + exported for unit tests: resolveSelectOutcome (classifies the response) and decisionAddArgs (shared with reviewer_decide, DRY). Gate JS suite 33/33 (gate_select_decision.test.js, +5); harness pytest 269 unchanged. Contract docs updated together: reviewer_select tool description, SKILL.md (widget summary, disciplines 2-3, Phase-1 single-pick), and the CLAUDE.md gate note. Live verification (model truly emits reviewer_select+decision, decision in review_decisions.jsonl, no follow-up gate) deferred to workstream G — it is model-behavior-dependent. Co-Authored-By: Claude Opus 4.8 --- CLAUDE.md | 5 +- PROJECT_STATE.md | 28 ++++-- .../__tests__/gate_select_decision.test.js | 48 ++++++++++ harness/.pi/extensions/tht-gate.js | 92 ++++++++++++++----- harness/.pi/skills/tht-sessione/SKILL.md | 38 ++++---- 5 files changed, 159 insertions(+), 52 deletions(-) create mode 100644 harness/.pi/extensions/gate/__tests__/gate_select_decision.test.js diff --git a/CLAUDE.md b/CLAUDE.md index dfa85692..3273fdfb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -59,8 +59,9 @@ frontend (React/SSE) → backend (Fastify) → pi --mode rpc → tht/harness → (`backend/data/settings.json`), not a DB. - **Human-in-the-loop gate contract.** The model proposes; a human reviewer decides at gates - via widgets (`reviewer_select` = iterate/no decision; `reviewer_decide` = the choice IS the - decision; `reviewer_confirm` = artifact/phase gate). The frontend renders these + via widgets (`reviewer_select` = single pick — a chosen option carrying a `decision` payload + auto-confirms/persists directly, an option without one only asks; `reviewer_decide` = multiselect, + each choice IS a decision; `reviewer_confirm` = artifact/phase gate). The frontend renders these widget-descriptors (`src/widgets/` registry) and the live transcript is rebuilt in-memory from the SSE stream (`src/store/sessionStore.ts`) — it is not persisted. diff --git a/PROJECT_STATE.md b/PROJECT_STATE.md index 509e049e..88181b64 100644 --- a/PROJECT_STATE.md +++ b/PROJECT_STATE.md @@ -81,8 +81,8 @@ Opens frontend at http://localhost:5173 → backend :8787. ## UI/UX redesign + Resume — IN PROGRESS (2026-06-30, evening) Approved multi-workstream plan: **`~/.claude/plans/foamy-forging-dahl.md`** (read it to resume). -Memory: `thothii-ui-redesign-inprogress.md`. **D + E merged @ `0eeb3f7` (pushed); B + C -implemented + live-verified (not yet committed as of this update).** +Memory: `thothii-ui-redesign-inprogress.md`. **D + E merged @ `0eeb3f7` (pushed); B + C @ `b056ff3` +(live-verified); F implemented + committed (live check deferred to G). All on `main`, not yet pushed.** - **D — DONE** (`c12bdcd`): session display `name` = 3-5 Italian keywords via **YAKE** (no LLM), derived in `tht session new` (CLI layer); `create_session` core unchanged (`name=None` default). @@ -92,26 +92,34 @@ implemented + live-verified (not yet committed as of this update).** model-stream tail**; `WorkingSpinner` extracted to its own module; the separate spinner button + orphaned `Transcript.tsx` removed. Frontend 87/87, tsc clean. **Live visual check DONE (2026-06-30):** inline spinner opens the panel; 5-line collapsed tail; expand → full transcript. -- **B — DONE** (uncommitted): `WorkflowBar` is now colored **dots** F1..F8, no phase-name text +- **B — DONE** (`b056ff3`): `WorkflowBar` is now colored **dots** F1..F8, no phase-name text (amber-translucent=running, green=done, red=error, gray=pending; green connectors lead the active dot). Each dot carries `data-state`. Error is lightweight: store `phaseError` set when an `info` `level=error` arrives during the phase, cleared on the next `ui_request` (`sessionStore.ts`). **All four states live-verified** via Playwright. -- **C — DONE** (uncommitted): right sidebar — single-line denser rows (inline status dot + name, +- **C — DONE** (`b056ff3`): right sidebar — single-line denser rows (inline status dot + name, `py-1`), a 3-level type hierarchy via **`/impeccable`** (L1 `SESSIONS` red/bold/wide-tracking · L2 section + group headers muted uppercase · L3 names normal-case), and the **"No group" label removed** (ungrouped sessions render after the last group; guarded so the empty-state still teaches when there are no groups). **Live-verified.** (Resume in `SessionMenu` stays with A1.) - **Tests:** frontend **93/93** (was 87; +3 store `phaseError`, +2 `WorkflowBar` dot-state, +1 AppShell no-"No group"), `tsc -b` clean. -- **F (pending):** single-select answers **auto-confirm** — `reviewer_select` persists the - decision directly on a concrete choice (no redundant `reviewer_decide` gate); contract change - (update its tool desc + `SKILL.md` + the `CLAUDE.md` note). back/exit/Other stay non-persisting. +- **F — DONE** (uncommitted; live check deferred to G): single-select answers **auto-confirm**. + `reviewer_select` options may carry a `decision` payload (`{type, subject, detail?, rationale?}`) + and an optional `advance`; picking such an option persists the decision directly via + `tht decision add` (shared `decisionAddArgs` helper, also used by `reviewer_decide`) — no redundant + `reviewer_decide`/`reviewer_confirm` gate. Options without a payload stay ask-only; back/exit/Other + never persist. Pure logic extracted to `resolveSelectOutcome`/`decisionAddArgs` (exported, unit- + tested). Contract docs updated: `reviewer_select` tool desc + `SKILL.md` (widget summary, + disciplines 2-3, Phase-1 single-pick) + the `CLAUDE.md` gate note. Gate JS **33/33**, harness 269. + **Live verification (model actually uses `reviewer_select`+decision, no follow-up gate, decision in + `review_decisions.jsonl`) deferred to G** — it is model-behavior-dependent. - **A (pending, riskiest):** Resume command + **FIX the resume cold-start stall** (open item #1). -- **G (later):** cross-model behavior matrix (Qwen3.6 / GLM 5.2 / Deepseek V4 / others). +- **G (later):** cross-model behavior matrix (Qwen3.6 / GLM 5.2 / Deepseek V4 / others) — also the + home for F's live verification. -**Next chunk:** **F** (single-select auto-confirm — harness gate/SKILL contract change), then **A** -(Resume in the kebab + the resume cold-start stall fix, diagnosis-first). **G** (cross-model) later. +**Next chunk:** **A** (Resume in the kebab + the resume cold-start stall fix, diagnosis-first). +**G** (cross-model, incl. F's live check) later. ## Live verification + reviewer_select fix (2026-06-30, afternoon) diff --git a/harness/.pi/extensions/gate/__tests__/gate_select_decision.test.js b/harness/.pi/extensions/gate/__tests__/gate_select_decision.test.js new file mode 100644 index 00000000..140e2bdf --- /dev/null +++ b/harness/.pi/extensions/gate/__tests__/gate_select_decision.test.js @@ -0,0 +1,48 @@ +const test = require("node:test"); +const assert = require("node:assert"); +const { resolveSelectOutcome, decisionAddArgs } = require("../../tht-gate.js"); + +// Workstream F — single-select answers auto-confirm. A concrete reviewer_select choice +// that carries a `decision` payload IS the confirmation: the gate persists it directly +// (tht decision add), with no redundant reviewer_decide gate. Control responses +// (back / exit / Other) and bare ask-only choices stay non-persisting. + +const OPTS = [ + { id: "last24", label: "Ultimi 24 mesi", decision: { type: "time_window", subject: "periodo", detail: "24m" } }, + { id: "ask_only", label: "Interpretazione A" }, // no decision payload -> ask-only +]; + +test("a concrete choice carrying a decision resolves to a persisting outcome", () => { + const out = resolveSelectOutcome(OPTS, { id: "u1", choices: ["last24"] }); + assert.equal(out.kind, "decision"); + assert.equal(out.option.label, "Ultimi 24 mesi"); + assert.deepEqual(out.decision, { type: "time_window", subject: "periodo", detail: "24m" }); +}); + +test("a concrete choice without a decision stays ask-only (no persistence)", () => { + const out = resolveSelectOutcome(OPTS, { id: "u1", choices: ["ask_only"] }); + assert.equal(out.kind, "choice"); + assert.equal(out.option.label, "Interpretazione A"); +}); + +test("control responses never persist", () => { + assert.equal(resolveSelectOutcome(OPTS, { control: "back" }).kind, "back"); + assert.equal(resolveSelectOutcome(OPTS, { control: "exit" }).kind, "exit"); + const ft = resolveSelectOutcome(OPTS, { control: "freetext", text: "qualcos'altro" }); + assert.equal(ft.kind, "freetext"); + assert.equal(ft.text, "qualcos'altro"); +}); + +test("decisionAddArgs builds the full tht decision add argv", () => { + assert.deepEqual( + decisionAddArgs("s1", { type: "time_window", subject: "periodo", detail: "24m", rationale: "scelto dal reviewer" }), + ["decision", "add", "--session", "s1", "--type", "time_window", "--subject", "periodo", "--detail", "24m", "--rationale", "scelto dal reviewer"], + ); +}); + +test("decisionAddArgs omits optional detail/rationale when absent", () => { + assert.deepEqual( + decisionAddArgs("s1", { type: "t", subject: "sub" }), + ["decision", "add", "--session", "s1", "--type", "t", "--subject", "sub"], + ); +}); diff --git a/harness/.pi/extensions/tht-gate.js b/harness/.pi/extensions/tht-gate.js index 5c2b9b17..926d4921 100644 --- a/harness/.pi/extensions/tht-gate.js +++ b/harness/.pi/extensions/tht-gate.js @@ -215,6 +215,38 @@ export function selectedChoice(resp) { return resp?.choice; } +// Builds the `tht decision add` argv for one decision payload {type, subject, detail?, +// rationale?}. Shared by reviewer_decide (multiselect) and reviewer_select (auto-confirm). +export function decisionAddArgs(session, d) { + const args = [ + "decision", + "add", + "--session", + session, + "--type", + d.type, + "--subject", + d.subject, + ]; + if (d.detail) args.push("--detail", d.detail); + if (d.rationale) args.push("--rationale", d.rationale); + return args; +} + +// Workstream F: classifies a reviewer_select response into the action the gate takes. +// A concrete choice that carries a `decision` payload auto-confirms (persist directly, +// no second gate); a bare choice stays ask-only; back/exit/Other never persist. +export function resolveSelectOutcome(opts, resp) { + if (resp?.control === "freetext") return { kind: "freetext", text: resp.text }; + if (resp?.control === "back") return { kind: "back" }; + if (resp?.control === "exit") return { kind: "exit" }; + const choice = selectedChoice(resp); + const option = (opts || []).find((o) => o.id === choice) || null; + if (option && option.decision) + return { kind: "decision", option, decision: option.decision }; + return { kind: "choice", option, choice }; +} + export async function emitAndWait(ctx, descriptor) { for (;;) { const value = await ctx.ui.input(JSON.stringify(descriptor), ""); @@ -365,8 +397,12 @@ export default function (pi) { label: "Domanda a scelta (reviewer)", description: "Pone una domanda a scelta singola al reviewer via un widget select. Le opzioni di " + - "controllo (Altro/Torna indietro/Esci) sono sempre presenti. NON persiste: serve a " + - "chiedere, non a decidere.", + "controllo (Altro/Torna indietro/Esci) sono sempre presenti. La scelta su un'opzione " + + "concreta È la conferma: se quell'opzione porta un payload `decision` {type, subject, " + + "detail?, rationale?}, la decisione viene PERSISTITA direttamente (tht decision add) " + + "senza un secondo gate di conferma; se p.advance è vero, tenta tht phase advance --if-ready. " + + "Un'opzione SENZA `decision` resta solo-richiesta (non persiste). Altro/Torna indietro/Esci " + + "non persistono mai e tornano come testo da gestire.", parameters: Type.Object({ session: Type.String({ description: "Id sessione (per determinare la fase).", @@ -376,15 +412,24 @@ export default function (pi) { Type.Object({ id: Type.String(), label: Type.String(), + decision: Type.Optional( + Type.Object({ + type: Type.String(), + subject: Type.String(), + detail: Type.Optional(Type.String()), + rationale: Type.Optional(Type.String()), + }), + ), recommended: Type.Optional(Type.Boolean()), }), ), intro: Type.Optional(Type.String()), + advance: Type.Optional(Type.Boolean()), }), prepareArguments: prepareReviewerArguments, async execute(_id, params, _signal, _onUpdate, ctx) { lockActive = true; - const { session, title, options: opts, intro } = params; + const { session, title, options: opts, intro, advance } = params; const phase = phaseName(ctx, currentPhase(ctx, session)); const recommended = opts.find((o) => o.recommended)?.id ?? null; @@ -399,17 +444,30 @@ export default function (pi) { .map((o) => ({ id: o.id, label: o.label })), }); const resp = await emitAndWait(ctx, widget); + const outcome = resolveSelectOutcome(opts, resp); // control responses (back/exit/other) are surfaced as text for the model to act on. - if (resp.control === "freetext") - return textResult(`Altro (reviewer): ${resp.text}`); - if (resp.control === "back") + if (outcome.kind === "freetext") + return textResult(`Altro (reviewer): ${outcome.text}`); + if (outcome.kind === "back") return textResult("Il reviewer vuole tornare indietro."); - if (resp.control === "exit") + if (outcome.kind === "exit") return textResult("Il reviewer vuole uscire."); - const choice = selectedChoice(resp); - const chosen = opts.find((o) => o.id === choice); + // concrete choice carrying a decision -> auto-confirm: persist directly, no second gate. + if (outcome.kind === "decision") { + const err = relayIfThtFails( + ctx, + decisionAddArgs(session, outcome.decision), + "", + ); + if (err) return err; + if (advance) advanceIfReady(ctx, session); + return textResult( + `Decisione registrata (${outcome.decision.type}): ${outcome.option.label}.`, + ); + } + // bare choice (no decision payload) -> ask-only, non-persisting. return textResult( - `Scelta del reviewer: ${chosen ? chosen.label : choice}`, + `Scelta del reviewer: ${outcome.option ? outcome.option.label : outcome.choice}`, ); }, }); @@ -469,19 +527,7 @@ export default function (pi) { const chosen = opts.filter((o) => (resp.choices ?? []).includes(o.id)); for (const c of chosen) { const d = c.decision; - const args = [ - "decision", - "add", - "--session", - session, - "--type", - d.type, - "--subject", - d.subject, - ]; - if (d.detail) args.push("--detail", d.detail); - if (d.rationale) args.push("--rationale", d.rationale); - const err = relayIfThtFails(ctx, args, ""); + const err = relayIfThtFails(ctx, decisionAddArgs(session, d), ""); if (err) return err; toAdd.push(d); } diff --git a/harness/.pi/skills/tht-sessione/SKILL.md b/harness/.pi/skills/tht-sessione/SKILL.md index 6e873a05..805cb0a3 100644 --- a/harness/.pi/skills/tht-sessione/SKILL.md +++ b/harness/.pi/skills/tht-sessione/SKILL.md @@ -11,10 +11,11 @@ One question to the reviewer at a time; wait for their answer before proceeding; NEVER advance a phase or record a decision without explicit reviewer confirmation. The reviewer answers via the gate's **widgets** (built by `tht-gate.js`): -`reviewer_select` (single pick, no decision recorded), `reviewer_decide` -(multiselect, each selected option IS a decision — the choice is the confirmation), -`reviewer_confirm` (gate on an artifact / phase transition). Free text arrives via -the "Altro/Other" option or by prefixing `!` in chat. +`reviewer_select` (single pick; a chosen option carrying a `decision` payload IS the +confirmation and is persisted directly — an option without a payload only asks), +`reviewer_decide` (multiselect, each selected option IS a decision — the choice is the +confirmation), `reviewer_confirm` (gate on an artifact / phase transition). Free text +arrives via the "Altro/Other" option or by prefixing `!` in chat. **Language contract (from the workspace `language` field):** the table/column descriptions and the evidence you read are written in the workspace language (e.g. @@ -28,16 +29,19 @@ about a domain term, ask the reviewer. command (you invoke `tht ...` via the shell tool). NEVER run `tht phase advance` or `tht decision add` from the shell — they are blocked by the gate's anti-bypass hook; the gate extension records every decision via the `reviewer_*` tools. -2. **The choice is the confirmation.** For `reviewer_decide`, do NOT add a separate - `reviewer_confirm` after — each selected option already records its decision and - (if `advance:true`) advances. Add a `reviewer_confirm kind:"phase"` ONLY where the +2. **The choice is the confirmation.** For `reviewer_decide`, and for a + `reviewer_select` whose chosen option carries a `decision`, do NOT add a separate + confirmation gate after — the choice already records its decision and (if + `advance:true`) advances. Add a `reviewer_confirm kind:"phase"` ONLY where the phase genuinely needs a deliberate gate (F1 close, F5 close, last CTE `kind:"cte_result"`, final SQL `kind:"sql"`, F8 close) — never as a redundant - echo of a `reviewer_decide`. -3. **`reviewer_select` is for iteration only.** Use it when you propose options and - want the reviewer to pick / refine before any decision is recorded (e.g. iterating - a clarification before confirming). It records NO decision. Never use it for a - substantive decision — that's `reviewer_decide`. + echo of a recorded choice. +3. **Single pick vs multi-answer.** For a single-pick clarification or decision, use + `reviewer_select` and attach a `decision` payload (`{type, subject, detail?, + rationale?}`) to each concrete option: picking it persists that decision directly — + no follow-up `reviewer_decide`/`reviewer_confirm`. Options WITHOUT a payload only + ask (use for pure iteration before you commit). For genuinely multi-answer + decisions (several options simultaneously true) use `reviewer_decide` (multiselect). 4. **"Accept the proposal" is always an option.** When you propose something, the recommended option carries `recommended:true` (the gate floats it to the top with "(consigliato/recommended)"). "Altro/Other — specify…" is ALWAYS offered by the @@ -104,8 +108,8 @@ Prerequisite: you must already be in Phase 1. candidate interpretations (`recommended:true` on the best) + "Altro". Pick the widget by the question's shape: - **Exactly one interpretation is correct** (mutually exclusive) → `reviewer_select` - (single-pick; it only asks — then record the choice with a `reviewer_decide` - `concept_clarified`). + with a `concept_clarified` `decision` on each concrete option: the reviewer's pick + IS the confirmation and is recorded directly (no follow-up `reviewer_decide`). - **Several answers can be simultaneously true** (e.g. more than one valid population, procedure code, or time window) → do NOT use `reviewer_select`: single-pick buttons force one answer and mislead the reviewer. Use `reviewer_decide` directly (it emits a @@ -116,9 +120,9 @@ Prerequisite: you must already be in Phase 1. When a clarification is settled, move on. Pass the FULL list of clarifications, not only the latest, when you close. 3. To close Phase 1: `reviewer_confirm kind:"phase"` (the deliberate "I'm done - clarifying" gate). Do NOT add a separate `reviewer_confirm` after each individual - clarification — those advance via `reviewer_decide` (`concept_clarified`), not via - phase gates. + clarifying" gate). Do NOT add a separate confirmation after each individual + clarification — each is already recorded by its `reviewer_select`/`reviewer_decide` + (`concept_clarified`) choice, not via phase gates. 4. After the phase advance, update the question with the gate's `rewrite_question` tool (it calls `tht session set-question`, which writes `question.md` deterministically — never edit `question.md` by hand).