-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add review risk assessment and optional repository review rules #2391
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
40e5188
104a696
5a74da3
fc8ee1d
d8cbb89
e237f87
13136c6
03e0a0d
2d099fa
71011b3
666a265
8c93ceb
15d8940
20a256d
b71e7be
b97e833
983d227
9298146
20673da
d2f6982
6020941
ca9ab5e
c133868
b2b3f6a
fecf295
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,6 +81,12 @@ require_security_review=true | |
| require_estimate_contribution_time_cost=false | ||
| require_todo_scan=false | ||
| require_ticket_analysis_review=true | ||
| require_risk_assessment=true | ||
| require_merge_recommendation=true | ||
| require_priority_files=true | ||
|
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
Outdated
|
||
| enable_review_rules=true | ||
| review_rules_paths=[".pr_agent/review_rules.md",".github/review_rules.md","docs/review_rules.md"] | ||
| max_review_rules_tokens=1200 | ||
|
Comment on lines
+84
to
+89
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. .pr_agent.toml missing new keys New /review configuration knobs were added to pr_agent/settings/configuration.toml, but the mirrored root .pr_agent.toml was not updated to include these options, risking divergence and user confusion about available/default behavior. This violates the mirror-sync requirement when behavior/config surface area changes. Agent Prompt
|
||
| # general options | ||
| publish_output_no_suggestions=true # Set to "false" if you only need the reviewer's remarks (not labels, not "security audit", etc.) and want to avoid noisy "No major issues detected" comments. | ||
| persistent_comment=true | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,9 @@ | ||
| import copy | ||
| import datetime | ||
| import traceback | ||
|
|
||
|
|
||
|
|
||
| from collections import OrderedDict | ||
|
Comment on lines
+4
to
5
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2. Extra blank lines in imports The PR introduces three consecutive blank lines within the import block, which is likely to fail Ruff/PEP8 blank-line rules and create unnecessary diff noise. This can break linting CI and violates the repository formatting conventions. Agent Prompt
|
||
| from functools import partial | ||
| from typing import List, Tuple | ||
|
|
@@ -13,7 +16,7 @@ | |
| get_pr_diff, | ||
| retry_with_fallback_models) | ||
| from pr_agent.algo.token_handler import TokenHandler | ||
| from pr_agent.algo.utils import (ModelType, PRReviewHeader, | ||
| from pr_agent.algo.utils import (ModelType, PRReviewHeader, clip_tokens, | ||
| convert_to_markdown_v2, github_action_output, | ||
| load_yaml, show_relevant_configurations) | ||
| from pr_agent.config_loader import get_settings | ||
|
|
@@ -66,6 +69,8 @@ def __init__(self, pr_url: str, is_answer: bool = False, is_auto: bool = False, | |
| answer_str, question_str = self._get_user_answers() | ||
| self.pr_description, self.pr_description_files = ( | ||
| self.git_provider.get_pr_description(split_changes_walkthrough=True)) | ||
| self.review_rules = self._get_review_rules() | ||
|
|
||
| if (self.pr_description_files and get_settings().get("config.is_auto_command", False) and | ||
| get_settings().get("config.enable_ai_metadata", False)): | ||
| add_ai_metadata_to_diff_files(self.git_provider, self.pr_description_files) | ||
|
|
@@ -85,6 +90,9 @@ def __init__(self, pr_url: str, is_answer: bool = False, is_auto: bool = False, | |
| "require_score": get_settings().pr_reviewer.require_score_review, | ||
| "require_tests": get_settings().pr_reviewer.require_tests_review, | ||
| "require_estimate_effort_to_review": get_settings().pr_reviewer.require_estimate_effort_to_review, | ||
| "require_risk_assessment": get_settings().pr_reviewer.get("require_risk_assessment", False), | ||
| "require_merge_recommendation": get_settings().pr_reviewer.get("require_merge_recommendation", False), | ||
| "require_priority_files": get_settings().pr_reviewer.get("require_priority_files", False), | ||
| "require_estimate_contribution_time_cost": get_settings().pr_reviewer.require_estimate_contribution_time_cost, | ||
| 'require_can_be_split_review': get_settings().pr_reviewer.require_can_be_split_review, | ||
| 'require_security_review': get_settings().pr_reviewer.require_security_review, | ||
|
|
@@ -99,6 +107,8 @@ def __init__(self, pr_url: str, is_answer: bool = False, is_auto: bool = False, | |
| "related_tickets": get_settings().get('related_tickets', []), | ||
| 'duplicate_prompt_examples': get_settings().config.get('duplicate_prompt_examples', False), | ||
| "date": datetime.datetime.now().strftime('%Y-%m-%d'), | ||
| "review_rules": self.review_rules, | ||
|
|
||
| } | ||
|
|
||
| self.token_handler = TokenHandler( | ||
|
|
@@ -116,6 +126,42 @@ def parse_incremental(self, args: List[str]): | |
| is_incremental = True | ||
| incremental = IncrementalPR(is_incremental) | ||
| return incremental | ||
|
|
||
|
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| def _get_review_rules(self) -> str: | ||
| if not get_settings().pr_reviewer.get("enable_review_rules", False): | ||
| return "" | ||
|
|
||
| rule_paths = get_settings().pr_reviewer.get("review_rules_paths", []) or [] | ||
| if isinstance(rule_paths, str): | ||
| rule_paths = [rule_paths] | ||
|
|
||
|
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
|
||
| ref = getattr(getattr(self.git_provider, 'pr', None), 'head', None) | ||
| ref = getattr(ref, 'sha', None) or self.git_provider.get_pr_branch() | ||
| loaded_rules = [] | ||
| loaded_rule_paths = [] | ||
|
|
||
| for rule_path in rule_paths: | ||
| try: | ||
| rule_content = self.git_provider.get_pr_file_content(rule_path, ref) | ||
| except Exception: | ||
| continue | ||
|
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
|
||
|
|
||
| if rule_content and rule_content.strip(): | ||
| loaded_rule_paths.append(rule_path) | ||
| loaded_rules.append(f"File: `{rule_path}`\n{rule_content.strip()}") | ||
|
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| if not loaded_rules: | ||
| get_logger().info("No review rules file found for this PR") | ||
| return "" | ||
|
|
||
| review_rules = "\n\n---\n\n".join(loaded_rules) | ||
| max_tokens = get_settings().pr_reviewer.get("max_review_rules_tokens", 0) | ||
| if max_tokens and int(max_tokens) > 0: | ||
| review_rules = clip_tokens(review_rules, int(max_tokens)) | ||
|
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| get_logger().info("Loaded review rules for this PR", artifacts={"rule_files": loaded_rule_paths}) | ||
|
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
|
||
| return review_rules | ||
|
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
|
||
|
|
||
| async def run(self) -> None: | ||
| try: | ||
|
|
@@ -173,7 +219,7 @@ async def run(self) -> None: | |
| if get_settings().pr_reviewer.persistent_comment and not self.incremental.is_incremental: | ||
| final_update_message = get_settings().pr_reviewer.final_update_message | ||
| self.git_provider.publish_persistent_comment(pr_review, | ||
| initial_header=f"{PRReviewHeader.REGULAR.value} 🔍", | ||
| initial_header=f"{PRReviewHeader.REGULAR.value}", | ||
| update_header=True, | ||
| final_update_message=final_update_message, ) | ||
|
Comment on lines
238
to
243
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. Persistent header string mismatch PRReviewer now uses an initial_header without the "🔍" suffix while convert_to_markdown_v2() still emits a header that includes it. In persistent-comment mode this causes publish_persistent_comment_full() to replace only the prefix and leave a stray "🔍" line in the updated comment body. Agent Prompt
|
||
| else: | ||
|
|
@@ -234,14 +280,17 @@ def _prepare_pr_review(self) -> str: | |
| first_key = 'review' | ||
| last_key = 'security_concerns' | ||
| data = load_yaml(self.prediction.strip(), | ||
| keys_fix_yaml=["ticket_compliance_check", "estimated_effort_to_review_[1-5]:", "security_concerns:", "key_issues_to_review:", | ||
| keys_fix_yaml=["ticket_compliance_check", "estimated_effort_to_review_[1-5]:", "risk_level:", "merge_recommendation:", "review_priority_files:", "security_concerns:", "key_issues_to_review:", | ||
| "relevant_file:", "relevant_line:", "suggestion:"], | ||
| first_key=first_key, last_key=last_key) | ||
|
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
Outdated
|
||
| github_action_output(data, 'review') | ||
|
|
||
| if 'review' not in data: | ||
| get_logger().exception("Failed to parse review data", artifact={"data": data}) | ||
| return "" | ||
| get_logger().info(f"Risk level: {data.get('review', {}).get('risk_level')}") | ||
| get_logger().info(f"Merge recommendation: {data.get('review', {}).get('merge_recommendation')}") | ||
| get_logger().info(f"Priority files: {data.get('review', {}).get('review_priority_files')}") | ||
|
|
||
| # move data['review'] 'key_issues_to_review' key to the end of the dictionary | ||
| if 'key_issues_to_review' in data['review']: | ||
|
|
@@ -262,7 +311,7 @@ def _prepare_pr_review(self) -> str: | |
|
|
||
| # Add help text if gfm_markdown is supported | ||
| if self.git_provider.is_supported("gfm_markdown") and get_settings().pr_reviewer.enable_help_text: | ||
| markdown_text += "<hr>\n\n<details> <summary><strong>💡 Tool usage guide:</strong></summary><hr> \n\n" | ||
| markdown_text += "<hr>\n\n<details> <summary><strong>濡絽鍟€?Tool usage guide:</strong></summary><hr> \n\n" | ||
| markdown_text += HelpMessage.get_review_usage_guide() | ||
|
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
Outdated
|
||
| markdown_text += "\n</details>\n" | ||
|
|
||
|
|
@@ -273,6 +322,7 @@ def _prepare_pr_review(self) -> str: | |
| # Add custom labels from the review prediction (effort, security) | ||
| self.set_review_labels(data) | ||
|
|
||
|
|
||
| if markdown_text == None or len(markdown_text) == 0: | ||
| markdown_text = "" | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.