diff --git a/.github/workflows/build-and-test.yaml b/.github/workflows/build-and-test.yaml new file mode 100644 index 00000000..960da61b --- /dev/null +++ b/.github/workflows/build-and-test.yaml @@ -0,0 +1,36 @@ +name: Build-and-test + +on: + push: + +jobs: + build-and-test: + runs-on: ubuntu-latest + + steps: + - id: checkout + uses: actions/checkout@v2 + + - id: dockerx + name: Setup Docker Buildx + uses: docker/setup-buildx-action@v2 + + - id: build + name: Build dev docker + uses: docker/build-push-action@v2 + with: + context: . + file: ./docker/Dockerfile + push: false + load: true + tags: codiumai/pr-agent:test + cache-from: type=gha,scope=dev + cache-to: type=gha,mode=max,scope=dev + target: test + + - id: test + name: Test dev docker + run: | + docker run --rm codiumai/pr-agent:test pytest -v + + diff --git a/.github/workflows/review.yaml b/.github/workflows/pr-agent-review.yaml similarity index 60% rename from .github/workflows/review.yaml rename to .github/workflows/pr-agent-review.yaml index e7612520..9dcf59b8 100644 --- a/.github/workflows/review.yaml +++ b/.github/workflows/pr-agent-review.yaml @@ -1,6 +1,17 @@ +# This workflow enables developers to call PR-Agents `/[actions]` in PR's comments and upon PR creation. +# Learn more at https://www.codium.ai/pr-agent/ +# This is v0.2 of this workflow file + +name: PR-Agent + on: pull_request: issue_comment: + +permissions: + issues: write + pull-requests: write + jobs: pr_agent_job: runs-on: ubuntu-latest diff --git a/README.md b/README.md index b8ab88c9..0ec00ec2 100644 --- a/README.md +++ b/README.md @@ -97,12 +97,12 @@ CodiumAI `PR-Agent` is an open-source tool aiming to help developers review pull | | Incremental PR Review | :white_check_mark: | | | Examples for invoking the different tools via the CLI: -- **Review**: python cli.py --pr-url= review -- **Describe**: python cli.py --pr-url= describe -- **Improve**: python cli.py --pr-url= improve -- **Ask**: python cli.py --pr-url= ask "Write me a poem about this PR" -- **Reflect**: python cli.py --pr-url= reflect -- **Update Changelog**: python cli.py --pr-url= update_changelog +- **Review**: python cli.py --pr_url= review +- **Describe**: python cli.py --pr_url= describe +- **Improve**: python cli.py --pr_url= improve +- **Ask**: python cli.py --pr_url= ask "Write me a poem about this PR" +- **Reflect**: python cli.py --pr_url= reflect +- **Update Changelog**: python cli.py --pr_url= update_changelog "" is the url of the relevant PR (for example: https://github.com/Codium-ai/pr-agent/pull/50). diff --git a/docker/Dockerfile b/docker/Dockerfile index 4a8b86d5..61ab74cf 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -4,17 +4,21 @@ WORKDIR /app ADD pyproject.toml . RUN pip install . && rm pyproject.toml ENV PYTHONPATH=/app -ADD pr_agent pr_agent FROM base as github_app +ADD pr_agent pr_agent CMD ["python", "pr_agent/servers/github_app.py"] FROM base as github_polling +ADD pr_agent pr_agent CMD ["python", "pr_agent/servers/github_polling.py"] FROM base as test ADD requirements-dev.txt . RUN pip install -r requirements-dev.txt && rm requirements-dev.txt +ADD pr_agent pr_agent +ADD tests tests FROM base as cli +ADD pr_agent pr_agent ENTRYPOINT ["python", "pr_agent/cli.py"] diff --git a/pr_agent/algo/pr_processing.py b/pr_agent/algo/pr_processing.py index be4f4d5f..140046a3 100644 --- a/pr_agent/algo/pr_processing.py +++ b/pr_agent/algo/pr_processing.py @@ -11,7 +11,7 @@ from github import RateLimitExceededException from pr_agent.algo import MAX_TOKENS from pr_agent.algo.git_patch_processing import convert_to_hunks_with_lines_numbers, extend_patch, handle_patch_deletions from pr_agent.algo.language_handler import sort_files_by_main_languages -from pr_agent.algo.token_handler import TokenHandler +from pr_agent.algo.token_handler import TokenHandler, get_token_encoder from pr_agent.config_loader import get_settings from pr_agent.git_providers.git_provider import FilePatchInfo, GitProvider @@ -300,3 +300,30 @@ def find_line_number_of_relevant_line_in_file(diff_files: List[FilePatchInfo], absolute_position = start2 + delta - 1 break return position, absolute_position + + +def clip_tokens(text: str, max_tokens: int) -> str: + """ + Clip the number of tokens in a string to a maximum number of tokens. + + Args: + text (str): The string to clip. + max_tokens (int): The maximum number of tokens allowed in the string. + + Returns: + str: The clipped string. + """ + # We'll estimate the number of tokens by hueristically assuming 2.5 tokens per word + try: + encoder = get_token_encoder() + num_input_tokens = len(encoder.encode(text)) + if num_input_tokens <= max_tokens: + return text + num_chars = len(text) + chars_per_token = num_chars / num_input_tokens + num_output_chars = int(chars_per_token * max_tokens) + clipped_text = text[:num_output_chars] + return clipped_text + except Exception as e: + logging.warning(f"Failed to clip tokens: {e}") + return text \ No newline at end of file diff --git a/pr_agent/algo/token_handler.py b/pr_agent/algo/token_handler.py index 3686f521..f018a92b 100644 --- a/pr_agent/algo/token_handler.py +++ b/pr_agent/algo/token_handler.py @@ -4,6 +4,10 @@ from tiktoken import encoding_for_model, get_encoding from pr_agent.config_loader import get_settings +def get_token_encoder(): + return encoding_for_model(get_settings().config.model) if "gpt" in get_settings().config.model else get_encoding( + "cl100k_base") + class TokenHandler: """ A class for handling tokens in the context of a pull request. @@ -27,7 +31,7 @@ class TokenHandler: - system: The system string. - user: The user string. """ - self.encoder = encoding_for_model(get_settings().config.model) if "gpt" in get_settings().config.model else get_encoding("cl100k_base") + self.encoder = get_token_encoder() self.prompt_tokens = self._get_system_user_tokens(pr, self.encoder, vars, system, user) def _get_system_user_tokens(self, pr, encoder, vars: dict, system, user): diff --git a/pr_agent/algo/utils.py b/pr_agent/algo/utils.py index 264575bb..725d75ec 100644 --- a/pr_agent/algo/utils.py +++ b/pr_agent/algo/utils.py @@ -8,8 +8,8 @@ import textwrap from datetime import datetime from typing import Any, List +import yaml from starlette_context import context - from pr_agent.config_loader import get_settings, global_settings @@ -258,3 +258,26 @@ def update_settings_from_args(args: List[str]) -> List[str]: else: other_args.append(arg) return other_args + + +def load_yaml(review_text: str) -> dict: + review_text = review_text.removeprefix('```yaml').rstrip('`') + try: + data = yaml.load(review_text, Loader=yaml.SafeLoader) + except Exception as e: + logging.error(f"Failed to parse AI prediction: {e}") + data = try_fix_yaml(review_text) + return data + +def try_fix_yaml(review_text: str) -> dict: + review_text_lines = review_text.split('\n') + data = {} + for i in range(1, len(review_text_lines)): + review_text_lines_tmp = '\n'.join(review_text_lines[:-i]) + try: + data = yaml.load(review_text_lines_tmp, Loader=yaml.SafeLoader) + logging.info(f"Successfully parsed AI prediction after removing {i} lines") + break + except: + pass + return data diff --git a/pr_agent/cli.py b/pr_agent/cli.py index 8dd21b3f..0f871041 100644 --- a/pr_agent/cli.py +++ b/pr_agent/cli.py @@ -10,13 +10,13 @@ from pr_agent.config_loader import get_settings def run(inargs=None): parser = argparse.ArgumentParser(description='AI based pull request analyzer', usage= """\ -Usage: cli.py --pr-url []. +Usage: cli.py --pr-url= []. For example: -- cli.py --pr-url=... review -- cli.py --pr-url=... describe -- cli.py --pr-url=... improve -- cli.py --pr-url=... ask "write me a poem about this PR" -- cli.py --pr-url=... reflect +- cli.py --pr_url=... review +- cli.py --pr_url=... describe +- cli.py --pr_url=... improve +- cli.py --pr_url=... ask "write me a poem about this PR" +- cli.py --pr_url=... reflect Supported commands: review / review_pr - Add a review that includes a summary of the PR and specific suggestions for improvement. @@ -27,7 +27,7 @@ reflect - Ask the PR author questions about the PR. update_changelog - Update the changelog based on the PR's contents. To edit any configuration parameter from 'configuration.toml', just add -config_path=. -For example: '- cli.py --pr-url=... review --pr_reviewer.extra_instructions="focus on the file: ..."' +For example: 'python cli.py --pr_url=... review --pr_reviewer.extra_instructions="focus on the file: ..."' """) parser.add_argument('--pr_url', type=str, help='The URL of the PR to review', required=True) parser.add_argument('command', type=str, help='The', choices=commands, default='review') diff --git a/pr_agent/git_providers/bitbucket_provider.py b/pr_agent/git_providers/bitbucket_provider.py index 122b0db3..07b92295 100644 --- a/pr_agent/git_providers/bitbucket_provider.py +++ b/pr_agent/git_providers/bitbucket_provider.py @@ -5,6 +5,7 @@ from urllib.parse import urlparse import requests from atlassian.bitbucket import Cloud +from ..algo.pr_processing import clip_tokens from ..config_loader import get_settings from .git_provider import FilePatchInfo @@ -81,6 +82,9 @@ class BitbucketProvider: return self.pr.source_branch def get_pr_description(self): + max_tokens = get_settings().get("CONFIG.MAX_DESCRIPTION_TOKENS", None) + if max_tokens: + return clip_tokens(self.pr.description, max_tokens) return self.pr.description def get_user_id(self): diff --git a/pr_agent/git_providers/git_provider.py b/pr_agent/git_providers/git_provider.py index 8e161252..2a891938 100644 --- a/pr_agent/git_providers/git_provider.py +++ b/pr_agent/git_providers/git_provider.py @@ -97,6 +97,10 @@ class GitProvider(ABC): def remove_reaction(self, issue_comment_id: int, reaction_id: int) -> bool: pass + @abstractmethod + def get_commit_messages(self): + pass + def get_main_pr_language(languages, files) -> str: """ Get the main language of the commit. Return an empty string if cannot determine. diff --git a/pr_agent/git_providers/github_provider.py b/pr_agent/git_providers/github_provider.py index f83216ef..be0fa645 100644 --- a/pr_agent/git_providers/github_provider.py +++ b/pr_agent/git_providers/github_provider.py @@ -12,7 +12,7 @@ from starlette_context import context from .git_provider import FilePatchInfo, GitProvider, IncrementalPR from ..algo.language_handler import is_valid_file from ..algo.utils import load_large_diff -from ..algo.pr_processing import find_line_number_of_relevant_line_in_file +from ..algo.pr_processing import find_line_number_of_relevant_line_in_file, clip_tokens from ..config_loader import get_settings from ..servers.utils import RateLimitExceeded @@ -234,6 +234,9 @@ class GithubProvider(GitProvider): return self.pr.head.ref def get_pr_description(self): + max_tokens = get_settings().get("CONFIG.MAX_DESCRIPTION_TOKENS", None) + if max_tokens: + return clip_tokens(self.pr.body, max_tokens) return self.pr.body def get_user_id(self): @@ -375,27 +378,33 @@ class GithubProvider(GitProvider): logging.exception(f"Failed to get labels, error: {e}") return [] - def get_commit_messages(self) -> str: + def get_commit_messages(self): """ Retrieves the commit messages of a pull request. Returns: str: A string containing the commit messages of the pull request. """ + max_tokens = get_settings().get("CONFIG.MAX_COMMITS_TOKENS", None) try: commit_list = self.pr.get_commits() commit_messages = [commit.commit.message for commit in commit_list] commit_messages_str = "\n".join([f"{i + 1}. {message}" for i, message in enumerate(commit_messages)]) - except: + except Exception: commit_messages_str = "" + if max_tokens: + commit_messages_str = clip_tokens(commit_messages_str, max_tokens) return commit_messages_str def generate_link_to_relevant_line_number(self, suggestion) -> str: try: - relevant_file = suggestion['relevant file'] + relevant_file = suggestion['relevant file'].strip('`').strip("'") relevant_line_str = suggestion['relevant line'] + if not relevant_line_str: + return "" + position, absolute_position = find_line_number_of_relevant_line_in_file \ - (self.diff_files, relevant_file.strip('`'), relevant_line_str) + (self.diff_files, relevant_file, relevant_line_str) if absolute_position != -1: # # link to right file only diff --git a/pr_agent/git_providers/gitlab_provider.py b/pr_agent/git_providers/gitlab_provider.py index a4d2d127..73a3a2f9 100644 --- a/pr_agent/git_providers/gitlab_provider.py +++ b/pr_agent/git_providers/gitlab_provider.py @@ -7,6 +7,7 @@ import gitlab from gitlab import GitlabGetError from ..algo.language_handler import is_valid_file +from ..algo.pr_processing import clip_tokens from ..algo.utils import load_large_diff from ..config_loader import get_settings from .git_provider import EDIT_TYPE, FilePatchInfo, GitProvider @@ -275,6 +276,9 @@ class GitLabProvider(GitProvider): return self.mr.source_branch def get_pr_description(self): + max_tokens = get_settings().get("CONFIG.MAX_DESCRIPTION_TOKENS", None) + if max_tokens: + return clip_tokens(self.mr.description, max_tokens) return self.mr.description def get_issue_comments(self): @@ -338,16 +342,19 @@ class GitLabProvider(GitProvider): def get_labels(self): return self.mr.labels - def get_commit_messages(self) -> str: + def get_commit_messages(self): """ Retrieves the commit messages of a pull request. Returns: str: A string containing the commit messages of the pull request. """ + max_tokens = get_settings().get("CONFIG.MAX_COMMITS_TOKENS", None) try: commit_messages_list = [commit['message'] for commit in self.mr.commits()._list] commit_messages_str = "\n".join([f"{i + 1}. {message}" for i, message in enumerate(commit_messages_list)]) - except: + except Exception: commit_messages_str = "" + if max_tokens: + commit_messages_str = clip_tokens(commit_messages_str, max_tokens) return commit_messages_str \ No newline at end of file diff --git a/pr_agent/servers/help.py b/pr_agent/servers/help.py index 838645f5..1c8e1c3f 100644 --- a/pr_agent/servers/help.py +++ b/pr_agent/servers/help.py @@ -3,7 +3,8 @@ commands_text = "> **/review [-i]**: Request a review of your Pull Request. For "> **/describe**: Modify the PR title and description based on the contents of the PR.\n" \ "> **/improve**: Suggest improvements to the code in the PR. \n" \ "> **/ask \\**: Pose a question about the PR.\n\n" \ - ">To edit any configuration parameter from 'configuration.toml', add --config_path=new_value\n" \ + "> **/update_changelog**: Update the changelog based on the PR's contents.\n\n" \ + ">To edit any configuration parameter from **configuration.toml**, add --config_path=new_value\n" \ ">For example: /review --pr_reviewer.extra_instructions=\"focus on the file: ...\" \n" \ ">To list the possible configuration parameters, use the **/config** command.\n" \ diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index 8334049d..0c502df9 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -8,6 +8,8 @@ verbosity_level=0 # 0,1,2 use_extra_bad_extensions=false use_repo_settings_file=true ai_timeout=180 +max_description_tokens = 500 +max_commits_tokens = 500 [pr_reviewer] # /review # require_focused_review=true diff --git a/pr_agent/settings/pr_description_prompts.toml b/pr_agent/settings/pr_description_prompts.toml index 95a12681..43dd8e3b 100644 --- a/pr_agent/settings/pr_description_prompts.toml +++ b/pr_agent/settings/pr_description_prompts.toml @@ -2,38 +2,67 @@ 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). - +- Notice that the 'Previous title', 'Previous description' and 'Commit messages' sections may be partial, simplistic, non-informative or not up-to-date. Hence, compare them to the PR diff code, and use them only as a reference. +- If needed, each YAML output should be in block scalar format ('|-') {%- if extra_instructions %} Extra instructions from the user: {{ extra_instructions }} {% endif %} -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" - }, - "PR Type": { - "type": "string", - "description": possible values are: ["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..." - } -} +You must use the following YAML schema to format your answer: +```yaml +PR Title: + type: string + description: an informative title for the PR, describing its main theme +PR Type: + type: array + items: + 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: array + maxItems: 10 + description: |- + a walkthrough of the PR changes. Review main files, and shortly describe the changes in each file (up to 10 most important files). + items: + filename: + type: string + description: the relevant file full path + changes in file: + type: string + description: minimal and concise description of the changes in the relevant file -Don't repeat the prompt in the answer, and avoid outputting the 'type' and 'description' fields. + +Example output: +```yaml +PR Title: |- + ... +PR Type: + - Bug fix +PR Description: |- + ... +PR Main Files Walkthrough: + - ... + - ... +``` + +Make sure to output a valid YAML. Don't repeat the prompt in the answer, and avoid outputting the 'type' and 'description' fields. """ user="""PR Info: +Previous title: '{{title}}' +Previous description: '{{description}}' Branch: '{{branch}}' {%- if language %} @@ -52,6 +81,6 @@ The PR Git 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 +Response (should be a valid YAML, and nothing else): +```yaml """ diff --git a/pr_agent/settings/pr_reviewer_prompts.toml b/pr_agent/settings/pr_reviewer_prompts.toml index 613f1360..cdf7f731 100644 --- a/pr_agent/settings/pr_reviewer_prompts.toml +++ b/pr_agent/settings/pr_reviewer_prompts.toml @@ -7,6 +7,7 @@ Your task is to provide constructive and concise feedback for the PR, and also p - Suggestions should focus on improving the new added code lines. - Make sure not to provide suggestions repeating modifications already implemented in the new PR code (the '+' lines). {%- endif %} +- If needed, each YAML output should be in block scalar format ('|-') {%- if extra_instructions %} @@ -14,117 +15,121 @@ Extra instructions from the user: {{ extra_instructions }} {% endif %} -You must use the following JSON schema to format your answer: -```json -{ - "PR Analysis": { - "Main theme": { - "type": "string", - "description": "a short explanation of the PR" - }, - "Type of PR": { - "type": "string", - "enum": ["Bug fix", "Tests", "Refactoring", "Enhancement", "Documentation", "Other"] - }, +You must use the following YAML schema to format your answer: +```yaml +PR Analysis: + Main theme: + type: string + description: a short explanation of the PR + Type of PR: + type: string + enum: + - Bug fix + - Tests + - Refactoring + - Enhancement + - Documentation + - Other {%- if require_score %} - "Score": { - "type": "int", - "description": "Rate this PR on a scale of 0-100 (inclusive), where 0 means the worst possible PR code, and 100 means PR code of the highest quality, without any bugs or performance issues, that is ready to be merged immediately and run in production at scale." - }, + Score: + type: int + description: >- + Rate this PR on a scale of 0-100 (inclusive), where 0 means the worst + possible PR code, and 100 means PR code of the highest quality, without + any bugs or performance issues, that is ready to be merged immediately and + run in production at scale. {%- endif %} {%- if require_tests %} - "Relevant tests added": { - "type": "string", - "description": "yes\\no question: does this PR have relevant tests ?" - }, + Relevant tests added: + type: string + description: yes\\no question: does this PR have relevant tests ? {%- endif %} {%- if question_str %} - "Insights from user's answer": { - "type": "string", - "description": "shortly summarize the insights you gained from the user's answers to the questions" - }, + Insights from user's answer: + type: string + description: >- + shortly summarize the insights you gained from the user's answers to the questions {%- endif %} {%- if require_focused %} - "Focused PR": { - "type": "string", - "description": "Is this a focused PR, in the sense that all the PR code diff changes are united under a single focused theme ? If the theme is too broad, or the PR code diff changes are too scattered, then the PR is not focused. Explain your answer shortly." - } - }, + Focused PR: + type: string + description: >- + Is this a focused PR, in the sense that all the PR code diff changes are + united under a single focused theme ? If the theme is too broad, or the PR + code diff changes are too scattered, then the PR is not focused. Explain + your answer shortly. {%- endif %} - "PR Feedback": { - "General suggestions": { - "type": "string", - "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. Don't address PR title and description, or lack of tests. Explain your suggestions." - }, +PR Feedback: + General suggestions: + type: string + 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. Don't address PR title and description, or lack of tests. Explain your + suggestions. {%- if num_code_suggestions > 0 %} - "Code feedback": { - "type": "array", - "maxItems": {{ num_code_suggestions }}, - "uniqueItems": true, - "items": { - "relevant file": { - "type": "string", - "description": "the relevant file full path" - }, - "suggestion": { - "type": "string", - "description": "a concrete suggestion for meaningfully improving the new PR code. Also describe how, specifically, the suggestion can be applied to new PR code. Add tags with importance measure that matches each suggestion ('important' or 'medium'). Do not make suggestions for updating or adding docstrings, renaming PR title and description, or linter like. - }, - "relevant line": { - "type": "string", - "description": "a single code line taken from the relevant file, to which the suggestion applies. The line should be a '+' line. Make sure to output the line exactly as it appears in the relevant file" - } - } - }, + Code feedback: + type: array + maxItems: {{ num_code_suggestions }} + uniqueItems: true + items: + relevant file: + type: string + description: the relevant file full path + suggestion: + type: string + description: | + a concrete suggestion for meaningfully improving the new PR code. Also + describe how, specifically, the suggestion can be applied to new PR + code. Add tags with importance measure that matches each suggestion + ('important' or 'medium'). Do not make suggestions for updating or + adding docstrings, renaming PR title and description, or linter like. + relevant line: + type: string + description: | + a single code line taken from the relevant file, to which the suggestion applies. + The line should be a '+' line. + Make sure to output the line exactly as it appears in the relevant file {%- endif %} {%- if require_security %} - "Security concerns": { - "type": "string", - "description": "yes\\no question: does this PR code introduce possible security concerns or issues, like SQL injection, XSS, CSRF, and others ? If answered 'yes', explain your answer shortly" - ? explain your answer shortly" - } + Security concerns: + type: string + description: >- + yes\\no question: does this PR code introduce possible security concerns or + issues, like SQL injection, XSS, CSRF, and others ? If answered 'yes',explain your answer shortly {%- endif %} - } -} ``` Example output: -' -{ - "PR Analysis": - { - "Main theme": "xxx", - "Type of PR": "Bug fix", +```yaml +PR Analysis: + Main theme: xxx + Type of PR: Bug fix {%- if require_score %} - "Score": 89, -{%- endif %} -{%- if require_tests %} - "Relevant tests added": "No", + Score: 89 {%- endif %} + Relevant tests added: No {%- if require_focused %} - "Focused PR": "yes\\no, because ..." + Focused PR: no, because ... {%- endif %} - }, - "PR Feedback": - { - "General PR suggestions": "..., `xxx`...", +PR Feedback: + General PR suggestions: ... {%- if num_code_suggestions > 0 %} - "Code feedback": [ - { - "relevant file": "directory/xxx.py", - "suggestion": "xxx [important]", - "relevant line": "xxx", - }, - ... - ] + Code feedback: + - relevant file: |- + directory/xxx.py + suggestion: xxx [important] + relevant line: |- + xxx + ... {%- endif %} {%- if require_security %} - "Security concerns": "No, because ..." + Security concerns: No {%- endif %} - } -} -' +``` +Make sure to output a valid YAML. Use multi-line block scalar ('|') if needed. Don't repeat the prompt in the answer, and avoid outputting the 'type' and 'description' fields. """ @@ -158,6 +163,6 @@ The PR Git 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 +Response (should be a valid YAML, and nothing else): +```yaml """ diff --git a/pr_agent/tools/pr_description.py b/pr_agent/tools/pr_description.py index 7f39a8b8..d55dd55a 100644 --- a/pr_agent/tools/pr_description.py +++ b/pr_agent/tools/pr_description.py @@ -8,6 +8,7 @@ from jinja2 import Environment, StrictUndefined from pr_agent.algo.ai_handler import AiHandler from pr_agent.algo.pr_processing import get_pr_diff, retry_with_fallback_models from pr_agent.algo.token_handler import TokenHandler +from pr_agent.algo.utils import load_yaml from pr_agent.config_loader import get_settings from pr_agent.git_providers import get_git_provider from pr_agent.git_providers.git_provider import get_main_pr_language @@ -139,34 +140,45 @@ class PRDescription: - title: a string containing the PR title. - pr_body: a string containing the PR body in a markdown format. - pr_types: a list of strings containing the PR types. - - markdown_text: a string containing the AI prediction data in a markdown format. + - markdown_text: a string containing the AI prediction data in a markdown format. used for publishing a comment """ # Load the AI prediction data into a dictionary - data = json.loads(self.prediction) + data = load_yaml(self.prediction.strip()) # Initialization - markdown_text = pr_body = "" pr_types = [] # Iterate over the dictionary items and append the key and value to 'markdown_text' in a markdown format + markdown_text = "" for key, value in data.items(): markdown_text += f"## {key}\n\n" markdown_text += f"{value}\n\n" # If the 'PR Type' key is present in the dictionary, split its value by comma and assign it to 'pr_types' if 'PR Type' in data: - pr_types = data['PR Type'].split(',') + if type(data['PR Type']) == list: + pr_types = data['PR Type'] + elif type(data['PR Type']) == str: + pr_types = data['PR Type'].split(',') # Assign the value of the 'PR Title' key to 'title' variable and remove it from the dictionary title = data.pop('PR Title') # Iterate over the remaining dictionary items and append the key and value to 'pr_body' in a markdown format, # except for the items containing the word 'walkthrough' + pr_body = "" for key, value in data.items(): pr_body += f"## {key}:\n" if 'walkthrough' in key.lower(): - pr_body += f"{value}\n" + # for filename, description in value.items(): + for file in value: + filename = file['filename'].replace("'", "`") + description = file['changes in file'] + pr_body += f'`{filename}`: {description}\n' else: + # if the value is a list, join its items by comma + if type(value) == list: + value = ', '.join(v for v in value) pr_body += f"{value}\n\n___\n" if get_settings().config.verbosity_level >= 2: diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index 982f5000..fd6479ae 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -4,13 +4,15 @@ import logging from collections import OrderedDict from typing import List, Tuple +import yaml from jinja2 import Environment, StrictUndefined +from yaml import SafeLoader from pr_agent.algo.ai_handler import AiHandler from pr_agent.algo.pr_processing import get_pr_diff, retry_with_fallback_models, \ - find_line_number_of_relevant_line_in_file + find_line_number_of_relevant_line_in_file, clip_tokens from pr_agent.algo.token_handler import TokenHandler -from pr_agent.algo.utils import convert_to_markdown, try_fix_json +from pr_agent.algo.utils import convert_to_markdown, try_fix_json, try_fix_yaml, load_yaml from pr_agent.config_loader import get_settings from pr_agent.git_providers import get_git_provider from pr_agent.git_providers.git_provider import IncrementalPR, get_main_pr_language @@ -160,19 +162,17 @@ class PRReviewer: Prepare the PR review by processing the AI prediction and generating a markdown-formatted text that summarizes the feedback. """ - review = self.prediction.strip() - - try: - data = json.loads(review) - except json.decoder.JSONDecodeError: - data = try_fix_json(review) + data = load_yaml(self.prediction.strip()) # Move 'Security concerns' key to 'PR Analysis' section for better display pr_feedback = data.get('PR Feedback', {}) security_concerns = pr_feedback.get('Security concerns') - if security_concerns: + if security_concerns is not None: del pr_feedback['Security concerns'] - data.setdefault('PR Analysis', {})['Security concerns'] = security_concerns + if type(security_concerns) == bool and security_concerns == False: + data.setdefault('PR Analysis', {})['Security concerns'] = 'No security concerns found' + else: + data.setdefault('PR Analysis', {})['Security concerns'] = security_concerns # if 'Code feedback' in pr_feedback: @@ -183,6 +183,12 @@ class PRReviewer: del pr_feedback['Code feedback'] else: for suggestion in code_feedback: + if ('relevant file' in suggestion) and (not suggestion['relevant file'].startswith('``')): + suggestion['relevant file'] = f"``{suggestion['relevant file']}``" + + if 'relevant line' not in suggestion: + suggestion['relevant line'] = '' + relevant_line_str = suggestion['relevant line'].split('\n')[0] # removing '+' @@ -219,7 +225,7 @@ class PRReviewer: logging.info(f"Markdown response:\n{markdown_text}") if markdown_text == None or len(markdown_text) == 0: - markdown_text = review + markdown_text = "" return markdown_text @@ -230,11 +236,13 @@ class PRReviewer: if get_settings().pr_reviewer.num_code_suggestions == 0: return - review = self.prediction.strip() + review_text = self.prediction.strip() + review_text = review_text.removeprefix('```yaml').rstrip('`') try: - data = json.loads(review) - except json.decoder.JSONDecodeError: - data = try_fix_json(review) + data = yaml.load(review_text, Loader=SafeLoader) + except Exception as e: + logging.error(f"Failed to parse AI prediction: {e}") + data = try_fix_yaml(review_text) comments: List[str] = [] for suggestion in data.get('PR Feedback', {}).get('Code feedback', []): diff --git a/pyproject.toml b/pyproject.toml index 4ca0c0b6..2e8f2b5c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -42,7 +42,8 @@ dependencies = [ "atlassian-python-api==3.39.0", "GitPython~=3.1.32", "starlette-context==0.3.6", - "litellm~=0.1.351" + "litellm~=0.1.351", + "PyYAML==6.0" ] [project.urls] diff --git a/requirements.txt b/requirements.txt index 07a33514..ebea2b71 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,4 +11,7 @@ pytest~=7.4.0 aiohttp~=3.8.4 atlassian-python-api==3.39.0 GitPython~=3.1.32 +litellm~=0.1.351 +PyYAML==6.0 +starlette-context==0.3.6 litellm~=0.1.351 \ No newline at end of file diff --git a/tests/unittest/test_load_yaml.py b/tests/unittest/test_load_yaml.py new file mode 100644 index 00000000..a345aee2 --- /dev/null +++ b/tests/unittest/test_load_yaml.py @@ -0,0 +1,32 @@ + +# Generated by CodiumAI + +import pytest +from pr_agent.algo.utils import load_yaml + + +class TestLoadYaml: + # Tests that load_yaml loads a valid YAML string + def test_load_valid_yaml(self): + yaml_str = 'name: John Smith\nage: 35' + expected_output = {'name': 'John Smith', 'age': 35} + assert load_yaml(yaml_str) == expected_output + + def test_load_complicated_yaml(self): + yaml_str = \ +'''\ +PR Analysis: + Main theme: Enhancing the `/describe` command prompt by adding title and description + Type of PR: Enhancement + Relevant tests added: No + Focused PR: Yes, the PR is focused on enhancing the `/describe` command prompt. + +PR Feedback: + General suggestions: The PR seems to be well-structured and focused on a specific enhancement. However, it would be beneficial to add tests to ensure the new feature works as expected. + Code feedback: + - relevant file: pr_agent/settings/pr_description_prompts.toml + suggestion: Consider using a more descriptive variable name than 'user' for the command prompt. A more descriptive name would make the code more readable and maintainable. [medium] + relevant line: 'user="""PR Info:' + Security concerns: No''' + expected_output = {'PR Analysis': {'Main theme': 'Enhancing the `/describe` command prompt by adding title and description', 'Type of PR': 'Enhancement', 'Relevant tests added': False, 'Focused PR': 'Yes, the PR is focused on enhancing the `/describe` command prompt.'}, 'PR Feedback': {'General suggestions': 'The PR seems to be well-structured and focused on a specific enhancement. However, it would be beneficial to add tests to ensure the new feature works as expected.', 'Code feedback': [{'relevant file': 'pr_agent/settings/pr_description_prompts.toml', 'suggestion': "Consider using a more descriptive variable name than 'user' for the command prompt. A more descriptive name would make the code more readable and maintainable. [medium]", 'relevant line': 'user="""PR Info:'}], 'Security concerns': False}} + assert load_yaml(yaml_str) == expected_output