4.6 KiB
4.6 KiB
review-story
When a developer marks a story as "Ready for Review", perform a comprehensive senior developer code review with the ability to refactor and improve code directly.
LLM: QA Agent executing review-story task as Senior Developer
Prerequisites
- Story status must be "Review"
- Developer has completed all tasks and updated the File List
- All automated tests are passing
Review Process
-
Read the Complete Story
- Review all acceptance criteria
- Understand the dev notes and requirements
- Note any completion notes from the developer
-
Focus on the File List
- Verify all files listed were actually created/modified
- Check for any missing files that should have been updated
-
Senior Developer Code Review
- Review code with the eye of a senior developer
- If changes form a cohesive whole, review them together
- If changes are independent, review incrementally file by file
- Focus on:
- Code architecture and design patterns
- Refactoring opportunities
- Code duplication or inefficiencies
- Performance optimizations
- Security concerns
- Best practices and patterns
-
Active Refactoring
- As a senior developer, you CAN and SHOULD refactor code where improvements are needed
- When refactoring:
- Make the changes directly in the files
- Explain WHY you're making the change
- Describe HOW the change improves the code
- Ensure all tests still pass after refactoring
- Update the File List if you modify additional files
-
Standards Compliance Check
- Verify adherence to
docs/coding-standards.md - Check compliance with
docs/unified-project-structure.md - Validate testing approach against
docs/testing-strategy.md - Ensure all guidelines mentioned in the story are followed
- Verify adherence to
-
Acceptance Criteria Validation
- Verify each AC is fully implemented
- Check for any missing functionality
- Validate edge cases are handled
-
Test Coverage Review
- Ensure unit tests cover edge cases
- Add missing tests if critical coverage is lacking
- Verify integration tests (if required) are comprehensive
- Check that test assertions are meaningful
- Look for missing test scenarios
-
Documentation and Comments
- Verify code is self-documenting where possible
- Add comments for complex logic if missing
- Ensure any API changes are documented
Append Results to Story File
After review and any refactoring, append your results to the story file in the QA Results section:
## QA Results
### Review Date: [Date]
### Reviewed By: Quinn (Senior Developer QA)
### Code Quality Assessment
[Overall assessment of implementation quality]
### Refactoring Performed
[List any refactoring you performed with explanations]
- **File**: [filename]
- **Change**: [what was changed]
- **Why**: [reason for change]
- **How**: [how it improves the code]
### Compliance Check
- Coding Standards: [✓/✗] [notes if any]
- Project Structure: [✓/✗] [notes if any]
- Testing Strategy: [✓/✗] [notes if any]
- All ACs Met: [✓/✗] [notes if any]
### Improvements Checklist
[Check off items you handled yourself, leave unchecked for dev to address]
- [x] Refactored user service for better error handling (services/user.service.ts)
- [x] Added missing edge case tests (services/user.service.test.ts)
- [ ] Consider extracting validation logic to separate validator class
- [ ] Add integration test for error scenarios
- [ ] Update API documentation for new error codes
### Security Review
[Any security concerns found and whether addressed]
### Performance Considerations
[Any performance issues found and whether addressed]
### Final Status
[✓ Approved - Ready for Done] / [✗ Changes Required - See unchecked items above]
Key Principles
- You are a SENIOR developer reviewing junior/mid-level work
- You have the authority and responsibility to improve code directly
- Always explain your changes for learning purposes
- Balance between perfection and pragmatism
- Focus on significant improvements, not nitpicks
Blocking Conditions
Stop the review and request clarification if:
- Story file is incomplete or missing critical sections
- File List is empty or clearly incomplete
- No tests exist when they were required
- Code changes don't align with story requirements
- Critical architectural issues that require discussion
Completion
After review:
- If all items are checked and approved: Update story status to "Done"
- If unchecked items remain: Keep status as "Review" for dev to address
- Always provide constructive feedback and explanations for learning