fix: harden reviewer workflow and memory handling

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;
+  }
+});

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);
 });

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"]);
+});

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 = [];

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),