review-pr

Review PR

Three rules that override everything else in this skill:

• Never auto-post to GitHub. Even non-interactively or in an "auto" mode, you MUST NOT post review comments without explicit sign-off from the user (step 10). The one exception: the user asked for posting when invoking the skill (e.g. "review PR 42 and post the comments").
• Verify before you report. Sub-agent findings are unverified candidates. Every one passes through triage (step 6) before it reaches the review. This is how we keep out the noise.
• Never modify the PR branch or push code. This skill reads and reports.

Severity scale

Used for individual findings and for dimension scores:

• 🔴 Red (Blocker): fatal — the PR must not merge until it is fixed. Data loss or corruption, a security hole, a crash or broken core path, an unintended breaking change.
• 🟠 Amber (Important): significant but not fatal — a real bug on a non-critical path, a meaningful gap in error handling, a missing test for important behaviour.
• 🟡 Yellow (Minor): real but minor — naming, small readability improvements, minor observability gaps. Does not block merge.
• 🟢 Green: no issues. A dimension score only — no individual finding is ever green, because anything that trivial is discarded in triage.

Writing for a reader outside this repository

This governs every word you write in the review document and in the GitHub comments — findings, summaries, rationales, diagram labels alike.

Once you have read the code you can no longer tell which parts of your explanation only work for someone who has also read it. Assume the reader has not. The goal is that they can decide whether they need to open the codebase at all to agree or disagree with you — not that they must open it before your sentence parses.

• Say what the code is for before saying what is wrong with it. A reader who does not know the purpose cannot tell which details matter. "Renaming a tenant used to 404 until the next restart" before "the loader keeps a second map keyed by the old alias".
• Name things by what they do in the world, then by symbol. "the filter that rejects unauthenticated requests (AuthFilter.java:52)", not "the filter". This is the same rule step 7 applies to diagram nodes.
• Give every symbol a location on first mention. Class, method, file, flag, table, environment variable: path:line, once, the first time it appears.
• Never coin a term. Use the name the codebase already uses — grep for it rather than inventing one. No "ownership boundary", "the resolution layer", "the new flow" unless the repository says so.
• Make every claim checkable. A line, an error string, a command, a number. An assertion the reader cannot verify is one they have to take on trust, which is exactly what this skill exists to avoid.
• Do not teach the language or the framework. Context is a clause, not a preamble. No paragraph explaining dependency injection at the top of a Spring finding.