pr-review

PR Review

Produce a high-signal review: findings a reviewer would act on, classified and ordered, ending in a clear verdict. The kit instructions (instructions/secure-coding.md, instructions/testing-standards.md) define what counts as blocking.

Scope the diff

• Reviewing an existing PR: fetch the diff and description from the configured host (prHost in .claude/dev-kit.json). github: gh pr diff <pr> + gh pr view <pr>. bitbucket: GET /2.0/repositories/{ws}/{repo}/pullrequests/{id}/diff and /pullrequests/{id} (REST, token from env). gitlab: glab mr diff <id> + glab mr view <id>.
• Reviewing the working tree (self-review before PR): git diff against the base branch, including staged changes.
• Read the linked ticket's acceptance criteria — a diff can be flawless and still not do what the story asked.
• Write the PR intent — one line on what this PR is for and what it deliberately leaves alone. It's the ruler for scope: a real defect inside the intent blocks; a valid concern outside it is a note or a follow-up, not a reason to expand the PR.

Review dimensions (in priority order)

1. Acceptance criteria: does the change actually satisfy each criterion? List any criterion not covered.
2. Correctness: behavioral regressions, broken edge cases, wrong logic. Read the code, don't skim the diff.
3. Contract drift: routes, payloads, enums, schemas, validation, status codes — every side that depends on the contract updated together.
4. Security: apply the checklist in instructions/secure-coding.md (auth on new endpoints, secrets, input validation, data exposure). Any automatic-blocker present is a blocking finding.
5. Tests (adaptive — judge against the project's own setup, see instructions/testing-standards.md): when the project has tests, every behavioral change has one that would fail without it and touched files stay at the project's bar (default ≥ 95%, no regression — run coverage-check if evidence is missing); when it does e2e, user-facing changes have e2e coverage with edge cases. A project with no test/e2e setup is not a blocking finding — flag it as a recommendation. Test-quality violations from instructions/testing-standards.md (assertion-free tests, suppressions, deleted/renamed tests) are findings.
6. Performance regressions introduced here (blocking): algorithmic blowups over collections that grow with usage; N+1 queries or per-item network calls on a request path; unbounded result sets / memory / missing pagination; blocking work on a hot path; a new query filtering/joining on an unindexed column. Not this: micro-optimizations or "could be faster" with no mechanism.
7. Duplication introduced by this PR (blocking): new code reimplementing logic already in the repo, or copy-paste between the files this PR adds — fix by reusing/extracting once. Not this: two blocks that merely look alike and are about to diverge; pre-existing duplication is a follow-up at most.
8. Accessibility (conditional — evaluate only when the diff changes user-facing UI in a frontend stack: changed components/templates/JSX/HTML/CSS in a node/angular/react/vue-style project. Skip entirely for backend or non-UI diffs — no cost when it doesn't apply). Honors a11y in .claude/dev-kit.json: auto (default — run on user-facing frontend diffs) · required (blocking gate) · off (never run). Check the high-value, low-effort basics on the changed markup only: images have meaningful alt, form controls have associated labels, interactive elements have an accessible name, keyboard/focus works (no click-only handlers, visible focus), no obvious color-contrast failures, ARIA present where needed and not misused. If the repo already runs a11y tooling (axe-core, eslint-plugin-jsx-a11y, Lighthouse), use its output; never scaffold one. Target WCAG 2.2 level AA, and cite the specific Success Criterion in each finding (e.g. missing alt → WCAG 1.1.1 (A), low contrast → WCAG 1.4.3 (AA)) so it's verifiable, not vague. Recommendation by default; blocking only when a11y: required.
9. Maintainability: only issues that materially affect future changes — no style nitpicks a formatter or linter should catch.