From 560d30dbb1d54cbf5ab641808786c67b9f0b09a4 Mon Sep 17 00:00:00 2001 From: zmeir Date: Wed, 3 Jan 2024 09:56:23 +0200 Subject: [PATCH 1/6] Fix `get_user_description` The headers changed from "PR Type"/"PR Description"/etc to "Type"/"Description"/etc --- pr_agent/git_providers/git_provider.py | 10 +++++++--- pr_agent/tools/pr_description.py | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/pr_agent/git_providers/git_provider.py b/pr_agent/git_providers/git_provider.py index b2d31322..036887e5 100644 --- a/pr_agent/git_providers/git_provider.py +++ b/pr_agent/git_providers/git_provider.py @@ -75,14 +75,18 @@ class GitProvider(ABC): def get_user_description(self) -> str: description = (self.get_pr_description_full() or "").strip() # if the existing description wasn't generated by the pr-agent, just return it as-is - if not any(description.startswith(header) for header in ("## PR Type", "## PR Description")): + if not self._is_generated_by_pr_agent(description): return description # if the existing description was generated by the pr-agent, but it doesn't contain the user description, # return nothing (empty string) because it means there is no user description - if "## User Description:" not in description: + if "## User Description" not in description: return "" # otherwise, extract the original user description from the existing pr-agent description and return it - return description.split("## User Description:", 1)[1].strip() + return description.split("## User Description", 1)[-1].split("\n", 1)[-1].strip() + + def _is_generated_by_pr_agent(self, description: str) -> bool: + possible_headers = ("## PR Type", "## PR Description", "## PR Labels", "## Type", "## Description", "## Labels", "### 🤖 Generated by PR Agent") + return any(description.startswith(header) for header in possible_headers) @abstractmethod def get_repo_settings(self): diff --git a/pr_agent/tools/pr_description.py b/pr_agent/tools/pr_description.py index d79c2d50..2e3c465e 100644 --- a/pr_agent/tools/pr_description.py +++ b/pr_agent/tools/pr_description.py @@ -297,7 +297,7 @@ class PRDescription: value = self.file_label_dict key_publish = "PR changes walkthrough" else: - key_publish = key.rstrip(':').replace("_", " ").capitalize() + key_publish = key.rstrip(':').replace("_", " ").title() pr_body += f"## {key_publish}\n" if 'walkthrough' in key.lower(): if self.git_provider.is_supported("gfm_markdown"): From 39c1866121677ead6eaa5b17165e225619bf74ff Mon Sep 17 00:00:00 2001 From: Zohar Meir <33152084+zmeir@users.noreply.github.com> Date: Thu, 4 Jan 2024 09:41:24 +0200 Subject: [PATCH 2/6] Revert title() to capitalize() --- pr_agent/tools/pr_description.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pr_agent/tools/pr_description.py b/pr_agent/tools/pr_description.py index 2e3c465e..d79c2d50 100644 --- a/pr_agent/tools/pr_description.py +++ b/pr_agent/tools/pr_description.py @@ -297,7 +297,7 @@ class PRDescription: value = self.file_label_dict key_publish = "PR changes walkthrough" else: - key_publish = key.rstrip(':').replace("_", " ").title() + key_publish = key.rstrip(':').replace("_", " ").capitalize() pr_body += f"## {key_publish}\n" if 'walkthrough' in key.lower(): if self.git_provider.is_supported("gfm_markdown"): From 8d2da74380dd4154f44fd2dde262f52c5b41cb80 Mon Sep 17 00:00:00 2001 From: Zohar Meir <33152084+zmeir@users.noreply.github.com> Date: Thu, 4 Jan 2024 09:41:55 +0200 Subject: [PATCH 3/6] Find user description in a case-insensitive way --- pr_agent/git_providers/git_provider.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/pr_agent/git_providers/git_provider.py b/pr_agent/git_providers/git_provider.py index 036887e5..38b532ef 100644 --- a/pr_agent/git_providers/git_provider.py +++ b/pr_agent/git_providers/git_provider.py @@ -74,19 +74,22 @@ class GitProvider(ABC): def get_user_description(self) -> str: description = (self.get_pr_description_full() or "").strip() + description_lowercase = description.lower() # if the existing description wasn't generated by the pr-agent, just return it as-is - if not self._is_generated_by_pr_agent(description): + if not self._is_generated_by_pr_agent(description_lowercase): return description # if the existing description was generated by the pr-agent, but it doesn't contain the user description, # return nothing (empty string) because it means there is no user description - if "## User Description" not in description: + user_description_header = "## user description" + if user_description_header not in description_lowercase: return "" # otherwise, extract the original user description from the existing pr-agent description and return it - return description.split("## User Description", 1)[-1].split("\n", 1)[-1].strip() + user_description_start_position = description_lowercase.find(user_description_header) + len(user_description_header) + return description[user_description_start_position:].split("\n", 1)[-1].strip() - def _is_generated_by_pr_agent(self, description: str) -> bool: - possible_headers = ("## PR Type", "## PR Description", "## PR Labels", "## Type", "## Description", "## Labels", "### 🤖 Generated by PR Agent") - return any(description.startswith(header) for header in possible_headers) + def _is_generated_by_pr_agent(self, description_lowercase: str) -> bool: + possible_headers = ("## pr type", "## pr description", "## pr labels", "## type", "## description", "## labels", "### 🤖 generated by pr agent") + return any(description_lowercase.startswith(header) for header in possible_headers) @abstractmethod def get_repo_settings(self): From 3c2ed8bbf125b75f459f4cdb78e23aa492713356 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Thu, 4 Jan 2024 09:42:15 +0200 Subject: [PATCH 4/6] feat: Remove file walkthrough feature from PR agent --- pr_agent/settings/configuration.toml | 1 - pr_agent/settings/pr_description_prompts.toml | 6 ------ pr_agent/tools/pr_description.py | 7 +++---- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index 04466c72..e5428fee 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -51,7 +51,6 @@ keep_original_user_title=false use_bullet_points=true extra_instructions = "" enable_pr_type=true -enable_file_walkthrough=false enable_semantic_files_types=true final_update_message = true diff --git a/pr_agent/settings/pr_description_prompts.toml b/pr_agent/settings/pr_description_prompts.toml index 38c53e48..be4f70a0 100644 --- a/pr_agent/settings/pr_description_prompts.toml +++ b/pr_agent/settings/pr_description_prompts.toml @@ -35,12 +35,6 @@ class PRType(str, Enum): {%- endif %} -{%- if enable_file_walkthrough %} -class FileWalkthrough(BaseModel): - filename: str = Field(description="the relevant file full path") - changes_in_file: str = Field(description="minimal and concise summary of the changes in the relevant file") -{%- endif %} - {%- if enable_semantic_files_types %} Class FileDescription(BaseModel): diff --git a/pr_agent/tools/pr_description.py b/pr_agent/tools/pr_description.py index d79c2d50..85fd89a4 100644 --- a/pr_agent/tools/pr_description.py +++ b/pr_agent/tools/pr_description.py @@ -52,7 +52,6 @@ class PRDescription: "commit_messages_str": self.git_provider.get_commit_messages(), "enable_custom_labels": get_settings().config.enable_custom_labels, "custom_labels_class": "", # will be filled if necessary in 'set_custom_labels' function - "enable_file_walkthrough": get_settings().pr_description.enable_file_walkthrough, "enable_semantic_files_types": get_settings().pr_description.enable_semantic_files_types, } @@ -248,12 +247,12 @@ class PRDescription: body = body.replace('pr_agent:summary', summary) if not re.search(r'', body): - ai_walkthrough = self.data.get('PR Main Files Walkthrough') + ai_walkthrough = self.data.get('PR changes walkthrough') if ai_walkthrough: walkthrough = str(ai_header) for file in ai_walkthrough: filename = file['filename'].replace("'", "`") - description = file['changes in file'].replace("'", "`") + description = file['changes_summary'].replace("'", "`") walkthrough += f'- `{filename}`: {description}\n' body = body.replace('pr_agent:walkthrough', walkthrough) @@ -329,7 +328,7 @@ class PRDescription: try: filename = file['filename'].replace("'", "`").replace('"', '`') changes_summary = file['changes_summary'] - label = file['label'] + label = file.get('label') if label not in self.file_label_dict: self.file_label_dict[label] = [] self.file_label_dict[label].append((filename, changes_summary)) From 4204d78d7e342ad65bf9befe86bf589e6e3cac9c Mon Sep 17 00:00:00 2001 From: mrT23 Date: Thu, 4 Jan 2024 09:59:44 +0200 Subject: [PATCH 5/6] feat: Remove file walkthrough feature from PR agent --- pr_agent/tools/pr_description.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/pr_agent/tools/pr_description.py b/pr_agent/tools/pr_description.py index 85fd89a4..61b40eb8 100644 --- a/pr_agent/tools/pr_description.py +++ b/pr_agent/tools/pr_description.py @@ -246,16 +246,15 @@ class PRDescription: summary = f"{ai_header}{ai_summary}" body = body.replace('pr_agent:summary', summary) - if not re.search(r'', body): - ai_walkthrough = self.data.get('PR changes walkthrough') - if ai_walkthrough: - walkthrough = str(ai_header) - for file in ai_walkthrough: - filename = file['filename'].replace("'", "`") - description = file['changes_summary'].replace("'", "`") - walkthrough += f'- `{filename}`: {description}\n' - - body = body.replace('pr_agent:walkthrough', walkthrough) + ai_walkthrough = self.data.get('pr_files') + if ai_walkthrough and not re.search(r'', body): + try: + walkthrough_gfm = "" + walkthrough_gfm = self.process_pr_files_prediction(walkthrough_gfm, self.file_label_dict) + body = body.replace('pr_agent:walkthrough', walkthrough_gfm) + except Exception as e: + get_logger().error(f"Failing to process walkthrough {self.pr_id}: {e}") + body = body.replace('pr_agent:walkthrough', "") return title, body From ed78bfd9461ce2328275d10493507711ff91ae8a Mon Sep 17 00:00:00 2001 From: mrT23 Date: Thu, 4 Jan 2024 10:27:07 +0200 Subject: [PATCH 6/6] use_collapsible_file_list --- docs/DESCRIBE.md | 3 ++- pr_agent/settings/configuration.toml | 6 +++--- pr_agent/tools/pr_description.py | 16 +++++++++++++--- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/docs/DESCRIBE.md b/docs/DESCRIBE.md index c4106008..222d79d3 100644 --- a/docs/DESCRIBE.md +++ b/docs/DESCRIBE.md @@ -48,7 +48,8 @@ Under the section 'pr_description', the [configuration file](./../pr_agent/setti - `final_update_message`: if set to true, it will add a comment message [`PR Description updated to latest commit...`](https://github.com/Codium-ai/pr-agent/pull/499#issuecomment-1837412176) after finishing calling `/describe`. Default is true. -- `enable_semantic_files_types`: if set to true, "PR changes walkthrough" section will be generated. Default is true. +- `enable_semantic_files_types`: if set to true, "Changes walkthrough" section will be generated. Default is true. +- `collapsible_file_list`: if set to true, the file list in the "Changes walkthrough" section will be collapsible. If set to "adaptive", the file list will be collapsible only if there are more than 8 files. Default is "adaptive". ### Markers template diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index e5428fee..6b6cdc9b 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -51,10 +51,10 @@ keep_original_user_title=false use_bullet_points=true extra_instructions = "" enable_pr_type=true -enable_semantic_files_types=true final_update_message = true - - +## changes walkthrough section +enable_semantic_files_types=true +collapsible_file_list='adaptive' # true, false, 'adaptive' # markers use_description_markers=false include_generated_by_header=true diff --git a/pr_agent/tools/pr_description.py b/pr_agent/tools/pr_description.py index 61b40eb8..362849d5 100644 --- a/pr_agent/tools/pr_description.py +++ b/pr_agent/tools/pr_description.py @@ -293,7 +293,7 @@ class PRDescription: for idx, (key, value) in enumerate(self.data.items()): if key == 'pr_files': value = self.file_label_dict - key_publish = "PR changes walkthrough" + key_publish = "Changes walkthrough" else: key_publish = key.rstrip(':').replace("_", " ").capitalize() pr_body += f"## {key_publish}\n" @@ -336,6 +336,9 @@ class PRDescription: pass def process_pr_files_prediction(self, pr_body, value): + use_collapsible_file_list = get_settings().pr_description.collapsible_file_list + if use_collapsible_file_list == "adaptive": + use_collapsible_file_list = len(value) > 8 if not self.git_provider.is_supported("gfm_markdown"): get_logger().info(f"Disabling semantic files types for {self.pr_id} since gfm_markdown is not supported") return pr_body @@ -350,7 +353,11 @@ class PRDescription: s_label = semantic_label.strip("'").strip('"') pr_body += f"""{s_label.capitalize()}""" list_tuples = value[semantic_label] - pr_body += f"""
{len(list_tuples)} files""" + + if use_collapsible_file_list: + pr_body += f"""""" + else: + pr_body += """
{len(list_tuples)} files""" + else: + pr_body += f"""""" + if use_collapsible_file_list: + pr_body += """
""" for filename, file_change_description in list_tuples: filename = filename.replace("'", "`") filename_publish = filename.split("/")[-1] @@ -389,7 +396,10 @@ class PRDescription: """ - pr_body += """
""" pr_body += """""" except Exception as e: