Reviewing Code

1---
2name: reviewing-code
3description: Review code for quality, maintainability, and correctness. Use when reviewing pull requests, evaluating code changes, or providing feedback on implementations. Focuses on API design, patterns, and actionable feedback.
4---
5 
6# Code Review
7 
8## Philosophy
9 
10Code review maintains a healthy codebase while helping contributors succeed. The burden of proof is on the PR to demonstrate it adds value. Your job is to help it get there through actionable feedback.
11 
12**Critical**: A perfectly written PR that adds unwanted functionality must still be rejected. The code must advance the codebase in the intended direction. When rejecting, provide clear guidance on how to align with project goals.
13 
14Be friendly and welcoming while maintaining high standards. Call out what works well. When code needs improvement, be specific about why and how to fix it.
15 
16## What to Focus On
17 
18### Does this advance the codebase correctly?
19 
20Even perfect code for unwanted features should be rejected.
21 
22### API design and naming
23 
24Identify confusing patterns or non-idiomatic code:
25- Parameter values that contradict defaults
26- Mutable default arguments
27- Unclear naming that will confuse future readers
28- Inconsistent patterns with the rest of the codebase
29 
30### Specific improvements
31 
32Provide actionable feedback, not generic observations.
33 
34### User ergonomics
35 
36Think about the API from a user's perspective. Is it intuitive? What's the learning curve?
37 
38## For Agent Reviewers
39 
401. **Read the full context**: Examine related files, tests, and documentation before reviewing
412. **Check against established patterns**: Look for consistency with codebase conventions
423. **Verify functionality claims**: Understand what the code actually does, not just what it claims
434. **Consider edge cases**: Think through error conditions and boundary scenarios
44 
45## What to Avoid
46 
47- Generic feedback without specifics
48- Hypothetical problems unlikely to occur
49- Nitpicking organizational choices without strong reason
50- Summarizing what the PR already describes
51- Star ratings or excessive emojis
52- Bikeshedding style preferences when functionality is correct
53- Requesting changes without suggesting solutions
54- Focusing on personal coding style over project conventions
55 
56## Tone
57 
58- Acknowledge good decisions: "This API design is clean"
59- Be direct but respectful
60- Explain impact: "This will confuse users because..."
61- Remember: Someone else maintains this code forever
62 
63## Decision Framework
64 
65Before approving, ask:
66 
671. Does this PR achieve its stated purpose?
682. Is that purpose aligned with where the codebase should go?
693. Would I be comfortable maintaining this code?
704. Have I actually understood what it does, not just what it claims?
715. Does this change introduce technical debt?
72 
73If something needs work, your review should help it get there through specific, actionable feedback. If it's solving the wrong problem, say so clearly.
74 
75## Comment Examples
76 
77**Good comments:**
78 
79| Instead of | Write |
80|------------|-------|
81| "Add more tests" | "The `handle_timeout` method needs tests for the edge case where timeout=0" |
82| "This API is confusing" | "The parameter name `data` is ambiguous - consider `message_content` to match the MCP specification" |
83| "This could be better" | "This approach works but creates a circular dependency. Consider moving the validation to `utils/validators.py`" |
84 
85## Checklist
86 
87Before approving, verify:
88 
89- [ ] All required development workflow steps completed (uv sync, prek, pytest)
90- [ ] Changes align with repository patterns and conventions
91- [ ] API changes are documented and backwards-compatible where possible
92- [ ] Error handling follows project patterns (specific exception types)
93- [ ] Tests cover new functionality and edge cases
94- [ ] The change advances the codebase in the intended direction
95