formatting

This commit is contained in:
mrT23
2024-02-08 17:08:42 +02:00
parent a3f4c44632
commit fa077dc516
12 changed files with 115 additions and 199 deletions

View File

@ -42,7 +42,6 @@ To edit [configurations](./../pr_agent/settings/configuration.toml#L19) related
- `require_focused_review`: if set to true, the tool will add a section - 'is the PR a focused one'. Default is false. - `require_focused_review`: if set to true, the tool will add a section - 'is the PR a focused one'. Default is false.
- `require_score_review`: if set to true, the tool will add a section that scores the PR. Default is false. - `require_score_review`: if set to true, the tool will add a section that scores the PR. Default is false.
- `require_tests_review`: if set to true, the tool will add a section that checks if the PR contains tests. Default is true. - `require_tests_review`: if set to true, the tool will add a section that checks if the PR contains tests. Default is true.
- `require_security_review`: if set to true, the tool will add a section that checks if the PR contains security issues. Default is true.
- `require_estimate_effort_to_review`: if set to true, the tool will add a section that estimates the effort needed to review the PR. Default is true. - `require_estimate_effort_to_review`: if set to true, the tool will add a section that estimates the effort needed to review the PR. Default is true.
#### SOC2 ticket compliance 💎 #### SOC2 ticket compliance 💎
This sub-tool checks if the PR description properly contains a ticket to a project management system (e.g., Jira, Asana, Trello, etc.), as required by SOC2 compliance. If not, it will add a label to the PR: "Missing SOC2 ticket". This sub-tool checks if the PR description properly contains a ticket to a project management system (e.g., Jira, Asana, Trello, etc.), as required by SOC2 compliance. If not, it will add a label to the PR: "Missing SOC2 ticket".

View File

@ -36,15 +36,11 @@ def convert_to_markdown(output_data: dict, gfm_supported: bool=True) -> str:
Returns: Returns:
str: The markdown formatted text generated from the input dictionary. str: The markdown formatted text generated from the input dictionary.
""" """
markdown_text = ""
emojis = { emojis = {
"Main theme": "🎯", "Possible issues": "🔍",
"PR summary": "📝",
"Type of PR": "📌",
"Score": "🏅", "Score": "🏅",
"Relevant tests added": "🧪", "Relevant tests added": "🧪",
"Unrelated changes": "⚠️",
"Focused PR": "", "Focused PR": "",
"Security concerns": "🔒", "Security concerns": "🔒",
"General suggestions": "💡", "General suggestions": "💡",
@ -52,76 +48,62 @@ def convert_to_markdown(output_data: dict, gfm_supported: bool=True) -> str:
"Code feedback": "🤖", "Code feedback": "🤖",
"Estimated effort to review [1-5]": "⏱️", "Estimated effort to review [1-5]": "⏱️",
} }
markdown_text = ""
for key, value in output_data.items(): markdown_text += f"## PR Review\n\n"
markdown_text += "<table>\n<tr>\n"
markdown_text += """<td> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;<strong>PR&nbsp;feedback</strong>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; </td> <td></td></tr>"""
for key, value in output_data['review'].items():
if value is None or value == '' or value == {} or value == []: if value is None or value == '' or value == {} or value == []:
continue continue
if isinstance(value, dict): key_nice = key.replace('_', ' ').capitalize()
if key.lower() == 'pr review': emoji = emojis.get(key_nice, "")
markdown_text += f"## {key}\n\n" markdown_text += f"<tr><td> {emoji} {key_nice}</td><td>\n\n{value}\n\n</td></tr>\n"
markdown_text += "<table>\n<tr>\n" markdown_text += "</table>\n"
markdown_text += """<td> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;<strong>Feedback</strong>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; </td> <td></td></tr>"""
if 'code_feedback' in output_data:
if gfm_supported:
markdown_text += f"\n\n"
markdown_text += f"<details><summary> <strong>Code feedback:</strong></summary>\n\n"
else:
markdown_text += f"\n\n** Code feedback:**\n\n"
markdown_text += "<hr>"
for i, value in enumerate(output_data['code_feedback']):
if value is None or value == '' or value == {} or value == []:
continue
markdown_text += parse_code_suggestion(value, i, gfm_supported)+"\n\n"
if markdown_text.endswith('<hr>'):
markdown_text = markdown_text[:-4]
if gfm_supported:
markdown_text += f"</details>"
#print(markdown_text)
markdown_text += convert_to_markdown(value, gfm_supported)
markdown_text += "\n</table>\n"
elif isinstance(value, list):
emoji = emojis.get(key, "")
if key.lower() == 'code feedback':
if gfm_supported:
markdown_text += f"\n\n"
markdown_text += f"<details><summary> <strong>{ emoji } Code feedback:</strong></summary>"
else:
markdown_text += f"\n\n**{emoji} Code feedback:**\n\n"
else:
markdown_text += f"- {emoji} **{key}:**\n\n"
for i, item in enumerate(value):
if isinstance(item, dict) and key.lower() == 'code feedback':
markdown_text += parse_code_suggestion(item, i, gfm_supported)
elif item:
markdown_text += f" - {item}\n"
if key.lower() == 'code feedback':
if gfm_supported:
markdown_text += "</details>\n\n"
else:
markdown_text += "\n\n"
elif value != 'n/a':
emoji = emojis.get(key, "")
if key.lower() == 'general suggestions':
if gfm_supported:
markdown_text += f"\n\n<strong>{emoji} General suggestions:</strong> {value}\n"
else:
markdown_text += f"{emoji} **General suggestions:** {value}\n"
else:
markdown_text += f"<tr><td> {emoji} {key}</td><td> {value}</td></tr>\n"
return markdown_text return markdown_text
def parse_code_suggestion(code_suggestions: dict, i: int = 0, gfm_supported: bool = True) -> str: def parse_code_suggestion(code_suggestion: dict, i: int = 0, gfm_supported: bool = True) -> str:
""" """
Convert a dictionary of data into markdown format. Convert a dictionary of data into markdown format.
Args: Args:
code_suggestions (dict): A dictionary containing data to be converted to markdown format. code_suggestion (dict): A dictionary containing data to be converted to markdown format.
Returns: Returns:
str: A string containing the markdown formatted text generated from the input dictionary. str: A string containing the markdown formatted text generated from the input dictionary.
""" """
markdown_text = "" markdown_text = ""
if gfm_supported and 'relevant line' in code_suggestions: if gfm_supported and 'relevant_line' in code_suggestion:
if i == 0:
markdown_text += "<hr>"
markdown_text += '<table>' markdown_text += '<table>'
for sub_key, sub_value in code_suggestions.items(): for sub_key, sub_value in code_suggestion.items():
try: try:
if sub_key.lower() == 'relevant file': if sub_key.lower() == 'relevant_file':
relevant_file = sub_value.strip('`').strip('"').strip("'") relevant_file = sub_value.strip('`').strip('"').strip("'")
markdown_text += f"<tr><td>{sub_key}</td><td>{relevant_file}</td></tr>" markdown_text += f"<tr><td>relevant file</td><td>{relevant_file}</td></tr>"
# continue # continue
elif sub_key.lower() == 'suggestion': elif sub_key.lower() == 'suggestion':
markdown_text += (f"<tr><td>{sub_key} &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;</td>" markdown_text += (f"<tr><td>{sub_key} &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;</td>"
f"<td><br>\n\n**{sub_value.strip()}**\n<br></td></tr>") f"<td>\n\n<strong>\n\n{sub_value.strip()}\n\n</strong>\n</td></tr>")
elif sub_key.lower() == 'relevant line': elif sub_key.lower() == 'relevant_line':
markdown_text += f"<tr><td>relevant line</td>" markdown_text += f"<tr><td>relevant line</td>"
sub_value_list = sub_value.split('](') sub_value_list = sub_value.split('](')
relevant_line = sub_value_list[0].lstrip('`').lstrip('[') relevant_line = sub_value_list[0].lstrip('`').lstrip('[')
@ -137,7 +119,7 @@ def parse_code_suggestion(code_suggestions: dict, i: int = 0, gfm_supported: boo
markdown_text += '</table>' markdown_text += '</table>'
markdown_text += "<hr>" markdown_text += "<hr>"
else: else:
for sub_key, sub_value in code_suggestions.items(): for sub_key, sub_value in code_suggestion.items():
if isinstance(sub_value, dict): # "code example" if isinstance(sub_value, dict): # "code example"
markdown_text += f" - **{sub_key}:**\n" markdown_text += f" - **{sub_key}:**\n"
for code_key, code_value in sub_value.items(): # 'before' and 'after' code for code_key, code_value in sub_value.items(): # 'before' and 'after' code
@ -145,12 +127,12 @@ def parse_code_suggestion(code_suggestions: dict, i: int = 0, gfm_supported: boo
code_str_indented = textwrap.indent(code_str, ' ') code_str_indented = textwrap.indent(code_str, ' ')
markdown_text += f" - **{code_key}:**\n{code_str_indented}\n" markdown_text += f" - **{code_key}:**\n{code_str_indented}\n"
else: else:
if "relevant file" in sub_key.lower(): if "relevant_file" in sub_key.lower():
markdown_text += f"\n - **{sub_key}:** {sub_value} \n" markdown_text += f"\n - **{sub_key}:** {sub_value} \n"
else: else:
markdown_text += f" **{sub_key}:** {sub_value} \n" markdown_text += f" **{sub_key}:** {sub_value} \n"
if not gfm_supported: if not gfm_supported:
if "relevant line" not in sub_key.lower(): # nicer presentation if "relevant_line" not in sub_key.lower(): # nicer presentation
# markdown_text = markdown_text.rstrip('\n') + "\\\n" # works for gitlab # markdown_text = markdown_text.rstrip('\n') + "\\\n" # works for gitlab
markdown_text = markdown_text.rstrip('\n') + " \n" # works for gitlab and bitbucker markdown_text = markdown_text.rstrip('\n') + " \n" # works for gitlab and bitbucker
@ -347,7 +329,7 @@ def load_yaml(response_text: str, keys_fix_yaml: List[str] = []) -> dict:
def try_fix_yaml(response_text: str, keys_fix_yaml: List[str] = []) -> dict: def try_fix_yaml(response_text: str, keys_fix_yaml: List[str] = []) -> dict:
response_text_lines = response_text.split('\n') response_text_lines = response_text.split('\n')
keys = ['relevant line:', 'suggestion content:', 'relevant file:', 'existing code:', 'improved code:'] keys = ['relevant line:', 'suggestion content:', 'relevant_file:', 'existing code:', 'improved code:']
keys = keys + keys_fix_yaml keys = keys + keys_fix_yaml
# first fallback - try to convert 'relevant line: ...' to relevant line: |-\n ...' # first fallback - try to convert 'relevant line: ...' to relevant line: |-\n ...'
response_text_lines_copy = response_text_lines.copy() response_text_lines_copy = response_text_lines.copy()

View File

@ -240,8 +240,8 @@ class BitbucketProvider(GitProvider):
def generate_link_to_relevant_line_number(self, suggestion) -> str: def generate_link_to_relevant_line_number(self, suggestion) -> str:
try: try:
relevant_file = suggestion['relevant file'].strip('`').strip("'") relevant_file = suggestion['relevant_file'].strip('`').strip("'")
relevant_line_str = suggestion['relevant line'] relevant_line_str = suggestion['relevant_line']
if not relevant_line_str: if not relevant_line_str:
return "" return ""

View File

@ -246,8 +246,8 @@ class BitbucketServerProvider(GitProvider):
def generate_link_to_relevant_line_number(self, suggestion) -> str: def generate_link_to_relevant_line_number(self, suggestion) -> str:
try: try:
relevant_file = suggestion['relevant file'].strip('`').strip("'") relevant_file = suggestion['relevant_file'].strip('`').strip("'")
relevant_line_str = suggestion['relevant line'] relevant_line_str = suggestion['relevant_line']
if not relevant_line_str: if not relevant_line_str:
return "" return ""

View File

@ -603,8 +603,8 @@ class GithubProvider(GitProvider):
def generate_link_to_relevant_line_number(self, suggestion) -> str: def generate_link_to_relevant_line_number(self, suggestion) -> str:
try: try:
relevant_file = suggestion['relevant file'].strip('`').strip("'") relevant_file = suggestion['relevant_file'].strip('`').strip("'").strip('\n')
relevant_line_str = suggestion['relevant line'] relevant_line_str = suggestion['relevant_line'].strip('\n')
if not relevant_line_str: if not relevant_line_str:
return "" return ""

View File

@ -450,8 +450,8 @@ class GitLabProvider(GitProvider):
def generate_link_to_relevant_line_number(self, suggestion) -> str: def generate_link_to_relevant_line_number(self, suggestion) -> str:
try: try:
relevant_file = suggestion['relevant file'].strip('`').strip("'") relevant_file = suggestion['relevant_file'].strip('`').strip("'")
relevant_line_str = suggestion['relevant line'] relevant_line_str = suggestion['relevant_line']
if not relevant_line_str: if not relevant_line_str:
return "" return ""

View File

@ -9,7 +9,7 @@ verbosity_level=0 # 0,1,2
use_extra_bad_extensions=false use_extra_bad_extensions=false
use_repo_settings_file=true use_repo_settings_file=true
use_global_settings_file=true use_global_settings_file=true
ai_timeout=90 ai_timeout=120 # 2minutes
max_description_tokens = 500 max_description_tokens = 500
max_commits_tokens = 500 max_commits_tokens = 500
max_model_tokens = 32000 # Limits the maximum number of tokens that can be used by any model, regardless of the model's default capabilities. max_model_tokens = 32000 # Limits the maximum number of tokens that can be used by any model, regardless of the model's default capabilities.
@ -22,7 +22,6 @@ cli_mode=false
require_focused_review=false require_focused_review=false
require_score_review=false require_score_review=false
require_tests_review=true require_tests_review=true
require_security_review=true
require_estimate_effort_to_review=true require_estimate_effort_to_review=true
# soc2 # soc2
require_soc2_ticket=false require_soc2_ticket=false
@ -149,7 +148,6 @@ push_commands = [
--pr_reviewer.require_focused_review=false \ --pr_reviewer.require_focused_review=false \
--pr_reviewer.require_score_review=false \ --pr_reviewer.require_score_review=false \
--pr_reviewer.require_tests_review=false \ --pr_reviewer.require_tests_review=false \
--pr_reviewer.require_security_review=false \
--pr_reviewer.require_estimate_effort_to_review=false \ --pr_reviewer.require_estimate_effort_to_review=false \
--pr_reviewer.num_code_suggestions=0 \ --pr_reviewer.num_code_suggestions=0 \
--pr_reviewer.inline_code_comments=false \ --pr_reviewer.inline_code_comments=false \

View File

@ -91,7 +91,7 @@ labels:
{%- endif %} {%- endif %}
``` ```
Answer should be a valid YAML, and nothing else. Each YAML output MUST be after a newline, with proper indent, and block scalar indicator ('|-') Answer should be a valid YAML, and nothing else. Each YAML output MUST be after a newline, with proper indent, and block scalar indicator ('|')
""" """
user="""PR Info: user="""PR Info:

View File

@ -43,121 +43,71 @@ Extra instructions from the user:
{% endif %} {% endif %}
You must use the following YAML schema to format your answer: The output must be a YAML object equivalent to type $PRReview, according to the following Pydantic definitions:
```yaml =====
PR Review: class Review(BaseModel)
{%- if require_estimate_effort_to_review %}
estimated_effort_to_review_[1-5]: str = Field(description="Estimate, on a scale of 1-5 (inclusive), the time and effort required to review this PR by an experienced and knowledgeable developer. 1 means short and easy review , 5 means long and hard review. Take into account the size, complexity, quality, and the needed changes of the PR code diff. Explain your answer in a short and concise manner.")
{%- endif %}
{%- if require_score %} {%- if require_score %}
Score: score: str = Field(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.")
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 %} {%- endif %}
{%- if require_tests %} {%- if require_tests %}
Relevant tests added: relevant_tests_added: str = Field(description="yes\\no question: does this PR have relevant tests ?")
type: string
description: yes\\no question: does this PR have relevant tests ?
{%- endif %} {%- endif %}
{%- if question_str %} {%- if question_str %}
Insights from user's answer: insights_from_user_answers: str = Field(description="shortly summarize the insights you gained from the user's answers to the questions")
type: string
description: |-
shortly summarize the insights you gained from the user's answers to the questions
{%- endif %} {%- endif %}
{%- if require_focused %} {%- if require_focused %}
Focused PR: focused_pr: str = Field(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.")
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 %} {%- endif %}
{%- if require_estimate_effort_to_review %} possible_issues: str = Field(description="Does this PR code introduce clear issues, bugs, or performance concerns ? Answer 'No' if there are no clear issues. Answer 'Yes, ...' if there are issues, and explain your answer shortly.")
Estimated effort to review [1-5]: security_concerns: str = Field(description="does this PR code introduce possible vulnerabilities such as exposure of sensitive information (e.g., API keys, secrets, passwords), or security concerns like SQL injection, XSS, CSRF, and others ? Answer 'No' if there are no possible issues. Answer 'Yes, because ...' if there are security concerns or issues. Explain your answer shortly.")
type: string
description: >- class CodeSuggestion(BaseModel)
Estimate, on a scale of 1-5 (inclusive), the time and effort required to review this PR by an experienced and knowledgeable developer. 1 means short and easy review , 5 means long and hard review. relevant_file: str = Field(description="the relevant file full path")
Take into account the size, complexity, quality, and the needed changes of the PR code diff. language: str = Field(description="the language of the relevant file")
Explain your answer shortly (1-2 sentences). Use the format: '1, because ...' suggestion: str = Field(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.")
{%- endif %} relevant_line: str = Field(description="a single code line taken from the relevant file, to which the suggestion applies. The code line should start with a '+'. Make sure to output the line exactly as it appears in the relevant file")
{%- if num_code_suggestions > 0 or require_security %}
PR Feedback: class PRReview(BaseModel)
{% endif %} review: Review
{%- if num_code_suggestions > 0 %} code_feedback: List[CodeSuggestion]
Code feedback:
type: array
maxItems: {{ num_code_suggestions }}
uniqueItems: true
items:
relevant file:
type: string
description: the relevant file full path
language:
type: string
description: the language of the relevant file
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 code line should start with a '+'.
Make sure to output the line exactly as it appears in the relevant file
{%- endif %}
{%- if require_security %}
Security concerns:
type: string
description: >-
does this PR code introduce possible vulnerabilities such as exposure of sensitive information (e.g., API keys, secrets, passwords), or security concerns like SQL injection, XSS, CSRF, and others ? Answer 'No' if there are no possible issues.
Answer 'Yes, because ...' if there are security concerns or issues. Explain your answer shortly.
{%- endif %}
```
Example output: Example output:
```yaml ```yaml
PR Review: review:
{%- if require_score %}
Score: 89
{%- endif %}
Relevant tests added: |-
No
{%- if require_focused %}
Focused PR: no, because ...
{%- endif %}
{%- if require_estimate_effort_to_review %} {%- if require_estimate_effort_to_review %}
Estimated effort to review [1-5]: |- estimated_effort_to_review_[1-5]: |
3, because ... 3, because ...
{%- endif %} {%- endif %}
{%- if num_code_suggestions > 0 or require_security %} {%- if require_score %}
PR Feedback: score: 89
{% endif %}
{%- if num_code_suggestions > 0 %}
Code feedback:
- relevant file: |-
directory/xxx.py
language: |-
python
suggestion: |-
xxx [important]
relevant line: |-
xxx
...
{%- endif %} {%- endif %}
{%- if require_security %} relevant_tests_added: |
Security concerns: No No
{%- if require_focused %}
focused_pr: |
no, because ...
{%- endif %}
possible_issues: |
No
security_concerns: |
No
{%- if num_code_suggestions > 0 %}
code_feedback
- relevant_file: |
directory/xxx.py
language: |
python
suggestion: |
xxx [important]
relevant_line: |
xxx
{%- endif %} {%- endif %}
``` ```
Each YAML output MUST be after a newline, indented, with block scalar indicator ('|-'). Answer should be a valid YAML, and nothing else. Each YAML output MUST be after a newline, with proper indent, and block scalar indicator ('|')
Don't repeat the prompt in the answer, and avoid outputting the 'type' and 'description' fields.
""" """
user="""PR Info: user="""PR Info:

View File

@ -12,7 +12,8 @@ from pr_agent.algo.ai_handlers.base_ai_handler import BaseAiHandler
from pr_agent.algo.ai_handlers.litellm_ai_handler import LiteLLMAIHandler from pr_agent.algo.ai_handlers.litellm_ai_handler import LiteLLMAIHandler
from pr_agent.algo.pr_processing import get_pr_diff, retry_with_fallback_models 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.token_handler import TokenHandler
from pr_agent.algo.utils import convert_to_markdown, load_yaml, try_fix_yaml, set_custom_labels, get_user_labels from pr_agent.algo.utils import convert_to_markdown, load_yaml, try_fix_yaml, set_custom_labels, get_user_labels, \
ModelType
from pr_agent.config_loader import get_settings from pr_agent.config_loader import get_settings
from pr_agent.git_providers import get_git_provider from pr_agent.git_providers import get_git_provider
from pr_agent.git_providers.git_provider import IncrementalPR, get_main_pr_language from pr_agent.git_providers.git_provider import IncrementalPR, get_main_pr_language
@ -62,7 +63,6 @@ class PRReviewer:
"diff": "", # empty diff for initial calculation "diff": "", # empty diff for initial calculation
"require_score": get_settings().pr_reviewer.require_score_review, "require_score": get_settings().pr_reviewer.require_score_review,
"require_tests": get_settings().pr_reviewer.require_tests_review, "require_tests": get_settings().pr_reviewer.require_tests_review,
"require_security": get_settings().pr_reviewer.require_security_review,
"require_focused": get_settings().pr_reviewer.require_focused_review, "require_focused": get_settings().pr_reviewer.require_focused_review,
"require_estimate_effort_to_review": get_settings().pr_reviewer.require_estimate_effort_to_review, "require_estimate_effort_to_review": get_settings().pr_reviewer.require_estimate_effort_to_review,
'num_code_suggestions': get_settings().pr_reviewer.num_code_suggestions, 'num_code_suggestions': get_settings().pr_reviewer.num_code_suggestions,
@ -113,7 +113,7 @@ class PRReviewer:
if get_settings().config.publish_output: if get_settings().config.publish_output:
self.git_provider.publish_comment("Preparing review...", is_temporary=True) self.git_provider.publish_comment("Preparing review...", is_temporary=True)
await retry_with_fallback_models(self._prepare_prediction) await retry_with_fallback_models(self._prepare_prediction, model_type=ModelType.TURBO)
if not self.prediction: if not self.prediction:
self.git_provider.remove_initial_comment() self.git_provider.remove_initial_comment()
return None return None
@ -192,41 +192,30 @@ class PRReviewer:
""" """
data = load_yaml(self.prediction.strip()) data = load_yaml(self.prediction.strip())
# Move 'Security concerns' key to 'PR Analysis' section for better display if 'code_feedback' in data:
pr_feedback = data.get('PR Feedback', {}) code_feedback = data['code_feedback']
security_concerns = pr_feedback.get('Security concerns')
if security_concerns is not None:
del pr_feedback['Security concerns']
if type(security_concerns) == bool and security_concerns == False:
data.setdefault('PR Review', {})['Security concerns'] = 'No security concerns found'
else:
data.setdefault('PR Review', {})['Security concerns'] = security_concerns
#
if 'Code feedback' in pr_feedback:
code_feedback = pr_feedback['Code feedback']
# Filter out code suggestions that can be submitted as inline comments # Filter out code suggestions that can be submitted as inline comments
if get_settings().pr_reviewer.inline_code_comments: if get_settings().pr_reviewer.inline_code_comments:
del pr_feedback['Code feedback'] del data['code_feedback']
else: else:
for suggestion in code_feedback: for suggestion in code_feedback:
if ('relevant file' in suggestion) and (not suggestion['relevant file'].startswith('``')): if ('relevant_file' in suggestion) and (not suggestion['relevant_file'].startswith('``')):
suggestion['relevant file'] = f"``{suggestion['relevant file']}``" suggestion['relevant_file'] = f"``{suggestion['relevant_file']}``"
if 'relevant line' not in suggestion: if 'relevant_line' not in suggestion:
suggestion['relevant line'] = '' suggestion['relevant_line'] = ''
relevant_line_str = suggestion['relevant line'].split('\n')[0] relevant_line_str = suggestion['relevant_line'].split('\n')[0]
# removing '+' # removing '+'
suggestion['relevant line'] = relevant_line_str.lstrip('+').strip() suggestion['relevant_line'] = relevant_line_str.lstrip('+').strip()
# try to add line numbers link to code suggestions # try to add line numbers link to code suggestions
if hasattr(self.git_provider, 'generate_link_to_relevant_line_number'): if hasattr(self.git_provider, 'generate_link_to_relevant_line_number'):
link = self.git_provider.generate_link_to_relevant_line_number(suggestion) link = self.git_provider.generate_link_to_relevant_line_number(suggestion)
if link: if link:
suggestion['relevant line'] = f"[{suggestion['relevant line']}]({link})" suggestion['relevant_line'] = f"[{suggestion['relevant_line']}]({link})"
else: else:
pass pass
@ -275,8 +264,8 @@ class PRReviewer:
data = load_yaml(self.prediction.strip()) data = load_yaml(self.prediction.strip())
comments: List[str] = [] comments: List[str] = []
for suggestion in data.get('PR Feedback', {}).get('Code feedback', []): for suggestion in data.get('PR Feedback', {}).get('Code feedback', []):
relevant_file = suggestion.get('relevant file', '').strip() relevant_file = suggestion.get('relevant_file', '').strip()
relevant_line_in_file = suggestion.get('relevant line', '').strip() relevant_line_in_file = suggestion.get('relevant_line', '').strip()
content = suggestion.get('suggestion', '') content = suggestion.get('suggestion', '')
if not relevant_file or not relevant_line_in_file or not content: if not relevant_file or not relevant_line_in_file or not content:
get_logger().info("Skipping inline comment with missing file/line/content") get_logger().info("Skipping inline comment with missing file/line/content")
@ -376,12 +365,12 @@ class PRReviewer:
try: try:
review_labels = [] review_labels = []
if get_settings().pr_reviewer.enable_review_labels_effort: if get_settings().pr_reviewer.enable_review_labels_effort:
estimated_effort = data['PR Review']['Estimated effort to review [1-5]'] estimated_effort = data['review']['estimated_effort_to_review_[1-5]']
estimated_effort_number = int(estimated_effort.split(',')[0]) estimated_effort_number = int(estimated_effort.split(',')[0])
if 1 <= estimated_effort_number <= 5: # 1, because ... if 1 <= estimated_effort_number <= 5: # 1, because ...
review_labels.append(f'Review effort [1-5]: {estimated_effort_number}') review_labels.append(f'Review effort [1-5]: {estimated_effort_number}')
if get_settings().pr_reviewer.enable_review_labels_security: if get_settings().pr_reviewer.enable_review_labels_security:
security_concerns = data['PR Review']['Security concerns'] # yes, because ... security_concerns = data['review']['security_concerns'] # yes, because ...
security_concerns_bool = 'yes' in security_concerns.lower() or 'true' in security_concerns.lower() security_concerns_bool = 'yes' in security_concerns.lower() or 'true' in security_concerns.lower()
if security_concerns_bool: if security_concerns_bool:
review_labels.append('Possible security concern') review_labels.append('Possible security concern')

View File

@ -49,7 +49,6 @@ class TestConvertToMarkdown:
'Main theme': 'Test', 'Main theme': 'Test',
'Type of PR': 'Test type', 'Type of PR': 'Test type',
'Relevant tests added': 'no', 'Relevant tests added': 'no',
'Unrelated changes': 'n/a', # won't be included in the output
'Focused PR': 'Yes', 'Focused PR': 'Yes',
'General PR suggestions': 'general suggestion...', 'General PR suggestions': 'general suggestion...',
'Code feedback': [ 'Code feedback': [
@ -87,7 +86,6 @@ class TestConvertToMarkdown:
'Main theme': {}, 'Main theme': {},
'Type of PR': {}, 'Type of PR': {},
'Relevant tests added': {}, 'Relevant tests added': {},
'Unrelated changes': {},
'Focused PR': {}, 'Focused PR': {},
'General PR suggestions': {}, 'General PR suggestions': {},
'Code suggestions': {} 'Code suggestions': {}

View File

@ -34,7 +34,7 @@ PR Feedback:
with pytest.raises(ScannerError): with pytest.raises(ScannerError):
yaml.safe_load(yaml_str) yaml.safe_load(yaml_str)
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: aaa'}], 'Security concerns': False}} 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: aaa'}], 'Security concerns': False}}
assert load_yaml(yaml_str) == expected_output assert load_yaml(yaml_str) == expected_output
def test_load_invalid_yaml2(self): def test_load_invalid_yaml2(self):
@ -45,7 +45,7 @@ PR Feedback:
with pytest.raises(ScannerError): with pytest.raises(ScannerError):
yaml.safe_load(yaml_str) yaml.safe_load(yaml_str)
expected_output =[{'relevant file': 'src/app.py:', expected_output =[{'relevant_file': 'src/app.py:',
'suggestion content': 'The print statement is outside inside the if __name__ ==: '}] 'suggestion content': 'The print statement is outside inside the if __name__ ==: '}]
assert load_yaml(yaml_str) == expected_output assert load_yaml(yaml_str) == expected_output