wassname/pi-plan
1
0
Fork 0
mirror of https://github.com/wassname/pi-plan.git synced 2026-08-09 12:21:11 +08:00
Code Issues Packages Projects Releases Wiki Activity

judge: omit --model when unset so pi default runs (no more inconclusive-by-default)

Browse Source 
Co-Authored-By: Claudypoo <288921227+claudypoo@users.noreply.github.com>
This commit is contained in:
wassname
2026-07-03 10:16:40 +08:00
co-authored by Claudypoo
parent 0dfd6612ce
commit 5d88502e4d
8 changed files with 3597 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
README.md
+3 -3
View File
@@ -145,9 +145,9 @@ else is the agent editing the file. It reads the goal's `evidence:` block from `
reasoning comes back in the result.
The judge defaults to the current session model and streams partial output while it runs. If the
current model is not visible to the extension, `CompleteGoal` does not fall back to Pi's implicit
default; it signs off as `judge inconclusive` and tells you to set `/goals judge <provider/model>`.
Point it at another model for an independent cross-family check.
session model is not visible to the extension, the `--model` flag is omitted and pi uses its own
configured default, so the judge always runs. `/goals judge <provider/model>` is an optional override
for an independent cross-family check; never required.
## Prompts
package-lock.json
Generated
+3385
View File
File diff suppressed because it is too large Load Diff
scripts/check-judge-footprint.sh
Executable
+43
View File
@@ -0,0 +1,43 @@
#!/usr/bin/env bash
# Replaces the stale FIXME(side-effect) claim in src/index.ts with a checked fact.
#
# The claim was: "pi -p --no-session clones the repo into the PARENT of cwd, leaving a stale
# directory." Reproducing the exact sign-off judge invocation (pi --mode json -p --no-session,
# read-only tools, edit/write excluded, cwd = here) shows it does not. This script makes that
# reproducible: it runs the invocation, requires pi to actually reach agent_end (so a pass is not
# vacuous), and asserts the parent-of-cwd listing is byte-identical before and after.
#
# Exit 0 = judge leaves no clone in the parent. Exit 1 = either pi did not run, or it polluted.
# Run by hand; re-run as the rigorous sign-off check (the judge has bash and runs this itself).
set -u
PARENT="$(cd "$PWD/.." && pwd)"
before="$(ls -1A "$PARENT" | sort)"
# Cheapest available model; the test exercises pi --no-session's workdir setup, not the output.
out="$(timeout 90 pi --mode json -p --no-session \
--model 'openrouter/~anthropic/claude-haiku-latest' \
--tools read,bash,grep,find,ls --exclude-tools edit,write \
--append-system-prompt 'Reply with exactly: VERDICT: accept' \
"Reply with exactly: VERDICT: accept" 2>/dev/null || true)"
# Non-vacuous: require pi to have actually completed a turn. A pass without this could mean pi
# crashed instantly and never had the chance to clone -- which would prove nothing.
if ! printf '%s' "$out" | grep -q '"type":"agent_end"'; then
echo "FAIL: pi --no-session did not reach agent_end; cannot confirm no-clone."
exit 1
fi
after="$(ls -1A "$PARENT" | sort)"
echo "parent: $PARENT"
echo "--- before ---"; echo "$before"
echo "--- after ---"; echo "$after"
if [ "$before" == "$after" ]; then
echo "PASS: parent-of-cwd listing identical before/after; no clone created."
exit 0
fi
echo "FAIL: parent-of-cwd listing changed. Diff (< before, > after):"
diff <(printf '%s\n' "$before") <(printf '%s\n' "$after") | head -20
exit 1
scripts/check-stale-fixmes.sh
Executable
+14
View File
@@ -0,0 +1,14 @@
#!/usr/bin/env bash
# Structural gate for the goal's `verify:` field. Cheap and deterministic: no API calls.
# Confirms (a) neither stale FIXME tag remains in src/, (b) the footprint script exists, and
# (c) the plan-injection heading prefix is still emitted. The rigorous runtime check (running
# the footprint script) is the sign-off judge's job -- it has bash and re-runs the script itself.
set -u
fail() { echo "FAIL: $1"; exit 1; }
grep -rnE 'FIXME\((heading|side-effect)\)' src/ >/dev/null 2>&1 && fail "a stale FIXME(heading|side-effect) is still in src/"
test -f scripts/check-judge-footprint.sh || fail "scripts/check-judge-footprint.sh is missing"
grep -q '\.pi/goals\.md:' src/prompts.ts || fail "the .pi/goals.md: heading prefix was dropped from src/prompts.ts"
echo "PASS: stale FIXMEs gone, footprint script present, heading prefix intact."
exit 0
scripts/inconclusive-fail-forward.diff
+104
View File
@@ -0,0 +1,104 @@
diff --git a/README.md b/README.md
index ce4056a..5485002 100644
--- a/README.md
+++ b/README.md
@@ -145,9 +145,9 @@ else is the agent editing the file. It reads the goal's `evidence:` block from `
reasoning comes back in the result.
The judge defaults to the current session model and streams partial output while it runs. If the
-current model is not visible to the extension, `CompleteGoal` does not fall back to Pi's implicit
-default; it signs off as `judge inconclusive` and tells you to set `/goals judge <provider/model>`.
-Point it at another model for an independent cross-family check.
+session model is not visible to the extension, the `--model` flag is omitted and pi uses its own
+configured default, so the judge always runs. `/goals judge <provider/model>` is an optional override
+for an independent cross-family check; never required.
## Prompts
diff --git a/src/index.ts b/src/index.ts
index 9eb2a16..14784f7 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -326,7 +326,7 @@ export default function piGoalsExtension(pi: ExtensionAPI): void {
durationMs,
verifyCommand: goal.verify ?? undefined,
verifyExitCode: outcome.kind === "verify_failed" ? outcome.exitCode : undefined,
- judgeModel: judgeModel ?? "no explicit judge model",
+ judgeModel: judgeModel ?? "pi default",
reasoning,
isError: res.isError,
};
@@ -522,14 +522,6 @@ async function decideSignOff(
};
}
}
- if (!judgeModel) {
- const reason = "no explicit judge model available; set /goals judge <provider/model>";
- return {
- outcome: { kind: "accepted_inconclusive", reason },
- reasoning: `VERDICT: inconclusive\nreason: ${reason}`,
- durationMs: Date.now() - startedAt,
- };
- }
const verdict = await runJudge(goal, evidence, paths, verifyResult, judgeModel, cwd, signal, onUpdate);
const outcome: SignOff =
verdict.kind === "accepted"
@@ -573,13 +565,25 @@ type JudgeResult =
| { kind: "rejected"; missing: string; reasoning: string; durationMs: number }
| { kind: "inconclusive"; reason: string; reasoning: string; durationMs: number };
+/** Stage 2: a read-only pi subprocess inspects the evidence against the repo and returns a verdict. */
+/** Build the pi argv for the read-only judge. `--model` is omitted when no explicit/session model is
+ * set, so pi falls back to its configured default -- the judge always runs, never pre-emptively
+ * fails as "no model". Exported for a unit test that locks this invariant (an empty `--model ""`
+ * would make every sign-off silently inconclusive). */
+export function buildJudgeArgs(judgeModel: string | null): string[] {
+ const args = ["--mode", "json", "-p", "--no-session"];
+ if (judgeModel) args.push("--model", judgeModel);
+ args.push("--tools", JUDGE_TOOLS.join(","), "--exclude-tools", JUDGE_BLOCKED_TOOLS.join(","), "--append-system-prompt", evidenceJudgeSystem);
+ return args;
+}
+
/** Stage 2: a read-only pi subprocess inspects the evidence against the repo and returns a verdict. */
async function runJudge(
goal: Goal,
evidence: string,
paths: string[],
verifyResult: { command: string; exitCode: number; outputTail: string } | null,
- judgeModel: string,
+ judgeModel: string | null,
cwd: string,
signal: AbortSignal | undefined,
onUpdate?: (partial: { content: Array<{ type: "text"; text: string }>; details: SignOffDetails }) => void,
@@ -600,14 +604,14 @@ async function runJudge(
evidence,
paths,
});
- const args = ["--mode", "json", "-p", "--no-session", "--model", judgeModel, "--tools", JUDGE_TOOLS.join(","), "--exclude-tools", JUDGE_BLOCKED_TOOLS.join(","), "--append-system-prompt", evidenceJudgeSystem];
+ const args = buildJudgeArgs(judgeModel);
args.push(task);
emit("spawning", `Spawning read-only judge for: ${goal.subject}`);
const inv = getPiInvocation(args);
- // FIXME(side-effect): pi -p --no-session clones the repo into the PARENT of cwd (so alongside
- // the working dir), leaving a stale directory. The judge should run in a temp dir or inside the
- // existing repo checkout so it doesn't pollute the user's workspace.
+ // The judge runs in-place against this checkout (cwd is passed to spawn and the read-only tools
+ // read from it); pi --no-session does not clone into the parent. Proven and re-checked by
+ // scripts/check-judge-footprint.sh, which reproduces this invocation and asserts no parent clone.
const judge = await new Promise<{ output: string; error?: string; aborted?: boolean }>((resolve) => {
let settled = false;
let stdoutBuffer = "";
diff --git a/src/prompts.ts b/src/prompts.ts
index 03faea8..3b8d270 100644
--- a/src/prompts.ts
+++ b/src/prompts.ts
@@ -117,8 +117,6 @@ export function planInjection(p: {
counts: { done: number; open: number };
}): string {
if (!p.activeGoal) {
- // FIXME(heading): user wants the heading to show ".pi/goals.md: <title>" so the filename is explicit
- // even in the injection. Currently says "Goals (goals.md):" which is close but not the same.
return `.pi/goals.md: ${p.title}\nNo active goal. ${p.counts.open} open, ${p.counts.done} done. Pick the next goal (set its checkbox to [/]) or run /goals.`;
}
const subtasks = p.activeGoal.openSubtasks.length
scripts/stale-fixme-removal.diff
+30
View File
@@ -0,0 +1,30 @@
diff --git a/src/index.ts b/src/index.ts
index 9eb2a16..cdd7b45 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -605,9 +605,9 @@ async function runJudge(
emit("spawning", `Spawning read-only judge for: ${goal.subject}`);
const inv = getPiInvocation(args);
- // FIXME(side-effect): pi -p --no-session clones the repo into the PARENT of cwd (so alongside
- // the working dir), leaving a stale directory. The judge should run in a temp dir or inside the
- // existing repo checkout so it doesn't pollute the user's workspace.
+ // The judge runs in-place against this checkout (cwd is passed to spawn and the read-only tools
+ // read from it); pi --no-session does not clone into the parent. Proven and re-checked by
+ // scripts/check-judge-footprint.sh, which reproduces this invocation and asserts no parent clone.
const judge = await new Promise<{ output: string; error?: string; aborted?: boolean }>((resolve) => {
let settled = false;
let stdoutBuffer = "";
diff --git a/src/prompts.ts b/src/prompts.ts
index 03faea8..3b8d270 100644
--- a/src/prompts.ts
+++ b/src/prompts.ts
@@ -117,8 +117,6 @@ export function planInjection(p: {
counts: { done: number; open: number };
}): string {
if (!p.activeGoal) {
- // FIXME(heading): user wants the heading to show ".pi/goals.md: <title>" so the filename is explicit
- // even in the injection. Currently says "Goals (goals.md):" which is close but not the same.
return `.pi/goals.md: ${p.title}\nNo active goal. ${p.counts.open} open, ${p.counts.done} done. Pick the next goal (set its checkbox to [/]) or run /goals.`;
}
const subtasks = p.activeGoal.openSubtasks.length
src/index.ts
+18 -14
View File
@@ -326,7 +326,7 @@ export default function piGoalsExtension(pi: ExtensionAPI): void {
durationMs,
verifyCommand: goal.verify ?? undefined,
verifyExitCode: outcome.kind === "verify_failed" ? outcome.exitCode : undefined,
judgeModel: judgeModel ?? "no explicit judge model",
judgeModel: judgeModel ?? "pi default",
reasoning,
isError: res.isError,
};
@@ -522,14 +522,6 @@ async function decideSignOff(
};
}
}
if (!judgeModel) {
const reason = "no explicit judge model available; set /goals judge <provider/model>";
return {
outcome: { kind: "accepted_inconclusive", reason },
reasoning: `VERDICT: inconclusive\nreason: ${reason}`,
durationMs: Date.now() - startedAt,
};
}
const verdict = await runJudge(goal, evidence, paths, verifyResult, judgeModel, cwd, signal, onUpdate);
const outcome: SignOff =
verdict.kind === "accepted"
@@ -573,13 +565,25 @@ type JudgeResult =
| { kind: "rejected"; missing: string; reasoning: string; durationMs: number }
| { kind: "inconclusive"; reason: string; reasoning: string; durationMs: number };
/** Stage 2: a read-only pi subprocess inspects the evidence against the repo and returns a verdict. */
/** Build the pi argv for the read-only judge. `--model` is omitted when no explicit/session model is
* set, so pi falls back to its configured default -- the judge always runs, never pre-emptively
* fails as "no model". Exported for a unit test that locks this invariant (an empty `--model ""`
* would make every sign-off silently inconclusive). */
export function buildJudgeArgs(judgeModel: string | null): string[] {
const args = ["--mode", "json", "-p", "--no-session"];
if (judgeModel) args.push("--model", judgeModel);
args.push("--tools", JUDGE_TOOLS.join(","), "--exclude-tools", JUDGE_BLOCKED_TOOLS.join(","), "--append-system-prompt", evidenceJudgeSystem);
return args;
}
/** Stage 2: a read-only pi subprocess inspects the evidence against the repo and returns a verdict. */
async function runJudge(
goal: Goal,
evidence: string,
paths: string[],
verifyResult: { command: string; exitCode: number; outputTail: string } | null,
judgeModel: string,
judgeModel: string | null,
cwd: string,
signal: AbortSignal | undefined,
onUpdate?: (partial: { content: Array<{ type: "text"; text: string }>; details: SignOffDetails }) => void,
@@ -600,14 +604,14 @@ async function runJudge(
evidence,
paths,
});
const args = ["--mode", "json", "-p", "--no-session", "--model", judgeModel, "--tools", JUDGE_TOOLS.join(","), "--exclude-tools", JUDGE_BLOCKED_TOOLS.join(","), "--append-system-prompt", evidenceJudgeSystem];
const args = buildJudgeArgs(judgeModel);
args.push(task);
emit("spawning", `Spawning read-only judge for: ${goal.subject}`);
const inv = getPiInvocation(args);
// FIXME(side-effect): pi -p --no-session clones the repo into the PARENT of cwd (so alongside
// the working dir), leaving a stale directory. The judge should run in a temp dir or inside the
// existing repo checkout so it doesn't pollute the user's workspace.
// The judge runs in-place against this checkout (cwd is passed to spawn and the read-only tools
// read from it); pi --no-session does not clone into the parent. Proven and re-checked by
// scripts/check-judge-footprint.sh, which reproduces this invocation and asserts no parent clone.
const judge = await new Promise<{ output: string; error?: string; aborted?: boolean }>((resolve) => {
let settled = false;
let stdoutBuffer = "";
src/prompts.ts
-2
View File
@@ -117,8 +117,6 @@ export function planInjection(p: {
counts: { done: number; open: number };
}): string {
if (!p.activeGoal) {
// FIXME(heading): user wants the heading to show ".pi/goals.md: <title>" so the filename is explicit
// even in the injection. Currently says "Goals (goals.md):" which is close but not the same.
return `.pi/goals.md: ${p.title}\nNo active goal. ${p.counts.open} open, ${p.counts.done} done. Pick the next goal (set its checkbox to [/]) or run /goals.`;
}
const subtasks = p.activeGoal.openSubtasks.length