From acefbff62b75b7deeaedc44285129d5b84e0b497 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Tue, 5 Mar 2024 17:29:17 +0200 Subject: [PATCH 1/4] Add 'final_update_message' option to control publishing of update message in persistent comments --- Usage.md | 4 ++-- docs/REVIEW.md | 2 ++ pr_agent/git_providers/bitbucket_provider.py | 11 ++++++++--- pr_agent/git_providers/git_provider.py | 6 +++++- pr_agent/git_providers/github_provider.py | 11 ++++++++--- pr_agent/git_providers/gitlab_provider.py | 11 ++++++++--- pr_agent/settings/configuration.toml | 1 + pr_agent/tools/pr_reviewer.py | 4 +++- 8 files changed, 37 insertions(+), 13 deletions(-) diff --git a/Usage.md b/Usage.md index ac0775287..3253985dd 100644 --- a/Usage.md +++ b/Usage.md @@ -211,8 +211,8 @@ The configuration parameter `push_commands` defines the list of tools that will [github_app] handle_push_trigger = true push_commands = [ - "/describe --pr_description.add_original_user_description=true --pr_description.keep_original_user_title=true", - "/review --pr_reviewer.num_code_suggestions=0", + "/describe --pr_description.add_original_user_description=true --pr_description.keep_original_user_title=true --pr_description.final_update_message=false", + "/review --pr_reviewer.num_code_suggestions=0 --pr_reviewer.final_update_message=false", ] ``` This means that when new code is pushed to the PR, the PR-Agent will run the `describe` and `review` tools, with the specified parameters. diff --git a/docs/REVIEW.md b/docs/REVIEW.md index 39783c483..37af97482 100644 --- a/docs/REVIEW.md +++ b/docs/REVIEW.md @@ -38,6 +38,8 @@ To edit [configurations](./../pr_agent/settings/configuration.toml#L19) related - `inline_code_comments`: if set to true, the tool will publish the code suggestions as comments on the code diff. Default is false. - `persistent_comment`: if set to true, the review comment will be persistent, meaning that every new review request will edit the previous one. Default is true. - `extra_instructions`: Optional extra instructions to the tool. For example: "focus on the changes in the file X. Ignore change in ...". +- `final_update_message`: if set to true, it will add a comment message after finishing calling `/review`, if a message already exits. Default is true. + #### Enable\\disable features - `require_focused_review`: if set to true, the tool will add a section - 'is the PR a focused one'. Default is false. - `require_score_review`: if set to true, the tool will add a section that scores the PR. Default is false. diff --git a/pr_agent/git_providers/bitbucket_provider.py b/pr_agent/git_providers/bitbucket_provider.py index 62ea46c75..103ea5cdb 100644 --- a/pr_agent/git_providers/bitbucket_provider.py +++ b/pr_agent/git_providers/bitbucket_provider.py @@ -160,7 +160,11 @@ def get_latest_commit_url(self): def get_comment_url(self, comment): return comment.data['links']['html']['href'] - def publish_persistent_comment(self, pr_comment: str, initial_header: str, update_header: bool = True, name='review'): + def publish_persistent_comment(self, pr_comment: str, + initial_header: str, + update_header: bool = True, + name='review', + final_update_message=True): try: for comment in self.pr.comments(): body = comment.raw @@ -175,8 +179,9 @@ def publish_persistent_comment(self, pr_comment: str, initial_header: str, updat get_logger().info(f"Persistent mode - updating comment {comment_url} to latest {name} message") d = {"content": {"raw": pr_comment_updated}} response = comment._update_data(comment.put(None, data=d)) - self.publish_comment( - f"**[Persistent {name}]({comment_url})** updated to latest commit {latest_commit_url}") + if final_update_message: + self.publish_comment( + f"**[Persistent {name}]({comment_url})** updated to latest commit {latest_commit_url}") return except Exception as e: get_logger().exception(f"Failed to update persistent review, error: {e}") diff --git a/pr_agent/git_providers/git_provider.py b/pr_agent/git_providers/git_provider.py index 3ef86709c..e8a78b4b3 100644 --- a/pr_agent/git_providers/git_provider.py +++ b/pr_agent/git_providers/git_provider.py @@ -124,7 +124,11 @@ def get_line_link(self, relevant_file: str, relevant_line_start: int, relevant_l def publish_comment(self, pr_comment: str, is_temporary: bool = False): pass - def publish_persistent_comment(self, pr_comment: str, initial_header: str, update_header: bool, name='review'): + def publish_persistent_comment(self, pr_comment: str, + initial_header: str, + update_header: bool = True, + name='review', + final_update_message=True): self.publish_comment(pr_comment) @abstractmethod diff --git a/pr_agent/git_providers/github_provider.py b/pr_agent/git_providers/github_provider.py index c1bff0dc4..22117af6f 100644 --- a/pr_agent/git_providers/github_provider.py +++ b/pr_agent/git_providers/github_provider.py @@ -197,7 +197,11 @@ def get_latest_commit_url(self) -> str: def get_comment_url(self, comment) -> str: return comment.html_url - def publish_persistent_comment(self, pr_comment: str, initial_header: str, update_header: bool = True, name='review'): + def publish_persistent_comment(self, pr_comment: str, + initial_header: str, + update_header: bool = True, + name='review', + final_update_message=True): prev_comments = list(self.pr.get_issue_comments()) for comment in prev_comments: body = comment.body @@ -211,8 +215,9 @@ def publish_persistent_comment(self, pr_comment: str, initial_header: str, updat pr_comment_updated = pr_comment get_logger().info(f"Persistent mode- updating comment {comment_url} to latest review message") response = comment.edit(pr_comment_updated) - self.publish_comment( - f"**[Persistent {name}]({comment_url})** updated to latest commit {latest_commit_url}") + if final_update_message: + self.publish_comment( + f"**[Persistent {name}]({comment_url})** updated to latest commit {latest_commit_url}") return self.publish_comment(pr_comment) diff --git a/pr_agent/git_providers/gitlab_provider.py b/pr_agent/git_providers/gitlab_provider.py index d0e8d5750..8d2e97c85 100644 --- a/pr_agent/git_providers/gitlab_provider.py +++ b/pr_agent/git_providers/gitlab_provider.py @@ -151,7 +151,11 @@ def get_latest_commit_url(self): def get_comment_url(self, comment): return f"{self.mr.web_url}#note_{comment.id}" - def publish_persistent_comment(self, pr_comment: str, initial_header: str, update_header: bool = True, name='review'): + def publish_persistent_comment(self, pr_comment: str, + initial_header: str, + update_header: bool = True, + name='review', + final_update_message=True): try: for comment in self.mr.notes.list(get_all=True)[::-1]: if comment.body.startswith(initial_header): @@ -164,8 +168,9 @@ def publish_persistent_comment(self, pr_comment: str, initial_header: str, updat pr_comment_updated = pr_comment get_logger().info(f"Persistent mode - updating comment {comment_url} to latest {name} message") response = self.mr.notes.update(comment.id, {'body': pr_comment_updated}) - self.publish_comment( - f"**[Persistent {name}]({comment_url})** updated to latest commit {latest_commit_url}") + if final_update_message: + self.publish_comment( + f"**[Persistent {name}]({comment_url})** updated to latest commit {latest_commit_url}") return except Exception as e: get_logger().exception(f"Failed to update persistent review, error: {e}") diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index 0f2dab3cb..243b12caf 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -36,6 +36,7 @@ ask_and_reflect=false #automatic_review=true persistent_comment=true extra_instructions = "" +final_update_message = true # review labels enable_review_labels_security=true enable_review_labels_effort=true diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index f73e20f2d..39eb904ea 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -121,9 +121,11 @@ async def run(self) -> None: if get_settings().config.publish_output: # publish the review 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="## PR Review", - update_header=True) + update_header=True, + final_update_message=final_update_message, ) else: self.git_provider.publish_comment(pr_review) From 359a15c84e6d4449bc39efad3c34c3af5ec9de0f Mon Sep 17 00:00:00 2001 From: mrT23 Date: Tue, 5 Mar 2024 17:33:29 +0200 Subject: [PATCH 2/4] Add 'final_update_message' option to control publishing of update message in persistent comments --- Usage.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Usage.md b/Usage.md index 3253985dd..0768da311 100644 --- a/Usage.md +++ b/Usage.md @@ -4,7 +4,7 @@ - [Introduction](#introduction) - [Configuration Options](#configuration-options) - [Managing Mail Notifications](#managing-mail-notifications) -- [Automation and Usage](#usage-types) +- [Usage and Automation](#usage-and-automation) - [Local Repo (CLI)](#working-from-a-local-repo-cli) - [Online Usage](#online-usage) - [GitHub App](#working-with-github-app) @@ -109,7 +109,7 @@ https://www.quora.com/How-can-you-filter-emails-for-specific-people-in-Gmail#:~: -## Usage Types +## Usage and Automation ### Working from a local repo (CLI) When running from your local repo (CLI), your local configuration file will be used. From d77db93f80ee0031121de3c69b00ccef153f2aff Mon Sep 17 00:00:00 2001 From: mrT23 Date: Tue, 5 Mar 2024 18:34:18 +0200 Subject: [PATCH 3/4] protections --- pr_agent/tools/pr_code_suggestions.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/pr_agent/tools/pr_code_suggestions.py b/pr_agent/tools/pr_code_suggestions.py index b0e076b0e..2cb06ccda 100644 --- a/pr_agent/tools/pr_code_suggestions.py +++ b/pr_agent/tools/pr_code_suggestions.py @@ -87,9 +87,14 @@ async def run(self): else: data = await retry_with_fallback_models(self._prepare_prediction_extended, ModelType.TURBO) - - if (not data) or (not 'code_suggestions' in data): - get_logger().info('No code suggestions found for PR.') + if (not data) or (not 'code_suggestions' in data) or (not data['code_suggestions']): + get_logger().error('No code suggestions found for PR.') + pr_body = "## PR Code Suggestions\n\nNo code suggestions found for PR." + get_logger().debug(f"PR output", artifact=pr_body) + if self.progress_response: + self.git_provider.edit_comment(self.progress_response, body=pr_body) + else: + self.git_provider.publish_comment(pr_body) return if (not self.is_extended and get_settings().pr_code_suggestions.rank_suggestions) or \ @@ -179,6 +184,7 @@ def _prepare_pr_code_suggestions(self) -> Dict: suggestion_list = [] one_sentence_summary_list = [] for i, suggestion in enumerate(data['code_suggestions']): + continue if get_settings().pr_code_suggestions.summarize: if not suggestion or 'one_sentence_summary' not in suggestion or 'label' not in suggestion or 'relevant_file' not in suggestion: get_logger().debug(f"Skipping suggestion {i + 1}, because it is invalid: {suggestion}") @@ -317,7 +323,7 @@ async def rank_suggestions(self, data: List) -> List: suggestion_list.append(suggestion) data_sorted = [[]] * len(suggestion_list) - if len(suggestion_list ) == 1: + if len(suggestion_list) == 1: return suggestion_list try: From c7eb70d00dbaa926445a0b85b592e61747bc83e0 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Tue, 5 Mar 2024 20:11:39 +0200 Subject: [PATCH 4/4] protections --- pr_agent/tools/pr_code_suggestions.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pr_agent/tools/pr_code_suggestions.py b/pr_agent/tools/pr_code_suggestions.py index 2cb06ccda..51dbe130c 100644 --- a/pr_agent/tools/pr_code_suggestions.py +++ b/pr_agent/tools/pr_code_suggestions.py @@ -184,7 +184,6 @@ def _prepare_pr_code_suggestions(self) -> Dict: suggestion_list = [] one_sentence_summary_list = [] for i, suggestion in enumerate(data['code_suggestions']): - continue if get_settings().pr_code_suggestions.summarize: if not suggestion or 'one_sentence_summary' not in suggestion or 'label' not in suggestion or 'relevant_file' not in suggestion: get_logger().debug(f"Skipping suggestion {i + 1}, because it is invalid: {suggestion}")