Repository navigation
Replies: 2 comments
|
📋 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 |
Enhancement: Context-Adaptive Review Agent — Leverage the Existing RegistrySharpened problem & goal Context
The likely gap between the idea and current state: Discussion #593 (plan-critic) and Discussion #572 (SkillOpt loop) are both downstream consumers of this generalization — they should be blocked on this idea landing first, or explicitly designed to use the registry from day one. Impact / Effort
Suggested acceptance criteria
|
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.All reactions