-
Notifications
You must be signed in to change notification settings - Fork 74
pr pre-flight, github action version #5558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Review updated until commit da4516a Description
|
| Relevant files | |||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Enhancement | 7 files
| ||||||||||||||
| Tests |
PR Reviewer Guide
Here are some key observations to aid the review process:
| 🧪 No relevant tests |
| ⚡ Recommended focus areas for review |
Security Risk
|
Greptile OverviewGreptile SummaryThis PR implements automated PR review capabilities using AI CLIs (Gemini and Claude) through GitHub Actions workflows. The implementation includes:
Key Architecture Decisions:
Issues Found:
Confidence Score: 3/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant GHA as GitHub Actions
participant Launcher as pr_preflight_launcher.py
participant Git as git_helpers.py
participant Wrapper as ai_cli_wrapper.py
participant AI as AI CLI (Gemini/Claude)
participant Router as Claude Code Router
participant NIM as NVIDIA NIM API
GHA->>GHA: Trigger on PR event
GHA->>GHA: Setup Node.js & Python
alt Claude Backend
GHA->>Router: Install & start ccr
GHA->>Router: Write config with NIM provider
Router->>NIM: Proxy requests
else Gemini Backend
GHA->>GHA: Install Gemini CLI
end
GHA->>Launcher: python -m tools.pr_preflight_launcher
Launcher->>Launcher: Parse args & environment vars
Launcher->>Git: resolve_base_sha_from_ref(base_ref)
Git-->>Launcher: base_sha
Launcher->>Git: compute_merge_base_sha(base_sha, head_sha)
Git-->>Launcher: merge_base_sha
Launcher->>Launcher: Build review prompt with SHAs
Launcher->>Wrapper: launch_ai_cli(tool, prompt, verdict_marker)
Wrapper->>AI: subprocess.run([tool, args, prompt])
alt Claude
AI->>Router: API request
Router->>NIM: Forward to NIM API
NIM-->>Router: Response
Router-->>AI: Transformed response
else Gemini
AI->>AI: Direct Gemini API call
end
AI->>AI: Execute git/grep/ls commands
AI->>AI: Analyze PR changes
AI-->>Wrapper: Output with VERDICT: PASSED/FAILED
Wrapper->>Wrapper: Parse verdict from output
Wrapper->>Wrapper: Write success_raw_output.txt
Wrapper->>Wrapper: Write review_verdict.txt
Wrapper-->>Launcher: Exit code (0=pass, 3=fail)
Launcher-->>GHA: Exit code
GHA->>GHA: Read output files
GHA->>GHA: Post comment to PR via github-script
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
11 files reviewed, 7 comments
Edit Code Review Agent Settings | Greptile
React with 👍 or 👎 to share your feedback on this new summary format
tools/ai_cli_wrapper.py
Outdated
|
|
||
| def launch_ai_cli( | ||
| prompt: str, | ||
| tool: None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: type annotation is incorrect - tool: None should be tool: str | None
| tool: None, | |
| tool: str | None, |
tools/ai_cli_wrapper.py
Outdated
| return 4 | ||
|
|
||
| except subprocess.TimeoutExpired: | ||
| error_msg = "{tool} command timed out after {timeout_seconds} seconds" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: missing 'f' prefix for f-string - this will output literal string instead of interpolating {tool} and {timeout_seconds}
| error_msg = "{tool} command timed out after {timeout_seconds} seconds" | |
| error_msg = f"{tool} command timed out after {timeout_seconds} seconds" |
tools/ai_cli_wrapper.py
Outdated
| return 1 | ||
|
|
||
| except FileNotFoundError: | ||
| error_msg = "Error: '{tool}' command not found. Is it installed and in PATH?" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: missing 'f' prefix for f-string - this will output literal string instead of interpolating {tool}
| error_msg = "Error: '{tool}' command not found. Is it installed and in PATH?" | |
| error_msg = f"Error: '{tool}' command not found. Is it installed and in PATH?" |
tools/pr_preflight_launcher.py
Outdated
|
|
||
|
|
||
| tool_args = [] | ||
| ok_commands = ["git", "ls", "grep", "ls","stat"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: "ls" appears twice in the list
| ok_commands = ["git", "ls", "grep", "ls","stat"] | |
| ok_commands = ["git", "ls", "grep", "stat"] |
| - name: Checkout code | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: missing fetch-depth: 0 - without full git history, git merge-base in pr_preflight_launcher.py:85 will fail when computing the merge base between branches
| - name: Checkout code | |
| uses: actions/checkout@v4 | |
| - name: Checkout code | |
| uses: actions/checkout@v4 | |
| with: | |
| fetch-depth: 0 |
tools/ai_cli_wrapper.py
Outdated
| ensure_dir(OUTPUT_DIR) | ||
|
|
||
| if tool == None: | ||
| write_to_path(OUTPUT_DIR, "error.txt", f"Error (Exit 1 - no tool specified") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: missing closing parenthesis in error message
| write_to_path(OUTPUT_DIR, "error.txt", f"Error (Exit 1 - no tool specified") | |
| write_to_path(OUTPUT_DIR, "error.txt", f"Error (Exit 1 - no tool specified)") |
| @@ -0,0 +1,6 @@ | |||
| # once inside, can test with | |||
| # python3 /usr/local/bin/pr_preflight_launcher_gemini.py --output-dir=/tmp --base-sha=f8b8551a720cd5c3a9aa8950e5a50fb7d420cbe5 --head-sha=9f9a6b2f91519b3dc02fe7ec7a5f2a3b98398338 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: outdated reference - file is now pr_preflight_launcher.py, not pr_preflight_launcher_gemini.py
| # python3 /usr/local/bin/pr_preflight_launcher_gemini.py --output-dir=/tmp --base-sha=f8b8551a720cd5c3a9aa8950e5a50fb7d420cbe5 --head-sha=9f9a6b2f91519b3dc02fe7ec7a5f2a3b98398338 | |
| # python3 /usr/local/bin/pr_preflight_launcher.py --output-dir=/tmp --base-sha=f8b8551a720cd5c3a9aa8950e5a50fb7d420cbe5 --head-sha=9f9a6b2f91519b3dc02fe7ec7a5f2a3b98398338 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
11 files reviewed, 5 comments
Edit Code Review Agent Settings | Greptile
React with 👍 or 👎 to share your feedback on this new summary format
| - name: Run Claude Code via wrapper | ||
| env: | ||
| ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} | ||
| ANTHROPIC_BASE_URL: http://localhost:3456 | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| PR_SHA: ${{ github.event.pull_request.head.sha }} | ||
| PR_BASE_REF: ${{ github.event.pull_request.base.ref }} | ||
| PR_HEAD_REF: ${{ github.event.pull_request.head.ref }} | ||
| run: | | ||
| python -m tools.pr_preflight_launcher --ai-backend claude --output-dir "${CLAUDE_OUTPUT_DIR}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: missing PR_BASE_SHA environment variable - pr_preflight_launcher.py:52 expects either PR_BASE_SHA or PR_BASE_REF to be set, but only PR_BASE_REF is provided, so the workflow will attempt to resolve it via git rev-parse origin/{base.ref} which requires fetch-depth: 0
| - name: Run Claude Code via wrapper | |
| env: | |
| ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} | |
| ANTHROPIC_BASE_URL: http://localhost:3456 | |
| PR_NUMBER: ${{ github.event.pull_request.number }} | |
| PR_SHA: ${{ github.event.pull_request.head.sha }} | |
| PR_BASE_REF: ${{ github.event.pull_request.base.ref }} | |
| PR_HEAD_REF: ${{ github.event.pull_request.head.ref }} | |
| run: | | |
| python -m tools.pr_preflight_launcher --ai-backend claude --output-dir "${CLAUDE_OUTPUT_DIR}" | |
| env: | |
| ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} | |
| ANTHROPIC_BASE_URL: http://localhost:3456 | |
| PR_NUMBER: ${{ github.event.pull_request.number }} | |
| PR_SHA: ${{ github.event.pull_request.head.sha }} | |
| PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} | |
| PR_BASE_REF: ${{ github.event.pull_request.base.ref }} | |
| PR_HEAD_REF: ${{ github.event.pull_request.head.ref }} |
| - name: Run Gemini PR review | ||
| env: | ||
| #GEMINI_MODEL: gemini-1.5-flash-latest | ||
| GEMINI_OUTPUT_DIR: artifacts/gemini_review/${{ github.event.pull_request.number }}-${{ github.event.pull_request.head.sha }} | ||
| GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }} | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| PR_SHA: ${{ github.event.pull_request.head.sha }} | ||
| PR_BASE_REF: ${{ github.event.pull_request.base.ref }} | ||
| PR_HEAD_REF: ${{ github.event.pull_request.head.ref }} | ||
| run: | | ||
| python -m tools.pr_preflight_launcher --ai-backend gemini --output-dir "${GEMINI_OUTPUT_DIR}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: missing PR_BASE_SHA environment variable - pr_preflight_launcher.py:52 expects either PR_BASE_SHA or PR_BASE_REF to be set, but only PR_BASE_REF is provided, so the workflow will attempt to resolve it via git rev-parse origin/{base.ref} which requires fetch-depth: 0
| - name: Run Gemini PR review | |
| env: | |
| #GEMINI_MODEL: gemini-1.5-flash-latest | |
| GEMINI_OUTPUT_DIR: artifacts/gemini_review/${{ github.event.pull_request.number }}-${{ github.event.pull_request.head.sha }} | |
| GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }} | |
| PR_NUMBER: ${{ github.event.pull_request.number }} | |
| PR_SHA: ${{ github.event.pull_request.head.sha }} | |
| PR_BASE_REF: ${{ github.event.pull_request.base.ref }} | |
| PR_HEAD_REF: ${{ github.event.pull_request.head.ref }} | |
| run: | | |
| python -m tools.pr_preflight_launcher --ai-backend gemini --output-dir "${GEMINI_OUTPUT_DIR}" | |
| - name: Run Gemini PR review | |
| env: | |
| #GEMINI_MODEL: gemini-1.5-flash-latest | |
| GEMINI_OUTPUT_DIR: artifacts/gemini_review/${{ github.event.pull_request.number }}-${{ github.event.pull_request.head.sha }} | |
| GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }} | |
| PR_NUMBER: ${{ github.event.pull_request.number }} | |
| PR_SHA: ${{ github.event.pull_request.head.sha }} | |
| PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} | |
| PR_BASE_REF: ${{ github.event.pull_request.base.ref }} | |
| PR_HEAD_REF: ${{ github.event.pull_request.head.ref }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
11 files reviewed, no comments
Edit Code Review Agent Settings | Greptile
React with 👍 or 👎 to share your feedback on this new summary format
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
11 files reviewed, no comments
Edit Code Review Agent Settings | Greptile
React with 👍 or 👎 to share your feedback on this new summary format
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
11 files reviewed, no comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
11 files reviewed, no comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
11 files reviewed, 3 comments
🤖 Gemini PR ReviewCommit: 4137af0 Comparing This pull request introduces a new CI workflow to automatically review pull requests using AI models (Gemini and Claude). The changes consist of new GitHub Actions workflows, Python scripts to drive the AI CLI tools, and a Docker setup for testing. Code Quality and StyleThe code is well-structured and follows good practices. The Python scripts are modular, with clear responsibilities for handling Git operations, wrapping the AI CLIs, and launching the review process. The code is readable, well-documented with docstrings and comments, and uses type hints. The GitHub Actions workflows are well-organized, using environment variables for configuration and secrets. The shell scripts are simple and serve their purpose. Test CoverageWhile there are no formal unit tests for the Python scripts, the pull request includes a Docker setup for testing the entire workflow in an isolated environment. This allows for integration testing of the scripts and the AI CLI tools. The inclusion of a DocumentationThe code is well-documented. The Python scripts have docstrings explaining the purpose of each module and function, as well as the meaning of different exit codes. The command-line arguments for the launcher script are also well-described. The GitHub Actions workflows contain comments that clarify the purpose of each step. Potential Bugs or Issues
ConclusionThis pull request is a well-engineered solution for automating pull request reviews using AI. The code is of high quality, the testing strategy is practical, and the documentation is clear. The only minor issue is the incomplete implementation of the Claude review posting, but the Gemini workflow is complete and provides a solid foundation. The addition of this automated review system has the potential to significantly improve the development workflow. VERDICT: PASSED Review generated at 2025-11-20T21:31:49.715Z |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
11 files reviewed, no comments
- Add Post Claude review to PR step that posts review comments to PRs - Post comments even when verdict is FAILED to help developers fix issues - Only skip posting if error occurred or no review output - Temporarily disable Gemini workflow with if: false to avoid burning API tokens 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
11 files reviewed, 1 comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
11 files reviewed, 1 comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
11 files reviewed, 1 comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
11 files reviewed, 2 comments
| console.log('Created new PR comment'); | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: file ends with blank lines after closing brace - syntax error leaves JavaScript incomplete
| } |
| "transformers": [] | ||
| } | ||
| EOF | ||
| nohup ccr restart & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: ccr restart requires router already running - use ccr start for initial launch
| nohup ccr restart & | |
| nohup ccr start & |
This reverts commit 9439709.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
11 files reviewed, 2 comments
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: trailing blank lines after closing brace - the github-script block's JavaScript is incomplete and will cause syntax errors when the workflow runs
| } | |
| } |
| "transformers": [] | ||
| } | ||
| EOF | ||
| nohup ccr restart & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: use ccr start for first-time initialization - restart requires router already running and will fail
| nohup ccr restart & | |
| nohup ccr start & |
This update supports Gemini (tested) and Claude (untested)