fix: auto-approve phase 3 rewrite
This commit is contained in:
@@ -0,0 +1,44 @@
|
|||||||
|
# F3 Rewrite Auto-Approval Implementation Plan
|
||||||
|
|
||||||
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||||
|
|
||||||
|
**Goal:** Remove the F3 reviewer confirmation while preserving the persisted rewrite and normal phase transition.
|
||||||
|
|
||||||
|
**Architecture:** `rewrite_question` owns the ordered document write, decision recording, and phase advance. The F3 skill calls it once, without a reviewer widget.
|
||||||
|
|
||||||
|
**Tech Stack:** JavaScript Pi extension, Python `tht` CLI, Node test runner, pytest.
|
||||||
|
|
||||||
|
## Global Constraints
|
||||||
|
|
||||||
|
- Preserve the eight phase numbers, `question.md`, and `question_rewritten` ledger type.
|
||||||
|
- Never write decisions or advance phases outside F3.
|
||||||
|
- Use `tht` commands with `--session` after the command group.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 1: Test and implement automatic F3 completion
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `harness/.pi/extensions/tht-gate.js`
|
||||||
|
- Test: `harness/.pi/extensions/gate/__tests__/gate_rewrite_auto_approval.test.js`
|
||||||
|
|
||||||
|
- [ ] Add a fake Pi integration test that invokes the registered `rewrite_question` tool at F3 and asserts these calls in order: `session set-question`, `decision add question_rewritten`, `phase advance`.
|
||||||
|
- [ ] Run `cd harness && npm test -- --test-name-pattern="rewrite question"` and observe failure because the current tool only writes the document.
|
||||||
|
- [ ] Make `rewrite_question` reject non-F3 sessions, record the decision after a successful write, advance after a successful decision, and return a no-widget completion message.
|
||||||
|
- [ ] Re-run the focused test and then `cd harness && npm test`.
|
||||||
|
|
||||||
|
### Task 2: Remove the F3 reviewer instruction
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `harness/.pi/skills/tht-sessione/SKILL.md`
|
||||||
|
|
||||||
|
- [ ] Replace the F3 `reviewer_decide` and `reviewer_confirm` procedure with one direct `rewrite_question` call and an explicit prohibition on reviewer widgets.
|
||||||
|
|
||||||
|
### Task 3: Verify workflow compatibility
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Test: `harness/tests/test_workflow.py`
|
||||||
|
- Test: `harness/tests/test_phase_effective.py`
|
||||||
|
|
||||||
|
- [ ] Run `cd harness && .venv/bin/pytest -q tests/test_workflow.py tests/test_phase_effective.py`.
|
||||||
|
- [ ] Run `cd harness && .venv/bin/pytest -q`.
|
||||||
@@ -0,0 +1,15 @@
|
|||||||
|
# F3 Rewrite Auto-Approval Design
|
||||||
|
|
||||||
|
## Goal
|
||||||
|
|
||||||
|
Remove the redundant human confirmation titled "Conferma riscrittura della domanda" while retaining Phase 3, its deterministic `question.md` artifact, and its audit trail.
|
||||||
|
|
||||||
|
## Design
|
||||||
|
|
||||||
|
The existing privileged `rewrite_question` tool becomes the owner of F3 completion. It writes `question.md`, records `question_rewritten` with the full rewritten question, then advances F3. It refuses calls outside F3. The orchestration skill calls this tool directly and must not emit `reviewer_decide` or `reviewer_confirm` in F3.
|
||||||
|
|
||||||
|
If the write fails, it records nothing. If decision recording fails, it does not advance. If the advance fails, the persisted artifact and decision remain for a safe retry. Phase numbers, workflow YAML, artifact paths, decision type, and old sessions remain compatible.
|
||||||
|
|
||||||
|
## Tests
|
||||||
|
|
||||||
|
Gate coverage will verify ordered CLI calls and F3-only protection. The existing Python phase tests will verify the unchanged workflow invariant.
|
||||||
@@ -0,0 +1,83 @@
|
|||||||
|
const test = require("node:test");
|
||||||
|
const assert = require("node:assert");
|
||||||
|
const fs = require("node:fs");
|
||||||
|
const os = require("node:os");
|
||||||
|
const path = require("node:path");
|
||||||
|
const { createFakePi } = require("./fake_pi_runtime.js");
|
||||||
|
|
||||||
|
globalThis.require = require;
|
||||||
|
|
||||||
|
const installGatePromise = import("../../tht-gate.js").then((m) => m.default);
|
||||||
|
|
||||||
|
function setupFakeTht(t, phase) {
|
||||||
|
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "fake-tht-rewrite-"));
|
||||||
|
const log = path.join(dir, "calls.log");
|
||||||
|
fs.writeFileSync(log, "");
|
||||||
|
const bin = path.join(dir, "tht");
|
||||||
|
fs.writeFileSync(
|
||||||
|
bin,
|
||||||
|
`#!/bin/bash
|
||||||
|
echo "$@" >> "${log}"
|
||||||
|
case "$1 $2" in
|
||||||
|
"phase show") echo "Fase corrente: ${phase}" ;;
|
||||||
|
"phase meta") echo '{"max_phase":8,"phases":[{"num":3,"id":"F3","emits":["question_rewritten"]}]}' ;;
|
||||||
|
*) echo "OK" ;;
|
||||||
|
esac
|
||||||
|
`,
|
||||||
|
{ mode: 0o755 },
|
||||||
|
);
|
||||||
|
const oldPath = process.env.PATH;
|
||||||
|
process.env.PATH = `${dir}:${oldPath}`;
|
||||||
|
t.after(() => {
|
||||||
|
process.env.PATH = oldPath;
|
||||||
|
fs.rmSync(dir, { recursive: true, force: true });
|
||||||
|
});
|
||||||
|
return { dir, calls: () => fs.readFileSync(log, "utf8").trim().split("\n").filter(Boolean) };
|
||||||
|
}
|
||||||
|
|
||||||
|
test("rewrite question completes F3 without a reviewer widget", async (t) => {
|
||||||
|
const fake = setupFakeTht(t, 3);
|
||||||
|
const installGate = await installGatePromise;
|
||||||
|
const { pi, ctx, tools } = createFakePi();
|
||||||
|
ctx.cwd = fake.dir;
|
||||||
|
installGate(pi);
|
||||||
|
|
||||||
|
const { def } = tools.get("rewrite_question");
|
||||||
|
const result = await def.execute(
|
||||||
|
"rewrite-1",
|
||||||
|
{ session: "s1", question: "pazienti con ablazione", assumptions: ["anno 2025"] },
|
||||||
|
null,
|
||||||
|
null,
|
||||||
|
ctx,
|
||||||
|
);
|
||||||
|
|
||||||
|
assert.deepEqual(fake.calls(), [
|
||||||
|
"phase show --session s1",
|
||||||
|
"session set-question s1 --question pazienti con ablazione --assumption anno 2025",
|
||||||
|
"decision add --session s1 --type question_rewritten --subject domanda --detail pazienti con ablazione",
|
||||||
|
"phase advance --session s1",
|
||||||
|
"phase meta --json",
|
||||||
|
]);
|
||||||
|
assert.equal(ctx.uiCalls.length, 0);
|
||||||
|
assert.match(result.content[0].text, /Fase 3 completata/);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("rewrite question refuses calls outside F3", async (t) => {
|
||||||
|
const fake = setupFakeTht(t, 4);
|
||||||
|
const installGate = await installGatePromise;
|
||||||
|
const { pi, ctx, tools } = createFakePi();
|
||||||
|
ctx.cwd = fake.dir;
|
||||||
|
installGate(pi);
|
||||||
|
|
||||||
|
const { def } = tools.get("rewrite_question");
|
||||||
|
const result = await def.execute(
|
||||||
|
"rewrite-2",
|
||||||
|
{ session: "s1", question: "pazienti con ablazione" },
|
||||||
|
null,
|
||||||
|
null,
|
||||||
|
ctx,
|
||||||
|
);
|
||||||
|
|
||||||
|
assert.deepEqual(fake.calls(), ["phase show --session s1"]);
|
||||||
|
assert.match(result.content[0].text, /solo in Fase 3/);
|
||||||
|
});
|
||||||
@@ -1266,10 +1266,11 @@ export default function (pi) {
|
|||||||
|
|
||||||
pi.registerTool({
|
pi.registerTool({
|
||||||
name: "rewrite_question",
|
name: "rewrite_question",
|
||||||
label: "Riscrittura domanda (deterministica)",
|
label: "Riscrittura domanda e chiusura F3 (deterministica)",
|
||||||
description:
|
description:
|
||||||
"Scrive deterministicamente question.md via tht session set-question (evita il tool " +
|
"In Fase 3 scrive deterministicamente question.md, registra question_rewritten e " +
|
||||||
"di edit unreliable). assumptions puo' essere array o stringa JSON.",
|
"chiude la fase senza chiedere conferma al reviewer. assumptions puo' essere array " +
|
||||||
|
"o stringa JSON.",
|
||||||
parameters: Type.Object({
|
parameters: Type.Object({
|
||||||
session: Type.String(),
|
session: Type.String(),
|
||||||
question: Type.String(),
|
question: Type.String(),
|
||||||
@@ -1278,6 +1279,10 @@ export default function (pi) {
|
|||||||
async execute(_id, params, _signal, _onUpdate, ctx) {
|
async execute(_id, params, _signal, _onUpdate, ctx) {
|
||||||
lockActive = true;
|
lockActive = true;
|
||||||
const { session, question, assumptions } = params;
|
const { session, question, assumptions } = params;
|
||||||
|
const curNum = currentPhase(ctx, session);
|
||||||
|
if (curNum !== 3) {
|
||||||
|
return textResult("La riscrittura automatica e' disponibile solo in Fase 3.");
|
||||||
|
}
|
||||||
let assumps = assumptions;
|
let assumps = assumptions;
|
||||||
if (typeof assumps === "string") {
|
if (typeof assumps === "string") {
|
||||||
try {
|
try {
|
||||||
@@ -1295,7 +1300,19 @@ export default function (pi) {
|
|||||||
}
|
}
|
||||||
const err = relayIfThtFails(ctx, args, "");
|
const err = relayIfThtFails(ctx, args, "");
|
||||||
if (err) return err;
|
if (err) return err;
|
||||||
return textResult(`Domanda riscritta per la sessione ${session}.`);
|
const decisionErr = relayIfThtFails(
|
||||||
|
ctx,
|
||||||
|
decisionAddArgs(session, {
|
||||||
|
type: "question_rewritten",
|
||||||
|
subject: "domanda",
|
||||||
|
detail: question,
|
||||||
|
}),
|
||||||
|
"",
|
||||||
|
);
|
||||||
|
if (decisionErr) return decisionErr;
|
||||||
|
const advanced = advancePhaseAndFinalize(ctx, session, curNum);
|
||||||
|
if (advanced.err) return advanced.err;
|
||||||
|
return textResult(`Domanda riscritta e Fase 3 completata (sessione ${session}).`);
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -243,15 +243,12 @@ Phase 3 (CLI exit 5).
|
|||||||
terms, each condition as a separate numbered clause, ambiguous terms replaced with
|
terms, each condition as a separate numbered clause, ambiguous terms replaced with
|
||||||
the concepts clarified in Phase 1 citing the defining evidence, expected output
|
the concepts clarified in Phase 1 citing the defining evidence, expected output
|
||||||
made explicit).
|
made explicit).
|
||||||
2. Present in **a single** `reviewer_decide(advance:false, allow_other:true)`. The
|
2. Call `rewrite_question` once with the completed rewritten question and assumptions. The
|
||||||
"Confirm rewriting" option is `recommended:true` with
|
gate writes `question.md` (including `## Assunzioni`), records `question_rewritten`, and
|
||||||
`{type:"question_rewritten", subject:"domanda", detail:"<full rewritten question>"}`.
|
closes F3 automatically.
|
||||||
3. **Order matters:** (a) the `reviewer_decide` records `question_rewritten` → (b) call the
|
3. Do **not** call `reviewer_decide` or `reviewer_confirm` in F3: the rewrite is assumed
|
||||||
gate's `rewrite_question` tool, which runs `tht session set-question` to write
|
approved. Continue at F4 only after `rewrite_question` reports success. To revise the
|
||||||
`question.md` (regenerates question + an "## Assunzioni" section; never edit it by hand)
|
rewrite, use "Torna indietro" to reopen F1.
|
||||||
→ (c) close the phase with `reviewer_confirm kind:"phase"`. F3 does NOT auto-advance:
|
|
||||||
the `question_rewritten` decision alone does not move the phase.
|
|
||||||
4. "Altro" iterates (re-propose a new `reviewer_decide`). "Torna indietro" reopens F1.
|
|
||||||
|
|
||||||
## Phase 4 — Schema linking
|
## Phase 4 — Schema linking
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user