# Code Review > Review code for quality, security, and style using structured checklists. Use when reviewing PRs, giving feedback on code, or auditing code quality. Use when this capability is needed. - Skill: `tomevault-io/code-review-7` (Agent Skill, multi-file: 2 files) - Install (CLI): `npx skillmds@latest add tomevault-io/code-review-7` - Raw SKILL.md: https://api.skillmd.com/api/skills/tomevault-io/code-review-7/raw - Safety review: pending (external: skill-scanner PASS, skillspector PASS) - Works with: Claude Code, Claude.ai, OpenAI Codex - Category: Security - Author: tomevault-io (https://skillmd.com/u/tomevault-io) - Updated: 2026-09-17 - Page: https://skillmd.com/skills/tomevault-io/code-review-7 --- # Code Review Review code against structured checklists for correctness, security, performance, and maintainability. Give actionable, prioritized feedback. ## When to Use - User asks for a code review, PR review, or feedback on code - User wants to check code quality, security, or style before merging - User is auditing a file or module ## Workflow 1. **Understand context**: What is the change for? (issue, feature, refactor) 2. **Read the diff**: What actually changed? 3. **Collect all issues—never stop at first error**: Review the entire scope and return every issue you find. Do not stop when you hit the first bug or suggestion. Use manage_todos, a scratch file, or spawn_subagent (for large PRs) to track issues as you go, then output the complete list. 4. **Gather usage context**: Before judging changes in isolation, gather enough context to give complete feedback: - **Surrounding code**: Read the function/module containing the change, not just the modified lines. What invariants, preconditions, or patterns exist? - **Call sites & usage**: How is this code called? Search for callers, imports, or usages. Are there edge cases or misuse patterns at call sites? - **Integration points**: Does this touch APIs, DBs, external services, or shared state? How does it fit into the larger flow? - **Tests**: If tests exist for this area, read them to understand expected behavior and coverage gaps. 5. **Run checklist**: Logic, security, performance, style, tests (using the gathered context) 6. **Consider language & framework best practices**: Does the code follow idiomatic patterns for the stack? 7. **Prioritize**: Critical → must fix; Suggestion → consider; Nice-to-have → optional 8. **Respond**: Summary + categorized comments + concrete suggestions + what was done well ## Using git_diff Efficiently The `git_diff` tool truncates large diffs (default 500 lines). For PRs with many files or 1k+ lines of changes: 1. **Get the file list first**: Call `git_diff` with `commit` (e.g. `main...HEAD`) and `nameOnly: true`. You receive `paths` — the full list of changed files. 2. **Fetch content by batch**: - **Small PR** (few files, under ~500 lines total): Call `git_diff` with just `commit` for the full diff. - **Large PR**: Call `git_diff` with `commit` and `paths` set to 5–10 files per batch (e.g. `paths: ["src/foo.ts", "src/bar.ts"]`). Use `maxLines: 2000` if batches are large. Iterate until you've reviewed all files. 3. **Prioritize**: Review highest-risk areas first (auth, input handling, external calls). Security-sensitive files warrant extra attention. Don't assume a single `git_diff` call shows everything. If the result says `truncated` or you only see a few files, use the batched workflow above. ## Review Checklist ### Correctness & Logic - [ ] Does it do what it claims? Edge cases? - [ ] Off-by-one, null/undefined, empty inputs? - [ ] Error handling: failures caught and handled? - [ ] Concurrency: races, deadlocks, shared state? ### Security - [ ] User input validated and sanitized? - [ ] No secrets in code or logs? - [ ] Auth/authz checked where needed? - [ ] Dangerous functions (eval, exec, SQL concatenation) avoided? ### Performance - [ ] Obvious inefficiency? (e.g. loop in loop when avoidable) - [ ] Large data: streaming, pagination, or limits? - [ ] Caching or repeated work that could be reused? ### Maintainability - [ ] Naming clear? Functions do one thing? - [ ] Duplication that could be factored? - [ ] Magic numbers/strings that should be constants? - [ ] Comments only where needed (why, not what)? ### Tests - [ ] New behavior covered by tests? - [ ] Tests readable and stable (no flake)? - [ ] Mocks/fixtures appropriate? ### Style & Conventions - [ ] Matches project style (lint, formatter)? - [ ] Imports organized? Dead code removed? ## Feedback Format Use a consistent severity and format: ```markdown ## Code Review: [PR/File name] ### Summary [1-2 sentences: overall assessment and main concerns] ### Well done - [Highlight specific things done very well—clear naming, solid error handling, idiomatic patterns, etc.] - [Recognize good use of language/framework features.] ### Critical (must address) - **[Location]** [Issue]. [Suggestion or fix.] - ... ### Suggestions (consider) - **[Location]** [Issue]. [Optional improvement.] - ... ### Nice to have - [Minor polish or future improvement.] ``` **Location**: file path, function name, or "line N" (if line numbers available). Be specific so the author can jump to it. **Suggestion**: Prefer concrete fix (code snippet or exact change) over vague "consider improving X." ## Tone - Respectful and constructive - Assume good intent; explain the "why" behind requests - Distinguish "this is wrong" vs "this could be clearer" - No nitpicking without value; batch trivial style nits ## What Not to Do - Don't skip the "Well done" section—acknowledging good work motivates and reinforces best practices - Don't demand personal style preferences unless they're project convention - Don't repeat what the diff already shows ("you added a function") - Don't leave only "LGTM" without at least a one-line summary - Don't mix severity: critical items must be clearly marked ## Small vs Large PRs - **Small**: Full checklist, quick pass. One round of feedback. - **Large**: Summarize by area (e.g. "Auth logic", "UI components"). Call out highest-risk areas first. Suggest splitting if it would help. ## Security-Sensitive Areas Extra scrutiny for: - Auth (login, sessions, permissions) - Input handling (forms, API params, file upload) - External calls (HTTP, DB, shell) - Crypto and secrets For these, explicitly note: "No issues found" or list concrete concerns. ## Anti-Patterns - ❌ Reviewing the diff in isolation without checking surrounding code or call sites—context-free feedback is often incomplete or wrong - ❌ Vague feedback ("this could be better") - ❌ Only praising without actionable items when issues exist - ❌ Blocking on style nits that aren't in the style guide - ❌ Missing the main bug or security issue while commenting on formatting --- > Converted and distributed by [TomeVault](https://tomevault.io/claim/lvndry) — claim your Tome and manage your conversions.