pr-review

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.