diff --git a/PROJECT_STATE.md b/PROJECT_STATE.md index 71c54c04..ff04c353 100644 --- a/PROJECT_STATE.md +++ b/PROJECT_STATE.md @@ -1054,8 +1054,8 @@ whole-branch review + fix wave). ≤200 chars); new read-only `tht cte info --session --json` (index/total from `cte_plan.json`, same source as `next_cte`); `tht cte plan --doc -` writes `cte_plan_doc.json` (chain documentation; `cte_plan.json` stays a load-bearing `list[str]`). -- **Gate (JS):** `gate/artifact-contracts.js` (soft validators → self-corrective `textResult`, - TypeBox untouched) + `gate/enrich.js` (pure, catalog lookups injected); +- **Gate (JS):** `gate/core/artifact-contracts.js` (soft validators → self-corrective + `textResult`, TypeBox untouched) + `gate/core/enrich.js` (pure, catalog lookups injected); `prepareReviewerArguments` now coerces `artifact.data` too (GLM stringified-param mitigation); `SKILL.md` Phase 5/6 + disciplines rewritten (plan via `reviewer_confirm kind:"cte_plan"` with payload A; `cte_result` gates send THIN data only — never SQL/preview diff --git a/docs/general/pi-configuration.md b/docs/general/pi-configuration.md index 86a6d912..5cf8fe25 100644 --- a/docs/general/pi-configuration.md +++ b/docs/general/pi-configuration.md @@ -128,8 +128,10 @@ harness/.pi/ ├── extensions/ │ ├── aritmolab-provider.js ← provider LLM solo-progetto │ ├── tht-gate.js ← gate human-in-the-loop -│ ├── reserved-labels.mjs ← utility condivisa (NON auto-caricata; .mjs ignorato) -│ └── gate/ ← modulo usato da tht-gate.js +│ └── gate/ +│ ├── core/ ← enforcement e utility condivise del gate +│ ├── disambiguation/ ← policy F1/F3 +│ └── memory/ ← policy F2/F8 ├── settings.json ← (opzionale) override delle impostazioni utente └── themes/ └── thothii-mono.json ← tema del progetto diff --git a/harness/.pi/extensions/gate/__tests__/artifact_contracts.test.js b/harness/.pi/extensions/gate/__tests__/artifact_contracts.test.js index 6b604f5b..42431010 100644 --- a/harness/.pi/extensions/gate/__tests__/artifact_contracts.test.js +++ b/harness/.pi/extensions/gate/__tests__/artifact_contracts.test.js @@ -7,7 +7,7 @@ const { validateCtePlanV2, validateCteResultThin, validatePhaseSummaryV2, -} = require("../artifact-contracts.js"); +} = require("../core/artifact-contracts.js"); // --- validateCtePlanV2 (payload A) ------------------------------------------------ diff --git a/harness/.pi/extensions/gate/__tests__/builders.test.js b/harness/.pi/extensions/gate/__tests__/builders.test.js index 25a6c0e9..64e7e32b 100644 --- a/harness/.pi/extensions/gate/__tests__/builders.test.js +++ b/harness/.pi/extensions/gate/__tests__/builders.test.js @@ -18,7 +18,7 @@ const { buildSchemaLinkingRequest, buildJoinReviewRequest, withChildLinkage, -} = require("../builders.js"); +} = require("../core/builders.js"); const GOLDEN = path.join(__dirname, "golden"); const golden = (name) => JSON.parse(fs.readFileSync(path.join(GOLDEN, name))); diff --git a/harness/.pi/extensions/gate/__tests__/enrich.test.js b/harness/.pi/extensions/gate/__tests__/enrich.test.js index 5002b76a..8a743a6d 100644 --- a/harness/.pi/extensions/gate/__tests__/enrich.test.js +++ b/harness/.pi/extensions/gate/__tests__/enrich.test.js @@ -6,7 +6,7 @@ const { enrichCtePlanV2, buildCteResultV2, enrichPhaseSummaryV2, -} = require("../enrich.js"); +} = require("../core/enrich.js"); // --- enrichCtePlanV2 -------------------------------------------------------------- @@ -202,7 +202,7 @@ test("enrichPhaseSummaryV2 preserves open_questions/checks untouched", () => { // --- appendLedgerSection ----------------------------------------------------------- -const { appendLedgerSection } = require("../enrich.js"); +const { appendLedgerSection } = require("../core/enrich.js"); test("appendLedgerSection appends only the phase's substantive decisions", () => { const data = { schema_version: 2, summary: "s", sections: [{ title: "Criteri", items: [] }] }; 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 100114d4..9b00f878 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,8 @@ const test = require("node:test"); const assert = require("node:assert"); const { createMemoryGate } = require("../memory/index.js"); +const { buildMultiselectRequest } = require("../core/builders.js"); +const { isReserved } = require("../core/reserved-labels.mjs"); const { createFakePi } = require("./fake_pi_runtime.js"); @@ -74,6 +76,10 @@ test("the public Memory facade owns F8 policy and mutation ordering", async () = return null; }, }, + reviewer: { + buildMultiselect: buildMultiselectRequest, + isReserved, + }, waitForReviewer: async (runtimeContext, widget) => { const response = await runtimeContext.ui.input(JSON.stringify(widget), ""); return JSON.parse(response); 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 fe654550..cbbea336 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,8 @@ const test = require("node:test"); const assert = require("node:assert"); const { createMemoryGate } = require("../memory/index.js"); +const { buildMultiselectRequest } = require("../core/builders.js"); +const { isReserved } = require("../core/reserved-labels.mjs"); const { createFakePi } = require("./fake_pi_runtime.js"); @@ -71,6 +73,10 @@ function setupRecall(choices) { return null; }, }, + reviewer: { + buildMultiselect: buildMultiselectRequest, + isReserved, + }, waitForReviewer: async (runtimeContext, widget) => { const response = await runtimeContext.ui.input(JSON.stringify(widget), ""); return JSON.parse(response); diff --git a/harness/.pi/extensions/gate/__tests__/gate_stringified_params.test.js b/harness/.pi/extensions/gate/__tests__/gate_stringified_params.test.js index 1c24c6c3..e7169eb9 100644 --- a/harness/.pi/extensions/gate/__tests__/gate_stringified_params.test.js +++ b/harness/.pi/extensions/gate/__tests__/gate_stringified_params.test.js @@ -75,7 +75,7 @@ test("prepareReviewerArguments leaves a legacy markdown string artifact.data UNC test("GATE_CODE_FILES blocks writes to gate extensions, not session artifacts", () => { assert.ok(GATE_CODE_FILES.test("harness/.pi/extensions/tht-gate.js")); - assert.ok(GATE_CODE_FILES.test(".pi/extensions/reserved-labels.mjs")); + assert.ok(GATE_CODE_FILES.test(".pi/extensions/gate/core/reserved-labels.mjs")); assert.equal(GATE_CODE_FILES.test("sessions/s1/schema_linking.json"), false); assert.equal(GATE_CODE_FILES.test("sessions/s1/sql_final.sql"), false); }); diff --git a/harness/.pi/extensions/gate/__tests__/module_boundaries.test.js b/harness/.pi/extensions/gate/__tests__/module_boundaries.test.js new file mode 100644 index 00000000..32ff4b10 --- /dev/null +++ b/harness/.pi/extensions/gate/__tests__/module_boundaries.test.js @@ -0,0 +1,43 @@ +const test = require("node:test"); +const assert = require("node:assert"); +const { readdirSync, readFileSync } = require("node:fs"); +const { dirname, extname, join, relative, resolve, sep } = require("node:path"); + +const gateRoot = resolve(__dirname, ".."); +const domainRoots = ["disambiguation", "memory"]; + + +function productionFiles(root) { + return readdirSync(root, { withFileTypes: true }).flatMap((entry) => { + const path = join(root, entry.name); + if (entry.isDirectory()) { + return entry.name === "__tests__" ? [] : productionFiles(path); + } + return [".js", ".mjs"].includes(extname(entry.name)) ? [path] : []; + }); +} + + +function relativeImports(path) { + const source = readFileSync(path, "utf8"); + return [...source.matchAll(/(?:from\s+|import\s*(?:\(\s*)?|require\s*\(\s*)["'](\.[^"']+)["']/g)] + .map((match) => match[1]); +} + + +test("gate domain modules receive shared behavior as capabilities", () => { + for (const domain of domainRoots) { + const root = join(gateRoot, domain); + for (const path of productionFiles(root)) { + const imports = relativeImports(path).map((specifier) => + relative(gateRoot, resolve(dirname(path), specifier)).split(sep).join("/")); + const violations = imports.filter((target) => + target !== domain && !target.startsWith(`${domain}/`)); + assert.deepEqual( + violations, + [], + `${relative(gateRoot, path)} imports gate implementation directly: ${violations.join(", ")}`, + ); + } + } +}); diff --git a/harness/.pi/extensions/gate/__tests__/reserved_labels.test.js b/harness/.pi/extensions/gate/__tests__/reserved_labels.test.js index f71d1da1..9dd83870 100644 --- a/harness/.pi/extensions/gate/__tests__/reserved_labels.test.js +++ b/harness/.pi/extensions/gate/__tests__/reserved_labels.test.js @@ -1,6 +1,6 @@ const test = require("node:test"); const assert = require("node:assert"); -const { isReserved, stripReserved } = require("../../reserved-labels.mjs"); +const { isReserved, stripReserved } = require("../core/reserved-labels.mjs"); test("Altro variants (any punctuation/case) are reserved", () => { assert.equal(isReserved("Altro — specifica…"), true); diff --git a/harness/.pi/extensions/gate/artifact-contracts.js b/harness/.pi/extensions/gate/core/artifact-contracts.js similarity index 100% rename from harness/.pi/extensions/gate/artifact-contracts.js rename to harness/.pi/extensions/gate/core/artifact-contracts.js diff --git a/harness/.pi/extensions/gate/builders.js b/harness/.pi/extensions/gate/core/builders.js similarity index 100% rename from harness/.pi/extensions/gate/builders.js rename to harness/.pi/extensions/gate/core/builders.js diff --git a/harness/.pi/extensions/gate/enrich.js b/harness/.pi/extensions/gate/core/enrich.js similarity index 100% rename from harness/.pi/extensions/gate/enrich.js rename to harness/.pi/extensions/gate/core/enrich.js diff --git a/harness/.pi/extensions/reserved-labels.mjs b/harness/.pi/extensions/gate/core/reserved-labels.mjs similarity index 100% rename from harness/.pi/extensions/reserved-labels.mjs rename to harness/.pi/extensions/gate/core/reserved-labels.mjs diff --git a/harness/.pi/extensions/gate/disambiguation/index.js b/harness/.pi/extensions/gate/disambiguation/index.js index bed430b5..4c1ca098 100644 --- a/harness/.pi/extensions/gate/disambiguation/index.js +++ b/harness/.pi/extensions/gate/disambiguation/index.js @@ -1,6 +1,17 @@ import { Type } from "typebox"; +export const NEW_SESSION_CLARIFICATION_KICKOFF = + "2. Nel primo turno identifica la SOLA ambiguità con maggiore impatto sulla query e " + + "chiama subito `reviewer_select` con opzioni concrete. Niente lunga narrazione, elenco " + + "di tutte le ambiguità o ricapitolazione preliminare.\n"; + + +export const EMPTY_SESSION_CLARIFICATION_KICKOFF = + "Se la sessione e' in fase 1 e non ha decisioni, dopo `session show` chiama " + + "`reviewer_select` subito: non produrre testo libero.\n"; + + function normalizedAssumptions(assumptions) { let normalized = assumptions; if (typeof normalized === "string") { diff --git a/harness/.pi/extensions/gate/memory/index.js b/harness/.pi/extensions/gate/memory/index.js index e7da0fc9..f9fb6851 100644 --- a/harness/.pi/extensions/gate/memory/index.js +++ b/harness/.pi/extensions/gate/memory/index.js @@ -1,8 +1,5 @@ import { Type } from "typebox"; -import { buildMultiselectRequest } from "../builders.js"; -import { isReserved } from "../../reserved-labels.mjs"; - // Compatibility note: reviewer labels move verbatim from the composition root. // Tickets #22 and #23 require observable parity; translating existing chrome is a // separate product behavior change rather than part of these extractions. @@ -80,14 +77,14 @@ function shouldSkipEmptyRecall({ meritCount, allowEmpty, advance }) { async function reviewRecall(ctx, params, phase, dependencies) { - const { workflow, ledger, waitForReviewer, toTextResult } = dependencies; + const { workflow, ledger, reviewer, waitForReviewer, toTextResult } = dependencies; try { const { session, advance } = params; const options = normalizeMemoryOptions(params.options); const typeError = ledger.validate(ctx, options, session); if (typeError) return toTextResult(typeError); const meritOptions = options - .filter((option) => !isReserved(option.label)) + .filter((option) => !reviewer.isReserved(option.label)) .map((option) => ({ id: option.id, label: option.label, @@ -110,7 +107,7 @@ async function reviewRecall(ctx, params, phase, dependencies) { "avanzamento automatico alla fase successiva.", ); } - const widget = buildMultiselectRequest({ + const widget = reviewer.buildMultiselect({ id: `u${Date.now()}`, phase, allowEmpty: params.allow_empty ?? false, @@ -225,7 +222,7 @@ function splitPromotionChoices(candidates, choices) { */ function installMemoryGate( pi, - { workflow, memory, ledger, waitForReviewer, toTextResult }, + { workflow, memory, ledger, reviewer, waitForReviewer, toTextResult }, ) { pi.registerTool({ name: "reviewer_memory_promote", @@ -285,7 +282,7 @@ function installMemoryGate( ); } const options = promotionOptions(candidates); - const widget = buildMultiselectRequest({ + const widget = reviewer.buildMultiselect({ id: `u${Date.now()}`, phase: phase.id, title: "Quali concetti chiariti salvare nella memoria riutilizzabile?", diff --git a/harness/.pi/extensions/tht-gate.js b/harness/.pi/extensions/tht-gate.js index b3ec142e..6cf8179d 100644 --- a/harness/.pi/extensions/tht-gate.js +++ b/harness/.pi/extensions/tht-gate.js @@ -6,7 +6,7 @@ // the reference implementation PHASE_NAMES array (truncated to 7) is gone; F8/datamart can no // longer drift out of sync. // (2) D2/D4 widget-descriptor: the reviewer interaction is emitted as a -// widget-descriptor JSON (built by ./gate/builders.js) and awaited by id, instead +// widget-descriptor JSON (built by ./gate/core/builders.js) and awaited by id, instead // of rendered by blocking native TUI primitives (ctx.ui.select/custom). // // PRESERVED VERBATIM from the source (load-bearing runtime glue, spec D4): @@ -30,12 +30,12 @@ import { buildArtifactGate, buildSchemaLinkingRequest, buildJoinReviewRequest, -} from "./gate/builders.js"; +} from "./gate/core/builders.js"; import { validateCtePlanV2, validateCteResultThin, validatePhaseSummaryV2, -} from "./gate/artifact-contracts.js"; +} from "./gate/core/artifact-contracts.js"; import { memoizeGetColumns, enrichCtePlanV2, @@ -43,10 +43,14 @@ import { enrichCteResultColumns, enrichPhaseSummaryV2, appendLedgerSection, -} from "./gate/enrich.js"; +} from "./gate/core/enrich.js"; import { createMemoryGate } from "./gate/memory/index.js"; -import { createDisambiguationGate } from "./gate/disambiguation/index.js"; -import { isReserved } from "./reserved-labels.mjs"; +import { + createDisambiguationGate, + EMPTY_SESSION_CLARIFICATION_KICKOFF, + NEW_SESSION_CLARIFICATION_KICKOFF, +} from "./gate/disambiguation/index.js"; +import { isReserved } from "./gate/core/reserved-labels.mjs"; // Load the workflow contract once when Pi loads the extension. Asking the model to // discover/read the skill as its first action proved unreliable with remote models: @@ -204,9 +208,7 @@ const NUOVA_DOMANDA_KICKOFF_PROVIDED = (sessionId, hasRetrievalPack = false) => : "1. Il retrieval pack non era ancora disponibile: come PRIMA chiamata tool esegui " + "subito `tht search pack \"\" --session " + sessionId + "` e usa il risultato.\n") + - "2. Nel primo turno identifica la SOLA ambiguità con maggiore impatto sulla query e " + - "chiama subito `reviewer_select` con opzioni concrete. Niente lunga narrazione, elenco " + - "di tutte le ambiguità o ricapitolazione preliminare.\n" + + NEW_SESSION_CLARIFICATION_KICKOFF + "Regole non negoziabili: una domanda al reviewer per volta; mai promuovere/escludere/" + "correggere senza conferma; le interazioni passano dai tool reviewer_*; testo libero col prefisso '!'. " + "MAI `tht phase advance|reopen` né `tht decision add` da shell."; @@ -225,7 +227,8 @@ const RIPRENDI_KICKOFF = (hasRetrievalPack = false) => "cio' che non e' registrato non e' avvenuto) e riprendi da li'.\n" + "Esegui il passo 1 ORA, in QUESTO stesso turno, chiamando subito il tool `bash` per " + "`tht session show`: NON limitarti a dichiarare l'intenzione e NON " + - "terminare il turno prima di aver chiamato i tool. Se la sessione e' in fase 1 e non ha decisioni, dopo `session show` chiama `reviewer_select` subito: non produrre testo libero.\n" + + "terminare il turno prima di aver chiamato i tool. " + + EMPTY_SESSION_CLARIFICATION_KICKOFF + "Valgono le stesse regole non negoziabili: una domanda per volta, conferma esplicita, tool " + "reviewer_*, niente phase advance/reopen o decision add da shell, una fase alla volta."; @@ -617,6 +620,10 @@ export default function (pi) { ctx, decisionAddArgs(session, decision), recovery, ), }, + reviewer: { + buildMultiselect: buildMultiselectRequest, + isReserved, + }, waitForReviewer: emitAndWait, toTextResult: textResult, }); diff --git a/harness/tests/test_module_boundaries.py b/harness/tests/test_module_boundaries.py new file mode 100644 index 00000000..3c14ed4b --- /dev/null +++ b/harness/tests/test_module_boundaries.py @@ -0,0 +1,72 @@ +"""Architecture contracts for the internal workflow modules.""" + +from __future__ import annotations + +import ast +from pathlib import Path + +import pytest + + +THT_ROOT = Path(__file__).resolve().parents[1] / "tht" +DOMAIN_PACKAGES = ("evidence", "memory") + + +def _package(path: Path) -> tuple[str, ...]: + relative = path.relative_to(THT_ROOT).with_suffix("").parts + return ("tht", *relative[:-1]) + + +def _resolve_import_from(package: tuple[str, ...], node: ast.ImportFrom) -> set[str]: + if node.level: + keep = len(package) - (node.level - 1) + base = package[: max(keep, 0)] + else: + base = () + module = (*base, *node.module.split(".")) if node.module else base + resolved = {".".join(module)} if module else set() + if not node.module or node.module == "tht": + resolved.update( + ".".join((*module, alias.name)) + for alias in node.names + if alias.name != "*" + ) + return resolved + + +def _imports(path: Path) -> set[str]: + tree = ast.parse(path.read_text(), filename=str(path)) + package = _package(path) + imported: set[str] = set() + for node in ast.walk(tree): + if isinstance(node, ast.Import): + imported.update(alias.name for alias in node.names) + elif isinstance(node, ast.ImportFrom): + imported.update(_resolve_import_from(package, node)) + return imported + + +def test_relative_import_resolution_reaches_a_sibling_domain() -> None: + imports = ( + "from ..memory import core", + "from .. import memory", + "from tht import memory", + ) + + for statement in imports: + node = ast.parse(statement).body[0] + assert isinstance(node, ast.ImportFrom) + assert "tht.memory" in _resolve_import_from(("tht", "evidence"), node) + + +@pytest.mark.parametrize("domain", DOMAIN_PACKAGES) +def test_python_domain_does_not_import_another_domain(domain: str) -> None: + forbidden = {f"tht.{other}" for other in DOMAIN_PACKAGES if other != domain} + violations: list[str] = [] + + for path in sorted((THT_ROOT / domain).rglob("*.py")): + for imported in sorted(_imports(path)): + if any(imported == root or imported.startswith(f"{root}.") for root in forbidden): + violations.append(f"{path.relative_to(THT_ROOT)} -> {imported}") + + assert violations == [] diff --git a/tools/replay/extract.mjs b/tools/replay/extract.mjs index 657c96c8..64fc42f4 100644 --- a/tools/replay/extract.mjs +++ b/tools/replay/extract.mjs @@ -138,7 +138,7 @@ if (sessionCounts.size > 1) { } const answered = calls.filter((c) => resultsByCallId.has(c.callId)); -// --- build descriptors (mirrors harness/.pi/extensions/gate/builders.js) ---- +// --- build descriptors (mirrors harness/.pi/extensions/gate/core/builders.js) ---- // Infer the workflow phase (F1..F8) of a gate from its title, so the frontend's // WorkflowBar lights up the correct dot during replay. Patterns (from real