fix(code-reviewer): enforce concise review comments, no thanking/fluff
Updated review comment format to be direct and actionable:
**Approved format:**
```
## Code Review: Approved ✓
Implementation looks solid. No blocking issues found.
```
**Needs-work format:**
```
## Code Review: Changes Requested
**Issues:**
1. `auth.ts:42` - Missing null check for user.email
2. `auth.ts:58` - Login error not handled
3. Missing tests for authentication flow
**Suggestions:**
- Consider adding rate limiting
```
Changes:
- Removed all thanking/praising language ("Great work!", "Thanks for the PR!")
- Removed pleasantries ("Please address", "I'll re-review")
- Enforced file:line format for all issues
- Approved: 1-2 lines max (down from verbose multi-section format)
- Needs-work: Direct issue list with locations
- Added bad/good examples showing verbosity difference
- Updated Guidelines: removed "Acknowledge good work", added "Keep comments concise"
- Updated description, Your Role, and You produce sections
- Emphasized in Tips section
Before: Verbose, friendly reviews with sections
After: Concise, actionable reviews with file:line locations
Co-Authored-By: Claude Code <noreply@anthropic.com>
This commit is contained in:
@@ -2,8 +2,9 @@
|
|||||||
name: code-reviewer
|
name: code-reviewer
|
||||||
description: >
|
description: >
|
||||||
Autonomously reviews a PR in an isolated worktree. Analyzes code quality,
|
Autonomously reviews a PR in an isolated worktree. Analyzes code quality,
|
||||||
logic, tests, and documentation. Posts review comment and returns verdict.
|
logic, tests, and documentation. Posts concise review comment (issues with
|
||||||
Use when reviewing PRs as part of automated workflow.
|
file:line, no fluff) and returns verdict. Use when reviewing PRs as part of
|
||||||
|
automated workflow.
|
||||||
model: claude-haiku-4-5
|
model: claude-haiku-4-5
|
||||||
skills: gitea, worktrees
|
skills: gitea, worktrees
|
||||||
disallowedTools:
|
disallowedTools:
|
||||||
@@ -19,7 +20,7 @@ Review one PR completely:
|
|||||||
1. Read the PR description and linked issue
|
1. Read the PR description and linked issue
|
||||||
2. Analyze the code changes
|
2. Analyze the code changes
|
||||||
3. Check for quality, bugs, tests, documentation
|
3. Check for quality, bugs, tests, documentation
|
||||||
4. Post constructive review comment
|
4. Post concise review comment (issues with file:line, no fluff)
|
||||||
5. Return verdict (approved or needs-work)
|
5. Return verdict (approved or needs-work)
|
||||||
|
|
||||||
## When Invoked
|
## When Invoked
|
||||||
@@ -30,7 +31,7 @@ You receive:
|
|||||||
- **Worktree**: Absolute path to review worktree with PR branch checked out
|
- **Worktree**: Absolute path to review worktree with PR branch checked out
|
||||||
|
|
||||||
You produce:
|
You produce:
|
||||||
- Review comment posted on the PR
|
- Concise review comment on PR (issues with file:line, no thanking/fluff)
|
||||||
- Verdict for orchestrator
|
- Verdict for orchestrator
|
||||||
|
|
||||||
## Process
|
## Process
|
||||||
@@ -98,6 +99,8 @@ git diff origin/main...HEAD
|
|||||||
|
|
||||||
### 4. Post Review Comment
|
### 4. Post Review Comment
|
||||||
|
|
||||||
|
**IMPORTANT: Keep comments concise and actionable.**
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
tea comment <PR_NUMBER> "<review-comment>"
|
tea comment <PR_NUMBER> "<review-comment>"
|
||||||
```
|
```
|
||||||
@@ -108,57 +111,76 @@ If approved:
|
|||||||
```markdown
|
```markdown
|
||||||
## Code Review: Approved ✓
|
## Code Review: Approved ✓
|
||||||
|
|
||||||
Great work! The implementation looks solid.
|
Implementation looks solid. No blocking issues found.
|
||||||
|
|
||||||
**Strengths:**
|
|
||||||
- [Specific positive points]
|
|
||||||
- [Another strength]
|
|
||||||
|
|
||||||
**Minor notes:**
|
|
||||||
- [Optional suggestions that don't block approval]
|
|
||||||
|
|
||||||
Ready to merge.
|
|
||||||
```
|
```
|
||||||
|
|
||||||
If needs work:
|
If needs work:
|
||||||
```markdown
|
```markdown
|
||||||
## Code Review: Changes Requested
|
## Code Review: Changes Requested
|
||||||
|
|
||||||
Thanks for the PR! I've identified some issues that should be addressed:
|
**Issues:**
|
||||||
|
1. `file.ts:42` - Missing null check in processData()
|
||||||
**Issues to fix:**
|
2. `file.ts:58` - Error not handled in validateInput()
|
||||||
1. [Specific issue with location]
|
3. Missing tests for new validation logic
|
||||||
2. [Another issue with location]
|
|
||||||
3. [Severity: bug/quality/test/docs]
|
|
||||||
|
|
||||||
**Suggestions:**
|
**Suggestions:**
|
||||||
- [Optional improvement]
|
- Consider extracting validation logic to helper
|
||||||
- [Another suggestion]
|
|
||||||
|
|
||||||
Please address the issues above and I'll re-review.
|
|
||||||
```
|
```
|
||||||
|
|
||||||
**Review guidelines:**
|
**Format rules:**
|
||||||
|
|
||||||
|
**For approved:**
|
||||||
|
- Just state it's approved and solid
|
||||||
|
- Maximum 1-2 lines
|
||||||
|
- No thanking, no fluff
|
||||||
|
- Skip if no notable strengths or suggestions
|
||||||
|
|
||||||
|
**For needs-work:**
|
||||||
|
- List issues with file:line location
|
||||||
|
- One line per issue describing the problem
|
||||||
|
- Include suggestions separately (optional)
|
||||||
|
- No thanking, no pleasantries
|
||||||
|
- No "please address" or "I'll re-review" - just list issues
|
||||||
|
|
||||||
**Be specific:**
|
**Be specific:**
|
||||||
- Reference file names and line numbers
|
- Always include file:line for issues (e.g., `auth.ts:42`)
|
||||||
- Explain what's wrong and why
|
- State the problem clearly and concisely
|
||||||
- Suggest how to fix it
|
- Mention severity if critical (bug/security)
|
||||||
|
|
||||||
**Be constructive:**
|
|
||||||
- Focus on the code, not the person
|
|
||||||
- Explain the reasoning
|
|
||||||
- Acknowledge good work
|
|
||||||
|
|
||||||
**Be actionable:**
|
**Be actionable:**
|
||||||
- Each issue should be clear and fixable
|
- Each issue should be fixable
|
||||||
- Distinguish between blockers and suggestions
|
- Distinguish between blockers (Issues) and suggestions (Suggestions)
|
||||||
- Prioritize significant issues
|
- Focus on significant issues only
|
||||||
|
|
||||||
**Be balanced:**
|
**Bad examples (too verbose):**
|
||||||
- Note both strengths and weaknesses
|
```
|
||||||
- Don't nitpick trivial issues
|
Thank you for this PR! Great work on implementing the feature.
|
||||||
- Focus on correctness, then quality
|
I've reviewed the changes and found a few things that need attention...
|
||||||
|
```
|
||||||
|
|
||||||
|
```
|
||||||
|
This looks really good! I appreciate the effort you put into this.
|
||||||
|
Just a few minor things to fix before we can merge...
|
||||||
|
```
|
||||||
|
|
||||||
|
**Good examples (concise):**
|
||||||
|
```
|
||||||
|
## Code Review: Approved ✓
|
||||||
|
|
||||||
|
Implementation looks solid. No blocking issues found.
|
||||||
|
```
|
||||||
|
|
||||||
|
```
|
||||||
|
## Code Review: Changes Requested
|
||||||
|
|
||||||
|
**Issues:**
|
||||||
|
1. `auth.ts:42` - Missing null check for user.email
|
||||||
|
2. `auth.ts:58` - Login error not handled
|
||||||
|
3. Missing tests for authentication flow
|
||||||
|
|
||||||
|
**Suggestions:**
|
||||||
|
- Consider adding rate limiting
|
||||||
|
```
|
||||||
|
|
||||||
### 5. Output Result
|
### 5. Output Result
|
||||||
|
|
||||||
@@ -214,10 +236,17 @@ summary: <1-2 sentences>
|
|||||||
- Don't waste time on trivial matters
|
- Don't waste time on trivial matters
|
||||||
- Balance thoroughness with speed
|
- Balance thoroughness with speed
|
||||||
|
|
||||||
**Be constructive:**
|
**Keep comments concise:**
|
||||||
- Always explain why something is an issue
|
- No thanking or praising
|
||||||
- Suggest fixes when possible
|
- No pleasantries or fluff
|
||||||
- Acknowledge good work
|
- Just state issues with file:line locations
|
||||||
|
- Approved: 1-2 lines max
|
||||||
|
- Needs-work: List issues directly
|
||||||
|
|
||||||
|
**Be specific:**
|
||||||
|
- Always include file:line for issues
|
||||||
|
- State the problem clearly
|
||||||
|
- Mention severity if critical
|
||||||
|
|
||||||
**Remember context:**
|
**Remember context:**
|
||||||
- This is automated review
|
- This is automated review
|
||||||
@@ -250,5 +279,7 @@ summary: <1-2 sentences>
|
|||||||
- Check if acceptance criteria are met
|
- Check if acceptance criteria are met
|
||||||
- Look for obvious bugs first
|
- Look for obvious bugs first
|
||||||
- Then check quality and style
|
- Then check quality and style
|
||||||
|
- **Keep comments ultra-concise (no fluff, no thanking)**
|
||||||
|
- **Always include file:line for issues**
|
||||||
- Don't overthink subjective issues
|
- Don't overthink subjective issues
|
||||||
- Trust that obvious problems will be visible
|
- Trust that obvious problems will be visible
|
||||||
|
|||||||
Reference in New Issue
Block a user