Code Review Mode

Recommended model tier: smart (opus) - this skill requires complex reasoning

Comprehensive code review covering quality, security, and maintainability.

Review Checklist

Decision Conformance

Recorded decisions are this project's binding architectural commitments. Check them first — a conformance violation outranks any style or quality issue below.

• Called decision_list (or noted the fallback) before reading any code
• Every changed file checked against every decision that could apply to it
• Every ✅ backed by a cited check, not by inspection alone
• Every decision on its own line, including the ones that do not apply — no bulk dismissals
• "Decisions loaded: N" counted, not estimated, and matching the number of lines
• No decision violated by the change under review
• Change does not extend a pre-existing violation
• Where a decision seems wrong rather than the code, that is stated explicitly

Code Quality

• Clear naming (variables, functions, classes)
• Single responsibility (functions do one thing)
• DRY (no unnecessary duplication)
• Appropriate abstraction level
• Error handling coverage
• Edge cases considered

Security (OWASP Top 10)

• Input validation (no injection vulnerabilities)
• Authentication checks (routes protected)
• Authorization (proper access control)
• Sensitive data handling (no secrets in code)
• SQL/NoSQL injection prevention
• XSS prevention (output encoding)
• CSRF protection
• Secure dependencies (no known vulnerabilities)

Maintainability

• Code is readable without comments
• Comments explain "why" not "what"
• Consistent with codebase patterns
• Tests cover critical paths
• No dead code

Performance

• No N+1 queries
• Appropriate caching
• No memory leaks
• Efficient algorithms

Context-Efficient Reading

Choose retrieval from the review evidence already available. Inspect the diff and known ranges directly; read full files when they are small or most contents are relevant. Use navigation tools when they resolve missing context, since extra rounds can outweigh smaller responses:

• code_outline -- Collapsed skeleton with signatures and line ranges. Useful for navigating unfamiliar large files.
• code_symbols -- Quick symbol list when you only need names and kinds.
• code_search / code_references -- Find symbol definitions or callers across the codebase.
• Read with offset/limit -- Read known ranges from the diff, search results, or an outline.
• code_read_symbol -- Read known symbols; batch several names with symbols (up to 10).
• Grep -- Find patterns in code content (loops, queries, string literals) that the index doesn't cover.

For reviews spanning many files, consider using Task sub-agents (explore type) which run in their own context and return summaries.

Review Process

1. Load the decisions - Call decision_list. It is authoritative: live, and complete with Details and References. The "Project Decisions" block in session context is a convenience copy — a snapshot taken at session start that omits Details and References and goes stale the moment a decision changes, so it is a fallback when the tool is unavailable, never the source you check against. Use decision_get for one decision's full text when the list entry is not enough to judge conformance. See "Decision Conformance Pass" below for how to check them.
2. Inspect changed files - Read the diff and identify areas needing surrounding context. Use code_outline when unfamiliar large-file structure would help locate that context.
3. Read relevant code - Read the functions/sections needed for detailed review, using known ranges or symbol reads. Read a full file when its size or the review scope warrants it.
4. Search for context - Use code_search, code_references, and Grep:
   • code_search — Find related function/class/type definitions by name
   • code_references — Find all callers/usages of a modified symbol (exact name match)
   • Grep — Find code patterns in bodies (error handling, SQL queries, security-sensitive calls)
5. Check integration - How does it fit the larger system?
6. Run static analysis - Use lsp_diagnostics, ast_grep if available
7. Document findings - Use severity levels

Decision Conformance Pass

This is the part of the review that no static analyzer can do for you. assess-findings grades findings the analyzers already produced; this pass reads the code and looks for violations nothing flagged.

Scope. Default to the changed code — the diff, or the files the user named. Say which scope you used. Only sweep the whole repo when the user asks for it; a full sweep is slow and mostly re-reports known debt.

Method. For each decision, work out what would falsify it, then go looking for that:

A decision whose text gives you nothing falsifiable is not checkable here. Say so plainly rather than inventing a check — and mention it to the user, because a decision that cannot be checked is a decision that cannot be enforced.

Evidence is required, including for a pass. Every ✅ must cite what produced it — the Grep pattern you ran, the code_references call, the findings_list filter, or the specific lines you read. "Looks fine" is not a check, and a conformance block full of uncited ticks is worse than no block at all: it reports assurance you did not earn.

Every decision gets its own line. Report all of them, individually, with a reason — including the ones that do not apply. Never collapse several into a category dismissal ("the remaining N are out of scope"): a bulk ➖ is the same evasion as an uncited ✅, and it is how a decision that did apply gets swept up with the ones that did not. A ➖ still needs its own evidence — the path check or grep that shows the diff does not touch what the decision governs. Grouping several decisions under one shared piece of evidence is fine when the evidence genuinely covers them all (one path check disposing of every survey-* decision, say), as long as each still has its own line naming it.

State the count and make it match. "Decisions loaded: N" must equal the number of lines below it. Count them; do not estimate.

A hidden finding is not an absent one. Any findings_* call made to discharge a decision must pass include_accepted=true; the default hides accepted findings, so a single earlier acceptance turns every later review into a cited, confident, wrong ✅ — the exact failure the evidence rule above exists to prevent. When an accepted finding is decision-linked, report the acceptance rather than passing over it: it may predate the decision, and nothing re-examines it otherwise.

Reporting. A confirmed violation is 🔴 Critical regardless of how small the code change is, and must name the decision topic. If the code looks right and the decision looks wrong or stale, say that instead of forcing a violation — recommend /decide on that topic.

MCP Tools

Use these tools during review:

• mcp__plugin_aide_aide__code_outline - Navigate unfamiliar large files with signatures and line ranges when needed
• mcp__plugin_aide_aide__code_search - Find symbols related to changes (e.g., code_search query="getUserById")
• mcp__plugin_aide_aide__code_symbols - List all symbols in a file being reviewed
• mcp__plugin_aide_aide__code_references - Find all callers/usages of a modified symbol
• mcp__plugin_aide_aide__decision_list - Load first. The authoritative list of recorded decisions — current, and including Details and References
• mcp__plugin_aide_aide__decision_get - Full text of one decision by topic, including rationale
• mcp__plugin_aide_aide__memory_search - Check for related past learnings, gotchas, or issues
• mcp__plugin_aide_aide__findings_search - Search static analysis findings (complexity, secrets, clones) related to changed code
• mcp__plugin_aide_aide__findings_list - List findings filtered by file, severity, or analyzer
• mcp__plugin_aide_aide__findings_stats - Overview of finding counts by analyzer and severity

Output Format

## Code Review: [Feature/PR Name]

### Summary

[1-2 sentence overview]

### Decision Conformance

Scope checked: [diff / named files / whole repo]
Decisions loaded: N

- ✅ `<topic>` — conforms — checked by: [grep pattern / tool call / lines read]
- ❌ `<topic>` — violated at `file:line`: [what the code does vs what the decision requires]
- ➖ `<topic>` — n/a: [the path check or grep showing the diff does not touch what it governs]
- ➖ `<topic>` — not checkable: [nothing falsifiable in the decision text as written]

One line per decision, no exceptions — the list length must equal "Decisions loaded".
Decisions sharing one piece of evidence still get one line each.

(State "no decisions recorded" if `decision_list` returns none.)

### Findings

#### 🔴 Critical (must fix)

- **[Issue]** `file:line`
  - Problem: [description]
  - Fix: [recommendation]

#### 🟡 Warning (should fix)

- **[Issue]** `file:line`
  - Problem: [description]
  - Fix: [recommendation]

#### 🔵 Suggestion (consider)

- **[Issue]** `file:line`
  - Suggestion: [recommendation]

### Security Notes

- [Any security-specific observations]

### Verdict

[ ] ✅ Approve
[ ] ⚠️ Approve with comments
[ ] ❌ Request changes

Severity Guide

Failure Handling

If unable to complete review:

1. Missing files - Report which files could not be read
2. Ambiguous scope - Ask user to clarify what code to review
3. Large changeset - Break into smaller chunks, review systematically
4. No decisions recorded - decision_list returns none; skip the conformance pass, say so in the report, and continue with the rest of the review. Never invent decisions
5. decision_list unavailable - Fall back to the injected "Project Decisions" block and say in the report that you used it, since it omits Details and References and may predate a decision changed this session. If neither is available, report the conformance pass as not run rather than passing it

Reporting blockers:

## Review Status: Incomplete

### Blockers

- Could not access: `path/to/file.ts` (permission denied)
- Missing context: Need to understand `AuthService` implementation

### Partial Findings

[Include any findings from files that were reviewed]

Verification Criteria

A complete code review must:

1. Inspect all changed files - Examine every file in the diff/scope
2. Read critical sections - Read flagged areas and enough surrounding code to evaluate their behavior
3. Check for related code - Use code_search and code_references to find callers/callees
4. Verify test coverage - Check if tests exist for critical paths
5. Document all findings - Even if no issues found, state that explicitly

Checklist before submitting review:

• All files in diff/scope have been examined
• Critical functions/sections and necessary context read in detail
• Related symbols searched (callers, implementations)
• Security checklist evaluated
• Findings documented with file:line references
• Verdict provided with clear reasoning