From 669f1245fd9fdab1c7edd24e0aeb18aa820f9f85 Mon Sep 17 00:00:00 2001 From: Glenn Jocher Date: Thu, 25 Dec 2025 03:04:12 +0100 Subject: [PATCH] Skip large files in PR summaries (#692) Co-authored-by: UltralyticsAssistant --- actions/__init__.py | 2 +- actions/first_interaction.py | 13 ++++- actions/review_pr.py | 41 ++------------- actions/summarize_pr.py | 16 ++++-- actions/utils/__init__.py | 10 ++++ actions/utils/common_utils.py | 94 +++++++++++++++++++++++++++++++++++ actions/utils/openai_utils.py | 18 ++++--- 7 files changed, 143 insertions(+), 51 deletions(-) diff --git a/actions/__init__.py b/actions/__init__.py index 4d5628f..dbd4d86 100644 --- a/actions/__init__.py +++ b/actions/__init__.py @@ -27,4 +27,4 @@ # ├── test_summarize_pr.py # └── ... -__version__ = "0.2.9" +__version__ = "0.2.10" diff --git a/actions/first_interaction.py b/actions/first_interaction.py index a56c69d..739ff1c 100644 --- a/actions/first_interaction.py +++ b/actions/first_interaction.py @@ -7,7 +7,15 @@ import time from . import review_pr from .summarize_pr import SUMMARY_MARKER -from .utils import ACTIONS_CREDIT, Action, filter_labels, get_pr_open_response, get_response, remove_html_comments +from .utils import ( + ACTIONS_CREDIT, + Action, + filter_labels, + format_skipped_files_dropdown, + get_pr_open_response, + get_response, + remove_html_comments, +) BLOCK_USER = os.getenv("BLOCK_USER", "false").lower() == "true" AUTO_PR_REVIEW = os.getenv("REVIEW", "true").lower() == "true" @@ -192,7 +200,8 @@ def main(*args, **kwargs): if summary := response.get("summary"): print("Updating PR description with summary...") - event.update_pr_description(number, f"{SUMMARY_MARKER}\n\n{ACTIONS_CREDIT}\n\n{summary}") + skipped_dropdown = format_skipped_files_dropdown(response.get("skipped_files", [])) + event.update_pr_description(number, f"{SUMMARY_MARKER}\n\n{ACTIONS_CREDIT}\n\n{summary}{skipped_dropdown}") else: summary = body diff --git a/actions/review_pr.py b/actions/review_pr.py index 8f6d570..ba7a87b 100644 --- a/actions/review_pr.py +++ b/actions/review_pr.py @@ -11,50 +11,21 @@ from .utils import ( GITHUB_API_URL, MAX_PROMPT_CHARS, Action, + format_skipped_files_dropdown, get_response, remove_html_comments, sanitize_ai_text, + should_skip_file, ) REVIEW_MARKER = "## 🔍 PR Review" ERROR_MARKER = "⚠️ Review generation encountered an error" EMOJI_MAP = {"CRITICAL": "❗", "HIGH": "⚠️", "MEDIUM": "💡", "LOW": "📝", "SUGGESTION": "💭"} -SKIP_PATTERN_STRINGS = [ - r"\.lock$", # Lock files - r"-lock\.(json|yaml|yml)$", - r"\.min\.(js|css)$", # Minified - r"\.bundle\.(js|css)$", - r"(^|/)dist/", # Generated/vendored directories - r"(^|/)build/", - r"(^|/)vendor/", - r"(^|/)node_modules/", - r"(^|/)coverage/", # Coverage reports - r"\.pb\.py$", # Proto generated - r"_pb2\.py$", - r"_pb2_grpc\.py$", - r"^package-lock\.json$", # Package locks - r"^yarn\.lock$", - r"^poetry\.lock$", - r"^Pipfile\.lock$", - r"\.(svg|png|jpe?g|gif|ico|webp)$", # Images - r"\.(woff2?|ttf|eot|otf)$", # Fonts - r"\.(mp4|webm|mov|avi|mkv)$", # Videos - r"\.(pdf|doc|docx|xls|xlsx)$", # Documents - r"\.generated\.", # Common generated file pattern -] -SKIP_PATTERNS = tuple(re.compile(pattern) for pattern in SKIP_PATTERN_STRINGS) MAX_CONTEXT_FILE_CHARS = 5000 MAX_REVIEW_COMMENTS = 8 SEVERITY_RANK = {"CRITICAL": 0, "HIGH": 1, "MEDIUM": 2, "LOW": 3, "SUGGESTION": 4, None: 5} -def should_skip_file(path: str) -> bool: - """Return True if file path matches a generated/minified skip pattern.""" - normalized = Path(path).as_posix().lstrip("./") - filename = normalized.rsplit("/", 1)[-1] - return any(pattern.search(candidate) for pattern in SKIP_PATTERNS for candidate in (normalized, filename)) - - def parse_diff_files(diff_text: str) -> tuple[dict, str]: """Parse diff and return file mapping with line numbers AND augmented diff with explicit line numbers. @@ -416,13 +387,7 @@ def post_review_summary(event: Action, review_data: dict, review_number: int) -> if review_data.get("diff_truncated"): body += "\n⚠️ **Large PR**: Review focused on critical issues. Some details may not be covered.\n" - if skipped := review_data.get("skipped_files"): - count = len(skipped) - skip_str = f"📋 Skipped {count} file{'s' if count != 1 else ''} (lock files, minified, images, etc.)" - file_list = "\n".join(f"- `{f}`" for f in sorted(skipped[:100])) - if count > 100: - file_list += f"\n{count - 100} more..." - body += f"\n
{skip_str}\n\n{file_list}\n
\n" + body += format_skipped_files_dropdown(review_data.get("skipped_files", [])) # Build inline comments for the review review_comments = [] diff --git a/actions/summarize_pr.py b/actions/summarize_pr.py index cb36200..06f2c4d 100644 --- a/actions/summarize_pr.py +++ b/actions/summarize_pr.py @@ -2,7 +2,14 @@ from __future__ import annotations -from .utils import ACTIONS_CREDIT, GITHUB_API_URL, Action, get_pr_summary_prompt, get_response +from .utils import ( + ACTIONS_CREDIT, + GITHUB_API_URL, + Action, + format_skipped_files_dropdown, + get_pr_summary_prompt, + get_response, +) SUMMARY_MARKER = "## 🛠️ PR Summary" @@ -59,7 +66,7 @@ def generate_issue_comment(pr_url, pr_summary, pr_credit, pr_title=""): def generate_pr_summary(repository, diff_text): """Generates a concise, professional summary of a PR using OpenAI's API.""" - prompt, is_large = get_pr_summary_prompt(repository, diff_text) + prompt, is_large, skipped_files = get_pr_summary_prompt(repository, diff_text) messages = [ { @@ -72,7 +79,10 @@ def generate_pr_summary(repository, diff_text): if is_large: reply = "**WARNING ⚠️** this PR is very large, summary may not cover all changes.\n\n" + reply - return f"{SUMMARY_MARKER}\n\n{ACTIONS_CREDIT}\n\n{reply}" + # Add skipped files dropdown if any files were filtered + skipped_dropdown = format_skipped_files_dropdown(skipped_files) + + return f"{SUMMARY_MARKER}\n\n{ACTIONS_CREDIT}\n\n{reply}{skipped_dropdown}" def label_fixed_issues(event, pr_summary): diff --git a/actions/utils/__init__.py b/actions/utils/__init__.py index df1e869..cab363e 100644 --- a/actions/utils/__init__.py +++ b/actions/utils/__init__.py @@ -5,9 +5,14 @@ from .common_utils import ( REDIRECT_END_IGNORE_LIST, REDIRECT_START_IGNORE_LIST, REQUESTS_HEADERS, + SKIP_PATTERNS, URL_IGNORE_LIST, allow_redirect, + filter_diff_text, + format_skipped_files_dropdown, + format_skipped_files_note, remove_html_comments, + should_skip_file, ) from .github_utils import GITHUB_API_URL, GITHUB_GRAPHQL_URL, Action, ultralytics_actions_info from .openai_utils import ( @@ -29,17 +34,22 @@ __all__ = ( "REDIRECT_END_IGNORE_LIST", "REDIRECT_START_IGNORE_LIST", "REQUESTS_HEADERS", + "SKIP_PATTERNS", "URL_IGNORE_LIST", "Action", "allow_redirect", "check_pubdev_version", "check_pypi_version", + "filter_diff_text", "filter_labels", + "format_skipped_files_dropdown", + "format_skipped_files_note", "get_pr_open_response", "get_pr_summary_guidelines", "get_pr_summary_prompt", "get_response", "remove_html_comments", "sanitize_ai_text", + "should_skip_file", "ultralytics_actions_info", ) diff --git a/actions/utils/common_utils.py b/actions/utils/common_utils.py index 5613f31..c9c3742 100644 --- a/actions/utils/common_utils.py +++ b/actions/utils/common_utils.py @@ -6,10 +6,38 @@ import os import re import time from concurrent.futures import ThreadPoolExecutor +from pathlib import Path from urllib import parse import requests +# Patterns for files that should be skipped in PR summaries and reviews (lock files, generated, minified, etc.) +SKIP_PATTERN_STRINGS = [ + r"\.lock$", # Lock files + r"-lock\.(json|yaml|yml)$", + r"\.min\.(js|css)$", # Minified + r"\.bundle\.(js|css)$", + r"(^|/)dist/", # Generated/vendored directories + r"(^|/)build/", + r"(^|/)vendor/", + r"(^|/)node_modules/", + r"(^|/)coverage/", # Coverage reports + r"\.pb\.py$", # Proto generated + r"_pb2\.py$", + r"_pb2_grpc\.py$", + r"^package-lock\.json$", # Package locks + r"^yarn\.lock$", + r"^poetry\.lock$", + r"^Pipfile\.lock$", + r"^uv\.lock$", + r"\.(svg|png|jpe?g|gif|ico|webp)$", # Images + r"\.(woff2?|ttf|eot|otf)$", # Fonts + r"\.(mp4|webm|mov|avi|mkv)$", # Videos + r"\.(pdf|doc|docx|xls|xlsx)$", # Documents + r"\.generated\.", # Common generated file pattern +] +SKIP_PATTERNS = tuple(re.compile(pattern) for pattern in SKIP_PATTERN_STRINGS) + REQUESTS_HEADERS = { "User-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/132.0.0.0 Safari/537.36", "Accept": "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7", @@ -137,6 +165,72 @@ def remove_html_comments(body: str) -> str: return re.sub(r"", "", body, flags=re.DOTALL).strip() if body else "" +def should_skip_file(path: str) -> bool: + """Return True if file path matches a generated/minified skip pattern (lock files, images, etc.).""" + normalized = Path(path).as_posix().removeprefix("./") + filename = normalized.rsplit("/", 1)[-1] + return any(pattern.search(candidate) for pattern in SKIP_PATTERNS for candidate in (normalized, filename)) + + +def filter_diff_text(diff_text: str) -> tuple[str, list[str]]: + """Filter diff text to exclude lock files and other generated files. + + Returns: + tuple: (filtered_diff_text, list of skipped file paths) + """ + if not diff_text or diff_text.startswith("ERROR"): + return diff_text, [] + + filtered_lines = [] + skipped_files = set() + current_file = None + skip_current = False + + for line in diff_text.split("\n"): + if line.startswith("diff --git"): + # Extract file path from diff header, handling quoted paths (spaces/renames) + # Formats: "diff --git a/path b/path" or 'diff --git "a/path" "b/path"' + if match := re.search(r' "?b/(.+?)"?$', line): + current_file = match.group(1).rstrip('"') + else: + current_file = None + skip_current = current_file and should_skip_file(current_file) + + if skip_current and current_file: + skipped_files.add(current_file) + else: + filtered_lines.append(line) + elif skip_current: + continue + else: + filtered_lines.append(line) + + return "\n".join(filtered_lines), sorted(skipped_files) + + +def format_skipped_files_dropdown(skipped_files: list[str], max_files: int = 100) -> str: + """Format skipped files as a collapsible HTML details dropdown for GitHub markdown.""" + if not skipped_files: + return "" + count = len(skipped_files) + summary = f"📋 Skipped {count} file{'s' if count != 1 else ''} (lock files, generated, images, etc.)" + file_list = "\n".join(f"- `{f}`" for f in sorted(skipped_files)[:max_files]) + if count > max_files: + file_list += f"\n- ... and {count - max_files} more" + return f"\n
{summary}\n\n{file_list}\n
\n" + + +def format_skipped_files_note(skipped_files: list[str], max_files: int = 10) -> str: + """Format skipped files as a brief inline note for AI prompts.""" + if not skipped_files: + return "" + note = "\n\nNote: The following auto-generated/lock files were also modified but diff details omitted: " + note += ", ".join(f"`{f}`" for f in skipped_files[:max_files]) + if len(skipped_files) > max_files: + note += f" and {len(skipped_files) - max_files} more" + return note + + def clean_url(url): """Remove extra characters from URL strings.""" url = str(url).strip('"').strip("'").rstrip(".,:;!?`\\").replace(".git@main", "").replace("git+", "") diff --git a/actions/utils/openai_utils.py b/actions/utils/openai_utils.py index ed3b63f..bcab0e0 100644 --- a/actions/utils/openai_utils.py +++ b/actions/utils/openai_utils.py @@ -9,7 +9,7 @@ import time import requests -from actions.utils.common_utils import check_links_in_string +from actions.utils.common_utils import check_links_in_string, filter_diff_text, format_skipped_files_note OPENAI_API_KEY = os.getenv("OPENAI_API_KEY") OPENAI_MODEL = os.getenv("OPENAI_MODEL", "gpt-5.2-2025-12-11") @@ -99,10 +99,12 @@ def get_pr_summary_guidelines() -> str: - (bullet points explaining benefits and potential impact to users)""" -def get_pr_summary_prompt(repository: str, diff_text: str) -> tuple[str, bool]: - """Returns the complete PR summary generation prompt with diff (used by PR update/merge).""" - prompt = f"{get_pr_summary_guidelines()}\n\nRepository: '{repository}'\n\nHere's the PR diff:\n\n{diff_text[:MAX_PROMPT_CHARS]}" - return prompt, len(diff_text) > MAX_PROMPT_CHARS +def get_pr_summary_prompt(repository: str, diff_text: str) -> tuple[str, bool, list[str]]: + """Returns the complete PR summary generation prompt with filtered diff (used by PR update/merge).""" + filtered_diff, skipped_files = filter_diff_text(diff_text) + prompt = f"{get_pr_summary_guidelines()}\n\nRepository: '{repository}'\n\nHere's the PR diff:\n\n{filtered_diff[:MAX_PROMPT_CHARS]}" + prompt += format_skipped_files_note(skipped_files) + return prompt, len(filtered_diff) > MAX_PROMPT_CHARS, skipped_files def get_pr_first_comment_template(repository: str, username: str) -> str: @@ -219,7 +221,8 @@ def get_response( def get_pr_open_response(repository: str, diff_text: str, title: str, username: str, available_labels: dict) -> dict: """Generates unified PR response with summary, labels, and first comment in a single API call.""" - is_large = len(diff_text) > MAX_PROMPT_CHARS + filtered_diff, skipped_files = filter_diff_text(diff_text) + is_large = len(filtered_diff) > MAX_PROMPT_CHARS filtered_labels = filter_labels(available_labels, is_pr=True) labels_str = "\n".join(f"- {name}: {description}" for name, description in filtered_labels.items()) @@ -227,7 +230,7 @@ def get_pr_open_response(repository: str, diff_text: str, title: str, username: prompt = f"""You are processing a new GitHub PR by @{username} for the {repository} repository. Generate 3 outputs in a single JSON response for the PR titled '{title}' with the following diff: -{diff_text[:MAX_PROMPT_CHARS]} +{filtered_diff[:MAX_PROMPT_CHARS]}{format_skipped_files_note(skipped_files)} --- FIRST JSON OUTPUT (PR SUMMARY) --- @@ -274,6 +277,7 @@ Example comment template (adapt as needed, keep all links): result["summary"] = ( "**WARNING ⚠️** this PR is very large, summary may not cover all changes.\n\n" + result["summary"] ) + result["skipped_files"] = skipped_files return result