mptyl/ThothII
1
0
Fork 0
Code Issues 25 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix: realign workflow advance semantics and phase labels with workflow.yaml

Browse Source 
Audit findings 2.1 + 2.2 (high).

2.1 WorkflowBar's static phase list was fiction from F2 on (F2 "Schema
linking" vs memoria, F4 "SQL plan" vs schema_linking, …): every live
session showed the wrong phase name. Both maps now mirror
harness/workflow.yaml (F1 chiarimento … F8 datamart).

2.2 forceAdvance (6ee5bda) let reviewer_decide advance:true bypass the
phase gate on ANY phase, contradicting SKILL.md's "auto-advance only
empty F2 / skipped F6". reviewer_decide is back on advanceIfReady (exit-6
no-op) and tells the model to close via reviewer_confirm; forceAdvance
stays only where selection IS the approval by design: reviewer_schema_linking
(F4) and the F8 promotion close path. SKILL.md now names the three
self-closing gates (F3 rewrite_question, F4 schema-linking advance:true,
F8 memory_promote) so gate and skill state one contract.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
marcopan
2026-07-20 01:28:18 +02:00
co-authored by Claude Fable 5
parent 075883370e
commit 3e072fe652
3 changed files with 39 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/shell/WorkflowBar.tsx
+18 -15
View File
@@ -2,15 +2,18 @@
import { useSessionStore } from "../store/sessionStore";
import { ElapsedTimer } from "./ElapsedTimer";
// Mirror of harness/workflow.yaml phase ids/names (F1 chiarimento … F8 datamart).
// Keep the two lists in lockstep: the harness is the single source of phase truth,
// and a drifted label here mislabels every live session from that phase on.
const PHASES = [
{ id: "F1", name: "Disambiguation" },
{ id: "F2", name: "Schema linking" },
{ id: "F3", name: "Exploration" },
{ id: "F4", name: "SQL plan" },
{ id: "F5", name: "SQL generation" },
{ id: "F6", name: "Validation" },
{ id: "F7", name: "Review" },
{ id: "F8", name: "Results" },
{ id: "F1", name: "Clarification" },
{ id: "F2", name: "Memory" },
{ id: "F3", name: "Rewrite" },
{ id: "F4", name: "Schema linking" },
{ id: "F5", name: "Plan" },
{ id: "F6", name: "CTE build" },
{ id: "F7", name: "Final SQL" },
{ id: "F8", name: "Datamart" },
];
// Synthetic, human-readable title for the current phase, shown under the dots.
@@ -19,13 +22,13 @@ const PHASES = [
const PHASE_LANG: "en" | "it" = "en";
const PHASE_TITLES: Record<string, { en: string; it: string }> = {
F1: { en: "Clarifying the question", it: "Chiarimento della domanda" },
F2: { en: "Linking the schema", it: "Collegamento dello schema" },
F3: { en: "Exploring the data", it: "Esplorazione dei dati" },
F4: { en: "Planning the SQL", it: "Pianificazione della query" },
F5: { en: "Generating the SQL", it: "Generazione della query" },
F6: { en: "Validating the results", it: "Validazione dei risultati" },
F7: { en: "Reviewing with you", it: "Revisione con te" },
F8: { en: "Presenting results", it: "Presentazione dei risultati" },
F2: { en: "Recalling relevant memory", it: "Recupero delle memorie utili" },
F3: { en: "Rewriting the question", it: "Riscrittura della domanda" },
F4: { en: "Linking the schema", it: "Collegamento dello schema" },
F5: { en: "Planning the SQL", it: "Pianificazione della query" },
F6: { en: "Building the CTEs", it: "Costruzione delle CTE" },
F7: { en: "Finalizing the SQL", it: "Finalizzazione della query" },
F8: { en: "Building the datamart", it: "Costruzione del datamart" },
};
type DotState = "done" | "running" | "error" | "pending";
harness/.pi/extensions/tht-gate.js
+12 -5
View File
@@ -367,9 +367,10 @@ 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.
// Unconditional phase advance — the reviewer already approved via the widget interaction.
// ONLY for gates whose selection IS the phase approval by design: reviewer_schema_linking
// (F4 curation) and the F8 close path. Generic reviewer_decide must use advanceIfReady —
// forcing there bypasses the reviewer_confirm phase gate and contradicts SKILL.md.
function forceAdvance(ctx, session) {
try {
tht(ctx, ["phase", "advance", "--session", session]);
@@ -891,9 +892,15 @@ export default function (pi) {
const msg = "Nessuna decisione registrata (il reviewer non ha selezionato opzioni di merito).";
return textResult(adv.advanced ? msg + " Fase avanzata automaticamente." : msg);
}
const adv = advance ? forceAdvance(ctx, session) : { advanced: false };
// SKILL.md contract: advance:true means auto-advance-IF-ELIGIBLE (empty F2 /
// skipped F6), never a bypass of the phase gate once substantive decisions
// exist. The only gates that close their phase on selection are
// reviewer_schema_linking (F4) and reviewer_memory_promote (F8).
const adv = advance ? advanceIfReady(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.");
parts.push(adv.advanced
? "Fase avanzata automaticamente."
: 'La fase resta aperta: chiudila con reviewer_confirm kind:"phase" quando pronta.');
return textResult(parts.join(" "));
} catch (fatal) {
const msg = (fatal.stderr || fatal.message || String(fatal)).toString().trim();
harness/.pi/skills/tht-sessione/SKILL.md
+9 -6
View File
@@ -29,15 +29,17 @@ A phase advances ONLY when a `phase_approved:phase:N` decision is recorded for t
phase — written by `reviewer_confirm kind:"phase"`. (F7 is two-step: `kind:"sql"` records
`sql_approved`, then `kind:"phase"` advances.) A `reviewer_decide`/
`reviewer_select` choice records its OWN decision but does NOT advance the phase. `advance:true`
auto-advances only F2 (empty memory) and F6 (skipped/empty) — never a phase that recorded
substantive decisions.
on `reviewer_decide` auto-advances only F2 (empty memory) and F6 (skipped/empty) — never a
phase that recorded substantive decisions. Exactly three gates close their phase themselves,
because there the human interaction IS the phase approval: `rewrite_question` (F3),
`reviewer_schema_linking` with `advance:true` (F4), and `reviewer_memory_promote` (F8).
| Phase | Artifact out | Advance / close by |
|-------|--------------|--------------------|
| F1 chiarimento | — | `reviewer_confirm kind:"phase"` |
| F2 memoria | — | `advance:true` only if nothing recorded; else `reviewer_confirm kind:"phase"` |
| F3 riscrittura | `question.md` | `rewrite_question` records approval and advances automatically |
| F4 schema_linking | `schema_linking.json` | `reviewer_confirm kind:"phase"` (after `reviewer_schema_linking` + `write_schema_linking`). Promoted columns are the reviewer-approved OUTPUT columns — project exactly those in the final SELECT. |
| F4 schema_linking | `schema_linking.json` | `reviewer_schema_linking` with `advance:true` closes the phase itself (the curation IS the approval; `reviewer_confirm kind:"phase"` only as fallback if it reports an error). Promoted columns are the reviewer-approved OUTPUT columns — project exactly those in the final SELECT. |
| F5 sintesi | — | `reviewer_confirm kind:"phase"` (after `tht session check`) |
| F6 cte | `cte_plan.json`, `ctes/`, `cte_tests.json` | approve each CTE `kind:"cte_result"`, then `reviewer_confirm kind:"phase"` |
| F7 sql_finale | `sql_final.sql` | `kind:"sql"` records `sql_approved`, then `reviewer_confirm kind:"phase"` |
@@ -56,9 +58,10 @@ substantive decisions.
`kind:"phase"` to advance), the deliberate "this phase is
done" gate. The `advance:true` flag on `reviewer_decide` is a shortcut that auto-advances
ONLY F2 when the memory phase recorded nothing and F6 when it is skipped/empty; everywhere
else it is a silent no-op, so never rely on it to advance. Do NOT add a `reviewer_confirm`
that merely echoes a decision already recorded by a choice — the phase gate is a separate,
deliberate step, not an echo of a decision.
else it is a silent no-op, so never rely on it to advance. The self-closing gates are the
three listed above (F3 `rewrite_question`, F4 `reviewer_schema_linking` `advance:true`,
F8 `reviewer_memory_promote`) — after one of those, do NOT add a `reviewer_confirm` that
merely echoes it; the phase is already closed.
3. **Single pick vs multi-answer.** For a single-pick clarification or decision, use
`reviewer_select` and attach a `decision` payload (`{type, subject, detail?,
rationale?}`) to each concrete option: picking it persists that decision directly —