From 3386ab4e7541f6eadaa0b9299a6352a339476306 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Wed, 15 Nov 2023 09:06:26 +0200 Subject: [PATCH 1/5] more feedback --- pr_agent/git_providers/bitbucket_provider.py | 9 +++++---- pr_agent/git_providers/git_provider.py | 2 +- pr_agent/git_providers/github_provider.py | 12 ++++++++---- pr_agent/git_providers/gitlab_provider.py | 9 +++++---- pr_agent/tools/pr_description.py | 3 +++ pr_agent/tools/pr_reviewer.py | 7 +++++-- 6 files changed, 27 insertions(+), 15 deletions(-) diff --git a/pr_agent/git_providers/bitbucket_provider.py b/pr_agent/git_providers/bitbucket_provider.py index 47f2b32a8..bef33ae55 100644 --- a/pr_agent/git_providers/bitbucket_provider.py +++ b/pr_agent/git_providers/bitbucket_provider.py @@ -153,13 +153,14 @@ def get_diff_files(self) -> list[FilePatchInfo]: self.diff_files = diff_files return diff_files - def publish_persistent_comment(self, pr_comment: str, initial_text: str, updated_text: str): + def publish_persistent_comment(self, pr_comment: str, initial_header: str, update_header: str = True): try: for comment in self.pr.comments(): body = comment.raw - if initial_text in body: - if updated_text: - pr_comment_updated = pr_comment.replace(initial_text, updated_text) + if initial_header in body: + if update_header: + updated_header = f"{initial_header}\n\n ### (updated)\n" + pr_comment_updated = pr_comment.replace(initial_header, updated_header) else: pr_comment_updated = pr_comment d = {"content": {"raw": pr_comment_updated}} diff --git a/pr_agent/git_providers/git_provider.py b/pr_agent/git_providers/git_provider.py index 4a41e252c..98b6e4f11 100644 --- a/pr_agent/git_providers/git_provider.py +++ b/pr_agent/git_providers/git_provider.py @@ -44,7 +44,7 @@ def publish_description(self, pr_title: str, pr_body: str): def publish_comment(self, pr_comment: str, is_temporary: bool = False): pass - def publish_persistent_comment(self, pr_comment: str, initial_text: str, updated_text: str): + def publish_persistent_comment(self, pr_comment: str, initial_header: str, update_header: bool): 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 c0b9cc114..529247256 100644 --- a/pr_agent/git_providers/github_provider.py +++ b/pr_agent/git_providers/github_provider.py @@ -154,16 +154,20 @@ def get_diff_files(self) -> list[FilePatchInfo]: def publish_description(self, pr_title: str, pr_body: str): self.pr.edit(title=pr_title, body=pr_body) - def publish_persistent_comment(self, pr_comment: str, initial_text: str, updated_text: str): + def publish_persistent_comment(self, pr_comment: str, initial_header: str, update_header: bool=True): prev_comments = list(self.pr.get_issue_comments()) for comment in prev_comments: body = comment.body - if body.startswith(initial_text): - if updated_text: - pr_comment_updated = pr_comment.replace(initial_text, updated_text) + if body.startswith(initial_header): + latest_commit = self.pr.get_commits().reversed[0].html_url + if update_header: + updated_text = f"{initial_header}\n\n### (review updated to commit {latest_commit})\n" + pr_comment_updated = pr_comment.replace(initial_header, updated_text) else: pr_comment_updated = pr_comment + get_logger().info(f"Persistent mode- updating comment {comment.html_url} to latest review message") response = comment.edit(pr_comment_updated) + self.publish_comment(f"**[Persistent review]({comment.html_url})** updated to latest commit {latest_commit}") 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 396483a5f..a4d2090c5 100644 --- a/pr_agent/git_providers/gitlab_provider.py +++ b/pr_agent/git_providers/gitlab_provider.py @@ -136,12 +136,13 @@ def publish_description(self, pr_title: str, pr_body: str): except Exception as e: get_logger().exception(f"Could not update merge request {self.id_mr} description: {e}") - def publish_persistent_comment(self, pr_comment: str, initial_text: str, updated_text: str): + def publish_persistent_comment(self, pr_comment: str, initial_header: str, update_header: str = True): try: for comment in self.mr.notes.list(get_all=True)[::-1]: - if comment.body.startswith(initial_text): - if updated_text: - pr_comment_updated = pr_comment.replace(initial_text, updated_text) + if comment.body.startswith(initial_header): + if update_header: + updated_header = f"{initial_header}\n\n ### (updated)\n" + pr_comment_updated = pr_comment.replace(initial_header, updated_header) else: pr_comment_updated = pr_comment response = self.mr.notes.update(comment.id, {'body': pr_comment_updated}) diff --git a/pr_agent/tools/pr_description.py b/pr_agent/tools/pr_description.py index 611523eaf..47e3f03f1 100644 --- a/pr_agent/tools/pr_description.py +++ b/pr_agent/tools/pr_description.py @@ -158,6 +158,9 @@ async def _get_prediction(self, model: str) -> str: user=user_prompt ) + if get_settings().config.verbosity_level >= 2: + get_logger().info(f"\nAI response:\n{response}") + return response def _prepare_data(self): diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index 5b8e5472b..7a839ce0d 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -121,8 +121,8 @@ async def run(self) -> None: # publish the review if get_settings().pr_reviewer.persistent_comment and not self.incremental.is_incremental: self.git_provider.publish_persistent_comment(pr_comment, - initial_text="## PR Analysis", - updated_text="## PR Analysis (updated)") + initial_header="## PR Analysis", + update_header=True) else: self.git_provider.publish_comment(pr_comment) @@ -178,6 +178,9 @@ async def _get_prediction(self, model: str) -> str: user=user_prompt ) + if get_settings().config.verbosity_level >= 2: + get_logger().info(f"\nAI response:\n{response}") + return response def _prepare_pr_review(self) -> str: From fb86441caba8fd97e8620c236f646351a5fdef1a Mon Sep 17 00:00:00 2001 From: mrT23 Date: Wed, 15 Nov 2023 09:40:45 +0200 Subject: [PATCH 2/5] gitlab --- pr_agent/git_providers/bitbucket_provider.py | 2 +- pr_agent/git_providers/git_provider.py | 83 +++++++++++--------- pr_agent/git_providers/github_provider.py | 20 +++-- pr_agent/git_providers/gitlab_provider.py | 14 +++- 4 files changed, 73 insertions(+), 46 deletions(-) diff --git a/pr_agent/git_providers/bitbucket_provider.py b/pr_agent/git_providers/bitbucket_provider.py index bef33ae55..5d1d92d7c 100644 --- a/pr_agent/git_providers/bitbucket_provider.py +++ b/pr_agent/git_providers/bitbucket_provider.py @@ -153,7 +153,7 @@ def get_diff_files(self) -> list[FilePatchInfo]: self.diff_files = diff_files return diff_files - def publish_persistent_comment(self, pr_comment: str, initial_header: str, update_header: str = True): + def publish_persistent_comment(self, pr_comment: str, initial_header: str, update_header: bool = True): try: for comment in self.pr.comments(): body = comment.raw diff --git a/pr_agent/git_providers/git_provider.py b/pr_agent/git_providers/git_provider.py index 98b6e4f11..05122f9c3 100644 --- a/pr_agent/git_providers/git_provider.py +++ b/pr_agent/git_providers/git_provider.py @@ -40,45 +40,10 @@ def get_diff_files(self) -> list[FilePatchInfo]: def publish_description(self, pr_title: str, pr_body: str): pass - @abstractmethod - 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): - self.publish_comment(pr_comment) - - @abstractmethod - def publish_inline_comment(self, body: str, relevant_file: str, relevant_line_in_file: str): - pass - - @abstractmethod - def create_inline_comment(self, body: str, relevant_file: str, relevant_line_in_file: str): - pass - - @abstractmethod - def publish_inline_comments(self, comments: list[dict]): - pass - @abstractmethod def publish_code_suggestions(self, code_suggestions: list) -> bool: pass - @abstractmethod - def publish_labels(self, labels): - pass - - @abstractmethod - def get_labels(self): - pass - - @abstractmethod - def remove_initial_comment(self): - pass - - @abstractmethod - def remove_comment(self, comment): - pass - @abstractmethod def get_languages(self): pass @@ -116,12 +81,55 @@ def get_user_description(self) -> str: # otherwise, extract the original user description from the existing pr-agent description and return it return description.split("## User Description:", 1)[1].strip() + @abstractmethod + def get_repo_settings(self): + pass + + def get_pr_id(self): + return "" + + #### comments operations #### + @abstractmethod + 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): + self.publish_comment(pr_comment) + + @abstractmethod + def publish_inline_comment(self, body: str, relevant_file: str, relevant_line_in_file: str): + pass + + @abstractmethod + def create_inline_comment(self, body: str, relevant_file: str, relevant_line_in_file: str): + pass + + @abstractmethod + def publish_inline_comments(self, comments: list[dict]): + pass + + @abstractmethod + def remove_initial_comment(self): + pass + + @abstractmethod + def remove_comment(self, comment): + pass + @abstractmethod def get_issue_comments(self): pass + def get_comment_url(self, comment) -> str: + return "" + + #### labels operations #### @abstractmethod - def get_repo_settings(self): + def publish_labels(self, labels): + pass + + @abstractmethod + def get_labels(self): pass @abstractmethod @@ -132,11 +140,12 @@ def add_eyes_reaction(self, issue_comment_id: int) -> Optional[int]: def remove_reaction(self, issue_comment_id: int, reaction_id: int) -> bool: pass + #### commits operations #### @abstractmethod def get_commit_messages(self): pass - def get_pr_id(self): + def get_latest_commit_url(self) -> str: return "" def get_main_pr_language(languages, files) -> str: diff --git a/pr_agent/git_providers/github_provider.py b/pr_agent/git_providers/github_provider.py index 529247256..c9abe2b19 100644 --- a/pr_agent/git_providers/github_provider.py +++ b/pr_agent/git_providers/github_provider.py @@ -154,20 +154,28 @@ def get_diff_files(self) -> list[FilePatchInfo]: def publish_description(self, pr_title: str, pr_body: str): self.pr.edit(title=pr_title, body=pr_body) - def publish_persistent_comment(self, pr_comment: str, initial_header: str, update_header: bool=True): + def get_latest_commit_url(self) -> str: + return self.last_commit_id.html_url + + 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): prev_comments = list(self.pr.get_issue_comments()) for comment in prev_comments: body = comment.body if body.startswith(initial_header): - latest_commit = self.pr.get_commits().reversed[0].html_url + latest_commit_url = self.get_latest_commit_url() + comment_url = self.get_comment_url(comment) if update_header: - updated_text = f"{initial_header}\n\n### (review updated to commit {latest_commit})\n" - pr_comment_updated = pr_comment.replace(initial_header, updated_text) + updated_header = f"{initial_header}\n\n### (review updated until commit {latest_commit_url})\n" + pr_comment_updated = pr_comment.replace(initial_header, updated_header) else: pr_comment_updated = pr_comment - get_logger().info(f"Persistent mode- updating comment {comment.html_url} to latest review message") + 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 review]({comment.html_url})** updated to latest commit {latest_commit}") + self.publish_comment( + f"**[Persistent review]({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 a4d2090c5..2dc9d3ed7 100644 --- a/pr_agent/git_providers/gitlab_provider.py +++ b/pr_agent/git_providers/gitlab_provider.py @@ -136,16 +136,26 @@ def publish_description(self, pr_title: str, pr_body: str): except Exception as e: get_logger().exception(f"Could not update merge request {self.id_mr} description: {e}") - def publish_persistent_comment(self, pr_comment: str, initial_header: str, update_header: str = True): + def get_latest_commit_url(self): + return self.mr.commits().next().web_url + + 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): try: for comment in self.mr.notes.list(get_all=True)[::-1]: if comment.body.startswith(initial_header): + latest_commit_url = self.get_latest_commit_url() + comment_url = self.get_comment_url(comment) if update_header: - updated_header = f"{initial_header}\n\n ### (updated)\n" + updated_header = f"{initial_header}\n\n### (review updated until commit {latest_commit_url})\n" pr_comment_updated = pr_comment.replace(initial_header, updated_header) else: pr_comment_updated = pr_comment response = self.mr.notes.update(comment.id, {'body': pr_comment_updated}) + self.publish_comment( + f"**[Persistent review]({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}") From 04debba6ffefed9bacc71fa6b0f0a48b543c85a5 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Wed, 15 Nov 2023 09:45:10 +0200 Subject: [PATCH 3/5] gitlab --- pr_agent/git_providers/gitlab_provider.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pr_agent/git_providers/gitlab_provider.py b/pr_agent/git_providers/gitlab_provider.py index 2dc9d3ed7..078ca9dd8 100644 --- a/pr_agent/git_providers/gitlab_provider.py +++ b/pr_agent/git_providers/gitlab_provider.py @@ -153,6 +153,7 @@ def publish_persistent_comment(self, pr_comment: str, initial_header: str, updat pr_comment_updated = pr_comment.replace(initial_header, updated_header) else: pr_comment_updated = pr_comment + get_logger().info(f"Persistent mode- updating comment {comment_url} to latest review message") response = self.mr.notes.update(comment.id, {'body': pr_comment_updated}) self.publish_comment( f"**[Persistent review]({comment_url})** updated to latest commit {latest_commit_url}") From 0dff8e769350a06c112fe3295abca26c0802e336 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Wed, 15 Nov 2023 12:11:02 +0200 Subject: [PATCH 4/5] bitbucket --- pr_agent/algo/pr_processing.py | 2 +- pr_agent/git_providers/bitbucket_provider.py | 13 ++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/pr_agent/algo/pr_processing.py b/pr_agent/algo/pr_processing.py index e5b6f59e3..6063deced 100644 --- a/pr_agent/algo/pr_processing.py +++ b/pr_agent/algo/pr_processing.py @@ -282,7 +282,7 @@ def find_line_number_of_relevant_line_in_file(diff_files: List[FilePatchInfo], r"^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@[ ]?(.*)") for file in diff_files: - if file.filename.strip() == relevant_file: + if file.filename and (file.filename.strip() == relevant_file): patch = file.patch patch_lines = patch.splitlines() diff --git a/pr_agent/git_providers/bitbucket_provider.py b/pr_agent/git_providers/bitbucket_provider.py index 5d1d92d7c..e2431645d 100644 --- a/pr_agent/git_providers/bitbucket_provider.py +++ b/pr_agent/git_providers/bitbucket_provider.py @@ -153,18 +153,29 @@ def get_diff_files(self) -> list[FilePatchInfo]: self.diff_files = diff_files return diff_files + def get_latest_commit_url(self): + return self.pr.data['source']['commit']['links']['html']['href'] + + 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): try: for comment in self.pr.comments(): body = comment.raw if initial_header in body: + latest_commit_url = self.get_latest_commit_url() + comment_url = self.get_comment_url(comment) if update_header: - updated_header = f"{initial_header}\n\n ### (updated)\n" + updated_header = f"{initial_header}\n\n### (review updated until commit {latest_commit_url})\n" pr_comment_updated = pr_comment.replace(initial_header, updated_header) else: pr_comment_updated = pr_comment + get_logger().info(f"Persistent mode- updating comment {comment_url} to latest review message") d = {"content": {"raw": pr_comment_updated}} response = comment._update_data(comment.put(None, data=d)) + self.publish_comment( + f"**[Persistent review]({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}") From 1dc424c09af82d7e247c3e8441d7a18027b3be3f Mon Sep 17 00:00:00 2001 From: mrT23 Date: Wed, 15 Nov 2023 13:32:32 +0200 Subject: [PATCH 5/5] docs --- docs/REVIEW.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/REVIEW.md b/docs/REVIEW.md index 342504e20..82eef6c39 100644 --- a/docs/REVIEW.md +++ b/docs/REVIEW.md @@ -16,16 +16,18 @@ The `review` tool can also be triggered automatically every time a new PR is ope Under the section 'pr_reviewer', the [configuration file](./../pr_agent/settings/configuration.toml#L16) contains options to customize the 'review' tool: +#### 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. - `require_tests_review`: if set to true, the tool will add a section that checks if the PR contains tests. Default is true. - `require_security_review`: if set to true, the tool will add a section that checks if the PR contains security issues. Default is true. - `require_estimate_effort_to_review`: if set to true, the tool will add a section that estimates thed effort needed to review the PR. Default is true. +#### general options - `num_code_suggestions`: number of code suggestions provided by the 'review' tool. Default is 4. - `inline_code_comments`: if set to true, the tool will publish the code suggestions as comments on the code diff. Default is false. - `automatic_review`: if set to false, no automatic reviews will be done. Default is true. - `remove_previous_review_comment`: if set to true, the tool will remove the previous review comment before adding a new one. Default is false. -- `persistent_comment`: if set to true, the review comment will be persistent. Default is true. +- `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 ...". - To enable `custom labels`, apply the configuration changes described [here](./GENERATE_CUSTOM_LABELS.md#configuration-changes) #### Incremental Mode