From 754d47f1875a4839abbc6a98858886ca33b937e4 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Fri, 18 Jul 2025 08:51:48 +0300 Subject: [PATCH] refactor(pr_description): redesign changes walkthrough section and improve file processing --- pr_agent/algo/utils.py | 5 +- .../git_providers/azuredevops_provider.py | 2 +- pr_agent/settings/configuration.toml | 2 +- pr_agent/tools/pr_description.py | 60 +++++++++++++------ 4 files changed, 47 insertions(+), 22 deletions(-) diff --git a/pr_agent/algo/utils.py b/pr_agent/algo/utils.py index 00d52a14..eaa8ce47 100644 --- a/pr_agent/algo/utils.py +++ b/pr_agent/algo/utils.py @@ -70,7 +70,8 @@ class ReasoningEffort(str, Enum): class PRDescriptionHeader(str, Enum): - CHANGES_WALKTHROUGH = "### **Changes walkthrough** 📝" + DIAGRAM_WALKTHROUGH = "Diagram Walkthrough" + FILE_WALKTHROUGH = "File Walkthrough" def get_setting(key: str) -> Any: @@ -1284,7 +1285,7 @@ def process_description(description_full: str) -> Tuple[str, List]: if not description_full: return "", [] - description_split = description_full.split(PRDescriptionHeader.CHANGES_WALKTHROUGH.value) + description_split = description_full.split(PRDescriptionHeader.FILE_WALKTHROUGH.value) base_description_str = description_split[0] changes_walkthrough_str = "" files = [] diff --git a/pr_agent/git_providers/azuredevops_provider.py b/pr_agent/git_providers/azuredevops_provider.py index 283f28c7..cba81654 100644 --- a/pr_agent/git_providers/azuredevops_provider.py +++ b/pr_agent/git_providers/azuredevops_provider.py @@ -380,7 +380,7 @@ class AzureDevopsProvider(GitProvider): pr_body = pr_body[:ind] if len(pr_body) > MAX_PR_DESCRIPTION_AZURE_LENGTH: - changes_walkthrough_text = PRDescriptionHeader.CHANGES_WALKTHROUGH.value + changes_walkthrough_text = PRDescriptionHeader.FILE_WALKTHROUGH.value ind = pr_body.find(changes_walkthrough_text) if ind != -1: pr_body = pr_body[:ind] diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index e75cfa56..3613d136 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -106,7 +106,7 @@ extra_instructions = "" enable_pr_type=true final_update_message = true enable_help_text=false -enable_help_comment=true +enable_help_comment=false enable_pr_diagram=true # adds a section with a diagram of the PR changes # describe as comment publish_description_as_comment=false diff --git a/pr_agent/tools/pr_description.py b/pr_agent/tools/pr_description.py index f0446b05..c9ecdbe2 100644 --- a/pr_agent/tools/pr_description.py +++ b/pr_agent/tools/pr_description.py @@ -128,7 +128,7 @@ class PRDescription: pr_title, pr_body, changes_walkthrough, pr_file_changes = self._prepare_pr_answer() if not self.git_provider.is_supported( "publish_file_comments") or not get_settings().pr_description.inline_file_summary: - pr_body += "\n\n" + changes_walkthrough + pr_body += "\n\n" + changes_walkthrough + "___\n\n" get_logger().debug("PR output", artifact={"title": pr_title, "body": pr_body}) # Add help text if gfm_markdown is supported @@ -331,7 +331,8 @@ class PRDescription: else: original_prediction_dict = original_prediction_loaded if original_prediction_dict: - filenames_predicted = [file.get('filename', '').strip() for file in original_prediction_dict.get('pr_files', [])] + files = original_prediction_dict.get('pr_files', []) + filenames_predicted = [file.get('filename', '').strip() for file in files if isinstance(file, dict)] else: filenames_predicted = [] @@ -555,15 +556,11 @@ class PRDescription: """ # Iterate over the dictionary items and append the key and value to 'markdown_text' in a markdown format - markdown_text = "" # Don't display 'PR Labels' if 'labels' in self.data and self.git_provider.is_supported("get_labels"): self.data.pop('labels') if not get_settings().pr_description.enable_pr_type: self.data.pop('type') - for key, value in self.data.items(): - markdown_text += f"## **{key}**\n\n" - markdown_text += f"{value}\n\n" # Remove the 'PR Title' key from the dictionary ai_title = self.data.pop('title', self.vars["title"]) @@ -579,6 +576,10 @@ class PRDescription: pr_body, changes_walkthrough = "", "" pr_file_changes = [] for idx, (key, value) in enumerate(self.data.items()): + if key == 'changes_diagram': + pr_body += f"### {PRDescriptionHeader.DIAGRAM_WALKTHROUGH.value}\n\n" + pr_body += f"{value}\n\n" + continue if key == 'pr_files': value = self.file_label_dict else: @@ -597,9 +598,15 @@ class PRDescription: pr_body += f'- `{filename}`: {description}\n' if self.git_provider.is_supported("gfm_markdown"): pr_body += "\n" - elif 'pr_files' in key.lower() and get_settings().pr_description.enable_semantic_files_types: - changes_walkthrough, pr_file_changes = self.process_pr_files_prediction(changes_walkthrough, value) - changes_walkthrough = f"{PRDescriptionHeader.CHANGES_WALKTHROUGH.value}\n{changes_walkthrough}" + elif 'pr_files' in key.lower() and get_settings().pr_description.enable_semantic_files_types: # 'File Walkthrough' section + changes_walkthrough_table, pr_file_changes = self.process_pr_files_prediction(changes_walkthrough, value) + if get_settings().pr_description.get('file_table_collapsible_open_by_default', False): + initial_status = " open" + else: + initial_status = "" + changes_walkthrough = f"

{PRDescriptionHeader.FILE_WALKTHROUGH.value}

\n\n" + changes_walkthrough += f"{changes_walkthrough_table}\n\n" + changes_walkthrough += "\n\n" elif key.lower().strip() == 'description': if isinstance(value, list): value = ', '.join(v.rstrip() for v in value) @@ -633,14 +640,18 @@ class PRDescription: artifact={"file": file}) continue filename = file['filename'].replace("'", "`").replace('"', '`') - changes_summary = file.get('changes_summary', "").strip() + changes_summary = file.get('changes_summary', "") + if not changes_summary: + get_logger().warning(f"Empty changes summary in file label dict, skipping file", + artifact={"file": file}) + changes_summary = changes_summary.strip() changes_title = file['changes_title'].strip() label = file.get('label').strip().lower() if label not in file_label_dict: file_label_dict[label] = [] file_label_dict[label].append((filename, changes_title, changes_summary)) except Exception as e: - get_logger().error(f"Error preparing file label dict {self.pr_id}: {e}") + get_logger().exception(f"Error preparing file label dict {self.pr_id}") pass return file_label_dict @@ -720,7 +731,7 @@ class PRDescription: pr_body += """""" except Exception as e: - get_logger().error(f"Error processing PR files to markdown {self.pr_id}: {str(e)}") + get_logger().error(f"Error processing pr files to markdown {self.pr_id}: {str(e)}") pass return pr_body, pr_comments @@ -776,14 +787,21 @@ def insert_br_after_x_chars(text: str, x=70): if count_chars_without_html(text) < x: return text + is_list = text.lstrip().startswith(("- ", "* ")) + # replace odd instances of ` with and even instances of ` with text = replace_code_tags(text) - # convert list items to
  • - if text.startswith("- ") or text.startswith("* "): - text = "
  • " + text[2:] - text = text.replace("\n- ", '
  • ').replace("\n - ", '
  • ') - text = text.replace("\n* ", '
  • ').replace("\n * ", '
  • ') + # convert list items to
  • only if the text is identified as a list + if is_list: + # To handle lists that start with indentation + leading_whitespace = text[:len(text) - len(text.lstrip())] + body = text.lstrip() + body = "
  • " + body[2:] + text = leading_whitespace + body + + text = text.replace("\n- ", '
  • ').replace("\n - ", '
  • ') + text = text.replace("\n* ", '
  • ').replace("\n * ", '
  • ') # convert new lines to
    text = text.replace("\n", '
    ') @@ -823,7 +841,13 @@ def insert_br_after_x_chars(text: str, x=70): is_inside_code = True if "" in word: is_inside_code = False - return ''.join(new_text).strip() + + processed_text = ''.join(new_text).strip() + + if is_list: + processed_text = f"
      {processed_text}
    " + + return processed_text def replace_code_tags(text):