diff --git a/docs/superpowers/specs/2026-07-01-workflow-contract-hardening-design.md b/docs/superpowers/specs/2026-07-01-workflow-contract-hardening-design.md new file mode 100644 index 00000000..e368f7f1 --- /dev/null +++ b/docs/superpowers/specs/2026-07-01-workflow-contract-hardening-design.md @@ -0,0 +1,136 @@ +# Workflow contract hardening — design + +> Date: 2026-07-01 · Status: approved (design) · Scope: harness only (Pi gate + `tht` CLI + SKILL.md) + +## Problem + +Analysis of the last live session (`2026-06-30-165708`, GLM 5.2 — Pi transcript +`~/.pi/agent/sessions/--Users-mp-projects-ThothII-harness--/2026-06-30T16-57-09-634Z_*.jsonl`) +showed the model spending **63 of 77 tool calls** on exploration / reverse-engineering the +harness (reading `tht-gate.js` 3×, `phase.py`, `decision_cmd.py`, `session/models.py`; ~10 +`--help` probes) instead of executing the workflow. Three concrete defects drive this, all +verified against source: + +1. **`SKILL.md` misdescribes phase advancement.** Ground truth: `tht phase advance` (no + `--auto`) writes `phase_approved` unconditionally (`phase_cmd.py:99-105`); `reviewer_decide(advance:true)` + calls `advance --auto`, which advances only when `auto_advance_eligible` — phase ∈ `{2,6}` + **and zero substantive decisions** (`phase.py:_AUTO_ADVANCE_PHASES`, `auto_advance_eligible`). + So every phase that records real decisions (F1, F3, F4, F5, F6-with-CTEs, F7, F8) closes + **only** via `reviewer_confirm kind:"phase"`; `advance:true` effectively auto-advances just + F2-empty / F6-skipped. But `SKILL.md` Phase 3 (lines 170-171) says *"the reviewer_decide + already advances"* and tells the model NOT to add a phase gate — false. This is the exact + cause of the model's thrash at transcript turns 30-38 (question_rewritten recorded, phase + stayed at 3). + +2. **F6 CTE approval is broken (dead-ends the workflow).** The gate's `reviewer_confirm + kind:"cte_result"` registers `cte_approved --subject phase:6` (`tht-gate.js:623-640`), but + `decision_cmd.py:49-70` rejects `cte_approved` unless `subject` is a CTE name in the + persisted plan (`approved_ctes`/`next_cte` key on the CTE name) → **exit 5 every time**. + The last session died here (transcript turn 63). + +3. **`schema_linking.json` has no writer.** In F4 the model hand-writes the artifact against + the internal `SchemaLinking` pydantic model and validates it with ad-hoc `python -c` + one-liners (transcript turns 45-52, including a venv-vs-system-python misfire) because + there is no `tht` command to persist/validate it — unlike `question.md` (`set-question`). + +## Goals + +- Make the F6 CTE-approval path work end-to-end. +- Give the model a `tht`-mediated way to persist+validate `schema_linking.json`. +- Correct `SKILL.md` so it matches the real advance contract, and give the model a per-phase + cheat-sheet so it stops discovering the CLI at runtime. + +## Non-goals + +- No rewrite of the working 8-phase workflow, `phase.py`, or `workflow.yaml` semantics. +- No change to `_AUTO_ADVANCE_PHASES` or the auto-advance eligibility rule (it is intentional: + only trivially-empty phases auto-advance). +- Not touching the F7 `sql` path (`sql_approved --subject phase:7` is correct — no in-plan check). + +## Part 2 — Fix F6 CTE approval (chosen approach: A2, gate derives `next_cte`) + +The CTE being approved by `reviewer_confirm kind:"cte_result"` is always `next_cte` (the +first unapproved CTE in plan order) — `tht cte test ` already enforces `name == next_cte` +(`cte_cmd.py:44-54`). So the gate can derive the name authoritatively instead of trusting a +model-supplied value. + +**Changes** +- `harness/tht/cli/cte_cmd.py`: add `tht cte next --session ` → prints the bare `next_cte` + name on stdout (empty output when all CTEs are approved / no plan). Pristine stdout (machine + contract). Reuses `tht.phase.next_cte`. +- `harness/.pi/extensions/tht-gate.js`, `reviewer_confirm` `kind:"cte_result"` branch: replace + `--subject phase:${currentPhase}` with the name from `tht cte next --session `; register + `decision add --type cte_approved --subject `. If `cte next` is empty, return a + textResult explaining there is no CTE awaiting approval (no-op, should not occur in the normal + plan-order flow). `kind:"sql"` branch unchanged. + +**Tests** +- pytest `tests/test_cte_next.py` (new): `cte next` returns the first unapproved CTE, then the + next after an approval, then empty when all approved. RED→GREEN. +- pytest contract test (extend existing decision/cte test): `decision add --type cte_approved + --subject ` succeeds and `next_cte` advances; `--subject phase:6` exits 5 + (documents the regression the gate previously hit). +- The gate `execute()` glue is L2/live-verified by repo convention (`tht-gate.js:21-24`); the + behavioral guarantee is carried by the two pytest tests above + a deferred live F6 check. + +## Part 3 — `schema_linking.json` writer/validator + +Mirror the `question.md` pattern (`rewrite_question` gate tool → `tht session set-question` → +`store.set_question`). + +`SchemaLinking` shape to document + validate (`session/models.py`, `extra=forbid`): +`question:str`, `candidates:[{kind:"table"|"column", name, evidence?:[str], decision?:"promoted"|"excluded"|"pending", signals?:{}, grounded_values?:[{}]}]`, +`joins:[{from, to, source?, decision?}]` (JSON key is `from`, aliased), +`excluded:[{kind, name}]`, `open_questions:[str]`, `concept_formulas:[{}]`. + +**Changes** +- `harness/tht/session/store.py`: add `set_schema_linking(session_id, data: dict, sessions_path)` + → `SchemaLinking.model_validate(data)` (raises on invalid), then writes `schema_linking.json` + deterministically (`json.dumps(model.model_dump(by_alias=True), indent=2, ensure_ascii=False)`). +- `harness/tht/cli/session_cmd.py`: add `tht session set-schema-linking --file ` + (accepts `-` for stdin). Reads JSON, calls `set_schema_linking`; on `ValidationError` prints + the pydantic error to stderr and exits 5 (so the model gets a precise, actionable message); + on success prints `OK: schema_linking.json aggiornato (...)`. +- `harness/.pi/extensions/tht-gate.js`: add a `write_schema_linking` tool (parallel to + `rewrite_question`): params `{session, schema_linking: object}`; passes the JSON to + `tht session set-schema-linking --file -` via `execFileSync` `input` (stdin, no temp + file); relays validation errors back to the model via `relayIfThtFails`. + +**Tests** +- pytest `tests/test_set_schema_linking.py` (new): valid dict → file written + re-validates; + invalid (extra key / bad `kind`) → `ValidationError` / CLI exit 5, file not written; `from` + alias round-trips. RED→GREEN. + +## Part 1 — `SKILL.md` surgical corrections (done last, reflects Parts 2-3) + +Targeted edits, not a rewrite: +- **Phase 3:** close with `reviewer_confirm kind:"phase"`; delete the "the reviewer_decide + already advances" claim; set the rewriting `reviewer_decide` to `advance:false`. Keep the + order (decide records `question_rewritten` → `rewrite_question` writes `question.md`). +- **Discipline 2:** restate the advance rule accurately — every substantive phase closes with + `reviewer_confirm kind:"phase"` (F7: `kind:"sql"`); `advance:true` auto-advances ONLY F2 when + memory recorded nothing and F6 when skipped/empty; never rely on it elsewhere. +- **Phase 1 step 4 vs Phase 3:** disambiguate where `rewrite_question` is invoked (it belongs to + the Phase 3 flow after `question_rewritten`; the Phase 1 mention is removed/redirected). +- **Phase 4:** replace "write `schema_linking.json`" with "call `write_schema_linking`"; add the + documented `SchemaLinking` shape so the model does not read `session/models.py`. +- **Phase 6:** keep "approve each CTE in plan order via `reviewer_confirm kind:"cte_result"`"; + no signature change (A2 derives the name), close F6 with `reviewer_confirm kind:"phase"`. +- **Add a top cheat-sheet table:** one row per phase → {artifact out, closing gate}. This is the + state-machine at a glance that the model currently lacks. + +## Build order & testing + +Order: **Part 2 → Part 3 → Part 1** (SKILL documents the final contract). TDD on Parts 2-3 +(pytest RED→GREEN; run `cd harness && .venv/bin/pytest -q` and the gate suite +`node --test` under `.pi/extensions/gate/__tests__` where applicable). Part 1 is documentation; +verify by re-reading against the corrected contract. Full live F6/F4 verification through the +real stack is deferred (needs VPN — see PROJECT_STATE open items). + +## Risks + +- **Gate glue is not unit-tested** (repo convention): the F6 and `write_schema_linking` wiring + in `tht-gate.js` rely on pytest CLI contracts + a deferred live check. Mitigation: keep the + gate changes minimal and push all logic behind the CLI (which IS tested). +- **`--file -` / stdin handling** in the CLI must be covered by a test (empty stdin, invalid + JSON) so the gate path can't hang or write a partial file.