ultralytics/actions
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Ultralytics Actions PR Review feature (#550)

Browse Source 
Signed-off-by: Glenn Jocher <[email protected]>
Co-authored-by: UltralyticsAssistant <[email protected]>
This commit is contained in:
Glenn Jocher
2025-10-19 02:24:59 +02:00
committed by GitHub
co-authored by UltralyticsAssistant
parent bdc36fb885
commit 77e9b7f377
6 changed files with 315 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files
action.yml
+20 -4
View File
@@ -46,6 +46,10 @@ inputs:
description: "Run PR Summary"
required: false
default: "false"
review:
description: "Run PR Review"
required: false
default: "false"
header:
description: "File header"
required: false
@@ -86,11 +90,11 @@ runs:
version: "0.8.22"
- name: Install Dependencies
# Note tomli required for codespell with pyproject.toml
# sudo env "PATH=$PATH" uv pip install --system --break-system-packages git+https://github.com/ultralytics/actions@main codespell tomli
env:
INPUTS_SPELLING: ${{ inputs.spelling }}
run: |
packages="ultralytics-actions"
sudo env "PATH=$PATH" uv pip install --system --break-system-packages git+https://github.com/ultralytics/actions@review-pr-1 codespell tomli
packages=""
if [ "$INPUTS_SPELLING" = "true" ]; then
packages="$packages codespell tomli"
fi
@@ -114,7 +118,7 @@ runs:
# PR Summary -------------------------------------------------------------------------------------------------------
- name: PR Summary
if: (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && inputs.summary == 'true' && github.event.action != 'synchronize'
if: (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && inputs.summary == 'true' && github.event.action != 'synchronize' && github.event.action != 'review_requested'
env:
GITHUB_TOKEN: ${{ inputs.token }}
BRAVE_API_KEY: ${{ inputs.brave_api_key }}
@@ -226,7 +230,7 @@ runs:
shell: bash
continue-on-error: true
# Autolabel Issues and PRs (run before commit changes in case commit fails) ----------------------------------------
# Autolabel Issues and PRs (labels and first interaction) ---------------------------------------------------------
- name: Autolabel Issues and PRs
if: inputs.labels == 'true' && (github.event.action == 'opened' || github.event.action == 'created')
env:
@@ -241,6 +245,18 @@ runs:
shell: bash
continue-on-error: true
# PR Review --------------------------------------------------------------------------------------------------------
- name: PR Review
if: (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && inputs.review == 'true' && (github.event.action == 'opened' || github.event.action == 'review_requested')
env:
GITHUB_TOKEN: ${{ inputs.token }}
OPENAI_API_KEY: ${{ inputs.openai_api_key }}
OPENAI_MODEL: ${{ inputs.openai_model }}
run: |
ultralytics-actions-review-pr
shell: bash
continue-on-error: true
# Commit Changes ---------------------------------------------------------------------------------------------------
- name: Commit and Push Changes
if: (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && github.event.action != 'closed'
actions/__init__.py
+2 -1
View File
@@ -13,6 +13,7 @@
# │ │ ├── openai_utils.py
# │ │ └── common_utils.py
# │ ├── first_interaction.py
# │ ├── review_pr.py
# │ ├── summarize_pr.py
# │ ├── summarize_release.py
# │ └── update_markdown_code_blocks.py
@@ -22,4 +23,4 @@
# ├── test_summarize_pr.py
# └── ...
__version__ = "0.0.100"
__version__ = "0.1.0"
actions/review_pr.py
+247
View File
@@ -0,0 +1,247 @@
# Ultralytics 🚀 AGPL-3.0 License - https://ultralytics.com/license
from __future__ import annotations
import json
import re
from .utils import GITHUB_API_URL, Action, get_completion
REVIEW_MARKER = "🔍 PR Review"
EMOJI_MAP = {"CRITICAL": "❗", "HIGH": "⚠️", "MEDIUM": "💡", "LOW": "📝", "SUGGESTION": "💭"}
def parse_diff_files(diff_text: str) -> dict:
"""Parse diff to extract file paths, valid line numbers, and line content for comments."""
files, current_file, current_line = {}, None, 0
for line in diff_text.split("\n"):
if line.startswith("diff --git"):
match = re.search(r" b/(.+)$", line)
current_file = match.group(1) if match else None
current_line = 0
if current_file:
files[current_file] = {}
elif line.startswith("@@") and current_file:
match = re.search(r"@@.*\+(\d+)(?:,\d+)?", line)
current_line = int(match.group(1)) if match else 0
elif current_file and current_line > 0:
if line.startswith("+") and not line.startswith("+++"):
files[current_file][current_line] = line[1:]
current_line += 1
elif not line.startswith("-"):
current_line += 1
return files
def generate_pr_review(repository: str, diff_text: str, pr_title: str, pr_description: str) -> dict:
"""Generate comprehensive PR review with line-specific comments and overall assessment."""
if not diff_text or "**ERROR" in diff_text:
return {"comments": [], "summary": f"Unable to review: {diff_text if '**ERROR' in diff_text else 'diff empty'}"}
diff_files = parse_diff_files(diff_text)
if not diff_files:
return {"comments": [], "summary": "No files with changes detected in diff"}
file_list = list(diff_files.keys())
limit = round(128000 * 3.3 * 0.4)
diff_truncated = len(diff_text) > limit
lines_changed = sum(len(lines) for lines in diff_files.values())
valid_lines_text = "\n".join(
f" {file}: {sorted(list(lines.keys())[:20])}{' ...' if len(lines) > 20 else ''}"
for file, lines in list(diff_files.items())[:10]
) + ("\n ..." if len(diff_files) > 10 else "")
priority_guidance = (
"Prioritize the most critical/high-impact issues only"
if lines_changed >= 100
else "Prioritize commenting on different files/sections"
)
content = (
"You are an expert code reviewer for Ultralytics. Provide detailed inline comments on specific code changes.\n\n"
"Focus on: Code quality, style, best practices, bugs, edge cases, error handling, performance, security, documentation, test coverage\n\n"
"FORMATTING: Use backticks for code, file names, branch names, function names, variable names, packages\n\n"
"CRITICAL RULES:\n"
f"1. Generate inline comments with recommended changes for clear bugs/security/syntax issues (up to 10)\n"
"2. Each comment MUST reference a UNIQUE line number(s)\n"
"3. If a section has multiple issues, combine ALL issues into ONE comment for that section\n"
"4. Never create separate comments for the same line number(s)\n"
f"5. {priority_guidance}\n\n"
"CODE SUGGESTIONS:\n"
"- ONLY provide 'suggestion' field when you have high certainty the code is problematic AND sufficient context for a confident fix\n"
"- If uncertain about the correct fix, omit 'suggestion' field and explain the concern in 'message' only\n"
"- Suggestions must be ready-to-merge code with NO comments, placeholders, or explanations\n"
"- When providing suggestions, ensure they are complete, correct, and maintain existing indentation\n"
"- It's better to flag an issue without a suggestion than provide a wrong or uncertain fix\n\n"
"Return JSON: "
'{"comments": [{"file": "exact/path", "line": N, "severity": "HIGH", "message": "...", "suggestion": "..."}], "summary": "..."}\n\n'
"Rules:\n"
"- Only comment on NEW lines (starting with + in diff)\n"
"- Use exact file paths from diff (no ./ prefix)\n"
"- Line numbers must match NEW file line numbers from @@ hunks\n"
"- When '- old' then '+ new', new line keeps SAME line number\n"
"- Severity: CRITICAL, HIGH, MEDIUM, LOW, SUGGESTION\n"
f"- Files changed: {len(file_list)} ({', '.join(file_list[:10])}{'...' if len(file_list) > 10 else ''})\n"
f"- Total changed lines: {lines_changed}\n"
f"- Diff {'truncated' if diff_truncated else 'complete'}: {len(diff_text[:limit])} chars{f' of {len(diff_text)}' if diff_truncated else ''}\n\n"
f"VALID LINE NUMBERS (use ONLY these):\n{valid_lines_text}"
)
messages = [
{"role": "system", "content": content},
{
"role": "user",
"content": f"Review PR '{repository}':\nTitle: {pr_title}\nDescription: {pr_description[:500]}\n\nDiff:\n{diff_text[:limit]}",
},
]
try:
response = get_completion(messages, reasoning_effort="medium")
print("\n" + "=" * 80 + f"\nFULL AI RESPONSE:\n{response}\n" + "=" * 80 + "\n")
json_str = re.search(r"```(?:json)?\s*(\{.*?\})\s*```", response, re.DOTALL)
review_data = json.loads(json_str.group(1) if json_str else response)
print(f"AI generated {len(review_data.get('comments', []))} comments")
# Validate, filter, and deduplicate comments
unique_comments = {}
for c in review_data.get("comments", []):
file_path, line_num = c.get("file"), c.get("line", 0)
if file_path in diff_files and line_num in diff_files[file_path]:
key = f"{file_path}:{line_num}"
if key not in unique_comments:
unique_comments[key] = c
else:
print(f"⚠️ AI duplicate for {key}: {c.get('severity')} - {c.get('message')[:60]}...")
else:
print(f"Filtered out {file_path}:{line_num} (available: {list(diff_files.get(file_path, {}))[:10]}...)")
review_data.update(
{"comments": list(unique_comments.values()), "diff_files": diff_files, "diff_truncated": diff_truncated}
)
print(f"Valid comments after filtering: {len(review_data['comments'])}")
return review_data
except json.JSONDecodeError as e:
print(f"JSON parsing failed: {e}\nAttempted: {json_str[:500] if 'json_str' in locals() else response[:500]}...")
return {"comments": [], "summary": "Review generation encountered a JSON parsing error"}
except Exception as e:
print(f"Review generation failed: {e}")
import traceback
traceback.print_exc()
return {"comments": [], "summary": "Review generation encountered an error"}
def dismiss_previous_reviews(event: Action) -> int:
"""Dismiss previous bot reviews and delete inline comments, returns count for numbering."""
if not (pr_number := event.pr.get("number")) or not (bot_username := event.get_username()):
return 1
review_count = 0
reviews_url = f"{GITHUB_API_URL}/repos/{event.repository}/pulls/{pr_number}/reviews"
if (response := event.get(reviews_url)).status_code == 200:
for review in response.json():
if review.get("user", {}).get("login") == bot_username and REVIEW_MARKER in (review.get("body") or ""):
review_count += 1
if review.get("state") in ["APPROVED", "CHANGES_REQUESTED"] and (review_id := review.get("id")):
event.put(f"{reviews_url}/{review_id}/dismissals", json={"message": "Superseded by new review"})
# Delete previous inline comments
comments_url = f"{GITHUB_API_URL}/repos/{event.repository}/pulls/{pr_number}/comments"
if (response := event.get(comments_url)).status_code == 200:
for comment in response.json():
if comment.get("user", {}).get("login") == bot_username and (comment_id := comment.get("id")):
event.delete(
f"{GITHUB_API_URL}/repos/{event.repository}/pulls/comments/{comment_id}",
expected_status=[200, 204, 404],
)
return review_count + 1
def post_review_comments(event: Action, review_data: dict) -> None:
"""Post inline review comments on specific lines of the PR."""
if not (pr_number := event.pr.get("number")) or not (commit_sha := event.pr.get("head", {}).get("sha")):
return
url = f"{GITHUB_API_URL}/repos/{event.repository}/pulls/{pr_number}/comments"
diff_files = review_data.get("diff_files", {})
for comment in review_data.get("comments", [])[:50]:
if not (file_path := comment.get("file")) or not (line := comment.get("line", 0)):
continue
severity = comment.get("severity", "SUGGESTION")
body = f"{EMOJI_MAP.get(severity, '💭')} **{severity}**: {comment.get('message', '')}"
if suggestion := comment.get("suggestion"):
original_line = diff_files.get(file_path, {}).get(line, "")
indent = len(original_line) - len(original_line.lstrip())
indented = "\n".join(" " * indent + l if l.strip() else l for l in suggestion.split("\n"))
body += f"\n\n**Suggested change:**\n```suggestion\n{indented}\n```"
event.post(url, json={"body": body, "commit_id": commit_sha, "path": file_path, "line": line, "side": "RIGHT"})
def post_review_summary(event: Action, review_data: dict, review_number: int) -> None:
"""Post overall review summary as a PR review."""
if not (pr_number := event.pr.get("number")) or not (commit_sha := event.pr.get("head", {}).get("sha")):
return
review_title = f"{REVIEW_MARKER} {review_number}" if review_number > 1 else REVIEW_MARKER
comments = review_data.get("comments", [])
max_severity = max((c.get("severity") for c in comments), default="SUGGESTION") if comments else None
event_type = "APPROVE" if not comments or max_severity in ["LOW", "SUGGESTION"] else "REQUEST_CHANGES"
body = (
f"## {review_title}\n\n"
"<sub>Made with ❤️ by [Ultralytics Actions](https://github.com/ultralytics/actions)</sub>\n\n"
f"{review_data.get('summary', 'Review completed')}\n\n"
)
if comments:
shown = min(len(comments), 50)
body += f"💬 Posted {shown} inline comment{'s' if shown != 1 else ''}{' (50 shown, more available)' if len(comments) > 50 else ''}\n"
if review_data.get("diff_truncated"):
body += "\n⚠️ **Large PR**: Review focused on critical issues. Some details may not be covered.\n"
event.post(
f"{GITHUB_API_URL}/repos/{event.repository}/pulls/{pr_number}/reviews",
json={"commit_id": commit_sha, "body": body, "event": event_type},
)
def main(*args, **kwargs):
"""Main entry point for PR review action."""
event = Action(*args, **kwargs)
# Handle review requests
if event.event_name == "pull_request" and event.event_data.get("action") == "review_requested":
if event.event_data.get("requested_reviewer", {}).get("login") != event.get_username():
return
print(f"Review requested from {event.get_username()}")
if not event.pr or event.pr.get("state") != "open":
print(f"Skipping: PR state is {event.pr.get('state') if event.pr else 'None'}")
return
print(f"Starting PR review for #{event.pr['number']}")
review_number = dismiss_previous_reviews(event)
diff = event.get_pr_diff()
review = generate_pr_review(event.repository, diff, event.pr.get("title", ""), event.pr.get("body", ""))
post_review_summary(event, review, review_number)
print(f"Posting {len(review.get('comments', []))} inline comments")
post_review_comments(event, review)
print("PR review completed")
if __name__ == "__main__":
main()
actions/utils/github_utils.py
+44 -56
View File
@@ -17,17 +17,19 @@ GITHUB_GRAPHQL_URL = "https://api.github.com/graphql"
class Action:
"""Handles GitHub Actions API interactions and event processing."""
def __init__(
self,
token: str = None,
event_name: str = None,
event_data: dict = None,
verbose: bool = True,
):
def __init__(self, token: str = None, event_name: str = None, event_data: dict = None, verbose: bool = True):
"""Initializes a GitHub Actions API handler with token and event data for processing events."""
self.token = token or os.getenv("GITHUB_TOKEN")
self.event_name = event_name or os.getenv("GITHUB_EVENT_NAME")
self.event_data = event_data or self._load_event_data(os.getenv("GITHUB_EVENT_PATH"))
self.pr = self.event_data.get("pull_request", {})
self.repository = self.event_data.get("repository", {}).get("full_name")
self.headers = {"Authorization": f"Bearer {self.token}", "Accept": "application/vnd.github+json"}
self.headers_diff = {"Authorization": f"Bearer {self.token}", "Accept": "application/vnd.github.v3.diff"}
self.verbose = verbose
self.eyes_reaction_id = None
self._pr_diff_cache = None
self._username_cache = None
self._default_status = {
"get": [200],
"post": [200, 201],
@@ -36,34 +38,22 @@ class Action:
"delete": [200, 204],
}
self.pr = self.event_data.get("pull_request", {})
self.repository = self.event_data.get("repository", {}).get("full_name")
self.headers = {"Authorization": f"Bearer {self.token}", "Accept": "application/vnd.github+json"}
self.headers_diff = {"Authorization": f"Bearer {self.token}", "Accept": "application/vnd.github.v3.diff"}
self.eyes_reaction_id = None
self.verbose = verbose
def _request(self, method: str, url: str, headers=None, expected_status=None, hard=False, **kwargs):
"""Unified request handler with error checking."""
headers = headers or self.headers
expected_status = expected_status or self._default_status[method.lower()]
response = getattr(requests, method)(url, headers=headers, **kwargs)
status = response.status_code
success = status in expected_status
response = getattr(requests, method)(url, headers=headers or self.headers, **kwargs)
expected = expected_status or self._default_status[method]
success = response.status_code in expected
if self.verbose:
print(f"{'✓' if success else '✗'} {method.upper()} {url} → {status}")
print(f"{'✓' if success else '✗'} {method.upper()} {url} → {response.status_code}")
if not success:
try:
error_detail = response.json()
print(f" ❌ Error: {error_detail.get('message', 'Unknown error')}")
except Exception as e:
print(f" ❌ Error: {response.text[:100]}... {e}")
print(f" ❌ Error: {response.json().get('message', 'Unknown error')}")
except Exception:
print(f" ❌ Error: {response.text[:200]}")
if not success and hard:
response.raise_for_status()
return response
def get(self, url, **kwargs):
@@ -89,45 +79,47 @@ class Action:
@staticmethod
def _load_event_data(event_path: str) -> dict:
"""Load GitHub event data from path if it exists."""
if event_path and Path(event_path).exists():
return json.loads(Path(event_path).read_text())
return {}
return json.loads(Path(event_path).read_text()) if event_path and Path(event_path).exists() else {}
def is_repo_private(self) -> bool:
"""Checks if the repository is public using event data or GitHub API if needed."""
return self.event_data.get("repository", {}).get("private")
"""Checks if the repository is public using event data."""
return self.event_data.get("repository", {}).get("private", False)
def get_username(self) -> str | None:
"""Gets username associated with the GitHub token."""
"""Gets username associated with the GitHub token with caching."""
if self._username_cache:
return self._username_cache
response = self.post(GITHUB_GRAPHQL_URL, json={"query": "query { viewer { login } }"})
if response.status_code == 200:
try:
return response.json()["data"]["viewer"]["login"]
self._username_cache = response.json()["data"]["viewer"]["login"]
except KeyError as e:
print(f"Error parsing authenticated user response: {e}")
return None
return self._username_cache
def is_org_member(self, username: str) -> bool:
"""Checks if a user is a member of the organization using the GitHub API."""
org_name = self.repository.split("/")[0]
response = self.get(f"{GITHUB_API_URL}/orgs/{org_name}/members/{username}")
return response.status_code == 204 # 204 means the user is a member
"""Checks if a user is a member of the organization."""
return self.get(f"{GITHUB_API_URL}/orgs/{self.repository.split('/')[0]}/members/{username}").status_code == 204
def get_pr_diff(self) -> str:
"""Retrieves the diff content for a specified pull request."""
"""Retrieves the diff content for a specified pull request with caching."""
if self._pr_diff_cache:
return self._pr_diff_cache
url = f"{GITHUB_API_URL}/repos/{self.repository}/pulls/{self.pr.get('number')}"
response = self.get(url, headers=self.headers_diff)
if response.status_code == 200:
return response.text
self._pr_diff_cache = response.text
elif response.status_code == 406:
return "**ERROR: DIFF TOO LARGE - PR exceeds GitHub's 20,000 line limit, unable to retrieve diff."
self._pr_diff_cache = "ERROR: PR diff exceeds GitHub's 20,000 line limit, unable to retrieve diff."
else:
return "**ERROR: UNABLE TO RETRIEVE DIFF."
self._pr_diff_cache = "ERROR: UNABLE TO RETRIEVE DIFF."
return self._pr_diff_cache
def get_repo_data(self, endpoint: str) -> dict:
"""Fetches repository data from a specified endpoint."""
response = self.get(f"{GITHUB_API_URL}/repos/{self.repository}/{endpoint}")
return response.json()
return self.get(f"{GITHUB_API_URL}/repos/{self.repository}/{endpoint}").json()
def toggle_eyes_reaction(self, add: bool = True) -> None:
"""Adds or removes eyes emoji reaction."""
@@ -139,8 +131,8 @@ class Action:
id = self.event_data.get("issue", {}).get("number")
if not id:
return
url = f"{GITHUB_API_URL}/repos/{self.repository}/issues/{id}/reactions"
url = f"{GITHUB_API_URL}/repos/{self.repository}/issues/{id}/reactions"
if add:
response = self.post(url, json={"content": "eyes"})
if response.status_code == 201:
@@ -151,8 +143,7 @@ class Action:
def graphql_request(self, query: str, variables: dict = None) -> dict:
"""Executes a GraphQL query against the GitHub API."""
r = self.post(GITHUB_GRAPHQL_URL, json={"query": query, "variables": variables})
result = r.json()
result = self.post(GITHUB_GRAPHQL_URL, json={"query": query, "variables": variables}).json()
if "data" not in result or result.get("errors"):
print(result.get("errors"))
return result
@@ -166,11 +157,11 @@ class Action:
"github.repository.private": self.is_repo_private(),
"github.event.pull_request.number": self.pr.get("number"),
"github.event.pull_request.head.repo.full_name": self.pr.get("head", {}).get("repo", {}).get("full_name"),
"github.actor": os.environ.get("GITHUB_ACTOR"),
"github.actor": os.getenv("GITHUB_ACTOR"),
"github.event.pull_request.head.ref": self.pr.get("head", {}).get("ref"),
"github.ref": os.environ.get("GITHUB_REF"),
"github.head_ref": os.environ.get("GITHUB_HEAD_REF"),
"github.base_ref": os.environ.get("GITHUB_BASE_REF"),
"github.ref": os.getenv("GITHUB_REF"),
"github.head_ref": os.getenv("GITHUB_HEAD_REF"),
"github.base_ref": os.getenv("GITHUB_BASE_REF"),
"github.base_sha": self.pr.get("base", {}).get("sha"),
}
@@ -181,12 +172,9 @@ class Action:
"github.event.discussion.number": discussion.get("number"),
}
max_key_length = max(len(key) for key in info)
width = max(len(k) for k in info) + 5
header = f"Ultralytics Actions {__version__} Information " + "-" * 40
print(header)
for key, value in info.items():
print(f"{key:<{max_key_length + 5}}{value}")
print("-" * len(header))
print(f"{header}\n" + "\n".join(f"{k:<{width}}{v}" for k, v in info.items()) + f"\n{'-' * len(header)}")
def ultralytics_actions_info():
actions/utils/openai_utils.py
+1 -5
View File
@@ -58,11 +58,7 @@ def get_completion(
r = requests.post(url, json=data, headers=headers)
if r.status_code != 200:
try:
error_data = r.json()
print(f"❌ OpenAI API Error ({r.status_code}): {error_data}")
except Exception:
print(f"❌ OpenAI API Error ({r.status_code}): {r.text[:500]}")
print(f"❌ OpenAI error {r.status_code}:\n{r.text}\n")
r.raise_for_status()
response_data = r.json()
pyproject.toml
+1
View File
@@ -85,6 +85,7 @@ dev = [
[project.scripts]
ultralytics-actions-first-interaction = "actions.first_interaction:main"
ultralytics-actions-review-pr = "actions.review_pr:main"
ultralytics-actions-summarize-pr = "actions.summarize_pr:main"
ultralytics-actions-summarize-release = "actions.summarize_release:main"
ultralytics-actions-update-markdown-code-blocks = "actions.update_markdown_code_blocks:main"