From 24583b05f74728a5a869f1150b3b8f4aec69ba23 Mon Sep 17 00:00:00 2001 From: zmeir Date: Mon, 17 Jul 2023 10:41:02 +0300 Subject: [PATCH 1/3] Publish GitHub review comments with single API call --- pr_agent/git_providers/github_provider.py | 19 +++++++++++---- pr_agent/tools/pr_reviewer.py | 28 +++++++++++++++++++---- requirements.txt | 2 +- 3 files changed, 39 insertions(+), 10 deletions(-) diff --git a/pr_agent/git_providers/github_provider.py b/pr_agent/git_providers/github_provider.py index 4f7d44bc..6d6d1c13 100644 --- a/pr_agent/git_providers/github_provider.py +++ b/pr_agent/git_providers/github_provider.py @@ -3,7 +3,7 @@ from datetime import datetime from typing import Optional, Tuple from urllib.parse import urlparse -from github import AppAuthentication, Github +from github import AppAuthentication, Github, Auth from pr_agent.config_loader import settings @@ -54,6 +54,10 @@ class GithubProvider(GitProvider): self.pr.comments_list.append(response) def publish_inline_comment(self, body: str, relevant_file: str, relevant_line_in_file: str): + logging.warning("Using deprecated `publish_inline_comment` - use `publish_inline_comments` instead") + self.publish_inline_comments([self.create_inline_comment(body, relevant_file, relevant_line_in_file)]) + + def create_inline_comment(self, body: str, relevant_file: str, relevant_line_in_file: str): self.diff_files = self.diff_files if self.diff_files else self.get_diff_files() position = -1 for file in self.diff_files: @@ -72,9 +76,16 @@ class GithubProvider(GitProvider): if position == -1: if settings.config.verbosity_level >= 2: logging.info(f"Could not find position for {relevant_file} {relevant_line_in_file}") + subject_type = "FILE" else: - path = relevant_file.strip() - self.pr.create_review_comment(body=body, commit_id=self.last_commit_id, path=path, position=position) + subject_type = "LINE" + path = relevant_file.strip() + # placeholder for future API support (already supported in single inline comment) + # return dict(body=body, path=path, position=position, subject_type=subject_type) + return dict(body=body, path=path, position=position) if subject_type == "LINE" else {} + + def publish_inline_comments(self, comments: list[dict]): + self.pr.create_review(commit=self.last_commit_id, comments=comments) def publish_code_suggestion(self, body: str, relevant_file: str, @@ -212,7 +223,7 @@ class GithubProvider(GitProvider): raise ValueError( "GitHub token is required when using user deployment. See: " "https://github.com/Codium-ai/pr-agent#method-2-run-from-source") from e - return Github(token) + return Github(auth=Auth.Token(token)) def _get_repo(self): return self.github_client.get_repo(self.repo) diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index afcb574b..fe4a6986 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -92,7 +92,13 @@ class PRReviewer: if settings.config.git_provider == 'github' and \ settings.pr_reviewer.inline_code_comments and \ 'Code suggestions' in data['PR Feedback']: - del data['PR Feedback']['Code suggestions'] + # keeping only code suggestions that can't be submitted as inline comments + data['PR Feedback']['Code suggestions'] = [ + d for d in data['PR Feedback']['Code suggestions'] + if any(key not in d for key in ('relevant file', 'relevant line in file', 'suggestion content')) + ] + if not data['PR Feedback']['Code suggestions']: + del data['PR Feedback']['Code suggestions'] markdown_text = convert_to_markdown(data) user = self.git_provider.get_user_id() @@ -118,9 +124,21 @@ class PRReviewer: except json.decoder.JSONDecodeError: data = try_fix_json(review) + comments = [] for d in data['PR Feedback']['Code suggestions']: - relevant_file = d['relevant file'].strip() - relevant_line_in_file = d['relevant line in file'].strip() - content = d['suggestion content'] + relevant_file = d.get('relevant file', '').strip() + relevant_line_in_file = d.get('relevant line in file', '').strip() + content = d.get('suggestion content', '') + if not relevant_file or not relevant_line_in_file or not content: + logging.info("Skipping inline comment with missing file/line/content") + continue - self.git_provider.publish_inline_comment(content, relevant_file, relevant_line_in_file) \ No newline at end of file + if settings.config.git_provider == 'github': + comment = self.git_provider.create_inline_comment(content, relevant_file, relevant_line_in_file) + if comment: + comments.append(comment) + else: + self.git_provider.publish_inline_comment(content, relevant_file, relevant_line_in_file) + + if comments: + self.git_provider.publish_inline_comments(comments) diff --git a/requirements.txt b/requirements.txt index 8695f709..399cd3ab 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,6 @@ dynaconf==3.1.12 fastapi==0.99.0 -PyGithub==1.58.2 +PyGithub==1.59.* retry==0.9.2 openai==0.27.8 Jinja2==3.1.2 From 13092118dc404e3b67a695decb271a4ee991cc75 Mon Sep 17 00:00:00 2001 From: Ori Kotek Date: Tue, 18 Jul 2023 12:26:49 +0300 Subject: [PATCH 2/3] Move the new git provider function to the abstract interface --- pr_agent/git_providers/bitbucket_provider.py | 8 +++++++- pr_agent/git_providers/git_provider.py | 8 ++++++++ pr_agent/git_providers/github_provider.py | 1 - pr_agent/git_providers/gitlab_provider.py | 8 +++++++- pr_agent/tools/pr_reviewer.py | 2 +- 5 files changed, 23 insertions(+), 4 deletions(-) diff --git a/pr_agent/git_providers/bitbucket_provider.py b/pr_agent/git_providers/bitbucket_provider.py index 86e445ac..0be04177 100644 --- a/pr_agent/git_providers/bitbucket_provider.py +++ b/pr_agent/git_providers/bitbucket_provider.py @@ -26,7 +26,7 @@ class BitbucketProvider: self.set_pr(pr_url) def is_supported(self, capability: str) -> bool: - if capability == 'get_issue_comments': + if capability in ['get_issue_comments', 'create_inline_comment', 'publish_inline_comments']: return False return True @@ -63,6 +63,12 @@ class BitbucketProvider: def publish_inline_comment(self, body: str, relevant_file: str, relevant_line_in_file: str): pass + def create_inline_comment(self, body: str, relevant_file: str, relevant_line_in_file: str): + raise NotImplementedError("Bitbucket provider does not support creating inline comments yet") + + def publish_inline_comments(self, comments: list[dict]): + raise NotImplementedError("Bitbucket provider does not support publishing inline comments yet") + def get_title(self): return self.pr.title diff --git a/pr_agent/git_providers/git_provider.py b/pr_agent/git_providers/git_provider.py index 4beba204..f9ab1d5c 100644 --- a/pr_agent/git_providers/git_provider.py +++ b/pr_agent/git_providers/git_provider.py @@ -41,6 +41,14 @@ class GitProvider(ABC): 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_suggestion(self, body: str, relevant_file: str, relevant_lines_start: int, relevant_lines_end: int): diff --git a/pr_agent/git_providers/github_provider.py b/pr_agent/git_providers/github_provider.py index 5d11c586..a04f550a 100644 --- a/pr_agent/git_providers/github_provider.py +++ b/pr_agent/git_providers/github_provider.py @@ -57,7 +57,6 @@ class GithubProvider(GitProvider): self.pr.comments_list.append(response) def publish_inline_comment(self, body: str, relevant_file: str, relevant_line_in_file: str): - logging.warning("Using deprecated `publish_inline_comment` - use `publish_inline_comments` instead") self.publish_inline_comments([self.create_inline_comment(body, relevant_file, relevant_line_in_file)]) def create_inline_comment(self, body: str, relevant_file: str, relevant_line_in_file: str): diff --git a/pr_agent/git_providers/gitlab_provider.py b/pr_agent/git_providers/gitlab_provider.py index 0149662b..7f371fd3 100644 --- a/pr_agent/git_providers/gitlab_provider.py +++ b/pr_agent/git_providers/gitlab_provider.py @@ -33,7 +33,7 @@ class GitLabProvider(GitProvider): r"^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@[ ]?(.*)") def is_supported(self, capability: str) -> bool: - if capability == 'get_issue_comments': + if capability in ['get_issue_comments', 'create_inline_comment', 'publish_inline_comments']: return False return True @@ -101,6 +101,12 @@ class GitLabProvider(GitProvider): self.send_inline_comment(body, edit_type, found, relevant_file, relevant_line_in_file, source_line_no, target_file, target_line_no) + def create_inline_comment(self, body: str, relevant_file: str, relevant_line_in_file: str): + raise NotImplementedError("Gitlab provider does not support creating inline comments yet") + + def create_inline_comment(self, comments: list[dict]): + raise NotImplementedError("Gitlab provider does not support publishing inline comments yet") + def send_inline_comment(self, body, edit_type, found, relevant_file, relevant_line_in_file, source_line_no, target_file, target_line_no): if not found: diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index 0e96fea3..b9008fa6 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -140,7 +140,7 @@ class PRReviewer: logging.info("Skipping inline comment with missing file/line/content") continue - if settings.config.git_provider == 'github': + if self.git_provider.is_supported("create_inline_comment"): comment = self.git_provider.create_inline_comment(content, relevant_file, relevant_line_in_file) if comment: comments.append(comment) From 05e4e09dfcb8c99428944e0dbd56add76df62cce Mon Sep 17 00:00:00 2001 From: Ori Kotek Date: Tue, 18 Jul 2023 12:27:28 +0300 Subject: [PATCH 3/3] Lint --- 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 b9008fa6..542e50b6 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -11,7 +11,7 @@ from pr_agent.algo.utils import convert_to_markdown, try_fix_json from pr_agent.config_loader import settings from pr_agent.git_providers import get_git_provider from pr_agent.git_providers.git_provider import get_main_pr_language -from pr_agent.servers.help import bot_help_text, actions_help_text +from pr_agent.servers.help import actions_help_text, bot_help_text class PRReviewer: @@ -51,7 +51,7 @@ class PRReviewer: async def review(self): logging.info('Reviewing PR...') if settings.config.publish_output: - self.git_provider.publish_comment("Preparing review...", is_temporary=True) + self.git_provider.publish_comment("Preparing review...", is_temporary=True) logging.info('Getting PR diff...') self.patches_diff = get_pr_diff(self.git_provider, self.token_handler) logging.info('Getting AI prediction...')