diff --git a/README.md b/README.md index a2afa596..b6514c05 100644 --- a/README.md +++ b/README.md @@ -183,7 +183,6 @@ Here is a quick overview of the different sub-tools of PR Reviewer: - PR Analysis - Summarize main theme - - PR description and title - PR type classification - Is the PR covered by relevant tests - Is this a focused PR @@ -199,7 +198,6 @@ This is how a typical output of the PR Reviewer looks like: #### PR Analysis - ๐ŸŽฏ **Main theme:** Adding language extension handler and token handler -- ๐Ÿ” **Description and title:** Yes - ๐Ÿ“Œ **Type of PR:** Enhancement - ๐Ÿงช **Relevant tests added:** No - โœจ **Focused PR:** Yes, the PR is focused on adding two new handlers for language extension and token counting. diff --git a/pr_agent/algo/utils.py b/pr_agent/algo/utils.py index 67c6f52b..ff4bbdac 100644 --- a/pr_agent/algo/utils.py +++ b/pr_agent/algo/utils.py @@ -11,7 +11,6 @@ def convert_to_markdown(output_data: dict) -> str: emojis = { "Main theme": "๐ŸŽฏ", - "Description and title": "๐Ÿ”", "Type of PR": "๐Ÿ“Œ", "Relevant tests added": "๐Ÿงช", "Unrelated changes": "โš ๏ธ", diff --git a/pr_agent/cli.py b/pr_agent/cli.py index 66d00cd1..8b280065 100644 --- a/pr_agent/cli.py +++ b/pr_agent/cli.py @@ -3,6 +3,7 @@ import asyncio import logging import os +from pr_agent.tools.pr_description import PRDescription from pr_agent.tools.pr_questions import PRQuestions from pr_agent.tools.pr_reviewer import PRReviewer @@ -11,12 +12,17 @@ def run(): parser = argparse.ArgumentParser(description='AI based pull request analyzer') parser.add_argument('--pr_url', type=str, help='The URL of the PR to review', required=True) parser.add_argument('--question', type=str, help='Optional question to ask', required=False) + parser.add_argument('--pr_description', action='store_true', help='Optional question to ask', required=False) args = parser.parse_args() logging.basicConfig(level=os.environ.get("LOGLEVEL", "INFO")) if args.question: print(f"Question: {args.question} about PR {args.pr_url}") reviewer = PRQuestions(args.pr_url, args.question) asyncio.run(reviewer.answer()) + elif args.pr_description: + print(f"PR description: {args.pr_url}") + reviewer = PRDescription(args.pr_url) + asyncio.run(reviewer.describe()) else: print(f"Reviewing PR: {args.pr_url}") reviewer = PRReviewer(args.pr_url, cli_mode=True) diff --git a/pr_agent/config_loader.py b/pr_agent/config_loader.py index 550f743e..ed98fccd 100644 --- a/pr_agent/config_loader.py +++ b/pr_agent/config_loader.py @@ -11,6 +11,7 @@ settings = Dynaconf( "settings/configuration.toml", "settings/pr_reviewer_prompts.toml", "settings/pr_questions_prompts.toml", + "settings/pr_description_prompts.toml", "settings_prod/.secrets.toml" ]] ) diff --git a/pr_agent/git_providers/git_provider.py b/pr_agent/git_providers/git_provider.py index a30df90b..e86f461d 100644 --- a/pr_agent/git_providers/git_provider.py +++ b/pr_agent/git_providers/git_provider.py @@ -16,6 +16,10 @@ class GitProvider(ABC): def get_diff_files(self) -> list[FilePatchInfo]: pass + @abstractmethod + def publish_description(self, pr_title: str, pr_body: str): + pass + @abstractmethod def publish_comment(self, pr_comment: str, is_temporary: bool = False): pass diff --git a/pr_agent/git_providers/github_provider.py b/pr_agent/git_providers/github_provider.py index 49b0ccbe..958a56e8 100644 --- a/pr_agent/git_providers/github_provider.py +++ b/pr_agent/git_providers/github_provider.py @@ -39,6 +39,10 @@ class GithubProvider: diff_files.append(FilePatchInfo(original_file_content_str, new_file_content_str, file.patch, file.filename)) return diff_files + def publish_description(self, pr_title: str, pr_body: str): + self.pr.edit(title=pr_title, body=pr_body) + # self.pr.create_issue_comment(pr_comment) + def publish_comment(self, pr_comment: str, is_temporary: bool = False): response = self.pr.create_issue_comment(pr_comment) if hasattr(response, "user") and hasattr(response.user, "login"): diff --git a/pr_agent/git_providers/gitlab_provider.py b/pr_agent/git_providers/gitlab_provider.py index e9279a82..485e0cf9 100644 --- a/pr_agent/git_providers/gitlab_provider.py +++ b/pr_agent/git_providers/gitlab_provider.py @@ -44,6 +44,10 @@ class GitLabProvider(GitProvider): def get_files(self): return [change['new_path'] for change in self.mr.changes()['changes']] + def publish_description(self, pr_title: str, pr_body: str): + logging.exception("Not implemented yet") + pass + def publish_comment(self, mr_comment: str, is_temporary: bool = False): comment = self.mr.notes.create({'body': mr_comment}) if is_temporary: diff --git a/pr_agent/settings/pr_description_prompts.toml b/pr_agent/settings/pr_description_prompts.toml new file mode 100644 index 00000000..2ec31b1c --- /dev/null +++ b/pr_agent/settings/pr_description_prompts.toml @@ -0,0 +1,45 @@ +[pr_description_prompt] +system="""You are CodiumAI-PR-Reviewer, a language model designed to review git pull requests. +Your task is to provide full description of the PR content. +- Make sure not to focus the new PR code (the '+' lines). + +You must use the following JSON schema to format your answer: +```json +{ + "PR Title": { + "type": "string", + "description": "an informative title for the PR, describing its main theme" + }, + "Type of PR": { + "type": "string", + "enum": ["Bug fix", "Tests", "Bug fix with tests", "Refactoring", "Enhancement", "Documentation", "Other"] + }, + "PR Description": { + "type": "string", + "description": "an informative and concise description of the PR" + }, + "PR Main Files Walkthrough": { + "type": "string", + "description": "a walkthrough of the PR changes. Review main files, in bullet points, and shortly describe the changes in each file (up to 10 most important files). Format: -`filename`: description of changes\n..." + } +} + +Don't repeat the prompt in the answer, and avoid outputting the 'type' and 'description' fields. +""" + +user="""PR Info: +Branch: '{{branch}}' +{%- if language %} +Main language: {{language}} +{%- endif %} + + +The PR Git Diff: +``` +{{diff}} +``` +Note that lines in the diff body are prefixed with a symbol that represents the type of change: '-' for deletions, '+' for additions, and ' ' (a space) for unchanged lines. + +Response (should be a valid JSON, and nothing else): +```json +""" diff --git a/pr_agent/settings/pr_reviewer_prompts.toml b/pr_agent/settings/pr_reviewer_prompts.toml index 36b4c8cd..4a234905 100644 --- a/pr_agent/settings/pr_reviewer_prompts.toml +++ b/pr_agent/settings/pr_reviewer_prompts.toml @@ -13,10 +13,6 @@ You must use the following JSON schema to format your answer: "type": "string", "description": "a short explanation of the PR" }, - "Description and title": { - "type": "string", - "description": "yes\\no question: does this PR have a relevant description and title" - }, "Type of PR": { "type": "string", "enum": ["Bug fix", "Tests", "Bug fix with tests", "Refactoring", "Enhancement", "Documentation", "Other"] @@ -37,7 +33,7 @@ You must use the following JSON schema to format your answer: "PR Feedback": { "General PR suggestions": { "type": "string", - "description": "important suggestions for the contributors and maintainers of this PR, may include overall structure, primary purpose and best practices. consider using specific filenames, classes and functions names. explain yourself!" + "description": "General suggestions and feedback for the contributors and maintainers of this PR. May include important suggestions for the overall structure, primary purpose, best practices, critical bugs, and other aspects of the PR. Explain your suggestions." }, "Code suggestions": { "type": "array", @@ -75,7 +71,6 @@ Example output: "PR Analysis": { "Main theme": "xxx", - "Description and title": "Yes", "Type of PR": "Bug fix", {%- if require_tests %} "Relevant tests added": "No", diff --git a/pr_agent/tools/pr_description.py b/pr_agent/tools/pr_description.py new file mode 100644 index 00000000..334678e4 --- /dev/null +++ b/pr_agent/tools/pr_description.py @@ -0,0 +1,83 @@ +import copy +import json +import logging + +from jinja2 import Environment, StrictUndefined + +from pr_agent.algo.ai_handler import AiHandler +from pr_agent.algo.pr_processing import get_pr_diff +from pr_agent.algo.token_handler import TokenHandler +from pr_agent.algo.utils import convert_to_markdown +from pr_agent.config_loader import settings +from pr_agent.git_providers import get_git_provider +from pr_agent.git_providers.git_provider import get_main_pr_language + + +class PRDescription: + def __init__(self, pr_url: str): + self.git_provider = get_git_provider()(pr_url) + self.main_pr_language = get_main_pr_language( + self.git_provider.get_languages(), self.git_provider.get_files() + ) + self.ai_handler = AiHandler() + self.vars = { + "title": self.git_provider.pr.title, + "branch": self.git_provider.get_pr_branch(), + "description": self.git_provider.get_description(), + "language": self.main_pr_language, + "diff": "", # empty diff for initial calculation + } + self.token_handler = TokenHandler(self.git_provider.pr, + self.vars, + settings.pr_description_prompt.system, + settings.pr_description_prompt.user) + self.patches_diff = None + self.prediction = None + + async def describe(self): + logging.info('Answering a PR question...') + if settings.config.publish_review: + self.git_provider.publish_comment("Preparing pr description...", is_temporary=True) + logging.info('Getting PR diff...') + self.patches_diff = get_pr_diff(self.git_provider, self.token_handler) + logging.info('Getting AI prediction...') + self.prediction = await self._get_prediction() + logging.info('Preparing answer...') + pr_title, pr_body = self._prepare_pr_answer() + if settings.config.publish_review: + logging.info('Pushing answer...') + self.git_provider.publish_description(pr_title, pr_body) + self.git_provider.remove_initial_comment() + return "" + + async def _get_prediction(self): + variables = copy.deepcopy(self.vars) + variables["diff"] = self.patches_diff # update diff + environment = Environment(undefined=StrictUndefined) + system_prompt = environment.from_string(settings.pr_description_prompt.system).render(variables) + user_prompt = environment.from_string(settings.pr_description_prompt.user).render(variables) + if settings.config.verbosity_level >= 2: + logging.info(f"\nSystem prompt:\n{system_prompt}") + logging.info(f"\nUser prompt:\n{user_prompt}") + model = settings.config.model + response, finish_reason = await self.ai_handler.chat_completion(model=model, temperature=0.2, + system=system_prompt, user=user_prompt) + return response + + def _prepare_pr_answer(self): + data = json.loads(self.prediction) + pr_body = "" + # for key, value in data.items(): + # markdown_text += f"## {key}\n\n" + # markdown_text += f"{value}\n\n" + title = data['PR Title'] + del data['PR Title'] + for key, value in data.items(): + pr_body += f"{key}:\n" + if 'walkthrough' in key.lower(): + pr_body += f"{value}\n" + else: + pr_body += f"**{value}**\n\n___\n" + if settings.config.verbosity_level >= 2: + logging.info(f"title:\n{title}\n{pr_body}") + return title, pr_body diff --git a/tests/unit/test_convert_to_markdown.py b/tests/unit/test_convert_to_markdown.py index a40574ae..02e3ceac 100644 --- a/tests/unit/test_convert_to_markdown.py +++ b/tests/unit/test_convert_to_markdown.py @@ -46,7 +46,6 @@ class TestConvertToMarkdown: def test_simple_dictionary_input(self): input_data = { 'Main theme': 'Test', - 'Description and title': 'Test description', 'Type of PR': 'Test type', 'Relevant tests added': 'no', 'Unrelated changes': 'n/a', # won't be included in the output @@ -69,7 +68,6 @@ class TestConvertToMarkdown: } expected_output = """\ - ๐ŸŽฏ **Main theme:** Test -- ๐Ÿ” **Description and title:** Test description - ๐Ÿ“Œ **Type of PR:** Test type - ๐Ÿงช **Relevant tests added:** no - โœจ **Focused PR:** Yes @@ -108,7 +106,6 @@ class TestConvertToMarkdown: def test_dictionary_input_containing_only_empty_dictionaries(self): input_data = { 'Main theme': {}, - 'Description and title': {}, 'Type of PR': {}, 'Relevant tests added': {}, 'Unrelated changes': {}, diff --git a/tests/unit/test_fix_output.py b/tests/unit/test_fix_output.py index af0794ad..08e0504b 100644 --- a/tests/unit/test_fix_output.py +++ b/tests/unit/test_fix_output.py @@ -7,11 +7,10 @@ import pytest class TestTryFixJson: # Tests that JSON with complete 'Code suggestions' section returns expected output def test_incomplete_code_suggestions(self): - review = '{"PR Analysis": {"Main theme": "xxx", "Description and title": "Yes", "Type of PR": "Bug fix"}, "PR Feedback": {"General PR suggestions": "..., `xxx`...", "Code suggestions": [{"relevant file": "xxx.py", "suggestion content": "xxx [important]"}, {"suggestion number": 2, "relevant file": "yyy.py", "suggestion content": "yyy [incomp...' + review = '{"PR Analysis": {"Main theme": "xxx", "Type of PR": "Bug fix"}, "PR Feedback": {"General PR suggestions": "..., `xxx`...", "Code suggestions": [{"relevant file": "xxx.py", "suggestion content": "xxx [important]"}, {"suggestion number": 2, "relevant file": "yyy.py", "suggestion content": "yyy [incomp...' expected_output = { 'PR Analysis': { 'Main theme': 'xxx', - 'Description and title': 'Yes', 'Type of PR': 'Bug fix' }, 'PR Feedback': { @@ -27,11 +26,10 @@ class TestTryFixJson: assert try_fix_json(review) == expected_output def test_incomplete_code_suggestions_new_line(self): - review = '{"PR Analysis": {"Main theme": "xxx", "Description and title": "Yes", "Type of PR": "Bug fix"}, "PR Feedback": {"General PR suggestions": "..., `xxx`...", "Code suggestions": [{"relevant file": "xxx.py", "suggestion content": "xxx [important]"} \n\t, {"suggestion number": 2, "relevant file": "yyy.py", "suggestion content": "yyy [incomp...' + review = '{"PR Analysis": {"Main theme": "xxx", "Type of PR": "Bug fix"}, "PR Feedback": {"General PR suggestions": "..., `xxx`...", "Code suggestions": [{"relevant file": "xxx.py", "suggestion content": "xxx [important]"} \n\t, {"suggestion number": 2, "relevant file": "yyy.py", "suggestion content": "yyy [incomp...' expected_output = { 'PR Analysis': { 'Main theme': 'xxx', - 'Description and title': 'Yes', 'Type of PR': 'Bug fix' }, 'PR Feedback': { @@ -47,11 +45,10 @@ class TestTryFixJson: assert try_fix_json(review) == expected_output def test_incomplete_code_suggestions_many_close_brackets(self): - review = '{"PR Analysis": {"Main theme": "xxx", "Description and title": "Yes", "Type of PR": "Bug fix"}, "PR Feedback": {"General PR suggestions": "..., `xxx`...", "Code suggestions": [{"relevant file": "xxx.py", "suggestion content": "xxx [important]"} \n, {"suggestion number": 2, "relevant file": "yyy.py", "suggestion content": "yyy }, [}\n ,incomp.} ,..' + review = '{"PR Analysis": {"Main theme": "xxx", "Type of PR": "Bug fix"}, "PR Feedback": {"General PR suggestions": "..., `xxx`...", "Code suggestions": [{"relevant file": "xxx.py", "suggestion content": "xxx [important]"} \n, {"suggestion number": 2, "relevant file": "yyy.py", "suggestion content": "yyy }, [}\n ,incomp.} ,..' expected_output = { 'PR Analysis': { 'Main theme': 'xxx', - 'Description and title': 'Yes', 'Type of PR': 'Bug fix' }, 'PR Feedback': { @@ -67,11 +64,10 @@ class TestTryFixJson: assert try_fix_json(review) == expected_output def test_incomplete_code_suggestions_relevant_file(self): - review = '{"PR Analysis": {"Main theme": "xxx", "Description and title": "Yes", "Type of PR": "Bug fix"}, "PR Feedback": {"General PR suggestions": "..., `xxx`...", "Code suggestions": [{"relevant file": "xxx.py", "suggestion content": "xxx [important]"}, {"suggestion number": 2, "relevant file": "yyy.p' + review = '{"PR Analysis": {"Main theme": "xxx", "Type of PR": "Bug fix"}, "PR Feedback": {"General PR suggestions": "..., `xxx`...", "Code suggestions": [{"relevant file": "xxx.py", "suggestion content": "xxx [important]"}, {"suggestion number": 2, "relevant file": "yyy.p' expected_output = { 'PR Analysis': { 'Main theme': 'xxx', - 'Description and title': 'Yes', 'Type of PR': 'Bug fix' }, 'PR Feedback': {