Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions plugins/review-suite/scripts/review_plan.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 ==="
Expand Down
4 changes: 4 additions & 0 deletions plugins/review-suite/skills/review-plan/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
18 changes: 18 additions & 0 deletions plugins/review-suite/tests/test_review_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down