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>
This commit is contained in:
Claude Opus 4.8
@@ -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 <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.
|
||||
Reference in New Issue
Block a user