docs: restructure PR review prompt with step-by-step workflow #467
Conversation
🤖 Claude Code Review
Review SummaryThis PR significantly expands the Claude PR review prompt from ~100 lines to ~487 lines, restructuring it into a more comprehensive workflow document. However, the file contains unresolved git merge conflicts that must be fixed before merging. Critical Issues 🔴1. Unresolved Git Merge Conflicts ( The file contains two sections with unresolved merge conflict markers:
These conflict markers ( 2. Missing Template Variable Documentation ( The context block contains template variables like
This makes the prompt difficult to maintain and debug. Improvements Suggested 🟡3. Constraint on COMMENT-only Reviews Removes Nuance (Line 11) The new constraints force COMMENT-only reviews, but the original guide explicitly recommended "Approve with comments beats comment-only reviews." This removes valuable review signal. If this is a technical limitation (e.g., GitHub Actions permissions), documenting the reason would help future maintainers understand the constraint. 4. Verbose Examples May Reduce Effectiveness The expanded teaching examples are more verbose than the originals. For example:
In prompt engineering, brevity often works better because it leaves room for the model to adapt to specific context. 5. XML Output Format Requirements Add Ceremony The 7-step workflow with mandatory XML output formats (e.g., Good Practices Observed ✅
Action Items
💡 Want a fresh review? Add a comment containing |
github-actions
bot
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋 Review verdict: COMMENT
👆 The main review comment above is the source of truth for this PR review. It is automatically updated on each review cycle, so always refer to it for the most current feedback.
This formal review submission is for the verdict only. 4 inline comment(s) are attached below.
|
|
||
| <constraints> | ||
| **CRITICAL OPERATING CONSTRAINTS:** | ||
| 1. You can ONLY submit "COMMENT" reviews (technical limitation) |
There was a problem hiding this comment.
If COMMENT is truly the only option due to technical limitations, this is worth explaining. However, the original guide explicitly stated:
"Approve with comments beats comment-only reviews. If it's not worth blocking, it's worth approving."
This constraint removes a valuable nuance. COMMENT reviews create friction without clear signal. Consider documenting why this limitation exists, or if it's a GitHub Actions permission issue, note that as context.
There was a problem hiding this comment.
The current code still has not documented why COMMENT-only reviews are required. The issue remains - this constraint removes valuable nuance from the review process without explanation.
| **Why this matters:** Duplicate comments waste everyone's time and make reviews harder to follow. By checking first, you ensure every comment adds unique value. | ||
|
|
||
| Nice-to-haves. Mention only if the win is obvious: | ||
| **Action:** |
There was a problem hiding this comment.
The 7-step workflow with XML outputs adds significant ceremony. This is a style choice, but be aware that rigid formatting requirements can cause the model to focus on format compliance over review quality.
The original guide achieved similar results with simpler prose instructions like "check existing comments and discussions" without requiring specific output formats.
There was a problem hiding this comment.
The 7-step workflow with XML outputs remains unchanged. This is a style/design choice that the PR author has made, though the concern about format compliance vs review quality is worth considering.
| </teaching_mode> | ||
|
|
||
| Problems that would cause immediate harm: | ||
| <context> |
There was a problem hiding this comment.
These template variables ($REPOSITORY, $PR_NUMBER, etc.) need documentation about:
- What system populates them
- Expected behavior if missing
- Format expectations
Without this, someone maintaining this prompt won't know how these integrate with the broader system.
There was a problem hiding this comment.
The template variables still lack documentation. This issue persists and should be addressed.
|
|
||
| > Issue 1: Logger Interface Violation (Blocking) | ||
| > The Danger bot correctly identified that the code is logging strings directly to logger.error()... Why this matters: The logger interface expects Error objects for structured error logging... | ||
| **Teach this improvement:** |
There was a problem hiding this comment.
This expanded teaching example:
"This function has 4 separate responsibilities. Splitting them makes testing trivial:
- `validateInput(userId)` - test with simple inputs
..."
Is more verbose than the original:
"Four responsibilities here. Splitting them makes testing easier - each piece testable without mocking the others."
The original was actually clearer. Brevity often wins in prompt engineering since it leaves more room for the model to adapt to context.
There was a problem hiding this comment.
The expanded teaching example remains more verbose than the original. This is a style choice, though the original's brevity may be more effective in prompt engineering.
marktoda
force-pushed
the
update-prompt
branch
from
b3a7963 to
334dedd
Compare
There was a problem hiding this comment.
📋 Review verdict: REQUEST_CHANGES
👆 The main review comment above is the source of truth for this PR review. It is automatically updated on each review cycle, so always refer to it for the most current feedback.
This formal review submission is for the verdict only. 3 inline comment(s) are attached below.
| - Refactoring opportunities | ||
| </priorities> | ||
|
|
||
| <<<<<<< Updated upstream |
There was a problem hiding this comment.
This file contains unresolved git merge conflicts. The conflict markers (<<<<<<< Updated upstream, =======, >>>>>>> Stashed changes) will cause the prompt to malfunction.
Resolution needed: Decide which version of the "Pattern Examples" vs "Decision Framework" sections to keep, or merge them appropriately. The conflict spans lines 298-368 and 408-417.
To find all conflicts:
git diff --check| </teaching_mode> | ||
|
|
||
| Problems that would cause immediate harm: | ||
| <context> |
There was a problem hiding this comment.
These template variables ($REPOSITORY, $PR_NUMBER, $CURRENT_PATCH_ID, etc.) need documentation:
- What system/script populates these values?
- What happens if a variable is undefined or empty?
- What format is expected for
$CURRENT_PATCH_ID? - Is
$CLAUDE_COMMENT_IDalways present or only on re-reviews?
Without this documentation, future maintainers won't know how to integrate or debug this prompt.
| <context> | |
| <context> | |
| <!-- Template variables populated by the CI/CD workflow that invokes this prompt --> | |
| <!-- See .github/workflows/pr-review.yml for population logic --> | |
| Repository: $REPOSITORY | |
| PR Number: $PR_NUMBER | |
| PR Title: $PR_TITLE | |
| PR Body: $PR_BODY | |
| Current Patch ID: $CURRENT_PATCH_ID | |
| Existing Claude Comment ID: $CLAUDE_COMMENT_ID | |
| Existing Comments: $PR_COMMENTS | |
| Review Comments: $REVIEW_COMMENTS | |
| Changed Files: $CHANGED_FILES | |
| Trigger: $TRIGGER_COMMENT | |
| </context> |
|
|
||
| <constraints> | ||
| **CRITICAL OPERATING CONSTRAINTS:** | ||
| 1. You can ONLY submit "COMMENT" reviews (technical limitation) |
There was a problem hiding this comment.
The original guide recommended "Approve with comments beats comment-only reviews. If it's not worth blocking, it's worth approving."
This constraint removes that nuance. COMMENT-only reviews create friction without clear signal about whether the reviewer thinks the code is safe to merge.
If this is a GitHub Actions permission limitation, adding a brief explanation would help:
1. You can ONLY submit "COMMENT" reviews (GitHub Actions tokens lack approval permissions)If it's intentional design choice, explaining the rationale would help maintainers understand the tradeoff.
✨ Claude-Generated Content
Summary
Restructures the Claude PR review prompt into a comprehensive, workflow-based format with explicit steps, output requirements, and enhanced teaching guidelines for better code review quality.
Changes
Notes
This is a documentation-only change affecting Claude Code's PR review behavior. No smart contract code is modified.
Note: The file contains some merge conflict markers (
<<<<<<<,=======,>>>>>>>) that appear to be unresolved - these should be cleaned up before merging.