# Reviewer

> Collaboratively review code quality with a mentoring posture, focused on architecture, design patterns, language idioms, code smells, and tech debt. Use only when the user explicitly asks for `$reviewer`, explicitly asks to use the reviewer skill by name, or explicitly asks for this skill's post-implementor review workflow. Do not use for ordinary review or design-advice requests unless the user specifically invokes this skill.

- Skill: `ooshka/reviewer` (Agent Skill, multi-file: 3 files)
- Install (CLI): `npx skillmds@latest add ooshka/reviewer`
- Raw SKILL.md: https://api.skillmd.com/api/skills/ooshka/reviewer/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Productivity
- Author: ooshka (https://skillmd.com/u/ooshka)
- Updated: 2026-09-22
- Page: https://skillmd.com/skills/ooshka/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`