diff --git a/docs/REVIEW.md b/docs/REVIEW.md index 7f2f42cbc..ac4d1b4a8 100644 --- a/docs/REVIEW.md +++ b/docs/REVIEW.md @@ -29,12 +29,26 @@ Under the section 'pr_reviewer', the [configuration file](./../pr_agent/settings #### Incremental Mode For an incremental review, which only considers changes since the last PR-Agent review, this can be useful when working on the PR in an iterative manner, and you want to focus on the changes since the last review instead of reviewing the entire PR again, the following command can be used: ``` -/improve -i +/review -i ``` Note that the incremental mode is only available for GitHub. +Under the section 'pr_reviewer', the [configuration file](./../pr_agent/settings/configuration.toml#L16) contains options to customize the 'review -i' tool. +These configurations can be used to control the rate at which the incremental review tool will create new review comments when invoked automatically, to prevent making too much noise in the PR. +- `minimal_commits_for_incremental_review`: Minimal number of commits since the last review that are required to create incremental review. +If there are less than the specified number of commits since the last review, the tool will not perform any action. +Default is 0 - the tool will always run, no matter how many commits since the last review. +- `minimal_minutes_for_incremental_review`: Minimal number of minutes that need to pass since the last reviewed commit to create incremental review. +If less that the specified number of minutes have passed between the last reviewed commit and running this command, the tool will not perform any action. +Default is 0 - the tool will always run, no matter how much time have passed since the last reviewed commit. +- `require_all_thresholds_for_incremental_review`: If set to true, all the previous thresholds must be met for incremental review to run. If false, only one is enough to run the tool. +For example, if `minimal_commits_for_incremental_review=2` and `minimal_minutes_for_incremental_review=2`, and we have 3 commits since the last review, but the last reviewed commit is from 1 minute ago: +When `require_all_thresholds_for_incremental_review=true` the incremental review __will not__ run, because only 1 out of 2 conditions were met (we have enough commits but the last review is too recent), +but when `require_all_thresholds_for_incremental_review=false` the incremental review __will__ run, because one condition is enough (we have 3 commits which is more than the configured 2). +Default is false - the tool will run as long as at least once conditions is met. + #### PR Reflection By invoking: ``` diff --git a/pr_agent/git_providers/git_provider.py b/pr_agent/git_providers/git_provider.py index 9dee9e3c8..3511c2a66 100644 --- a/pr_agent/git_providers/git_provider.py +++ b/pr_agent/git_providers/git_provider.py @@ -190,6 +190,13 @@ 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 + self.first_new_commit = None + self.last_seen_commit = None + @property + def first_new_commit_sha(self): + return None if self.first_new_commit is None else self.first_new_commit.sha + + @property + def last_seen_commit_sha(self): + return None if self.last_seen_commit is None else self.last_seen_commit.sha diff --git a/pr_agent/git_providers/github_provider.py b/pr_agent/git_providers/github_provider.py index 40b0e1210..5ed35d044 100644 --- a/pr_agent/git_providers/github_provider.py +++ b/pr_agent/git_providers/github_provider.py @@ -66,10 +66,10 @@ def get_commit_range(self): first_new_commit_index = None 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 + self.incremental.first_new_commit = self.commits[index] first_new_commit_index = index else: - self.incremental.last_seen_commit_sha = self.commits[index].sha + self.incremental.last_seen_commit = self.commits[index] break return self.commits[first_new_commit_index:] if first_new_commit_index is not None else [] diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index ffe6d39d3..b0786cd3e 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -26,6 +26,10 @@ ask_and_reflect=false automatic_review=true remove_previous_review_comment=false extra_instructions = "" +# specific configurations for incremental review (/review -i) +require_all_thresholds_for_incremental_review=false +minimal_commits_for_incremental_review=0 +minimal_minutes_for_incremental_review=0 [pr_description] # /describe # publish_labels=true @@ -105,6 +109,9 @@ push_commands = [ --pr_reviewer.num_code_suggestions=0 \ --pr_reviewer.inline_code_comments=false \ --pr_reviewer.remove_previous_review_comment=true \ + --pr_reviewer.require_all_thresholds_for_incremental_review=false \ + --pr_reviewer.minimal_commits_for_incremental_review=5 \ + --pr_reviewer.minimal_minutes_for_incremental_review=30 \ --pr_reviewer.extra_instructions='' \ """ ] diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index b5091a6a6..29087a52a 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -1,4 +1,5 @@ import copy +import datetime from collections import OrderedDict from typing import List, Tuple @@ -100,8 +101,7 @@ async def run(self) -> None: if self.is_auto and not get_settings().pr_reviewer.automatic_review: get_logger().info(f'Automatic review is disabled {self.pr_url}') return None - if self.is_auto and self.incremental.is_incremental and not self.incremental.first_new_commit_sha: - get_logger().info(f"Incremental review is enabled for {self.pr_url} but there are no new commits") + if self.incremental.is_incremental and not self._can_run_incremental_review(): return None get_logger().info(f'Reviewing PR: {self.pr_url} ...') @@ -334,3 +334,34 @@ def _remove_previous_review_comment(self, comment): self.git_provider.remove_comment(comment) except Exception as e: get_logger().exception(f"Failed to remove previous review comment, error: {e}") + + def _can_run_incremental_review(self) -> bool: + """Checks if we can run incremental review according the various configurations and previous review""" + # checking if running is auto mode but there are no new commits + if self.is_auto and not self.incremental.first_new_commit_sha: + get_logger().info(f"Incremental review is enabled for {self.pr_url} but there are no new commits") + return False + # checking if there are enough commits to start the review + num_new_commits = len(self.incremental.commits_range) + num_commits_threshold = get_settings().pr_reviewer.minimal_commits_for_incremental_review + not_enough_commits = num_new_commits < num_commits_threshold + # checking if the commits are not too recent to start the review + recent_commits_threshold = datetime.datetime.now() - datetime.timedelta( + minutes=get_settings().pr_reviewer.minimal_minutes_for_incremental_review + ) + last_seen_commit_date = ( + self.incremental.last_seen_commit.commit.author.date if self.incremental.last_seen_commit else None + ) + all_commits_too_recent = ( + last_seen_commit_date > recent_commits_threshold if self.incremental.last_seen_commit else False + ) + # check all the thresholds or just one to start the review + condition = any if get_settings().pr_reviewer.require_all_thresholds_for_incremental_review else all + if condition((not_enough_commits, all_commits_too_recent)): + get_logger().info( + f"Incremental review is enabled for {self.pr_url} but didn't pass the threshold check to run:" + f"\n* Number of new commits = {num_new_commits} (threshold is {num_commits_threshold})" + f"\n* Last seen commit date = {last_seen_commit_date} (threshold is {recent_commits_threshold})" + ) + return False + return True