diff --git a/docs/superpowers/plans/2026-07-14-memory-selection-clarity.md b/docs/superpowers/plans/2026-07-14-memory-selection-clarity.md new file mode 100644 index 00000000..8309e3fb --- /dev/null +++ b/docs/superpowers/plans/2026-07-14-memory-selection-clarity.md @@ -0,0 +1,115 @@ +# Memory Selection Clarity Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make Phase 2 memory checkboxes mean only “apply this memory now”, with recommended memories preselected and unselected memories left undecided. + +**Architecture:** The Pi gate converts each `recommended:true` Phase 2 option into the existing multiselect descriptor’s `selected` flag. The shared React multiselect keeps its generic behavior, while Phase 2 supplies explicit Italian selection and confirmation copy through descriptor fields. Only selected IDs continue to create decisions; omitted IDs remain non-persisting. + +**Tech Stack:** Pi extension JavaScript (`node:test`), React 18/TypeScript, Vitest. + +## Global Constraints + +- UI copy is English except workspace/document content; this Phase 2 reviewer prompt is workspace-language Italian and stays Italian. +- No unselected memory may create `memory_rejected` or any other decision. +- Keep the generic multiselect compatible with existing widgets. + +--- + +### Task 1: Carry recommendations into checked Phase 2 options + +**Files:** +- Modify: `harness/.pi/extensions/tht-gate.js:686-750` +- Test: `harness/.pi/extensions/gate/__tests__/gate_reviewer_decide.test.js` + +**Interfaces:** +- Consumes: `reviewer_decide.options[].recommended?: boolean`. +- Produces: a multiselect descriptor whose recommended option IDs occur in `selected`, and whose Phase 2 title/action communicate application only. + +- [ ] **Step 1: Write the failing test** + + Exercise the gate with one recommended and one optional decision, capture its UI request, and assert `selected` is the recommended ID, the title asks to select memories to apply, and `confirm_label` is `Applica le memory selezionate`. + +- [ ] **Step 2: Run test to verify it fails** + + Run: `node --test harness/.pi/extensions/gate/__tests__/gate_reviewer_decide.test.js` + + Expected: FAIL because the descriptor has no `selected` recommendation mapping or memory-specific copy. + +- [ ] **Step 3: Write minimal implementation** + + In `reviewer_decide`, map each recommended option to `selected: true` before calling `buildMultiselectRequest`; add optional `selection_label` and `confirm_label` fields to the descriptor only for the F2 memory title. + +- [ ] **Step 4: Run the gate tests** + + Run: `node --test harness/.pi/extensions/gate/__tests__/*.test.js` + + Expected: PASS. + +### Task 2: Render Phase 2 action copy without changing generic multiselect behavior + +**Files:** +- Modify: `frontend/src/api/types.ts` +- Modify: `frontend/src/widgets/MultiselectWidget.tsx` +- Test: `frontend/src/widgets/MultiselectWidget.test.tsx` + +**Interfaces:** +- Consumes: optional `WidgetDescriptor.selection_label?: string` and `WidgetDescriptor.confirm_label?: string`. +- Produces: supplied labels in the count and confirm button; absent labels retain `selected` and `Confirm`. + +- [ ] **Step 1: Write the failing test** + + Render a descriptor with one checked recommended memory and the custom labels. Assert the checkbox is checked, the count reads `1 / 2 memory da applicare`, and confirmation uses `Applica le memory selezionate` while emitting only checked IDs. + +- [ ] **Step 2: Run test to verify it fails** + + Run: `cd frontend && npx vitest run src/widgets/MultiselectWidget.test.tsx` + + Expected: FAIL because the descriptor type and component ignore the custom labels. + +- [ ] **Step 3: Write minimal implementation** + + Add the two optional descriptor fields and use them with the existing generic strings as fallbacks. Do not alter checkbox response semantics. + +- [ ] **Step 4: Run widget test and typecheck** + + Run: `cd frontend && npx vitest run src/widgets/MultiselectWidget.test.tsx && npx tsc -b` + + Expected: PASS. + +### Task 3: Align the Phase 2 model instructions with implementation + +**Files:** +- Modify: `harness/.pi/skills/tht-sessione/memoria.md` +- Modify: `harness/.pi/skills/tht-sessione/SKILL.md` + +**Interfaces:** +- Consumes: selected options are applied; omitted options are not applied now. +- Produces: instructions that never ask the model to create a negative “do not use” option or a persistent rejection for unchecked memories. + +- [ ] **Step 1: Update the memory checklist contract** + + State that every row describes a candidate memory declaratively, `recommended:true` preselects only memories proposed for use, and unchecked candidates are not applied now and may be considered again after a reopen. + +- [ ] **Step 2: Verify no obsolete rejection instruction remains** + + Run: `rg -n "deselected.*memory_rejected|deselezionate.*memory_rejected|mem_id" harness/.pi/skills/tht-sessione` + + Expected: no matches. + +### Task 4: Full verification + +**Files:** +- Verify only. + +- [ ] **Step 1: Run focused gate and frontend checks** + + Run: `node --test harness/.pi/extensions/gate/__tests__/*.test.js && cd frontend && npx vitest run src/widgets/MultiselectWidget.test.tsx && npx tsc -b` + + Expected: all commands exit 0. + +- [ ] **Step 2: Review the diff** + + Run: `git diff --check && git diff -- harness/.pi/extensions/tht-gate.js frontend/src/api/types.ts frontend/src/widgets/MultiselectWidget.tsx harness/.pi/skills/tht-sessione` + + Expected: no whitespace errors; changes only implement the clarified memory-selection contract. diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index 3ca60f46..27ec7eea 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -36,6 +36,8 @@ export interface WidgetDescriptor { options?: WidgetOption[]; reserved?: string[]; allow_empty?: boolean; + selection_label?: string; + confirm_label?: string; artifact?: { kind: string; content?: string; [k: string]: unknown }; level?: "info" | "warning" | "error"; text?: string; diff --git a/frontend/src/widgets/MultiselectWidget.test.tsx b/frontend/src/widgets/MultiselectWidget.test.tsx index ce87d65c..925e1659 100644 --- a/frontend/src/widgets/MultiselectWidget.test.tsx +++ b/frontend/src/widgets/MultiselectWidget.test.tsx @@ -22,6 +22,30 @@ test("confirms the checked ids including initially selected", async () => { expect(onRespond).toHaveBeenCalledWith({ id: "u1", kind: "multiselect", choices: ["t1", "t2"] }); }); +test("uses memory-specific selection copy without changing selected ids", async () => { + const onRespond = vi.fn(); + render( + + ); + + expect(screen.getByRole("checkbox", { name: /memory raccomandata/i })).toBeChecked(); + expect(screen.getByText("1 / 2 memory da applicare")).toBeInTheDocument(); + await userEvent.click(screen.getByRole("button", { name: "Applica le memory selezionate" })); + expect(onRespond).toHaveBeenCalledWith({ id: "u-memory", kind: "multiselect", choices: ["recommended"] }); +}); + test("select-all checks all options", async () => { const onRespond = vi.fn(); render( diff --git a/frontend/src/widgets/MultiselectWidget.tsx b/frontend/src/widgets/MultiselectWidget.tsx index 89537db5..c6594690 100644 --- a/frontend/src/widgets/MultiselectWidget.tsx +++ b/frontend/src/widgets/MultiselectWidget.tsx @@ -39,7 +39,7 @@ export function MultiselectWidget({ descriptor, onRespond }: WidgetProps) { {allChecked ? "Deselect all" : "Select all"} - {checked.size} / {options.length} selected + {checked.size} / {options.length} {descriptor.selection_label ?? "selected"}
@@ -67,7 +67,7 @@ export function MultiselectWidget({ descriptor, onRespond }: WidgetProps) { disabled={isDisabled} onClick={() => onRespond({ id: descriptor.id, kind: "multiselect", choices: Array.from(checked) })} > - Confirm + {descriptor.confirm_label ?? "Confirm"}
{ + assert.deepEqual( + memorySelectionWidgetProps([ + { id: "recommended", label: "Memory A", recommended: true }, + { id: "optional", label: "Memory B" }, + ]), + { + title: "Seleziona le memory da applicare alla domanda", + selected: ["recommended"], + selectionLabel: "memory da applicare", + confirmLabel: "Applica le memory selezionate", + }, + ); +}); diff --git a/harness/.pi/extensions/gate/__tests__/gate_provided_session.test.js b/harness/.pi/extensions/gate/__tests__/gate_provided_session.test.js index 9c0d498e..0d96f346 100644 --- a/harness/.pi/extensions/gate/__tests__/gate_provided_session.test.js +++ b/harness/.pi/extensions/gate/__tests__/gate_provided_session.test.js @@ -7,6 +7,13 @@ const { createFakePi } = require("./fake_pi_runtime.js"); const installGate = require("../../tht-gate.js").default ?? require("../../tht-gate.js"); test("con THT_SESSION il kickoff usa l'id fornito e NON crea la sessione", async () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "tht-gate-no-pack-")); + const bin = path.join(tmp, "bin"); + fs.mkdirSync(bin); + const tht = path.join(bin, "tht"); + fs.writeFileSync(tht, "#!/bin/sh\nexit 1\n", { mode: 0o755 }); + const oldPath = process.env.PATH; + process.env.PATH = bin + ":" + oldPath; process.env.THT_SESSION = "2026-06-27-100000-test"; try { const { pi, ctx } = createFakePi(); @@ -19,13 +26,15 @@ test("con THT_SESSION il kickoff usa l'id fornito e NON crea la sessione", async assert.doesNotMatch(text, /tht session new/); assert.match(text, /retrieval pack non era ancora disponibile/i); assert.match(text, /tht search pack/); - assert.doesNotMatch(text, //); + assert.doesNotMatch(text, /\s*# Retrieval pack/); assert.match(text, //); assert.match(text, /# Thoth session workflow \(phases 1-8\)/); assert.match(text, /non esplorare il repository/i); assert.doesNotMatch(text, /Carica la skill leggendo/); } finally { + process.env.PATH = oldPath; delete process.env.THT_SESSION; + fs.rmSync(tmp, { recursive: true, force: true }); } }); diff --git a/harness/.pi/extensions/gate/builders.js b/harness/.pi/extensions/gate/builders.js index b723332d..8ea3087f 100644 --- a/harness/.pi/extensions/gate/builders.js +++ b/harness/.pi/extensions/gate/builders.js @@ -61,6 +61,7 @@ function buildSelectRequest({ // `allowEmpty:false` with zero options is a broken widget and throws. function buildMultiselectRequest({ id, phase, title, options, content = null, selected = [], allowEmpty = false, + selectionLabel, confirmLabel, }) { requireString(title, "title", "multiselect"); const opts = requireArray(options, "options", "multiselect"); @@ -77,10 +78,12 @@ function buildMultiselectRequest({ widget: "multiselect", title, allow_empty: allowEmpty, - options: [...opts], + options: opts.map((option) => ({ ...option, selected: selected.includes(option.id) })), selected: [...selected], content, reserved: RESERVED, + ...(selectionLabel ? { selection_label: selectionLabel } : {}), + ...(confirmLabel ? { confirm_label: confirmLabel } : {}), }; } diff --git a/harness/.pi/extensions/tht-gate.js b/harness/.pi/extensions/tht-gate.js index 73b9caed..375074d5 100644 --- a/harness/.pi/extensions/tht-gate.js +++ b/harness/.pi/extensions/tht-gate.js @@ -386,6 +386,17 @@ export function shouldSkipEmptyDecide({ meritCount, allowEmpty, advance }) { return meritCount === 0 && !!allowEmpty && !!advance; } +// Phase 2 uses a single positive checkbox meaning: apply this memory now. +// Candidates not checked by the reviewer remain undecided and may be shown again. +export function memorySelectionWidgetProps(options) { + return { + title: "Seleziona le memory da applicare alla domanda", + selected: options.filter((option) => option.recommended).map((option) => option.id), + selectionLabel: "memory da applicare", + confirmLabel: "Applica le memory selezionate", + }; +} + // --- F8 memory-promotion gate: pure candidate->widget mapping (L1-tested) ------ // The candidates come from `tht memory promote --preview --json` (deterministic, // reviewer-approved decisions only); the model never authors them. @@ -730,10 +741,10 @@ export default function (pi) { const widget = buildMultiselectRequest({ id: `u${Date.now()}`, phase, - title, + title: phase === "F2" ? memorySelectionWidgetProps(opts).title : title, allowEmpty: params.allow_empty ?? false, options: meritOptions, - recommended: opts.find((o) => o.recommended)?.id ?? null, + ...(phase === "F2" ? memorySelectionWidgetProps(opts) : {}), }); const resp = await emitAndWait(ctx, widget); if (resp.control === "freetext") { diff --git a/harness/.pi/skills/tht-sessione/SKILL.md b/harness/.pi/skills/tht-sessione/SKILL.md index 7d2f6a32..0b31c750 100644 --- a/harness/.pi/skills/tht-sessione/SKILL.md +++ b/harness/.pi/skills/tht-sessione/SKILL.md @@ -215,21 +215,21 @@ Prerequisite: Phase 1 closed. allow_empty:true)`. Rules: at most **5** candidates; ONLY the 3 reusable types (`concept_clarified`, `table_promoted`, `table_excluded`) — query-specific decisions (`question_rewritten`, `sql_approved`, …) are NOT transferable, never - propose them. Each option carries `mem_id:"mem-"` plus `type`/`subject`/ - `rationale`. Selected options are applied (register the decision citing the - `mem_id` in the rationale); deselected ones are recorded as `memory_rejected` by - the gate (so the next `tht memory search --session` won't re-propose them). The - checklist starts pre-selected with the recommended memories. With `allow_empty:true` - an empty selection is accepted (no memory applied; deselected still recorded) and - the phase advances — no separate gate. + propose them. Each option carries `type`/`subject`/`rationale`; cite the source + memory id (`mem-`) in its rationale when applying it. Every option describes a + candidate memory; never create an opposite "do not use" option. Only + `recommended:true` options start checked. A + deselected candidate is **not applied now**, not rejected, and may be considered + again if Phase 2 is reopened. With `allow_empty:true` an empty selection is accepted + (no memory applied) and the phase advances — no separate gate. When the memory search returned **zero** candidates, still issue the single `reviewer_decide(multi:true, advance:true, allow_empty:true)` with an empty merito list: the gate detects the empty+advance case, shows the reviewer an info notice ("Nessuna memory riutilizzabile … passo alla fase successiva") and auto-advances F2 — it does NOT present an empty checklist, and you do NOT add a separate `reviewer_confirm kind:"phase"`. -4. Closing: if memories were applied or rejected (substantive decisions), `advance:true` - no-ops — close with `reviewer_confirm kind:"phase"`. Only a truly empty memory phase - (nothing applied, nothing rejected) auto-advances via `advance:true`. +4. Closing: if one or more memories were applied (substantive decisions), `advance:true` + no-ops — close with `reviewer_confirm kind:"phase"`. If none is applied, F2 + auto-advances via `advance:true`. 5. Memories are promoted at the END of the workflow (Phase 8, the `reviewer_memory_promote` gate) — never promote from here, never run `tht memory promote`/`save-one` yourself (the gate blocks them). diff --git a/harness/.pi/skills/tht-sessione/memoria.md b/harness/.pi/skills/tht-sessione/memoria.md index d47ece28..f386af09 100644 --- a/harness/.pi/skills/tht-sessione/memoria.md +++ b/harness/.pi/skills/tht-sessione/memoria.md @@ -24,14 +24,13 @@ Rules: (e.g. `question_rewritten`, `sql_approved`) are NOT to be proposed: they don't transfer to other questions. - All candidate memories go in **a single** `reviewer_decide(multi:true, - advance:true, allow_empty:true)`: each selected option is applied (register the - decision with the appropriate type, citing the memory id in the rationale), each - deselected option is **recorded as `memory_rejected` by the gate** (so the next - `tht memory search --session` won't re-propose it). To enable this, EVERY memory - option MUST carry the field `mem_id:"mem-"` (besides `type`/`subject`/ - `rationale`). The checklist starts pre-selected with the recommended memories. - With `allow_empty:true` an **empty selection is accepted** (no memory applied; the - deselected ones are still recorded as rejected) and the phase advances — no + advance:true, allow_empty:true)`: every option describes a candidate memory, never + an opposite action such as "do not use it". Each selected option is applied + (register the decision with the appropriate type, citing the memory id in the + rationale). Mark `recommended:true` ONLY on memories proposed for use: those, and + only those, start checked. An unchecked memory is **not applied now**; it is not a + rejection and may be considered again if Phase 2 is reopened. With `allow_empty:true` + an **empty selection is accepted** (no memory applied) and the phase advances — no separate gate. - For "inspect": show the memory's full JSON record in the prose before presenting the checklist, if the reviewer asks.