mirror of
https://github.com/wassname/pi-goals.git
synced 2026-07-24 13:10:41 +08:00
Make goal judge use explicit session model
This commit is contained in:
wassname
@@ -144,8 +144,10 @@ else is the agent editing the file. It reads the goal's `evidence:` block from `
|
||||
inconclusive` log line and any partial judge output in the result. Either way the judge's
|
||||
reasoning comes back in the result.
|
||||
|
||||
The judge defaults to Pi's default model unless `/goals judge <provider/model>` is set. Point it at
|
||||
another model for an independent cross-family check.
|
||||
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.
|
||||
|
||||
## Prompts
|
||||
|
||||
|
||||
@@ -12,6 +12,7 @@ Out: broader autonomous loop work, plan-mode UX, model auto-selection.
|
||||
- R2: If `verify:` passes and the judge accepts, mark the goal done as before. Done means: log records normal judge accept. VERIFY: unit test for accepted sign-off still passes.
|
||||
- R3: If any `verify:` command passes but the judge times out or subprocess/model transport fails, mark the goal done with an explicit inconclusive-judge log. Goals without `verify:` use the same fail-forward rule once evidence exists. VERIFY: a unit test records accepted status and a log line containing `judge inconclusive`.
|
||||
- R4: Judge transport should parse `pi --mode json` message events instead of raw `-p` terminal output. Done means: code captures final assistant text and provider stop errors distinctly. VERIFY: `npm run typecheck` and tests pass.
|
||||
- R5: The judge should behave like oracle where it matters: explicit model, live streamed progress, and a timeout large enough for a cold reasoning turn. Done means: unset `/goals judge` resolves to the current session model when visible; if no model is visible, no implicit Pi default is used and sign-off is `judge inconclusive`. `message_update` emits throttled progress, and timeout is 600s. VERIFY: `npm run typecheck` and fresh-eyes diff review.
|
||||
|
||||
## Tasks
|
||||
- [x] T1 (R3): Add an accepted-with-warning sign-off outcome.
|
||||
@@ -32,6 +33,12 @@ Out: broader autonomous loop work, plan-mode UX, model auto-selection.
|
||||
- likely_fail: README still says all rejects keep goal open
|
||||
- sneaky_fail: docs imply subagent evidence was accepted when it timed out
|
||||
- UAT: [README.md](/home/wassname/.pi/agent/git/github.com/wassname/pi-plan/README.md)
|
||||
- [x] T4 (R5): Copy oracle's reliability shape for model/progress.
|
||||
- verify: `npm run typecheck`
|
||||
- success: `CompleteGoal` passes the current session model to the judge when no override is set, never spawns without `--model`, and streamed judge deltas are surfaced through `onUpdate`
|
||||
- likely_fail: judge still runs without `--model`
|
||||
- sneaky_fail: user sees no progress for several minutes and kills a working judge
|
||||
- UAT: [src/index.ts](/home/wassname/.pi/agent/git/github.com/wassname/pi-plan/src/index.ts)
|
||||
|
||||
## Context
|
||||
Observed result from downstream use:
|
||||
@@ -54,6 +61,7 @@ Interpretation: latest surfaced output proves the internal judge timed out. It d
|
||||
- 2026-06-29 unset `/goals judge` spawns the judge without `--model`, so Pi resolves its configured default model; do not describe this as the current session model.
|
||||
- 2026-06-29 timeout/transport failure now maps to `accepted_inconclusive`, preserving partial output in reasoning when available.
|
||||
- 2026-06-29 fresh-eyes review found loose `/accept/i` verdict parsing and caller-abort fail-forward risk; fixed exact verdict parsing and made caller abort reject.
|
||||
- 2026-06-29 oracle comparison suggests the important reliability pieces are explicit model selection, JSON streaming, live partial output, and no short wrapper timeout; updated CompleteGoal to use the current session model when visible, never spawn without `--model`, stream throttled progress, and wait 600s.
|
||||
|
||||
## TODO
|
||||
- Consider making `CompleteGoal` expose `verifyExitCode: 0` and `judgeOutcome` separately in details.
|
||||
|
||||
+31
-8
@@ -59,6 +59,7 @@ const WIDGET_KEY = "pi-goals-widget";
|
||||
// Names match pi's internal tool registry (grep→ffgrep, find→fffind, etc.).
|
||||
const JUDGE_TOOLS = ["read", "bash", "grep", "find", "ls"];
|
||||
const JUDGE_BLOCKED_TOOLS = ["edit", "write"];
|
||||
const JUDGE_TIMEOUT_MS = 600_000;
|
||||
// File mutators blocked while drafting goals (read-only plan mode, like narumiruna/pi-plan-mode), so
|
||||
// code isn't written before goals are agreed. The one allowed write is goals.md itself (the
|
||||
// deliverable). A read-only task (a pure search) can still be explored in plan mode by nature.
|
||||
@@ -86,7 +87,7 @@ const PLAN_REL = ".pi/goals.md"; // project-local, gitignored (pi-tasks conventi
|
||||
interface PlanState {
|
||||
isPlanMode: boolean;
|
||||
objective: string | null;
|
||||
/** Optional model ref for the sign-off judge; unset => the subprocess uses pi's default model. */
|
||||
/** Optional model ref for the sign-off judge; unset => use the current session model. */
|
||||
judgeModel: string | null;
|
||||
}
|
||||
|
||||
@@ -173,7 +174,7 @@ export default function piGoalsExtension(pi: ExtensionAPI): void {
|
||||
function setJudge(ref: string, ctx: ExtensionContext): void {
|
||||
state = { ...state, judgeModel: ref || null };
|
||||
persist();
|
||||
ctx.ui.notify(ref ? `Sign-off judge model set to ${ref}` : "Sign-off judge reset to the default model", "info");
|
||||
ctx.ui.notify(ref ? `Sign-off judge model set to ${ref}` : "Sign-off judge reset to the current session model", "info");
|
||||
}
|
||||
|
||||
async function clearPlan(ctx: ExtensionContext): Promise<void> {
|
||||
@@ -293,7 +294,8 @@ export default function piGoalsExtension(pi: ExtensionAPI): void {
|
||||
onUpdate?.(partial);
|
||||
};
|
||||
|
||||
const { outcome, reasoning, durationMs } = await decideSignOff(goal, goal.evidence.join("\n"), goal.evidence, state.judgeModel, ctx.cwd, signal, handleUpdate);
|
||||
const judgeModel = state.judgeModel ?? currentSessionModelRef(ctx);
|
||||
const { outcome, reasoning, durationMs } = await decideSignOff(goal, goal.evidence.join("\n"), goal.evidence, judgeModel, ctx.cwd, signal, handleUpdate);
|
||||
const res = recordSignOff(content, goal.subject, stamp(), outcome);
|
||||
if (res.content !== content) writePlan(ctx, res.content);
|
||||
updateWidget(ctx);
|
||||
@@ -309,7 +311,7 @@ export default function piGoalsExtension(pi: ExtensionAPI): void {
|
||||
durationMs,
|
||||
verifyCommand: goal.verify ?? undefined,
|
||||
verifyExitCode: outcome.kind === "verify_failed" ? outcome.exitCode : undefined,
|
||||
judgeModel: state.judgeModel ?? "pi default model",
|
||||
judgeModel: judgeModel ?? "no explicit judge model",
|
||||
reasoning,
|
||||
isError: res.isError,
|
||||
};
|
||||
@@ -471,6 +473,10 @@ function stamp(): string {
|
||||
return new Date().toISOString().slice(0, 16).replace("T", " ");
|
||||
}
|
||||
|
||||
function currentSessionModelRef(ctx: ExtensionContext): string | null {
|
||||
return ctx.model ? `${ctx.model.provider}/${ctx.model.id}` : null;
|
||||
}
|
||||
|
||||
/** Decide a sign-off: deterministic verify first (cheap; skip the model call if it fails), then the judge.
|
||||
* Returns the outcome plus the judge's (or verify's) reasoning so CompleteGoal can show WHY. */
|
||||
async function decideSignOff(
|
||||
@@ -501,6 +507,14 @@ 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"
|
||||
@@ -550,7 +564,7 @@ async function runJudge(
|
||||
evidence: string,
|
||||
paths: string[],
|
||||
verifyResult: { command: string; exitCode: number; outputTail: string } | null,
|
||||
judgeModel: string | null,
|
||||
judgeModel: string,
|
||||
cwd: string,
|
||||
signal: AbortSignal | undefined,
|
||||
onUpdate?: (partial: { content: Array<{ type: "text"; text: string }>; details: SignOffDetails }) => void,
|
||||
@@ -571,8 +585,7 @@ async function runJudge(
|
||||
evidence,
|
||||
paths,
|
||||
});
|
||||
const args = ["--mode", "json", "-p", "--no-session", "--tools", JUDGE_TOOLS.join(","), "--exclude-tools", JUDGE_BLOCKED_TOOLS.join(","), "--append-system-prompt", evidenceJudgeSystem];
|
||||
if (judgeModel) args.push("--model", judgeModel);
|
||||
const args = ["--mode", "json", "-p", "--no-session", "--model", judgeModel, "--tools", JUDGE_TOOLS.join(","), "--exclude-tools", JUDGE_BLOCKED_TOOLS.join(","), "--append-system-prompt", evidenceJudgeSystem];
|
||||
args.push(task);
|
||||
|
||||
emit("spawning", `Spawning read-only judge for: ${goal.subject}`);
|
||||
@@ -580,7 +593,6 @@ async function runJudge(
|
||||
// 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.
|
||||
const JUDGE_TIMEOUT_MS = 120_000;
|
||||
const judge = await new Promise<{ output: string; error?: string; aborted?: boolean }>((resolve) => {
|
||||
let settled = false;
|
||||
let stdoutBuffer = "";
|
||||
@@ -589,7 +601,14 @@ async function runJudge(
|
||||
let stderr = "";
|
||||
let lastStopReason: string | undefined;
|
||||
let lastErrorMessage: string | undefined;
|
||||
let lastProgressAt = 0;
|
||||
const partialOutput = (): string => [finalOutput, currentText, stderr.trim()].filter(Boolean).join("\n\n").trim();
|
||||
const emitProgress = (text: string, force = false) => {
|
||||
const now = Date.now();
|
||||
if (!force && now - lastProgressAt < 750) return;
|
||||
lastProgressAt = now;
|
||||
emit("judging", text);
|
||||
};
|
||||
const proc = spawn(inv.command, inv.args, { cwd, shell: false, stdio: ["ignore", "pipe", "pipe"], signal });
|
||||
if (!proc.stdout || !proc.stderr) throw new Error("judge subprocess stdio must be piped");
|
||||
const timer = setTimeout(() => {
|
||||
@@ -611,10 +630,12 @@ async function runJudge(
|
||||
|
||||
if (event.type === "message_start" && event.message?.role === "assistant") {
|
||||
currentText = "";
|
||||
emitProgress("Judge is reading evidence...", true);
|
||||
return;
|
||||
}
|
||||
if (event.type === "message_update" && event.assistantMessageEvent?.type === "text_delta") {
|
||||
currentText += event.assistantMessageEvent.delta ?? "";
|
||||
emitProgress(currentText || "Judge is reading evidence...");
|
||||
return;
|
||||
}
|
||||
if (event.type === "message_end" && event.message?.role === "assistant") {
|
||||
@@ -623,6 +644,7 @@ async function runJudge(
|
||||
currentText = "";
|
||||
lastStopReason = typeof event.message.stopReason === "string" ? event.message.stopReason : undefined;
|
||||
lastErrorMessage = typeof event.message.errorMessage === "string" ? event.message.errorMessage : undefined;
|
||||
emitProgress(finalOutput || "Judge finished without text.", true);
|
||||
}
|
||||
};
|
||||
|
||||
@@ -634,6 +656,7 @@ async function runJudge(
|
||||
});
|
||||
proc.stderr.on("data", (data) => {
|
||||
stderr += data.toString();
|
||||
emitProgress(partialOutput() || "Judge subprocess wrote stderr.");
|
||||
});
|
||||
proc.on("close", (code) => {
|
||||
if (!settled) {
|
||||
|
||||
Reference in New Issue
Block a user