From 0189e12fb159f73c64e64a244883824e58381be4 Mon Sep 17 00:00:00 2001 From: zmeir Date: Wed, 3 Jan 2024 14:21:38 +0200 Subject: [PATCH 1/8] Automatically enable improve extended mode for large PRs --- docs/IMPROVE.md | 4 ++++ pr_agent/settings/configuration.toml | 4 ++++ pr_agent/tools/pr_code_suggestions.py | 17 ++++++++++++++++- 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/docs/IMPROVE.md b/docs/IMPROVE.md index cb4b47dd..19fa038e 100644 --- a/docs/IMPROVE.md +++ b/docs/IMPROVE.md @@ -29,6 +29,10 @@ Under the section 'pr_code_suggestions', the [configuration file](./../pr_agent/ - `include_improved_code`: if set to true, the tool will include an improved code implementation in the suggestion. Default is true. #### params for '/improve --extended' mode +- `auto_extended_mode`: enable extended mode automatically for large PRs. Default is true. +- `auto_extended_mode_min_files`: minimum number of files in the PR to automatically enable extended mode. Default is 1. +- `auto_extended_mode_min_additions`: minimum number of line additions in the PR to automatically enable extended mode. Default is 500. +- `auto_extended_mode_min_deletions`: minimum number of line deletions in the PR to automatically enable extended mode. Default is 0. - `num_code_suggestions_per_chunk`: number of code suggestions provided by the 'improve' tool, per chunk. Default is 8. - `rank_extended_suggestions`: if set to true, the tool will rank the suggestions, based on importance. Default is true. - `max_number_of_calls`: maximum number of chunks. Default is 5. diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index 04466c72..3a868c7b 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -71,6 +71,10 @@ include_improved_code = true extra_instructions = "" rank_suggestions = false # params for '/improve --extended' mode +auto_extended_mode=true +auto_extended_mode_min_files=1 +auto_extended_mode_min_additions=500 +auto_extended_mode_min_deletions=0 num_code_suggestions_per_chunk=8 rank_extended_suggestions = true max_number_of_calls = 5 diff --git a/pr_agent/tools/pr_code_suggestions.py b/pr_agent/tools/pr_code_suggestions.py index b5006b85..1e4f25a8 100644 --- a/pr_agent/tools/pr_code_suggestions.py +++ b/pr_agent/tools/pr_code_suggestions.py @@ -26,7 +26,7 @@ class PRCodeSuggestions: # extended mode try: - self.is_extended = any(["extended" in arg for arg in args]) + self.is_extended = self._get_is_extended(args or []) except: self.is_extended = False if self.is_extended: @@ -206,6 +206,21 @@ class PRCodeSuggestions: return new_code_snippet + def _get_is_extended(self, args: list[str]) -> bool: + """Check if extended mode should be enabled by the `--extended` flag or automatically according to the PR""" + if any(["extended" in arg for arg in args]): + get_logger().info("Extended mode is enabled by the `--extended` flag") + return True + if ( + get_settings().pr_code_suggestions.auto_extended_mode + and self.git_provider.pr.changed_files >= get_settings().pr_code_suggestions.auto_extended_mode_min_files + and self.git_provider.pr.additions >= get_settings().pr_code_suggestions.auto_extended_mode_min_additions + and self.git_provider.pr.deletions >= get_settings().pr_code_suggestions.auto_extended_mode_min_deletions + ): + get_logger().info("Extended mode is enabled automatically based on the PR size") + return True + return False + async def _prepare_prediction_extended(self, model: str) -> dict: get_logger().info('Getting PR diff...') patches_diff_list = get_pr_multi_diffs(self.git_provider, self.token_handler, model, From 2f9fbbf0ac32d2be809c34613d34826529bca924 Mon Sep 17 00:00:00 2001 From: zmeir Date: Wed, 3 Jan 2024 16:26:32 +0200 Subject: [PATCH 2/8] Prevent reducing the number of suggestions if already low enough --- pr_agent/tools/pr_code_suggestions.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pr_agent/tools/pr_code_suggestions.py b/pr_agent/tools/pr_code_suggestions.py index 1e4f25a8..4490d02c 100644 --- a/pr_agent/tools/pr_code_suggestions.py +++ b/pr_agent/tools/pr_code_suggestions.py @@ -286,8 +286,14 @@ class PRCodeSuggestions: data_sorted[importance_order - 1] = suggestion_list[suggestion_number - 1] if get_settings().pr_code_suggestions.final_clip_factor != 1: - new_len = int(0.5 + len(data_sorted) * get_settings().pr_code_suggestions.final_clip_factor) - data_sorted = data_sorted[:new_len] + max_len = max( + len(data_sorted), + get_settings().pr_code_suggestions.num_code_suggestions, + get_settings().pr_code_suggestions.num_code_suggestions_per_chunk, + ) + new_len = int(0.5 + max_len * get_settings().pr_code_suggestions.final_clip_factor) + if new_len < len(data_sorted): + data_sorted = data_sorted[:new_len] except Exception as e: if get_settings().config.verbosity_level >= 1: get_logger().info(f"Could not sort suggestions, error: {e}") From 1ade09eaa3f429c213480d5f50926c9bd1e0e476 Mon Sep 17 00:00:00 2001 From: zmeir Date: Thu, 4 Jan 2024 14:49:34 +0200 Subject: [PATCH 3/8] Fix failing `/config` command All commands need the `ai_handler` argument. The PRConfig class was missing it in the `__init__` method and so it failed with this error: ``` File "/home/vcap/app/pr_agent/agent/pr_agent.py", line 76, in handle_request await command2class[action](pr_url, ai_handler=self.ai_handler, args=args).run() TypeError: PRConfig.__init__() got an unexpected keyword argument 'ai_handler' ``` --- pr_agent/tools/pr_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pr_agent/tools/pr_config.py b/pr_agent/tools/pr_config.py index 44ab7e23..1d18f9d0 100644 --- a/pr_agent/tools/pr_config.py +++ b/pr_agent/tools/pr_config.py @@ -7,7 +7,7 @@ class PRConfig: """ The PRConfig class is responsible for listing all configuration options available for the user. """ - def __init__(self, pr_url: str, args=None): + def __init__(self, pr_url: str, args=None, ai_handler=None): """ Initialize the PRConfig object with the necessary attributes and objects to comment on a pull request. From ac7aaa0cd357deb4d074e9b2f3d4ab1b44acca1f Mon Sep 17 00:00:00 2001 From: zmeir Date: Thu, 4 Jan 2024 16:08:10 +0200 Subject: [PATCH 4/8] Add support for Azure OpenAI in LangChainOpenAIHandler --- .../algo/ai_handlers/langchain_ai_handler.py | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/pr_agent/algo/ai_handlers/langchain_ai_handler.py b/pr_agent/algo/ai_handlers/langchain_ai_handler.py index 3e31bcb8..a7c6d345 100644 --- a/pr_agent/algo/ai_handlers/langchain_ai_handler.py +++ b/pr_agent/algo/ai_handlers/langchain_ai_handler.py @@ -1,5 +1,5 @@ try: - from langchain.chat_models import ChatOpenAI + from langchain.chat_models import ChatOpenAI, AzureChatOpenAI from langchain.schema import SystemMessage, HumanMessage except: # we don't enforce langchain as a dependency, so if it's not installed, just move on pass @@ -9,23 +9,41 @@ from pr_agent.config_loader import get_settings from pr_agent.log import get_logger from openai.error import APIError, RateLimitError, Timeout, TryAgain -from retry import retry +from retry import retry +import functools OPENAI_RETRIES = 5 class LangChainOpenAIHandler(BaseAiHandler): def __init__(self): # Initialize OpenAIHandler specific attributes here + super().__init__() + self.azure = get_settings().get("OPENAI.API_TYPE", "").lower() == "azure" try: - super().__init__() - self._chat = ChatOpenAI(openai_api_key=get_settings().openai.key) - + if self.azure: + # using a partial function so we can set the deployment_id later to support fallback_deployments + # but still need to access the other settings now so we can raise a proper exception if they're missing + self._chat = functools.partial( + lambda **kwargs: AzureChatOpenAI(**kwargs), + openai_api_key=get_settings().openai.key, + openai_api_base=get_settings().openai.api_base, + openai_api_version=get_settings().openai.api_version, + ) + else: + self._chat = ChatOpenAI(openai_api_key=get_settings().openai.key) except AttributeError as e: - raise ValueError("OpenAI key is required") from e + if getattr(e, "name"): + raise ValueError(f"OpenAI {e.name} is required") from e + else: + raise e @property def chat(self): - return self._chat + if self.azure: + # we must set the deployment_id only here (instead of the __init__ method) to support fallback_deployments + return self._chat(deployment_name=self.deployment_id) + else: + return self._chat @property def deployment_id(self): From ba3f22d81ecec073bd549c26b70dbe5d4f5accee Mon Sep 17 00:00:00 2001 From: zmeir Date: Thu, 4 Jan 2024 16:08:50 +0200 Subject: [PATCH 5/8] Move logging to a central location for all AI Handlers --- pr_agent/algo/ai_handlers/litellm_ai_handler.py | 5 ----- pr_agent/algo/pr_processing.py | 5 +++++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pr_agent/algo/ai_handlers/litellm_ai_handler.py b/pr_agent/algo/ai_handlers/litellm_ai_handler.py index 7061ca79..e3cbda39 100644 --- a/pr_agent/algo/ai_handlers/litellm_ai_handler.py +++ b/pr_agent/algo/ai_handlers/litellm_ai_handler.py @@ -101,11 +101,6 @@ class LiteLLMAIHandler(BaseAiHandler): """ try: deployment_id = self.deployment_id - if get_settings().config.verbosity_level >= 2: - get_logger().debug( - f"Generating completion with {model}" - f"{(' from deployment ' + deployment_id) if deployment_id else ''}" - ) if self.azure: model = 'azure/' + model messages = [{"role": "system", "content": system}, {"role": "user", "content": user}] diff --git a/pr_agent/algo/pr_processing.py b/pr_agent/algo/pr_processing.py index 4b380550..ecec3015 100644 --- a/pr_agent/algo/pr_processing.py +++ b/pr_agent/algo/pr_processing.py @@ -226,6 +226,11 @@ async def retry_with_fallback_models(f: Callable): # try each (model, deployment_id) pair until one is successful, otherwise raise exception for i, (model, deployment_id) in enumerate(zip(all_models, all_deployments)): try: + if get_settings().config.verbosity_level >= 2: + get_logger().debug( + f"Generating prediction with {model}" + f"{(' from deployment ' + deployment_id) if deployment_id else ''}" + ) get_settings().set("openai.deployment_id", deployment_id) return await f(model) except Exception as e: From c2b0891c0baa8c3dd3e57cad0644b58e750168e0 Mon Sep 17 00:00:00 2001 From: Zohar Meir <33152084+zmeir@users.noreply.github.com> Date: Thu, 4 Jan 2024 18:53:45 +0200 Subject: [PATCH 6/8] Simpler auto-extended toggle and keep the default as false --- docs/IMPROVE.md | 5 +---- pr_agent/settings/configuration.toml | 5 +---- pr_agent/tools/pr_code_suggestions.py | 11 +++-------- 3 files changed, 5 insertions(+), 16 deletions(-) diff --git a/docs/IMPROVE.md b/docs/IMPROVE.md index 19fa038e..3f561071 100644 --- a/docs/IMPROVE.md +++ b/docs/IMPROVE.md @@ -29,10 +29,7 @@ Under the section 'pr_code_suggestions', the [configuration file](./../pr_agent/ - `include_improved_code`: if set to true, the tool will include an improved code implementation in the suggestion. Default is true. #### params for '/improve --extended' mode -- `auto_extended_mode`: enable extended mode automatically for large PRs. Default is true. -- `auto_extended_mode_min_files`: minimum number of files in the PR to automatically enable extended mode. Default is 1. -- `auto_extended_mode_min_additions`: minimum number of line additions in the PR to automatically enable extended mode. Default is 500. -- `auto_extended_mode_min_deletions`: minimum number of line deletions in the PR to automatically enable extended mode. Default is 0. +- `auto_extended_mode`: enable extended mode automatically (no need for the `--extended` option). Default is false. - `num_code_suggestions_per_chunk`: number of code suggestions provided by the 'improve' tool, per chunk. Default is 8. - `rank_extended_suggestions`: if set to true, the tool will rank the suggestions, based on importance. Default is true. - `max_number_of_calls`: maximum number of chunks. Default is 5. diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index 3a868c7b..cbd2246c 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -71,10 +71,7 @@ include_improved_code = true extra_instructions = "" rank_suggestions = false # params for '/improve --extended' mode -auto_extended_mode=true -auto_extended_mode_min_files=1 -auto_extended_mode_min_additions=500 -auto_extended_mode_min_deletions=0 +auto_extended_mode=false num_code_suggestions_per_chunk=8 rank_extended_suggestions = true max_number_of_calls = 5 diff --git a/pr_agent/tools/pr_code_suggestions.py b/pr_agent/tools/pr_code_suggestions.py index 4490d02c..776b660f 100644 --- a/pr_agent/tools/pr_code_suggestions.py +++ b/pr_agent/tools/pr_code_suggestions.py @@ -207,17 +207,12 @@ class PRCodeSuggestions: return new_code_snippet def _get_is_extended(self, args: list[str]) -> bool: - """Check if extended mode should be enabled by the `--extended` flag or automatically according to the PR""" + """Check if extended mode should be enabled by the `--extended` flag or automatically according to the configuration""" if any(["extended" in arg for arg in args]): get_logger().info("Extended mode is enabled by the `--extended` flag") return True - if ( - get_settings().pr_code_suggestions.auto_extended_mode - and self.git_provider.pr.changed_files >= get_settings().pr_code_suggestions.auto_extended_mode_min_files - and self.git_provider.pr.additions >= get_settings().pr_code_suggestions.auto_extended_mode_min_additions - and self.git_provider.pr.deletions >= get_settings().pr_code_suggestions.auto_extended_mode_min_deletions - ): - get_logger().info("Extended mode is enabled automatically based on the PR size") + if get_settings().pr_code_suggestions.auto_extended_mode: + get_logger().info("Extended mode is enabled automatically based on the configuration toggle") return True return False From 9f88105f72c1320182eb87ba0dd66ea2af3b84ac Mon Sep 17 00:00:00 2001 From: Wes Vetter Date: Thu, 4 Jan 2024 10:32:53 -0800 Subject: [PATCH 7/8] =?UTF-8?q?=F0=9F=93=9A=20Minor=20update=20to=20REVIEW?= =?UTF-8?q?.md=20for=20inclusive=20language?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces "his [judgement]" with "their". --- docs/REVIEW.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/REVIEW.md b/docs/REVIEW.md index 9990819d..8aca684e 100644 --- a/docs/REVIEW.md +++ b/docs/REVIEW.md @@ -64,7 +64,7 @@ By invoking: ``` /reflect_and_review ``` -The tool will first ask the author questions about the PR, and will guide the review based on his answers. +The tool will first ask the author questions about the PR, and will guide the review based on their answers. @@ -75,7 +75,7 @@ The tool will first ask the author questions about the PR, and will guide the re - With current level of AI for code (GPT-4), mistakes can happen. Not all the suggestions will be perfect, and a user should not accept all of them automatically. -- Suggestions are not meant to be [simplistic](./../pr_agent/settings/pr_reviewer_prompts.toml#L29). Instead, they aim to give deep feedback and raise questions, ideas and thoughts to the user, who can then use his judgment, experience, and understanding of the code base. +- Suggestions are not meant to be [simplistic](./../pr_agent/settings/pr_reviewer_prompts.toml#L29). Instead, they aim to give deep feedback and raise questions, ideas and thoughts to the user, who can then use their judgment, experience, and understanding of the code base. - Recommended to use the 'extra_instructions' field to guide the model to suggestions that are more relevant to the specific needs of the project. From c69ede01385a002a7219bb318ea13758859efa02 Mon Sep 17 00:00:00 2001 From: Tal Date: Fri, 5 Jan 2024 11:10:55 +0200 Subject: [PATCH 8/8] Update REVIEW.md --- docs/REVIEW.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/REVIEW.md b/docs/REVIEW.md index 8aca684e..89c4fe9b 100644 --- a/docs/REVIEW.md +++ b/docs/REVIEW.md @@ -29,9 +29,10 @@ Under the section 'pr_reviewer', the [configuration file](./../pr_agent/settings - `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, 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 ...". -#### SOC2 compliance 💎 -- `require_soc2_review`: if set to true, the tool will add a section that checks if the PR description includes a link to a ticket in a project management system (e.g., Jira, Asana, Trello, etc.), as required by SOC2 compliance. Default is false. -- `soc2_ticket_prompt`: The prompt to be used for the SOC2 ticket review. Default is "Does the PR description include a link to ticket in a project management system (e.g., Jira, Asana, Trello, etc.) ?". Edit this field if your compliance requirements are different. +#### SOC2 ticket compliance 💎 +This sub-tool checks if the PR description properly contains a ticket to a project management system (e.g., Jira, Asana, Trello, etc.), as required by SOC2 compliance. If not, it will add a label to the PR: "Missing SOC2 ticket". +- `require_soc2_review`: If set to true, the SOC2 ticket checker sub-tool will be enabled. Default is false. +- `soc2_ticket_prompt`: The prompt for the SOC2 ticket review. Default is: `Does the PR description include a link to ticket in a project management system (e.g., Jira, Asana, Trello, etc.) ?`. Edit this field if your compliance requirements are different. #### review labels - `enable_review_labels_security`: if set to true, the tool will publish a 'possible security issue' label if it detects a security issue. Default is true. - `enable_review_labels_effort`: if set to true, the tool will publish a 'Review effort [1-5]: x' label. Default is false.