refactor(disambiguation): extract F3 rewrite path (#26)

2026-08-24 01:44:40 +02:00
parent eccf6212f1
commit 2ed55ef131
4 changed files with 247 additions and 61 deletions
@@ -0,0 +1,133 @@
+const test = require("node:test");
+const assert = require("node:assert");
+const { createDisambiguationGate } = require("../disambiguation/index.js");
+const { createFakePi } = require("./fake_pi_runtime.js");
+
+
+function setupRewrite({ phase = 3, failAt } = {}) {
+  const { pi, ctx, tools } = createFakePi();
+  const calls = [];
+  const error = (message) => ({ content: [{ type: "text", text: message }] });
+  const gate = createDisambiguationGate({
+    workflow: {
+      activate: () => calls.push(["activate"]),
+      phase: () => {
+        calls.push(["phase"]);
+        return { number: phase, id: `F${phase}` };
+      },
+      advance: (_ctx, session, current) => {
+        calls.push(["advance", session, current]);
+        return failAt === "advance" ? { err: error("advance failed") } : { finalized: false };
+      },
+    },
+    session: {
+      mutate: (_ctx, args) => {
+        calls.push(["session", ...args]);
+        return failAt === "session" ? error("question write failed") : null;
+      },
+    },
+    ledger: {
+      record: (_ctx, session, decision) => {
+        calls.push(["ledger", session, decision]);
+        return failAt === "ledger" ? error("ledger write failed") : null;
+      },
+    },
+    toTextResult: (text) => ({ content: [{ type: "text", text }] }),
+  });
+  gate.install(pi);
+  return { ctx, tools, calls };
+}
+
+
+for (const resumed of [false, true]) {
+  test(`the Disambiguation facade rewrites and closes F3 in order (${resumed ? "resume" : "new"})`, async () => {
+    const { ctx, tools, calls } = setupRewrite();
+    ctx.resumed = resumed;
+
+    const result = await tools.get("rewrite_question").def.execute(
+      `rewrite-${resumed}`,
+      {
+        session: "s1",
+        question: "pazienti con ablazione",
+        assumptions: resumed ? '["anno 2025"]' : ["anno 2025"],
+      },
+      null,
+      null,
+      ctx,
+    );
+
+    assert.deepEqual(calls, [
+      ["activate"],
+      ["phase"],
+      [
+        "session", "set-question", "s1", "--question", "pazienti con ablazione",
+        "--assumption", "anno 2025",
+      ],
+      [
+        "ledger", "s1", {
+          type: "question_rewritten",
+          subject: "domanda",
+          detail: "pazienti con ablazione",
+        },
+      ],
+      ["advance", "s1", 3],
+    ]);
+    assert.match(result.content[0].text, /Fase 3 completata/);
+  });
+}
+
+
+test("the Disambiguation facade preserves a non-JSON assumption as one item", async () => {
+  const { ctx, tools, calls } = setupRewrite();
+
+  await tools.get("rewrite_question").def.execute(
+    "rewrite-assumption",
+    { session: "s1", question: "q", assumptions: "assunzione libera" },
+    null,
+    null,
+    ctx,
+  );
+
+  assert.deepEqual(calls[2], [
+    "session", "set-question", "s1", "--question", "q",
+    "--assumption", "assunzione libera",
+  ]);
+});
+
+
+test("the Disambiguation facade refuses rewrite outside F3 before mutations", async () => {
+  const { ctx, tools, calls } = setupRewrite({ phase: 4 });
+
+  const result = await tools.get("rewrite_question").def.execute(
+    "rewrite-outside",
+    { session: "s1", question: "q" },
+    null,
+    null,
+    ctx,
+  );
+
+  assert.deepEqual(calls, [["activate"], ["phase"]]);
+  assert.match(result.content[0].text, /solo in Fase 3/);
+});
+
+
+for (const [failAt, expectedCalls, errorPattern] of [
+	["session", ["activate", "phase", "session"], /question write failed/],
+	["ledger", ["activate", "phase", "session", "ledger"], /ledger write failed/],
+	["advance", ["activate", "phase", "session", "ledger", "advance"], /advance failed/],
+]) {
+  test(`the Disambiguation facade stops F3 after a ${failAt} failure`, async () => {
+    const { ctx, tools, calls } = setupRewrite({ failAt });
+
+    const result = await tools.get("rewrite_question").def.execute(
+      `rewrite-${failAt}`,
+      { session: "s1", question: "q", assumptions: [] },
+      null,
+      null,
+      ctx,
+    );
+
+    assert.deepEqual(calls.map(([kind]) => kind), expectedCalls);
+		assert.match(result.content[0].text, errorPattern);
+  });
+}
@@ -0,0 +1,77 @@
+import { Type } from "typebox";
+
+
+function normalizedAssumptions(assumptions) {
+	let normalized = assumptions;
+	if (typeof normalized === "string") {
+		try {
+			normalized = JSON.parse(normalized);
+		} catch {
+			normalized = [normalized];
+		}
+	}
+	return Array.isArray(normalized) ? normalized.map(String) : [];
+}
+
+
+function installDisambiguationGate(
+	pi,
+	{ workflow, session, ledger, toTextResult },
+) {
+	// Compatibility: tool schema, labels and messages move verbatim from the
+	// composition root; this extraction changes ownership, not Pi behavior.
+	pi.registerTool({
+		name: "rewrite_question",
+		label: "Riscrittura domanda e chiusura F3 (deterministica)",
+		description:
+			"In Fase 3 scrive deterministicamente question.md, registra question_rewritten e " +
+			"chiude la fase senza chiedere conferma al reviewer. assumptions puo' essere array " +
+			"o stringa JSON.",
+		parameters: Type.Object({
+			session: Type.String(),
+			question: Type.String(),
+			assumptions: Type.Optional(Type.Any()),
+		}),
+		async execute(_id, params, _signal, _onUpdate, ctx) {
+			workflow.activate();
+			try {
+				const { session: sessionId, question } = params;
+				const current = workflow.phase(ctx, sessionId);
+				if (current.number !== 3) {
+					return toTextResult("La riscrittura automatica e' disponibile solo in Fase 3.");
+				}
+				// `tht session set-question` takes the session id as a positional argument
+				// (unlike phase/cte/decision, which use --session).
+				const args = ["set-question", sessionId, "--question", question];
+				for (const assumption of normalizedAssumptions(params.assumptions)) {
+					args.push("--assumption", assumption);
+				}
+				const mutationError = session.mutate(ctx, args, "");
+				if (mutationError) return mutationError;
+				const decisionError = ledger.record(ctx, sessionId, {
+					type: "question_rewritten",
+					subject: "domanda",
+					detail: question,
+				}, "");
+				if (decisionError) return decisionError;
+				const advanced = workflow.advance(ctx, sessionId, current.number);
+				if (advanced.err) return advanced.err;
+				return toTextResult(
+					`Domanda riscritta e Fase 3 completata (sessione ${sessionId}).`,
+				);
+			} catch (fatal) {
+				const message = (fatal.stderr || fatal.message || String(fatal)).toString().trim();
+				return toTextResult(
+					`[rewrite_question ERRORE INTERNO] ${message}. Riprova o usa un approccio diverso.`,
+				);
+			}
+		},
+	});
+}
+
+
+export function createDisambiguationGate(dependencies) {
+	return {
+		install: (pi) => installDisambiguationGate(pi, dependencies),
+	};
+}
@@ -45,6 +45,7 @@ import {
 	appendLedgerSection,
 } from "./gate/enrich.js";
 import { createMemoryGate } from "./gate/memory/index.js";
+import { createDisambiguationGate } from "./gate/disambiguation/index.js";
 import { isReserved } from "./reserved-labels.mjs";

 // Load the workflow contract once when Pi loads the extension. Asking the model to
@@ -619,6 +620,26 @@ export default function (pi) {
 		waitForReviewer: emitAndWait,
 		toTextResult: textResult,
 	});
+	const disambiguationGate = createDisambiguationGate({
+		workflow: {
+			activate: () => {
+				lockActive = true;
+			},
+			phase: (ctx, session) => ({ number: currentPhase(ctx, session) }),
+			advance: advancePhaseAndFinalize,
+		},
+		session: {
+			mutate: (ctx, args, recovery) => relayIfThtFails(
+				ctx, ["session", ...args], recovery,
+			),
+		},
+		ledger: {
+			record: (ctx, session, decision, recovery) => relayIfThtFails(
+				ctx, decisionAddArgs(session, decision), recovery,
+			),
+		},
+		toTextResult: textResult,
+	});

 	// 1) ANTI-BYPASS tool_call hook (spec D4, verbatim). Blocks direct phase/decision
 	//    calls and writes to protected files so the reviewer cannot bypass the gate.
@@ -1580,63 +1601,7 @@ export default function (pi) {
 	}

 	memoryGate.install(pi);
-
-	pi.registerTool({
-		name: "rewrite_question",
-		label: "Riscrittura domanda e chiusura F3 (deterministica)",
-		description:
-			"In Fase 3 scrive deterministicamente question.md, registra question_rewritten e " +
-			"chiude la fase senza chiedere conferma al reviewer. assumptions puo' essere array " +
-			"o stringa JSON.",
-		parameters: Type.Object({
-			session: Type.String(),
-			question: Type.String(),
-			assumptions: Type.Optional(Type.Any()),
-		}),
-		async execute(_id, params, _signal, _onUpdate, ctx) {
-			lockActive = true;
-			try {
-			const { session, question, assumptions } = params;
-			const curNum = currentPhase(ctx, session);
-			if (curNum !== 3) {
-				return textResult("La riscrittura automatica e' disponibile solo in Fase 3.");
-			}
-			let assumps = assumptions;
-			if (typeof assumps === "string") {
-				try {
-					assumps = JSON.parse(assumps);
-				} catch {
-					assumps = [assumps];
-				}
-			}
-			// `tht session set-question` takes the session id as a positional argument
-			// (the `session` command group uses positional ids, unlike phase/cte/decision
-			// which use --session).
-			const args = ["session", "set-question", session, "--question", question];
-			if (Array.isArray(assumps)) {
-				for (const a of assumps) args.push("--assumption", String(a));
-			}
-			const err = relayIfThtFails(ctx, args, "");
-			if (err) return err;
-			const decisionErr = relayIfThtFails(
-				ctx,
-				decisionAddArgs(session, {
-					type: "question_rewritten",
-					subject: "domanda",
-					detail: question,
-				}),
-				"",
-			);
-			if (decisionErr) return decisionErr;
-			const advanced = advancePhaseAndFinalize(ctx, session, curNum);
-			if (advanced.err) return advanced.err;
-			return textResult(`Domanda riscritta e Fase 3 completata (sessione ${session}).`);
-			} catch (fatal) {
-				const msg = (fatal.stderr || fatal.message || String(fatal)).toString().trim();
-				return textResult(`[rewrite_question ERRORE INTERNO] ${msg}. Riprova o usa un approccio diverso.`);
-			}
-		},
-	});
+	disambiguationGate.install(pi);

 	pi.registerTool({
 		name: "write_schema_linking",