Code Review Skill
Reviews diffs and PRs the way a senior engineer would: intent first, structure second, line-by-line third. Produces feedback grouped by severity (🔴 must / 🟡 should / 🟢 nit) with concrete suggestions. Does NOT rewrite the author's code — it directs, it does not replace.
When to Use
- Reviewing a pull request, branch diff, or staged changes.
- Auditing changes before merge.
- Self-reviewing the agent's own output before presenting it.
- Hunting for security, correctness, or performance regressions.
- The user asks "does this look right?" about a code change.
Prerequisites
- The
view, grep, and glob Copilot tools to read code.
git available so diffs can be inspected (git diff main, git log).- Read access to the repository being reviewed.
- For PR-scoped review:
gh CLI authenticated to fetch PR metadata.
How to Run
1. Read the PR title, description, and linked issue.
2. List changed files (`git diff main --stat`) and assess scope.
3. Walk the diff with the `view` tool, file by file.
4. Apply the severity rubric below.
5. Emit a structured review block with concrete fixes.
Quick Reference
| Phase |
Tool |
What to check |
| Intent |
PR description, linked issue |
Why does this change exist? |
| Scope |
git diff main --stat |
Files changed match stated intent |
| Correctness |
view on hot paths |
Logic, null/edge handling, off-by-one |
| Security |
grep for secrets, raw SQL, eval |
Injection, auth bypass, leaked secrets |
| Performance |
view on loops, queries |
N+1, unbounded allocations, missing indexes |
| Tests |
view on test files |
New paths actually covered |
Procedure
Step 1: Understand Intent
Read the PR description, commit messages, and any linked issue. If intent is unclear, stop and ask the author — reviewing code whose purpose you don't understand wastes everyone's time.
Step 2: Structural Pass
Use git diff main --stat to size the change. Check:
- Does the file list match the stated scope?
- Are new dependencies justified?
- Does the change fit existing patterns in the repo?
- Is the deletion ratio healthy? (Good PRs often delete as much as they add.)
Step 3: Line-by-Line Pass
Walk each changed file with view. Apply the rubric:
| Category |
Look for |
| Correctness |
Logic errors, off-by-one, null handling, missing edge cases |
| Security |
Injection, auth bypass, secrets in code, weak input validation |
| Performance |
N+1 queries, unbounded allocations, missing indexes |
| Readability |
Naming, complexity, comments (too few or too many) |
| Testing |
New paths actually covered, tests meaningful (not assert-true) |
| Error handling |
Errors handled or silently swallowed |
Step 4: Emit Structured Feedback
## Review Summary
### 🔴 Must Fix (Blocking)
- `path/to/file.ts:42` — <issue> + **Suggestion:** <concrete fix>
### 🟡 Should Fix (Non-Blocking)
- `path/to/file.ts:88` — <issue> + **Suggestion:** <better approach>
### 🟢 Nit (Optional)
- `path/to/file.ts:120` — <style or preference point>
### ✅ What's Good
- <Highlight something done well — positive reinforcement matters.>
Pitfalls
- DO NOT rubber-stamp ("LGTM 👍") without reading the diff — worse than no review.
- DO NOT block a PR over formatting if a linter exists — that is the linter's job.
- DO NOT rewrite the author's code in the review thread. Suggest direction; do not freelance.
- DO NOT miss the forest for the trees. Catching a typo while missing a SQL injection is a bad trade.
- DO NOT review code you do not understand. Read the surrounding code first.
Verification
1---2name: code-review-283description: Reviews diffs by severity to produce actionable feedback.4---56# Code Review Skill78Reviews diffs and PRs the way a senior engineer would: intent first, structure second, line-by-line third. Produces feedback grouped by severity (🔴 must / 🟡 should / 🟢 nit) with concrete suggestions. Does NOT rewrite the author's code — it directs, it does not replace.910## When to Use1112- Reviewing a pull request, branch diff, or staged changes.13- Auditing changes before merge.14- Self-reviewing the agent's own output before presenting it.15- Hunting for security, correctness, or performance regressions.16- The user asks "does this look right?" about a code change.1718## Prerequisites1920- The `view`, `grep`, and `glob` Copilot tools to read code.21- `git` available so diffs can be inspected (`git diff main`, `git log`).22- Read access to the repository being reviewed.23- For PR-scoped review: `gh` CLI authenticated to fetch PR metadata.2425## How to Run2627```text281. Read the PR title, description, and linked issue.292. List changed files (`git diff main --stat`) and assess scope.303. Walk the diff with the `view` tool, file by file.314. Apply the severity rubric below.325. Emit a structured review block with concrete fixes.33```3435## Quick Reference3637| Phase | Tool | What to check |38|---|---|---|39| Intent | PR description, linked issue | Why does this change exist? |40| Scope | `git diff main --stat` | Files changed match stated intent |41| Correctness | `view` on hot paths | Logic, null/edge handling, off-by-one |42| Security | `grep` for secrets, raw SQL, eval | Injection, auth bypass, leaked secrets |43| Performance | `view` on loops, queries | N+1, unbounded allocations, missing indexes |44| Tests | `view` on test files | New paths actually covered |4546## Procedure4748### Step 1: Understand Intent4950Read the PR description, commit messages, and any linked issue. If intent is unclear, stop and ask the author — reviewing code whose purpose you don't understand wastes everyone's time.5152### Step 2: Structural Pass5354Use `git diff main --stat` to size the change. Check:5556- Does the file list match the stated scope?57- Are new dependencies justified?58- Does the change fit existing patterns in the repo?59- Is the deletion ratio healthy? (Good PRs often delete as much as they add.)6061### Step 3: Line-by-Line Pass6263Walk each changed file with `view`. Apply the rubric:6465| Category | Look for |66|---|---|67| Correctness | Logic errors, off-by-one, null handling, missing edge cases |68| Security | Injection, auth bypass, secrets in code, weak input validation |69| Performance | N+1 queries, unbounded allocations, missing indexes |70| Readability | Naming, complexity, comments (too few or too many) |71| Testing | New paths actually covered, tests meaningful (not assert-true) |72| Error handling | Errors handled or silently swallowed |7374### Step 4: Emit Structured Feedback7576```markdown77## Review Summary7879### 🔴 Must Fix (Blocking)80- `path/to/file.ts:42` — <issue> + **Suggestion:** <concrete fix>8182### 🟡 Should Fix (Non-Blocking)83- `path/to/file.ts:88` — <issue> + **Suggestion:** <better approach>8485### 🟢 Nit (Optional)86- `path/to/file.ts:120` — <style or preference point>8788### ✅ What's Good89- <Highlight something done well — positive reinforcement matters.>90```9192## Pitfalls9394- **DO NOT** rubber-stamp ("LGTM 👍") without reading the diff — worse than no review.95- **DO NOT** block a PR over formatting if a linter exists — that is the linter's job.96- **DO NOT** rewrite the author's code in the review thread. Suggest direction; do not freelance.97- **DO NOT** miss the forest for the trees. Catching a typo while missing a SQL injection is a bad trade.98- **DO NOT** review code you do not understand. Read the surrounding code first.99100## Verification101102- [ ] Every changed file was opened with `view` at least once.103- [ ] Each issue cites a file and line number.104- [ ] Severity buckets used (🔴 / 🟡 / 🟢 / ✅).105- [ ] At least one positive observation included.106- [ ] If 🔴 issues exist, the PR is explicitly marked NOT ready to merge.
gabrielmoreira