From 3467826d4a74f8e551e20d614b03130c279c0682 Mon Sep 17 00:00:00 2001 From: kami Date: Sat, 13 Jun 2026 16:24:09 +0400 Subject: [PATCH] =?UTF-8?q?feat(workflows):=20reviewer=20narrow-question?= =?UTF-8?q?=20+=20static-first=20framing=20(role-reliability=20=C2=A75)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reviewer prompt told the model to judge "the analysis requirements," but the reviewer stage only needed [patch, impl_plan] — it never actually received the analysis artifact, so the acceptance criteria it was meant to check against were never in its context. A reliability gate checking against criteria it can't see is theater. - reviewer now needs `analysis`, so the diff is judged against concrete, pre-stated acceptance criteria (§5 narrow question) rather than whole files against taste; the §3 minItems enforcement guarantees those criteria are non-empty. - reviewer.md operationalizes §5's two principles: evaluate the diff against each requirement (findings must map to a requirement/plan-step/defect), and don't re-report what deterministic tools already catch (compiler/detekt/tests — trust verification, point at failures, don't re-derive them). This is the workflow-level realization of §5. The full static-first infrastructure (running static tools as a pipeline step and mechanically excluding their findings from reviewer context) needs a static-findings event representation and is deferred. §6 critic calibration needs runtime history before it can say anything (spec ordering #5). --- examples/workflows/prompts/reviewer.md | 19 +++++++++++++++---- examples/workflows/role_pipeline.toml | 6 ++++-- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/examples/workflows/prompts/reviewer.md b/examples/workflows/prompts/reviewer.md index 1e1d8be9..29bd1f74 100644 --- a/examples/workflows/prompts/reviewer.md +++ b/examples/workflows/prompts/reviewer.md @@ -1,22 +1,33 @@ You are the **Reviewer** — the quality gate. -You receive the `patch` (the implementer's work) and the `impl_plan` it was meant to satisfy. -Assess whether the implementation meets the plan and the original requirements. +You receive the `patch` (the implementer's work), the `impl_plan` it was meant to satisfy, and +the `analysis` whose `requirements` are the **acceptance criteria** for this work. + +Review the **diff against those acceptance criteria** — not whole files against personal taste. +The narrow question is: *does this change satisfy each stated requirement, correctly?* A finding +that doesn't map to a requirement, the plan, or a real defect is noise. Review against the **current** ground truth, not a stale one: the decision history above includes any user steering and your own prior verdicts. If the user steered the implementer away from the original plan, judge against the steered direction — not the superseded plan. Check: -1. Does the patch satisfy every step of `impl_plan` and the analysis requirements? +1. Does the patch satisfy **every** requirement in `analysis.requirements` and step of `impl_plan`? 2. Is it correct, consistent with the codebase's patterns, and free of obvious defects? 3. Did verification (build/tests) actually pass? 4. No unrequested scope, no placeholders, no regressions. +Do **not** spend the review re-finding what deterministic tools already catch: compiler errors, +formatter/detekt/lint violations, and failing tests are reported by the build itself. If +verification passed, trust it; if it failed, the failure is already on the record — point to it, +don't re-derive it. Your value is the judgment the tools can't give: whether the change actually +meets the requirements. + Emit your result as the `review_report` artifact (JSON, schema provided): - `verdict`: exactly `"approved"` or `"changes_requested"`. - `notes`: if `changes_requested`, list the specific, actionable changes needed (one per - line) so the implementer knows exactly what to fix. If `approved`, briefly say why. + line), each tied to the requirement or plan step it violates, so the implementer knows + exactly what to fix. If `approved`, briefly state which requirements the patch satisfies. Use `changes_requested` only for real problems — the loop is capped and will escalate to a human if it runs too long. Be decisive. diff --git a/examples/workflows/role_pipeline.toml b/examples/workflows/role_pipeline.toml index ec98b025..92e5c3cb 100644 --- a/examples/workflows/role_pipeline.toml +++ b/examples/workflows/role_pipeline.toml @@ -69,11 +69,13 @@ allowed_tools = ["file_read", "file_write", "file_edit", "ShellTool"] token_budget = 32768 max_retries = 3 -# 5. Review the patch against the plan; emit a structured verdict. +# 5. Review the patch against the plan AND the analyst's acceptance criteria. The reviewer +# needs `analysis` so it judges the diff against concrete, pre-stated criteria (§5 narrow +# question) rather than whole files against taste. [[stages]] id = "reviewer" prompt = "prompts/reviewer.md" -needs = ["patch", "impl_plan"] +needs = ["patch", "impl_plan", "analysis"] produces = [{ name = "review_report", kind = "review_report" }] token_budget = 32768 max_retries = 2