diff --git a/pr_agent/algo/ai_handler.py b/pr_agent/algo/ai_handler.py index 4e1f86e3..1a12564b 100644 --- a/pr_agent/algo/ai_handler.py +++ b/pr_agent/algo/ai_handler.py @@ -48,8 +48,6 @@ class AiHandler: litellm.replicate_key = get_settings().replicate.key if get_settings().get("HUGGINGFACE.KEY", None): litellm.huggingface_key = get_settings().huggingface.key - if get_settings().get("HUGGINGFACE.KEY", None): - litellm.huggingface_key = get_settings().huggingface.key except AttributeError as e: raise ValueError("OpenAI key is required") from e diff --git a/pr_agent/algo/pr_processing.py b/pr_agent/algo/pr_processing.py index 29709d29..1c34e603 100644 --- a/pr_agent/algo/pr_processing.py +++ b/pr_agent/algo/pr_processing.py @@ -24,7 +24,7 @@ OUTPUT_BUFFER_TOKENS_HARD_THRESHOLD = 600 PATCH_EXTRA_LINES = 3 def get_pr_diff(git_provider: GitProvider, token_handler: TokenHandler, model: str, - add_line_numbers_to_hunks: bool = True, disable_extra_lines: bool = True) -> str: + add_line_numbers_to_hunks: bool = False, disable_extra_lines: bool = False) -> str: """ Returns a string with the diff of the pull request, applying diff minimization techniques if needed. diff --git a/pr_agent/algo/utils.py b/pr_agent/algo/utils.py index 2139203f..4d09b6e7 100644 --- a/pr_agent/algo/utils.py +++ b/pr_agent/algo/utils.py @@ -32,33 +32,37 @@ def convert_to_markdown(output_data: dict) -> str: emojis = { "Main theme": "๐ŸŽฏ", + "PR summary": "๐Ÿ“", "Type of PR": "๐Ÿ“Œ", "Score": "๐Ÿ…", "Relevant tests added": "๐Ÿงช", "Unrelated changes": "โš ๏ธ", "Focused PR": "โœจ", "Security concerns": "๐Ÿ”’", - "General PR suggestions": "๐Ÿ’ก", + "General suggestions": "๐Ÿ’ก", "Insights from user's answers": "๐Ÿ“", "Code feedback": "๐Ÿค–", } for key, value in output_data.items(): - if not value: + if value is None or value == '' or value == {}: continue if isinstance(value, dict): markdown_text += f"## {key}\n\n" markdown_text += convert_to_markdown(value) elif isinstance(value, list): - if key.lower() == 'code feedback': - markdown_text += "\n" # just looks nicer with additional line breaks emoji = emojis.get(key, "") - markdown_text += f"- {emoji} **{key}:**\n\n" + if key.lower() == 'code feedback': + markdown_text += f"\n\n- **
{ emoji } Code feedback:**\n\n" + else: + markdown_text += f"- {emoji} **{key}:**\n\n" for item in value: if isinstance(item, dict) and key.lower() == 'code feedback': markdown_text += parse_code_suggestion(item) elif item: markdown_text += f" - {item}\n" + if key.lower() == 'code feedback': + markdown_text += "
\n\n" elif value != 'n/a': emoji = emojis.get(key, "") markdown_text += f"- {emoji} **{key}:** {value}\n" diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index 2f9a49ce..a58c3b88 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -13,11 +13,11 @@ max_commits_tokens = 500 litellm_debugger=false [pr_reviewer] # /review # -require_focused_review=true +require_focused_review=false require_score_review=false require_tests_review=true require_security_review=true -num_code_suggestions=3 +num_code_suggestions=4 inline_code_comments = false ask_and_reflect=false automatic_review=true diff --git a/pr_agent/settings/pr_code_suggestions_prompts.toml b/pr_agent/settings/pr_code_suggestions_prompts.toml index a65b546f..4e4b57e5 100644 --- a/pr_agent/settings/pr_code_suggestions_prompts.toml +++ b/pr_agent/settings/pr_code_suggestions_prompts.toml @@ -36,7 +36,7 @@ Specific instructions: - Provide the exact line number range (inclusive) for each issue. - Assume there is additional relevant code, that is not included in the diff. - Provide up to {{ num_code_suggestions }} code suggestions. -- Avoid making suggestions that have already been implemented in the PR code. For example, if you want to add logs or change a variable to const, or anything else, make sure it isn't already in the '__new hunk__' code. +- Avoid making suggestions that have already been implemented in the PR code. For example, if you want to add logs, or change a variable to const, or anything else, make sure it isn't already in the '__new hunk__' code. - Don't suggest to add docstring or type hints. {%- if extra_instructions %} diff --git a/pr_agent/settings/pr_reviewer_prompts.toml b/pr_agent/settings/pr_reviewer_prompts.toml index b65d62e4..7c21f433 100644 --- a/pr_agent/settings/pr_reviewer_prompts.toml +++ b/pr_agent/settings/pr_reviewer_prompts.toml @@ -1,13 +1,36 @@ [pr_review_prompt] -system="""You are CodiumAI-PR-Reviewer, a language model designed to review git pull requests. -Your task is to provide constructive and concise feedback for the PR, and also provide meaningful code suggestions to improve the new PR code (the '+' lines). +system="""You are PR-Reviewer, a language model designed to review git pull requests. +Your task is to provide constructive and concise feedback for the PR, and also provide meaningful code suggestions. + +Example PR Diff input: +' +## src/file1.py + +@@ -12,5 +12,5 @@ def func1(): +code line that already existed in the file... +code line that already existed in the file.... +-code line that was removed in the PR ++new code line added in the PR + code line that already existed in the file... + code line that already existed in the file... + +@@ ... @@ def func2(): +... + + +## src/file2.py +... +' + +Thre review should focus on new code added in the PR (lines starting with '+'), and not on code that already existed in the file (lines starting with '-', or without prefix). + {%- if num_code_suggestions > 0 %} - Provide up to {{ num_code_suggestions }} code suggestions. -- Try to focus on the most important suggestions, like fixing code problems, issues and bugs. As a second priority, provide suggestions for meaningful code improvements, like performance, vulnerability, modularity, and best practices. -- 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). +- Focus on important suggestions like fixing code problems, issues and bugs. As a second priority, provide suggestions for meaningful code improvements, like performance, vulnerability, modularity, and best practices. +- Avoid making suggestions that have already been implemented in the PR code. For example, if you want to add logs, or change a variable to const, or anything else, make sure it isn't already in the PR code. +- Don't suggest to add docstring or type hints. +- Suggestions should focus on improving the new code added in the PR (lines starting with '+') {%- endif %} -- If needed, each YAML output should be in block scalar format ('|-') {%- if extra_instructions %} @@ -21,6 +44,9 @@ PR Analysis: Main theme: type: string description: a short explanation of the PR + PR summary: + type: string + description: summary of the PR in 2-3 sentences. Type of PR: type: string enum: @@ -33,7 +59,7 @@ PR Analysis: {%- if require_score %} Score: type: int - description: >- + 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 @@ -47,13 +73,13 @@ PR Analysis: {%- if question_str %} Insights from user's answer: type: string - description: >- + description: |- shortly summarize the insights you gained from the user's answers to the questions {%- endif %} {%- if require_focused %} Focused PR: type: string - description: >- + 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 @@ -62,12 +88,11 @@ PR Analysis: PR Feedback: General suggestions: type: string - description: >- + 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. Don't address PR title and description, or lack of tests. Explain your suggestions. {%- if num_code_suggestions > 0 %} Code feedback: type: array @@ -79,7 +104,7 @@ PR Feedback: description: the relevant file full path suggestion: type: string - description: | + 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 @@ -87,9 +112,9 @@ PR Feedback: adding docstrings, renaming PR title and description, or linter like. relevant line: type: string - description: | + description: |- a single code line taken from the relevant file, to which the suggestion applies. - The line should be a '+' line. + The code line should start with a '+'. Make sure to output the line exactly as it appears in the relevant file {%- endif %} {%- if require_security %} @@ -104,22 +129,29 @@ PR Feedback: Example output: ```yaml PR Analysis: - Main theme: xxx - Type of PR: Bug fix + Main theme: |- + xxx + PR summary: |- + xxx + Type of PR: |- + Bug fix {%- if require_score %} Score: 89 {%- endif %} - Relevant tests added: No + Relevant tests added: |- + No {%- if require_focused %} Focused PR: no, because ... {%- endif %} PR Feedback: - General PR suggestions: ... + General PR suggestions: |- + ... {%- if num_code_suggestions > 0 %} Code feedback: - relevant file: |- directory/xxx.py - suggestion: xxx [important] + suggestion: |- + xxx [important] relevant line: |- xxx ... @@ -129,7 +161,7 @@ PR Feedback: {%- endif %} ``` -Make sure to output a valid YAML. Use multi-line block scalar ('|') if needed. +Each YAML output MUST be after a newline, indented, with block scalar indicator ('|-'). Don't repeat the prompt in the answer, and avoid outputting the 'type' and 'description' fields. """ @@ -161,7 +193,7 @@ 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. +Note that lines in the diff body are prefixed with a symbol that represents the type of change: '-' for deletions, '+' for additions. Focus on the '+' lines. Response (should be a valid YAML, and nothing else): ```yaml diff --git a/pyproject.toml b/pyproject.toml index 802cd0d8..9a945dca 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -42,7 +42,7 @@ dependencies = [ "atlassian-python-api==3.39.0", "GitPython~=3.1.32", "starlette-context==0.3.6", - "litellm~=0.1.351", + "litellm~=0.1.445", "PyYAML==6.0", "boto3~=1.28.25" ] diff --git a/tests/unittest/test_convert_to_markdown.py b/tests/unittest/test_convert_to_markdown.py index 4463513f..bb6f2268 100644 --- a/tests/unittest/test_convert_to_markdown.py +++ b/tests/unittest/test_convert_to_markdown.py @@ -67,33 +67,11 @@ class TestConvertToMarkdown: ] } expected_output = """\ -- ๐ŸŽฏ **Main theme:** Test -- ๐Ÿ“Œ **Type of PR:** Test type -- ๐Ÿงช **Relevant tests added:** no -- โœจ **Focused PR:** Yes -- ๐Ÿ’ก **General PR suggestions:** general suggestion... - -- ๐Ÿค– **Code feedback:** - - - **Code example:** - - **Before:** - ``` - Code before - ``` - - **After:** - ``` - Code after - ``` - - - **Code example:** - - **Before:** - ``` - Code before 2 - ``` - - **After:** - ``` - Code after 2 - ``` +- ๐ŸŽฏ **Main theme:** Test\n\ +- ๐Ÿ“Œ **Type of PR:** Test type\n\ +- ๐Ÿงช **Relevant tests added:** no\n\ +- โœจ **Focused PR:** Yes\n\ +- **General PR suggestions:** general suggestion...\n\n\n- **
๐Ÿค– Code feedback:**\n\n - **Code example:**\n - **Before:**\n ```\n Code before\n ```\n - **After:**\n ```\n Code after\n ```\n\n - **Code example:**\n - **Before:**\n ```\n Code before 2\n ```\n - **After:**\n ```\n Code after 2\n ```\n\n
\ """ assert convert_to_markdown(input_data).strip() == expected_output.strip() @@ -113,5 +91,5 @@ class TestConvertToMarkdown: 'General PR suggestions': {}, 'Code suggestions': {} } - expected_output = "" + expected_output = '' assert convert_to_markdown(input_data).strip() == expected_output.strip()