yaml
This commit is contained in:
mrT23
2023-08-09 08:50:15 +03:00
parent 7760f37dee
commit fe75e3f2ec
7 changed files with 241 additions and 135 deletions

View File

@ -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.lstrip('```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

View File

@ -392,10 +392,13 @@ class GithubProvider(GitProvider):
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

View File

@ -2,6 +2,7 @@
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 extra_instructions %}
@ -9,31 +10,57 @@ 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 +79,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
"""

View File

@ -14,118 +14,119 @@ 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 %}
}
}
'
```
Don't repeat the prompt in the answer, and avoid outputting the 'type' and 'description' fields.
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:
@ -158,6 +159,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
"""

View File

@ -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:

View File

@ -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
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.lstrip('```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', []):

View File

@ -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