Skip to content

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

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

factory: reviewer verdict — structured output + review against the issue, not just the diff#157
ALLiDoizCode merged 4 commits into
mainfrom
epic270/275-reviewer-verdict

Conversation

@ALLiDoizCode

Copy link
Copy Markdown

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

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. buzz's repo-specific review steps and gate commands (just fmt-check/clippy/test-unit, just desktop-*/web-*) are preserved.
  • 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 org-wide 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: agent-review.yml now checks out main (git refuses one branch in two worktrees — sandcastle checks the PR head out in its own worktree under .sandcastle/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.
  • main.ts (the reserved autonomous loop) passes ISSUE_NUMBER/ISSUE_TITLE to its reviewer so the shared prompt's placeholders always resolve; it does not yet consume the verdict (auto-merge wiring is toon-meta#270 work).

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

Verification

  • Extraction smoke-tested in-repo via tsx (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 to their env guards (fail on SANDCASTLE_ISSUE_NUMBER/SANDCASTLE_PR_NUMBER, nothing else).
  • The CI that gates these paths (the changes job's release/mobile/desktop contract scripts + file-size ratchet tests) run green locally; actionlint green on the workflow. Biome findings in .sandcastle/ are pre-existing on main (the directory is not format-gated); no new format errors introduced.
  • Live proof (clean + blocking on real PRs) comes from the first agent:review run after merge, since the 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 4 commits August 5, 2026 21:13
…sue (toon-meta#275)

Port of toon-protocol/toon-meta#299 to buzz:

- New .sandcastle/review-verdict.ts: <review> verdict schema + extraction
  (sandbox.run() ignores an output option in sandcastle 0.12.0, so the
  module extracts/validates itself, with one resume() retry then a hard
  fail), plus host-side postBlockingVerdict (PR review + needs:human via
  REST) and resolveIssueFromPrBody.
- agent-implement-issue.ts: reviewer phase runs runReviewerWithVerdict;
  a blocking verdict is never auto-merged (falls back to PR mode) and
  lands findings + needs:human on the opened PR.
- agent-review-pr.ts: materialise the PR head as a local branch, resolve
  the Spec-axis issue from the PR body's Closes #n, require the verdict,
  post blocking findings after sandbox close.
- review-prompt.md: two-axis (Standards + Spec) review against the target
  issue's acceptance criteria, WHAT YOU FIX vs WHAT IS BLOCKING, and the
  machine-parsed REQUIRED VERDICT block.
- agent-review.yml: checkout pins ref: main (sandcastle checks the PR head
  out in its own worktree; one branch cannot be in two worktrees) and the
  header documents the verdict contract. Forensics steps untouched (block#278).
- main.ts: reviewer promptArgs gain ISSUE_NUMBER/ISSUE_TITLE so the
  shared prompt's placeholders always resolve.

zod was already a root devDependency (^3.25.0, v3) — 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 76b41b5 into main Aug 6, 2026
23 checks passed
@ALLiDoizCode
ALLiDoizCode deleted the epic270/275-reviewer-verdict branch August 6, 2026 12:35
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