Repository navigation
[Phase 2] Adversarial plan-critic pass against the fixed rubric #603
Copy link
Copy link
Closed
Labels
dev-leadFor dev-lead agent pickupFor dev-lead agent pickupinitiativeEpic / initiative tracking issueEpic / initiative tracking issue
Description
Story
As a maintainer relying on the planner's output,
I want a single bounded critic pass that reviews Bob's draft plan.json against the fixed rubric, emits structured findings, and has Bob revise once before apply-plan.sh,
so that semantic defects a structural validator can't catch (contested ACs, missing success metric or cost cap, reward-hacking) are caught before the epic materializes.
Acceptance Criteria
- After Bob writes the draft plan.json and it passes validate-plan.py, a bounded critic pass reviews it against the fixed rubric from prompts/bmad/scrum-master.md (the same checks defined in story 2) and emits structured findings as JSON ({check, story_id?, severity, finding}).
- The pass is a SINGLE review plus ONE revise — no loop. It is implemented as one bounded step (a second claude-code-action step in initiative-planner.yml OR a second turn in Bob's prompt — the choice is made and documented; see open_questions).
- Findings Bob can resolve are folded back into plan.json (which is re-validated); findings he cannot resolve become
open_questionscarryingaffected_story_ids, so they gate per the open-questions-as-gate mechanism. - The Initiative: Eval-gated, human-reviewed self-improving skills (SkillOpt-style) #581 plan is captured as a static regression fixture and a test asserts the critic surfaces the six known hand-review findings (contested-AC, no success metric, no cost cap, reward-hacking/overfitting, untracked prereq, reviewability).
- The critic adds at most one extra LLM pass; the human
initiative:autogate remains the final say.
Tasks / Subtasks
- Wire the bounded critic step (second action step or second prompt turn) that loads plan.json + the rubric and emits structured JSON findings. (AC: test issue from agent #1, Go-live improvements for PR review agent #2)
- Have Bob fold resolvable findings back into plan.json and re-run validate-plan.py; route unresolved findings into open_questions with affected_story_ids. (AC: Add @claude delegation, auto-merge, and rebase handling #3)
- Capture the Initiative: Eval-gated, human-reviewed self-improving skills (SkillOpt-style) #581 plan as tests/fixtures and add a regression test asserting the six findings are flagged. (AC: Optimize review: small-PR and incremental fast paths #4)
- Confirm the pass is single-shot (no loop) and the inert-epic + human gate are untouched. (AC: Go-live improvements for PR review agent #2, feat: add Copilot engine support via REVIEW_ENGINE toggle #5)
Dev Notes
- The rubric is defined ONCE in prompts/bmad/scrum-master.md by story 2 — the critic consumes it verbatim; do not fork a second rubric.
- Wiring: the workflow runs a single claude-code-action step today (.github/workflows/initiative-planner.yml:88-176). Either add a second step that reads plan.json + rubric and writes findings then a revise, OR instruct Bob to self-critique in a second turn. Keep it bounded (single pass, one revise) — discussion 💡 Adversarial Plan-Critic + Structural Gates for the Initiative-Planner (Bob) #593's adversarial-review rebuttal is explicit that there is NO self-reflection loop.
- Findings must be STRUCTURED JSON (not prose), mirroring the production eval-gate practice cited in 💡 Adversarial Plan-Critic + Structural Gates for the Initiative-Planner (Bob) #593.
- Routing findings into open_questions requires the object shape (
question+affected_story_ids) introduced by story 3 — hence this story is blocked by both the rubric (story 2) and the gate schema (story 3). - Regression fixture: capture the Initiative: Eval-gated, human-reviewed self-improving skills (SkillOpt-style) #581 plan (open epic Initiative: Eval-gated, human-reviewed self-improving skills (SkillOpt-style) #581 / discussion 💡 SkillOpt-Style Self-Improving Skills: Eval-Gated, Human-Reviewed Agent Skill Evolution #572 in context) as a static plan.json under tests/fixtures and assert the six findings — this is the acceptance evidence discussion 💡 Adversarial Plan-Critic + Structural Gates for the Initiative-Planner (Bob) #593 names.
- Cost: the suggested-next-step in 💡 Adversarial Plan-Critic + Structural Gates for the Initiative-Planner (Bob) #593 treats this as the pilot AFTER the cheap structural fixes; keep it to exactly one extra pass.
Project Structure Notes
Spans the workflow (new bounded step/turn), the prompt (rubric reuse), and a small findings helper + a tests/fixtures regression case. Depends on the rubric (story 2) and the open-questions gate schema (story 3) being in place so findings have a defined home.
References
- prompts/bmad/scrum-master.md
- .github/workflows/initiative-planner.yml#L88-L176
- scripts/initiative-planner/validate-plan.py
- tests/fixtures
Likely target surface
.github/workflows/initiative-planner.ymlprompts/bmad/scrum-master.mdscripts/initiative-planner/tests/test_initiative_planner.bats
Story prepared by the BMAD Scrum Master (Bob) for epic #597. Status: ready-for-dev.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
dev-leadFor dev-lead agent pickupFor dev-lead agent pickupinitiativeEpic / initiative tracking issueEpic / initiative tracking issue
Activity
don-petry commentedon Jun 17, 2026
Dev-Lead Implementation Plan
Issue: #603 — [Phase 2] Adversarial plan-critic pass against the fixed rubric
Scope
Wire a single bounded adversarial critic pass into the initiative-planner that scores Bob's draft
plan.jsonagainst the fixed six-check rubric inprompts/bmad/scrum-master.md(consumed verbatim — no forked rubric), emits structured JSON findings, and has Bob revise exactly once: resolvable findings fold back intoplan.json(re-validated), unresolved ones becomeopen_questionscarryingaffected_story_idsso they gate per #682.Wiring decision (AC #2 — documented)
Second turn in Bob's prompt, inside the existing single
claude-code-actionstep — not a second action step. Rationale: keeps the critic to one extra reasoning pass in the existing LLM invocation (AC #5), needs no extra engine wiring, and theplan → critique → revise → validate → applysequence is one bounded session with no self-reflection loop (per discussion #593).Implementation Checklist
prompts/plan-review.md: align the critic to the six scrum-master.md checks (single source; drop the divergent fork); define findings shape{check, story_id?, severity, finding}.scripts/initiative-planner/plan.schema.json: addaffected_story_idsto theopen_questionsobject.scripts/initiative-planner/validate-plan.py: validateaffected_story_idsreference real story ids.scripts/initiative-planner/route-findings.sh: deterministic helper that folds unresolved findings intoopen_questions{question, affected_story_ids, blocking}..github/workflows/initiative-planner.yml: add the bounded critic turn (single review + one revise) to Bob's prompt; document the choice.tests/fixtures/initiative-planner/plan-581.json+plan-581.expected-findings.json: the Initiative: Eval-gated, human-reviewed self-improving skills (SkillOpt-style) #581 plan as a static regression fixture..github/workflows/lint.yml(repo-specific bats list, per AGENTS.md).Tests to Write
tests/test_plan_critic.bats: fixture passesvalidate-plan.py(structural validator can't catch the six semantic defects); rubric in scrum-master.md enumerates the six checks;plan-review.mdreferences scrum-master.md (no fork); expected-findings cover all six checks (contested-AC, success-metric, cost-cap, reward-hacking/overfitting, untracked-prereq, reviewability);route-findings.shroutes unresolved findings intoopen_questionswithaffected_story_idsand the result re-validates; major/critical →blocking:truegates apply-plan.tests/test_initiative_planner.bats:validate-planaccepts/validatesaffected_story_ids.Test Command
scripts/run-bats.sh tests/test_plan_critic.bats tests/test_initiative_planner.bats tests/test_plan_review.batsandbash .dev-lead/scripts/dev-lead-lint.shdon-petry commentedon Jun 17, 2026
Dev-Lead: Implementation Complete
Plan Execution
prompts/plan-review.mdto the single-source six-check rubric inprompts/bmad/scrum-master.md(no forked second rubric) and the{check, story_id?, severity, finding}findings shape (AC test issue from agent #1).initiative-planner.yml— single review + ONE revise, no loop, at most one extra reasoning pass (AC Go-live improvements for PR review agent #2, feat: add Copilot engine support via REVIEW_ENGINE toggle #5). Decision documented inline in the prompt.affected_story_idsto theopen_questionsobject inplan.schema.json;validate-plan.pynow rejects ids that don't reference a real story (AC Add @claude delegation, auto-merge, and rebase handling #3).scripts/initiative-planner/route-findings.sh— deterministic transform folding unresolved findings intoopen_questions{question, affected_story_ids, blocking}; major/critical →blocking:trueso they gate per initiative-planner: Bob materializes a plan even when he has unresolved open questions — should gate, not guess #682 (AC Add @claude delegation, auto-merge, and rebase handling #3).tests/fixtures/initiative-planner/plan-581.json+ goldenplan-581.expected-findings.jsonand addedtests/test_plan_critic.batsasserting the six findings (AC Optimize review: small-PR and incremental fast paths #4).initiative:autogate are untouched (AC Go-live improvements for PR review agent #2, feat: add Copilot engine support via REVIEW_ENGINE toggle #5); registered the new bats file inlint.yml.Wiring decision (AC #2, documented)
Second turn in Bob's prompt, inside the existing single
claude-code-actionstep — not a second action step. It keeps the critic to one extra reasoning pass in the existing LLM invocation, needs no extra engine wiring, and runsplan → critique → revise → re-validate → applyas one bounded session with no self-reflection loop (per #593).Note on the regression test (AC #4)
The LLM critic can't run deterministically in CI, so the test locks the regression contract: the #581 fixture is structurally valid (proving the structural validator can't catch the six semantic defects), the rubric is single-sourced and enumerates the six checks, the golden findings cover all six (
contested_ac,success_metric,cost_cap,eval_safeguards,untracked_prereq,reviewability), androute-findings.shfolds them into the gatingopen_questionsshape. The fixture is a faithful capture of the #581 plan's structure (discussion #572 / epic #581) exhibiting each of the six known defects.Test Results
Files Changed
prompts/plan-review.md: rewritten to consume the scrum-master.md rubric verbatim + new findings shape..github/workflows/initiative-planner.yml: added the bounded plan-critic turn (critique → revise once → re-validate) to Bob's prompt..github/workflows/lint.yml: registeredtests/test_plan_critic.bats.scripts/initiative-planner/plan.schema.json:open_questions.affected_story_ids.scripts/initiative-planner/validate-plan.py: validateaffected_story_idsrefs.scripts/initiative-planner/route-findings.sh: new deterministic routing helper.scripts/post-plan-findings.sh: header comment updated to the new findings shape (channel is field-name agnostic).tests/test_plan_critic.bats,tests/fixtures/initiative-planner/plan-581.json,tests/fixtures/initiative-planner/plan-581.expected-findings.json: new regression test + fixtures.Notes
review-plan.sh/post-plan-findings.shregistry path (engine.sh-based) is left intact; both it and the new in-prompt critic now reference the same single rubric (prompts/bmad/scrum-master.mdviaprompts/plan-review.md).affected_story_idsis the field this story adds for routing; theblockinggate field (initiative-planner: Bob materializes a plan even when he has unresolved open questions — should gate, not guess #682) was already present and is reused for gating.feat: implement issue #603 — [Phase 2] Adversarial plan-critic pass a…