---
name: pr-review
description: Zero-tolerance pull request review. Every issue is a blocker. Use when reviewing PRs for merge readiness.
---

Review the pull request with a **zero-tolerance standard**. Every issue you find is a blocker — there is no such thing as a "minor issue" or "non-blocking suggestion". Either the PR is flawless and ready to merge, or it has problems that MUST be fixed before merging. Do not approve a PR with caveats like "ready to merge but consider..." or "minor nit:". If you would mention it, it must be fixed.

**Your verdict must be one of**:
- ✅ **APPROVE** — The code is near-perfect. No issues found. Merge immediately.
- ❌ **CHANGES REQUIRED** — Issues found. List every one. All must be fixed before re-review.

Never approve with suggestions. Never say "looks good overall but...". If there's a "but", it's CHANGES REQUIRED.

## Scope and Refactor Standard

Code touched by a PR must be merge-and-forget quality — no rough edges, no avoidable duplication, no unconventional idioms.
Do not require refactors of untouched code unless they have clear immediate ROI.

- Require a broader refactor only when it has clear immediate ROI:
  - It removes active duplication in current code paths.
  - It creates one clear consolidation point.
  - It reduces net complexity after the change.
  - It is validated by meaningful tests in the same PR.
- Do not require broad refactors for hypothetical future needs.

## Review checklist

- **Code cleanliness**: Is the implementation clean and well-structured?
- **DRY principle**: Does it avoid duplication?
- **Architectural smells**: Identify scattered logic or the same policy/resolution logic being defined in multiple places instead of one source of truth.
- **Code reuse**: Are there parts that should be reused from other places?
- **Organization**: Is everything in the right place?
- **Consistency**: Is it in the same style as other parts of the codebase?
- **Simplicity**: Is it not over-engineered? Remember KISS and YAGNI. No dead code paths and NO defensive programming. No unnecessary try-excepts.
- **No pointless wrappers**: Identify functions/methods that just call another function and return its result. Callers should call the underlying function directly instead of going through unnecessary indirection.
- **Functional style**: Does it prefer functions over classes where appropriate? Are dataclasses used instead of raw dicts?
- **Imports**: Are all imports at the top of the file (not inside functions, unless avoiding circular imports)?
- **User experience**: Does it provide a good user experience?
- **PR**: Is the PR description and title clear and informative?
- **Docs**: Are docs updated anywhere the change affects users, operators, developers, configuration, tooling, workflows, or behavior that someone would need to learn later? Missing required docs is a blocker.
- **Tests**: Are there tests, and do they cover the changes adequately? Are they testing something meaningful or are they just trivial? On NixOS, run them inside `nix-shell shell.nix` (or use `nix-shell shell.nix --run 'uv run pytest -x -n 0 --no-cov -v'`). If `<nixpkgs>` is unresolved, retry with `nix-shell -I nixpkgs=/nix/var/nix/profiles/per-user/root/channels/nixos shell.nix`.
- **Live tests**: If feasible, test the changes with a local Matrix stack (`just local-matrix-up`) and the Matty CLI to verify agent behavior end-to-end.
- **Rules**: Does the code follow the project's coding standards and guidelines as laid out in @CLAUDE.md?

## How to review

Look at `git diff origin/main..HEAD` for the changes made in this pull request.
