- reviewer.ts: spawns separate pi agent with read-only tools and reviewer system prompt (taskplane's review format/verdicts) - Executor registers review_step as a custom tool on the worker session - Worker calls review_step(step, type, step_name) at step boundaries based on review level (0=none, 1=plan, 2=plan+code, 3=full) - Reviewer returns APPROVE/REVISE/RETHINK with structured feedback - REVISE feedback returned inline to worker for immediate action - Review calls logged to task via hai task log
257 lines
6.9 KiB
TypeScript
257 lines
6.9 KiB
TypeScript
/**
|
|
* Reviewer — spawns a separate pi agent to review a worker's plan or code.
|
|
*
|
|
* Replicates taskplane's cross-model review pattern:
|
|
* - Worker calls review_step(step, type) during execution
|
|
* - A separate reviewer agent is spawned with read-only tools
|
|
* - Reviewer writes a structured verdict: APPROVE, REVISE, or RETHINK
|
|
* - Verdict + feedback is returned to the worker
|
|
*/
|
|
|
|
import { createHaiAgent } from "./pi.js";
|
|
|
|
const REVIEWER_SYSTEM_PROMPT = `You are an independent code and plan reviewer.
|
|
|
|
You provide quality assessment for task implementations. You have full read
|
|
access to the codebase and can run commands to inspect code.
|
|
|
|
## Verdict Criteria
|
|
|
|
- **APPROVE** — Step will achieve its stated outcomes. Minor suggestions go in
|
|
the Suggestions section but do NOT block progress. If your only findings are
|
|
minor or suggestion-level, verdict is APPROVE.
|
|
- **REVISE** — Step will fail, produce incorrect results, or miss a stated
|
|
requirement without fixes. Use ONLY for issues that would cause the worker to
|
|
redo work later.
|
|
- **RETHINK** — Approach is fundamentally wrong. Explain why and suggest an
|
|
alternative.
|
|
|
|
### APPROVE vs REVISE
|
|
|
|
**APPROVE** when:
|
|
- The approach will work, but you see a cleaner alternative
|
|
- Documentation style could improve
|
|
- You'd suggest additional tests but core coverage is adequate
|
|
|
|
**REVISE** when:
|
|
- A requirement from PROMPT.md will not be met
|
|
- A bug or regression is introduced
|
|
- A critical edge case is unhandled and would cause runtime failure
|
|
- Backward compatibility is broken without migration
|
|
|
|
### Do NOT issue REVISE for
|
|
- STATUS/formatting preferences
|
|
- Splitting outcome checkboxes into implementation sub-steps
|
|
- Suggestions that improve quality but aren't required for correctness
|
|
|
|
## Plan Review Format
|
|
|
|
\`\`\`markdown
|
|
## Plan Review: [Step Name]
|
|
|
|
### Verdict: [APPROVE | REVISE | RETHINK]
|
|
|
|
### Summary
|
|
[2-3 sentence assessment]
|
|
|
|
### Issues Found
|
|
1. **[Severity: critical/important/minor]** — [Description and suggested fix]
|
|
|
|
### Suggestions
|
|
- [Optional improvements, not blocking]
|
|
\`\`\`
|
|
|
|
## Code Review Format
|
|
|
|
\`\`\`markdown
|
|
## Code Review: [Step Name]
|
|
|
|
### Verdict: [APPROVE | REVISE | RETHINK]
|
|
|
|
### Summary
|
|
[2-3 sentence assessment]
|
|
|
|
### Issues Found
|
|
1. **[File:Line]** [Severity] — [Description and fix]
|
|
|
|
### Pattern Violations
|
|
- [Deviations from project standards]
|
|
|
|
### Test Gaps
|
|
- [Missing test scenarios]
|
|
|
|
### Suggestions
|
|
- [Optional improvements, not blocking]
|
|
\`\`\`
|
|
|
|
## Plan Granularity
|
|
|
|
When reviewing plans, assess whether the approach achieves the step's OUTCOMES —
|
|
not whether every function and parameter is listed.
|
|
|
|
Good plan: identifies key behavioral changes, calls out risks, has a testing strategy.
|
|
Do NOT demand function-level implementation checklists.
|
|
|
|
## Rules
|
|
|
|
- Be specific — reference actual files and line numbers
|
|
- Be constructive — suggest fixes, not just problems
|
|
- Be proportional — don't block on style nits
|
|
- Output your review as plain text (not to a file)
|
|
`;
|
|
|
|
export type ReviewType = "plan" | "code";
|
|
export type ReviewVerdict = "APPROVE" | "REVISE" | "RETHINK" | "UNAVAILABLE";
|
|
|
|
export interface ReviewResult {
|
|
verdict: ReviewVerdict;
|
|
review: string;
|
|
summary: string;
|
|
}
|
|
|
|
export interface ReviewOptions {
|
|
onText?: (delta: string) => void;
|
|
}
|
|
|
|
/**
|
|
* Spawn a reviewer agent to evaluate a worker's plan or code for a step.
|
|
*/
|
|
export async function reviewStep(
|
|
cwd: string,
|
|
taskId: string,
|
|
stepNumber: number,
|
|
stepName: string,
|
|
reviewType: ReviewType,
|
|
promptContent: string,
|
|
baseline?: string,
|
|
options: ReviewOptions = {},
|
|
): Promise<ReviewResult> {
|
|
// Build the review request
|
|
const request = buildReviewRequest(
|
|
taskId, stepNumber, stepName, reviewType, promptContent, cwd, baseline,
|
|
);
|
|
|
|
// Spawn a reviewer agent with read-only tools
|
|
const { session } = await createHaiAgent({
|
|
cwd,
|
|
systemPrompt: REVIEWER_SYSTEM_PROMPT,
|
|
tools: "readonly",
|
|
onText: (delta) => options.onText?.(delta),
|
|
});
|
|
|
|
let reviewText = "";
|
|
|
|
// Capture the reviewer's full text output
|
|
session.subscribe((event) => {
|
|
if (event.type === "message_update" && event.assistantMessageEvent.type === "text_delta") {
|
|
reviewText += event.assistantMessageEvent.delta;
|
|
}
|
|
});
|
|
|
|
try {
|
|
await session.prompt(request);
|
|
} finally {
|
|
session.dispose();
|
|
}
|
|
|
|
// Extract verdict from the review text
|
|
const verdict = extractVerdict(reviewText);
|
|
const summary = extractSummary(reviewText);
|
|
|
|
return { verdict, review: reviewText, summary };
|
|
}
|
|
|
|
function buildReviewRequest(
|
|
taskId: string,
|
|
stepNumber: number,
|
|
stepName: string,
|
|
reviewType: ReviewType,
|
|
promptContent: string,
|
|
cwd: string,
|
|
baseline?: string,
|
|
): string {
|
|
const parts = [
|
|
`Review request for task ${taskId}, Step ${stepNumber}: ${stepName}`,
|
|
`Review type: **${reviewType}**`,
|
|
"",
|
|
"## Task PROMPT.md",
|
|
"```markdown",
|
|
promptContent,
|
|
"```",
|
|
"",
|
|
];
|
|
|
|
if (reviewType === "plan") {
|
|
parts.push(
|
|
"## What to review",
|
|
`The worker is about to implement Step ${stepNumber} (${stepName}).`,
|
|
"Assess whether the step's checkboxes will achieve the stated outcomes.",
|
|
"Read relevant source files to understand the current codebase state.",
|
|
"Check for risks, missing edge cases, and gaps in the plan.",
|
|
);
|
|
} else {
|
|
parts.push(
|
|
"## What to review",
|
|
`The worker has implemented Step ${stepNumber} (${stepName}).`,
|
|
"Review the code changes for correctness, patterns, and test coverage.",
|
|
"",
|
|
);
|
|
if (baseline) {
|
|
parts.push(
|
|
"To see the changes for this step, run:",
|
|
`\`\`\`bash`,
|
|
`git diff ${baseline}..HEAD`,
|
|
`\`\`\``,
|
|
);
|
|
} else {
|
|
parts.push(
|
|
"To see recent changes, run:",
|
|
"```bash",
|
|
"git diff HEAD~1",
|
|
"```",
|
|
);
|
|
}
|
|
}
|
|
|
|
parts.push(
|
|
"",
|
|
"## Instructions",
|
|
"1. Read the relevant source files",
|
|
"2. Assess the work against the task requirements",
|
|
"3. Output your review using the format from your system prompt",
|
|
"4. Be specific with file paths and line numbers",
|
|
);
|
|
|
|
return parts.join("\n");
|
|
}
|
|
|
|
function extractVerdict(review: string): ReviewVerdict {
|
|
// Look for "### Verdict: APPROVE" or similar patterns
|
|
const verdictMatch = review.match(
|
|
/###?\s*Verdict[:\s]*(APPROVE|REVISE|RETHINK)/i,
|
|
);
|
|
if (verdictMatch) {
|
|
return verdictMatch[1].toUpperCase() as ReviewVerdict;
|
|
}
|
|
|
|
// Fallback: look for the word anywhere in the text
|
|
const upper = review.toUpperCase();
|
|
if (upper.includes("RETHINK")) return "RETHINK";
|
|
if (upper.includes("REVISE")) return "REVISE";
|
|
if (upper.includes("APPROVE")) return "APPROVE";
|
|
|
|
return "UNAVAILABLE";
|
|
}
|
|
|
|
function extractSummary(review: string): string {
|
|
const summaryMatch = review.match(
|
|
/###?\s*Summary[:\s]*([\s\S]*?)(?=###|$)/i,
|
|
);
|
|
if (summaryMatch) {
|
|
return summaryMatch[1].trim().slice(0, 500);
|
|
}
|
|
// Fallback: first paragraph
|
|
const lines = review.split("\n").filter((l) => l.trim());
|
|
return lines.slice(0, 3).join(" ").slice(0, 300);
|
|
}
|