Files
ThothII/docs/superpowers/specs/2026-07-01-workflow-contract-hardening-design.md
T
marcopanandClaude Opus 4.8 d5af7b5d65 docs(spec): workflow contract hardening (F3 advance, F6 cte_approved, schema_linking writer)
Design doc for three coordinated harness fixes derived from the
2026-06-30-165708 session analysis: correct SKILL.md phase-advance
contract, fix the F6 cte_approved subject bug, and add a
tht-mediated schema_linking.json writer/validator.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 16:19:09 +02:00

137 lines
8.4 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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 <name>` 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 <id>` → 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 <id>`; register
`decision add --type cte_approved --subject <name>`. 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 <valid CTE name>` 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 <id> --file <path>`
(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 <id> --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.