Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

style: refine field descriptions in KeyIssuesComponentLink model #1262

Merged
merged 3 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 9 additions & 11 deletions pr_agent/algo/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ def convert_to_markdown_v2(output_data: dict,

emojis = {
"Can be split": "🔀",
"Possible issues": "⚡",
"Key issues to review": "⚡",
"Recommended focus areas for review": "⚡",
"Score": "🏅",
"Relevant tests": "🧪",
"Focused PR": "✨",
Expand Down Expand Up @@ -192,23 +192,21 @@ def convert_to_markdown_v2(output_data: dict,
if is_value_no(value):
if gfm_supported:
markdown_text += f"<tr><td>"
markdown_text += f"{emoji}&nbsp;<strong>No key issues to review</strong>"
markdown_text += f"{emoji}&nbsp;<strong>No major issues detected</strong>"
markdown_text += f"</td></tr>\n"
else:
markdown_text += f"### {emoji} No key issues to review\n\n"
markdown_text += f"### {emoji} No major issues detected\n\n"
else:
# issues = value.split('\n- ')
issues =value
# for i, _ in enumerate(issues):
# issues[i] = issues[i].strip().strip('-').strip()
issues = value
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Consider using a more descriptive variable name instead of 'issues' to improve code readability. For example, 'focus_areas' or 'review_points' would better reflect the content and purpose of the list. [maintainability, importance: 5]

Suggested change
issues = value
focus_areas = value

if gfm_supported:
markdown_text += f"<tr><td>"
markdown_text += f"{emoji}&nbsp;<strong>{key_nice}</strong><br><br>\n\n"
# markdown_text += f"{emoji}&nbsp;<strong>{key_nice}</strong><br><br>\n\n"
markdown_text += f"{emoji}&nbsp;<strong>Recommended focus areas for review</strong><br><br>\n\n"
else:
markdown_text += f"### {emoji} Key issues to review\n\n#### \n"
markdown_text += f"### {emoji} Recommended focus areas for review\n\n#### \n"
for i, issue in enumerate(issues):
try:
if not issue:
if not issue or not isinstance(issue, dict):
continue
mrT23 marked this conversation as resolved.
Show resolved Hide resolved
relevant_file = issue.get('relevant_file', '').strip()
issue_header = issue.get('issue_header', '').strip()
Expand All @@ -223,7 +221,7 @@ def convert_to_markdown_v2(output_data: dict,
issue_str = f"[**{issue_header}**]({reference_link})\n\n{issue_content}\n\n"
markdown_text += f"{issue_str}\n\n"
except Exception as e:
get_logger().exception(f"Failed to process key issues to review: {e}")
get_logger().exception(f"Failed to process 'Recommended focus areas for review': {e}")
if gfm_supported:
markdown_text += f"</td></tr>\n"
else:
Expand Down
8 changes: 4 additions & 4 deletions pr_agent/settings/pr_reviewer_prompts.toml
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,10 @@ class SubPR(BaseModel):

class KeyIssuesComponentLink(BaseModel):
relevant_file: str = Field(description="The full file path of the relevant file")
issue_header: str = Field(description="one or two word title for the the issue. For example: 'Possible Bug', 'Performance Issue', 'Code Smell', etc.")
issue_content: str = Field(description="a short and concise description of the issue that needs to be reviewed")
start_line: int = Field(description="the start line that corresponds to this issue in the relevant file")
end_line: int = Field(description="the end line that corresponds to this issue in the relevant file")
issue_header: str = Field(description="One or two word title for the the issue. For example: 'Possible Bug', 'Performance Issue', 'Code Smell', etc.")
issue_content: str = Field(description="A short and concise summary of what should be further inspected and validated during the PR review process for this issue. Don't state line numbers here")
start_line: int = Field(description="The start line that corresponds to this issue in the relevant file")
end_line: int = Field(description="The end line that corresponds to this issue in the relevant file")
mrT23 marked this conversation as resolved.
Show resolved Hide resolved

class Review(BaseModel):
{%- if require_estimate_effort_to_review %}
Expand Down
2 changes: 1 addition & 1 deletion tests/unittest/test_convert_to_markdown.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def test_simple_dictionary_input(self):
'relevant_line': '[return ""](https://github.com/Codium-ai/pr-agent-pro/pull/102/files#diff-52d45f12b836f77ed1aef86e972e65404634ea4e2a6083fb71a9b0f9bb9e062fR199)'}]}


expected_output = f'{PRReviewHeader.REGULAR.value} 🔍\n\nHere are some key observations to aid the review process:\n\n<table>\n<tr><td>⏱️&nbsp;<strong>Estimated effort to review</strong>: 1 🔵⚪⚪⚪⚪</td></tr>\n<tr><td>🧪&nbsp;<strong>No relevant tests</strong></td></tr>\n<tr><td>⚡&nbsp;<strong>Possible issues</strong>: No\n</td></tr>\n<tr><td>🔒&nbsp;<strong>No security concerns identified</strong></td></tr>\n</table>\n\n\n<details><summary> <strong>Code feedback:</strong></summary>\n\n<hr><table><tr><td>relevant file</td><td>pr_agent/git_providers/git_provider.py\n</td></tr><tr><td>suggestion &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;</td><td>\n\n<strong>\n\nConsider raising an exception or logging a warning when \'pr_url\' attribute is not found. This can help in debugging issues related to the absence of \'pr_url\' in instances where it\'s expected. [important]\n\n</strong>\n</td></tr><tr><td>relevant line</td><td><a href=\'https://github.com/Codium-ai/pr-agent-pro/pull/102/files#diff-52d45f12b836f77ed1aef86e972e65404634ea4e2a6083fb71a9b0f9bb9e062fR199\'>return ""</a></td></tr></table><hr>\n\n</details>'
expected_output = f'{PRReviewHeader.REGULAR.value} 🔍\n\nHere are some key observations to aid the review process:\n\n<table>\n<tr><td>⏱️&nbsp;<strong>Estimated effort to review</strong>: 1 🔵⚪⚪⚪⚪</td></tr>\n<tr><td>🧪&nbsp;<strong>No relevant tests</strong></td></tr>\n<tr><td>&nbsp;<strong>Possible issues</strong>: No\n</td></tr>\n<tr><td>🔒&nbsp;<strong>No security concerns identified</strong></td></tr>\n</table>\n\n\n<details><summary> <strong>Code feedback:</strong></summary>\n\n<hr><table><tr><td>relevant file</td><td>pr_agent/git_providers/git_provider.py\n</td></tr><tr><td>suggestion &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;</td><td>\n\n<strong>\n\nConsider raising an exception or logging a warning when \'pr_url\' attribute is not found. This can help in debugging issues related to the absence of \'pr_url\' in instances where it\'s expected. [important]\n\n</strong>\n</td></tr><tr><td>relevant line</td><td><a href=\'https://github.com/Codium-ai/pr-agent-pro/pull/102/files#diff-52d45f12b836f77ed1aef86e972e65404634ea4e2a6083fb71a9b0f9bb9e062fR199\'>return ""</a></td></tr></table><hr>\n\n</details>'

assert convert_to_markdown_v2(input_data).strip() == expected_output.strip()

Expand Down
Loading