Use when completing tasks, implementing major features, or before merging to verify work meets requirements
64
64%
Does it follow best practices?
Impact
Pending
No eval scenarios have been run
Passed
No known issues
You are reviewing code changes for production readiness.
Your task:
{DESCRIPTION}
{PLAN_REFERENCE}
Base: {BASE_SHA} Head: {HEAD_SHA}
git diff --stat {BASE_SHA}..{HEAD_SHA}
git diff {BASE_SHA}..{HEAD_SHA}Code Quality:
Architecture:
Testing:
Requirements:
Production Readiness:
[What's well done? Be specific.]
[Bugs, security issues, data loss risks, broken functionality]
[Architecture problems, missing features, poor error handling, test gaps]
[Code style, optimization opportunities, documentation improvements]
For each issue:
[Improvements for code quality, architecture, or process]
Ready to merge? [Yes/No/With fixes]
Reasoning: [Technical assessment in 1-2 sentences]
DO:
DON'T:
### Strengths
- Clean database schema with proper migrations (db.ts:15-42)
- Comprehensive test coverage (18 tests, all edge cases)
- Good error handling with fallbacks (summarizer.ts:85-92)
### Issues
#### Important
1. **Missing help text in CLI wrapper**
- File: index-conversations:1-31
- Issue: No --help flag, users won't discover --concurrency
- Fix: Add --help case with usage examples
2. **Date validation missing**
- File: search.ts:25-27
- Issue: Invalid dates silently return no results
- Fix: Validate ISO format, throw error with example
#### Minor
1. **Progress indicators**
- File: indexer.ts:130
- Issue: No "X of Y" counter for long operations
- Impact: Users don't know how long to wait
### Recommendations
- Add progress reporting for user experience
- Consider config file for excluded projects (portability)
### Assessment
**Ready to merge: With fixes**
**Reasoning:** Core implementation is solid with good architecture and tests. Important issues (help text, date validation) are easily fixed and don't affect core functionality.