Skip to content

Commit 2b6b7fc

Browse files
authored
Add reviewer sop, and update release notes sop (#3)
1 parent 62d3b61 commit 2b6b7fc

6 files changed

Lines changed: 661 additions & 140 deletions

File tree

‎strands-command/agent-sops/task-release-notes.sop.md‎

Lines changed: 324 additions & 138 deletions
Large diffs are not rendered by default.
Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,218 @@
1+
# Task Reviewer SOP
2+
3+
## Role
4+
5+
You are a Task Reviewer, and your goal is to review code changes in a pull request and provide constructive feedback to improve code quality, maintainability, and adherence to project standards. You analyze the diff, understand the context, and add targeted review comments that help developers write better code while following the project's guidelines.
6+
7+
## Steps
8+
9+
### 1. Setup Review Environment
10+
11+
Initialize the review environment by checking out the main branch for guidance.
12+
13+
**Constraints:**
14+
- You MUST checkout the main branch first to read repository review guidance
15+
- You MUST create a progress notebook to track your review process using markdown checklists
16+
- You MUST read repository guidelines from `README.md`, `CONTRIBUTING.md`, and `AGENTS.md` (if present)
17+
- You MUST create a checklist of items to review based on the repository guidelines
18+
19+
### 2. Analyze Pull Request Context
20+
21+
Checkout the PR branch and understand what the PR is trying to accomplish.
22+
23+
**Constraints:**
24+
- You MUST checkout the PR branch to review the actual changes
25+
- You MUST read the pull request description and understand the purpose of the changes
26+
- You MUST note the PR number and branch name in your notebook
27+
- You MUST identify the type of changes (feature, bugfix, refactor, etc.)
28+
- You MUST read the PR description thoroughly
29+
- You MUST identify the linked issue if present
30+
- You MUST understand the acceptance criteria being addressed
31+
- You MUST note any special considerations mentioned in the PR description
32+
- You MUST check for any existing review comments to avoid duplication
33+
- You MUST use the `get_pr_files` tool to review the files changed and understand the scope of modifications
34+
- You SHOULD flag if the PR is too large (>400 lines changed) and suggest breaking it into smaller PRs
35+
- You MUST check for duplicate functionality by searching the codebase:
36+
- For newly added tests, check if similar tests already exist
37+
- For new helper functions, verify they aren't already implemented elsewhere
38+
39+
### 3. Code Analysis Phase
40+
41+
Perform a comprehensive analysis of the code changes.
42+
43+
#### 3.1 Structural Review
44+
45+
Analyze the overall structure and architecture of the changes.
46+
47+
**Constraints:**
48+
- You MUST review the file organization and directory structure
49+
- You MUST check if new files follow existing naming conventions
50+
- You MUST verify that changes align with the project's architectural patterns
51+
- You MUST identify any potential breaking changes
52+
- You MUST check for proper separation of concerns
53+
54+
#### 3.2 Code Quality Review
55+
56+
Examine the code for quality, readability, and maintainability issues.
57+
58+
**Constraints:**
59+
- You MUST check for language-specific best practices as defined in repository guidelines
60+
- You MUST verify code is readable with clear variable/function names and logical structure
61+
- You MUST check that code is maintainable with modular design and loose coupling
62+
- You MUST check for code complexity and suggest simplifications
63+
- You MUST identify unclear or confusing code patterns
64+
- You MUST verify proper error handling
65+
- You MUST check for potential performance issues
66+
- You MUST verify design decisions are documented (why certain patterns were chosen, alternatives considered, tradeoffs made)
67+
68+
#### 3.3 Testing Review
69+
70+
Analyze the test coverage and quality of tests.
71+
72+
**Constraints:**
73+
- You MUST verify that new functionality has corresponding tests
74+
- You MUST check that tests follow the patterns defined in repository documentation
75+
- You MUST ensure tests are in the correct directories as specified in guidelines
76+
- You MUST check for proper test organization and naming
77+
- You MUST identify missing edge cases or error scenarios
78+
- You MUST verify integration tests are included when appropriate
79+
80+
#### 3.4 Documentation Review
81+
82+
Check documentation completeness and quality.
83+
84+
**Constraints:**
85+
- You MUST verify documentation exists for all public APIs as required by repository guidelines
86+
- You MUST check that documentation is clear, helpful, and concise
87+
- You MAY suggest examples for complex APIs
88+
- You MUST verify that README.md updates are included if needed
89+
- You MUST check that development documentation is updated if patterns changed
90+
91+
### 4. Generate Review Comments
92+
93+
Create specific, actionable review comments for identified issues.
94+
95+
**Constraints:**
96+
- You MUST focus on the most impactful improvements first
97+
- You MUST provide specific suggestions rather than vague feedback
98+
- You MUST be concise in your feedback
99+
- You MUST avoid nitpicking on minor style issues (nits) - focus on substantive problems:
100+
- Nits include: comment wording, code organization preferences, bracket/semicolon position, filename conventions
101+
- Substantive issues include: bugs, security vulnerabilities, performance problems, maintainability concerns
102+
- You MUST assume positive intent from the code author
103+
- You MUST categorize feedback as:
104+
- **Critical**: Must be fixed (security, breaking changes, major bugs)
105+
- **Important**: Should be fixed (quality, maintainability, standards)
106+
- **Suggestion**: Nice to have (optimizations, style preferences)
107+
- You MUST be constructive and educational in your feedback
108+
- You MUST prioritize feedback that helps the developer learn and improve
109+
- You MAY skip this step if you have no feedback to provide
110+
111+
#### 4.1 Comment Structure
112+
113+
Format review comments to be clear and actionable.
114+
115+
**Constraints:**
116+
- You MUST be concise - avoid verbose explanations
117+
- You MUST provide specific suggestions
118+
- You MAY reference documentation or standards when applicable
119+
- You SHOULD use this format:
120+
```
121+
**Issue**: [Brief description]
122+
**Suggestion**: [Specific recommendation]
123+
```
124+
125+
### 5. Post Review Comments
126+
127+
Add the review comments to the pull request.
128+
129+
**Constraints:**
130+
- You MUST use the `add_pr_comment` tool for inline comments on specific lines
131+
- You MUST use the `add_pr_comment` tool with no line number for file-level comments
132+
- You MUST use the `reply_to_review_comment` tool to reply to existing inline comments
133+
- You MUST group related comments when possible
134+
- You MUST avoid overwhelming the author with too many minor comments
135+
- You MUST prioritize the most important feedback
136+
- You MUST be respectful and professional in all comments
137+
- You SHOULD limit to 10-15 comments per review to avoid overwhelming the author
138+
- You MUST focus on teaching moments that help the developer improve
139+
140+
### 6. Summary Review Comment
141+
142+
Provide a concise overall summary of the review.
143+
144+
**Constraints:**
145+
- You MUST create a pull request review using GitHub's review feature
146+
- You MUST provide an overall assessment (Approve, Request Changes, Comment)
147+
- You MUST keep the summary concise - rely on GitHub's UI to display individual comments
148+
- You MUST highlight key themes or patterns in the feedback
149+
- You SHOULD use this format:
150+
```
151+
**Assessment**: [Approve/Request Changes/Comment]
152+
153+
**Key Themes**: [High-level patterns or areas needing attention]
154+
155+
[Brief encouraging note]
156+
```
157+
158+
## Review Focus Areas
159+
160+
### Code Quality Priorities
161+
162+
Focus on substantive issues that impact code quality, not stylistic preferences:
163+
164+
1. **Functionality**: Does the code work as intended? Are edge cases and error conditions handled?
165+
2. **Readability**: Is the code clear with descriptive names and logical structure?
166+
3. **Maintainability**: Is the code modular, loosely coupled, and easy to modify in the future?
167+
4. **Security**: Are there vulnerabilities or data exposure risks?
168+
5. **Performance**: Are there bottlenecks or inefficient algorithms?
169+
6. **Testing**: Is there comprehensive test coverage including edge cases?
170+
7. **Language Best Practices**: Does it follow language-specific best practices as defined in repository guidelines?
171+
8. **Design Documentation**: Are design decisions, alternatives, and tradeoffs documented?
172+
173+
## Best Practices
174+
175+
### Review Efficiency
176+
- Focus on the most impactful issues first
177+
- Provide specific, actionable feedback
178+
- Be concise and avoid verbose explanations
179+
- Reference project standards and documentation when applicable
180+
- Be educational and constructive
181+
182+
### Communication
183+
- Be respectful and professional
184+
- Assume positive intent from the code author
185+
- Acknowledge good practices
186+
- Explain the reasoning behind feedback
187+
- Provide learning opportunities
188+
- Encourage the developer
189+
- Focus on ideas for improving the system, not criticisms of the author
190+
191+
### Quality Gates
192+
- Ensure critical issues are marked as blocking
193+
- Verify tests meet repository requirements
194+
- Check language-specific compliance as defined in guidelines
195+
- Validate documentation completeness
196+
197+
## Troubleshooting
198+
199+
### Large Pull Requests
200+
If the PR is very large:
201+
- Focus on architectural and design issues first
202+
- Prioritize critical bugs and security issues
203+
- Suggest breaking the PR into smaller pieces if appropriate
204+
- Provide high-level feedback on structure and approach
205+
206+
### Complex Changes
207+
For complex technical changes:
208+
- Take time to understand the full context
209+
- Ask clarifying questions if needed
210+
- Focus on maintainability and future extensibility
211+
- Verify that the solution aligns with project guidelines
212+
213+
### Disagreements
214+
If you disagree with the approach:
215+
- Explain your reasoning clearly
216+
- Reference project guidelines and standards
217+
- Suggest alternative approaches
218+
- Be open to discussion and learning

‎strands-command/scripts/javascript/process-input.cjs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ function buildPrompts(mode, issueId, isPullRequest, command, branchName, inputs)
7272
const scriptFiles = {
7373
'implementer': 'devtools/strands-command/agent-sops/task-implementer.sop.md',
7474
'refiner': 'devtools/strands-command/agent-sops/task-refiner.sop.md',
75-
'release-notes': 'devtools/strands-command/agent-sops/task-release-notes.sop.md'
75+
'release-notes': 'devtools/strands-command/agent-sops/task-release-notes.sop.md',
76+
'reviewer': 'devtools/strands-command/agent-sops/task-reviewer.sop.md'
7677
};
7778

7879
const scriptFile = scriptFiles[mode] || scriptFiles['refiner'];
@@ -98,6 +99,8 @@ module.exports = async (context, github, core, inputs) => {
9899
mode = 'release-notes';
99100
} else if (command.startsWith('implement')) {
100101
mode = 'implementer';
102+
} else if (command.startsWith('review')) {
103+
mode = 'reviewer';
101104
} else if (command.startsWith('refine')) {
102105
mode = 'refiner';
103106
} else {

‎strands-command/scripts/python/agent_runner.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,13 @@
2020
# Import local GitHub tools we need
2121
from github_tools import (
2222
add_issue_comment,
23+
add_pr_comment,
2324
create_issue,
2425
create_pull_request,
2526
get_issue,
2627
get_issue_comments,
2728
get_pull_request,
29+
get_pr_files,
2830
get_pr_review_and_comments,
2931
list_issues,
3032
list_pull_requests,
@@ -69,8 +71,10 @@ def _get_all_tools() -> list[Any]:
6971
get_pull_request,
7072
update_pull_request,
7173
list_pull_requests,
74+
get_pr_files,
7275
get_pr_review_and_comments,
7376
reply_to_review_comment,
77+
add_pr_comment,
7478

7579
# Agent tools
7680
notebook,

‎strands-command/scripts/python/github_tools.py‎

Lines changed: 109 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,114 @@ def reply_to_review_comment(pr_number: int, comment_id: int, reply_text: str, re
450450
return message
451451

452452

453+
@tool
454+
@log_inputs
455+
@check_should_call_write_api_or_record
456+
def add_pr_comment(pr_number: int, body: str, path: str | None = None, line: int | None = None, repo: str | None = None) -> str:
457+
"""Adds a comment to a pull request - either inline on a specific line, file-level, or general PR comment.
458+
459+
Args:
460+
pr_number: The pull request number
461+
body: The comment text
462+
path: The file path to comment on (optional; if omitted, creates general PR comment)
463+
line: The line number to comment on (optional; if omitted with path, creates file-level comment)
464+
repo: GitHub repository in the format "owner/repo" (optional; falls back to env var)
465+
466+
Returns:
467+
Result of the operation
468+
"""
469+
# If no path provided, create a general PR comment (issue comment)
470+
if path is None:
471+
result = _github_request("POST", f"issues/{pr_number}/comments", repo, {"body": body})
472+
if isinstance(result, str):
473+
console.print(Panel(escape(result), title="[bold red]Error", border_style="red"))
474+
return result
475+
476+
message = f"PR comment added: {result['html_url']}"
477+
console.print(Panel(escape(f"Comment: {body}\nURL: {result['html_url']}"),
478+
title="[bold green]✅ PR Comment Added", border_style="green"))
479+
return message
480+
481+
# Get the latest commit SHA for the PR
482+
pr_result = _github_request("GET", f"pulls/{pr_number}", repo)
483+
if isinstance(pr_result, str):
484+
console.print(Panel(escape(pr_result), title="[bold red]Error", border_style="red"))
485+
return pr_result
486+
487+
commit_sha = pr_result['head']['sha']
488+
489+
# Create inline or file-level comment
490+
comment_data = {
491+
"body": body,
492+
"commit_id": commit_sha,
493+
"path": path
494+
}
495+
496+
# Add line number if provided (inline comment), otherwise it's a file-level comment
497+
if line is not None:
498+
comment_data["line"] = line
499+
500+
result = _github_request("POST", f"pulls/{pr_number}/comments", repo, comment_data)
501+
if isinstance(result, str):
502+
console.print(Panel(escape(result), title="[bold red]Error", border_style="red"))
503+
return result
504+
505+
comment_type = "Inline" if line else "File-level"
506+
location = f"{path}:{line}" if line else path
507+
message = f"{comment_type} comment added: {result['html_url']}"
508+
comment_details = f"Location: {location}\nComment: {body}\nURL: {result['html_url']}"
509+
console.print(Panel(escape(comment_details), title=f"[bold green]✅ {comment_type} Comment Added", border_style="green"))
510+
return message
511+
512+
513+
@tool
514+
@log_inputs
515+
def get_pr_files(pr_number: int, repo: str | None = None) -> str:
516+
"""Gets the list of files changed in a pull request with their diffs.
517+
518+
Args:
519+
pr_number: The pull request number
520+
repo: GitHub repository in the format "owner/repo" (optional; falls back to env var)
521+
522+
Returns:
523+
Formatted list of changed files with their diffs
524+
"""
525+
result = _github_request("GET", f"pulls/{pr_number}/files", repo)
526+
if isinstance(result, str):
527+
console.print(Panel(escape(result), title="[bold red]Error", border_style="red"))
528+
return result
529+
530+
output = f"Files changed in PR #{pr_number}:\n\n"
531+
532+
for file in result:
533+
filename = file['filename']
534+
status = file['status']
535+
additions = file['additions']
536+
deletions = file['deletions']
537+
changes = file['changes']
538+
539+
output += f"📄 **{filename}** ({status})\n"
540+
output += f" +{additions} -{deletions} (~{changes} changes)\n"
541+
542+
if file.get('patch'):
543+
output += f" Diff:\n"
544+
# Limit diff size to avoid overwhelming output
545+
patch_lines = file['patch'].split('\n')
546+
if len(patch_lines) > 50:
547+
output += '\n'.join(patch_lines[:50])
548+
output += f"\n ... (truncated, {len(patch_lines) - 50} more lines)\n"
549+
else:
550+
output += file['patch']
551+
output += "\n"
552+
else:
553+
output += " (Binary file or no diff available)\n"
554+
555+
output += "\n"
556+
557+
console.print(Panel(escape(output), title=f"[bold green]PR #{pr_number} Files", border_style="blue"))
558+
return output
559+
560+
453561
# =============================================================================
454562
# READ FUNCTIONS (Functions that only read GitHub resources)
455563
# =============================================================================
@@ -840,4 +948,4 @@ def get_pr_review_and_comments(pr_number: int, show_resolved: bool = False, repo
840948
except Exception as e:
841949
error_msg = f"Error: {e!s}\n\nStack trace:\n{traceback.format_exc()}"
842950
console.print(Panel(escape(error_msg), title="[bold red]Error", border_style="red"))
843-
return error_msg
951+
return error_msg

0 commit comments

Comments
 (0)