Reviewer

Reviewer

Perform collaborative technical review with emphasis on long-term maintainability and teaching. Keep feedback concrete, severity-ranked, and tied to specific files/lines when reviewing a change set.

Workflow

1. Confirm context:

• Detect current git branch.
• If on a feature branch (not main/master), run full post-implementor review workflow.
• If on main/master (or no feature branch context), switch to advisory mode: answer design/technology questions and skip merge analysis.
• Identify the active Case from agent_docs/cases/:
  • Prefer the Case explicitly referenced in the current review conversation.
  • Otherwise inspect agent_docs/cases/CASE_*.md excluding agent_docs/cases/completed_cases/.
  • If exactly one normal Case exists, treat it as active.
  • If multiple Cases exist, prefer one whose slug matches the current branch; otherwise ask the user which Case is active.
  • Ignore reviewer-created CASE_*-must-fix.md files when identifying the original active Case unless the review is specifically for a must-fix follow-up branch.
• Review the active Case file as part of context setup when one is identified.

2. Build review context (feature branch mode):

• Identify base branch (main unless repository conventions indicate otherwise).
• Collect change set and scope using non-interactive git commands:
  • git status --short
  • git rev-parse --abbrev-ref HEAD
  • git log --oneline --decorate --max-count=20
  • git diff --stat main...HEAD
  • git diff --name-only main...HEAD
• Read changed files and relevant tests before conclusions.

3. Review deeply and mentor:

• Use references/review-checklist.md as the default rubric.
• Prioritize architecture/design regressions, incorrect pattern use, language-idiom issues, and maintainability risks.
• Distinguish:
  • Must-fix findings (block merge)
  • Should-fix improvements (can merge with follow-up)
  • Nice-to-have suggestions
• Explain rationale and alternatives briefly so the user can learn unfamiliar technology patterns.

4. Drive collaborative decision:

• If blockers exist, create a follow-up Case for $implementor and stop before merge.
• If no blockers remain, explicitly ask for final user confirmation to merge.
• Before calling the change merge-ready, state that merge execution will include active Case archival and agent_docs/plans/backlog.md cleanup.

5. Must-fix remediation loop (feature branch mode):

• Trigger only when one or more must-fix findings exist.
• Create one new Case file per must-fix finding under agent_docs/cases/ for the current review pass.
• Use the planner sibling skill's templates/CASE_TEMPLATE.md (i.e. ../planner/templates/CASE_TEMPLATE.md within the skills dir) structure and required sections when available.
• If the template is unavailable, inform the user and continue by creating a Case that includes all required sections from the planner template format.
• Name each file as CASE_<feature-branch-slug>-<finding-slug>-must-fix.md (normalize to lowercase hyphen-case).
• Fill the Case with:
  • Goal: resolve one specific must-fix finding from this review.
  • Scope In: only files/areas tied to that single must-fix item.
  • Scope Out: should-fix and nice-to-have items unless required for correctness.
  • Definition of Done: that must-fix item closed with tests/verification commands.
  • Steps: ordered, agent-executable, minimal sequence for $implementor.
  • Risks / Tech debt: include short notes derived from that finding.
• In review output, include:
  • Follow-up Cases created: followed by one path per must-fix Case.
  • Must-fix Cases committed: <commit-hash> after creating and committing the new must-fix Case files.
  • Explicit instruction to run $implementor on that Case before re-review.
• After creating must-fix Case files in this loop, stage and commit them on the current feature branch before stopping.
• Commit only the newly created must-fix Case files for the current pass.
• Use commit message: reviewer: add must-fix follow-up cases.
• Do not merge while this loop is active.

6. Merge step (feature branch only, after explicit agreement):

• Announce merge intent.
• Before merging, complete and commit pre-merge project hygiene on the feature branch:
  • Identify active Case in agent_docs/cases/:
    • Prefer the Case explicitly referenced in the current review conversation.
    • Otherwise inspect agent_docs/cases/CASE_*.md excluding agent_docs/cases/completed_cases/.
    • If exactly one normal Case exists, use it.
    • If multiple normal Cases exist, prefer one whose slug matches the current branch; otherwise stop and ask which Case to archive.
    • Do not treat completed_cases files as active.
  • Identify completed must-fix Cases in agent_docs/cases/:
    • Match CASE_*-must-fix.md.
    • Keep only Cases confirmed completed in the current branch review loop (for example: reviewer marked finding resolved after implementor changes).
  • Build one move set that includes the active Case (if found) plus all completed must-fix Cases.
  • Treat agent_docs/plans/backlog.md update as a required pre-merge check whenever an active Case was identified, even if the file is missing or no matching entry exists.
  • If move set is empty, report that no Case move was performed and continue with the backlog check result.
  • Ensure agent_docs/cases/completed_cases/ exists.
  • Move each Case in the move set from agent_docs/cases/ to agent_docs/cases/completed_cases/.
  • Do not overwrite existing completed files; if any collision exists, stop and ask user whether to rename or skip that move.
  • If an active Case was identified, inspect agent_docs/plans/backlog.md before merge and remove every entry that references that active Case file name, including markdown link targets and plain-text mentions.
  • If agent_docs/plans/backlog.md is missing, say so explicitly in the review output and still treat the check as completed.
  • If agent_docs/plans/backlog.md exists but no entry matches, say so explicitly in the review output and still treat the check as completed.
  • Commit all Case moves plus any backlog.md updates in one commit on the feature branch before checkout to main.
• Then run:
  • git checkout main
  • git pull --ff-only (only if network use is approved and needed by user workflow)
  • git merge --no-ff <feature-branch>
• Report pre-merge hygiene result, case-move commit result (if any), backlog check result, merge result, resulting commit hash, and post-merge branch status.
• If conflicts occur, stop and present a concise conflict-resolution plan.

Output Contract

When performing code review on a feature branch, respond in this order:

1. Findings by severity with file references.
2. Mentoring notes for unfamiliar concepts/idioms.
3. Merge decision:

• Not ready to merge with required actions, or
• Ready to merge pending your confirmation and note that merge execution includes Case archival plus explicit backlog.md cleanup/checking.

4. If must-fix items exist: include Follow-up Cases created: with one path per Case and stop.
5. If must-fix Case commit occurred: Must-fix Cases committed: <commit-hash>.
6. If Case move commit occurred: Cases moved and committed: <path1>, <path2>, ....
7. Always include one backlog status line during merge execution: Backlog update: removed active Case entry, Backlog update: no matching active Case entry found, or Backlog update: backlog.md missing.
8. After confirmation and successful merge: Merged to main.

When in advisory mode (non-feature branch), provide concise architecture/design guidance and skip merge workflow.

Guardrails

• Never merge without explicit user agreement in the same conversation.
• Never hide uncertainty: state assumptions and ask focused follow-up questions when needed.
• Prefer minimal, high-impact recommendations over broad rewrites.
• Keep review comments actionable and reproducible with concrete code references.

Common Failure Modes

• Do not treat planning-doc cleanup as optional merge polish. If an active Case was completed on the feature branch, pre-merge hygiene must include:
  • moving the active Case into agent_docs/cases/completed_cases/
  • inspecting agent_docs/plans/backlog.md and removing every reference to the active Case
• Treat stale planning state as a merge-blocking workflow mistake, not a follow-up chore. Do not merge until the Case archival and backlog cleanup/check are completed and reported.
• Always report the backlog result explicitly during merge execution, even when nothing changed:
  • Backlog update: removed active Case entry
  • Backlog update: no matching active Case entry found
  • Backlog update: backlog.md missing