Replies: 1 comment
-
|
📋 Initiative planned by the BMAD Scrum Master (Bob). Epic #610 — Generalize pr-review into a context-adaptive review agent (artifact contract + rubric registry) 6 stories created (inert — labelled
Open questions for review:
Review the epic and its sub-issue DAG, adjust as needed, then add |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
Summary
Generalize the existing
pr-reviewagent into a single context-adaptive "review" agent that reviews any reviewable artifact — a PR diff today, an initiativeplan.json(Epic #597's critic), a candidate skill edit (Epic #581's gate) tomorrow — by selecting its rubric and output channel from context rather than being hard-wired to pull requests. Instead of forking a second "plan critic" alongsidepr-review(the naive reading of Epic #597's #603), we keep one maintained review brain and teach it new artifact types. The review logic, model routing, and approval discipline already built for PRs become reusable across the whole agentic pipeline.Market Signal
"Review" is converging into a reusable, cross-cutting agent capability rather than a per-surface bolt-on:
Hype filter: this is consolidation, not new capability — we already run a competent reviewer. The shippable move is a rubric/output-channel abstraction over the current
pr-review, not a new model or framework.User Signal
This fell directly out of answering Epic #597's open questions. The critic-wiring question (B1 / story #603) asked "second
claude-code-actionstep vs. second prompt turn?" — and the better answer was neither: reusepr-review. That implies a generalization bigger than #603:agents/pr-reviewer.md+ theprompts/triage.md → deep-review.md → synthesize.mdcascade +scripts/engine.shmodel routing + the approval discipline that supplies the org-leads CODEOWNER review.Consolidating means a fix or improvement to "how we review" lands once and benefits PRs, plans, and skills together — the highest-leverage shape for a small org.
Technical Opportunity
Refactor
pr-reviewaround an explicit artifact contract rather than assuming a PR:{artifact_type, content_ref, rubric, output_channel}.artifact_type=pr_diffreproduces today's behavior exactly (no regression);plan_jsonandskill_candidateare new types.prompts/cascade; the plan rubric is Epic Initiative: Adversarial plan-critic + structural gates for the initiative-planner (Bob) #597's fixed checklist; the skill rubric is Epic Initiative: Eval-gated, human-reviewed self-improving skills (SkillOpt-style) #581's eval criteria) — CODEOWNER-gated like any review logic.scripts/engine.shmodel routing and the existing idempotency markers / advisory-gate behavior; only the input adapter and rubric vary.pr_diffas the first (and initially only) registered type sopr-reviewconsumers are untouched, then registerplan_jsonto satisfy [Phase 2] Adversarial plan-critic pass against the fixed rubric #603, thenskill_candidatefor Initiative: Eval-gated, human-reviewed self-improving skills (SkillOpt-style) #581.Assessment
pr_diffmust be behavior-preserving and well-tested before new types land.Adversarial Review
Strongest objection:
pr-reviewis load-bearing production infrastructure — it supplies the required CODEOWNER approval on real PRs across the org. Refactoring it to chase reuse risks regressing the one agent we most depend on, to serve two initiatives that are still inert. Premature abstraction over two hypothetical artifact types is exactly how a clean reviewer turns into a leaky god-object.Rebuttal: The migration is explicitly behavior-preserving:
artifact_type=pr_diffis the only registered type at first and must pass the existing PR-review tests unchanged before anything else is added — so production review is untouched until the abstraction is proven. The abstraction isn't speculative: we have two concrete, already-planned second consumers (#603 and #581's gate), which is the bar for "extract, don't guess." And the alternative is worse for exactly the reliability reason raised — three separately-evolving review prompts drift apart, so a hardening fix (e.g. a prompt-injection guard) has to be applied three times and will be missed once. Consolidation reduces the attack/regression surface, it doesn't grow it. If the second consumer never materializes, we simply never register the second type — the refactor still leavespr-reviewcleaner.Suggested Next Step
Do the behavior-preserving extraction first: introduce the
{artifact_type, content_ref, rubric, output_channel}contract withpr_diffas the sole registered type, and prove the existing PR-review suite passes unchanged. Then registerplan_jsonto satisfy Epic #597's #603 (re-scoped to "invoke the review agent with the plan rubric"), andskill_candidatefor Epic #581's gate. Coordinate with the Safe Release Strategy so any change topr-reviewrides its existing ring/channel promotion rather than shipping straight tostable.Beta Was this translation helpful? Give feedback.
All reactions