Use JSON schema (#629)
Co-authored-by: UltralyticsAssistant <[email protected]>
This commit is contained in:
co-authored by
UltralyticsAssistant
parent
8cf9fd720c
commit
04d6506b12
@@ -7,7 +7,7 @@ import time
|
||||
|
||||
from . import review_pr
|
||||
from .summarize_pr import SUMMARY_MARKER
|
||||
from .utils import ACTIONS_CREDIT, Action, filter_labels, get_completion, get_pr_open_response, remove_html_comments
|
||||
from .utils import ACTIONS_CREDIT, Action, filter_labels, 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"
|
||||
@@ -91,7 +91,7 @@ YOUR RESPONSE (label names only):
|
||||
},
|
||||
{"role": "user", "content": prompt},
|
||||
]
|
||||
suggested_labels = get_completion(messages, temperature=1.0)
|
||||
suggested_labels = get_response(messages, temperature=1.0)
|
||||
if "none" in suggested_labels.lower():
|
||||
return []
|
||||
|
||||
@@ -170,7 +170,7 @@ YOUR {issue_type.upper()} RESPONSE:
|
||||
},
|
||||
{"role": "user", "content": prompt},
|
||||
]
|
||||
return get_completion(messages)
|
||||
return get_response(messages)
|
||||
|
||||
|
||||
def main(*args, **kwargs):
|
||||
|
||||
+35
-10
@@ -6,7 +6,7 @@ import json
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
from .utils import ACTIONS_CREDIT, GITHUB_API_URL, MAX_PROMPT_CHARS, Action, get_completion, remove_html_comments
|
||||
from .utils import ACTIONS_CREDIT, GITHUB_API_URL, MAX_PROMPT_CHARS, Action, get_response, remove_html_comments
|
||||
|
||||
REVIEW_MARKER = "## 🔍 PR Review"
|
||||
ERROR_MARKER = "⚠️ Review generation encountered an error"
|
||||
@@ -179,10 +179,37 @@ def generate_pr_review(
|
||||
# print(f"\nUser prompt (first 3000 chars):\n{messages[1]['content'][:3000]}...\n")
|
||||
|
||||
try:
|
||||
response = get_completion(
|
||||
schema = {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"comments": {
|
||||
"type": "array",
|
||||
"items": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"file": {"type": "string"},
|
||||
"line": {"type": "integer"},
|
||||
"side": {"type": "string", "enum": ["LEFT", "RIGHT"]},
|
||||
"severity": {"type": "string", "enum": ["CRITICAL", "HIGH", "MEDIUM", "LOW", "SUGGESTION"]},
|
||||
"message": {"type": "string"},
|
||||
"start_line": {"type": ["integer", "null"]},
|
||||
"suggestion": {"type": ["string", "null"]},
|
||||
},
|
||||
"required": ["file", "line", "side", "severity", "message", "start_line", "suggestion"],
|
||||
"additionalProperties": False,
|
||||
},
|
||||
},
|
||||
"summary": {"type": "string"},
|
||||
},
|
||||
"required": ["comments", "summary"],
|
||||
"additionalProperties": False,
|
||||
}
|
||||
|
||||
response = get_response(
|
||||
messages,
|
||||
reasoning_effort="low",
|
||||
model="gpt-5-codex",
|
||||
text_format={"format": {"type": "json_schema", "name": "pr_review", "strict": True, "schema": schema}},
|
||||
tools=[
|
||||
{
|
||||
"type": "web_search",
|
||||
@@ -197,17 +224,15 @@ def generate_pr_review(
|
||||
],
|
||||
)
|
||||
|
||||
json_str = re.search(r"```(?:json)?\s*(\{.*?\})\s*```", response, re.DOTALL)
|
||||
review_data = json.loads(json_str.group(1) if json_str else response)
|
||||
print(json.dumps(review_data, indent=2))
|
||||
print(json.dumps(response, indent=2))
|
||||
|
||||
# Count comments BEFORE filtering (for COMMENT vs APPROVE decision)
|
||||
comments_before_filtering = len(review_data.get("comments", []))
|
||||
comments_before_filtering = len(response.get("comments", []))
|
||||
print(f"AI generated {comments_before_filtering} comments")
|
||||
|
||||
# Validate, filter, and deduplicate comments
|
||||
unique_comments = {}
|
||||
for c in review_data.get("comments", []):
|
||||
for c in response.get("comments", []):
|
||||
file_path, line_num = c.get("file"), c.get("line", 0)
|
||||
start_line = c.get("start_line")
|
||||
side = (c.get("side") or "RIGHT").upper() # Default to RIGHT (added lines)
|
||||
@@ -245,7 +270,7 @@ def generate_pr_review(
|
||||
else:
|
||||
print(f"⚠️ AI duplicate for {key}: {c.get('severity')} - {(c.get('message') or '')[:60]}...")
|
||||
|
||||
review_data.update(
|
||||
response.update(
|
||||
{
|
||||
"comments": list(unique_comments.values()),
|
||||
"comments_before_filtering": comments_before_filtering,
|
||||
@@ -254,8 +279,8 @@ def generate_pr_review(
|
||||
"skipped_files": skipped_count,
|
||||
}
|
||||
)
|
||||
print(f"Valid comments after filtering: {len(review_data['comments'])}")
|
||||
return review_data
|
||||
print(f"Valid comments after filtering: {len(response['comments'])}")
|
||||
return response
|
||||
|
||||
except Exception as e:
|
||||
import traceback
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from .utils import ACTIONS_CREDIT, GITHUB_API_URL, Action, get_completion, get_pr_summary_prompt
|
||||
from .utils import ACTIONS_CREDIT, GITHUB_API_URL, Action, get_pr_summary_prompt, get_response
|
||||
|
||||
SUMMARY_MARKER = "## 🛠️ PR Summary"
|
||||
|
||||
@@ -24,7 +24,7 @@ def generate_merge_message(pr_summary, pr_credit, pr_url):
|
||||
),
|
||||
},
|
||||
]
|
||||
return get_completion(messages)
|
||||
return get_response(messages)
|
||||
|
||||
|
||||
def generate_issue_comment(pr_url, pr_summary, pr_credit, pr_title=""):
|
||||
@@ -54,7 +54,7 @@ def generate_issue_comment(pr_url, pr_summary, pr_credit, pr_title=""):
|
||||
f"5. Thank 🙏 for reporting the issue and welcome any further feedback if the issue persists\n\n",
|
||||
},
|
||||
]
|
||||
return get_completion(messages)
|
||||
return get_response(messages)
|
||||
|
||||
|
||||
def generate_pr_summary(repository, diff_text):
|
||||
@@ -68,7 +68,7 @@ def generate_pr_summary(repository, diff_text):
|
||||
},
|
||||
{"role": "user", "content": prompt},
|
||||
]
|
||||
reply = get_completion(messages, temperature=1.0)
|
||||
reply = get_response(messages, temperature=1.0)
|
||||
if is_large:
|
||||
reply = "**WARNING ⚠️** this PR is very large, summary may not cover all changes.\n\n" + reply
|
||||
|
||||
|
||||
@@ -8,7 +8,7 @@ import subprocess
|
||||
import time
|
||||
from datetime import datetime
|
||||
|
||||
from .utils import GITHUB_API_URL, Action, get_completion, remove_html_comments
|
||||
from .utils import GITHUB_API_URL, Action, get_response, remove_html_comments
|
||||
|
||||
# Environment variables
|
||||
CURRENT_TAG = os.getenv("CURRENT_TAG")
|
||||
@@ -150,7 +150,7 @@ def generate_release_summary(
|
||||
},
|
||||
]
|
||||
# print(messages[-1]["content"]) # for debug
|
||||
return get_completion(messages, temperature=1.0) + release_suffix
|
||||
return get_response(messages, temperature=1.0) + release_suffix
|
||||
|
||||
|
||||
def create_github_release(event, tag_name: str, name: str, body: str):
|
||||
|
||||
@@ -13,10 +13,10 @@ from .github_utils import GITHUB_API_URL, GITHUB_GRAPHQL_URL, Action, ultralytic
|
||||
from .openai_utils import (
|
||||
MAX_PROMPT_CHARS,
|
||||
filter_labels,
|
||||
get_completion,
|
||||
get_pr_open_response,
|
||||
get_pr_summary_guidelines,
|
||||
get_pr_summary_prompt,
|
||||
get_response,
|
||||
)
|
||||
from .version_utils import check_pubdev_version, check_pypi_version
|
||||
|
||||
@@ -34,10 +34,10 @@ __all__ = (
|
||||
"check_pubdev_version",
|
||||
"check_pypi_version",
|
||||
"filter_labels",
|
||||
"get_completion",
|
||||
"get_pr_open_response",
|
||||
"get_pr_summary_guidelines",
|
||||
"get_pr_summary_prompt",
|
||||
"get_response",
|
||||
"remove_html_comments",
|
||||
"ultralytics_actions_info",
|
||||
)
|
||||
|
||||
@@ -109,13 +109,13 @@ def get_pr_first_comment_template(repository: str, username: str) -> str:
|
||||
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 get_completion(
|
||||
def get_response(
|
||||
messages: list[dict[str, str]],
|
||||
check_links: bool = True,
|
||||
remove: list[str] = (" @giscus[bot]",),
|
||||
temperature: float = 1.0,
|
||||
reasoning_effort: str | None = None,
|
||||
response_format: dict | None = None,
|
||||
text_format: dict | None = None,
|
||||
model: str = OPENAI_MODEL,
|
||||
tools: list[dict] | None = None,
|
||||
) -> str | dict:
|
||||
@@ -130,6 +130,8 @@ def get_completion(
|
||||
data = {"model": model, "input": messages, "store": False, "temperature": temperature}
|
||||
if "gpt-5" in model:
|
||||
data["reasoning"] = {"effort": reasoning_effort or "low"}
|
||||
if text_format:
|
||||
data["text"] = text_format
|
||||
if tools:
|
||||
data["tools"] = tools
|
||||
|
||||
@@ -145,6 +147,11 @@ def get_completion(
|
||||
time.sleep(2**attempt)
|
||||
continue
|
||||
|
||||
if r.status_code >= 400:
|
||||
error_body = r.text
|
||||
print(f"API Error {r.status_code}: {error_body}")
|
||||
r.reason = f"{r.reason}\n{error_body}" # Add error body to exception message
|
||||
|
||||
r.raise_for_status()
|
||||
|
||||
# Parse response
|
||||
@@ -173,7 +180,7 @@ def get_completion(
|
||||
token_str += f" (+{thinking_tokens} thinking)"
|
||||
print(f"{model} ({token_str} = {input_tokens + output_tokens} tokens, ${cost:.5f}, {elapsed:.1f}s)")
|
||||
|
||||
if response_format and response_format.get("type") == "json_object":
|
||||
if text_format and text_format.get("format", {}).get("type") in ["json_object", "json_schema"]:
|
||||
return json.loads(content)
|
||||
|
||||
content = remove_outer_codeblocks(content)
|
||||
@@ -230,16 +237,28 @@ Customized welcome message adapting the template below:
|
||||
- No spaces between bullet points
|
||||
|
||||
Example comment template (adapt as needed, keep all links):
|
||||
{get_pr_first_comment_template(repository, username)}
|
||||
{get_pr_first_comment_template(repository, username)}"""
|
||||
|
||||
Return ONLY valid JSON in this exact format:
|
||||
{{"summary": "...", "labels": [...], "first_comment": "..."}}"""
|
||||
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": "Welcome comment with checklist"},
|
||||
},
|
||||
"required": ["summary", "labels", "first_comment"],
|
||||
"additionalProperties": False,
|
||||
}
|
||||
|
||||
messages = [
|
||||
{"role": "system", "content": "You are an Ultralytics AI assistant processing GitHub PRs."},
|
||||
{"role": "user", "content": prompt},
|
||||
]
|
||||
result = get_completion(messages, temperature=1.0, response_format={"type": "json_object"})
|
||||
result = get_response(
|
||||
messages,
|
||||
temperature=1.0,
|
||||
text_format={"format": {"type": "json_schema", "name": "pr_open_response", "strict": True, "schema": schema}},
|
||||
)
|
||||
if is_large and "summary" in result:
|
||||
result["summary"] = (
|
||||
"**WARNING ⚠️** this PR is very large, summary may not cover all changes.\n\n" + result["summary"]
|
||||
@@ -252,5 +271,5 @@ if __name__ == "__main__":
|
||||
{"role": "system", "content": "You are a helpful AI assistant."},
|
||||
{"role": "user", "content": "Explain how to export a YOLO11 model to CoreML."},
|
||||
]
|
||||
response = get_completion(messages)
|
||||
response = get_response(messages)
|
||||
print(response)
|
||||
|
||||
@@ -59,10 +59,10 @@ def test_get_event_content_pr():
|
||||
assert action == "opened"
|
||||
|
||||
|
||||
@patch("actions.first_interaction.get_completion")
|
||||
def test_get_relevant_labels(mock_get_completion):
|
||||
@patch("actions.first_interaction.get_response")
|
||||
def test_get_relevant_labels(mock_get_response):
|
||||
"""Test getting relevant labels for an issue."""
|
||||
mock_get_completion.return_value = "bug, enhancement"
|
||||
mock_get_response.return_value = "bug, enhancement"
|
||||
|
||||
available_labels = {
|
||||
"bug": "A bug in the software",
|
||||
@@ -79,13 +79,13 @@ def test_get_relevant_labels(mock_get_completion):
|
||||
)
|
||||
|
||||
assert labels == ["bug", "enhancement"]
|
||||
mock_get_completion.assert_called_once()
|
||||
mock_get_response.assert_called_once()
|
||||
|
||||
|
||||
@patch("actions.first_interaction.get_completion")
|
||||
def test_get_first_interaction_response(mock_get_completion):
|
||||
@patch("actions.first_interaction.get_response")
|
||||
def test_get_first_interaction_response(mock_get_response):
|
||||
"""Test generating first interaction response."""
|
||||
mock_get_completion.return_value = "Thank you for your issue"
|
||||
mock_get_response.return_value = "Thank you for your issue"
|
||||
|
||||
mock_event = MagicMock()
|
||||
mock_event.repository = "test/repo"
|
||||
@@ -95,4 +95,4 @@ def test_get_first_interaction_response(mock_get_completion):
|
||||
)
|
||||
|
||||
assert response == "Thank you for your issue"
|
||||
mock_get_completion.assert_called_once()
|
||||
mock_get_response.assert_called_once()
|
||||
|
||||
+2
-2
@@ -12,7 +12,7 @@ from actions.utils import (
|
||||
Action,
|
||||
allow_redirect,
|
||||
check_pypi_version,
|
||||
get_completion,
|
||||
get_response,
|
||||
remove_html_comments,
|
||||
ultralytics_actions_info,
|
||||
)
|
||||
@@ -38,7 +38,7 @@ def test_util_module_imports():
|
||||
# Check function imports
|
||||
assert callable(allow_redirect)
|
||||
assert callable(check_pypi_version)
|
||||
assert callable(get_completion)
|
||||
assert callable(get_response)
|
||||
assert callable(remove_html_comments)
|
||||
assert callable(ultralytics_actions_info)
|
||||
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from actions.utils.openai_utils import get_completion, remove_outer_codeblocks
|
||||
from actions.utils.openai_utils import get_response, remove_outer_codeblocks
|
||||
|
||||
|
||||
def test_remove_outer_codeblocks():
|
||||
@@ -23,7 +23,7 @@ def test_remove_outer_codeblocks():
|
||||
|
||||
|
||||
@patch("requests.post")
|
||||
def test_get_completion(mock_post):
|
||||
def test_get_response(mock_post):
|
||||
"""Test OpenAI Responses API completion function with mocked response."""
|
||||
# Setup mock response with Responses API structure
|
||||
mock_response = MagicMock()
|
||||
@@ -45,7 +45,7 @@ def test_get_completion(mock_post):
|
||||
# Use a context manager for the environment variable
|
||||
with patch.dict("os.environ", {"OPENAI_API_KEY": "test-key"}, clear=False):
|
||||
with patch("actions.utils.openai_utils.OPENAI_API_KEY", "test-key"):
|
||||
result = get_completion(messages, check_links=False)
|
||||
result = get_response(messages, check_links=False)
|
||||
|
||||
assert result == "Test response from OpenAI"
|
||||
mock_post.assert_called_once()
|
||||
@@ -53,8 +53,8 @@ def test_get_completion(mock_post):
|
||||
|
||||
@patch("requests.post")
|
||||
@patch("actions.utils.openai_utils.check_links_in_string")
|
||||
def test_get_completion_with_link_check(mock_check_links, mock_post):
|
||||
"""Test get_completion with link checking."""
|
||||
def test_get_response_with_link_check(mock_check_links, mock_post):
|
||||
"""Test get_response with link checking."""
|
||||
# Setup mocks with Responses API structure
|
||||
mock_response = MagicMock()
|
||||
mock_response.status_code = 200
|
||||
@@ -75,7 +75,7 @@ def test_get_completion_with_link_check(mock_check_links, mock_post):
|
||||
# Use a context manager for the environment variable
|
||||
with patch.dict("os.environ", {"OPENAI_API_KEY": "test-key"}, clear=False):
|
||||
with patch("actions.utils.openai_utils.OPENAI_API_KEY", "test-key"):
|
||||
result = get_completion(messages)
|
||||
result = get_response(messages)
|
||||
|
||||
assert result == "Response with https://example.com link"
|
||||
mock_check_links.assert_called_once()
|
||||
|
||||
+12
-12
@@ -5,34 +5,34 @@ from unittest.mock import patch
|
||||
from actions.summarize_pr import generate_issue_comment, generate_merge_message, generate_pr_summary
|
||||
|
||||
|
||||
@patch("actions.summarize_pr.get_completion")
|
||||
def test_generate_pr_summary(mock_get_completion):
|
||||
@patch("actions.summarize_pr.get_response")
|
||||
def test_generate_pr_summary(mock_get_response):
|
||||
"""Test generating PR summary with expected formatting."""
|
||||
mock_get_completion.return_value = "Test PR summary content"
|
||||
mock_get_response.return_value = "Test PR summary content"
|
||||
summary = generate_pr_summary("test/repo", "diff content")
|
||||
|
||||
assert summary.startswith("## 🛠️ PR Summary")
|
||||
assert "Test PR summary content" in summary
|
||||
mock_get_completion.assert_called_once()
|
||||
mock_get_response.assert_called_once()
|
||||
|
||||
|
||||
@patch("actions.summarize_pr.get_completion")
|
||||
def test_generate_merge_message(mock_get_completion):
|
||||
@patch("actions.summarize_pr.get_response")
|
||||
def test_generate_merge_message(mock_get_response):
|
||||
"""Test generating merge thank you messages."""
|
||||
mock_get_completion.return_value = "Thank you for your contribution!"
|
||||
mock_get_response.return_value = "Thank you for your contribution!"
|
||||
|
||||
message = generate_merge_message(
|
||||
pr_summary="Feature implementation", pr_credit="@testuser", pr_url="https://github.com/test/repo/pull/1"
|
||||
)
|
||||
|
||||
assert message == "Thank you for your contribution!"
|
||||
mock_get_completion.assert_called_once()
|
||||
mock_get_response.assert_called_once()
|
||||
|
||||
|
||||
@patch("actions.summarize_pr.get_completion")
|
||||
def test_generate_issue_comment(mock_get_completion):
|
||||
@patch("actions.summarize_pr.get_response")
|
||||
def test_generate_issue_comment(mock_get_response):
|
||||
"""Test generating issue comments about PR fixes."""
|
||||
mock_get_completion.return_value = "This issue is fixed in PR #123"
|
||||
mock_get_response.return_value = "This issue is fixed in PR #123"
|
||||
|
||||
comment = generate_issue_comment(
|
||||
pr_url="https://api.github.com/repos/owner/repo/pulls/123",
|
||||
@@ -42,4 +42,4 @@ def test_generate_issue_comment(mock_get_completion):
|
||||
)
|
||||
|
||||
assert comment == "This issue is fixed in PR #123"
|
||||
mock_get_completion.assert_called_once()
|
||||
mock_get_response.assert_called_once()
|
||||
|
||||
@@ -82,10 +82,10 @@ def test_get_prs_between_tags_empty_commits():
|
||||
assert get_prs_between_tags(mock_event, "v1.0.0", "v1.1.0") == []
|
||||
|
||||
|
||||
@patch("actions.summarize_release.get_completion")
|
||||
def test_generate_release_summary(mock_get_completion):
|
||||
@patch("actions.summarize_release.get_response")
|
||||
def test_generate_release_summary(mock_get_response):
|
||||
"""Test generating release summary."""
|
||||
mock_get_completion.return_value = "Release summary content"
|
||||
mock_get_response.return_value = "Release summary content"
|
||||
|
||||
mock_event = MagicMock()
|
||||
mock_event.repository = "test/repo"
|
||||
@@ -111,7 +111,7 @@ def test_generate_release_summary(mock_get_completion):
|
||||
assert "Release summary content" in summary
|
||||
assert "What's Changed" in summary
|
||||
assert "https://github.com/test/repo/compare/v1.0.0...v1.1.0" in summary
|
||||
mock_get_completion.assert_called_once()
|
||||
mock_get_response.assert_called_once()
|
||||
|
||||
|
||||
def test_create_github_release():
|
||||
|
||||
Reference in New Issue
Block a user