Skip to content

factory: reviewer verdict — structured output + review against the issue, not just the diff - #90

Merged
ALLiDoizCode merged 2 commits into
mainfrom
epic270/275-reviewer-verdict
Aug 6, 2026
Merged

ALLiDoizCode merged 2 commits into
mainfrom
epic270/275-reviewer-verdict

Conversation

@ALLiDoizCode

Copy link
Copy Markdown
Contributor

Gives the factory reviewer a verdict channel and the missing Spec axis (toon-meta#275, epic toon-meta#270) — port of toon-protocol/toon-meta#299 to swap.

What changes

  • Structured verdict, enforced. Every reviewer run must end with <review>{"verdict":"clean"|"blocking","blockingFindings":[{file,line,summary,why}]}</review>. The schema is declared as sandcastle.Output.object({ tag: "review", schema }); extraction/validation is performed by the new .sandcastle/review-verdict.ts because — verified against the @ai-hero/sandcastle@0.12.0 dist — the engine's structured-output surface exists only on top-level run(), and the sandbox.run() the runners use silently ignores an output option. Extraction mirrors the engine's semantics exactly (last tag wins, fence-aware unwrap, JSON parse, zod schema validation).
  • Malformed verdicts fail the run. One retry via SandboxRunResult.resume() (the engine's own structured-output retry mechanism, exactly one iteration, with engine-style error feedback); if the verdict is still malformed the runner throws and the Actions job goes red — a missing verdict is never mistaken for a clean one.
  • Spec axis. The implement runner forwards ISSUE_NUMBER/ISSUE_TITLE via promptArgs; the standalone agent:review runner resolves the issue from the PR body's Closes #n (no reference → Standards-only review). The prompt instructs the reviewer to gh issue view the target FIRST and review the diff against its acceptance criteria. main.ts (the reserved autonomous loop) passes the new promptArgs too so the prompt's placeholders always resolve.
  • Blocking side effects. Findings are posted as a PR review (event COMMENT) and needs:human is applied — pure REST via gh api (porcelain gh pr edit is broken in repos with a classic Project attached by the projectCards GraphQL deprecation). In auto-merge mode a blocking verdict refuses to merge and falls back to PR mode.
  • Standalone runner unbroken. Adopts connector#634's proven first-live-run fix: the workflow checks out main (git refuses one branch in two worktrees), and the runner materialises the PR head as a local branch so the engine's worktree add -b … HEAD fallback cannot silently review an empty diff.

Forensics steps (toon-meta#278) and the ~200k context-budget prompt wording (toon-meta#273) are preserved. zod was already a devDependency here (^3.25.0, v3 API) — no lockfile change.

Verification

  • Extraction smoke-tested in-repo (valid clean/blocking, fenced JSON, last-tag-wins, missing tag, invalid JSON, bad enum, blocking-without-findings, clean-with-findings, null line) — all 10 pass; temp script deleted before commit.
  • Both runners import-smoke-tested: each fails on its env guard (SANDCASTLE_PR_NUMBER / SANDCASTLE_ISSUE_NUMBER) and nothing else.
  • swap's own gate green: pnpm run gate:correctness (build + eslint 0/352 vs frozen 0/352 + typecheck 0 vs frozen 25), pnpm test (285 passed, 2 skipped), pnpm run gate:no-regression (26.3s vs 30.9s baseline).
  • Live proof (clean + blocking on real PRs) comes with the first agent:review run after merge, since this label→runner executes main's code.

Part of toon-protocol/toon-meta#270
Part of toon-protocol/toon-meta#275

🤖 Generated with Claude Code

ALLiDoizCode and others added 2 commits August 5, 2026 21:12
…sue, not just the diff

Port of toon-protocol/toon-meta#299 (toon-meta#275) to swap:

- .sandcastle/review-verdict.ts: shared verdict channel — declares
  sandcastle.Output.object({ tag: "review", schema }) and does its own
  extraction (sandbox.run() in @ai-hero/sandcastle@0.12.0 ignores an
  output option); one resume() retry on a malformed verdict, then fail.
- agent-implement-issue.ts: reviewer phase runs runReviewerWithVerdict;
  blocking verdicts post findings + needs:human on the opened PR, and
  auto-merge mode refuses to merge a blocking verdict (falls back to PR
  mode).
- agent-review-pr.ts: resolves the Spec-axis issue from the PR body's
  Closes #n, materialises the PR head as a local branch (the workflow
  now checks out main), and posts blocking verdicts after sandbox close.
- review-prompt.md: two-axis framing (Standards + Spec), target-issue
  context, WHAT YOU FIX vs WHAT IS BLOCKING, and the REQUIRED VERDICT
  structured-output contract; context-budget section unchanged and last.
- agent-review.yml: checkout pins ref: main (sandcastle checks the PR
  head out in its own worktree; one branch cannot be in two worktrees);
  header documents the verdict contract. Forensics steps untouched.
- main.ts: reviewer promptArgs gain ISSUE_NUMBER/ISSUE_TITLE so the
  prompt's placeholders resolve in the reserved autonomous loop.

zod was already a devDependency (^3.25.0, v3 API) — no lockfile change.

Part of toon-protocol/toon-meta#270
Part of toon-protocol/toon-meta#275

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ALLiDoizCode
ALLiDoizCode merged commit 7c4eeda into main Aug 6, 2026
4 checks passed
@ALLiDoizCode
ALLiDoizCode deleted the epic270/275-reviewer-verdict branch August 6, 2026 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant