From 6ee5bda7f0c2f512a5b8deac9512d21df53d2dcc Mon Sep 17 00:00:00 2001 From: mptyl Date: Sun, 19 Jul 2026 14:14:44 +0200 Subject: [PATCH] feat: gate decision-type validation, force-advance, and frontend fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gate (tht-gate.js): - Pre-validate decision types against workflow.yaml before showing reviewer widget - Reject decisions emitted by later phases (min-phase check) - Copy top-level `kind` into artifact when model forgets it (prevents loop) - Force-advance on reviewer_decide/schema_linking when advance:true — skip redundant reviewer_confirm gate Backend: - Emit agent_end on clean Pi exit (code 0 + bridge idle) instead of marking failed Frontend: - Strip tags from transcript and activity panel - Fix mermaid render with offscreen container + cleanup - Graceful mermaid error: show source code instead of red error, fall back to table Workflow: - F2 now emits table_promoted and table_excluded (early schema linking decisions) Co-Authored-By: Claude Opus 4.6 --- backend/src/pi/pi-process-manager.ts | 8 ++ frontend/src/shell/CentralStatus.tsx | 2 + frontend/src/shell/ModelActivityPanel.tsx | 2 + frontend/src/viewers/MarkdownView.tsx | 2 +- frontend/src/viewers/SchemaLinkingViewer.tsx | 6 +- frontend/src/viewers/mermaid.ts | 13 ++- harness/.pi/extensions/tht-gate.js | 93 ++++++++++++++++++-- harness/tests/test_decision_min_phase.py | 2 +- harness/workflow.yaml | 2 +- 9 files changed, 114 insertions(+), 16 deletions(-) diff --git a/backend/src/pi/pi-process-manager.ts b/backend/src/pi/pi-process-manager.ts index d7be6c22..8827927d 100644 --- a/backend/src/pi/pi-process-manager.ts +++ b/backend/src/pi/pi-process-manager.ts @@ -122,6 +122,14 @@ export class PiProcessManager { child.on("exit", (code) => { console.error(`[pi:${sessionId}] exited code=${code ?? "?"} mapped=${this.runtimes.get(sessionId) === runtime}`); if (this.runtimes.get(sessionId) === runtime) { + // Clean exit (code 0) while the bridge is idle means the model completed its + // turn and Pi shut down normally (e.g. after session finalization). Just clean + // up — no failure events. + if (code === 0 && runtime.bridge.turnState() === "idle") { + this.runtimes.delete(sessionId); + runtime.bridge.emitClientEvent({ type: "system_event", event: "agent_end" }); + return; + } runtime.bridge.markFailed(); this.runtimes.delete(sessionId); runtime.bridge.emitClientEvent({ diff --git a/frontend/src/shell/CentralStatus.tsx b/frontend/src/shell/CentralStatus.tsx index 393ef00c..5879c117 100644 --- a/frontend/src/shell/CentralStatus.tsx +++ b/frontend/src/shell/CentralStatus.tsx @@ -5,6 +5,8 @@ import { isNearBottom } from "./activityScroll"; function transcriptLines(transcript: Array<{ text: string }>): string[] { return transcript.flatMap(({ text }) => text + .replace(/[\s\S]*?<\/think>/g, "") + .replace(/<\/?think>/g, "") .split("\n") .map((line) => line.trimEnd()) .filter((line) => line.trim() !== ""), diff --git a/frontend/src/shell/ModelActivityPanel.tsx b/frontend/src/shell/ModelActivityPanel.tsx index b56c839f..5bfb3e56 100644 --- a/frontend/src/shell/ModelActivityPanel.tsx +++ b/frontend/src/shell/ModelActivityPanel.tsx @@ -25,6 +25,8 @@ export function isVisibleModelActivity(entry: ActivityEntry): boolean { export function formatModelActivity(text: string): string { return text + .replace(/[\s\S]*?<\/think>/g, "") + .replace(/<\/?think>/g, "") .replace(/\r\n?/g, "\n") .replace(/([.!?])(?=[A-ZÀ-ÖØ-Þ])/g, "$1\n\n") .replace(/^[\t ]*[•‣–]\s+/gm, "- ") diff --git a/frontend/src/viewers/MarkdownView.tsx b/frontend/src/viewers/MarkdownView.tsx index a4a78e82..50edcc4a 100644 --- a/frontend/src/viewers/MarkdownView.tsx +++ b/frontend/src/viewers/MarkdownView.tsx @@ -16,7 +16,7 @@ function MermaidBlock({ code }: { code: string }) { ); }, [code]); - if (error) return 
{error}
; + if (error) return 
{code}
; if (!svg) return 
{code}
; // eslint-disable-next-line react/no-danger return
; diff --git a/frontend/src/viewers/SchemaLinkingViewer.tsx b/frontend/src/viewers/SchemaLinkingViewer.tsx index dc741004..6f7f1a34 100644 --- a/frontend/src/viewers/SchemaLinkingViewer.tsx +++ b/frontend/src/viewers/SchemaLinkingViewer.tsx @@ -116,9 +116,9 @@ export function SchemaLinkingViewer({ linking }: { linking: SchemaLinking }) { if (view !== "chart" || oversized) return; let cancelled = false; const def = buildErDiagram(promoted, linking.joins); - renderMermaid(def).then((s) => { - if (!cancelled) setSvg(s); - }); + renderMermaid(def) + .then((s) => { if (!cancelled) setSvg(s); }) + .catch(() => { if (!cancelled) setView("table"); }); return () => { cancelled = true; }; diff --git a/frontend/src/viewers/mermaid.ts b/frontend/src/viewers/mermaid.ts index c81cad00..7288eda9 100644 --- a/frontend/src/viewers/mermaid.ts +++ b/frontend/src/viewers/mermaid.ts @@ -7,6 +7,15 @@ export async function renderMermaid(def: string): Promise { _initialized = true; } const id = `mermaid-${Math.random().toString(36).slice(2)}`; - const { svg } = await mermaid.render(id, def); - return svg; + const container = document.createElement("div"); + container.style.position = "absolute"; + container.style.left = "-9999px"; + document.body.appendChild(container); + try { + const { svg } = await mermaid.render(id, def, container); + return svg; + } finally { + container.remove(); + document.getElementById(id)?.remove(); + } } diff --git a/harness/.pi/extensions/tht-gate.js b/harness/.pi/extensions/tht-gate.js index 357af984..ce91a039 100644 --- a/harness/.pi/extensions/tht-gate.js +++ b/harness/.pi/extensions/tht-gate.js @@ -129,6 +129,11 @@ export function prepareReviewerArguments(input) { if (args.artifact && typeof args.artifact === "object" && args.artifact.data !== undefined) { args.artifact = { ...args.artifact, data: jsonObjectOrSelf(args.artifact.data) }; } + // Models frequently forget artifact.kind while providing it at top level — + // copy it over so validation doesn't loop. + if (args.artifact && typeof args.artifact === "object" && !args.artifact.kind && args.kind) { + args.artifact = { ...args.artifact, kind: args.kind }; + } return args; } @@ -275,6 +280,52 @@ function phaseId(ctx, num) { const p = meta.phases.find((x) => x.num === num); return p ? p.id : "?"; } +// Known decision types from workflow.yaml emits + meta types. Pre-validates decision +// payloads BEFORE showing the reviewer widget so a typo in the type name doesn't waste +// a human interaction (the CLI would reject it only after the reviewer has already picked). +let _knownTypes = null; +function knownDecisionTypes(ctx) { + if (_knownTypes) return _knownTypes; + const meta = phaseMeta(ctx); + const types = new Set([ + "phase_approved", "phase_auto_approved", "phase_reopened", + "phase_skipped", "decision_retracted", + ]); + for (const p of meta.phases) { + for (const t of (p.emits || [])) types.add(t); + } + _knownTypes = types; + return types; +} +function decisionMinPhaseMap(ctx) { + const meta = phaseMeta(ctx); + const mins = {}; + for (const p of meta.phases) { + for (const t of (p.emits || [])) { + if (!(t in mins) || p.num < mins[t]) mins[t] = p.num; + } + } + return mins; +} +function validateDecisionTypes(ctx, options, session) { + const known = knownDecisionTypes(ctx); + for (const o of options) { + if (o.decision && !known.has(o.decision.type)) { + return `Tipo di decisione '${o.decision.type}' non valido. Tipi ammessi: ${[...known].join(", ")}. Correggi e riprova.`; + } + } + if (session) { + const cur = currentPhase(ctx, session); + const mins = decisionMinPhaseMap(ctx); + for (const o of options) { + if (o.decision && mins[o.decision.type] && mins[o.decision.type] > cur) { + return `Tipo '${o.decision.type}' ammesso dalla Fase ${mins[o.decision.type]}, sessione alla Fase ${cur}. Chiudi prima la fase corrente.`; + } + } + } + return null; +} + // Full phase descriptor {id, num, name} for the v2 phase-summary payload's `phase`. function phaseMetaForNum(ctx, num) { const meta = phaseMeta(ctx); @@ -316,6 +367,21 @@ function advanceIfReady(ctx, session) { } } +// Unconditional phase advance — the reviewer already approved via the widget interaction +// (reviewer_decide selection IS the human confirmation). Used when reviewer_decide persists +// substantive decisions with advance:true, so no separate reviewer_confirm gate is needed. +function forceAdvance(ctx, session) { + try { + tht(ctx, ["phase", "advance", "--session", session]); + return { advanced: true }; + } catch (e) { + return { + advanced: false, + error: (e.stderr || e.message || String(e)).toString().trim(), + }; + } +} + // --- widget emission + wait — usa l'API UI NATIVA di Pi (ctx.ui.input) -------- // // emitAndWait(ctx, descriptor) sends the widget-descriptor as JSON in the `title` @@ -662,6 +728,8 @@ export default function (pi) { async execute(_id, params, _signal, _onUpdate, ctx) { lockActive = true; const { session, title, options: opts, intro, advance } = params; + const typeErr = validateDecisionTypes(ctx, opts, session); + if (typeErr) return textResult(typeErr); const phase = phaseId(ctx, currentPhase(ctx, session)); const recommended = opts.find((o) => o.recommended)?.id ?? null; @@ -734,6 +802,8 @@ export default function (pi) { async execute(_id, params, _signal, _onUpdate, ctx) { lockActive = true; const { session, title, options: opts, advance } = params; + const typeErr = validateDecisionTypes(ctx, opts, session); + if (typeErr) return textResult(typeErr); const phase = phaseId(ctx, currentPhase(ctx, session)); const toAdd = []; const meritOptions = opts @@ -813,12 +883,17 @@ export default function (pi) { toAdd.push(d); } } - if (advance) advanceIfReady(ctx, session); - return textResult( - toAdd.length - ? `Registrate ${toAdd.length} decisioni: ${toAdd.map((d) => d.type).join(", ")}.` - : "Nessuna decisione registrata (il reviewer non ha selezionato opzioni di merito).", - ); + if (!toAdd.length) { + const adv = advance ? advanceIfReady(ctx, session) : { advanced: false }; + const msg = "Nessuna decisione registrata (il reviewer non ha selezionato opzioni di merito)."; + return textResult(adv.advanced ? msg + " Fase avanzata automaticamente." : msg); + } + // Decisions were recorded AND advance requested: the reviewer's selection IS the + // approval — force-advance without a separate reviewer_confirm gate. + const adv = advance ? forceAdvance(ctx, session) : { advanced: false }; + const parts = [`Registrate ${toAdd.length} decisioni: ${toAdd.map((d) => d.type).join(", ")}.`]; + if (adv.advanced) parts.push("Fase avanzata automaticamente — nessun gate aggiuntivo necessario."); + return textResult(parts.join(" ")); }, }); @@ -924,8 +999,10 @@ export default function (pi) { const eSync = relayIfThtFails(ctx, ["session", "sync-schema-linking", session], ""); if (eSync) return eSync; - if (advance) advanceIfReady(ctx, session); - return textResult(`Schema linking registrato dal reviewer (${n} tabelle + colonne curate).`); + const adv = advance ? forceAdvance(ctx, session) : { advanced: false }; + const parts = [`Schema linking registrato dal reviewer (${n} tabelle + colonne curate). schema_linking.json scritto.`]; + if (adv.advanced) parts.push("Fase avanzata automaticamente — nessun gate aggiuntivo necessario."); + return textResult(parts.join(" ")); }, }); diff --git a/harness/tests/test_decision_min_phase.py b/harness/tests/test_decision_min_phase.py index 5b3fe112..df05e45c 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", 4), + ("table_promoted", 2), ("column_corrected", 4), ("evidence_accepted", 4), ("value_grounded", 4), diff --git a/harness/workflow.yaml b/harness/workflow.yaml index 8303b5c2..9d8a3a79 100644 --- a/harness/workflow.yaml +++ b/harness/workflow.yaml @@ -21,7 +21,7 @@ phases: advance: auto_if_empty prerequisites: [] artifacts_out: [] - emits: [memory_rejected] + emits: [memory_rejected, table_promoted, table_excluded] - id: F3 name: riscrittura advance: kind:phase