docs: define agent pull request and bookkeeping workflow - #1
Conversation
Signed-off-by: jameskinyua590@gmail.com <20414083+JayKay24@users.noreply.github.com>
…views using Gemini Signed-off-by: jameskinyua590@gmail.com <20414083+JayKay24@users.noreply.github.com>
|
Here is a professional code review of your Pull Request. 🤖 AI PR Review SummaryThis PR introduces an automated code review workflow triggered by GitHub Actions. It contains:
💡 Key Feedback & Recommendations1. 🛑 Critical Bug: Invalid Gemini Model NameFile: The script references model = genai.GenerativeModel("gemini-3.5-flash")Google Gemini does not currently have a
2. 🧼 Pythonic Style: Modularity & Code OrganizationFile: The entire script is currently encapsulated inside a single monolithic
Below is an optimized refactoring structure: import os
import sys
from typing import List
from github import Github, PullRequest
import google.generativeai as genai
import pathspec
def get_ignore_spec() -> pathspec.PathSpec:
"""Loads .gitignore patterns and appends custom file exclusion wildcards."""
ignore_patterns = []
if os.path.exists(".gitignore"):
try:
with open(".gitignore", "r", encoding="utf-8") as f:
ignore_patterns.extend(f.readlines())
except Exception as e:
print(f"Warning: Failed to parse .gitignore: {e}", file=sys.stderr)
ignore_patterns.extend([
"*.lock", "*.png", "*.jpg", "*.jpeg", "*.zip", "*.pdf"
])
return pathspec.PathSpec.from_lines("gitwildmatch", ignore_patterns)
def build_diff_content(pr: PullRequest.PullRequest, ignore_spec: pathspec.PathSpec) -> str:
"""Retrieves and filters PR file diffs."""
diff_content: List[str] = []
for file in pr.get_files():
if ignore_spec.match_file(file.filename):
print(f"Skipping {file.filename} (ignored)")
continue
file_header = f"=== File: {file.filename} ===\n"
if file.patch:
diff_content.append(f"{file_header}{file.patch}\n")
else:
diff_content.append(f"{file_header}[File modified, but no patch details available]\n")
return "\n".join(diff_content)3. 🛡️ Robustness: Defensive Handling of Large PR DiffsFile: Extremely large PRs (e.g., changes to autogenerated files, DB schemas, or massive data assets) can result in a payload that exceeds the context token limits of the LLM or consumes excessive APIs tokens.
MAX_DIFF_CHARACTERS = 150000 # Safe threshold
full_diff = build_diff_content(pr, ignore_spec)
if len(full_diff) > MAX_DIFF_CHARACTERS:
full_diff = (
full_diff[:MAX_DIFF_CHARACTERS]
+ "\n\n... [TRUNCATED DUE TO EXTREME SIZE] ..."
)
print("Warning: PR diff is extremely large. Truncating content to stay within safe token limits.")4. 🚀 GitHub PR UX: Post as PR Review instead of Issue CommentFile: The script currently uses
✅ VerdictRequest Changes While the implementation is clean and the feature is extremely useful, the invalid Gemini model name ( |
…ncation Signed-off-by: jameskinyua590@gmail.com <20414083+JayKay24@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Here is a professional code review of the proposed Pull Request.
🤖 AI PR Review Summary
This PR establishes an automated AI-powered Code Reviewer workflow utilizing GitHub Actions, Python, PyGithub, and the Gemini API.
The key modifications are:
- GitHub Action Workflow (
.github/workflows/ai-review.yml): Automates the trigger on pull request lifecycle events (opened,synchronize,reopened) and provisions environment secrets. - Workflow Guidelines (
agents.md): Instructs AI agents on the expected PR lifecycle and bookkeeping. - Core Script (
scripts/ai_pr_reviewer.py): A Python utility that pulls PR file changes, respects.gitignorerules, curates a unified diff (with a safety threshold truncation), and prompts the Gemini API to generate structured markdown feedback.
💡 Key Feedback & Recommendations
1. 🛑 Critical Bug: Invalid Gemini Model Name
In scripts/ai_pr_reviewer.py, the code references "gemini-3.5-flash". As of currently available releases, there is no model named gemini-3.5-flash. Using this invalid model name will cause the Google GenAI SDK to throw a google.api_core.exceptions.NotFound error, crashing the workflow.
- Before:
model = genai.GenerativeModel("gemini-3.5-flash")
- After (Recommended):
Usegemini-1.5-flashfor fast, cost-effective reviews, orgemini-1.5-pro/gemini-2.5-flashfor deeper reasoning.model = genai.GenerativeModel("gemini-1.5-flash")
2. ⚠️ Security & Fork PR Limitations
The workflow is triggered via the standard pull_request event.
-
The issue: In public repositories, secrets like
GEMINI_API_KEYare not passed to workflows running on PRs initiated from forks to prevent secret leakage. This will cause the workflow to fail silently or crash on external contributions. -
Recommendation: Handle missing API keys gracefully so the runner exits with a neutral status (
sys.exit(0)) instead of hard-failing, or note in your documentation that fork reviews require alternative configuration (likepull_request_target, though this requires extreme care due to security risks). -
Improvement in
scripts/ai_pr_reviewer.py:if not gemini_api_key: print("Warning: GEMINI_API_KEY is missing. Skipping AI Review (expected for external forks).") sys.exit(0)
3. 📦 Best Practice: Pin Python Dependencies
In .github/workflows/ai-review.yml, the workflow installs dependencies without pinning versions. Upstream updates to google-generativeai or PyGithub could introduce breaking changes that disrupt your CI pipeline.
- Before:
- name: Install Dependencies run: | python -m pip install --upgrade pip pip install google-generativeai PyGithub pathspec
- After:
(Note: Adjust the pinned versions to match your tested local environment.)
- name: Install Dependencies run: | python -m pip install --upgrade pip pip install google-generativeai==0.8.3 PyGithub==2.5.0 pathspec==0.12.1
4. 🐍 Python Code Quality & Robustness
The script is generally well-structured, but adding minor error boundaries when parsing the PR_NUMBER conversion will prevent unexpected tracebacks if the environment variables are malformed.
- Before:
pr_number = int(pr_number_str)
- After:
try: pr_number = int(pr_number_str) except ValueError: print(f"Error: PR_NUMBER '{pr_number_str}' is not a valid integer.", file=sys.stderr) sys.exit(1)
✅ Verdict
Request Changes
The workflow is highly valuable and the Python integration is exceptionally clean. However, the use of an invalid model identifier (gemini-3.5-flash) will cause the action to fail instantly on execution. Once the model name is updated to a valid one (such as gemini-1.5-flash) and dependencies are safely pinned, this PR is ready to merge!
…PI keys and invalid PR numbers Signed-off-by: jameskinyua590@gmail.com <20414083+JayKay24@users.noreply.github.com>
There was a problem hiding this comment.
🤖 AI PR Review Summary
This Pull Request introduces an automated AI Code Reviewer pipeline. It adds a GitHub Actions workflow (.github/workflows/ai-review.yml) that triggers on PR events, updates the agent coordination playbook (agents.md), and implements a Python script (scripts/ai_pr_reviewer.py) that fetches PR diffs using PyGithub, processes them with the Google Gemini API, and posts a structured code review comment back to the PR.
💡 Key Feedback & Recommendations
1. 🚨 Critical Bug: Invalid Gemini Model Name
In scripts/ai_pr_reviewer.py, the model identifier is specified as "gemini-3.5-flash". As of the current Google Generative AI release, there is no model named gemini-3.5-flash. Attempting to initialize this will result in an API error (typically a 404 or InvalidArgument exception).
- Recommendation: Update the model string to a valid production-ready model, such as
"gemini-1.5-flash"(highly performant and cost-effective) or"gemini-1.5-pro"(highly capable for coding reasoning).
Before:
def generate_review(gemini_api_key: str, diff: str) -> str:
"""Sends the PR diff to the Gemini API and returns the markdown review."""
genai.configure(api_key=gemini_api_key)
model = genai.GenerativeModel("gemini-3.5-flash")After:
def generate_review(gemini_api_key: str, diff: str) -> str:
"""Sends the PR diff to the Gemini API and returns the markdown review."""
genai.configure(api_key=gemini_api_key)
# Using the production-grade Gemini 1.5 Flash model
model = genai.GenerativeModel("gemini-1.5-flash")2. Robustness: Parsing .gitignore Lines Cleanly
In get_ignore_spec(), you are extending ignore_patterns using raw lines from .gitignore using f.readlines(). This preserves trailing newlines (\n) and potential whitespace, which can occasionally lead to unexpected matching behavior in pathspec.
- Recommendation: Cleanse the lines by stripping whitespace and filtering out empty lines or pure comment lines.
Before:
if os.path.exists(".gitignore"):
try:
with open(".gitignore", "r", encoding="utf-8") as f:
ignore_patterns.extend(f.readlines())
except Exception as e:
print(f"Warning: Failed to parse .gitignore: {e}", file=sys.stderr)After:
if os.path.exists(".gitignore"):
try:
with open(".gitignore", "r", encoding="utf-8") as f:
lines = [
line.strip()
for line in f
if line.strip() and not line.strip().startswith("#")
]
ignore_patterns.extend(lines)
except Exception as e:
print(f"Warning: Failed to parse .gitignore: {e}", file=sys.stderr)3. Security & Workflow Scope: External Fork PRs
The workflow is triggered via the standard pull_request event.
- By default, GITHUB_TOKEN has read-only permissions for PRs originating from external forks, and repository secrets (like
GEMINI_API_KEY) are not shared with forks. - Your script handles the missing
GEMINI_API_KEYgracefully (exits with status code0), which is excellent design and prevents workflow failures on external contributions. - Note: If you ever want this AI reviewer to run on external forks, you would need to switch the workflow trigger to
pull_request_target. However, this should only be done after assessing the security implications of exposing your secrets to external PR code.
✅ Verdict
Request Changes
The workflow and Python script are clean, well-structured, and follow excellent architectural patterns (including token truncation and defensive checks). However, the incorrect Gemini model name (gemini-3.5-flash) will cause runtime execution failures in production. Once the model name is updated to a valid identifier (such as gemini-1.5-flash), this PR will be ready to merge.
…ing comment lines Signed-off-by: jameskinyua590@gmail.com <20414083+JayKay24@users.noreply.github.com>
There was a problem hiding this comment.
🤖 AI PR Review Summary
This Pull Request introduces an automated AI Code Reviewer workflow for GitHub Pull Requests. It includes:
- GitHub Actions Workflow (
.github/workflows/ai-review.yml): Automates running the reviewer on PR creation, synchronization, or reopening. - Workflow Guidelines (
agents.md): Adds explicit standards and expectations for PR and bookkeeping tasks. - Core Script (
scripts/ai_pr_reviewer.py): A Python program that fetches PR diffs usingPyGithub, applies gitignore exclusions, sends the code changes to Google's Gemini API, and posts the generated markdown review back as a PR comment.
💡 Key Feedback & Recommendations
1. 🛑 Critical Bug: Invalid Gemini Model Identifier
In scripts/ai_pr_reviewer.py, the model identifier is configured as "gemini-3.5-flash":
model = genai.GenerativeModel("gemini-3.5-flash")Google Gemini does not currently have a gemini-3.5-flash model. Attempting to initialize this will throw an API error (such as a 404 or model not found exception), causing the GitHub Action to fail.
- Recommendation: Update this to a valid, stable Gemini model. For rapid code reviews,
"gemini-1.5-flash"is highly recommended due to its low latency, high throughput, and massive context window (1 million tokens). Alternatively, use"gemini-2.5-flash"if utilizing the latest generation features.
👉 Before:
model = genai.GenerativeModel("gemini-3.5-flash")👉 After:
model = genai.GenerativeModel("gemini-1.5-flash")2. 🛡️ Fault Tolerance: Wrap Gemini API & GitHub API Calls in Try-Except Blocks
Currently, calls to generate_review and post_review inside main() are not wrapped in safety blocks. If the Gemini API experiences temporary downtime, rate-limiting, or content-blocking, the entire pipeline will crash with an unhandled exception, causing the GHA runner to report a failure status.
- Recommendation: Catch API exceptions gracefully to avoid failing the overall workflow unnecessarily, or log errors constructively.
👉 Before:
# Generate review
print("Generating review with Gemini...")
review_body = generate_review(gemini_api_key, diff)
# Post review
print("Posting review back to GitHub...")
post_review(pr, review_body)👉 After:
# Generate review
print("Generating review with Gemini...")
try:
review_body = generate_review(gemini_api_key, diff)
except Exception as e:
print(f"Error generating review via Gemini API: {e}", file=sys.stderr)
sys.exit(1)
# Post review
print("Posting review back to GitHub...")
try:
post_review(pr, review_body)
except Exception as e:
print(f"Error posting review comment to GitHub: {e}", file=sys.stderr)
sys.exit(1)3. 🔍 Optimization: Limit Max File Size in Diff Collection
While you have a guardrail for total diff characters (MAX_DIFF_CHARACTERS = 150000), a single massively updated file (e.g., autogenerated code, large database exports, or raw JSON data) could consume the entire budget before other code files are examined.
- Recommendation: Skip adding patches for files exceeding a specific single-file threshold (e.g., 20,000 characters) or exclude specific structured data extensions (such as
.json,.csv,.tsv) dynamically if they do not require structural code analysis.
👉 Code Quality Improvement Idea:
# Skip overly large individual patches to ensure fairness across files
if file.patch and len(file.patch) > 30000:
diff_content.append(f"{file_header}[File patch omitted: Exceeds size limits]\n")
continue✅ Verdict
Request Changes
The addition of an automated AI PR reviewer is a fantastic engineering productivity enhancement. However, because "gemini-3.5-flash" is an invalid model name, the script will crash immediately upon execution. This must be resolved before merging to ensure a working integration.
…Gemini model support Signed-off-by: jameskinyua590@gmail.com <20414083+JayKay24@users.noreply.github.com>
There was a problem hiding this comment.
🤖 AI PR Review Summary
This PR establishes an automated AI Code Reviewer workflow using GitHub Actions and Google's Gemini API.
.github/workflows/ai-review.yml: Configures a GitHub Actions workflow triggered on pull requests that installs necessary packages and runs the review script.agents.md: Updates documentation to specify guidelines for local development, commit workflows, and PR description standards.scripts/ai_pr_reviewer.py: A python script that collects PR file changes, filters out ignored files (via.gitignoreand hardcoded wildcards), formats a prompt with the diff, calls the Gemini API, and posts the resulting code review as a pull request comment.
💡 Key Feedback & Recommendations
1. Gracefully Handle Fork PRs (Permissions Issue)
When a Pull Request is submitted from a forked repository, GitHub restricts the GITHUB_TOKEN to Read-Only access for security reasons, even if permissions: pull-requests: write is explicitly requested in the workflow.
Currently, if an external contributor opens a PR, pr.create_review will fail with an HTTP 403 Forbidden exception, causing the GitHub Action runner to fail (exit code 1). We should catch this exception and exit gracefully (exit code 0) so external contributions don't trigger red failing builds.
2. Leverage Gemini's Native system_instruction
Rather than bundling the reviewer persona, PySpark guidelines, and formatting rules directly inside the user prompt, modern versions of google-generativeai support a dedicated system_instruction parameter during model initialization. Separating the system instruction from the prompt improves model adherence and reduces prompt injection risks.
👉 Before:
def generate_review(gemini_api_key: str, model_name: str, diff: str) -> str:
genai.configure(api_key=gemini_api_key)
model = genai.GenerativeModel(model_name)
prompt = f"""
You are an expert Data Engineer...
PR Diff:
{diff}
"""
response = model.generate_content(prompt)👉 After:
def generate_review(gemini_api_key: str, model_name: str, diff: str) -> str:
genai.configure(api_key=gemini_api_key)
system_instruction = (
"You are an expert Data Engineer and Python Code Reviewer.\n"
"Your task is to conduct a professional, constructive code review.\n\n"
"Specific areas to analyze:\n"
"1. PySpark & Data Engineering Best Practices (unpartitioned writes, redundant caching, .collect() issues).\n"
"2. Python Code Quality (Ruff/PEP8 standards, naming, docstrings).\n"
"3. Bugs & Edge Cases (logical bugs, unhandled exceptions).\n\n"
"Format your review in Markdown with the following sections:\n"
"- 🤖 AI PR Review Summary\n"
"- 💡 Key Feedback & Recommendations (with before/after code blocks)\n"
"- ✅ Verdict (Approve, Comment, Request Changes)"
)
model = genai.GenerativeModel(
model_name,
system_instruction=system_instruction
)
prompt = f"Please review the following PR Diff:\n\n{diff}"
response = model.generate_content(prompt)
return response.text3. Optimize Diff Generation Loop for Large PRs
In build_diff_content, the script continues to request file details from GitHub even after the internal accumulated diff has exceeded MAX_DIFF_CHARACTERS (150k). For large PRs, this results in unnecessary network requests. Adding an early-exit optimization saves API bandwidth and workflow runtime.
👉 Before:
for file in pr.get_files():
# ... processing file ...
# ... (appends and checks at the very end after the loop completes)👉 After:
current_size = 0
for file in pr.get_files():
if current_size >= MAX_DIFF_CHARACTERS:
diff_content.append("\n\n... [TRUNCATED: MAX CHARACTER LIMIT REACHED] ...")
break
if ignore_spec.match_file(file.filename):
continue
file_header = f"=== File: {file.filename} ===\n"
patch_str = ""
if file.patch:
if len(file.patch) > 30000:
patch_str = f"{file_header}[File patch omitted: Exceeds size limit]\n"
else:
patch_str = f"{file_header}{file.patch}\n"
else:
patch_str = f"{file_header}[File modified, but no patch details available]\n"
diff_content.append(patch_str)
current_size += len(patch_str)✅ Verdict
Comment: The code is structured professionally, contains strong defensive sizing policies, and handles missing Gemini tokens safely. Merging these recommended stability/optimization improvements will ensure the action behaves seamlessly across internal and fork PRs.
…to utilize system instructions with graceful error handling for external forks Signed-off-by: jameskinyua590@gmail.com <20414083+JayKay24@users.noreply.github.com>
There was a problem hiding this comment.
🤖 AI PR Review Summary
This Pull Request introduces an automated, AI-driven Code Reviewer workflow utilizing the Gemini API and PyGithub.
What's done well:
- Strong Defense-in-Depth: The script safely handles missing environment variables (e.g.,
GEMINI_API_KEYin external forks) and handles rate/token limits by truncating large diffs (MAX_DIFF_CHARACTERS = 150000) and ignoring overly large single-file patches. - Great Contextual Tuning: The Gemini system instructions are tailored specifically to your project's engineering domain (PySpark best practices, PEP8/Ruff, logical bugs).
- Proper Resource Handling: Correctly handles
.gitignorefiles usingpathspec.
💡 Key Feedback & Recommendations
1. Redundant Exception Handling in main() and post_review()
Issue:
In scripts/ai_pr_reviewer.py, the post_review function catches any Exception during the review post and exits the script with sys.exit(0) to gracefully handle fork permission limits.
However, in main(), post_review is called inside another try...except block that exits with sys.exit(1). Because post_review never raises the exception outward, the outer except block in main() is dead code and will never run.
Recommendation:
Simplify the exception handling flow. Have post_review handle its internal error and return a boolean status indicator, or allow it to raise exceptions to be handled cleanly at the caller (main) level.
Before:
# scripts/ai_pr_reviewer.py
def post_review(pr: PullRequest, review_body: str) -> None:
"""Submits the review as an official Pull Request Review on GitHub. Handles fork permissions gracefully."""
try:
pr.create_review(body=review_body, event="COMMENT")
except Exception as e:
print(f"Warning: Failed to post PR review comment: {e}", file=sys.stderr)
print("This is expected for Pull Requests from external forks where GITHUB_TOKEN has read-only access.")
print("Exiting gracefully with code 0.")
sys.exit(0)
# ... inside main()
# Post review
print("Posting review back to GitHub...")
try:
post_review(pr, review_body)
except Exception as e:
print(f"Error posting review comment to GitHub: {e}", file=sys.stderr)
sys.exit(1)After:
# scripts/ai_pr_reviewer.py
def post_review(pr: PullRequest, review_body: str) -> bool:
"""Submits the review as an official Pull Request Review on GitHub.
Returns True on success, False if it fails (e.g. read-only permissions on forks).
"""
try:
pr.create_review(body=review_body, event="COMMENT")
return True
except Exception as e:
print(f"Warning: Failed to post PR review comment: {e}", file=sys.stderr)
print("This is expected for Pull Requests from external forks where GITHUB_TOKEN has read-only access.")
return False
# ... inside main()
# Post review
print("Posting review back to GitHub...")
success = post_review(pr, review_body)
if not success:
sys.exit(0) # Exit gracefully as designed for forks2. Missing Return Type Hint on main()
Issue:
While almost all functions in scripts/ai_pr_reviewer.py have complete PEP 484 type annotations, the entrypoint main() is missing its return type hint.
Recommendation:
Add -> None to main() to satisfy strict type-checkers (like mypy).
def main() -> None:
# Load Environment Variables✅ Verdict
Approve with suggestions.
The implementation is robust, clean, and well-thought-out. Implementing the refactored exception handling will clean up dead code and make the execution pipeline easier to maintain!
1 participant
Summary
This Pull Request introduces a structured Pull Request and Bookkeeping Workflow for AI agents collaborating on this repository. It documents the exact steps an agent must follow when implementing features and coordinating local reviews and PR creation with the user.
Key Changes
4. Pull Request & Bookkeeping Workflowdefining local verification, user reviews, local git commit procedures, and PR description formatting guidelines.Verification
agents.mdfile format conforms to Markdown guidelines.