From cbacead1544dce08e86c99193b9d3592ad0534b3 Mon Sep 17 00:00:00 2001 From: Rachit Mehta Date: Tue, 23 Dec 2025 10:10:20 -0500 Subject: [PATCH 01/12] test reviewer agent --- .github/agent-sops/task-reviewer.sop.md | 322 +++++++++++++++++++ .github/scripts/javascript/process-input.cjs | 5 +- .github/scripts/python/agent_runner.py | 4 + .github/scripts/python/github_tools.py | 97 ++++++ .github/scripts/python/write_executor.py | 2 + 5 files changed, 429 insertions(+), 1 deletion(-) create mode 100644 .github/agent-sops/task-reviewer.sop.md diff --git a/.github/agent-sops/task-reviewer.sop.md b/.github/agent-sops/task-reviewer.sop.md new file mode 100644 index 000000000..1ac4a866a --- /dev/null +++ b/.github/agent-sops/task-reviewer.sop.md @@ -0,0 +1,322 @@ +# Task Reviewer SOP + +## Role + +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 development tenets and guidelines. + +## Steps + +### 1. Setup Review Environment + +Initialize the review environment and understand the pull request context. + +**Constraints:** +- You MUST create a progress notebook to track your review process using markdown checklists +- You MUST read the pull request description and understand the purpose of the changes +- You MUST check the current branch and ensure you're reviewing the correct code +- You MUST note the PR number and branch name in your notebook +- You MUST identify the type of changes (feature, bugfix, refactor, etc.) + +### 2. Analyze Pull Request Context + +Understand what the PR is trying to accomplish and gather relevant context. + +**Constraints:** +- You MUST read the PR description thoroughly +- You MUST identify the linked issue if present +- You MUST understand the acceptance criteria being addressed +- You MUST note any special considerations mentioned in the PR description +- You MUST check for any existing review comments to avoid duplication +- You MUST review the files changed and understand the scope of modifications + +### 3. Review Repository Guidelines + +Review the project's coding standards and development principles. + +**Constraints:** +- You MUST read and understand the development tenets from `CONTRIBUTING.md`: + 1. Simple at any scale + 2. Extensible by design + 3. Composability + 4. The obvious path is the happy path + 5. We are accessible to humans and agents + 6. Embrace common standards +- You MUST review the coding patterns and testing patterns from `AGENTS.md` +- You MUST understand the project's quality requirements: + - 80%+ test coverage + - No `any` types allowed + - Explicit return types required + - TSDoc comments for all exported functions + - ESLint compliance + - Prettier formatting +- You MUST note any specific patterns or conventions used in the codebase + +### 4. Code Analysis Phase + +Perform a comprehensive analysis of the code changes. + +#### 4.1 Structural Review + +Analyze the overall structure and architecture of the changes. + +**Constraints:** +- You MUST review the file organization and directory structure +- You MUST check if new files follow existing naming conventions +- You MUST verify that changes align with the project's architectural patterns +- You MUST identify any potential breaking changes +- You MUST check for proper separation of concerns +- You MUST note any violations of the composability tenet + +#### 4.2 Code Quality Review + +Examine the code for quality, readability, and maintainability issues. + +**Constraints:** +- You MUST check for TypeScript best practices: + - No `any` types + - Explicit return types + - Proper type definitions + - Appropriate use of generics +- You MUST verify adherence to the "simple at any scale" tenet +- You MUST check for code complexity and suggest simplifications +- You MUST identify unclear or confusing code patterns +- You MUST verify proper error handling +- You MUST check for potential performance issues +- You MUST ensure the "obvious path is the happy path" tenet is followed + +#### 4.3 Testing Review + +Analyze the test coverage and quality of tests. + +**Constraints:** +- You MUST verify that new functionality has corresponding tests +- You MUST check that tests follow the patterns in `docs/TESTING.md` if available +- You MUST ensure tests are in the correct directories (`src/**/__tests__/**` for unit tests) +- You MUST verify test coverage meets the 80% requirement +- You MUST check for proper test organization and naming +- You MUST identify missing edge cases or error scenarios +- You MUST verify integration tests are included when appropriate + +#### 4.4 Documentation Review + +Check documentation completeness and quality. + +**Constraints:** +- You MUST verify TSDoc comments exist for all exported functions +- You MUST check that documentation is clear and helpful +- You MUST ensure examples are provided for complex APIs +- You MUST verify that README.md updates are included if needed +- You MUST check that AGENTS.md is updated if development patterns changed +- You MUST ensure the code is "accessible to humans and agents" per the tenet + +### 5. Generate Review Comments + +Create specific, actionable review comments for identified issues. + +**Constraints:** +- You MUST focus on the most impactful improvements first +- You MUST provide specific suggestions rather than vague feedback +- You MUST include code examples when suggesting changes +- You MUST reference the relevant development tenets when applicable +- You MUST categorize feedback as: + - **Critical**: Must be fixed (security, breaking changes, major bugs) + - **Important**: Should be fixed (quality, maintainability, standards) + - **Suggestion**: Nice to have (optimizations, style preferences) +- You MUST be constructive and educational in your feedback +- You MUST avoid nitpicking on minor style issues if they don't impact functionality +- You MUST prioritize feedback that helps the developer learn and improve + +#### 5.1 Comment Structure + +Format review comments to be clear and actionable. + +**Constraints:** +- You MUST start with a clear summary of the issue +- You MUST explain why the change is needed +- You MUST provide a specific suggestion or solution +- You MUST include code examples when helpful +- You MUST reference documentation or standards when applicable +- You SHOULD use this format: + ``` + **Issue**: [Brief description of the problem] + + **Why**: [Explanation of why this matters] + + **Suggestion**: [Specific recommendation] + + ```[language] + // Example code if applicable + ``` + + **Reference**: [Link to relevant documentation/standards] + ``` + +### 6. Post Review Comments + +Add the review comments to the pull request. + +**Constraints:** +- You MUST post comments on specific lines where issues are identified +- You MUST use the `reply_to_review_comment` tool for line-specific feedback +- You MUST group related comments when possible +- You MUST avoid overwhelming the author with too many minor comments +- You MUST prioritize the most important feedback +- You MUST be respectful and professional in all comments +- You SHOULD limit to 10-15 comments per review to avoid overwhelming the author +- You MUST focus on teaching moments that help the developer improve + +### 7. Summary Review Comment + +Provide an overall summary of the review. + +**Constraints:** +- You MUST add a general comment summarizing the review +- You MUST highlight the positive aspects of the PR +- You MUST provide an overall assessment (Approve, Request Changes, Comment) +- You MUST list the main areas for improvement +- You MUST encourage the developer and acknowledge good practices +- You MUST be clear about which items are blocking vs. suggestions +- You SHOULD use this format: + ``` + ## Review Summary + + **Overall Assessment**: [Approve/Request Changes/Comment] + + **Positive Highlights**: + - [List good practices and well-implemented features] + + **Key Areas for Improvement**: + - [List main issues that should be addressed] + + **Suggestions for Future**: + - [List nice-to-have improvements] + + Great work on [specific positive aspect]! The implementation shows good understanding of [relevant concept]. + ``` + +### 8. Follow-up Review + +If the author makes changes based on feedback, review the updates. + +**Constraints:** +- You MAY skip this step if this is the initial review +- You MUST check if your previous comments have been addressed +- You MUST verify that new changes don't introduce other issues +- You MUST acknowledge when feedback has been properly addressed +- You MUST provide approval when all critical issues are resolved +- You SHOULD be responsive to questions from the author + +## Review Focus Areas + +### Code Quality Priorities + +1. **Type Safety**: Ensure proper TypeScript usage without `any` types +2. **Error Handling**: Verify robust error handling and edge cases +3. **Performance**: Identify potential performance bottlenecks +4. **Security**: Check for security vulnerabilities or data exposure +5. **Maintainability**: Ensure code is readable and maintainable +6. **Testing**: Verify comprehensive test coverage and quality + +### Development Tenets Application + +- **Simple at any scale**: Is the solution as simple as possible while meeting requirements? +- **Extensible by design**: Does the code provide appropriate extension points? +- **Composability**: Do the components work well with existing features? +- **Obvious path is happy path**: Is the API intuitive and guide users correctly? +- **Accessible to humans and agents**: Is the code well-documented and understandable? +- **Embrace common standards**: Does the code follow established patterns and conventions? + +## Examples + +### Example Critical Comment +``` +**Issue**: Using `any` type defeats TypeScript's type safety + +**Why**: This violates our type safety requirements and makes the code harder to maintain. The `any` type bypasses all type checking and can lead to runtime errors. + +**Suggestion**: Define a proper interface for the expected shape: + +```typescript +interface UserData { + id: string; + name: string; + email: string; +} + +function processUser(userData: UserData): void { + // Implementation +} +``` + +**Reference**: See CONTRIBUTING.md - "No `any` types allowed" +``` + +### Example Suggestion Comment +``` +**Issue**: This function could be more composable + +**Why**: Following our "Composability" tenet, this function could be broken down into smaller, reusable pieces that work well with other parts of the system. + +**Suggestion**: Consider extracting the validation logic: + +```typescript +function validateInput(input: string): boolean { + return input.length > 0 && input.trim() !== ''; +} + +function processValidInput(input: string): Result { + if (!validateInput(input)) { + throw new Error('Invalid input'); + } + // Process the input +} +``` + +This makes the validation reusable and the processing logic clearer. +``` + +## Best Practices + +### Review Efficiency +- Focus on the most impactful issues first +- Provide specific, actionable feedback +- Include code examples in suggestions +- Reference project standards and documentation +- Be educational and constructive + +### Communication +- Be respectful and professional +- Acknowledge good practices +- Explain the reasoning behind feedback +- Provide learning opportunities +- Encourage the developer + +### Quality Gates +- Ensure critical issues are marked as blocking +- Verify test coverage meets requirements +- Check TypeScript compliance +- Validate documentation completeness +- Confirm adherence to development tenets + +## Troubleshooting + +### Large Pull Requests +If the PR is very large: +- Focus on architectural and design issues first +- Prioritize critical bugs and security issues +- Suggest breaking the PR into smaller pieces if appropriate +- Provide high-level feedback on structure and approach + +### Complex Changes +For complex technical changes: +- Take time to understand the full context +- Ask clarifying questions if needed +- Focus on maintainability and future extensibility +- Verify that the solution aligns with project tenets + +### Disagreements +If you disagree with the approach: +- Explain your reasoning clearly +- Reference project tenets and standards +- Suggest alternative approaches +- Be open to discussion and learning diff --git a/.github/scripts/javascript/process-input.cjs b/.github/scripts/javascript/process-input.cjs index b7ed29263..278b38d8e 100644 --- a/.github/scripts/javascript/process-input.cjs +++ b/.github/scripts/javascript/process-input.cjs @@ -70,7 +70,8 @@ function buildPrompts(mode, issueId, isPullRequest, command, branchName, inputs) const scriptFiles = { 'implementer': '.github/agent-sops/task-implementer.sop.md', 'refiner': '.github/agent-sops/task-refiner.sop.md', - 'release-notes': '.github/agent-sops/task-release-notes.sop.md' + 'release-notes': '.github/agent-sops/task-release-notes.sop.md', + 'reviewer': '.github/agent-sops/task-reviwer.sop.md' }; const scriptFile = scriptFiles[mode] || scriptFiles['refiner']; @@ -96,6 +97,8 @@ module.exports = async (context, github, core, inputs) => { mode = 'release-notes'; } else if (command.startsWith('implement')) { mode = 'implementer'; + } else if (command.startsWith('review')) { + mode = "reviewer"; } else if (command.startsWith('refine')) { mode = 'refiner'; } else { diff --git a/.github/scripts/python/agent_runner.py b/.github/scripts/python/agent_runner.py index fcde83b66..c1a6ac705 100644 --- a/.github/scripts/python/agent_runner.py +++ b/.github/scripts/python/agent_runner.py @@ -22,9 +22,11 @@ add_issue_comment, create_issue, create_pull_request, + create_pr_review, get_issue, get_issue_comments, get_pull_request, + get_pr_files, get_pr_review_and_comments, list_issues, list_pull_requests, @@ -69,8 +71,10 @@ def _get_all_tools() -> list[Any]: get_pull_request, update_pull_request, list_pull_requests, + get_pr_files, get_pr_review_and_comments, reply_to_review_comment, + create_pr_review, # Agent tools notebook, diff --git a/.github/scripts/python/github_tools.py b/.github/scripts/python/github_tools.py index 8826b4611..c698788b7 100644 --- a/.github/scripts/python/github_tools.py +++ b/.github/scripts/python/github_tools.py @@ -450,6 +450,103 @@ def reply_to_review_comment(pr_number: int, comment_id: int, reply_text: str, re return message +@tool +@log_inputs +@check_should_call_write_api_or_record +def create_pr_review(pr_number: int, body: str = "", event: str = "COMMENT", comments: list = None, repo: str | None = None) -> str: + """Creates a pull request review with optional line-specific comments. + + Args: + pr_number: The pull request number + body: The overall review comment body (optional) + event: The review event type - "APPROVE", "REQUEST_CHANGES", or "COMMENT" (default: "COMMENT") + comments: List of line-specific comments, each with keys: path, line, body (optional) + repo: GitHub repository in the format "owner/repo" (optional; falls back to env var) + + Returns: + Result of the operation + + Example comments format: + [ + { + "path": "src/example.ts", + "line": 42, + "body": "Consider using a more descriptive variable name here." + } + ] + """ + if event not in ["APPROVE", "REQUEST_CHANGES", "COMMENT"]: + return f"Error: Invalid event type '{event}'. Must be APPROVE, REQUEST_CHANGES, or COMMENT" + + review_data = { + "event": event + } + + if body: + review_data["body"] = body + + if comments: + review_data["comments"] = comments + + result = _github_request("POST", f"pulls/{pr_number}/reviews", repo, review_data) + if isinstance(result, str): + console.print(Panel(escape(result), title="[bold red]Error", border_style="red")) + return result + + message = f"PR review created: {result['html_url']}" + review_details = f"Event: {event}\nBody: {body or 'No general comment'}\nLine comments: {len(comments) if comments else 0}\nURL: {result['html_url']}" + console.print(Panel(escape(review_details), title="[bold green]✅ Review Created", border_style="green")) + return message + + +@tool +@log_inputs +def get_pr_files(pr_number: int, repo: str | None = None) -> str: + """Gets the list of files changed in a pull request with their diffs. + + Args: + pr_number: The pull request number + repo: GitHub repository in the format "owner/repo" (optional; falls back to env var) + + Returns: + Formatted list of changed files with their diffs + """ + result = _github_request("GET", f"pulls/{pr_number}/files", repo) + if isinstance(result, str): + console.print(Panel(escape(result), title="[bold red]Error", border_style="red")) + return result + + output = f"Files changed in PR #{pr_number}:\n\n" + + for file in result: + filename = file['filename'] + status = file['status'] + additions = file['additions'] + deletions = file['deletions'] + changes = file['changes'] + + output += f"📄 **{filename}** ({status})\n" + output += f" +{additions} -{deletions} (~{changes} changes)\n" + + if file.get('patch'): + output += f" Diff:\n" + # Limit diff size to avoid overwhelming output + patch_lines = file['patch'].split('\n') + if len(patch_lines) > 50: + output += '\n'.join(patch_lines[:50]) + output += f"\n ... (truncated, {len(patch_lines) - 50} more lines)\n" + else: + output += file['patch'] + output += "\n" + else: + output += " (Binary file or no diff available)\n" + + output += "\n" + + console.print(Panel(escape(output), title=f"[bold green]PR #{pr_number} Files", border_style="blue")) + return output + + # ============================================================================= # READ FUNCTIONS (Functions that only read GitHub resources) # ============================================================================= diff --git a/.github/scripts/python/write_executor.py b/.github/scripts/python/write_executor.py index 6d3b6b84d..51e20b6ea 100755 --- a/.github/scripts/python/write_executor.py +++ b/.github/scripts/python/write_executor.py @@ -23,6 +23,7 @@ create_pull_request, update_pull_request, reply_to_review_comment, + create_pr_review, ) # Configure structured logging @@ -43,6 +44,7 @@ def get_function_mapping() -> Dict[str, Any]: create_pull_request.tool_name: create_pull_request, update_pull_request.tool_name: update_pull_request, reply_to_review_comment.tool_name: reply_to_review_comment, + create_pr_review.tool_name: create_pr_review, } From f3a1f4ed8c5c7d9b5be660d6fef5f0186cad4965 Mon Sep 17 00:00:00 2001 From: Rachit Mehta Date: Mon, 12 Jan 2026 09:34:35 -0500 Subject: [PATCH 02/12] fix typo --- .github/scripts/javascript/process-input.cjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/scripts/javascript/process-input.cjs b/.github/scripts/javascript/process-input.cjs index 278b38d8e..8e44e5241 100644 --- a/.github/scripts/javascript/process-input.cjs +++ b/.github/scripts/javascript/process-input.cjs @@ -71,7 +71,7 @@ function buildPrompts(mode, issueId, isPullRequest, command, branchName, inputs) 'implementer': '.github/agent-sops/task-implementer.sop.md', 'refiner': '.github/agent-sops/task-refiner.sop.md', 'release-notes': '.github/agent-sops/task-release-notes.sop.md', - 'reviewer': '.github/agent-sops/task-reviwer.sop.md' + 'reviewer': '.github/agent-sops/task-reviewer.sop.md' }; const scriptFile = scriptFiles[mode] || scriptFiles['refiner']; From 93e021a91e1ad5174062c2f91c44afe72e93a31a Mon Sep 17 00:00:00 2001 From: Rachit Mehta Date: Mon, 12 Jan 2026 09:50:03 -0500 Subject: [PATCH 03/12] fix typo --- .github/actions/strands-agent-runner/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/strands-agent-runner/action.yml b/.github/actions/strands-agent-runner/action.yml index 6d4c2d7fb..36c6dd572 100644 --- a/.github/actions/strands-agent-runner/action.yml +++ b/.github/actions/strands-agent-runner/action.yml @@ -91,7 +91,7 @@ runs: role-session-name: GitHubActions-StrandsAgent-${{ github.run_id }} aws-region: us-west-2 mask-aws-account-id: true - inline_session_policy: >- + inline-session-policy: >- { "Version": "2012-10-17", "Statement": [ From f01604c413f2584c8eae8b33674d34c2a68b4a6c Mon Sep 17 00:00:00 2001 From: Rachit Mehta Date: Tue, 13 Jan 2026 09:08:59 -0500 Subject: [PATCH 04/12] fix string error in github workflow --- .github/actions/strands-agent-runner/action.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/actions/strands-agent-runner/action.yml b/.github/actions/strands-agent-runner/action.yml index 36c6dd572..4f9f2197c 100644 --- a/.github/actions/strands-agent-runner/action.yml +++ b/.github/actions/strands-agent-runner/action.yml @@ -108,16 +108,16 @@ runs: "Action": [ "s3:PutObject", "s3:GetObject", - "s3:DeleteObject", + "s3:DeleteObject" ], "Resource": [ - "arn:aws:s3:::strands-typescript-project-sessions/*", + "arn:aws:s3:::strands-typescript-project-sessions/*" ] }, { "Effect": "Allow", "Action": "s3:ListBucket", "Resource": [ - "arn:aws:s3:::strands-typescript-project-sessions", + "arn:aws:s3:::strands-typescript-project-sessions" ] } ] From 8976849126e62b8d8d0dacedb47a3a44117672c6 Mon Sep 17 00:00:00 2001 From: Rachit Mehta Date: Tue, 13 Jan 2026 09:15:32 -0500 Subject: [PATCH 05/12] fix error in github workflow --- .github/actions/strands-agent-runner/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/strands-agent-runner/action.yml b/.github/actions/strands-agent-runner/action.yml index 4f9f2197c..3e94e4e0b 100644 --- a/.github/actions/strands-agent-runner/action.yml +++ b/.github/actions/strands-agent-runner/action.yml @@ -96,7 +96,7 @@ runs: "Version": "2012-10-17", "Statement": [ { - "Sid":"Bedrock Access", + "Sid":"BedrockAccess", "Effect": "Allow", "Action": [ "bedrock:InvokeModelWithResponseStream", From 1a929e0c44127d1a781b87ac636e85e6062abfe3 Mon Sep 17 00:00:00 2001 From: Rachit Mehta Date: Tue, 13 Jan 2026 09:48:13 -0500 Subject: [PATCH 06/12] test review agent --- .../actions/strands-agent-runner/action.yml | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/.github/actions/strands-agent-runner/action.yml b/.github/actions/strands-agent-runner/action.yml index 3e94e4e0b..d8b152e2e 100644 --- a/.github/actions/strands-agent-runner/action.yml +++ b/.github/actions/strands-agent-runner/action.yml @@ -91,37 +91,6 @@ runs: role-session-name: GitHubActions-StrandsAgent-${{ github.run_id }} aws-region: us-west-2 mask-aws-account-id: true - inline-session-policy: >- - { - "Version": "2012-10-17", - "Statement": [ - { - "Sid":"BedrockAccess", - "Effect": "Allow", - "Action": [ - "bedrock:InvokeModelWithResponseStream", - "bedrock:InvokeModel" - ], - "Resource": "*" - }, { - "Effect": "Allow", - "Action": [ - "s3:PutObject", - "s3:GetObject", - "s3:DeleteObject" - ], - "Resource": [ - "arn:aws:s3:::strands-typescript-project-sessions/*" - ] - }, { - "Effect": "Allow", - "Action": "s3:ListBucket", - "Resource": [ - "arn:aws:s3:::strands-typescript-project-sessions" - ] - } - ] - } - name: Execute strands command From 738d8d935dd1061738ed4465e13f6515bd43245c Mon Sep 17 00:00:00 2001 From: Rachit Mehta Date: Tue, 13 Jan 2026 10:36:21 -0500 Subject: [PATCH 07/12] fix github tools --- .github/agent-sops/task-reviewer.sop.md | 4 +- .github/scripts/python/agent_runner.py | 4 +- .github/scripts/python/github_tools.py | 49 ++++++++++-------------- .github/scripts/python/write_executor.py | 4 +- 4 files changed, 28 insertions(+), 33 deletions(-) diff --git a/.github/agent-sops/task-reviewer.sop.md b/.github/agent-sops/task-reviewer.sop.md index 1ac4a866a..6eb433e67 100644 --- a/.github/agent-sops/task-reviewer.sop.md +++ b/.github/agent-sops/task-reviewer.sop.md @@ -27,7 +27,7 @@ Understand what the PR is trying to accomplish and gather relevant context. - You MUST understand the acceptance criteria being addressed - You MUST note any special considerations mentioned in the PR description - You MUST check for any existing review comments to avoid duplication -- You MUST review the files changed and understand the scope of modifications +- You MUST use the `get_pr_files` tool to review the files changed and understand the scope of modifications ### 3. Review Repository Guidelines @@ -157,7 +157,9 @@ Add the review comments to the pull request. **Constraints:** - You MUST post comments on specific lines where issues are identified +- You MUST use the `add_pr_inline_comment` tool for line-specific feedback on code - You MUST use the `reply_to_review_comment` tool for line-specific feedback +- You MUST use the `add_issue_comment` tool for general PR-level comments - You MUST group related comments when possible - You MUST avoid overwhelming the author with too many minor comments - You MUST prioritize the most important feedback diff --git a/.github/scripts/python/agent_runner.py b/.github/scripts/python/agent_runner.py index c1a6ac705..816832abf 100644 --- a/.github/scripts/python/agent_runner.py +++ b/.github/scripts/python/agent_runner.py @@ -20,9 +20,9 @@ # Import local GitHub tools we need from github_tools import ( add_issue_comment, + add_pr_inline_comment, create_issue, create_pull_request, - create_pr_review, get_issue, get_issue_comments, get_pull_request, @@ -74,7 +74,7 @@ def _get_all_tools() -> list[Any]: get_pr_files, get_pr_review_and_comments, reply_to_review_comment, - create_pr_review, + add_pr_inline_comment, # Agent tools notebook, diff --git a/.github/scripts/python/github_tools.py b/.github/scripts/python/github_tools.py index c698788b7..ed83e8de7 100644 --- a/.github/scripts/python/github_tools.py +++ b/.github/scripts/python/github_tools.py @@ -453,49 +453,42 @@ def reply_to_review_comment(pr_number: int, comment_id: int, reply_text: str, re @tool @log_inputs @check_should_call_write_api_or_record -def create_pr_review(pr_number: int, body: str = "", event: str = "COMMENT", comments: list = None, repo: str | None = None) -> str: - """Creates a pull request review with optional line-specific comments. +def add_pr_inline_comment(pr_number: int, path: str, line: int, body: str, repo: str | None = None) -> str: + """Adds an inline comment to a specific line in a pull request file. Args: pr_number: The pull request number - body: The overall review comment body (optional) - event: The review event type - "APPROVE", "REQUEST_CHANGES", or "COMMENT" (default: "COMMENT") - comments: List of line-specific comments, each with keys: path, line, body (optional) + path: The file path to comment on + line: The line number to comment on + body: The comment text repo: GitHub repository in the format "owner/repo" (optional; falls back to env var) Returns: Result of the operation - - Example comments format: - [ - { - "path": "src/example.ts", - "line": 42, - "body": "Consider using a more descriptive variable name here." - } - ] """ - if event not in ["APPROVE", "REQUEST_CHANGES", "COMMENT"]: - return f"Error: Invalid event type '{event}'. Must be APPROVE, REQUEST_CHANGES, or COMMENT" + # Get the latest commit SHA for the PR + pr_result = _github_request("GET", f"pulls/{pr_number}", repo) + if isinstance(pr_result, str): + console.print(Panel(escape(pr_result), title="[bold red]Error", border_style="red")) + return pr_result - review_data = { - "event": event - } + commit_sha = pr_result['head']['sha'] - if body: - review_data["body"] = body - - if comments: - review_data["comments"] = comments + comment_data = { + "body": body, + "commit_id": commit_sha, + "path": path, + "line": line + } - result = _github_request("POST", f"pulls/{pr_number}/reviews", repo, review_data) + result = _github_request("POST", f"pulls/{pr_number}/comments", repo, comment_data) if isinstance(result, str): console.print(Panel(escape(result), title="[bold red]Error", border_style="red")) return result - message = f"PR review created: {result['html_url']}" - review_details = f"Event: {event}\nBody: {body or 'No general comment'}\nLine comments: {len(comments) if comments else 0}\nURL: {result['html_url']}" - console.print(Panel(escape(review_details), title="[bold green]✅ Review Created", border_style="green")) + message = f"Inline comment added: {result['html_url']}" + comment_details = f"File: {path}:{line}\nComment: {body}\nURL: {result['html_url']}" + console.print(Panel(escape(comment_details), title="[bold green]✅ Inline Comment Added", border_style="green")) return message diff --git a/.github/scripts/python/write_executor.py b/.github/scripts/python/write_executor.py index 51e20b6ea..87fc8e14b 100755 --- a/.github/scripts/python/write_executor.py +++ b/.github/scripts/python/write_executor.py @@ -23,7 +23,7 @@ create_pull_request, update_pull_request, reply_to_review_comment, - create_pr_review, + add_pr_inline_comment, ) # Configure structured logging @@ -44,7 +44,7 @@ def get_function_mapping() -> Dict[str, Any]: create_pull_request.tool_name: create_pull_request, update_pull_request.tool_name: update_pull_request, reply_to_review_comment.tool_name: reply_to_review_comment, - create_pr_review.tool_name: create_pr_review, + add_pr_inline_comment.tool_name: add_pr_inline_comment, } From 06a9ac84cb8948f0f60188103f833e633b12e0da Mon Sep 17 00:00:00 2001 From: Rachit Mehta Date: Tue, 13 Jan 2026 10:41:17 -0500 Subject: [PATCH 08/12] add back session policy --- .../actions/strands-agent-runner/action.yml | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/.github/actions/strands-agent-runner/action.yml b/.github/actions/strands-agent-runner/action.yml index d8b152e2e..3e94e4e0b 100644 --- a/.github/actions/strands-agent-runner/action.yml +++ b/.github/actions/strands-agent-runner/action.yml @@ -91,6 +91,37 @@ runs: role-session-name: GitHubActions-StrandsAgent-${{ github.run_id }} aws-region: us-west-2 mask-aws-account-id: true + inline-session-policy: >- + { + "Version": "2012-10-17", + "Statement": [ + { + "Sid":"BedrockAccess", + "Effect": "Allow", + "Action": [ + "bedrock:InvokeModelWithResponseStream", + "bedrock:InvokeModel" + ], + "Resource": "*" + }, { + "Effect": "Allow", + "Action": [ + "s3:PutObject", + "s3:GetObject", + "s3:DeleteObject" + ], + "Resource": [ + "arn:aws:s3:::strands-typescript-project-sessions/*" + ] + }, { + "Effect": "Allow", + "Action": "s3:ListBucket", + "Resource": [ + "arn:aws:s3:::strands-typescript-project-sessions" + ] + } + ] + } - name: Execute strands command From 5e77097c2e6fbce129d469a8109bc3055b183ce1 Mon Sep 17 00:00:00 2001 From: Rachit Mehta Date: Tue, 13 Jan 2026 10:57:48 -0500 Subject: [PATCH 09/12] add package-lock.json --- package-lock.json | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/package-lock.json b/package-lock.json index 732798594..e33fa9ca0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4571,6 +4571,7 @@ "integrity": "sha512-gqkrWUsS8hcm0r44yn7/xZeV1ERva/nLgrLxFRUGb7aoNMIJfZJ3AC261zDQuOAKC7MiXai1WCpYc48jAHoShQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "undici-types": "~7.16.0" } @@ -4610,6 +4611,7 @@ "integrity": "sha512-N9lBGA9o9aqb1hVMc9hzySbhKibHmB+N3IpoShyV6HyQYRGIhlrO5rQgttypi+yEeKsKI4idxC8Jw6gXKD4THA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.49.0", "@typescript-eslint/types": "8.49.0", @@ -4814,6 +4816,7 @@ "integrity": "sha512-zedtczX688KehaIaAv7m25CeDLb0gBtAOa2Oi1G1cqvSO5aLSVfH6lpZMJLW8BKYuWMxLQc9/5GYoM+jgvGIrw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@vitest/mocker": "4.0.15", "@vitest/utils": "4.0.15", @@ -4837,6 +4840,7 @@ "integrity": "sha512-94yVpDbb+ykiT7mK6ToonGnq2GIHEQGBTZTAzGxBGQXcVNCh54YKC2/WkfaDzxy0m6Kgw05kq3FYHKHu+wRdIA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@vitest/browser": "4.0.15", "@vitest/mocker": "4.0.15", @@ -5017,6 +5021,7 @@ "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "dev": true, "license": "MIT", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -5507,6 +5512,7 @@ "integrity": "sha512-LEyamqS7W5HB3ujJyvi0HQK/dtVINZvd5mAAp9eT5S/ujByGjiZLCzPcHVzuXbpJDJF/cxwHlfceVUDZ2lnSTw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -5961,6 +5967,7 @@ "resolved": "https://registry.npmjs.org/express/-/express-5.2.1.tgz", "integrity": "sha512-hIS4idWWai69NezIdRt2xFVofaF4j+6INOpJlVOLDO8zXGpUVEVzIYk12UUi2JzjEzWL3IOAxcTubgz9Po0yXw==", "license": "MIT", + "peer": true, "dependencies": { "accepts": "^2.0.0", "body-parser": "^2.2.1", @@ -7116,6 +7123,7 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -7151,6 +7159,7 @@ "integrity": "sha512-ilYQj1s8sr2ppEJ2YVadYBN0Mb3mdo9J0wQ+UuDhzYqURwSoW4n1Xs5vs7ORwgDGmyEh33tRMeS8KhdkMoLXQw==", "dev": true, "license": "Apache-2.0", + "peer": true, "dependencies": { "playwright-core": "1.57.0" }, @@ -8376,6 +8385,7 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -8425,6 +8435,7 @@ "integrity": "sha512-NL8jTlbo0Tn4dUEXEsUg8KeyG/Lkmc4Fnzb8JXN/Ykm9G4HNImjtABMJgkQoVjOBN/j2WAwDTRytdqJbZsah7w==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.5.0", @@ -8515,6 +8526,7 @@ "integrity": "sha512-n1RxDp8UJm6N0IbJLQo+yzLZ2sQCDyl1o0LeugbPWf8+8Fttp29GghsQBjYJVmWq3gBFfe9Hs1spR44vovn2wA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@vitest/expect": "4.0.15", "@vitest/mocker": "4.0.15", @@ -8641,6 +8653,7 @@ "integrity": "sha512-PEIGCY5tSlUt50cqyMXfCzX+oOPqN0vuGqWzbcJ2xvnkzkq46oOpz7dQaTDBdfICb4N14+GARUDw2XV2N4tvzg==", "devOptional": true, "license": "MIT", + "peer": true, "engines": { "node": ">=10.0.0" }, @@ -8675,6 +8688,7 @@ "resolved": "https://registry.npmjs.org/zod/-/zod-4.3.5.tgz", "integrity": "sha512-k7Nwx6vuWx1IJ9Bjuf4Zt1PEllcwe7cls3VNzm4CQ1/hgtFUK2bRNG3rvnpPUhFjmqJKAKtjV576KnUkHocg/g==", "license": "MIT", + "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } From a23187838a20a4be16288694024263bab7ae6379 Mon Sep 17 00:00:00 2001 From: Rachit Mehta Date: Wed, 14 Jan 2026 13:44:23 -0500 Subject: [PATCH 10/12] address comments and improvements --- .github/agent-sops/task-reviewer.sop.md | 223 ++++++----------------- .github/scripts/python/agent_runner.py | 4 +- .github/scripts/python/github_tools.py | 36 +++- .github/scripts/python/write_executor.py | 4 +- 4 files changed, 89 insertions(+), 178 deletions(-) diff --git a/.github/agent-sops/task-reviewer.sop.md b/.github/agent-sops/task-reviewer.sop.md index 6eb433e67..c7a605f7a 100644 --- a/.github/agent-sops/task-reviewer.sop.md +++ b/.github/agent-sops/task-reviewer.sop.md @@ -2,60 +2,44 @@ ## Role -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 development tenets and guidelines. +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. ## Steps ### 1. Setup Review Environment -Initialize the review environment and understand the pull request context. +Initialize the review environment by checking out the main branch for guidance. **Constraints:** +- You MUST checkout the main branch first to read repository review guidance - You MUST create a progress notebook to track your review process using markdown checklists -- You MUST read the pull request description and understand the purpose of the changes -- You MUST check the current branch and ensure you're reviewing the correct code -- You MUST note the PR number and branch name in your notebook -- You MUST identify the type of changes (feature, bugfix, refactor, etc.) +- You MUST read repository guidelines from `README.md`, `CONTRIBUTING.md`, and `AGENTS.md` (if present) +- You MUST create a checklist of items to review based on the repository guidelines ### 2. Analyze Pull Request Context -Understand what the PR is trying to accomplish and gather relevant context. +Checkout the PR branch and understand what the PR is trying to accomplish. **Constraints:** +- You MUST checkout the PR branch to review the actual changes +- You MUST read the pull request description and understand the purpose of the changes +- You MUST note the PR number and branch name in your notebook +- You MUST identify the type of changes (feature, bugfix, refactor, etc.) - You MUST read the PR description thoroughly - You MUST identify the linked issue if present - You MUST understand the acceptance criteria being addressed - You MUST note any special considerations mentioned in the PR description - You MUST check for any existing review comments to avoid duplication - You MUST use the `get_pr_files` tool to review the files changed and understand the scope of modifications +- You MUST check for duplicate functionality by searching the codebase: + - For newly added tests, check if similar tests already exist + - For new helper functions, verify they aren't already implemented elsewhere -### 3. Review Repository Guidelines - -Review the project's coding standards and development principles. - -**Constraints:** -- You MUST read and understand the development tenets from `CONTRIBUTING.md`: - 1. Simple at any scale - 2. Extensible by design - 3. Composability - 4. The obvious path is the happy path - 5. We are accessible to humans and agents - 6. Embrace common standards -- You MUST review the coding patterns and testing patterns from `AGENTS.md` -- You MUST understand the project's quality requirements: - - 80%+ test coverage - - No `any` types allowed - - Explicit return types required - - TSDoc comments for all exported functions - - ESLint compliance - - Prettier formatting -- You MUST note any specific patterns or conventions used in the codebase - -### 4. Code Analysis Phase +### 3. Code Analysis Phase Perform a comprehensive analysis of the code changes. -#### 4.1 Structural Review +#### 3.1 Structural Review Analyze the overall structure and architecture of the changes. @@ -65,59 +49,49 @@ Analyze the overall structure and architecture of the changes. - You MUST verify that changes align with the project's architectural patterns - You MUST identify any potential breaking changes - You MUST check for proper separation of concerns -- You MUST note any violations of the composability tenet -#### 4.2 Code Quality Review +#### 3.2 Code Quality Review Examine the code for quality, readability, and maintainability issues. **Constraints:** -- You MUST check for TypeScript best practices: - - No `any` types - - Explicit return types - - Proper type definitions - - Appropriate use of generics -- You MUST verify adherence to the "simple at any scale" tenet +- You MUST check for language-specific best practices as defined in repository guidelines - You MUST check for code complexity and suggest simplifications - You MUST identify unclear or confusing code patterns - You MUST verify proper error handling - You MUST check for potential performance issues -- You MUST ensure the "obvious path is the happy path" tenet is followed -#### 4.3 Testing Review +#### 3.3 Testing Review Analyze the test coverage and quality of tests. **Constraints:** - You MUST verify that new functionality has corresponding tests -- You MUST check that tests follow the patterns in `docs/TESTING.md` if available -- You MUST ensure tests are in the correct directories (`src/**/__tests__/**` for unit tests) -- You MUST verify test coverage meets the 80% requirement +- You MUST check that tests follow the patterns defined in repository documentation +- You MUST ensure tests are in the correct directories as specified in guidelines - You MUST check for proper test organization and naming - You MUST identify missing edge cases or error scenarios - You MUST verify integration tests are included when appropriate -#### 4.4 Documentation Review +#### 3.4 Documentation Review Check documentation completeness and quality. **Constraints:** -- You MUST verify TSDoc comments exist for all exported functions -- You MUST check that documentation is clear and helpful -- You MUST ensure examples are provided for complex APIs +- You MUST verify documentation exists for all public APIs as required by repository guidelines +- You MUST check that documentation is clear, helpful, and concise +- You MAY suggest examples for complex APIs - You MUST verify that README.md updates are included if needed -- You MUST check that AGENTS.md is updated if development patterns changed -- You MUST ensure the code is "accessible to humans and agents" per the tenet +- You MUST check that development documentation is updated if patterns changed -### 5. Generate Review Comments +### 4. Generate Review Comments Create specific, actionable review comments for identified issues. **Constraints:** - You MUST focus on the most impactful improvements first - You MUST provide specific suggestions rather than vague feedback -- You MUST include code examples when suggesting changes -- You MUST reference the relevant development tenets when applicable +- You MUST be concise in your feedback - You MUST categorize feedback as: - **Critical**: Must be fixed (security, breaking changes, major bugs) - **Important**: Should be fixed (quality, maintainability, standards) @@ -125,41 +99,30 @@ Create specific, actionable review comments for identified issues. - You MUST be constructive and educational in your feedback - You MUST avoid nitpicking on minor style issues if they don't impact functionality - You MUST prioritize feedback that helps the developer learn and improve +- You MAY skip this step if you have no feedback to provide -#### 5.1 Comment Structure +#### 4.1 Comment Structure Format review comments to be clear and actionable. **Constraints:** -- You MUST start with a clear summary of the issue -- You MUST explain why the change is needed -- You MUST provide a specific suggestion or solution -- You MUST include code examples when helpful -- You MUST reference documentation or standards when applicable +- You MUST be concise - avoid verbose explanations +- You MUST provide specific suggestions +- You MAY reference documentation or standards when applicable - You SHOULD use this format: ``` - **Issue**: [Brief description of the problem] - - **Why**: [Explanation of why this matters] - + **Issue**: [Brief description] **Suggestion**: [Specific recommendation] - - ```[language] - // Example code if applicable - ``` - - **Reference**: [Link to relevant documentation/standards] ``` -### 6. Post Review Comments +### 5. Post Review Comments Add the review comments to the pull request. **Constraints:** -- You MUST post comments on specific lines where issues are identified -- You MUST use the `add_pr_inline_comment` tool for line-specific feedback on code -- You MUST use the `reply_to_review_comment` tool for line-specific feedback -- You MUST use the `add_issue_comment` tool for general PR-level comments +- You MUST use the `add_pr_comment` tool for inline comments on specific lines +- You MUST use the `add_pr_comment` tool with no line number for file-level comments +- You MUST use the `reply_to_review_comment` tool to reply to existing inline comments - You MUST group related comments when possible - You MUST avoid overwhelming the author with too many minor comments - You MUST prioritize the most important feedback @@ -167,36 +130,25 @@ Add the review comments to the pull request. - You SHOULD limit to 10-15 comments per review to avoid overwhelming the author - You MUST focus on teaching moments that help the developer improve -### 7. Summary Review Comment +### 6. Summary Review Comment -Provide an overall summary of the review. +Provide a concise overall summary of the review. **Constraints:** -- You MUST add a general comment summarizing the review -- You MUST highlight the positive aspects of the PR +- You MUST create a pull request review using GitHub's review feature - You MUST provide an overall assessment (Approve, Request Changes, Comment) -- You MUST list the main areas for improvement -- You MUST encourage the developer and acknowledge good practices -- You MUST be clear about which items are blocking vs. suggestions +- You MUST keep the summary concise - rely on GitHub's UI to display individual comments +- You MUST highlight key themes or patterns in the feedback - You SHOULD use this format: ``` - ## Review Summary - - **Overall Assessment**: [Approve/Request Changes/Comment] - - **Positive Highlights**: - - [List good practices and well-implemented features] + **Assessment**: [Approve/Request Changes/Comment] - **Key Areas for Improvement**: - - [List main issues that should be addressed] + **Key Themes**: [High-level patterns or areas needing attention] - **Suggestions for Future**: - - [List nice-to-have improvements] - - Great work on [specific positive aspect]! The implementation shows good understanding of [relevant concept]. + [Brief encouraging note] ``` -### 8. Follow-up Review +### 7. Follow-up Review If the author makes changes based on feedback, review the updates. @@ -212,78 +164,20 @@ If the author makes changes based on feedback, review the updates. ### Code Quality Priorities -1. **Type Safety**: Ensure proper TypeScript usage without `any` types -2. **Error Handling**: Verify robust error handling and edge cases -3. **Performance**: Identify potential performance bottlenecks -4. **Security**: Check for security vulnerabilities or data exposure -5. **Maintainability**: Ensure code is readable and maintainable -6. **Testing**: Verify comprehensive test coverage and quality - -### Development Tenets Application - -- **Simple at any scale**: Is the solution as simple as possible while meeting requirements? -- **Extensible by design**: Does the code provide appropriate extension points? -- **Composability**: Do the components work well with existing features? -- **Obvious path is happy path**: Is the API intuitive and guide users correctly? -- **Accessible to humans and agents**: Is the code well-documented and understandable? -- **Embrace common standards**: Does the code follow established patterns and conventions? - -## Examples - -### Example Critical Comment -``` -**Issue**: Using `any` type defeats TypeScript's type safety - -**Why**: This violates our type safety requirements and makes the code harder to maintain. The `any` type bypasses all type checking and can lead to runtime errors. - -**Suggestion**: Define a proper interface for the expected shape: - -```typescript -interface UserData { - id: string; - name: string; - email: string; -} - -function processUser(userData: UserData): void { - // Implementation -} -``` - -**Reference**: See CONTRIBUTING.md - "No `any` types allowed" -``` - -### Example Suggestion Comment -``` -**Issue**: This function could be more composable - -**Why**: Following our "Composability" tenet, this function could be broken down into smaller, reusable pieces that work well with other parts of the system. - -**Suggestion**: Consider extracting the validation logic: - -```typescript -function validateInput(input: string): boolean { - return input.length > 0 && input.trim() !== ''; -} - -function processValidInput(input: string): Result { - if (!validateInput(input)) { - throw new Error('Invalid input'); - } - // Process the input -} -``` - -This makes the validation reusable and the processing logic clearer. -``` +1. **Error Handling**: Verify robust error handling and edge cases +2. **Performance**: Identify potential performance bottlenecks +3. **Security**: Check for security vulnerabilities or data exposure +4. **Maintainability**: Ensure code is readable and maintainable +5. **Testing**: Verify comprehensive test coverage and quality +6. **Language Best Practices**: Follow language-specific best practices as defined in repository guidelines ## Best Practices ### Review Efficiency - Focus on the most impactful issues first - Provide specific, actionable feedback -- Include code examples in suggestions -- Reference project standards and documentation +- Be concise and avoid verbose explanations +- Reference project standards and documentation when applicable - Be educational and constructive ### Communication @@ -295,10 +189,9 @@ This makes the validation reusable and the processing logic clearer. ### Quality Gates - Ensure critical issues are marked as blocking -- Verify test coverage meets requirements -- Check TypeScript compliance +- Verify tests meet repository requirements +- Check language-specific compliance as defined in guidelines - Validate documentation completeness -- Confirm adherence to development tenets ## Troubleshooting @@ -314,11 +207,11 @@ For complex technical changes: - Take time to understand the full context - Ask clarifying questions if needed - Focus on maintainability and future extensibility -- Verify that the solution aligns with project tenets +- Verify that the solution aligns with project guidelines ### Disagreements If you disagree with the approach: - Explain your reasoning clearly -- Reference project tenets and standards +- Reference project guidelines and standards - Suggest alternative approaches - Be open to discussion and learning diff --git a/.github/scripts/python/agent_runner.py b/.github/scripts/python/agent_runner.py index 816832abf..e131a77a4 100644 --- a/.github/scripts/python/agent_runner.py +++ b/.github/scripts/python/agent_runner.py @@ -20,7 +20,7 @@ # Import local GitHub tools we need from github_tools import ( add_issue_comment, - add_pr_inline_comment, + add_pr_comment, create_issue, create_pull_request, get_issue, @@ -74,7 +74,7 @@ def _get_all_tools() -> list[Any]: get_pr_files, get_pr_review_and_comments, reply_to_review_comment, - add_pr_inline_comment, + add_pr_comment, # Agent tools notebook, diff --git a/.github/scripts/python/github_tools.py b/.github/scripts/python/github_tools.py index ed83e8de7..df64628b5 100644 --- a/.github/scripts/python/github_tools.py +++ b/.github/scripts/python/github_tools.py @@ -453,19 +453,31 @@ def reply_to_review_comment(pr_number: int, comment_id: int, reply_text: str, re @tool @log_inputs @check_should_call_write_api_or_record -def add_pr_inline_comment(pr_number: int, path: str, line: int, body: str, repo: str | None = None) -> str: - """Adds an inline comment to a specific line in a pull request file. +def add_pr_comment(pr_number: int, body: str, path: str | None = None, line: int | None = None, repo: str | None = None) -> str: + """Adds a comment to a pull request - either inline on a specific line, file-level, or general PR comment. Args: pr_number: The pull request number - path: The file path to comment on - line: The line number to comment on body: The comment text + path: The file path to comment on (optional; if omitted, creates general PR comment) + line: The line number to comment on (optional; if omitted with path, creates file-level comment) repo: GitHub repository in the format "owner/repo" (optional; falls back to env var) Returns: Result of the operation """ + # If no path provided, create a general PR comment (issue comment) + if path is None: + result = _github_request("POST", f"issues/{pr_number}/comments", repo, {"body": body}) + if isinstance(result, str): + console.print(Panel(escape(result), title="[bold red]Error", border_style="red")) + return result + + message = f"PR comment added: {result['html_url']}" + console.print(Panel(escape(f"Comment: {body}\nURL: {result['html_url']}"), + title="[bold green]✅ PR Comment Added", border_style="green")) + return message + # Get the latest commit SHA for the PR pr_result = _github_request("GET", f"pulls/{pr_number}", repo) if isinstance(pr_result, str): @@ -474,21 +486,27 @@ def add_pr_inline_comment(pr_number: int, path: str, line: int, body: str, repo: commit_sha = pr_result['head']['sha'] + # Create inline or file-level comment comment_data = { "body": body, "commit_id": commit_sha, - "path": path, - "line": line + "path": path } + # Add line number if provided (inline comment), otherwise it's a file-level comment + if line is not None: + comment_data["line"] = line + result = _github_request("POST", f"pulls/{pr_number}/comments", repo, comment_data) if isinstance(result, str): console.print(Panel(escape(result), title="[bold red]Error", border_style="red")) return result - message = f"Inline comment added: {result['html_url']}" - comment_details = f"File: {path}:{line}\nComment: {body}\nURL: {result['html_url']}" - console.print(Panel(escape(comment_details), title="[bold green]✅ Inline Comment Added", border_style="green")) + comment_type = "Inline" if line else "File-level" + location = f"{path}:{line}" if line else path + message = f"{comment_type} comment added: {result['html_url']}" + comment_details = f"Location: {location}\nComment: {body}\nURL: {result['html_url']}" + console.print(Panel(escape(comment_details), title=f"[bold green]✅ {comment_type} Comment Added", border_style="green")) return message diff --git a/.github/scripts/python/write_executor.py b/.github/scripts/python/write_executor.py index 87fc8e14b..648f6f73a 100755 --- a/.github/scripts/python/write_executor.py +++ b/.github/scripts/python/write_executor.py @@ -23,7 +23,7 @@ create_pull_request, update_pull_request, reply_to_review_comment, - add_pr_inline_comment, + add_pr_comment, ) # Configure structured logging @@ -44,7 +44,7 @@ def get_function_mapping() -> Dict[str, Any]: create_pull_request.tool_name: create_pull_request, update_pull_request.tool_name: update_pull_request, reply_to_review_comment.tool_name: reply_to_review_comment, - add_pr_inline_comment.tool_name: add_pr_inline_comment, + add_pr_comment.tool_name: add_pr_comment, } From 15673f172d49b302098095a11425e47384f7ca11 Mon Sep 17 00:00:00 2001 From: Rachit Mehta Date: Wed, 14 Jan 2026 13:53:34 -0500 Subject: [PATCH 11/12] remove follow up review --- .github/agent-sops/task-reviewer.sop.md | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/.github/agent-sops/task-reviewer.sop.md b/.github/agent-sops/task-reviewer.sop.md index c7a605f7a..38ae0cec7 100644 --- a/.github/agent-sops/task-reviewer.sop.md +++ b/.github/agent-sops/task-reviewer.sop.md @@ -148,18 +148,6 @@ Provide a concise overall summary of the review. [Brief encouraging note] ``` -### 7. Follow-up Review - -If the author makes changes based on feedback, review the updates. - -**Constraints:** -- You MAY skip this step if this is the initial review -- You MUST check if your previous comments have been addressed -- You MUST verify that new changes don't introduce other issues -- You MUST acknowledge when feedback has been properly addressed -- You MUST provide approval when all critical issues are resolved -- You SHOULD be responsive to questions from the author - ## Review Focus Areas ### Code Quality Priorities From 92a23bd5df5bd7f0c77f04f891c4f7c1dde66c90 Mon Sep 17 00:00:00 2001 From: Rachit Mehta Date: Wed, 14 Jan 2026 13:59:46 -0500 Subject: [PATCH 12/12] update sop based on graphite best practices --- .github/agent-sops/task-reviewer.sop.md | 27 ++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/.github/agent-sops/task-reviewer.sop.md b/.github/agent-sops/task-reviewer.sop.md index 38ae0cec7..7281a5119 100644 --- a/.github/agent-sops/task-reviewer.sop.md +++ b/.github/agent-sops/task-reviewer.sop.md @@ -31,6 +31,7 @@ Checkout the PR branch and understand what the PR is trying to accomplish. - You MUST note any special considerations mentioned in the PR description - You MUST check for any existing review comments to avoid duplication - You MUST use the `get_pr_files` tool to review the files changed and understand the scope of modifications +- You SHOULD flag if the PR is too large (>400 lines changed) and suggest breaking it into smaller PRs - You MUST check for duplicate functionality by searching the codebase: - For newly added tests, check if similar tests already exist - For new helper functions, verify they aren't already implemented elsewhere @@ -56,10 +57,13 @@ Examine the code for quality, readability, and maintainability issues. **Constraints:** - You MUST check for language-specific best practices as defined in repository guidelines +- You MUST verify code is readable with clear variable/function names and logical structure +- You MUST check that code is maintainable with modular design and loose coupling - You MUST check for code complexity and suggest simplifications - You MUST identify unclear or confusing code patterns - You MUST verify proper error handling - You MUST check for potential performance issues +- You MUST verify design decisions are documented (why certain patterns were chosen, alternatives considered, tradeoffs made) #### 3.3 Testing Review @@ -92,12 +96,15 @@ Create specific, actionable review comments for identified issues. - You MUST focus on the most impactful improvements first - You MUST provide specific suggestions rather than vague feedback - You MUST be concise in your feedback +- You MUST avoid nitpicking on minor style issues (nits) - focus on substantive problems: + - Nits include: comment wording, code organization preferences, bracket/semicolon position, filename conventions + - Substantive issues include: bugs, security vulnerabilities, performance problems, maintainability concerns +- You MUST assume positive intent from the code author - You MUST categorize feedback as: - **Critical**: Must be fixed (security, breaking changes, major bugs) - **Important**: Should be fixed (quality, maintainability, standards) - **Suggestion**: Nice to have (optimizations, style preferences) - You MUST be constructive and educational in your feedback -- You MUST avoid nitpicking on minor style issues if they don't impact functionality - You MUST prioritize feedback that helps the developer learn and improve - You MAY skip this step if you have no feedback to provide @@ -152,12 +159,16 @@ Provide a concise overall summary of the review. ### Code Quality Priorities -1. **Error Handling**: Verify robust error handling and edge cases -2. **Performance**: Identify potential performance bottlenecks -3. **Security**: Check for security vulnerabilities or data exposure -4. **Maintainability**: Ensure code is readable and maintainable -5. **Testing**: Verify comprehensive test coverage and quality -6. **Language Best Practices**: Follow language-specific best practices as defined in repository guidelines +Focus on substantive issues that impact code quality, not stylistic preferences: + +1. **Functionality**: Does the code work as intended? Are edge cases and error conditions handled? +2. **Readability**: Is the code clear with descriptive names and logical structure? +3. **Maintainability**: Is the code modular, loosely coupled, and easy to modify in the future? +4. **Security**: Are there vulnerabilities or data exposure risks? +5. **Performance**: Are there bottlenecks or inefficient algorithms? +6. **Testing**: Is there comprehensive test coverage including edge cases? +7. **Language Best Practices**: Does it follow language-specific best practices as defined in repository guidelines? +8. **Design Documentation**: Are design decisions, alternatives, and tradeoffs documented? ## Best Practices @@ -170,10 +181,12 @@ Provide a concise overall summary of the review. ### Communication - Be respectful and professional +- Assume positive intent from the code author - Acknowledge good practices - Explain the reasoning behind feedback - Provide learning opportunities - Encourage the developer +- Focus on ideas for improving the system, not criticisms of the author ### Quality Gates - Ensure critical issues are marked as blocking