From 40e518834b82badba812c35db5ce433a387c9027 Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Thu, 14 May 2026 14:23:00 -0500 Subject: [PATCH 01/25] Add review risk assessment and optional repository review rules --- pr_agent/algo/utils.py | 32 ++++ pr_agent/settings/configuration.toml | 6 + pr_agent/settings/pr_reviewer_prompts.toml | 56 +++++++ pr_agent/tools/pr_reviewer.py | 181 ++++++++++++++++++++- 4 files changed, 271 insertions(+), 4 deletions(-) diff --git a/pr_agent/algo/utils.py b/pr_agent/algo/utils.py index 3e8576753f..979e7f91ce 100644 --- a/pr_agent/algo/utils.py +++ b/pr_agent/algo/utils.py @@ -236,6 +236,38 @@ def convert_to_markdown_v2(output_data: dict, markdown_text += f"### {emoji} Security concerns\n\n" value = emphasize_header(value.strip(), only_markdown=True) markdown_text += f"{value}\n\n" + elif 'risk level' in key_nice.lower(): + risk_value = str(value).strip().lower().replace('_', ' ') + risk_display = risk_value.capitalize() if risk_value else 'Unknown' + if gfm_supported: + markdown_text += f"Risk level: {risk_display}\n" + else: + markdown_text += f"### Risk level: {risk_display}\n\n" + elif 'merge recommendation' in key_nice.lower(): + recommendation = str(value).strip().replace('_', ' ') + recommendation_display = recommendation.capitalize() if recommendation else 'Unknown' + if gfm_supported: + markdown_text += f"Merge recommendation: {recommendation_display}\n" + else: + markdown_text += f"### Merge recommendation: {recommendation_display}\n\n" + elif 'review priority files' in key_nice.lower(): + if gfm_supported: + markdown_text += "" + if not value: + markdown_text += "Priority files: None" + else: + markdown_text += "Priority files

" + for priority_file in value: + markdown_text += f"- {priority_file}
" + markdown_text += "\n" + else: + if not value: + markdown_text += "### Priority files: None\n\n" + else: + markdown_text += "### Priority files\n\n" + for priority_file in value: + markdown_text += f"- {priority_file}\n" + markdown_text += "\n" elif 'todo sections' in key_nice.lower(): if gfm_supported: markdown_text += "" diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index c2533b54ec..49fb07dea4 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -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 +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 # 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 diff --git a/pr_agent/settings/pr_reviewer_prompts.toml b/pr_agent/settings/pr_reviewer_prompts.toml index bbe6c6d04c..3d489071cf 100644 --- a/pr_agent/settings/pr_reviewer_prompts.toml +++ b/pr_agent/settings/pr_reviewer_prompts.toml @@ -68,6 +68,27 @@ Extra instructions from the user: {{ extra_instructions }} ====== {% endif %} +{%- if review_rules %} + +Repository/team review rules: +====== +{{ review_rules }} +====== +- Apply these repository rules in addition to the general review criteria. +- When a repository rule conflicts with a generic style preference, prioritize the repository rule. +- Use the repository rules to decide what deserves extra attention, but do not invent violations that are not supported by the diff. +{% endif %} + +{%- if review_history %} + +Relevant past review history from the same repository: +====== +{{ review_history }} +====== +- Use this history as supporting context only. +- Prefer the current diff over past patterns. +- Reuse past patterns only when they are clearly relevant to the current changed files or issue types. +{% endif %} The output must be a YAML object equivalent to type $PRReview, according to the following Pydantic definitions: @@ -117,6 +138,15 @@ class Review(BaseModel): {%- if require_estimate_effort_to_review %} estimated_effort_to_review_[1-5]: int = 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.") {%- endif %} +{%- if require_risk_assessment %} + risk_level: str = Field(description="Overall risk level of this PR. Answer with exactly one of: low, medium, high. Use high only when the PR introduces a clear bug, security concern, or major logic risk. Use medium when the PR is not clearly broken but contains non-trivial areas that require careful human verification. Use low when the PR is small, low-impact, and no important issues are identified.") +{%- endif %} +{%- if require_merge_recommendation %} + merge_recommendation: str = Field(description="Overall merge recommendation for this PR. Answer with exactly one of: safe_to_merge, merge_with_caution, changes_required. Use changes_required when there are clear issues that should be fixed before merge. Use merge_with_caution when the PR seems acceptable but still deserves focused reviewer attention. Use safe_to_merge when no important blockers or risks are identified.") +{%- endif %} +{%- if require_priority_files %} + review_priority_files: List[str] = Field(description="A short list of the most important files a human reviewer should inspect first. Return an empty list if the PR is too small or no file deserves special attention.") +{%- endif %} {%- if require_estimate_contribution_time_cost %} contribution_time_cost_estimate: ContributionTimeCostEstimate = Field(description="An estimate of the time required to implement the changes, based on the quantity, quality, and complexity of the contribution, as well as the context from the PR description and commit messages.") {%- endif %} @@ -165,6 +195,19 @@ review: estimated_effort_to_review_[1-5]: | 3 {%- endif %} +{%- if require_risk_assessment %} + risk_level: | + low +{%- endif %} +{%- if require_merge_recommendation %} + merge_recommendation: | + safe_to_merge +{%- endif %} +{%- if require_priority_files %} + review_priority_files: + - | + src/example.py +{%- endif %} {%- if require_score %} score: 89 {%- endif %} @@ -303,6 +346,19 @@ review: estimated_effort_to_review_[1-5]: | 3 {%- endif %} +{%- if require_risk_assessment %} + risk_level: | + low +{%- endif %} +{%- if require_merge_recommendation %} + merge_recommendation: | + safe_to_merge +{%- endif %} +{%- if require_priority_files %} + review_priority_files: + - | + src/example.py +{%- endif %} {%- if require_score %} score: 89 {%- endif %} diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index c4917f3597..c502bb4ad8 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -1,6 +1,9 @@ import copy import datetime import traceback +import json +from pathlib import Path + from collections import OrderedDict 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() + self.review_history = self._get_review_history() 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, + "review_history": self.review_history, } self.token_handler = TokenHandler( @@ -116,6 +126,165 @@ def parse_incremental(self, args: List[str]): is_incremental = True incremental = IncrementalPR(is_incremental) return incremental + + + def _get_review_memory_file(self) -> Path: + configured = get_settings().pr_reviewer.get( + "review_history_file", "review_memory/review_history.jsonl" + ) + return Path(__file__).resolve().parents[2] / configured + + def _get_changed_filenames(self) -> List[str]: + names = [] + for file in self.git_provider.get_files(): + name = getattr(file, "filename", None) or getattr(file, "path", None) + if name: + names.append(name) + return names + + def _get_review_history(self) -> str: + if not get_settings().pr_reviewer.get("enable_review_history", False): + return "" + + memory_file = self._get_review_memory_file() + if not memory_file.exists(): + get_logger().info("No review history file found yet") + return "" + + repo_id = getattr(self.git_provider, "repo", "") + current_files = set(self._get_changed_filenames()) + rows = [] + + for line in memory_file.read_text(encoding="utf-8").splitlines(): + if not line.strip(): + continue + try: + entry = json.loads(line) + except Exception: + continue + + if entry.get("repo") != repo_id: + continue + + overlap = len(current_files.intersection(entry.get("files", []))) + if current_files and overlap == 0: + continue + + rows.append((overlap, entry)) + + rows.sort( + key=lambda item: (item[0], item[1].get("reviewed_at", "")), + reverse=True, + ) + + max_matches = int( + get_settings().pr_reviewer.get("max_review_history_matches", 3) + ) + selected = [item[1] for item in rows[:max_matches]] + + if not selected: + get_logger().info("No matching review history found for this PR") + return "" + + chunks = [] + for entry in selected: + chunks.append( + f"PR: {entry.get('title', '')}\n" + f"Risk level: {entry.get('risk_level', '')}\n" + f"Merge recommendation: {entry.get('merge_recommendation', '')}\n" + f"Files: {', '.join(entry.get('files', []))}\n" + f"Key issues: {'; '.join(entry.get('key_issue_headers', [])) or 'None'}" + ) + + history_text = "\n\n---\n\n".join(chunks) + max_tokens = int( + get_settings().pr_reviewer.get("max_review_history_tokens", 800) + ) + if max_tokens > 0: + history_text = clip_tokens(history_text, max_tokens) + + get_logger().info( + "Loaded review history for this PR", + artifacts={"matches": len(selected)}, + ) + return history_text + + def _store_review_memory(self, data: dict) -> None: + if not get_settings().pr_reviewer.get("enable_review_history", False): + return + + review = data.get("review", {}) + entry = { + "reviewed_at": datetime.datetime.now().isoformat(), + "repo": getattr(self.git_provider, "repo", ""), + "pr_url": self.pr_url, + "title": self.git_provider.pr.title, + "files": self._get_changed_filenames(), + "risk_level": review.get("risk_level", ""), + "merge_recommendation": review.get("merge_recommendation", ""), + "key_issue_headers": [ + issue.get("issue_header", "").strip() + for issue in review.get("key_issues_to_review", []) + if isinstance(issue, dict) + ], + } + + memory_file = self._get_review_memory_file() + memory_file.parent.mkdir(parents=True, exist_ok=True) + + existing = [] + if memory_file.exists(): + existing = [ + line + for line in memory_file.read_text(encoding="utf-8").splitlines() + if line.strip() + ] + + existing.append(json.dumps(entry, ensure_ascii=False)) + + max_entries = int( + get_settings().pr_reviewer.get("max_review_history_entries", 200) + ) + if max_entries > 0 and len(existing) > max_entries: + existing = existing[-max_entries:] + + memory_file.write_text("\n".join(existing) + "\n", encoding="utf-8") + + + 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] + + 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 + + if rule_content and rule_content.strip(): + loaded_rule_paths.append(rule_path) + loaded_rules.append(f"File: `{rule_path}`\n{rule_content.strip()}") + + 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)) + + get_logger().info("Loaded review rules for this PR", artifacts={"rule_files": loaded_rule_paths}) + return review_rules async def run(self) -> None: try: @@ -173,7 +342,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, ) else: @@ -234,7 +403,7 @@ 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) github_action_output(data, 'review') @@ -242,6 +411,9 @@ def _prepare_pr_review(self) -> str: 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 +434,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 += "
\n\n
💡 Tool usage guide:
\n\n" + markdown_text += "
\n\n
濡絽鍟€?Tool usage guide:
\n\n" markdown_text += HelpMessage.get_review_usage_guide() markdown_text += "\n
\n" @@ -272,6 +444,7 @@ def _prepare_pr_review(self) -> str: # Add custom labels from the review prediction (effort, security) self.set_review_labels(data) + self._store_review_memory(data) if markdown_text == None or len(markdown_text) == 0: markdown_text = "" From 104a69686144a8ed6164a8cb11eb5d7ceacfe03a Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Thu, 14 May 2026 14:57:49 -0500 Subject: [PATCH 02/25] Remove local review history from this PR --- pr_agent/settings/pr_reviewer_prompts.toml | 12 -- pr_agent/tools/pr_reviewer.py | 133 +-------------------- 2 files changed, 5 insertions(+), 140 deletions(-) diff --git a/pr_agent/settings/pr_reviewer_prompts.toml b/pr_agent/settings/pr_reviewer_prompts.toml index 3d489071cf..2477bb28ed 100644 --- a/pr_agent/settings/pr_reviewer_prompts.toml +++ b/pr_agent/settings/pr_reviewer_prompts.toml @@ -79,18 +79,6 @@ Repository/team review rules: - Use the repository rules to decide what deserves extra attention, but do not invent violations that are not supported by the diff. {% endif %} -{%- if review_history %} - -Relevant past review history from the same repository: -====== -{{ review_history }} -====== -- Use this history as supporting context only. -- Prefer the current diff over past patterns. -- Reuse past patterns only when they are clearly relevant to the current changed files or issue types. -{% endif %} - - The output must be a YAML object equivalent to type $PRReview, according to the following Pydantic definitions: ===== {%- if require_can_be_split_review %} diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index c502bb4ad8..fb0e9f0bd7 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -1,8 +1,8 @@ import copy import datetime import traceback -import json -from pathlib import Path + + from collections import OrderedDict from functools import partial @@ -70,7 +70,7 @@ def __init__(self, pr_url: str, is_answer: bool = False, is_auto: bool = False, self.pr_description, self.pr_description_files = ( self.git_provider.get_pr_description(split_changes_walkthrough=True)) self.review_rules = self._get_review_rules() - self.review_history = self._get_review_history() + 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) @@ -108,7 +108,7 @@ def __init__(self, pr_url: str, is_answer: bool = False, is_auto: bool = False, 'duplicate_prompt_examples': get_settings().config.get('duplicate_prompt_examples', False), "date": datetime.datetime.now().strftime('%Y-%m-%d'), "review_rules": self.review_rules, - "review_history": self.review_history, + } self.token_handler = TokenHandler( @@ -128,129 +128,6 @@ def parse_incremental(self, args: List[str]): return incremental - def _get_review_memory_file(self) -> Path: - configured = get_settings().pr_reviewer.get( - "review_history_file", "review_memory/review_history.jsonl" - ) - return Path(__file__).resolve().parents[2] / configured - - def _get_changed_filenames(self) -> List[str]: - names = [] - for file in self.git_provider.get_files(): - name = getattr(file, "filename", None) or getattr(file, "path", None) - if name: - names.append(name) - return names - - def _get_review_history(self) -> str: - if not get_settings().pr_reviewer.get("enable_review_history", False): - return "" - - memory_file = self._get_review_memory_file() - if not memory_file.exists(): - get_logger().info("No review history file found yet") - return "" - - repo_id = getattr(self.git_provider, "repo", "") - current_files = set(self._get_changed_filenames()) - rows = [] - - for line in memory_file.read_text(encoding="utf-8").splitlines(): - if not line.strip(): - continue - try: - entry = json.loads(line) - except Exception: - continue - - if entry.get("repo") != repo_id: - continue - - overlap = len(current_files.intersection(entry.get("files", []))) - if current_files and overlap == 0: - continue - - rows.append((overlap, entry)) - - rows.sort( - key=lambda item: (item[0], item[1].get("reviewed_at", "")), - reverse=True, - ) - - max_matches = int( - get_settings().pr_reviewer.get("max_review_history_matches", 3) - ) - selected = [item[1] for item in rows[:max_matches]] - - if not selected: - get_logger().info("No matching review history found for this PR") - return "" - - chunks = [] - for entry in selected: - chunks.append( - f"PR: {entry.get('title', '')}\n" - f"Risk level: {entry.get('risk_level', '')}\n" - f"Merge recommendation: {entry.get('merge_recommendation', '')}\n" - f"Files: {', '.join(entry.get('files', []))}\n" - f"Key issues: {'; '.join(entry.get('key_issue_headers', [])) or 'None'}" - ) - - history_text = "\n\n---\n\n".join(chunks) - max_tokens = int( - get_settings().pr_reviewer.get("max_review_history_tokens", 800) - ) - if max_tokens > 0: - history_text = clip_tokens(history_text, max_tokens) - - get_logger().info( - "Loaded review history for this PR", - artifacts={"matches": len(selected)}, - ) - return history_text - - def _store_review_memory(self, data: dict) -> None: - if not get_settings().pr_reviewer.get("enable_review_history", False): - return - - review = data.get("review", {}) - entry = { - "reviewed_at": datetime.datetime.now().isoformat(), - "repo": getattr(self.git_provider, "repo", ""), - "pr_url": self.pr_url, - "title": self.git_provider.pr.title, - "files": self._get_changed_filenames(), - "risk_level": review.get("risk_level", ""), - "merge_recommendation": review.get("merge_recommendation", ""), - "key_issue_headers": [ - issue.get("issue_header", "").strip() - for issue in review.get("key_issues_to_review", []) - if isinstance(issue, dict) - ], - } - - memory_file = self._get_review_memory_file() - memory_file.parent.mkdir(parents=True, exist_ok=True) - - existing = [] - if memory_file.exists(): - existing = [ - line - for line in memory_file.read_text(encoding="utf-8").splitlines() - if line.strip() - ] - - existing.append(json.dumps(entry, ensure_ascii=False)) - - max_entries = int( - get_settings().pr_reviewer.get("max_review_history_entries", 200) - ) - if max_entries > 0 and len(existing) > max_entries: - existing = existing[-max_entries:] - - memory_file.write_text("\n".join(existing) + "\n", encoding="utf-8") - - def _get_review_rules(self) -> str: if not get_settings().pr_reviewer.get("enable_review_rules", False): return "" @@ -444,7 +321,7 @@ def _prepare_pr_review(self) -> str: # Add custom labels from the review prediction (effort, security) self.set_review_labels(data) - self._store_review_memory(data) + if markdown_text == None or len(markdown_text) == 0: markdown_text = "" From 5a74da3c6cb52ad2da3b7567e593f0ee58a559b5 Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Thu, 14 May 2026 15:14:56 -0500 Subject: [PATCH 03/25] Fix help header text and trailing whitespace --- pr_agent/tools/pr_reviewer.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index fb0e9f0bd7..3b3ab9e1ab 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -126,7 +126,6 @@ def parse_incremental(self, args: List[str]): is_incremental = True incremental = IncrementalPR(is_incremental) return incremental - def _get_review_rules(self) -> str: if not get_settings().pr_reviewer.get("enable_review_rules", False): @@ -311,7 +310,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 += "
\n\n
濡絽鍟€?Tool usage guide:
\n\n" + markdown_text += "
\n\n
💡 Tool usage guide:
\n\n" markdown_text += HelpMessage.get_review_usage_guide() markdown_text += "\n
\n" From fc8ee1dafc7dc8797be80ab334f16274e39cc59f Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 01:00:55 -0500 Subject: [PATCH 04/25] Trigger bot review again From d8cbb89bf355148ddf965c1fc7c33d7aeab5b570 Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 01:12:33 -0500 Subject: [PATCH 05/25] Validate max review rules tokens config --- pr_agent/tools/pr_reviewer.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index 3b3ab9e1ab..f12397e179 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -156,9 +156,17 @@ def _get_review_rules(self) -> str: 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)) - + try: + max_tokens = int(max_tokens) + except (TypeError, ValueError): + get_logger().warning( + "Invalid max_review_rules_tokens value; skipping token clipping", + artifacts={"max_review_rules_tokens": max_tokens}, + ) + max_tokens = 0 + + if max_tokens > 0: + review_rules = clip_tokens(review_rules, max_tokens) get_logger().info("Loaded review rules for this PR", artifacts={"rule_files": loaded_rule_paths}) return review_rules From e237f87b3cdd35e6624eb217f859f26ce90128a8 Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 01:26:33 -0500 Subject: [PATCH 06/25] Fix review rules token limit parsing --- pr_agent/tools/pr_reviewer.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index f12397e179..b2574d4403 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -160,10 +160,10 @@ def _get_review_rules(self) -> str: max_tokens = int(max_tokens) except (TypeError, ValueError): get_logger().warning( - "Invalid max_review_rules_tokens value; skipping token clipping", - artifacts={"max_review_rules_tokens": max_tokens}, - ) - max_tokens = 0 + "Invalid max_review_rules_tokens value; skipping token clipping", + artifacts={"max_review_rules_tokens": max_tokens}, + ) + max_tokens = 0 if max_tokens > 0: review_rules = clip_tokens(review_rules, max_tokens) From 13136c60397c6dfe9466a181cdbae2ff69792a9b Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 01:33:24 -0500 Subject: [PATCH 07/25] Fix review rules warning indentation --- pr_agent/tools/pr_reviewer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index b2574d4403..a76c466056 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -160,8 +160,8 @@ def _get_review_rules(self) -> str: max_tokens = int(max_tokens) except (TypeError, ValueError): get_logger().warning( - "Invalid max_review_rules_tokens value; skipping token clipping", - artifacts={"max_review_rules_tokens": max_tokens}, + "Invalid max_review_rules_tokens value; skipping token clipping", + artifacts={"max_review_rules_tokens": max_tokens}, ) max_tokens = 0 From 03e0a0d52b72448c91692a225bba0ea6cc487274 Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 01:44:12 -0500 Subject: [PATCH 08/25] Remove review priority files from YAML fallback keys --- pr_agent/tools/pr_reviewer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index a76c466056..3bc236cf07 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -287,7 +287,7 @@ 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]:", "risk_level:", "merge_recommendation:", "review_priority_files:", "security_concerns:", "key_issues_to_review:", + keys_fix_yaml=["ticket_compliance_check", "estimated_effort_to_review_[1-5]:", "risk_level:", "merge_recommendation:", "security_concerns:", "key_issues_to_review:", "relevant_file:", "relevant_line:", "suggestion:"], first_key=first_key, last_key=last_key) github_action_output(data, 'review') From 2d099faa43d62038a2d34c64e2770615a477747c Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 02:23:25 -0500 Subject: [PATCH 09/25] Align structured review sections with shared emoji rendering --- pr_agent/algo/utils.py | 57 ++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 32 deletions(-) diff --git a/pr_agent/algo/utils.py b/pr_agent/algo/utils.py index 979e7f91ce..b3029a58af 100644 --- a/pr_agent/algo/utils.py +++ b/pr_agent/algo/utils.py @@ -1,5 +1,4 @@ from __future__ import annotations - import ast import copy import difflib @@ -153,6 +152,9 @@ def convert_to_markdown_v2(output_data: dict, "Estimated effort to review [1-5]": "⏱️", "Contribution time cost estimate": "⏳", "Ticket compliance check": "🎫", + "Risk level": "⚠️", + "Merge recommendation": "✅", + "Review priority files": "📂", } markdown_text = "" if not incremental_review: @@ -236,38 +238,29 @@ def convert_to_markdown_v2(output_data: dict, markdown_text += f"### {emoji} Security concerns\n\n" value = emphasize_header(value.strip(), only_markdown=True) markdown_text += f"{value}\n\n" - elif 'risk level' in key_nice.lower(): - risk_value = str(value).strip().lower().replace('_', ' ') - risk_display = risk_value.capitalize() if risk_value else 'Unknown' - if gfm_supported: - markdown_text += f"Risk level: {risk_display}\n" - else: - markdown_text += f"### Risk level: {risk_display}\n\n" - elif 'merge recommendation' in key_nice.lower(): - recommendation = str(value).strip().replace('_', ' ') - recommendation_display = recommendation.capitalize() if recommendation else 'Unknown' - if gfm_supported: - markdown_text += f"Merge recommendation: {recommendation_display}\n" + elif "risk level" in key_nice.lower(): + risk_value = str(value).strip().lower().replace("_", " ") + risk_display = risk_value.capitalize() if risk_value else "Unknown" + markdown_text += f"### {emoji} Risk level: {risk_display}\n\n" + + elif "merge recommendation" in key_nice.lower(): + recommendation = str(value).strip().replace("_", " ") + recommendation_display = ( + recommendation.capitalize() if recommendation else "Unknown" + ) + markdown_text += ( + f"### {emoji} Merge recommendation: {recommendation_display}\n\n" + ) + + elif "Review priority files" in key_nice.lower(): + if not value: + markdown_text += f"### {emoji} Priority files: None\n\n" else: - markdown_text += f"### Merge recommendation: {recommendation_display}\n\n" - elif 'review priority files' in key_nice.lower(): - if gfm_supported: - markdown_text += "" - if not value: - markdown_text += "Priority files: None" - else: - markdown_text += "Priority files

" - for priority_file in value: - markdown_text += f"- {priority_file}
" - markdown_text += "\n" - else: - if not value: - markdown_text += "### Priority files: None\n\n" - else: - markdown_text += "### Priority files\n\n" - for priority_file in value: - markdown_text += f"- {priority_file}\n" - markdown_text += "\n" + markdown_text += f"### {emoji} Priority files\n\n" + for priority_file in value: + markdown_text += f"- {priority_file}\n" + markdown_text += "\n" + elif 'todo sections' in key_nice.lower(): if gfm_supported: markdown_text += "" From 71011b35f336748081aa8ed7e3b7a8b95dd1b703 Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 02:40:13 -0500 Subject: [PATCH 10/25] Load review rules from trusted base ref --- pr_agent/tools/pr_reviewer.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index 3bc236cf07..29a97470df 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -135,8 +135,16 @@ def _get_review_rules(self) -> str: if isinstance(rule_paths, str): rule_paths = [rule_paths] - ref = getattr(getattr(self.git_provider, 'pr', None), 'head', None) - ref = getattr(ref, 'sha', None) or self.git_provider.get_pr_branch() + base = getattr(getattr(self.git_provider, "pr", None), "base", None) + ref = ( + getattr(base, "sha", None) + or getattr(base, "ref", None) + or getattr(self.git_provider, "base_sha", None) + or getattr(self.git_provider, "base_ref", None) + ) + if not ref: + get_logger().warning("Could not resolve a trusted base ref for review rules") + return "" loaded_rules = [] loaded_rule_paths = [] From 666a265729de59aa1e6dda321e6c923a0ec24997 Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 02:47:51 -0500 Subject: [PATCH 11/25] Guard review priority files rendering --- pr_agent/algo/utils.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pr_agent/algo/utils.py b/pr_agent/algo/utils.py index b3029a58af..8e3c71a905 100644 --- a/pr_agent/algo/utils.py +++ b/pr_agent/algo/utils.py @@ -252,12 +252,17 @@ def convert_to_markdown_v2(output_data: dict, f"### {emoji} Merge recommendation: {recommendation_display}\n\n" ) - elif "Review priority files" in key_nice.lower(): - if not value: + elif "review priority files" in key_nice.lower(): + if not isinstance(value, list): + value = [] + + priority_files = [str(priority_file).strip() for priority_file in value if str(priority_file).strip()] + + if not priority_files: markdown_text += f"### {emoji} Priority files: None\n\n" else: markdown_text += f"### {emoji} Priority files\n\n" - for priority_file in value: + for priority_file in priority_files: markdown_text += f"- {priority_file}\n" markdown_text += "\n" From 8c93ceb9dad0d213314ee20cc9260c13e5bea52a Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 02:55:49 -0500 Subject: [PATCH 12/25] Harden priority files rendering and yaml repair keys --- pr_agent/algo/utils.py | 11 +++++++---- pr_agent/tools/pr_reviewer.py | 24 ++++++++++++++++++------ 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/pr_agent/algo/utils.py b/pr_agent/algo/utils.py index 8e3c71a905..5adb67c510 100644 --- a/pr_agent/algo/utils.py +++ b/pr_agent/algo/utils.py @@ -253,10 +253,13 @@ def convert_to_markdown_v2(output_data: dict, ) elif "review priority files" in key_nice.lower(): - if not isinstance(value, list): - value = [] - - priority_files = [str(priority_file).strip() for priority_file in value if str(priority_file).strip()] + priority_files = [] + if isinstance(value, list): + priority_files = [ + str(priority_file).strip() + for priority_file in value + if str(priority_file).strip() + ] if not priority_files: markdown_text += f"### {emoji} Priority files: None\n\n" diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index 29a97470df..17434d8723 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -292,12 +292,24 @@ def _prepare_pr_review(self) -> str: Prepare the PR review by processing the AI prediction and generating a markdown-formatted text that summarizes the feedback. """ - 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]:", "risk_level:", "merge_recommendation:", "security_concerns:", "key_issues_to_review:", - "relevant_file:", "relevant_line:", "suggestion:"], - first_key=first_key, last_key=last_key) + 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]:", + "risk_level:", + "merge_recommendation:", + "security_concerns:", + "key_issues_to_review:", + "relevant_file:", + "relevant_line:", + "suggestion:", + ], + first_key=first_key, + last_key=last_key, + ) github_action_output(data, 'review') if 'review' not in data: From 15d894037bd57af2db5c1195506b767c9bc27c47 Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 03:06:08 -0500 Subject: [PATCH 13/25] Support GitLab base ref for review rules --- pr_agent/tools/pr_reviewer.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index 17434d8723..ecf5e7007b 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -1,4 +1,4 @@ -import copy +import copy import datetime import traceback @@ -141,6 +141,7 @@ def _get_review_rules(self) -> str: or getattr(base, "ref", None) or getattr(self.git_provider, "base_sha", None) or getattr(self.git_provider, "base_ref", None) + or getattr(getattr(self.git_provider, "mr", None), "target_branch", None) ) if not ref: get_logger().warning("Could not resolve a trusted base ref for review rules") From 20a256df3a08246ffbfe4f86235c5f19b3b4222a Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 08:40:06 -0500 Subject: [PATCH 14/25] Validate review rules paths config --- pr_agent/tools/pr_reviewer.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index ecf5e7007b..7bcb7116e7 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -134,6 +134,18 @@ def _get_review_rules(self) -> str: rule_paths = get_settings().pr_reviewer.get("review_rules_paths", []) or [] if isinstance(rule_paths, str): rule_paths = [rule_paths] + elif isinstance(rule_paths, (list, tuple)): + rule_paths = [ + str(rule_path).strip() + for rule_path in rule_paths + if str(rule_path).strip() + ] + else: + get_logger().warning( + "Invalid review_rules_paths value; expected string or list of strings", + artifacts={"review_rules_paths": rule_paths}, + ) + rule_paths = [] base = getattr(getattr(self.git_provider, "pr", None), "base", None) ref = ( From b71e7bef4dd920c8ecabb471948d26a00e857a43 Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 09:02:40 -0500 Subject: [PATCH 15/25] Trigger review refresh From b97e8334892cd114bd1ac47dd75396837c3758eb Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 09:15:29 -0500 Subject: [PATCH 16/25] Disable new review fields by default and remove BOM --- pr_agent/settings/configuration.toml | 6 +++--- pr_agent/tools/pr_reviewer.py | 4 +--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index 49fb07dea4..9142cf1808 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -81,9 +81,9 @@ 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 +require_risk_assessment=false +require_merge_recommendation=false +require_priority_files=false 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 diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index 7bcb7116e7..3705d6198d 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -1,9 +1,7 @@ -import copy +import copy import datetime import traceback - - from collections import OrderedDict from functools import partial from typing import List, Tuple From 983d2273bd49a8489f2eb0ac8c3bce7e59046b23 Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 09:27:20 -0500 Subject: [PATCH 17/25] Add provider-level trusted base ref for review rules --- pr_agent/git_providers/bitbucket_provider.py | 3 +++ pr_agent/git_providers/bitbucket_server_provider.py | 3 +++ pr_agent/git_providers/git_provider.py | 4 ++++ pr_agent/git_providers/gitea_provider.py | 3 +++ pr_agent/git_providers/github_provider.py | 3 +++ pr_agent/git_providers/gitlab_provider.py | 3 +++ pr_agent/tools/pr_reviewer.py | 9 +-------- 7 files changed, 20 insertions(+), 8 deletions(-) diff --git a/pr_agent/git_providers/bitbucket_provider.py b/pr_agent/git_providers/bitbucket_provider.py index 6944e41fa2..135f66aee3 100644 --- a/pr_agent/git_providers/bitbucket_provider.py +++ b/pr_agent/git_providers/bitbucket_provider.py @@ -489,6 +489,9 @@ def get_languages(self): def get_pr_branch(self): return self.pr.source_branch + def get_pr_base_ref(self) -> str: + return self.pr.destination_branch or "" + # This function attempts to get the default branch of the repository. As a fallback, uses the PR destination branch. # Note: Must be running from a PR context. def get_repo_default_branch(self): diff --git a/pr_agent/git_providers/bitbucket_server_provider.py b/pr_agent/git_providers/bitbucket_server_provider.py index c929221af9..00a9a61c3d 100644 --- a/pr_agent/git_providers/bitbucket_server_provider.py +++ b/pr_agent/git_providers/bitbucket_server_provider.py @@ -417,6 +417,9 @@ def get_languages(self): def get_pr_branch(self): return self.pr.fromRef['displayId'] + def get_pr_base_ref(self) -> str: + return self.pr.toRef.get("latestCommit") or self.pr.toRef.get("displayId") or "" + def get_pr_owner_id(self) -> str | None: return self.workspace_slug diff --git a/pr_agent/git_providers/git_provider.py b/pr_agent/git_providers/git_provider.py index 631e189c04..1f09fc5aeb 100644 --- a/pr_agent/git_providers/git_provider.py +++ b/pr_agent/git_providers/git_provider.py @@ -181,6 +181,10 @@ def get_languages(self): def get_pr_branch(self): pass + def get_pr_base_ref(self) -> str: + get_logger().warning("Not implemented! Returning empty base ref") + return "" + @abstractmethod def get_user_id(self): pass diff --git a/pr_agent/git_providers/gitea_provider.py b/pr_agent/git_providers/gitea_provider.py index 89a6248e9b..ab5055a5a0 100644 --- a/pr_agent/git_providers/gitea_provider.py +++ b/pr_agent/git_providers/gitea_provider.py @@ -578,6 +578,9 @@ def get_pr_branch(self) -> str: return self.pr.head.ref if self.pr.head.ref else "" + def get_pr_base_ref(self) -> str: + return self.base_sha or self.base_ref or "" + def get_pr_description_full(self) -> str: """Get full PR description with metadata""" if not self.pr: diff --git a/pr_agent/git_providers/github_provider.py b/pr_agent/git_providers/github_provider.py index fa52b7dc05..4d001b06d8 100644 --- a/pr_agent/git_providers/github_provider.py +++ b/pr_agent/git_providers/github_provider.py @@ -701,6 +701,9 @@ def get_languages(self): def get_pr_branch(self): return self.pr.head.ref + def get_pr_base_ref(self) -> str: + return self.pr.base.sha or self.pr.base.ref or "" + def get_pr_owner_id(self) -> str | None: if not self.repo: return None diff --git a/pr_agent/git_providers/gitlab_provider.py b/pr_agent/git_providers/gitlab_provider.py index b3f54920d0..be1ac29c26 100644 --- a/pr_agent/git_providers/gitlab_provider.py +++ b/pr_agent/git_providers/gitlab_provider.py @@ -774,6 +774,9 @@ def get_languages(self): def get_pr_branch(self): return self.mr.source_branch + def get_pr_base_ref(self) -> str: + return getattr(self.mr, "target_branch", "") or "" + def get_pr_owner_id(self) -> str | None: if not self.gitlab_url or 'gitlab.com' in self.gitlab_url: if not self.id_project: diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index 3705d6198d..66f1d78b04 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -145,14 +145,7 @@ def _get_review_rules(self) -> str: ) rule_paths = [] - base = getattr(getattr(self.git_provider, "pr", None), "base", None) - ref = ( - getattr(base, "sha", None) - or getattr(base, "ref", None) - or getattr(self.git_provider, "base_sha", None) - or getattr(self.git_provider, "base_ref", None) - or getattr(getattr(self.git_provider, "mr", None), "target_branch", None) - ) + ref = self.git_provider.get_pr_base_ref() if not ref: get_logger().warning("Could not resolve a trusted base ref for review rules") return "" From 9298146de8684d4807966933a95ef6be6aa59f6e Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 09:43:04 -0500 Subject: [PATCH 18/25] Add review rules coverage and structured review field tests --- pr_agent/algo/utils.py | 2 +- tests/unittest/test_convert_to_markdown.py | 43 ++++++++++++ tests/unittest/test_pr_reviewer.py | 82 ++++++++++++++++++++++ 3 files changed, 126 insertions(+), 1 deletion(-) create mode 100644 tests/unittest/test_pr_reviewer.py diff --git a/pr_agent/algo/utils.py b/pr_agent/algo/utils.py index 5adb67c510..748d526911 100644 --- a/pr_agent/algo/utils.py +++ b/pr_agent/algo/utils.py @@ -174,7 +174,7 @@ def convert_to_markdown_v2(output_data: dict, todo_summary = output_data['review'].pop('todo_summary', '') for key, value in output_data['review'].items(): if value is None or value == '' or value == {} or value == []: - if key.lower() not in ['can_be_split', 'key_issues_to_review']: + if key.lower() not in ['can_be_split', 'key_issues_to_review', 'review_priority_files']: continue key_nice = key.replace('_', ' ').capitalize() emoji = emojis.get(key_nice, "") diff --git a/tests/unittest/test_convert_to_markdown.py b/tests/unittest/test_convert_to_markdown.py index 0d18e03cec..aa254e1a69 100644 --- a/tests/unittest/test_convert_to_markdown.py +++ b/tests/unittest/test_convert_to_markdown.py @@ -255,6 +255,49 @@ def test_contribution_time_cost_estimate(self): """) assert convert_to_markdown_v2(input_data, gfm_supported=False).strip() == expected_output_no_gfm.strip() + def test_structured_review_fields_render(self): + input_data = { + "review": { + "risk_level": "medium", + "merge_recommendation": "merge_with_caution", + "review_priority_files": ["gui_app.py", "app.py"], + } + } + + markdown = convert_to_markdown_v2(input_data, gfm_supported=False) + + assert "Risk level: Medium" in markdown + assert "Merge recommendation: Merge with caution" in markdown + assert "Priority files" in markdown + assert "- gui_app.py" in markdown + assert "- app.py" in markdown + + def test_structured_review_fields_render_empty_priority_files(self): + input_data = { + "review": { + "risk_level": "low", + "merge_recommendation": "safe_to_merge", + "review_priority_files": [], + } + } + + markdown = convert_to_markdown_v2(input_data, gfm_supported=False) + + assert "Risk level: Low" in markdown + assert "Merge recommendation: Safe to merge" in markdown + assert "Priority files: None" in markdown + + def test_structured_review_fields_ignore_invalid_priority_files_type(self): + input_data = { + "review": { + "review_priority_files": {"file": "gui_app.py"}, + } + } + + markdown = convert_to_markdown_v2(input_data, gfm_supported=False) + + assert "Priority files: None" in markdown + # Tests that the function works correctly with an empty dictionary input def test_empty_dictionary_input(self): diff --git a/tests/unittest/test_pr_reviewer.py b/tests/unittest/test_pr_reviewer.py new file mode 100644 index 0000000000..7a4c9b8f75 --- /dev/null +++ b/tests/unittest/test_pr_reviewer.py @@ -0,0 +1,82 @@ +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from pr_agent.tools.pr_reviewer import PRReviewer + + +def _make_reviewer(): + with patch.object(PRReviewer, "__init__", lambda self, *a, **kw: None): + reviewer = PRReviewer.__new__(PRReviewer) + reviewer.git_provider = MagicMock() + return reviewer + + +class TestPRReviewerReviewRules: + @patch("pr_agent.tools.pr_reviewer.get_settings") + def test_get_review_rules_disabled_returns_empty(self, mock_get_settings): + mock_get_settings.return_value = SimpleNamespace( + pr_reviewer={"enable_review_rules": False} + ) + reviewer = _make_reviewer() + + assert reviewer._get_review_rules() == "" + + @patch("pr_agent.tools.pr_reviewer.get_settings") + def test_get_review_rules_invalid_paths_type_returns_empty(self, mock_get_settings): + mock_get_settings.return_value = SimpleNamespace( + pr_reviewer={ + "enable_review_rules": True, + "review_rules_paths": 123, + "max_review_rules_tokens": 0, + } + ) + reviewer = _make_reviewer() + reviewer.git_provider.get_pr_base_ref.return_value = "main" + + assert reviewer._get_review_rules() == "" + reviewer.git_provider.get_pr_file_content.assert_not_called() + + @patch("pr_agent.tools.pr_reviewer.get_settings") + def test_get_review_rules_missing_base_ref_returns_empty(self, mock_get_settings): + mock_get_settings.return_value = SimpleNamespace( + pr_reviewer={ + "enable_review_rules": True, + "review_rules_paths": [".pr_agent/review_rules.md"], + "max_review_rules_tokens": 0, + } + ) + reviewer = _make_reviewer() + reviewer.git_provider.get_pr_base_ref.return_value = "" + + assert reviewer._get_review_rules() == "" + reviewer.git_provider.get_pr_file_content.assert_not_called() + + @patch("pr_agent.tools.pr_reviewer.get_settings") + def test_get_review_rules_loads_and_concatenates_files(self, mock_get_settings): + mock_get_settings.return_value = SimpleNamespace( + pr_reviewer={ + "enable_review_rules": True, + "review_rules_paths": [ + ".pr_agent/review_rules.md", + ".github/review_rules.md", + ], + "max_review_rules_tokens": 0, + } + ) + reviewer = _make_reviewer() + reviewer.git_provider.get_pr_base_ref.return_value = "main" + + def _get_file_content(path, ref): + if path == ".pr_agent/review_rules.md": + return "Rule A" + if path == ".github/review_rules.md": + return "Rule B" + return "" + + reviewer.git_provider.get_pr_file_content.side_effect = _get_file_content + + review_rules = reviewer._get_review_rules() + + assert "File: `.pr_agent/review_rules.md`\nRule A" in review_rules + assert "File: `.github/review_rules.md`\nRule B" in review_rules + assert "\n\n---\n\n" in review_rules From 20673dad05c10237e76f3f91f82989a918253b99 Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 09:53:00 -0500 Subject: [PATCH 19/25] Trigger review refresh again From d2f6982d0ed859a3031ec92f986dc9a305256dec Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 10:42:47 -0500 Subject: [PATCH 20/25] Trigger review refresh after latest fixes From 602094154d1f0304b251df7fa94a0f6b5cbd3264 Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 10:56:16 -0500 Subject: [PATCH 21/25] Add Bitbucket Server review rules file loading --- pr_agent/git_providers/bitbucket_server_provider.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pr_agent/git_providers/bitbucket_server_provider.py b/pr_agent/git_providers/bitbucket_server_provider.py index 00a9a61c3d..ff99c00a1d 100644 --- a/pr_agent/git_providers/bitbucket_server_provider.py +++ b/pr_agent/git_providers/bitbucket_server_provider.py @@ -420,6 +420,15 @@ def get_pr_branch(self): def get_pr_base_ref(self) -> str: return self.pr.toRef.get("latestCommit") or self.pr.toRef.get("displayId") or "" + def get_pr_file_content(self, file_path: str, branch: str) -> str: + try: + return self.get_file(file_path, branch) + except Exception as e: + get_logger().warning( + f"Error retrieving file {file_path} from ref {branch}: {e}" + ) + return "" + def get_pr_owner_id(self) -> str | None: return self.workspace_slug From ca9ab5e3e5387b625839469393a6500fcee209ac Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 11:05:52 -0500 Subject: [PATCH 22/25] Trigger review refresh once more From c133868d59c4425b77813b1ecfe066ff3854a69b Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 11:18:12 -0500 Subject: [PATCH 23/25] Add Gitea review rules file loading --- pr_agent/git_providers/gitea_provider.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pr_agent/git_providers/gitea_provider.py b/pr_agent/git_providers/gitea_provider.py index ab5055a5a0..c60b966705 100644 --- a/pr_agent/git_providers/gitea_provider.py +++ b/pr_agent/git_providers/gitea_provider.py @@ -581,6 +581,20 @@ def get_pr_branch(self) -> str: def get_pr_base_ref(self) -> str: return self.base_sha or self.base_ref or "" + def get_pr_file_content(self, file_path: str, branch: str) -> str: + try: + return self.repo_api.get_file_content( + owner=self.owner, + repo=self.repo, + commit_sha=branch, + filepath=file_path, + ) + except Exception as e: + self.logger.warning( + f"Error retrieving file {file_path} from ref {branch}: {e}" + ) + return "" + def get_pr_description_full(self) -> str: """Get full PR description with metadata""" if not self.pr: From b2b3f6a91c9743e3ce36aefa6db8d0d2e0f1df95 Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 11:28:54 -0500 Subject: [PATCH 24/25] Trigger review refresh after latest provider fixes From fecf2953d7426b32a3b8c1a2f6bf2f069b4e5c27 Mon Sep 17 00:00:00 2001 From: xiaodeng666a <2998563885@qq.com> Date: Fri, 15 May 2026 11:41:34 -0500 Subject: [PATCH 25/25] Trigger review refresh once more