Menu
Systematic code review guidance covering correctness, maintainability, security, and performance. Activates for PR reviews and code quality checks.
Install with Codex or Claude Copy this prompt, paste it into Codex, Claude, or another assistant, and let it review the skill page and install it for you.
Based on SOC occupation classification
Audience-first documentation guidance for READMEs, API docs, architecture docs, and tutorials. Ensures clarity, completeness, and maintainability.
Project-agnostic skill for delegating execution work to subagents with skill activation, parallel execution, and quality verification.
Defines the technical lead role for main thread in agentic coding workflows - planning, delegation, coordination, and quality oversight
Comprehensive testing guidance covering test planning, TDD workflow, testing pyramid, and coverage targets. Ensures confidence through layered testing.
| name | code-review |
| description | Systematic code review guidance covering correctness, maintainability, security, and performance. Activates for PR reviews and code quality checks. |
Review code through multiple dimensions with constructive, actionable feedback.
Code review is not just about finding bugs. It's about improving overall quality, sharing knowledge, and maintaining consistency across the codebase. Every review should make the code better while respecting the author's effort and intent.
Effective reviews are:
Review code across five dimensions:
Use consistent severity levels to prioritize issues:
Action: Block merge until resolved.
Action: Request changes, consider blocking merge.
Action: Request changes, but allow merge if time-constrained.
Action: Suggest but don't block. Consider auto-formatting tools.
How you phrase feedback matters as much as what you say.
Good:
MAJOR: This database query runs inside a loop (lines 45-52), which could cause
N+1 query problems with large datasets. Consider moving the query outside the
loop and fetching all records at once:
// Suggested approach:
const userIds = items.map(item => item.userId);
const users = await User.findAll({ where: { id: userIds } });
Bad:
This is slow and inefficient.
Why it matters: The good example explains the issue, shows the impact, and provides a concrete solution. The bad example is vague and unhelpful.
Good:
MINOR: I notice we're using a different error handling pattern here than in
the rest of the module. Is there a reason for the inconsistency, or should
we standardize on the try/catch pattern used elsewhere?
Bad:
Use the standard error handling pattern.
Why it matters: The good approach assumes the author may have a reason and invites discussion. The bad approach is prescriptive and may miss important context.
Good:
Nice work on the comprehensive error handling in this function. The specific
error messages will make debugging much easier.
MINOR: Consider extracting the validation logic (lines 23-35) into a separate
function to make this more testable.
Bad:
Extract the validation logic into a separate function.
Why it matters: Positive reinforcement encourages good practices and makes critical feedback easier to receive.
Good:
MAJOR: This function modifies the input array in-place (line 67), which could
cause unexpected behavior for callers who don't expect mutation.
Example of potential bug:
const originalData = [1, 2, 3];
processData(originalData);
console.log(originalData); // [2, 4, 6] - caller didn't expect this!
Consider returning a new array instead:
return data.map(x => x * 2);
Bad:
Don't mutate the input.
Why it matters: Showing the potential problem with an example makes the issue concrete and demonstrates why the change matters.
Correctness:
except:)Maintainability:
Security:
eval() or exec() with user inputPerformance:
Correctness:
.catch() or try/catch)Maintainability:
Security:
eval() or Function() with user inputPerformance:
Correctness:
go test -race)Maintainability:
gofmt formattingSecurity:
Performance:
Correctness:
? operator or explicit handling)Maintainability:
cargo fmt formattingSecurity:
cargo audit)Performance:
cargo bench for performance-critical codeCode review happens at different stages:
Before committing code:
# 1. Review your own diff
git diff --staged
# 2. Run automated checks
npm test && npm run lint # JavaScript
pytest && black --check . # Python
cargo test && cargo clippy # Rust
# 3. Self-review checklist
- Does this code do what I intended?
- Are there edge cases I missed?
- Is this code readable to someone else?
- Are there adequate tests?
- Did I remove debug code and TODOs?
# 4. Commit with descriptive message
git commit -m "[task-X] Brief description"
When reviewing others' code:
After merge (for continuous improvement):
Before approving a pull request:
DON'T:
DO:
Escalate to team discussion when:
How to escalate:
Bad Review:
This is inefficient.
Good Review:
MAJOR: This query runs inside a loop (lines 45-52), creating an N+1 query
problem. With 1000 items, this will run 1000 separate queries instead of 1.
Impact: Page load time increases from ~200ms to ~5s with typical data volumes.
Suggested fix:
// Before (N+1 queries):
for (const item of items) {
const user = await User.findById(item.userId);
item.userName = user.name;
}
// After (single query):
const userIds = items.map(item => item.userId);
const users = await User.findAll({ where: { id: userIds } });
const userMap = Object.fromEntries(users.map(u => [u.id, u]));
items.forEach(item => item.userName = userMap[item.userId].name);
Bad Review:
Add error handling.
Good Review:
MAJOR: This async function doesn't handle errors (lines 23-30). If the API
call fails, the promise will be rejected and potentially cause an unhandled
rejection crash.
Consider wrapping in try/catch:
async function fetchUserData(userId) {
try {
const response = await api.getUser(userId);
return response.data;
} catch (error) {
logger.error('Failed to fetch user data', { userId, error });
throw new UserFetchError('Unable to retrieve user data', { cause: error });
}
}
This allows callers to handle the error appropriately.
Bad Review:
DRY violation.
Good Review:
MINOR: I notice the validation logic is duplicated between createUser() and
updateUser() (lines 45-52 and lines 78-85). Consider extracting to a shared
function:
function validateUserInput(data) {
if (!data.email || !isValidEmail(data.email)) {
throw new ValidationError('Invalid email');
}
if (!data.name || data.name.length < 2) {
throw new ValidationError('Name must be at least 2 characters');
}
return true;
}
This makes the validation logic easier to maintain and test in isolation.
Bad Review:
Security problem here.
Good Review:
CRITICAL: User input is concatenated directly into SQL query (line 34),
creating a SQL injection vulnerability.
Vulnerable code:
const query = `SELECT * FROM users WHERE email = '${userEmail}'`;
Exploit example:
userEmail = "' OR '1'='1'; --"
// This returns all users instead of filtering by email
Fix with parameterized query:
const query = 'SELECT * FROM users WHERE email = ?';
const users = await db.query(query, [userEmail]);
This ensures user input is properly escaped and cannot modify the query structure.
Bad Review:
Line 67: Extract this to a helper.
Good Review:
Nice work on the comprehensive input validation! The descriptive error messages
will make debugging much easier for API consumers.
MINOR: The validation logic (lines 60-75) might be useful in other endpoints.
Consider extracting to a shared validator module:
// validators/userInput.js
export function validateUserCreation(data) {
const errors = [];
if (!data.email || !isValidEmail(data.email)) {
errors.push({ field: 'email', message: 'Invalid email format' });
}
// ... rest of validation
return errors;
}
This would make it easier to maintain consistent validation across endpoints.
Code review integrates with task management:
Example workflow:
# 1. Complete task implementation
# 2. Self-review using checklist above
git diff --staged
npm test && npm run lint
# 3. Commit with task reference
git commit -m "[task-42] Add user authentication
Implements JWT-based auth with refresh tokens. All security best practices
followed (bcrypt for passwords, secure token storage, CSRF protection)."
# 4. Push and create PR
git push -u origin feature/task-42-auth
gh pr create --title "[task-42] Add user authentication" --body "..."
# 5. Team reviews PR using code-review skill
# 6. Address feedback, push updates
# 7. After approval and merge, update task
task_edit(
id="task-42",
status="Done",
notesAppend=["PR #123 merged, commit abc123"]
)
Remember: Code review is a collaborative process focused on improving quality and sharing knowledge. Approach reviews with curiosity and respect, prioritize high-impact issues, and make feedback actionable. Good reviews make code better and teams stronger.