# Code Review Guidelines

> When asynchronously reviewing peer code before merging into the main branch.

- Skill: `kraitdev/code-review-guidelines` (Agent Skill)
- Install (CLI): `npx skillmds@latest add kraitdev/code-review-guidelines`
- Raw SKILL.md: https://api.skillmd.com/api/skills/kraitdev/code-review-guidelines/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Coding & Dev Tools
- License: MIT
- Author: KraitDev (https://skillmd.com/u/kraitdev)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/kraitdev/code-review-guidelines

---


# Code Review Guidelines

## Purpose
Code review catches bugs, ensures maintainability, and builds team knowledge. This skill provides a structured, objective, empathetic framework for reviewing code without causing friction. The goal is to improve the codebase while growing the team.

## When to use
- Evaluating a Pull Request or Merge Request
- Providing constructive feedback to peers
- Ensuring code quality and architectural standards are met
- Building new team members' skills through feedback

## When NOT to use
- Nitpicking personal coding style (use linters/formatters)
- Blocking on subjective preferences
- Reviewing without understanding context

## Inputs required
- Pull request with code changes
- PR description explaining the why
- Linked ticket with context
- Test coverage (unit/integration tests)

## Workflow
1. **Understand the Goal**: Read the PR description and linked ticket FIRST. Understand WHY the change exists before reading code.
2. **Review Architecture**: Check structural integrity and design. Does it fit the codebase? Are new dependencies justified? Is it the right abstraction level?
3. **Review Logic**: Check for correct error handling, edge cases, off-by-one errors, and security implications.
4. **Check Tests**: Verify new logic is covered by unit/integration tests that test actual behavior (not just code coverage).
5. **Consider Performance**: Check for obvious performance issues (N+1 queries, memory leaks, synchronous blocking in async code).
6. **Provide Feedback**: Leave comments that explain the WHY. Differentiate between blocking requests and suggestions (use "Nit:" or "Note:").
7. **Be Constructive**: Phrase feedback as questions when possible ("Have you considered...?"). Assume positive intent.

## Rules
- MUST assume positive intent; critique the code, not the author
- MUST automate style and formatting checks via Linters/Formatters (do NOT argue over spacing in PR)
- MUST approve immediately if code improves the codebase, even if not perfect
- MUST provide context and links when referring to standards or patterns
- MUST distinguish between blocking requests and optional suggestions
- MUST NOT block on personal coding style preferences
- MUST NOT require unrelated changes (keep scope focused)

## Anti-patterns
- **Gatekeeping**: Blocking PRs over personal style preferences or "I would have done it differently"
- **Rubber Stamping**: Approving large PRs without reading or thinking
- **Scope Creep**: Asking the author to fix unrelated legacy code nearby
- **Vague Comments**: "This is bad" or "Improve this" without explanation
- **Late Requests**: Requesting major architectural changes after code is already implemented
- **Tone Issues**: Comments that feel dismissive or condescending

## Failure conditions
- PR blocked without clear explanation
- Security or correctness issue missed
- No tests for new logic
- Feedback based on personal preference, not code quality
- Author feels disrespected or attacked

## Validation checklist
- [ ] PR description explains the why (not just what)
- [ ] Tests cover new logic
- [ ] No obvious performance issues
- [ ] Error handling is correct
- [ ] Edge cases are considered
- [ ] Security implications reviewed (if applicable)
- [ ] Code follows established patterns in the codebase
- [ ] Dependencies are justified
- [ ] Comments explain complex logic
- [ ] No scope creep (focused on this change)

## Output format
- **Feedback structure**: Problem + context + suggestion
- **Tone**: Constructive, curious, respectful
- **Clarity**: Blocking requests vs. optional suggestions clearly marked
- **Actionability**: Comments are specific and actionable
- **Decision**: Clear approval, changes requested, or comment

## Security considerations
- Security issues MUST be flagged as blocking
- SQL injection, XSS, authentication bypasses MUST be caught
- Secrets/credentials MUST NEVER be committed (catch before merge)
- Permission checks MUST be enforced
- Data exposure MUST be reviewed

## Agent execution notes
- Agent MAY: Leave constructive feedback, ask clarifying questions, suggest improvements
- Agent MUST NEVER: Block on style/preference, be dismissive, miss security issues, approve without reading
- Agent MUST ASK: When context is missing, when architectural impact is unclear
- Agent MUST VALIDATE: Tests are present, no security holes, code improves codebase

## Example

**❌ Anti-pattern (Vague, dismissive, scope creep):**
```
"This code is bad"
"Why are you doing it this way?"
"Also, can you fix the bug in the file next to this?"
"Blocked - needs improvement"
"LGTM" (without reading)
```

**✅ Correct pattern (Constructive, specific, respectful):**
```
✅ Good catch on the edge case - this prevents the race condition we had in production.

💭 Question: In the validateEmail function, what happens if the email service is down?
   Have you considered adding a timeout or fallback behavior?

🔒 Security: Double-check that user input is sanitized before the database query.

📝 Nit: This comment could be clearer - what does "process the thing" mean?

✅ The test coverage looks good. I especially like the edge case tests.

🚀 Approved! This is a clear improvement to the codebase. Nice work.
```