Code Review

Code Review

Three-stage method: produce findings, filter them, report verdict.

Stages 1 and 2 are internal reasoning. Never print them. Only the Stage 3 report reaches the user.

Review target

• User named a target (file, directory, feature, commit range, MR) → review that.
• No target named → review current diff (uncommitted changes, or the branch diff against the main branch).

Read the code with workspace tools. When code is supplied inline in the prompt, review what is supplied and read further files only for context.

Stage 1 — Reviewer

Review target. Collect findings.

Review algorithm (follow this order)

1. Read project rules — MANDATORY. Read every instruction file covering the reviewed code: root AGENTS.md plus nested ones in the touched directories, and any docs they point to. Project rules override skill rules on conflict
2. Load skills — MANDATORY. Load workspace skills matching the reviewed stack and area (framework, language, UI text, testing). Read their rule files via workspace tools, never from memory
3. Big picture — scan the whole change set for cross-file context
4. Analyze change — split core change vs plumbing. Core design wrong → focus there
5. Full-file context — check the change makes sense: imports used, removed items not referenced, control flow correct
6. API compatibility — check callers match changes
7. Apply skill rules — check violations from this change only, not pre-existing code
8. Apply project rules — check violations of AGENTS.md
9. Deduplicate — skip findings already covered by existing review comments
10. Verify relevance — each finding must fit actual code context (file type, framework, stack), not generic rule mismatch
11. Verify against source — re-read the cited lines. Drop findings referencing code, symbols, imports, or behavior that does not exist in the file. Never describe code from memory or assumption
12. Check the fix — proposed fix must be the smallest change that solves the problem. Drop or rewrite fixes that add abstractions, layers, options, or generalization the problem does not need
13. Keep — only confidence >= 80, reviewed code only, no linter catches or nitpicks. Fix line says WHAT to fix in 1 sentence, not HOW

Focus on

• Data loss, security holes (XSS, injection, secrets in code)
• Runtime crashes, unhandled errors, null/undefined access
• Broken core logic, wrong conditions, off-by-one errors
• Race conditions, missing await, wrong async handling
• API contract violations, wrong types passed between components
• Functions whose behavior contradicts their name or return type
• Stub or placeholder implementations merged without TODO/FIXME markers
• Public functions where invalid but plausible input gives silently wrong results instead of error
• Unreachable code paths or dead branches caused by change
• New exports or public API surface not consumed anywhere
• Obvious performance pitfalls: O(n²) or worse in loops over collections, sync blocking in async paths, missing pagination on unbounded queries, repeated expensive operations that should be cached or batched
• Hardcoded values that clearly should be configurable or computed (e.g., hardcoded URLs, credentials, environment-specific paths, magic numbers used as thresholds or limits) — flag as suggestion unless marked TODO
• Functions that claim operation by name but have trivial implementation that does not do it (e.g., isFileExists returning constant without checking filesystem, sendEmail with empty body) — flag unless marked TODO
• Misused dependencies — components, functions, composables or utilities called with wrong arguments, wrong types, ignored return values, missing required props/options, or invoked in way that violates their contract (when signature visible in provided context)
• Reinvented shared utilities — new logic that duplicates existing shared helper/component/composable from project (e.g., hand-rolled deep clone, date formatter, query builder, or validator) when equivalent visible in provided context; flag and point to existing one to reuse