diff --git a/deploy/compose.psd-local.yaml.example b/deploy/compose.psd-local.yaml.example index 52d6a932..2a52b893 100644 --- a/deploy/compose.psd-local.yaml.example +++ b/deploy/compose.psd-local.yaml.example @@ -32,6 +32,10 @@ services: - ${THT_PSD_WORKSPACE_HOST_PATH:?set THT_PSD_WORKSPACE_HOST_PATH}:/data/workspaces/psd frontend: + build: + args: + VITE_BASE: / + VITE_BACKEND_URL: /api ports: - "127.0.0.1:8099:8080" networks: !override diff --git a/frontend/src/widgets/MultiselectWidget.test.tsx b/frontend/src/widgets/MultiselectWidget.test.tsx index 925e1659..dea75230 100644 --- a/frontend/src/widgets/MultiselectWidget.test.tsx +++ b/frontend/src/widgets/MultiselectWidget.test.tsx @@ -46,6 +46,56 @@ test("uses memory-specific selection copy without changing selected ids", async expect(onRespond).toHaveBeenCalledWith({ id: "u-memory", kind: "multiselect", choices: ["recommended"] }); }); +test("shows the exact content and provenance for every memory option", () => { + render( + + ); + + expect(screen.getByText("flag_attivo = TRUE")).toBeInTheDocument(); + expect(screen.getByText(/Confermato dal reviewer/)).toBeInTheDocument(); + expect(screen.getByText(/Conta i pazienti attivi/)).toBeInTheDocument(); +}); + +test("renders memory detail as human-friendly Markdown", () => { + render( + + ); + + expect(screen.getByText("Definizione").tagName).toBe("STRONG"); + expect(screen.getByText("flag_attivo = TRUE").tagName).toBe("CODE"); + expect(screen.getByText("record non annullato").closest("li")).not.toBeNull(); + expect(screen.queryByText("**Definizione**")).not.toBeInTheDocument(); +}); + 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 c6594690..f38991da 100644 --- a/frontend/src/widgets/MultiselectWidget.tsx +++ b/frontend/src/widgets/MultiselectWidget.tsx @@ -1,4 +1,6 @@ import { useState } from "react"; +import ReactMarkdown from "react-markdown"; +import remarkGfm from "remark-gfm"; import type { WidgetProps } from "./types"; import { ReservedControls } from "./ReservedControls"; @@ -56,7 +58,24 @@ export function MultiselectWidget({ descriptor, onRespond }: WidgetProps) { checked={checked.has(o.id)} onChange={() => toggle(o.id)} /> - {o.label} + + {o.label} + {o.detail && ( +
+ {o.detail} +
+ )} + {o.rationale && ( + + Motivo: {o.rationale} + + )} + {typeof o.meta?.question_context === "string" && o.meta.question_context && ( + + Domanda di contesto: {o.meta.question_context} + + )} +
))} diff --git a/frontend/src/widgets/SchemaLinkingGateWidget.test.tsx b/frontend/src/widgets/SchemaLinkingGateWidget.test.tsx index 372bd734..4650d586 100644 --- a/frontend/src/widgets/SchemaLinkingGateWidget.test.tsx +++ b/frontend/src/widgets/SchemaLinkingGateWidget.test.tsx @@ -33,6 +33,13 @@ const descriptor: WidgetDescriptor = { reserved: ["back", "exit", "other"], }; +test("starts include rows checked and exclude rows unchecked even for legacy payloads", () => { + render(); + + expect(screen.getByRole("checkbox", { name: "dim_patient" })).toBeChecked(); + expect(screen.getByRole("checkbox", { name: "fact_sostituzione" })).not.toBeChecked(); +}); + test("Confirm emits enacted tables with suggested columns (catalog order)", async () => { const onRespond = vi.fn(); render(); @@ -42,7 +49,7 @@ test("Confirm emits enacted tables with suggested columns (catalog order)", asyn kind: "schema-linking", tables: [ { id: "t-pat", enacted: true, columns: ["cod_paz"] }, - { id: "x-sub", enacted: true }, + { id: "x-sub", enacted: false }, ], }); }); @@ -60,7 +67,7 @@ test("selecting a column in the modal adds it to the response", async () => { kind: "schema-linking", tables: [ { id: "t-pat", enacted: true, columns: ["cod_paz", "nome"] }, - { id: "x-sub", enacted: true }, + { id: "x-sub", enacted: false }, ], }); }); @@ -75,7 +82,7 @@ test("declining a table's enact checkbox drops its columns", async () => { kind: "schema-linking", tables: [ { id: "t-pat", enacted: false }, - { id: "x-sub", enacted: true }, + { id: "x-sub", enacted: false }, ], }); }); diff --git a/frontend/src/widgets/SchemaLinkingGateWidget.tsx b/frontend/src/widgets/SchemaLinkingGateWidget.tsx index 2c680bd7..353facfd 100644 --- a/frontend/src/widgets/SchemaLinkingGateWidget.tsx +++ b/frontend/src/widgets/SchemaLinkingGateWidget.tsx @@ -7,7 +7,7 @@ import { ReservedControls } from "./ReservedControls"; export function SchemaLinkingGateWidget({ descriptor, onRespond }: WidgetProps) { const tables: SchemaTable[] = descriptor.tables ?? []; const [enacted, setEnacted] = useState>( - () => new Set(tables.filter((t) => t.recommended).map((t) => t.id)) + () => new Set(tables.filter((t) => t.kind === "promote" && t.recommended).map((t) => t.id)) ); const [selected, setSelected] = useState>>(() => { const m = new Map>(); diff --git a/harness/.pi/extensions/gate/__tests__/gate_datamart_choice.test.js b/harness/.pi/extensions/gate/__tests__/gate_datamart_choice.test.js new file mode 100644 index 00000000..a7b1b88c --- /dev/null +++ b/harness/.pi/extensions/gate/__tests__/gate_datamart_choice.test.js @@ -0,0 +1,104 @@ +const test = require("node:test"); +const assert = require("node:assert"); +const cp = require("node:child_process"); +const { createRequire } = require("node:module"); +const path = require("node:path"); + +const GATE = path.join(__dirname, "..", "..", "tht-gate.js"); +if (typeof globalThis.require === "undefined") { + globalThis.require = createRequire(GATE); +} + +let activeStub = cp.execFileSync; +const dispatcher = (...args) => activeStub(...args); +Object.defineProperty(cp, "execFileSync", { + configurable: true, + get: () => dispatcher, + set: (fn) => { activeStub = fn; }, +}); + +async function loadGate() { + const gate = require(GATE); + const { createFakePi } = require("./fake_pi_runtime.js"); + const { pi, ctx, tools } = createFakePi(); + ctx.cwd = "/nonexistent-thothii-test-cwd"; + gate.default(pi); + await pi.emit("session_start", {}); + return { ctx, tools }; +} + +function phase8Stub(calls) { + return (_file, args) => { + calls.push(args); + if (args[0] === "phase" && args[1] === "meta") { + return JSON.stringify({ + max_phase: 8, + phases: [{ + num: 8, + id: "F8", + name: "datamart", + emits: ["datamart_requested", "datamart_declined"], + }], + }); + } + if (args[0] === "phase" && args[1] === "show") return "Fase corrente: 8\n"; + return ""; + }; +} + +test("reviewer_datamart skips the question and records decline on workstation", async () => { + const previousProfile = process.env.THT_PROFILE; + const originalExec = cp.execFileSync; + const calls = []; + process.env.THT_PROFILE = "workstation"; + cp.execFileSync = phase8Stub(calls); + try { + const { ctx, tools } = await loadGate(); + const tool = tools.get("reviewer_datamart"); + assert.ok(tool, "reviewer_datamart must be registered"); + + const result = await tool.def.execute("call-1", { session: "s1" }, null, null, ctx); + + assert.equal(ctx.uiCalls.length, 0, "workstation must not show a pointless question"); + assert.ok(calls.some((args) => + args.join(" ").includes("decision add --session s1 --type datamart_declined --subject phase:8") + )); + assert.match(result.content[0].text, /workstation.*saltato automaticamente/i); + } finally { + cp.execFileSync = originalExec; + if (previousProfile === undefined) delete process.env.THT_PROFILE; + else process.env.THT_PROFILE = previousProfile; + } +}); + +test("reviewer_datamart shows both yes and no choices on server", async () => { + const previousProfile = process.env.THT_PROFILE; + const originalExec = cp.execFileSync; + const calls = []; + process.env.THT_PROFILE = "server"; + cp.execFileSync = phase8Stub(calls); + try { + const { ctx, tools } = await loadGate(); + ctx.ui.input = async (title) => { + const descriptor = JSON.parse(title); + assert.deepEqual( + descriptor.options.slice(0, 2).map((option) => option.label), + ["Sì, genera il datamart", "No, salta il datamart"], + ); + return JSON.stringify({ id: descriptor.id, choices: ["generate"] }); + }; + + const result = await tools.get("reviewer_datamart").def.execute( + "call-1", { session: "s1" }, null, null, ctx, + ); + + assert.ok(calls.some((args) => + args.join(" ").includes("decision add --session s1 --type datamart_requested --subject phase:8") + )); + assert.match(result.content[0].text, /datamart_requested/); + } finally { + cp.execFileSync = originalExec; + if (previousProfile === undefined) delete process.env.THT_PROFILE; + else process.env.THT_PROFILE = previousProfile; + } +}); diff --git a/harness/.pi/extensions/gate/__tests__/gate_memory_promote.test.js b/harness/.pi/extensions/gate/__tests__/gate_memory_promote.test.js index 6f40809d..4add0105 100644 --- a/harness/.pi/extensions/gate/__tests__/gate_memory_promote.test.js +++ b/harness/.pi/extensions/gate/__tests__/gate_memory_promote.test.js @@ -1,6 +1,7 @@ const test = require("node:test"); const assert = require("node:assert"); const { + dedupePromotionCandidates, promotionOptions, promotionContent, splitPromotionChoices, @@ -20,26 +21,47 @@ const CANDIDATES = [ ]; test("promotionOptions maps candidates to seq-keyed options", () => { - assert.deepEqual(promotionOptions(CANDIDATES), [ - { id: "seq-3", label: "table_promoted: fact_seeablazione" }, - { id: "seq-5", label: "concept_clarified: paziente attivo" }, + assert.deepEqual(promotionOptions(dedupePromotionCandidates(CANDIDATES)), [ + { + id: "seq-5", + label: "concept_clarified: paziente attivo", + detail: "flag_attivo = TRUE", + rationale: "", + meta: { question_context: "quante ablazioni nel 2023" }, + }, ]); }); -test("promotionContent lists every candidate with detail and context", () => { - const c = promotionContent(CANDIDATES); - assert.ok(c.includes("fact_seeablazione")); +test("F8 proposes semantically identical memory content only once", () => { + const duplicate = { ...CANDIDATES[0], decision_seq: 9 }; + assert.deepEqual( + dedupePromotionCandidates([CANDIDATES[0], duplicate, CANDIDATES[1]]) + .map((candidate) => candidate.decision_seq), + [5], + ); +}); + +test("F8 never proposes promoted tables as memory", () => { + const candidates = dedupePromotionCandidates(CANDIDATES); + const options = promotionOptions(candidates); + const c = promotionContent(candidates); + + assert.deepEqual(candidates.map((candidate) => candidate.type), ["concept_clarified"]); + assert.deepEqual(options.map((option) => option.id), ["seq-5"]); + assert.ok(!c.includes("fact_seeablazione")); assert.ok(c.includes("flag_attivo = TRUE")); assert.ok(c.includes("quante ablazioni nel 2023")); }); test("splitPromotionChoices partitions by selection", () => { - const { promote, decline } = splitPromotionChoices(CANDIDATES, ["seq-5"]); + const candidates = dedupePromotionCandidates(CANDIDATES); + const { promote, decline } = splitPromotionChoices(candidates, ["seq-5"]); assert.deepEqual(promote.map((c) => c.decision_seq), [5]); - assert.deepEqual(decline.map((c) => c.decision_seq), [3]); + assert.deepEqual(decline.map((c) => c.decision_seq), []); }); test("empty or missing choices declines everything", () => { - assert.equal(splitPromotionChoices(CANDIDATES, []).decline.length, 2); - assert.equal(splitPromotionChoices(CANDIDATES, undefined).decline.length, 2); + const candidates = dedupePromotionCandidates(CANDIDATES); + assert.equal(splitPromotionChoices(candidates, []).decline.length, 1); + assert.equal(splitPromotionChoices(candidates, undefined).decline.length, 1); }); diff --git a/harness/.pi/extensions/gate/__tests__/gate_memory_selection.test.js b/harness/.pi/extensions/gate/__tests__/gate_memory_selection.test.js index 7f18ab86..ce92041f 100644 --- a/harness/.pi/extensions/gate/__tests__/gate_memory_selection.test.js +++ b/harness/.pi/extensions/gate/__tests__/gate_memory_selection.test.js @@ -1,6 +1,20 @@ const test = require("node:test"); const assert = require("node:assert"); -const { memorySelectionWidgetProps } = require("../../tht-gate.js"); +const path = require("node:path"); +const { + memorySelectionWidgetProps, + normalizeMemoryOptions, +} = require("../../tht-gate.js"); + +test("reviewer_decide accepts the exact memory content in option.description", () => { + const { createFakePi } = require("./fake_pi_runtime.js"); + const { pi, tools } = createFakePi(); + require(path.join(__dirname, "..", "..", "tht-gate.js")).default(pi); + + const optionProperties = tools.get("reviewer_decide") + .def.parameters.properties.options.items.properties; + assert.ok(optionProperties.description); +}); test("F2 preseleziona solo le memory raccomandate e parla di applicazione", () => { assert.deepEqual( @@ -16,3 +30,65 @@ test("F2 preseleziona solo le memory raccomandate e parla di applicazione", () = }, ); }); + +test("F2 preserves the exact memory content and proposes each memory id once", () => { + const options = normalizeMemoryOptions([ + { + id: "first", + label: "Regola paziente attivo", + description: "Decisione concept_clarified: paziente attivo\nflag_attivo = TRUE", + decision: { + type: "concept_clarified", + subject: "paziente attivo", + detail: "flag_attivo = TRUE", + rationale: "Riusa mem-0042 per la stessa definizione", + }, + recommended: true, + }, + { + id: "duplicate", + label: "La stessa regola", + description: "Decisione concept_clarified: paziente attivo\nflag_attivo = TRUE", + decision: { + type: "concept_clarified", + subject: "paziente attivo", + detail: "flag_attivo = TRUE", + rationale: "Memory mem-0042", + }, + }, + ]); + + assert.equal(options.length, 1); + assert.equal( + options[0].detail, + "Decisione concept_clarified: paziente attivo\nflag_attivo = TRUE", + ); + assert.equal(options[0].recommended, true); +}); + +test("F2 accepts only concept_clarified memory options", () => { + const options = normalizeMemoryOptions([ + { + id: "mem-0042", + label: "Concetto paziente attivo", + description: "Definizione riusabile", + decision: { + type: "concept_clarified", + subject: "paziente attivo", + detail: "flag_attivo = TRUE", + }, + }, + { + id: "mem-0043", + label: "Tabella promossa", + description: "fact_pazienti", + decision: { + type: "table_promoted", + subject: "fact_pazienti", + detail: "tabella principale", + }, + }, + ]); + + assert.deepEqual(options.map((option) => option.id), ["mem-0042"]); +}); diff --git a/harness/.pi/extensions/gate/__tests__/gate_schema_linking.test.js b/harness/.pi/extensions/gate/__tests__/gate_schema_linking.test.js index 257d9a1e..cd247afc 100644 --- a/harness/.pi/extensions/gate/__tests__/gate_schema_linking.test.js +++ b/harness/.pi/extensions/gate/__tests__/gate_schema_linking.test.js @@ -132,6 +132,64 @@ test("reviewer_schema_linking records table/column decisions and syncs schema_li } }); +test("reviewer_schema_linking preselects promoted tables but not excluded tables", async () => { + _handler = (_file, args) => { + if (args[0] === "phase" && args[1] === "meta") + return JSON.stringify({ phases: [{ num: 4, id: "F4" }] }); + if (args[0] === "phase" && args[1] === "show") return "Fase corrente: 4\n"; + if (args[0] === "schema" && args[1] === "columns") + return JSON.stringify({ ...CATALOG, table: args[2] }); + return ""; + }; + + try { + const gate = require(GATE); + const { createFakePi } = require("./fake_pi_runtime.js"); + const { pi, ctx, tools } = createFakePi(); + ctx.cwd = "/nonexistent-thothii-table-defaults"; + gate.default(pi); + + let capturedDescriptor; + ctx.ui.input = async (title) => { + capturedDescriptor = JSON.parse(title); + return JSON.stringify({ + id: capturedDescriptor.id, + kind: "schema-linking", + tables: capturedDescriptor.tables.map((table) => ({ + id: table.id, + enacted: table.recommended, + columns: [], + })), + }); + }; + + await tools.get("reviewer_schema_linking").def.execute( + "call-defaults", + { + session: "s1", + title: "Schema linking", + tables: [ + { id: "keep", name: "fact_keep", kind: "promote" }, + { id: "drop", name: "fact_drop", kind: "exclude", recommended: true }, + ], + }, + null, + null, + ctx, + ); + + assert.deepEqual( + capturedDescriptor.tables.map(({ id, recommended }) => ({ id, recommended })), + [ + { id: "keep", recommended: true }, + { id: "drop", recommended: false }, + ], + ); + } finally { + _handler = null; + } +}); + test("reviewer_schema_linking auto-corrects typos in table names via fuzzy matching", async () => { const calls = []; const inputs = []; diff --git a/harness/.pi/extensions/tht-gate.js b/harness/.pi/extensions/tht-gate.js index 9a5dcb1b..6b3be7f1 100644 --- a/harness/.pi/extensions/tht-gate.js +++ b/harness/.pi/extensions/tht-gate.js @@ -573,13 +573,85 @@ export function memorySelectionWidgetProps(options) { }; } +const MEMORY_ID_RE = /\bmem-\d{4,}\b/i; + +function normalizedText(value) { + return String(value ?? "").trim().replace(/\s+/g, " ").toLowerCase(); +} + +function memoryOptionKey(option) { + const searchable = [ + option.id, + option.label, + option.description, + option.decision?.rationale, + ].join(" "); + const memoryId = searchable.match(MEMORY_ID_RE)?.[0]?.toLowerCase(); + if (memoryId) return `id:${memoryId}`; + return [ + "content", + normalizedText(option.decision?.type), + normalizedText(option.decision?.subject), + normalizedText(option.decision?.detail), + ].join(":"); +} + +// F2 is model-orchestrated, so normalize at the trust boundary: retain the exact +// retrieved memory text for the reviewer and collapse repeated representations of +// the same memory id/content before rendering or persisting a choice. +export function normalizeMemoryOptions(options) { + const seen = new Set(); + const out = []; + for (const option of options) { + if (option.decision?.type !== "concept_clarified") continue; + const key = memoryOptionKey(option); + if (seen.has(key)) continue; + seen.add(key); + const memoryId = [option.id, option.label, option.description, option.decision?.rationale] + .join(" ").match(MEMORY_ID_RE)?.[0]?.toLowerCase(); + const fallbackDetail = [ + option.decision?.type && option.decision?.subject + ? `Decisione ${option.decision.type}: ${option.decision.subject}` + : "", + option.decision?.detail ?? "", + ].filter(Boolean).join("\n"); + out.push({ + ...option, + detail: option.description?.trim() || fallbackDetail, + rationale: option.decision?.rationale ?? "", + ...(memoryId ? { meta: { memory_id: memoryId } } : {}), + }); + } + return out; +} + // --- 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. +export function dedupePromotionCandidates(candidates) { + const seen = new Set(); + return candidates.filter((candidate) => { + if (candidate.type !== "concept_clarified") return false; + const key = [ + candidate.type, + candidate.subject, + candidate.detail, + candidate.rationale, + candidate.question_context, + ].map(normalizedText).join("\u0000"); + if (seen.has(key)) return false; + seen.add(key); + return true; + }); +} + export function promotionOptions(candidates) { return candidates.map((c) => ({ id: `seq-${c.decision_seq}`, label: `${c.type}: ${c.subject}`, + detail: c.detail || "", + rationale: c.rationale || "", + meta: { question_context: c.question_context || "" }, })); } @@ -795,7 +867,7 @@ export default function (pi) { } }); - // --- the four reviewer tools (widget-descriptor emit + await) --------------- + // --- reviewer tools (widget-descriptor emit + await) ------------------------- pi.registerTool({ name: "reviewer_select", @@ -881,6 +953,87 @@ export default function (pi) { }, }); + pi.registerTool({ + name: "reviewer_datamart", + label: "Scelta datamart (reviewer)", + description: + "F8: gestisce deterministicamente la scelta di generazione del datamart. " + + "Con THT_PROFILE=workstation non mostra alcun widget e registra automaticamente " + + "datamart_declined; con profile=server mostra sempre entrambe le opzioni Si/No e " + + "registra la decisione scelta. Dopo questo tool prosegui con reviewer_memory_promote.", + parameters: Type.Object({ + session: Type.String(), + }), + async execute(_id, params, _signal, _onUpdate, ctx) { + lockActive = true; + try { + const { session } = params; + const curNum = currentPhase(ctx, session); + if (curNum !== 8) { + return textResult( + `La scelta datamart e' disponibile solo in Fase 8 (fase corrente: ${curNum}).`, + ); + } + const subject = "phase:8"; + if (process.env.THT_PROFILE === "workstation") { + const decision = { type: "datamart_declined", subject }; + const err = relayIfThtFails(ctx, decisionAddArgs(session, decision), ""); + if (err) return err; + return textResult( + "Profilo workstation: datamart saltato automaticamente " + + "e decisione datamart_declined registrata. Invoca ora reviewer_memory_promote.", + ); + } + + const options = [ + { + id: "generate", + label: "Sì, genera il datamart", + decision: { type: "datamart_requested", subject }, + }, + { + id: "skip", + label: "No, salta il datamart", + decision: { type: "datamart_declined", subject }, + recommended: true, + }, + ]; + const widget = buildSelectRequest({ + id: `u${Date.now()}`, + phase: phaseId(ctx, curNum), + title: "Vuoi generare un datamart?", + intro: null, + recommended: "skip", + options: options.map((option) => ({ id: option.id, label: option.label })), + }); + const resp = await emitAndWait(ctx, widget); + const outcome = resolveSelectOutcome(options, resp); + if (outcome.kind === "freetext") + return textResult(`Altro (reviewer): ${outcome.text}`); + if (outcome.kind === "back") + return textResult("Il reviewer vuole tornare indietro."); + if (outcome.kind === "exit") + return textResult("Il reviewer vuole uscire."); + if (outcome.kind !== "decision") + return textResult("Scelta datamart non valida: ripresenta reviewer_datamart."); + const err = relayIfThtFails( + ctx, + decisionAddArgs(session, outcome.decision), + "", + ); + if (err) return err; + return textResult( + decisionRecordedResultText(outcome.decision, outcome.option), + ); + } catch (fatal) { + const msg = (fatal.stderr || fatal.message || String(fatal)).toString().trim(); + return textResult( + `[reviewer_datamart ERRORE INTERNO] ${msg}. Riprova o usa un approccio diverso.`, + ); + } + }, + }); + pi.registerTool({ name: "reviewer_decide", label: "Decisione di merito (reviewer)", @@ -895,6 +1048,7 @@ export default function (pi) { Type.Object({ id: Type.String(), label: Type.String(), + description: Type.Optional(Type.String()), decision: Type.Object({ type: Type.String(), subject: Type.String(), @@ -911,14 +1065,23 @@ export default function (pi) { async execute(_id, params, _signal, _onUpdate, ctx) { lockActive = true; try { - const { session, title, options: opts, advance } = params; + const { session, title, advance } = params; + const phase = phaseId(ctx, currentPhase(ctx, session)); + const opts = phase === "F2" + ? normalizeMemoryOptions(params.options) + : params.options; const typeErr = validateDecisionTypes(ctx, opts, session); if (typeErr) return textResult(typeErr); - const phase = phaseId(ctx, currentPhase(ctx, session)); const toAdd = []; const meritOptions = opts .filter((o) => !isReserved(o.label)) - .map((o) => ({ id: o.id, label: o.label })); + .map((o) => ({ + id: o.id, + label: o.label, + ...(o.detail ? { detail: o.detail } : {}), + ...(o.rationale ? { rationale: o.rationale } : {}), + ...(o.meta ? { meta: o.meta } : {}), + })); const joinOnly = meritOptions.length > 0 && opts .filter((o) => !isReserved(o.label)) .every((o) => o.decision.type === "join_modified"); @@ -1084,7 +1247,7 @@ export default function (pi) { id: t.id, name: t.name, kind: t.kind, - recommended: t.recommended ?? true, + recommended: t.kind === "promote" ? (t.recommended ?? true) : false, description: cat.description ?? "", rationale: t.rationale ?? "", columns: cat.columns.map((c) => ({ ...c, suggested: suggested.has(c.name) })), @@ -1529,8 +1692,8 @@ export default function (pi) { label: "Promozione memorie riusabili (reviewer)", description: "F8 (prima della chiusura di fase): propone al reviewer i candidati di promozione " + - "calcolati dalla CLI (tht memory promote --preview: tipi riusabili concept_clarified/" + - "table_promoted/table_excluded, max 5, esclusi i gia' promossi/rifiutati). Le selezioni " + + "calcolati dalla CLI (tht memory promote --preview: solo concept_clarified, " + + "max 5, esclusi i gia' promossi/rifiutati). Le selezioni " + "vengono salvate nel vectordb (tht memory save-one) e registrate come memory_promoted; " + "le deselezioni come memory_promotion_declined (non riproposte). Nessun parametro oltre " + "alla sessione: i candidati sono deterministici, NON li scrivi tu. Registrata la " + @@ -1567,11 +1730,25 @@ export default function (pi) { "Nessun candidato di promozione: prosegui con la chiusura della sessione.", ); } + candidates = dedupePromotionCandidates(candidates); + if (candidates.length === 0) { + await ctx.ui.notify( + "Nessun concetto chiarito da salvare come memory per questa sessione.", + "info", + ); + const closed = closeAfterPromotion( + ctx, session, curNum, "Nessun candidato di promozione.", + ); + if (closed) return closed; + return textResult( + "Nessun candidato di promozione: prosegui con la chiusura della sessione.", + ); + } const options = promotionOptions(candidates); const widget = buildMultiselectRequest({ id: `u${Date.now()}`, phase, - title: "Quali decisioni salvare nella memoria riutilizzabile?", + title: "Quali concetti chiariti salvare nella memoria riutilizzabile?", allowEmpty: true, options, selected: options.map((o) => o.id), diff --git a/harness/.pi/skills/tht-sessione/SKILL.md b/harness/.pi/skills/tht-sessione/SKILL.md index 167e9f05..25a40c04 100644 --- a/harness/.pi/skills/tht-sessione/SKILL.md +++ b/harness/.pi/skills/tht-sessione/SKILL.md @@ -225,11 +225,15 @@ Prerequisite: Phase 1 closed. 2. The hit comes with full metadata (subject/detail/rationale): read what it says, where it comes from, why it might apply here, the out-of-context risk. 3. Present candidates in **a single** `reviewer_decide(multi:true, advance:true, - 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 `type`/`subject`/`rationale`; cite the source - memory id (`mem-`) in its rationale when applying it. Every option describes a + allow_empty:true)`. Rules: at most **5** candidates; ONLY + `concept_clarified`. Table choices (`table_promoted`, `table_excluded`) and all + other query-specific decisions (`question_rewritten`, `sql_approved`, …) are NOT + transferable and must never be stored, retrieved, or proposed as memories. Each + option carries `type`/`subject`/`rationale`; cite the source + memory id (`mem-`) in its rationale when applying it. Copy the hit's full + `content` verbatim into the option `description`: the reviewer must see the exact + memory text before deciding. Deduplicate hits by memory id before calling the gate. + 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 @@ -411,8 +415,15 @@ Prerequisite: Phase 6 closed. Prerequisite: Phase 7 closed. -1. Ask the reviewer whether they want a datamart (`reviewer_select` yes/no). -2. If yes: `tht datamart generate` (stub — raises NotImplementedError for now). Tell +1. Call `reviewer_datamart` with the session id. This gate is deployment-aware and is + the ONLY allowed way to record the datamart choice: + - `THT_PROFILE=workstation`: it records `datamart_declined` automatically and shows + no question to the reviewer; + - `THT_PROFILE=server` (including the default): it always shows both choices, + "Sì, genera il datamart" and "No, salta il datamart", and records the selected one. + Never replace this gate with a hand-built `reviewer_select`. +2. On a server, if the reviewer chose yes: `tht datamart generate` (stub — raises + NotImplementedError for now). Tell the reviewer that dbt generation is not implemented yet. 3. **Memory promotion closes the session.** Call `reviewer_memory_promote` with ONLY the session id: the gate computes the candidates itself (`tht memory promote diff --git a/harness/.pi/skills/tht-sessione/memoria.md b/harness/.pi/skills/tht-sessione/memoria.md index f386af09..2f2260db 100644 --- a/harness/.pi/skills/tht-sessione/memoria.md +++ b/harness/.pi/skills/tht-sessione/memoria.md @@ -9,8 +9,7 @@ already discarded (even after a Phase 2 reopen). For each memory to present in the checklist, include in the option's `label` and/or `description`: -- **What it says**: type + subject + detail (e.g. "table_promoted: - fact_seeablazione — main table for ablazioni"). +- **What it says**: the clarified concept, its subject, and its full definition. - **Where it comes from**: question_context and origin session_id. - **Why it might apply here**: overlap of concepts/tables with the current question (fields tables/concepts), similarity score. @@ -19,10 +18,10 @@ For each memory to present in the checklist, include in the option's `label` and Rules: -- Propose at most **5** candidates. Include ONLY memories of the 3 reusable types: - `concept_clarified`, `table_promoted`, `table_excluded`. Query-specific decisions - (e.g. `question_rewritten`, `sql_approved`) are NOT to be proposed: they don't - transfer to other questions. +- Propose at most **5** candidates. Include ONLY `concept_clarified` memories. + Table choices (`table_promoted`, `table_excluded`) and all other query-specific + decisions are not memories: never store, retrieve, or propose them because they + do not transfer to other questions. - All candidate memories go in **a single** `reviewer_decide(multi:true, 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 diff --git a/harness/tests/l2/test_memory_save_one_real.py b/harness/tests/l2/test_memory_save_one_real.py index 7861eff8..2399c496 100644 --- a/harness/tests/l2/test_memory_save_one_real.py +++ b/harness/tests/l2/test_memory_save_one_real.py @@ -34,9 +34,10 @@ def test_save_one_upserts_to_real_pgvector(l2_env): record = MemoryRecord( id="mem-l2test", ts=datetime.now(), session_id="l2-self-test", - decision_seq=999, type="table_promoted", subject="fct_ricoveri", - detail="ablazione", rationale="L2 self-test (idempotent)", - question_context="ablazione 2025", tables=["fct_ricoveri"], concepts=[], + decision_seq=999, type="concept_clarified", subject="ablazione recente", + detail="evento di ablazione negli ultimi 15 anni", + rationale="L2 self-test (idempotent)", + question_context="ablazione 2025", tables=[], concepts=["ablazione recente"], ) from tht.adapters.vector import ThothHttpVectorStore diff --git a/harness/tests/test_adapter_command_regressions.py b/harness/tests/test_adapter_command_regressions.py index ebadf06a..3ef3c3ac 100644 --- a/harness/tests/test_adapter_command_regressions.py +++ b/harness/tests/test_adapter_command_regressions.py @@ -68,9 +68,11 @@ def test_memory_command_writes_through_factory_vector_store(monkeypatch): cfg = SimpleNamespace(profile="server", embeddings=object(), vector_write_rest=None) manifest = SimpleNamespace(id="s1") snapshot = SimpleNamespace(manifest=manifest, decisions=[], artifacts={}) - record = MemoryRecord(id="m1", ts=datetime(2026, 1, 1), session_id="s1", - decision_seq=7, type="table_promoted", subject="t", - question_context="q") + record = MemoryRecord( + id="m1", ts=datetime(2026, 1, 1), session_id="s1", + decision_seq=7, type="concept_clarified", subject="paziente attivo", + detail="flag_attivo = TRUE", question_context="q", + ) monkeypatch.setattr(memory_cmd, "_load_config_or_exit", lambda path: cfg) monkeypatch.setattr(memory_cmd, "load_snapshot_or_exit", lambda cfg, session: snapshot) monkeypatch.setattr(memory_cmd, "registry_path", lambda cfg: None) diff --git a/harness/tests/test_decision_min_phase.py b/harness/tests/test_decision_min_phase.py index df05e45c..5b3fe112 100644 --- a/harness/tests/test_decision_min_phase.py +++ b/harness/tests/test_decision_min_phase.py @@ -16,7 +16,7 @@ from tht.workflow import load_workflow ("concept_clarified", 1), ("memory_rejected", 2), ("question_rewritten", 3), - ("table_promoted", 2), + ("table_promoted", 4), ("column_corrected", 4), ("evidence_accepted", 4), ("value_grounded", 4), diff --git a/harness/tests/test_memory_metadata.py b/harness/tests/test_memory_metadata.py index 4e5ed79d..67185740 100644 --- a/harness/tests/test_memory_metadata.py +++ b/harness/tests/test_memory_metadata.py @@ -13,9 +13,9 @@ from tht.memory import MemoryRecord, memory_vector_records def _record(**kw) -> MemoryRecord: base = dict( id="mem-x", ts=datetime(2025, 1, 1), session_id="s", decision_seq=1, - type="table_promoted", subject="dim_pazienti", detail="promossa", + type="concept_clarified", subject="paziente attivo", detail="flag_attivo = TRUE", rationale="perche' serve", question_context="dammi pazienti", - tables=["t"], concepts=[], + tables=[], concepts=["paziente attivo"], ) base.update(kw) return MemoryRecord(**base) @@ -23,17 +23,17 @@ def _record(**kw) -> MemoryRecord: def test_memory_vector_record_has_subject_detail_rationale_in_metadata(): vr = memory_vector_records([_record()])[0] - assert vr.metadata["subject"] == "dim_pazienti" - assert vr.metadata["detail"] == "promossa" + assert vr.metadata["subject"] == "paziente attivo" + assert vr.metadata["detail"] == "flag_attivo = TRUE" assert vr.metadata["rationale"] == "perche' serve" def test_memory_vector_record_metadata_keeps_existing_fields(): vr = memory_vector_records([_record()])[0] # i campi che gia' c'erano restano (backward compat) - assert vr.metadata["type"] == "table_promoted" - assert vr.metadata["tables"] == ["t"] - assert vr.metadata["concepts"] == [] + assert vr.metadata["type"] == "concept_clarified" + assert vr.metadata["tables"] == [] + assert vr.metadata["concepts"] == ["paziente attivo"] assert vr.metadata["session_id"] == "s" @@ -62,6 +62,6 @@ def test_save_one_memory_preserves_subject_through_upsert_row(): save_one_memory([_record(decision_seq=1)], decision_seq=1, store=writer, embedder=embedder) row = writer.upsert.call_args[0][1][0] md = row.record.metadata - assert md["subject"] == "dim_pazienti" - assert md["detail"] == "promossa" + assert md["subject"] == "paziente attivo" + assert md["detail"] == "flag_attivo = TRUE" assert md["rationale"] == "perche' serve" diff --git a/harness/tests/test_memory_promotion.py b/harness/tests/test_memory_promotion.py index 1bd867fd..420c08ba 100644 --- a/harness/tests/test_memory_promotion.py +++ b/harness/tests/test_memory_promotion.py @@ -9,7 +9,13 @@ la rende un passo del workflow. Questi test fissano il contratto harness-side: from datetime import datetime from tht.decisions import DecisionRecord, append_decision -from tht.memory import declined_promotion_seqs, reusable_promotions +from tht.memory import ( + MemoryRecord, + declined_promotion_seqs, + memory_vector_records, + promote, + reusable_promotions, +) from tht.session.models import SessionManifest from tht.workflow import load_workflow @@ -62,3 +68,48 @@ def test_reusable_promotions_exclude_declined(tmp_path): subject="fact_a", detail="seq:1") # seq 3 cand = reusable_promotions(tmp_path, _manifest(), tmp_path / "registry.jsonl") assert [c.decision_seq for c in cand] == [2] + + +def test_reusable_promotions_deduplicate_identical_memory_content(tmp_path): + append_decision(tmp_path, type="concept_clarified", subject="paziente attivo", + detail="flag_attivo = TRUE", rationale="scelta reviewer") + append_decision(tmp_path, type="concept_clarified", subject="paziente attivo", + detail="flag_attivo = TRUE", rationale="scelta reviewer") + + cand = reusable_promotions(tmp_path, _manifest(), tmp_path / "registry.jsonl") + + assert [c.decision_seq for c in cand] == [1] + + +def test_only_concept_clarified_is_proposed_or_promoted(tmp_path): + append_decision(tmp_path, type="table_promoted", subject="fact_a", + detail="tabella principale") + append_decision(tmp_path, type="table_excluded", subject="fact_b", + detail="tabella non pertinente") + append_decision(tmp_path, type="concept_clarified", subject="paziente attivo", + detail="flag_attivo = TRUE") + registry = tmp_path / "registry.jsonl" + + candidates = reusable_promotions(tmp_path, _manifest(), registry) + promoted = promote(tmp_path, _manifest(), seqs=[1, 2, 3], registry_path=registry) + + assert [(c.decision_seq, c.type) for c in candidates] == [(3, "concept_clarified")] + assert [(c.decision_seq, c.type) for c in promoted] == [(3, "concept_clarified")] + + +def test_legacy_table_records_are_not_published_as_memory_vectors(): + records = [ + MemoryRecord( + id="mem-0001", ts=datetime(2026, 1, 1), session_id="s1", + decision_seq=1, type="table_promoted", subject="fact_a", + ), + MemoryRecord( + id="mem-0002", ts=datetime(2026, 1, 1), session_id="s1", + decision_seq=2, type="concept_clarified", subject="paziente attivo", + detail="flag_attivo = TRUE", + ), + ] + + vectors = memory_vector_records(records) + + assert [record.ref for record in vectors] == ["mem-0002"] diff --git a/harness/tests/test_memory_save_one.py b/harness/tests/test_memory_save_one.py index 7a679668..bed7cce7 100644 --- a/harness/tests/test_memory_save_one.py +++ b/harness/tests/test_memory_save_one.py @@ -16,8 +16,9 @@ from tht.memory import MemoryRecord, memory_vector_record_for_decision, save_one def _record(seq: int = 7, **kw) -> MemoryRecord: base = dict( id="mem-0007", ts=datetime(2025, 1, 1), session_id="s1", decision_seq=seq, - type="table_promoted", subject="pazienti", detail="promossa", rationale="r", - question_context="dammi i pazienti", tables=["pazienti"], concepts=[], + type="concept_clarified", subject="paziente attivo", + detail="flag_attivo = TRUE", rationale="r", + question_context="dammi i pazienti", tables=[], concepts=["paziente attivo"], ) base.update(kw) return MemoryRecord(**base) diff --git a/harness/tests/test_psd_local_compose_contract.py b/harness/tests/test_psd_local_compose_contract.py index 3ddff6da..c6a37333 100644 --- a/harness/tests/test_psd_local_compose_contract.py +++ b/harness/tests/test_psd_local_compose_contract.py @@ -54,6 +54,10 @@ def test_psd_overlay_uses_generated_workspace_for_default_and_named_commands(): assert core["networks"]["default"]["aliases"] == ["core", "thothii-core"] frontend = compose["services"]["frontend"] assert frontend["ports"] == ["127.0.0.1:8099:8080"] + assert frontend["build"]["args"] == { + "VITE_BASE": "/", + "VITE_BACKEND_URL": "/api", + } assert frontend["networks"] == { "default": {"aliases": ["frontend", "thothii-frontend"]} } diff --git a/harness/tests/test_solved_search_cli.py b/harness/tests/test_solved_search_cli.py index 356d86ea..fe6f0018 100644 --- a/harness/tests/test_solved_search_cli.py +++ b/harness/tests/test_solved_search_cli.py @@ -7,10 +7,12 @@ puro (`[]` in modalita' --json) ed exit 0, cosi' il modello prosegue senza exemplar. Il finalize-hook gestisce gia' lo stesso scenario in modo analogo. """ import json +from datetime import datetime from typer.testing import CliRunner from tht.cli import app +from tht.memory import MemoryRecord, save_registry from tht.ports.vector import VectorReadUnavailable from tht.vectorstore.rest_client import VectorRestError from tht.vectorstore.store import VectorHit @@ -97,3 +99,44 @@ def test_solved_search_json_maps_hit_metadata(tmp_path, monkeypatch): "session_id": "s1", "question": "quante ablazioni nel 2023", "sql": "SELECT 1", "tables": ["fact_seeablazione"], "score": 0.91, }] + + +def test_memory_search_excludes_legacy_table_records(tmp_path, monkeypatch): + records = [ + MemoryRecord( + id="mem-0001", ts=datetime(2026, 1, 1), session_id="s1", + decision_seq=1, type="table_promoted", subject="fact_pazienti", + ), + MemoryRecord( + id="mem-0002", ts=datetime(2026, 1, 1), session_id="s1", + decision_seq=2, type="concept_clarified", subject="paziente attivo", + detail="flag_attivo = TRUE", + ), + ] + cfg = _cfg(tmp_path) + save_registry(records, tmp_path / "a" / "memory" / "registry.jsonl") + + class FakeSearcher: + def search(self, vec, top_n=10, kinds=None): + return [ + VectorHit( + id=f"memory:{record.id}", kind="memory", ref=record.id, + title=record.subject, content=record.detail, metadata={}, + similarity=0.9, + ) + for record in records + ] + + class FakeEmbedder: + def embed_query(self, text): + return [0.1] * 8 + + monkeypatch.setattr("tht.cli.vector_cmd.open_searcher", lambda workspace: FakeSearcher()) + monkeypatch.setattr("tht.cli.vector_cmd.make_embedder", lambda embeddings: FakeEmbedder()) + + res = CliRunner().invoke( + app, ["memory", "search", "pazienti", "--json", "-c", str(cfg)] + ) + + assert res.exit_code == 0, res.output + assert [record["id"] for record in json.loads(res.stdout)] == ["mem-0002"] diff --git a/harness/tht/cli/memory_cmd.py b/harness/tht/cli/memory_cmd.py index ea0d1fe9..0f1ea79e 100644 --- a/harness/tht/cli/memory_cmd.py +++ b/harness/tht/cli/memory_cmd.py @@ -384,7 +384,7 @@ def search_cmd( from rich.table import Table from tht.cli.vector_cmd import make_embedder, open_searcher, require_vector_cfg - from tht.memory import decided_memory_ids, load_registry + from tht.memory import REUSABLE_TYPES, decided_memory_ids, load_registry cfg = _load_config_or_exit(config) require_vector_cfg(cfg) @@ -401,6 +401,8 @@ def search_cmd( rec = by_id.get(h.ref) if rec is None: continue # indice piu' avanti del registro: ignora + if rec.type not in REUSABLE_TYPES: + continue # record legacy non-concept: non e' una memory riusabile if rec.id in excluded: continue # gia' decisa in questa sessione: non riproporla results.append({ diff --git a/harness/tht/memory.py b/harness/tht/memory.py index cdd2ed2d..84a82ddf 100644 --- a/harness/tht/memory.py +++ b/harness/tht/memory.py @@ -115,14 +115,6 @@ def delete_record(path: Path, mem_id: str) -> MemoryRecord: raise MemoryNotFound(mem_id) -def _default_tables(decision: DecisionRecord) -> list[str]: - if decision.type in ("table_promoted", "table_excluded"): - return [decision.subject] - if decision.type in ("column_corrected", "join_modified") and "." in decision.subject: - return [decision.subject.split(".")[0]] - return [] - - def _default_concepts(decision: DecisionRecord) -> list[str]: if decision.type == "concept_clarified": return [decision.subject] @@ -139,9 +131,9 @@ def _next_id_num(existing: list[MemoryRecord]) -> int: return (max(nums) + 1) if nums else 1 -# Solo questi tipi di decisione sono concetti riusabili in altre generazioni -# (scelta reviewer): il resto e' query-specifico e non va proposto in promozione. -REUSABLE_TYPES = frozenset({"concept_clarified", "table_promoted", "table_excluded"}) +# Solo i concetti chiariti sono riusabili tra domande. Le scelte sulle tabelle +# dipendono dallo schema-linking della singola domanda e non sono memory. +REUSABLE_TYPES = frozenset({"concept_clarified"}) MAX_PROMOTION_CANDIDATES = 5 @@ -159,6 +151,8 @@ def _compute_promotions( context = question_context(decisions, manifest) out: list[MemoryRecord] = [] for d in selected: + if d.type not in REUSABLE_TYPES: + continue if (manifest.id, d.seq) in already: continue out.append( @@ -166,7 +160,7 @@ def _compute_promotions( id=f"mem-{n:04d}", ts=datetime.now(UTC), session_id=manifest.id, decision_seq=d.seq, type=d.type, subject=d.subject, detail=d.detail, rationale=d.rationale, question_context=context, - tables=_default_tables(d), concepts=_default_concepts(d), + tables=[], concepts=_default_concepts(d), ) ) n += 1 @@ -206,9 +200,10 @@ def reusable_promotions( session_dir, manifest, seqs=None, existing=load_registry(registry_path) ) declined = declined_promotion_seqs(effective_decisions(session_dir)) - return [ + reusable = [ c for c in cand if c.type in REUSABLE_TYPES and c.decision_seq not in declined ] + return _dedupe_reusable_promotions(reusable) def reusable_promotions_snapshot(snapshot, registry_path: Path) -> list[MemoryRecord]: @@ -216,7 +211,30 @@ def reusable_promotions_snapshot(snapshot, registry_path: Path) -> list[MemoryRe cand = _compute_promotions(snapshot, snapshot.manifest, seqs=None, existing=load_registry(registry_path)) declined = declined_promotion_seqs(effective_decisions(snapshot)) - return [c for c in cand if c.type in REUSABLE_TYPES and c.decision_seq not in declined] + reusable = [c for c in cand if c.type in REUSABLE_TYPES and c.decision_seq not in declined] + return _dedupe_reusable_promotions(reusable) + + +def _dedupe_reusable_promotions(records: list[MemoryRecord]) -> list[MemoryRecord]: + """Keep the first proposal for identical reviewer-visible memory content.""" + seen: set[tuple[str, str, str, str, str]] = set() + out: list[MemoryRecord] = [] + for record in records: + key = tuple( + " ".join(value.split()).casefold() + for value in ( + record.type, + record.subject, + record.detail, + record.rationale, + record.question_context, + ) + ) + if key in seen: + continue + seen.add(key) + out.append(record) + return out def preview_promotions( @@ -235,6 +253,8 @@ def preview_promotions_snapshot(snapshot, registry_path: Path) -> list[MemoryRec def memory_vector_records(records: list[MemoryRecord]) -> list[VectorRecord]: out: list[VectorRecord] = [] for r in records: + if r.type not in REUSABLE_TYPES: + continue lines = [ f"Decisione {r.type}: {r.subject}", r.detail, @@ -268,10 +288,12 @@ def memory_vector_record_for_decision( D11 save-one builds only this one record (not the full memory_vector_records list) so the remote upsert is a single row. """ - match = [r for r in records if r.decision_seq == decision_seq] - if not match: + vectors = memory_vector_records( + [r for r in records if r.decision_seq == decision_seq] + ) + if not vectors: return None - return memory_vector_records(match)[0] + return vectors[0] def save_one_memory( diff --git a/harness/workflow.yaml b/harness/workflow.yaml index 9d8a3a79..af17f1d2 100644 --- a/harness/workflow.yaml +++ b/harness/workflow.yaml @@ -21,7 +21,7 @@ phases: advance: auto_if_empty prerequisites: [] artifacts_out: [] - emits: [memory_rejected, table_promoted, table_excluded] + emits: [memory_rejected, concept_clarified] - id: F3 name: riscrittura advance: kind:phase