Automatic PR reviews (#562)
Signed-off-by: Glenn Jocher <[email protected]> Co-authored-by: UltralyticsAssistant <[email protected]>
This commit is contained in:
Glenn Jocher
co-authored by
UltralyticsAssistant
UltralyticsAssistant
parent
4f504fcbd9
commit
59069122fd
+5
-5
@@ -49,7 +49,7 @@ inputs:
|
||||
review:
|
||||
description: "Run PR Review"
|
||||
required: false
|
||||
default: "false"
|
||||
default: "true"
|
||||
header:
|
||||
description: "File header"
|
||||
required: false
|
||||
@@ -242,6 +242,7 @@ runs:
|
||||
BRAVE_API_KEY: ${{ inputs.brave_api_key }}
|
||||
OPENAI_API_KEY: ${{ inputs.openai_api_key }}
|
||||
OPENAI_MODEL: ${{ inputs.openai_model }}
|
||||
REVIEW: ${{ inputs.review }}
|
||||
run: |
|
||||
ultralytics-actions-first-interaction
|
||||
shell: bash
|
||||
@@ -250,10 +251,9 @@ runs:
|
||||
# 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' && github.event.requested_reviewer.login == inputs.github_username)
|
||||
)
|
||||
(github.event_name == 'pull_request' || github.event_name == 'pull_request_target') &&
|
||||
github.event.action == 'review_requested' &&
|
||||
github.event.requested_reviewer.login == inputs.github_username
|
||||
env:
|
||||
GITHUB_TOKEN: ${{ inputs.token }}
|
||||
OPENAI_API_KEY: ${{ inputs.openai_api_key }}
|
||||
|
||||
+1
-1
@@ -23,4 +23,4 @@
|
||||
# ├── test_summarize_pr.py
|
||||
# └── ...
|
||||
|
||||
__version__ = "0.1.2"
|
||||
__version__ = "0.1.3"
|
||||
|
||||
@@ -5,12 +5,14 @@ from __future__ import annotations
|
||||
import os
|
||||
import time
|
||||
|
||||
from . import review_pr
|
||||
from .utils import Action, filter_labels, get_completion, get_pr_open_response, remove_html_comments
|
||||
|
||||
SUMMARY_START = (
|
||||
"## 🛠️ PR Summary\n\n<sub>Made with ❤️ by [Ultralytics Actions](https://github.com/ultralytics/actions)<sub>\n\n"
|
||||
)
|
||||
BLOCK_USER = os.getenv("BLOCK_USER", "false").lower() == "true"
|
||||
AUTO_PR_REVIEW = os.getenv("REVIEW", "true").lower() == "true"
|
||||
|
||||
|
||||
def apply_and_check_labels(event, number, node_id, issue_type, username, labels, label_descriptions):
|
||||
@@ -185,13 +187,18 @@ def main(*args, **kwargs):
|
||||
|
||||
# Use unified PR open response for new PRs (summary + labels + first comment in 1 API call)
|
||||
if issue_type == "pull request" and action == "opened":
|
||||
if event.should_skip_pr_author():
|
||||
return
|
||||
|
||||
print("Processing PR open with unified API call...")
|
||||
diff = event.get_pr_diff()
|
||||
response = get_pr_open_response(event.repository, diff, title, body, label_descriptions)
|
||||
|
||||
if summary := response.get("summary"):
|
||||
print("Updating PR description with summary...")
|
||||
event.update_pr_description(number, SUMMARY_START + summary)
|
||||
event.update_pr_description(number, SUMMARY_START + summary + "\n\n" + body)
|
||||
else:
|
||||
summary = body
|
||||
|
||||
if relevant_labels := response.get("labels", []):
|
||||
apply_and_check_labels(event, number, node_id, issue_type, username, relevant_labels, label_descriptions)
|
||||
@@ -200,6 +207,14 @@ def main(*args, **kwargs):
|
||||
print("Adding first interaction comment...")
|
||||
time.sleep(1) # sleep to ensure label added first
|
||||
event.add_comment(number, node_id, first_comment, issue_type)
|
||||
|
||||
# Automatic PR review after first interaction
|
||||
if AUTO_PR_REVIEW:
|
||||
print("Starting automatic PR review...")
|
||||
review_number = review_pr.dismiss_previous_reviews(event)
|
||||
review_data = review_pr.generate_pr_review(event.repository, diff, title, summary)
|
||||
review_pr.post_review_summary(event, review_data, review_number)
|
||||
print("PR review completed")
|
||||
return
|
||||
|
||||
# Handle issues and discussions (NOT PRs)
|
||||
|
||||
+14
-30
@@ -81,24 +81,18 @@ def generate_pr_review(repository: str, diff_text: str, pr_title: str, pr_descri
|
||||
|
||||
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, performance, edge cases, error handling, security, documentation, test coverage\n\n"
|
||||
"FORMATTING: Use backticks for all summary and suggestion code, files, branches, functions, variables, packages, e.g. `x=3`\n\n"
|
||||
"Focus on: Bugs, security, performance, best practices, edge cases, error handling\n\n"
|
||||
"FORMATTING: Use backticks for code: `x=3`, `file.py`, `function()`\n\n"
|
||||
"CRITICAL RULES:\n"
|
||||
"1. Quality over quantity: Zero comments is fine for clean code - only flag truly important issues\n"
|
||||
"2. Be selective and prioritize: Focus on the most impactful issues that would genuinely improve the code\n"
|
||||
"3. CRITICAL: Do not post separate comments on adjacent/nearby lines (within 10 lines). Combine all related issues into ONE comment\n"
|
||||
"4. When combining issues from multiple lines, use 'start_line' (first line) and 'line' (last line) to highlight the entire range\n"
|
||||
"5. Each comment must reference separate areas - no overlapping line ranges\n"
|
||||
"6. Prioritize: CRITICAL bugs/security > HIGH impact issues > code quality\n"
|
||||
"7. Keep comments concise, friendly, and easy to understand - avoid jargon when possible\n"
|
||||
"8. DO not comment on routine changes: adding imports, adding dependencies, updating version numbers, standard refactoring\n"
|
||||
"9. Trust the developer - only flag issues with clear evidence of problems, not hypothetical concerns\n\n"
|
||||
"SUMMARY GUIDELINES:\n"
|
||||
"- Keep summary brief, clear, and actionable - avoid overly detailed explanations\n"
|
||||
"- Highlight only the most important findings\n"
|
||||
"- Do NOT include file names or line numbers in the summary - inline comments already show exact locations\n"
|
||||
"- Focus on what needs to be fixed, not where\n\n"
|
||||
"CODE SUGGESTIONS:\n"
|
||||
"1. Quality over quantity - zero comments is fine for clean code, only flag truly important issues\n"
|
||||
"2. Combine issues that are directly related to the same problem\n"
|
||||
"3. Use 'start_line' and 'line' to highlight multi-line ranges when issues span multiple lines\n"
|
||||
"4. Prioritize: CRITICAL bugs/security > HIGH impact > code quality improvements\n"
|
||||
"5. Keep comments concise and friendly - avoid jargon\n"
|
||||
"6. Skip routine changes: imports, version updates, standard refactoring\n\n"
|
||||
"SUMMARY:\n"
|
||||
"- Brief and actionable - what needs fixing, not where (locations shown in inline comments)\n\n"
|
||||
"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"
|
||||
@@ -110,10 +104,7 @@ def generate_pr_review(repository: str, diff_text: str, pr_title: str, pr_descri
|
||||
"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"
|
||||
"- Only NEW lines (+ in diff), exact paths (no ./), correct line numbers from @@ hunks\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"- Lines changed: {lines_changed}\n"
|
||||
@@ -301,15 +292,8 @@ def main(*args, **kwargs):
|
||||
return
|
||||
|
||||
# Skip self-authored or bot PRs unless manually review_requested
|
||||
if event.event_data.get("action") != "review_requested":
|
||||
if pr_author := event.pr.get("user", {}).get("login"):
|
||||
if pr_author == event.get_username():
|
||||
print(f"Skipping: PR author ({pr_author}) is the same as reviewer")
|
||||
return
|
||||
# Check both user.type and [bot] suffix for robust bot detection
|
||||
if event.pr.get("user", {}).get("type") == "Bot" or pr_author.endswith("[bot]"):
|
||||
print(f"Skipping: PR author ({pr_author}) is a bot")
|
||||
return
|
||||
if event.event_data.get("action") != "review_requested" and event.should_skip_pr_author():
|
||||
return
|
||||
|
||||
print(f"Starting PR review for #{event.pr['number']}")
|
||||
review_number = dismiss_previous_reviews(event)
|
||||
|
||||
@@ -188,6 +188,20 @@ class Action:
|
||||
"""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 should_skip_pr_author(self) -> bool:
|
||||
"""Checks if PR should be skipped based on author (self-authored or bot PRs)."""
|
||||
if not self.pr:
|
||||
return False
|
||||
if pr_author := self.pr.get("user", {}).get("login"):
|
||||
if pr_author == self.get_username():
|
||||
print(f"Skipping: PR author ({pr_author}) is the same as bot")
|
||||
return True
|
||||
# Check both user.type and [bot] suffix for robust bot detection
|
||||
if self.pr.get("user", {}).get("type") == "Bot" or pr_author.endswith("[bot]"):
|
||||
print(f"Skipping: PR author ({pr_author}) is a bot")
|
||||
return True
|
||||
return False
|
||||
|
||||
def is_fork_pr(self) -> bool:
|
||||
"""Checks if PR is from a fork (different repo than base)."""
|
||||
if not self.pr:
|
||||
|
||||
Reference in New Issue
Block a user