From 59069122fd4eb6dec97fc46eb8434d669f9ebe49 Mon Sep 17 00:00:00 2001 From: Glenn Jocher Date: Tue, 21 Oct 2025 01:51:02 +0200 Subject: [PATCH] Automatic PR reviews (#562) Signed-off-by: Glenn Jocher Co-authored-by: UltralyticsAssistant --- action.yml | 10 ++++---- actions/__init__.py | 2 +- actions/first_interaction.py | 17 +++++++++++++- actions/review_pr.py | 44 +++++++++++------------------------ actions/utils/github_utils.py | 14 +++++++++++ 5 files changed, 50 insertions(+), 37 deletions(-) diff --git a/action.yml b/action.yml index d9b6ed5..735ce52 100644 --- a/action.yml +++ b/action.yml @@ -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 }} diff --git a/actions/__init__.py b/actions/__init__.py index d924218..0afbefb 100644 --- a/actions/__init__.py +++ b/actions/__init__.py @@ -23,4 +23,4 @@ # ├── test_summarize_pr.py # └── ... -__version__ = "0.1.2" +__version__ = "0.1.3" diff --git a/actions/first_interaction.py b/actions/first_interaction.py index 0942017..f514e1d 100644 --- a/actions/first_interaction.py +++ b/actions/first_interaction.py @@ -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\nMade with ❤️ by [Ultralytics Actions](https://github.com/ultralytics/actions)\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) diff --git a/actions/review_pr.py b/actions/review_pr.py index 612950c..03273c1 100644 --- a/actions/review_pr.py +++ b/actions/review_pr.py @@ -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) diff --git a/actions/utils/github_utils.py b/actions/utils/github_utils.py index 7bb3e54..cc36258 100644 --- a/actions/utils/github_utils.py +++ b/actions/utils/github_utils.py @@ -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: