From 8038b6ab99381c54fd4ef89944541ab8233a7ef5 Mon Sep 17 00:00:00 2001 From: "Hussam.lawen" Date: Wed, 19 Jul 2023 14:22:34 +0300 Subject: [PATCH] refactor and clean --- pr_agent/agent/pr_agent.py | 18 +++--- pr_agent/algo/utils.py | 1 - pr_agent/cli.py | 6 +- pr_agent/git_providers/git_provider.py | 8 +++ pr_agent/git_providers/github_provider.py | 68 ++++++++++++----------- pr_agent/tools/pr_reviewer.py | 23 +++++--- 6 files changed, 68 insertions(+), 56 deletions(-) diff --git a/pr_agent/agent/pr_agent.py b/pr_agent/agent/pr_agent.py index 66b844d7..ea1c3f47 100644 --- a/pr_agent/agent/pr_agent.py +++ b/pr_agent/agent/pr_agent.py @@ -13,24 +13,20 @@ class PRAgent: pass async def handle_request(self, pr_url, request) -> bool: - if any(cmd in request for cmd in ["/answer"]): + action, *args = request.split(" ") + if any(cmd == action for cmd in ["/answer"]): await PRReviewer(pr_url, is_answer=True).review() - elif any(cmd in request for cmd in ["/review", "/review_pr", "/reflect_and_review"]): - words = request.split(" ") - incremental_review = False - if len(words) > 1: - arg = words[1] - if arg == "-i": - incremental_review = True + elif any(cmd == action for cmd in ["/review", "/review_pr", "/reflect_and_review"]): + incremental_review = await self.parse_args(request) if settings.pr_reviewer.ask_and_reflect or "/reflect_and_review" in request: await PRInformationFromUser(pr_url).generate_questions() else: await PRReviewer(pr_url, is_incremental=incremental_review).review() - elif any(cmd in request for cmd in ["/describe", "/describe_pr"]): + elif any(cmd == action for cmd in ["/describe", "/describe_pr"]): await PRDescription(pr_url).describe() - elif any(cmd in request for cmd in ["/improve", "/improve_code"]): + elif any(cmd == action for cmd in ["/improve", "/improve_code"]): await PRCodeSuggestions(pr_url).suggest() - elif any(cmd in request for cmd in ["/ask", "/ask_question"]): + elif any(cmd == action for cmd in ["/ask", "/ask_question"]): pattern = r'(/ask|/ask_question)\s*(.*)' matches = re.findall(pattern, request, re.IGNORECASE) if matches: diff --git a/pr_agent/algo/utils.py b/pr_agent/algo/utils.py index 76bb98c1..56148e9d 100644 --- a/pr_agent/algo/utils.py +++ b/pr_agent/algo/utils.py @@ -23,7 +23,6 @@ def convert_to_markdown(output_data: dict) -> str: "General PR suggestions": "💡", "Insights from user's answers": "📝", "Code suggestions": "🤖", - "Review for commits since previous PR-Agent review": "⏮️", } for key, value in output_data.items(): diff --git a/pr_agent/cli.py b/pr_agent/cli.py index acb331a1..35769a1e 100644 --- a/pr_agent/cli.py +++ b/pr_agent/cli.py @@ -57,11 +57,7 @@ reflect - Ask the PR author questions about the PR. asyncio.run(reviewer.suggest()) elif command in ['review', 'review_pr']: print(f"Reviewing PR: {args.pr_url}") - incremental_review = False - if len(args.rest) > 0: - incremental_review = args.rest[0].startswith("-i") - - reviewer = PRReviewer(args.pr_url, cli_mode=True, is_incremental=incremental_review) + reviewer = PRReviewer(args.pr_url, cli_mode=True, args=args.rest) asyncio.run(reviewer.review()) elif command in ['reflect']: print(f"Asking the PR author questions: {args.pr_url}") diff --git a/pr_agent/git_providers/git_provider.py b/pr_agent/git_providers/git_provider.py index f7c7fa98..32f6e315 100644 --- a/pr_agent/git_providers/git_provider.py +++ b/pr_agent/git_providers/git_provider.py @@ -121,3 +121,11 @@ def get_main_pr_language(languages, files) -> str: pass return main_language_str + + +class IncrementalPR: + def __init__(self, is_incremental: bool = False): + self.is_incremental = is_incremental + self.commits_range = None + self.first_new_commit_sha = None + self.last_seen_commit_sha = None diff --git a/pr_agent/git_providers/github_provider.py b/pr_agent/git_providers/github_provider.py index 415daa06..7e1a5d03 100644 --- a/pr_agent/git_providers/github_provider.py +++ b/pr_agent/git_providers/github_provider.py @@ -7,13 +7,13 @@ from github import AppAuthentication, Github, Auth from pr_agent.config_loader import settings -from .git_provider import FilePatchInfo, GitProvider +from .git_provider import FilePatchInfo, GitProvider, IncrementalPR from ..algo.language_handler import is_valid_file from ..algo.utils import load_large_diff class GithubProvider(GitProvider): - def __init__(self, pr_url: Optional[str] = None, incremental: Optional[bool] = False): + def __init__(self, pr_url: Optional[str] = None, incremental: Optional[IncrementalPR] = False): self.installation_id = settings.get("GITHUB.INSTALLATION_ID") self.github_client = self._get_github_client() self.repo = None @@ -32,38 +32,42 @@ class GithubProvider(GitProvider): def set_pr(self, pr_url: str): self.repo, self.pr_num = self._parse_pr_url(pr_url) self.pr = self._get_pr() - if self.incremental: - self.commits = list(self.pr.get_commits()) - self.comments = list(self.pr.get_issue_comments()) - self.previous_review = None - self.first_new_commit_sha = None - self.incremental_files = None + if self.incremental.is_incremental: + self.get_incremental_commits() - for index in range(len(self.comments) - 1, -1, -1): - if self.comments[index].user.login == "github-actions[bot]" or \ - self.comments[index].user.login == "CodiumAI-Agent" and \ - self.comments[index].body.startswith("## PR Analysis"): - self.previous_review = self.comments[index] - break - if self.previous_review: - last_review_time = self.previous_review.created_at - first_new_commit_index = 0 - self.last_seen_commit_sha = None - for index in range(len(self.commits) - 1, -1, -1): - if self.commits[index].commit.author.date > last_review_time: - self.first_new_commit_sha = self.commits[index].sha - first_new_commit_index = index - else: - self.last_seen_commit_sha = self.commits[index].sha - break + def get_incremental_commits(self): + self.commits = list(self.pr.get_commits()) - self.commits = self.commits[first_new_commit_index:] - self.file_set = dict() - for commit in self.commits: - self.file_set.update({file.filename: file for file in commit.files}) + self.get_previous_review() + if self.previous_review: + self.incremental.commits_range = self.get_commit_range() + # Get all files changed during the commit range + self.file_set = dict() + for commit in self.incremental.commits_range: + self.file_set.update({file.filename: file for file in commit.files}) + + def get_commit_range(self): + last_review_time = self.previous_review.created_at + first_new_commit_index = 0 + for index in range(len(self.commits) - 1, -1, -1): + if self.commits[index].commit.author.date > last_review_time: + self.incremental.first_new_commit_sha = self.commits[index].sha + first_new_commit_index = index + else: + self.incremental.last_seen_commit_sha = self.commits[index].sha + break + return self.commits[first_new_commit_index:] + + def get_previous_review(self): + self.previous_review = None + self.comments = list(self.pr.get_issue_comments()) + for index in range(len(self.comments) - 1, -1, -1): + if self.comments[index].body.startswith("## PR Analysis"): + self.previous_review = self.comments[index] + break def get_files(self): - if self.incremental and self.file_set: + if self.incremental.is_incremental and self.file_set: return self.file_set.values() return self.pr.get_files() @@ -74,8 +78,8 @@ class GithubProvider(GitProvider): if is_valid_file(file.filename): new_file_content_str = self._get_pr_file_content(file, self.pr.head.sha) patch = file.patch - if self.incremental and self.file_set: - original_file_content_str = self._get_pr_file_content(file, self.last_seen_commit_sha) + if self.incremental.is_incremental and self.file_set: + original_file_content_str = self._get_pr_file_content(file, self.incremental.last_seen_commit_sha) patch = load_large_diff(file, new_file_content_str, original_file_content_str, diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index 4488d360..c4630b35 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -11,20 +11,20 @@ from pr_agent.algo.token_handler import TokenHandler 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.git_providers.git_provider import get_main_pr_language, IncrementalPR from pr_agent.servers.help import actions_help_text, bot_help_text class PRReviewer: - def __init__(self, pr_url: str, cli_mode=False, is_answer: bool = False, is_incremental: bool = False): + def __init__(self, pr_url: str, cli_mode=False, is_answer: bool = False, args=None): + self.parse_args(args) - self.git_provider = get_git_provider()(pr_url, incremental=is_incremental) + self.git_provider = get_git_provider()(pr_url, incremental=self.incremental) self.main_language = get_main_pr_language( self.git_provider.get_languages(), self.git_provider.get_files() ) self.pr_url = pr_url self.is_answer = is_answer - self.is_incremental = is_incremental if self.is_answer and not self.git_provider.is_supported("get_issue_comments"): raise Exception(f"Answer mode is not supported for {settings.config.git_provider} for now") answer_str, question_str = self._get_user_answers() @@ -51,6 +51,14 @@ class PRReviewer: settings.pr_review_prompt.system, settings.pr_review_prompt.user) + def parse_args(self, args): + is_incremental = False + if len(args) >= 1: + arg = args[0] + if arg == "-i": + is_incremental = True + self.incremental = IncrementalPR(is_incremental) + async def review(self): logging.info('Reviewing PR...') if settings.config.publish_output: @@ -110,11 +118,12 @@ class PRReviewer: if not data['PR Feedback']['Code suggestions']: del data['PR Feedback']['Code suggestions'] - if self.is_incremental: + if self.incremental.is_incremental: # Rename title when incremental review - Add to the beginning of the dict - last_commit_url = f"{self.pr_url}/commits/{self.git_provider.first_new_commit_sha}" + last_commit_url = f"{self.pr_url}/commits/{self.git_provider.incremental.first_new_commit_sha}" data = OrderedDict(data) - data.update({'Incremental PR Review': {"Review for commits since previous PR-Agent review": f"Starting from commit {last_commit_url}"}}) + data.update({'Incremental PR Review': { + "⏮️ Review for commits since previous PR-Agent review": f"Starting from commit {last_commit_url}"}}) data.move_to_end('Incremental PR Review', last=False) markdown_text = convert_to_markdown(data)