Restore contributor-facing PR responses (#864)
This commit is contained in:
Glenn Jocher
+1
-1
@@ -28,4 +28,4 @@
|
||||
# ├── test_summarize_pr.py
|
||||
# └── ...
|
||||
|
||||
__version__ = "0.3.2"
|
||||
__version__ = "0.3.3"
|
||||
|
||||
@@ -24,6 +24,21 @@ AUTO_PR_SUMMARY = os.getenv("SUMMARY", "true").lower() == "true"
|
||||
AUTO_PR_REVIEW = os.getenv("REVIEW", "true").lower() == "true"
|
||||
|
||||
|
||||
def get_pr_first_comment_template(repository: str, username: str) -> str:
|
||||
"""Return the required contributor guidance for new pull requests."""
|
||||
return f"""👋 Hello @{username}, thank you for submitting a `{repository}` 🚀 PR! This automated message confirms your contribution was received, and an Ultralytics engineer will assist with the review. To ensure a seamless integration of your work, please review the following checklist:
|
||||
|
||||
- ✅ **Define a Purpose**: Clearly explain the purpose of your fix or feature in your PR description, and link to any [relevant issues](https://github.com/{repository}/issues). Ensure your commit messages are clear, concise, and adhere to the project's conventions.
|
||||
- ✅ **Synchronize with Source**: Confirm your PR is synchronized with the `{repository}` `main` branch. If it's behind, update it by clicking the 'Update branch' button or by running `git pull` and `git merge main` locally.
|
||||
- ✅ **Ensure CI Checks Pass**: Verify all Ultralytics [Continuous Integration (CI)](https://docs.ultralytics.com/help/CI) checks are passing. If any checks fail, please address the issues.
|
||||
- ✅ **Update Documentation**: Update the relevant [documentation](https://docs.ultralytics.com/) for any new or modified features.
|
||||
- ✅ **Add Tests**: If applicable, include or update tests to cover your changes, and confirm that all tests are passing.
|
||||
- ✅ **Sign the CLA**: Please ensure you have signed our [Contributor License Agreement](https://docs.ultralytics.com/help/CLA) if this is your first Ultralytics PR by writing "I have read the CLA Document and I sign the CLA" in a new message.
|
||||
- ✅ **Minimize Changes**: Limit your changes to the **minimum** necessary for your bug fix or feature addition. _"It is not daily increase but daily decrease, hack away the unessential. The closer to the source, the less wastage there is."_ — Bruce Lee
|
||||
|
||||
For more guidance, please refer to our [Contributing Guide](https://docs.ultralytics.com/help/contributing). Don't hesitate to leave a comment if you have any questions. Thank you for contributing to Ultralytics! 🚀"""
|
||||
|
||||
|
||||
def apply_and_check_labels(event, number, node_id, issue_type, username, labels, label_descriptions):
|
||||
"""Normalizes, applies labels, and handles Alert label if present."""
|
||||
if not labels:
|
||||
@@ -194,7 +209,6 @@ def main(*args, **kwargs):
|
||||
body,
|
||||
repository_context,
|
||||
summarize=AUTO_PR_SUMMARY,
|
||||
acknowledge=AUTO_LABELS,
|
||||
current_labels=[label["name"] for label in event.pr.get("labels", [])],
|
||||
)
|
||||
|
||||
@@ -219,9 +233,10 @@ def main(*args, **kwargs):
|
||||
response.get("labels", []),
|
||||
label_descriptions,
|
||||
)
|
||||
if first_comment := response.get("first_comment"):
|
||||
print("Adding first interaction comment...")
|
||||
event.add_comment(number, node_id, first_comment, issue_type)
|
||||
print("Adding first interaction comment...")
|
||||
event.add_comment(
|
||||
number, node_id, get_pr_first_comment_template(event.repository, username), issue_type
|
||||
)
|
||||
|
||||
# Automatic PR review after first interaction
|
||||
if AUTO_PR_REVIEW:
|
||||
|
||||
+29
-1
@@ -15,6 +15,31 @@ from .utils import (
|
||||
SUMMARY_MARKER = "## 🛠️ PR Summary"
|
||||
|
||||
|
||||
def generate_merge_message(pr_summary, pr_credit, pr_url):
|
||||
"""Generate a personalized thank-you message for a merged pull request."""
|
||||
messages = [
|
||||
{
|
||||
"role": "system",
|
||||
"content": (
|
||||
"You are an Ultralytics AI assistant. Your response is posted verbatim as a GitHub comment on a "
|
||||
"merged PR. Return only the final comment body with no preamble, sign-off, or horizontal rule."
|
||||
),
|
||||
},
|
||||
{
|
||||
"role": "user",
|
||||
"content": f"""Thank {pr_credit} for the merged PR {pr_url} using the verified context below.
|
||||
|
||||
{pr_summary}
|
||||
|
||||
- Start with an enthusiastic note that the PR was merged.
|
||||
- Include exactly one short, accurately attributed inspirational quote in a Markdown blockquote.
|
||||
- Connect the quote to the concrete impact described in the PR summary without inventing results or benefits.
|
||||
- Keep the complete comment concise and meaningful.""",
|
||||
},
|
||||
]
|
||||
return get_response(messages)
|
||||
|
||||
|
||||
def generate_pr_summary(repository, diff_text, title="", description=""):
|
||||
"""Generates a concise, professional summary of a PR using the OpenAI or Anthropic API."""
|
||||
prompt, is_large, skipped_files = get_pr_summary_prompt(repository, diff_text, title, description)
|
||||
@@ -94,7 +119,10 @@ def main(*args, **kwargs):
|
||||
if pr_credit:
|
||||
print("Posting PR author thank you message...")
|
||||
event.add_comment(
|
||||
event.pr["number"], None, f"🚀 Merged! Thank you {pr_credit} for the contribution.", "pull request"
|
||||
event.pr["number"],
|
||||
None,
|
||||
generate_merge_message(summary, pr_credit, event.pr["html_url"]),
|
||||
"pull request",
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -149,11 +149,6 @@ def get_pr_summary_prompt(
|
||||
return prompt, len(filtered_diff) > MAX_PROMPT_CHARS, skipped_files
|
||||
|
||||
|
||||
def get_pr_first_comment_template(repository: str, username: str) -> str:
|
||||
"""Return the concise acknowledgment that the PR-open model customizes."""
|
||||
return f"👋 Thanks @{username} for the `{repository}` contribution! This is an automated first response; an Ultralytics engineer will review the PR soon."
|
||||
|
||||
|
||||
def _is_anthropic_model(model: str) -> bool:
|
||||
"""Check if the model is an Anthropic model."""
|
||||
return model.startswith("claude")
|
||||
@@ -668,10 +663,9 @@ def get_pr_open_response(
|
||||
description: str = "",
|
||||
repository_context: str = "",
|
||||
summarize: bool = True,
|
||||
acknowledge: bool = True,
|
||||
current_labels: list | None = None,
|
||||
) -> dict:
|
||||
"""Generates unified PR response with summary, labels, and first comment in a single API call."""
|
||||
"""Generate a PR summary and labels in a single API call."""
|
||||
filtered_diff, skipped_files = filter_diff_text(diff_text)
|
||||
is_large = len(filtered_diff) > MAX_PROMPT_CHARS
|
||||
|
||||
@@ -684,26 +678,12 @@ def get_pr_open_response(
|
||||
if filtered_labels
|
||||
else "Return an empty labels array."
|
||||
)
|
||||
comment_instructions = (
|
||||
f"""Write a concise acknowledgment that:
|
||||
- Opens with one specific sentence showing what the PR changes; do not repeat the generated summary
|
||||
- Requests an action only when the description or diff provides concrete evidence it is missing
|
||||
- Never claims tests passed, asks for generic checklist work, or assumes a language, package manager, branch name, or release process
|
||||
- States that the response is automated and an Ultralytics engineer will review soon
|
||||
- Uses at most 90 words, no headings, sign-off, inspirational quote, or external links
|
||||
|
||||
Base acknowledgment:
|
||||
{get_pr_first_comment_template(repository, username)}"""
|
||||
if acknowledge
|
||||
else "Return an empty first_comment string."
|
||||
)
|
||||
|
||||
prompt = f"""You are processing a new GitHub PR by @{username} for the {repository} repository.
|
||||
|
||||
Repository context: {repository_context or "No additional metadata provided."}
|
||||
PR description: {description[:8000] or "No description provided."}
|
||||
|
||||
Generate 3 outputs in a single JSON response for the PR titled '{title}' with the following diff:
|
||||
Generate 2 outputs in a single JSON response for the PR titled '{title}' with the following diff:
|
||||
{filtered_diff[:MAX_PROMPT_CHARS]}{format_skipped_files_note(skipped_files)}
|
||||
|
||||
|
||||
@@ -711,19 +691,15 @@ Generate 3 outputs in a single JSON response for the PR titled '{title}' with th
|
||||
{summary_instructions}
|
||||
|
||||
--- SECOND JSON OUTPUT (PR LABELS) ---
|
||||
{label_instructions}
|
||||
|
||||
--- THIRD JSON OUTPUT (PR FIRST COMMENT) ---
|
||||
{comment_instructions}"""
|
||||
{label_instructions}"""
|
||||
|
||||
schema = {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"summary": {"type": "string", "description": "PR summary with emoji sections"},
|
||||
"labels": {"type": "array", "items": {"type": "string"}, "description": "Array of label names"},
|
||||
"first_comment": {"type": "string", "description": "Concise automated acknowledgment"},
|
||||
},
|
||||
"required": ["summary", "labels", "first_comment"],
|
||||
"required": ["summary", "labels"],
|
||||
"additionalProperties": False,
|
||||
}
|
||||
|
||||
|
||||
@@ -5,7 +5,18 @@ from unittest.mock import MagicMock, call, patch
|
||||
import pytest
|
||||
|
||||
from actions import first_interaction, review_pr
|
||||
from actions.first_interaction import get_event_content, get_first_interaction_response
|
||||
from actions.first_interaction import get_event_content, get_first_interaction_response, get_pr_first_comment_template
|
||||
|
||||
|
||||
def test_pr_first_comment_preserves_contributor_guidance():
|
||||
"""Test PR acknowledgments retain the complete contributor checklist and links."""
|
||||
comment = get_pr_first_comment_template("owner/repo", "contributor")
|
||||
|
||||
assert comment.count("- ✅") == 7
|
||||
assert "[Continuous Integration (CI)](https://docs.ultralytics.com/help/CI)" in comment
|
||||
assert "[Contributor License Agreement](https://docs.ultralytics.com/help/CLA)" in comment
|
||||
assert "[Contributing Guide](https://docs.ultralytics.com/help/contributing)" in comment
|
||||
assert "Bruce Lee" in comment
|
||||
|
||||
|
||||
def test_get_event_content_issue():
|
||||
@@ -83,7 +94,7 @@ def test_get_event_content_pr():
|
||||
def test_open_pr_review_description(mock_action, mock_content, mock_response, mock_review, body, expected):
|
||||
"""Test automatic reviews use the best available author and generated description context."""
|
||||
mock_content.return_value = (456, "node456", "Test PR", body, "testuser", "pull request", "opened")
|
||||
mock_response.return_value = {"summary": "Generated summary", "labels": [], "first_comment": ""}
|
||||
mock_response.return_value = {"summary": "Generated summary", "labels": []}
|
||||
event = mock_action.return_value
|
||||
event.should_skip_llm.return_value = False
|
||||
event.should_skip_pr_author.return_value = False
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from actions.summarize_pr import generate_pr_summary, label_fixed_issues
|
||||
from actions.summarize_pr import generate_merge_message, generate_pr_summary, label_fixed_issues
|
||||
|
||||
|
||||
@patch("actions.summarize_pr.get_response")
|
||||
@@ -16,6 +16,20 @@ def test_generate_pr_summary(mock_get_response):
|
||||
mock_get_response.assert_called_once()
|
||||
|
||||
|
||||
@patch("actions.summarize_pr.get_response")
|
||||
def test_generate_merge_message_requires_quote_and_pr_impact(mock_get_response):
|
||||
"""Test merged PR comments retain the personalized quote and impact requirements."""
|
||||
mock_get_response.return_value = "Merged response with quote"
|
||||
|
||||
message = generate_merge_message("Improved export reliability", "@testuser", "https://github.com/test/repo/pull/1")
|
||||
|
||||
assert message == "Merged response with quote"
|
||||
prompt = mock_get_response.call_args.args[0][1]["content"]
|
||||
assert "exactly one short, accurately attributed inspirational quote" in prompt
|
||||
assert "concrete impact" in prompt
|
||||
assert "Improved export reliability" in prompt
|
||||
|
||||
|
||||
def test_label_fixed_issues_posts_repository_neutral_comment():
|
||||
"""Test merged-issue responses avoid language, package-manager, and branch assumptions."""
|
||||
event = MagicMock(repository="owner/swift-app")
|
||||
|
||||
Reference in New Issue
Block a user