diff --git a/actions/__init__.py b/actions/__init__.py index e9e67eb..ce075ef 100644 --- a/actions/__init__.py +++ b/actions/__init__.py @@ -28,4 +28,4 @@ # ├── test_summarize_pr.py # └── ... -__version__ = "0.3.2" +__version__ = "0.3.3" diff --git a/actions/first_interaction.py b/actions/first_interaction.py index a67f47f..5614a64 100644 --- a/actions/first_interaction.py +++ b/actions/first_interaction.py @@ -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: diff --git a/actions/summarize_pr.py b/actions/summarize_pr.py index 851d861..c30fd62 100644 --- a/actions/summarize_pr.py +++ b/actions/summarize_pr.py @@ -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", ) diff --git a/actions/utils/openai_utils.py b/actions/utils/openai_utils.py index cfc5570..9ec7ead 100644 --- a/actions/utils/openai_utils.py +++ b/actions/utils/openai_utils.py @@ -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, } diff --git a/tests/test_first_interaction.py b/tests/test_first_interaction.py index 44cffc1..158ef18 100644 --- a/tests/test_first_interaction.py +++ b/tests/test_first_interaction.py @@ -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 diff --git a/tests/test_summarize_pr.py b/tests/test_summarize_pr.py index ea294c3..2d62bae 100644 --- a/tests/test_summarize_pr.py +++ b/tests/test_summarize_pr.py @@ -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")