diff --git a/plugins/review-suite/scripts/review_plan.py b/plugins/review-suite/scripts/review_plan.py index 6a4ad5e..6a60493 100644 --- a/plugins/review-suite/scripts/review_plan.py +++ b/plugins/review-suite/scripts/review_plan.py @@ -123,14 +123,20 @@ def load_plan_input(args: argparse.Namespace) -> tuple[str, str]: def build_prompt(plan_text: str) -> str: return ( - "Review this implementation plan and scope.\n\n" - "Focus on:\n" - "- simpler implementation paths\n" - "- reuse opportunities\n" - "- duplicate or redundant logic risk across planned and adjacent paths\n" - "- bad boundaries or unnecessary complexity\n\n" - "Return only actionable items with concrete edits or concrete design changes.\n" - "Do not give style-only feedback.\n\n" + "Review this implementation plan against the repository.\n\n" + "Before recommending a solution shape:\n" + "- identify the root cause, canonical owner, and invariants that must hold\n" + "- find materially distinct credible solution shapes supported by the repository\n" + "- compare those shapes; if one solution is clearly the only credible one, say so " + "instead of inventing alternatives\n" + "- recommend the best shape in this order: contract correctness; canonical " + "ownership; fewer durable concepts and less coupling; justified blast radius and " + "validation; then diff minimality only as a tie-breaker\n\n" + "End with exactly one verdict: PROCEED, REVISE, or RETHINK.\n" + "Support it with concise repository evidence and actionable plan changes.\n" + "Use PROCEED when the proposed shape is best, REVISE when bounded plan edits are " + "enough, and RETHINK when the root cause, owner, or solution shape should change.\n" + "Do not give style-only feedback or treat RETHINK as scope authority.\n\n" "=== BEGIN PLAN ===\n" f"{plan_text.strip()}\n" "=== END PLAN ===" diff --git a/plugins/review-suite/skills/review-plan/SKILL.md b/plugins/review-suite/skills/review-plan/SKILL.md index 2f3a4dc..bddbada 100644 --- a/plugins/review-suite/skills/review-plan/SKILL.md +++ b/plugins/review-suite/skills/review-plan/SKILL.md @@ -20,4 +20,8 @@ Rules: - Prefer `--input-file`. - With no input flags, the script reads `task_plan.md` from the current directory; stdin is accepted for pipes. - Use once per meaningful plan draft. +- Inspect the repository to identify root cause, canonical ownership, and relevant invariants before recommending a shape. +- Compare only materially distinct credible solution shapes. Accept one obvious solution instead of inventing alternatives. +- Rank contract correctness, canonical ownership, durable concepts/coupling, and justified blast radius/validation ahead of diff size; use diff size only as a tie-breaker. +- Return exactly one `PROCEED`, `REVISE`, or `RETHINK` verdict with concise evidence and actionable changes. `RETHINK` recommends reconsideration; it does not grant scope authority. - Verify findings before changing the plan. diff --git a/plugins/review-suite/tests/test_review_local.py b/plugins/review-suite/tests/test_review_local.py index 1edfb93..77f6503 100644 --- a/plugins/review-suite/tests/test_review_local.py +++ b/plugins/review-suite/tests/test_review_local.py @@ -636,6 +636,24 @@ def test_agent_wrapper_help_keeps_useful_targeting_controls_visible() -> None: assert "--status" in review.build_parser().format_help() +def test_review_plan_prompt_chooses_the_best_credible_solution_shape() -> None: + prompt = review_plan.build_prompt("Keep the existing wrapper.") + + for requirement in ( + "root cause, canonical owner, and invariants", + "materially distinct credible solution shapes", + "only credible one", + "contract correctness", + "diff minimality only as a tie-breaker", + "exactly one verdict: PROCEED, REVISE, or RETHINK", + "RETHINK as scope authority", + ): + assert requirement in prompt + assert prompt.endswith( + "=== BEGIN PLAN ===\nKeep the existing wrapper.\n=== END PLAN ===" + ) + + def test_local_review_wrappers_expose_short_wsl_flag() -> None: for help_text in ( review_followup.build_parser().format_help(),