# Reviewing Code

> Reviewing code changes like a senior engineer. Use when doing code reviews, checking PR quality, auditing changes before commit, or providing feedback on implementations. Use when this capability is needed.

- Skill: `tomevault-io/reviewing-code-7` (Agent Skill, multi-file: 2 files)
- Install (CLI): `npx skillmds@latest add tomevault-io/reviewing-code-7`
- Raw SKILL.md: https://api.skillmd.com/api/skills/tomevault-io/reviewing-code-7/raw
- Safety review: pending (external: skill-scanner PASS, skillspector PASS)
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Coding & Dev Tools
- Author: tomevault-io (https://skillmd.com/u/tomevault-io)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/tomevault-io/reviewing-code-7

---


# Code Review

Review code with precision. Flag real issues, skip noise.

## Severity Levels

### 🔴 Critical (Block merge)
- Security vulnerabilities (injection, auth bypass, secrets exposure)
- Data loss or corruption risks
- Breaking changes to public APIs
- Race conditions with data integrity impact
- Unhandled errors that crash the system

### 🟡 Important (Should fix)
- Bugs that affect functionality
- Performance issues (N+1 queries, unbounded loops, memory leaks)
- Missing error handling for likely failure cases
- Violation of established patterns/architecture
- Missing input validation on external data
- Hardcoded values that should be configurable

### 🟢 Minor (Consider fixing)
- Naming improvements for clarity
- Code duplication that could be extracted
- Missing edge case tests
- Documentation gaps
- Style inconsistencies not caught by linter

### ⚪ Skip (Don't mention)
- Issues a linter/formatter would catch
- Personal style preferences not in guidelines
- Pre-existing issues not introduced by this change
- Nitpicks that don't improve the code meaningfully

## Review Checklist

**Correctness**
- Does the code do what it claims?
- Are edge cases handled?
- Are error conditions caught and handled appropriately?

**Security**
- User input sanitized before use?
- Auth/authz checks in place?
- Secrets kept out of code?
- SQL/command injection prevented?

**Performance**
- Database queries efficient? (indexes, N+1)
- Appropriate caching?
- Unbounded operations paginated?
- Expensive operations async where appropriate?

**Maintainability**
- Clear naming and structure?
- Appropriate abstractions?
- Tests cover the important paths?
- Would a new team member understand this?

**Architecture**
- Follows established patterns?
- Correct layer boundaries?
- Dependencies flow in the right direction?
- No circular dependencies introduced?

## Giving Feedback

### Format
```
🔴 path/to/file.ts:42 - Brief issue description
   → Suggested fix or approach
```

### Good Feedback
- Specific: Points to exact location and problem
- Actionable: Suggests a concrete fix
- Educational: Explains why it matters
- Proportional: Severity matches actual impact

### Avoid
- Vague complaints ("this could be better")
- Rewrites without explanation
- Blocking on style preferences
- Piling on - one comment per issue type is enough

## Confidence Filter

Before reporting an issue, ask:
1. Is this a real problem or a preference?
2. Would a senior engineer call this out?
3. Is this introduced by this change (not pre-existing)?
4. Does it violate explicit guidelines or just implicit taste?

Only report issues where you're 80%+ confident they matter.

## Review Output Structure

```
**Summary:** One-line assessment of the change

**Issues:**
🔴 Critical issues (if any)
🟡 Important issues (if any)  
🟢 Minor suggestions (limit to 2-3)

**Good:** What the code does well (always include something)
```

## Anti-patterns to Watch

| Pattern | Problem | Look For |
|---------|---------|----------|
| God object | Too many responsibilities | Class with 10+ methods, mixed concerns |
| Primitive obsession | Stringly-typed data | IDs as strings, status as raw strings |
| Deep nesting | Hard to follow logic | 4+ levels of indentation |
| Magic values | Unclear intent | Numbers/strings without constants |
| Silent failures | Bugs hide | Empty catch blocks, ignored return values |
| Temporal coupling | Order-dependent calls | Init must be called before use |
| Feature envy | Wrong home for logic | Method uses more of another class |

## Positive Signals

Also call out good patterns:
- Clean separation of concerns
- Good test coverage on critical paths  
- Defensive error handling
- Clear naming that documents intent
- Appropriate use of types
- Thoughtful API design

---
> Converted and distributed by [TomeVault](https://tomevault.io/claim/martinffx) — claim your Tome and manage your conversions.
<!-- tomevault:4.0:skill_md:2026-04-15 -->