From 05ab5f699faf2c211602dcf925bd68bb778bbf16 Mon Sep 17 00:00:00 2001 From: kkan9ma Date: Fri, 16 May 2025 17:51:22 +0900 Subject: [PATCH 01/46] Improve token calculation logic based on model type - Rename calc_tokens to get_token_count_by_model_type for clearer intent - Separate model type detection logic to improve maintainability --- pr_agent/algo/token_handler.py | 44 +++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/pr_agent/algo/token_handler.py b/pr_agent/algo/token_handler.py index 60cf2c84..0c8851e8 100644 --- a/pr_agent/algo/token_handler.py +++ b/pr_agent/algo/token_handler.py @@ -107,25 +107,37 @@ class TokenHandler: get_logger().error( f"Error in Anthropic token counting: {e}") return MaxTokens - def estimate_token_count_for_non_anth_claude_models(self, model, default_encoder_estimate): + def is_openai_model(self, model_name): + from re import match + + return 'gpt' in model_name or match(r"^o[1-9](-mini|-preview)?$", model_name) + + def apply_estimation_factor(self, model_name, default_estimate): from math import ceil - import re - model_is_from_o_series = re.match(r"^o[1-9](-mini|-preview)?$", model) - if ('gpt' in get_settings().config.model.lower() or model_is_from_o_series) and get_settings(use_context=False).get('openai.key'): - return default_encoder_estimate - #else: Model is not an OpenAI one - therefore, cannot provide an accurate token count and instead, return a higher number as best effort. + factor = 1 + get_settings().get('config.model_token_count_estimate_factor', 0) + get_logger().warning(f"{model_name}'s token count cannot be accurately estimated. Using factor of {factor}") + + return ceil(factor * default_estimate) - elbow_factor = 1 + get_settings().get('config.model_token_count_estimate_factor', 0) - get_logger().warning(f"{model}'s expected token count cannot be accurately estimated. Using {elbow_factor} of encoder output as best effort estimate") - return ceil(elbow_factor * default_encoder_estimate) - - def count_tokens(self, patch: str, force_accurate=False) -> int: + def get_token_count_by_model_type(self, patch: str, default_estimate: int) -> int: + model_name = get_settings().config.model.lower() + + if 'claude' in model_name and get_settings(use_context=False).get('anthropic.key'): + return self.calc_claude_tokens(patch) + + if self.is_openai_model(model_name) and get_settings(use_context=False).get('openai.key'): + return default_estimate + + return self.apply_estimation_factor(model_name, default_estimate) + + def count_tokens(self, patch: str, force_accurate: bool = False) -> int: """ Counts the number of tokens in a given patch string. Args: - patch: The patch string. + - force_accurate: If True, uses a more precise calculation method. Returns: The number of tokens in the patch string. @@ -135,11 +147,5 @@ class TokenHandler: #If an estimate is enough (for example, in cases where the maximal allowed tokens is way below the known limits), return it. if not force_accurate: return encoder_estimate - - #else, force_accurate==True: User requested providing an accurate estimation: - model = get_settings().config.model.lower() - if 'claude' in model and get_settings(use_context=False).get('anthropic.key'): - return self.calc_claude_tokens(patch) # API call to Anthropic for accurate token counting for Claude models - - #else: Non Anthropic provided model: - return self.estimate_token_count_for_non_anth_claude_models(model, encoder_estimate) + else: + return self.get_token_count_by_model_type(patch, encoder_estimate=encoder_estimate) From 8aa89ff8e66f4a9f2de1bbc675228635ed07f6a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Walson=20Low=20=EF=BC=88=E5=88=98=E7=BB=B4=E6=98=87?= =?UTF-8?q?=EF=BC=89?= Date: Tue, 20 May 2025 10:27:13 +0800 Subject: [PATCH 02/46] docs: include [aws] in .secrets.template --- docs/docs/usage-guide/changing_a_model.md | 2 +- pr_agent/settings/.secrets_template.toml | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/docs/usage-guide/changing_a_model.md b/docs/docs/usage-guide/changing_a_model.md index bfeb7c3a..e7f9e16a 100644 --- a/docs/docs/usage-guide/changing_a_model.md +++ b/docs/docs/usage-guide/changing_a_model.md @@ -226,7 +226,7 @@ To use Amazon Bedrock and its foundational models, add the below configuration: model="bedrock/anthropic.claude-3-5-sonnet-20240620-v1:0" fallback_models=["bedrock/anthropic.claude-3-5-sonnet-20240620-v1:0"] -[aws] +[aws] # in .secrets.toml AWS_ACCESS_KEY_ID="..." AWS_SECRET_ACCESS_KEY="..." AWS_REGION_NAME="..." diff --git a/pr_agent/settings/.secrets_template.toml b/pr_agent/settings/.secrets_template.toml index 6572677d..17c5e8ee 100644 --- a/pr_agent/settings/.secrets_template.toml +++ b/pr_agent/settings/.secrets_template.toml @@ -111,4 +111,9 @@ api_base = "" # Your Azure OpenAI service base URL (e.g., https://openai.xyz.co [openrouter] key = "" -api_base = "" \ No newline at end of file +api_base = "" + +[aws] +AWS_ACCESS_KEY_ID = "" +AWS_SECRET_ACCESS_KEY = "" +AWS_REGION_NAME = "" \ No newline at end of file From 81fa22e4df6c8fd4ce3da0466bd181019bb6d108 Mon Sep 17 00:00:00 2001 From: kkan9ma Date: Tue, 20 May 2025 13:47:15 +0900 Subject: [PATCH 03/46] Add model name validation --- pr_agent/algo/token_handler.py | 44 ++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/pr_agent/algo/token_handler.py b/pr_agent/algo/token_handler.py index 0c8851e8..2781fc5c 100644 --- a/pr_agent/algo/token_handler.py +++ b/pr_agent/algo/token_handler.py @@ -1,4 +1,6 @@ from threading import Lock +from math import ceil +import re from jinja2 import Environment, StrictUndefined from tiktoken import encoding_for_model, get_encoding @@ -7,6 +9,16 @@ from pr_agent.config_loader import get_settings from pr_agent.log import get_logger +class ModelTypeValidator: + @staticmethod + def is_openai_model(model_name: str) -> bool: + return 'gpt' in model_name or re.match(r"^o[1-9](-mini|-preview)?$", model_name) + + @staticmethod + def is_claude_model(model_name: str) -> bool: + return 'claude' in model_name + + class TokenEncoder: _encoder_instance = None _model = None @@ -51,6 +63,9 @@ class TokenHandler: - user: The user string. """ self.encoder = TokenEncoder.get_token_encoder() + self.settings = get_settings() + self.model_validator = ModelTypeValidator() + if pr is not None: self.prompt_tokens = self._get_system_user_tokens(pr, self.encoder, vars, system, user) @@ -79,19 +94,20 @@ class TokenHandler: get_logger().error(f"Error in _get_system_user_tokens: {e}") return 0 - def calc_claude_tokens(self, patch): + def calc_claude_tokens(self, patch: str) -> int: try: import anthropic from pr_agent.algo import MAX_TOKENS - client = anthropic.Anthropic(api_key=get_settings(use_context=False).get('anthropic.key')) - MaxTokens = MAX_TOKENS[get_settings().config.model] + + client = anthropic.Anthropic(api_key=self.settings.get('anthropic.key')) + max_tokens = MAX_TOKENS[self.settings.config.model] # Check if the content size is too large (9MB limit) if len(patch.encode('utf-8')) > 9_000_000: get_logger().warning( "Content too large for Anthropic token counting API, falling back to local tokenizer" ) - return MaxTokens + return max_tokens response = client.messages.count_tokens( model="claude-3-7-sonnet-20250219", @@ -104,29 +120,21 @@ class TokenHandler: return response.input_tokens except Exception as e: - get_logger().error( f"Error in Anthropic token counting: {e}") - return MaxTokens + get_logger().error(f"Error in Anthropic token counting: {e}") + return max_tokens - def is_openai_model(self, model_name): - from re import match - - return 'gpt' in model_name or match(r"^o[1-9](-mini|-preview)?$", model_name) - - def apply_estimation_factor(self, model_name, default_estimate): - from math import ceil - - factor = 1 + get_settings().get('config.model_token_count_estimate_factor', 0) + def apply_estimation_factor(self, model_name: str, default_estimate: int) -> int: + factor = 1 + self.settings.get('config.model_token_count_estimate_factor', 0) get_logger().warning(f"{model_name}'s token count cannot be accurately estimated. Using factor of {factor}") - return ceil(factor * default_estimate) def get_token_count_by_model_type(self, patch: str, default_estimate: int) -> int: model_name = get_settings().config.model.lower() - if 'claude' in model_name and get_settings(use_context=False).get('anthropic.key'): + if self.model_validator.is_claude_model(model_name) and get_settings(use_context=False).get('anthropic.key'): return self.calc_claude_tokens(patch) - if self.is_openai_model(model_name) and get_settings(use_context=False).get('openai.key'): + if self.model_validator.is_openai_model(model_name) and get_settings(use_context=False).get('openai.key'): return default_estimate return self.apply_estimation_factor(model_name, default_estimate) From e72bb28c4e9cad740f9240b25a5adb7339943044 Mon Sep 17 00:00:00 2001 From: kkan9ma Date: Tue, 20 May 2025 13:50:30 +0900 Subject: [PATCH 04/46] Replace get_settings() with self.settings --- pr_agent/algo/token_handler.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pr_agent/algo/token_handler.py b/pr_agent/algo/token_handler.py index 2781fc5c..99dbc635 100644 --- a/pr_agent/algo/token_handler.py +++ b/pr_agent/algo/token_handler.py @@ -129,12 +129,12 @@ class TokenHandler: return ceil(factor * default_estimate) def get_token_count_by_model_type(self, patch: str, default_estimate: int) -> int: - model_name = get_settings().config.model.lower() + model_name = self.settings.config.model.lower() - if self.model_validator.is_claude_model(model_name) and get_settings(use_context=False).get('anthropic.key'): + if self.model_validator.is_claude_model(model_name) and self.settings.get('anthropic.key'): return self.calc_claude_tokens(patch) - if self.model_validator.is_openai_model(model_name) and get_settings(use_context=False).get('openai.key'): + if self.model_validator.is_openai_model(model_name) and self.settings.get('openai.key'): return default_estimate return self.apply_estimation_factor(model_name, default_estimate) From f198e6fa097a9ad27a08da7b8f9e14dbe7be5a9b Mon Sep 17 00:00:00 2001 From: kkan9ma Date: Tue, 20 May 2025 14:12:24 +0900 Subject: [PATCH 05/46] Add constants and improve token calculation logic --- pr_agent/algo/token_handler.py | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/pr_agent/algo/token_handler.py b/pr_agent/algo/token_handler.py index 99dbc635..8bd3f115 100644 --- a/pr_agent/algo/token_handler.py +++ b/pr_agent/algo/token_handler.py @@ -52,6 +52,10 @@ class TokenHandler: method. """ + # Constants + CLAUDE_MODEL = "claude-3-7-sonnet-20250219" + CLAUDE_MAX_CONTENT_SIZE = 9_000_000 # Maximum allowed content size (9MB) for Claude API + def __init__(self, pr=None, vars: dict = {}, system="", user=""): """ Initializes the TokenHandler object. @@ -102,15 +106,14 @@ class TokenHandler: client = anthropic.Anthropic(api_key=self.settings.get('anthropic.key')) max_tokens = MAX_TOKENS[self.settings.config.model] - # Check if the content size is too large (9MB limit) - if len(patch.encode('utf-8')) > 9_000_000: + if len(patch.encode('utf-8')) > self.CLAUDE_MAX_CONTENT_SIZE: get_logger().warning( "Content too large for Anthropic token counting API, falling back to local tokenizer" ) return max_tokens response = client.messages.count_tokens( - model="claude-3-7-sonnet-20250219", + model=self.CLAUDE_MODEL, system="system", messages=[{ "role": "user", @@ -126,9 +129,20 @@ class TokenHandler: def apply_estimation_factor(self, model_name: str, default_estimate: int) -> int: factor = 1 + self.settings.get('config.model_token_count_estimate_factor', 0) get_logger().warning(f"{model_name}'s token count cannot be accurately estimated. Using factor of {factor}") + return ceil(factor * default_estimate) def get_token_count_by_model_type(self, patch: str, default_estimate: int) -> int: + """ + Get token count based on model type. + + Args: + patch: The text to count tokens for. + default_estimate: The default token count estimate. + + Returns: + int: The calculated token count. + """ model_name = self.settings.config.model.lower() if self.model_validator.is_claude_model(model_name) and self.settings.get('anthropic.key'): @@ -152,8 +166,8 @@ class TokenHandler: """ encoder_estimate = len(self.encoder.encode(patch, disallowed_special=())) - #If an estimate is enough (for example, in cases where the maximal allowed tokens is way below the known limits), return it. - if not force_accurate: - return encoder_estimate - else: + if force_accurate: return self.get_token_count_by_model_type(patch, encoder_estimate=encoder_estimate) + + # If an estimate is enough (for example, in cases where the maximal allowed tokens is way below the known limits), return it. + return encoder_estimate From 97f2b6f7360117355b46da6436e7f88ecfbd16c8 Mon Sep 17 00:00:00 2001 From: kkan9ma Date: Tue, 20 May 2025 15:29:27 +0900 Subject: [PATCH 06/46] Fix TypeError --- pr_agent/algo/token_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pr_agent/algo/token_handler.py b/pr_agent/algo/token_handler.py index 8bd3f115..0a1aeb89 100644 --- a/pr_agent/algo/token_handler.py +++ b/pr_agent/algo/token_handler.py @@ -167,7 +167,7 @@ class TokenHandler: encoder_estimate = len(self.encoder.encode(patch, disallowed_special=())) if force_accurate: - return self.get_token_count_by_model_type(patch, encoder_estimate=encoder_estimate) + return self.get_token_count_by_model_type(patch, encoder_estimate) # If an estimate is enough (for example, in cases where the maximal allowed tokens is way below the known limits), return it. return encoder_estimate From 648829b770773c4db8b7dd671acb2b6c57310d6e Mon Sep 17 00:00:00 2001 From: kkan9ma Date: Wed, 21 May 2025 17:51:03 +0900 Subject: [PATCH 07/46] Rename method --- pr_agent/algo/token_handler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pr_agent/algo/token_handler.py b/pr_agent/algo/token_handler.py index 0a1aeb89..b8eaac89 100644 --- a/pr_agent/algo/token_handler.py +++ b/pr_agent/algo/token_handler.py @@ -15,7 +15,7 @@ class ModelTypeValidator: return 'gpt' in model_name or re.match(r"^o[1-9](-mini|-preview)?$", model_name) @staticmethod - def is_claude_model(model_name: str) -> bool: + def is_anthropic_model(model_name: str) -> bool: return 'claude' in model_name @@ -145,7 +145,7 @@ class TokenHandler: """ model_name = self.settings.config.model.lower() - if self.model_validator.is_claude_model(model_name) and self.settings.get('anthropic.key'): + if self.model_validator.is_anthropic_model(model_name) and self.settings.get('anthropic.key'): return self.calc_claude_tokens(patch) if self.model_validator.is_openai_model(model_name) and self.settings.get('openai.key'): From c3ea048b718510f04637027f0f3ef6a504209a94 Mon Sep 17 00:00:00 2001 From: kkan9ma Date: Wed, 21 May 2025 17:52:51 +0900 Subject: [PATCH 08/46] Restore original return logic for force_accurate condition --- pr_agent/algo/token_handler.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pr_agent/algo/token_handler.py b/pr_agent/algo/token_handler.py index b8eaac89..5a29da36 100644 --- a/pr_agent/algo/token_handler.py +++ b/pr_agent/algo/token_handler.py @@ -166,8 +166,8 @@ class TokenHandler: """ encoder_estimate = len(self.encoder.encode(patch, disallowed_special=())) - if force_accurate: - return self.get_token_count_by_model_type(patch, encoder_estimate) - # If an estimate is enough (for example, in cases where the maximal allowed tokens is way below the known limits), return it. - return encoder_estimate + if not force_accurate: + return encoder_estimate + + return self.get_token_count_by_model_type(patch, encoder_estimate) From df0355d827b0f83f50031bb4133e8acc4929e1d8 Mon Sep 17 00:00:00 2001 From: kkan9ma Date: Wed, 21 May 2025 18:07:47 +0900 Subject: [PATCH 09/46] Remove member variable for restroring get_settings() --- pr_agent/algo/token_handler.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/pr_agent/algo/token_handler.py b/pr_agent/algo/token_handler.py index 5a29da36..da239cfb 100644 --- a/pr_agent/algo/token_handler.py +++ b/pr_agent/algo/token_handler.py @@ -67,7 +67,6 @@ class TokenHandler: - user: The user string. """ self.encoder = TokenEncoder.get_token_encoder() - self.settings = get_settings() self.model_validator = ModelTypeValidator() if pr is not None: @@ -103,8 +102,8 @@ class TokenHandler: import anthropic from pr_agent.algo import MAX_TOKENS - client = anthropic.Anthropic(api_key=self.settings.get('anthropic.key')) - max_tokens = MAX_TOKENS[self.settings.config.model] + client = anthropic.Anthropic(api_key=get_settings().get('anthropic.key')) + max_tokens = MAX_TOKENS[get_settings().config.model] if len(patch.encode('utf-8')) > self.CLAUDE_MAX_CONTENT_SIZE: get_logger().warning( @@ -127,7 +126,7 @@ class TokenHandler: return max_tokens def apply_estimation_factor(self, model_name: str, default_estimate: int) -> int: - factor = 1 + self.settings.get('config.model_token_count_estimate_factor', 0) + factor = 1 + get_settings().get('config.model_token_count_estimate_factor', 0) get_logger().warning(f"{model_name}'s token count cannot be accurately estimated. Using factor of {factor}") return ceil(factor * default_estimate) @@ -143,12 +142,12 @@ class TokenHandler: Returns: int: The calculated token count. """ - model_name = self.settings.config.model.lower() + model_name = get_settings().config.model.lower() - if self.model_validator.is_anthropic_model(model_name) and self.settings.get('anthropic.key'): return self.calc_claude_tokens(patch) + if self.model_validator.is_anthropic_model(model_name) and get_settings(use_context=False).get('anthropic.key'): - if self.model_validator.is_openai_model(model_name) and self.settings.get('openai.key'): + if self.model_validator.is_openai_model(model_name) and get_settings(use_context=False).get('openai.key'): return default_estimate return self.apply_estimation_factor(model_name, default_estimate) From ead7491ca91e24ab185b33c2867dd3f4a8baad0a Mon Sep 17 00:00:00 2001 From: kkan9ma Date: Wed, 21 May 2025 18:08:48 +0900 Subject: [PATCH 10/46] Apply convention for marking private --- pr_agent/algo/token_handler.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pr_agent/algo/token_handler.py b/pr_agent/algo/token_handler.py index da239cfb..e45d611f 100644 --- a/pr_agent/algo/token_handler.py +++ b/pr_agent/algo/token_handler.py @@ -97,7 +97,7 @@ class TokenHandler: get_logger().error(f"Error in _get_system_user_tokens: {e}") return 0 - def calc_claude_tokens(self, patch: str) -> int: + def _calc_claude_tokens(self, patch: str) -> int: try: import anthropic from pr_agent.algo import MAX_TOKENS @@ -125,13 +125,13 @@ class TokenHandler: get_logger().error(f"Error in Anthropic token counting: {e}") return max_tokens - def apply_estimation_factor(self, model_name: str, default_estimate: int) -> int: + def _apply_estimation_factor(self, model_name: str, default_estimate: int) -> int: factor = 1 + get_settings().get('config.model_token_count_estimate_factor', 0) get_logger().warning(f"{model_name}'s token count cannot be accurately estimated. Using factor of {factor}") return ceil(factor * default_estimate) - def get_token_count_by_model_type(self, patch: str, default_estimate: int) -> int: + def _get_token_count_by_model_type(self, patch: str, default_estimate: int) -> int: """ Get token count based on model type. @@ -144,13 +144,13 @@ class TokenHandler: """ model_name = get_settings().config.model.lower() - return self.calc_claude_tokens(patch) if self.model_validator.is_anthropic_model(model_name) and get_settings(use_context=False).get('anthropic.key'): + return self._calc_claude_tokens(patch) if self.model_validator.is_openai_model(model_name) and get_settings(use_context=False).get('openai.key'): return default_estimate - return self.apply_estimation_factor(model_name, default_estimate) + return self._apply_estimation_factor(model_name, default_estimate) def count_tokens(self, patch: str, force_accurate: bool = False) -> int: """ @@ -169,4 +169,4 @@ class TokenHandler: if not force_accurate: return encoder_estimate - return self.get_token_count_by_model_type(patch, encoder_estimate) + return self._get_token_count_by_model_type(patch, encoder_estimate) From cc686ef26d961bd515b731c447f98cd23a240633 Mon Sep 17 00:00:00 2001 From: kkan9ma Date: Thu, 22 May 2025 13:12:04 +0900 Subject: [PATCH 11/46] Reorder model check: OpenAI before Anthropic OpenAI is the default in most cases, so checking it first skips unnecessary Anthropic logic. --- pr_agent/algo/token_handler.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pr_agent/algo/token_handler.py b/pr_agent/algo/token_handler.py index e45d611f..67a6eed9 100644 --- a/pr_agent/algo/token_handler.py +++ b/pr_agent/algo/token_handler.py @@ -144,11 +144,11 @@ class TokenHandler: """ model_name = get_settings().config.model.lower() - if self.model_validator.is_anthropic_model(model_name) and get_settings(use_context=False).get('anthropic.key'): - return self._calc_claude_tokens(patch) - if self.model_validator.is_openai_model(model_name) and get_settings(use_context=False).get('openai.key'): return default_estimate + + if self.model_validator.is_anthropic_model(model_name) and get_settings(use_context=False).get('anthropic.key'): + return self._calc_claude_tokens(patch) return self._apply_estimation_factor(model_name, default_estimate) From facfb5f46b1be200b7dd5cfe87d3c745fa0cd85e Mon Sep 17 00:00:00 2001 From: kkan9ma Date: Thu, 22 May 2025 13:32:20 +0900 Subject: [PATCH 12/46] Add missing code: use_context=False --- pr_agent/algo/token_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pr_agent/algo/token_handler.py b/pr_agent/algo/token_handler.py index 67a6eed9..998391b1 100644 --- a/pr_agent/algo/token_handler.py +++ b/pr_agent/algo/token_handler.py @@ -102,7 +102,7 @@ class TokenHandler: import anthropic from pr_agent.algo import MAX_TOKENS - client = anthropic.Anthropic(api_key=get_settings().get('anthropic.key')) + client = anthropic.Anthropic(api_key=get_settings(use_context=False).get('anthropic.key')) max_tokens = MAX_TOKENS[get_settings().config.model] if len(patch.encode('utf-8')) > self.CLAUDE_MAX_CONTENT_SIZE: From 466ec4ce90ab2dfbc584a3461c45340a82ddd54e Mon Sep 17 00:00:00 2001 From: Kangmoon Seo <100016044+KangmoonSeo@users.noreply.github.com> Date: Thu, 22 May 2025 15:04:16 +0900 Subject: [PATCH 13/46] fix: exclude RateLimitError from retry logic --- .../algo/ai_handlers/langchain_ai_handler.py | 22 +++++++++++------ .../algo/ai_handlers/litellm_ai_handler.py | 12 +++++----- .../algo/ai_handlers/openai_ai_handler.py | 24 ++++++++++--------- 3 files changed, 34 insertions(+), 24 deletions(-) diff --git a/pr_agent/algo/ai_handlers/langchain_ai_handler.py b/pr_agent/algo/ai_handlers/langchain_ai_handler.py index d4ea0aa5..4d708fcb 100644 --- a/pr_agent/algo/ai_handlers/langchain_ai_handler.py +++ b/pr_agent/algo/ai_handlers/langchain_ai_handler.py @@ -6,8 +6,8 @@ except: # we don't enforce langchain as a dependency, so if it's not installed, import functools -from openai import APIError, RateLimitError, Timeout -from retry import retry +import openai +from tenacity import retry, retry_if_exception_type, retry_if_not_exception_type, stop_after_attempt from pr_agent.algo.ai_handlers.base_ai_handler import BaseAiHandler from pr_agent.config_loader import get_settings @@ -36,8 +36,10 @@ class LangChainOpenAIHandler(BaseAiHandler): """ return get_settings().get("OPENAI.DEPLOYMENT_ID", None) - @retry(exceptions=(APIError, Timeout, AttributeError, RateLimitError), - tries=OPENAI_RETRIES, delay=2, backoff=2, jitter=(1, 3)) + @retry( + retry=retry_if_exception_type(openai.APIError) & retry_if_not_exception_type(openai.RateLimitError), + stop=stop_after_attempt(OPENAI_RETRIES), + ) async def chat_completion(self, model: str, system: str, user: str, temperature: float = 0.2): try: messages = [SystemMessage(content=system), HumanMessage(content=user)] @@ -47,9 +49,15 @@ class LangChainOpenAIHandler(BaseAiHandler): finish_reason = "completed" return resp.content, finish_reason - except (Exception) as e: - get_logger().error("Unknown error during OpenAI inference: ", e) - raise e + except openai.RateLimitError as e: + get_logger().error(f"Rate limit error during LLM inference: {e}") + raise + except openai.APIError as e: + get_logger().warning(f"Error during LLM inference: {e}") + raise + except Exception as e: + get_logger().warning(f"Unknown error during LLM inference: {e}") + raise openai.APIError from e def _create_chat(self, deployment_id=None): try: diff --git a/pr_agent/algo/ai_handlers/litellm_ai_handler.py b/pr_agent/algo/ai_handlers/litellm_ai_handler.py index 8d727b8b..f20b03f8 100644 --- a/pr_agent/algo/ai_handlers/litellm_ai_handler.py +++ b/pr_agent/algo/ai_handlers/litellm_ai_handler.py @@ -3,7 +3,7 @@ import litellm import openai import requests from litellm import acompletion -from tenacity import retry, retry_if_exception_type, stop_after_attempt +from tenacity import retry, retry_if_exception_type, retry_if_not_exception_type, stop_after_attempt from pr_agent.algo import CLAUDE_EXTENDED_THINKING_MODELS, NO_SUPPORT_TEMPERATURE_MODELS, SUPPORT_REASONING_EFFORT_MODELS, USER_MESSAGE_ONLY_MODELS from pr_agent.algo.ai_handlers.base_ai_handler import BaseAiHandler @@ -274,8 +274,8 @@ class LiteLLMAIHandler(BaseAiHandler): return get_settings().get("OPENAI.DEPLOYMENT_ID", None) @retry( - retry=retry_if_exception_type((openai.APIError, openai.APIConnectionError, openai.APITimeoutError)), # No retry on RateLimitError - stop=stop_after_attempt(OPENAI_RETRIES) + retry=retry_if_exception_type(openai.APIError) & retry_if_not_exception_type(openai.RateLimitError), + stop=stop_after_attempt(OPENAI_RETRIES), ) async def chat_completion(self, model: str, system: str, user: str, temperature: float = 0.2, img_path: str = None): try: @@ -371,13 +371,13 @@ class LiteLLMAIHandler(BaseAiHandler): get_logger().info(f"\nUser prompt:\n{user}") response = await acompletion(**kwargs) - except (openai.RateLimitError) as e: + except openai.RateLimitError as e: get_logger().error(f"Rate limit error during LLM inference: {e}") raise - except (openai.APIError, openai.APITimeoutError) as e: + except openai.APIError as e: get_logger().warning(f"Error during LLM inference: {e}") raise - except (Exception) as e: + except Exception as e: get_logger().warning(f"Unknown error during LLM inference: {e}") raise openai.APIError from e if response is None or len(response["choices"]) == 0: diff --git a/pr_agent/algo/ai_handlers/openai_ai_handler.py b/pr_agent/algo/ai_handlers/openai_ai_handler.py index f74444a1..253282b0 100644 --- a/pr_agent/algo/ai_handlers/openai_ai_handler.py +++ b/pr_agent/algo/ai_handlers/openai_ai_handler.py @@ -1,8 +1,8 @@ from os import environ from pr_agent.algo.ai_handlers.base_ai_handler import BaseAiHandler import openai -from openai import APIError, AsyncOpenAI, RateLimitError, Timeout -from retry import retry +from openai import AsyncOpenAI +from tenacity import retry, retry_if_exception_type, retry_if_not_exception_type, stop_after_attempt from pr_agent.algo.ai_handlers.base_ai_handler import BaseAiHandler from pr_agent.config_loader import get_settings @@ -38,8 +38,10 @@ class OpenAIHandler(BaseAiHandler): """ return get_settings().get("OPENAI.DEPLOYMENT_ID", None) - @retry(exceptions=(APIError, Timeout, AttributeError, RateLimitError), - tries=OPENAI_RETRIES, delay=2, backoff=2, jitter=(1, 3)) + @retry( + retry=retry_if_exception_type(openai.APIError) & retry_if_not_exception_type(openai.RateLimitError), + stop=stop_after_attempt(OPENAI_RETRIES), + ) async def chat_completion(self, model: str, system: str, user: str, temperature: float = 0.2): try: get_logger().info("System: ", system) @@ -57,12 +59,12 @@ class OpenAIHandler(BaseAiHandler): get_logger().info("AI response", response=resp, messages=messages, finish_reason=finish_reason, model=model, usage=usage) return resp, finish_reason - except (APIError, Timeout) as e: - get_logger().error("Error during OpenAI inference: ", e) + except openai.RateLimitError as e: + get_logger().error(f"Rate limit error during LLM inference: {e}") raise - except (RateLimitError) as e: - get_logger().error("Rate limit error during OpenAI inference: ", e) - raise - except (Exception) as e: - get_logger().error("Unknown error during OpenAI inference: ", e) + except openai.APIError as e: + get_logger().warning(f"Error during LLM inference: {e}") raise + except Exception as e: + get_logger().warning(f"Unknown error during LLM inference: {e}") + raise openai.APIError from e From e79c34e0392f848a4a581d8bf0cecfdd3875026c Mon Sep 17 00:00:00 2001 From: jwsong98 Date: Thu, 22 May 2025 02:24:50 +0900 Subject: [PATCH 14/46] test: add test case for YAML wrapped with braces --- tests/unittest/test_try_fix_yaml.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/unittest/test_try_fix_yaml.py b/tests/unittest/test_try_fix_yaml.py index 826d7312..4233f463 100644 --- a/tests/unittest/test_try_fix_yaml.py +++ b/tests/unittest/test_try_fix_yaml.py @@ -83,3 +83,22 @@ We can further improve the code by using the `const` keyword instead of `var` in ''' expected_output = {'code_suggestions': [{'relevant_file': 'src/index.ts\n', 'label': 'best practice\n'}, {'relevant_file': 'src/index2.ts\n', 'label': 'enhancment'}]} assert try_fix_yaml(review_text, first_key='code_suggestions', last_key='label') == expected_output + + + def test_with_brackets_yaml_content(self): + review_text = '''\ +{ +code_suggestions: +- relevant_file: | + src/index.ts + label: | + best practice + +- relevant_file: | + src/index2.ts + label: | + enhancement +} +''' + expected_output = {'code_suggestions': [{'relevant_file': 'src/index.ts\n', 'label': 'best practice\n'}, {'relevant_file': 'src/index2.ts\n', 'label': 'enhancement'}]} + assert try_fix_yaml(review_text, first_key='code_suggestions', last_key='label') == expected_output \ No newline at end of file From 069f36fc1f814f3f88d9c45a9c8f47f892b54d9c Mon Sep 17 00:00:00 2001 From: jwsong98 Date: Thu, 22 May 2025 02:26:08 +0900 Subject: [PATCH 15/46] test: add test case for YAML with tab indentation in block scalar --- tests/unittest/test_try_fix_yaml.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/unittest/test_try_fix_yaml.py b/tests/unittest/test_try_fix_yaml.py index 4233f463..94df8d14 100644 --- a/tests/unittest/test_try_fix_yaml.py +++ b/tests/unittest/test_try_fix_yaml.py @@ -101,4 +101,20 @@ code_suggestions: } ''' expected_output = {'code_suggestions': [{'relevant_file': 'src/index.ts\n', 'label': 'best practice\n'}, {'relevant_file': 'src/index2.ts\n', 'label': 'enhancement'}]} - assert try_fix_yaml(review_text, first_key='code_suggestions', last_key='label') == expected_output \ No newline at end of file + assert try_fix_yaml(review_text, first_key='code_suggestions', last_key='label') == expected_output + + def test_tab_indent_yaml(self): + review_text = '''\ +code_suggestions: +- relevant_file: | + src/index.ts + label: | +\tbest practice + +- relevant_file: | + src/index2.ts + label: | + enhancement +''' + expected_output = {'code_suggestions': [{'relevant_file': 'src/index.ts\n', 'label': 'best practice\n'}, {'relevant_file': 'src/index2.ts\n', 'label': 'enhancement\n'}]} + assert try_fix_yaml(review_text, first_key='code_suggestions', last_key='label') == expected_output From 20e69c35306f637ad2c96de66e740cf6cab2a11e Mon Sep 17 00:00:00 2001 From: jwsong98 Date: Thu, 22 May 2025 02:27:41 +0900 Subject: [PATCH 16/46] test: add test case for YAML block scalar with leading plus signs in code --- tests/unittest/test_try_fix_yaml.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/unittest/test_try_fix_yaml.py b/tests/unittest/test_try_fix_yaml.py index 94df8d14..2d64b755 100644 --- a/tests/unittest/test_try_fix_yaml.py +++ b/tests/unittest/test_try_fix_yaml.py @@ -118,3 +118,24 @@ code_suggestions: ''' expected_output = {'code_suggestions': [{'relevant_file': 'src/index.ts\n', 'label': 'best practice\n'}, {'relevant_file': 'src/index2.ts\n', 'label': 'enhancement\n'}]} assert try_fix_yaml(review_text, first_key='code_suggestions', last_key='label') == expected_output + + + def test_leading_plus_mark_code(self): + review_text = '''\ +code_suggestions: +- relevant_file: | + src/index.ts + label: | + best practice + existing_code: | ++ var router = createBrowserRouter([ + improved_code: | ++ const router = createBrowserRouter([ +''' + expected_output = {'code_suggestions': [{ + 'relevant_file': 'src/index.ts\n', + 'label': 'best practice\n', + 'existing_code': 'var router = createBrowserRouter([\n', + 'improved_code': 'const router = createBrowserRouter([\n' + }]} + assert try_fix_yaml(review_text, first_key='code_suggestions', last_key='improved_code') == expected_output From f10c38940686f78611656ac6e1bd03e76291837f Mon Sep 17 00:00:00 2001 From: jwsong98 Date: Thu, 22 May 2025 02:29:05 +0900 Subject: [PATCH 17/46] test: add test cases for YAML block scalar with inconsistent and insufficient indentation --- tests/unittest/test_try_fix_yaml.py | 79 +++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/tests/unittest/test_try_fix_yaml.py b/tests/unittest/test_try_fix_yaml.py index 2d64b755..09bdbff8 100644 --- a/tests/unittest/test_try_fix_yaml.py +++ b/tests/unittest/test_try_fix_yaml.py @@ -139,3 +139,82 @@ code_suggestions: 'improved_code': 'const router = createBrowserRouter([\n' }]} assert try_fix_yaml(review_text, first_key='code_suggestions', last_key='improved_code') == expected_output + + + def test_inconsistent_indentation_in_block_scalar_yaml(self): + """ + This test case represents a situation where the AI outputs the opening '{' with 5 spaces + (resulting in an inferred indent level of 5), while the closing '}' is output with only 4 spaces. + This inconsistency makes it impossible for the YAML parser to automatically determine the correct + indent level, causing a parsing failure. + + The root cause may be the LLM miscounting spaces or misunderstanding the active block scalar context + while generating YAML output. + """ + + review_text = '''\ +code_suggestions: +- relevant_file: | + tsconfig.json + existing_code: | + { + "key1": "value1", + "key2": { + "subkey": "value" + } + } +''' + expected_json = '''\ + { + "key1": "value1", + "key2": { + "subkey": "value" + } +} +''' + expected_output = { + 'code_suggestions': [{ + 'relevant_file': 'tsconfig.json\n', + 'existing_code': expected_json + }] + } + assert try_fix_yaml(review_text, first_key='code_suggestions', last_key='existing_code') == expected_output + + + def test_inconsistent_and_insufficient_indentation_in_block_scalar_yaml(self): + """ + This test case reproduces a YAML parsing failure where the block scalar content + generated by the AI includes inconsistent and insufficient indentation levels. + + The root cause may be the LLM miscounting spaces or misunderstanding the active block scalar context + while generating YAML output. + """ + + review_text = '''\ +code_suggestions: +- relevant_file: | + tsconfig.json + existing_code: | + { + "key1": "value1", + "key2": { + "subkey": "value" + } + } +''' + expected_json = '''\ +{ + "key1": "value1", + "key2": { + "subkey": "value" + } +} +''' + expected_output = { + 'code_suggestions': [{ + 'relevant_file': 'tsconfig.json\n', + 'existing_code': expected_json + }] + } + assert try_fix_yaml(review_text, first_key='code_suggestions', last_key='existing_code') == expected_output + From 684a438167fd92402bb7968a6ddd2d53256b636e Mon Sep 17 00:00:00 2001 From: jwsong98 Date: Thu, 22 May 2025 16:18:45 +0900 Subject: [PATCH 18/46] test: add test case for fixing incorrect indentation in YAML code block scalar --- tests/unittest/test_try_fix_yaml.py | 33 +++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/unittest/test_try_fix_yaml.py b/tests/unittest/test_try_fix_yaml.py index 09bdbff8..b23e9dd3 100644 --- a/tests/unittest/test_try_fix_yaml.py +++ b/tests/unittest/test_try_fix_yaml.py @@ -218,3 +218,36 @@ code_suggestions: } assert try_fix_yaml(review_text, first_key='code_suggestions', last_key='existing_code') == expected_output + + def test_wrong_indentation_code_block_scalar(self): + review_text = '''\ +code_suggestions: +- relevant_file: | + a.c + existing_code: | + int sum(int a, int b) { + return a + b; + } + + int sub(int a, int b) { + return a - b; + } +''' + expected_code_block = '''\ +int sum(int a, int b) { + return a + b; +} + +int sub(int a, int b) { + return a - b; +} +''' + expected_output = { + "code_suggestions": [ + { + "relevant_file": "a.c\n", + "existing_code": expected_code_block + } + ] + } + assert try_fix_yaml(review_text, first_key='code_suggestions', last_key='existing_code') == expected_output From f6a9d3c2ccea6cd7eee881a6be42136c64fa57f5 Mon Sep 17 00:00:00 2001 From: jwsong98 Date: Thu, 22 May 2025 16:55:52 +0900 Subject: [PATCH 19/46] fix:typo in test_with_initial_yaml, test_no_initial_yaml --- tests/unittest/test_try_fix_yaml.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unittest/test_try_fix_yaml.py b/tests/unittest/test_try_fix_yaml.py index b23e9dd3..98773c81 100644 --- a/tests/unittest/test_try_fix_yaml.py +++ b/tests/unittest/test_try_fix_yaml.py @@ -53,12 +53,12 @@ code_suggestions: - relevant_file: | src/index2.ts label: | - enhancment + enhancement ``` We can further improve the code by using the `const` keyword instead of `var` in the `src/index.ts` file. ''' - expected_output = {'code_suggestions': [{'relevant_file': 'src/index.ts\n', 'label': 'best practice\n'}, {'relevant_file': 'src/index2.ts\n', 'label': 'enhancment'}]} + expected_output = {'code_suggestions': [{'relevant_file': 'src/index.ts\n', 'label': 'best practice\n'}, {'relevant_file': 'src/index2.ts\n', 'label': 'enhancement'}]} assert try_fix_yaml(review_text, first_key='code_suggestions', last_key='label') == expected_output @@ -76,12 +76,12 @@ code_suggestions: - relevant_file: | src/index2.ts label: | - enhancment + enhancement ``` We can further improve the code by using the `const` keyword instead of `var` in the `src/index.ts` file. ''' - expected_output = {'code_suggestions': [{'relevant_file': 'src/index.ts\n', 'label': 'best practice\n'}, {'relevant_file': 'src/index2.ts\n', 'label': 'enhancment'}]} + expected_output = {'code_suggestions': [{'relevant_file': 'src/index.ts\n', 'label': 'best practice\n'}, {'relevant_file': 'src/index2.ts\n', 'label': 'enhancement'}]} assert try_fix_yaml(review_text, first_key='code_suggestions', last_key='label') == expected_output From 10703a9098ca2f285f6545f09025381ce426e125 Mon Sep 17 00:00:00 2001 From: Hiroyuki Otomo Date: Fri, 23 May 2025 14:16:44 +0900 Subject: [PATCH 20/46] feat: add support for Claude 4 --- pr_agent/algo/__init__.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pr_agent/algo/__init__.py b/pr_agent/algo/__init__.py index e38bd713..f9a055f1 100644 --- a/pr_agent/algo/__init__.py +++ b/pr_agent/algo/__init__.py @@ -53,9 +53,11 @@ MAX_TOKENS = { 'vertex_ai/claude-3-5-haiku@20241022': 100000, 'vertex_ai/claude-3-sonnet@20240229': 100000, 'vertex_ai/claude-3-opus@20240229': 100000, + 'vertex_ai/claude-opus-4@20250514': 200000, 'vertex_ai/claude-3-5-sonnet@20240620': 100000, 'vertex_ai/claude-3-5-sonnet-v2@20241022': 100000, 'vertex_ai/claude-3-7-sonnet@20250219': 200000, + 'vertex_ai/claude-sonnet-4@20250514': 200000, 'vertex_ai/gemini-1.5-pro': 1048576, 'vertex_ai/gemini-2.5-pro-preview-03-25': 1048576, 'vertex_ai/gemini-2.5-pro-preview-05-06': 1048576, @@ -74,20 +76,24 @@ MAX_TOKENS = { 'anthropic.claude-v1': 100000, 'anthropic.claude-v2': 100000, 'anthropic/claude-3-opus-20240229': 100000, + 'anthropic/claude-opus-4-20250514': 200000, 'anthropic/claude-3-5-sonnet-20240620': 100000, 'anthropic/claude-3-5-sonnet-20241022': 100000, 'anthropic/claude-3-7-sonnet-20250219': 200000, + 'anthropic/claude-sonnet-4-20250514': 200000, 'claude-3-7-sonnet-20250219': 200000, 'anthropic/claude-3-5-haiku-20241022': 100000, 'bedrock/anthropic.claude-instant-v1': 100000, 'bedrock/anthropic.claude-v2': 100000, 'bedrock/anthropic.claude-v2:1': 100000, 'bedrock/anthropic.claude-3-sonnet-20240229-v1:0': 100000, + 'bedrock/anthropic.claude-opus-4-20250514-v1:0': 100000, 'bedrock/anthropic.claude-3-haiku-20240307-v1:0': 100000, 'bedrock/anthropic.claude-3-5-haiku-20241022-v1:0': 100000, 'bedrock/anthropic.claude-3-5-sonnet-20240620-v1:0': 100000, 'bedrock/anthropic.claude-3-5-sonnet-20241022-v2:0': 100000, 'bedrock/anthropic.claude-3-7-sonnet-20250219-v1:0': 200000, + 'bedrock/anthropic.claude-sonnet-4-20250514-v1:0': 200000, "bedrock/us.anthropic.claude-3-5-sonnet-20241022-v2:0": 100000, "bedrock/us.anthropic.claude-3-7-sonnet-20250219-v1:0": 200000, 'claude-3-5-sonnet': 100000, From c10be827a158a6a2f4dcec90036fa0d9f2309889 Mon Sep 17 00:00:00 2001 From: Hiroyuki Otomo Date: Fri, 23 May 2025 14:23:51 +0900 Subject: [PATCH 21/46] chore: update the version of anthropic sdk --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 95145ec9..864a3d48 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ aiohttp==3.9.5 -anthropic>=0.48 +anthropic>=0.52.0 #anthropic[vertex]==0.47.1 atlassian-python-api==3.41.4 azure-devops==7.1.0b3 From c4358d1ca0d6fc7f0040aa2e989e6a708128e28f Mon Sep 17 00:00:00 2001 From: Hiroyuki Otomo Date: Fri, 23 May 2025 19:18:09 +0900 Subject: [PATCH 22/46] chore: update the version of litellm to 1.70.4 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 864a3d48..d1587f25 100644 --- a/requirements.txt +++ b/requirements.txt @@ -13,7 +13,7 @@ google-cloud-aiplatform==1.38.0 google-generativeai==0.8.3 google-cloud-storage==2.10.0 Jinja2==3.1.2 -litellm==1.69.3 +litellm==1.70.4 loguru==0.7.2 msrest==0.7.1 openai>=1.55.3 From 1f836e405d4a7a1313a94d7bf3cb7b1aeb48be5a Mon Sep 17 00:00:00 2001 From: Hiroyuki Otomo Date: Sat, 24 May 2025 09:45:27 +0900 Subject: [PATCH 23/46] fix: reflect comments --- pr_agent/algo/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pr_agent/algo/__init__.py b/pr_agent/algo/__init__.py index f9a055f1..dbda2f43 100644 --- a/pr_agent/algo/__init__.py +++ b/pr_agent/algo/__init__.py @@ -87,15 +87,17 @@ MAX_TOKENS = { 'bedrock/anthropic.claude-v2': 100000, 'bedrock/anthropic.claude-v2:1': 100000, 'bedrock/anthropic.claude-3-sonnet-20240229-v1:0': 100000, - 'bedrock/anthropic.claude-opus-4-20250514-v1:0': 100000, + 'bedrock/anthropic.claude-opus-4-20250514-v1:0': 200000, 'bedrock/anthropic.claude-3-haiku-20240307-v1:0': 100000, 'bedrock/anthropic.claude-3-5-haiku-20241022-v1:0': 100000, 'bedrock/anthropic.claude-3-5-sonnet-20240620-v1:0': 100000, 'bedrock/anthropic.claude-3-5-sonnet-20241022-v2:0': 100000, 'bedrock/anthropic.claude-3-7-sonnet-20250219-v1:0': 200000, 'bedrock/anthropic.claude-sonnet-4-20250514-v1:0': 200000, + "bedrock/us.anthropic.claude-opus-4-20250514-v1:0": 200000, "bedrock/us.anthropic.claude-3-5-sonnet-20241022-v2:0": 100000, "bedrock/us.anthropic.claude-3-7-sonnet-20250219-v1:0": 200000, + "bedrock/us.anthropic.claude-sonnet-4-20250514-v1:0": 200000, 'claude-3-5-sonnet': 100000, 'groq/meta-llama/llama-4-scout-17b-16e-instruct': 131072, 'groq/meta-llama/llama-4-maverick-17b-128e-instruct': 131072, From 1bc0d488d5ee5afafb4c8e5ed601ada9922e4b78 Mon Sep 17 00:00:00 2001 From: TaskerJang Date: Thu, 22 May 2025 14:33:36 +0900 Subject: [PATCH 24/46] test: add comprehensive unit tests for clip_tokens function - Add 21 test cases covering edge cases and parameter combinations - Include tests for Unicode, special characters, and long text processing - Verify exception handling and backward compatibility --- tests/unittest/test_clip_tokens.py | 301 ++++++++++++++++++++++++++++- 1 file changed, 295 insertions(+), 6 deletions(-) diff --git a/tests/unittest/test_clip_tokens.py b/tests/unittest/test_clip_tokens.py index 79de6294..f791b92f 100644 --- a/tests/unittest/test_clip_tokens.py +++ b/tests/unittest/test_clip_tokens.py @@ -1,13 +1,302 @@ - -# Generated by CodiumAI - import pytest - +from unittest.mock import patch, MagicMock from pr_agent.algo.utils import clip_tokens +from pr_agent.algo.token_handler import TokenEncoder class TestClipTokens: - def test_clip(self): + """Comprehensive test suite for the clip_tokens function.""" + + def test_empty_input_text(self): + """Test that empty input returns empty string.""" + assert clip_tokens("", 10) == "" + assert clip_tokens(None, 10) == None + + def test_text_under_token_limit(self): + """Test that text under the token limit is returned unchanged.""" + text = "Short text" + max_tokens = 100 + result = clip_tokens(text, max_tokens) + assert result == text + + def test_text_exactly_at_token_limit(self): + """Test text that is exactly at the token limit.""" + text = "This is exactly at the limit" + # Mock the token encoder to return exact limit + with patch.object(TokenEncoder, 'get_token_encoder') as mock_encoder: + mock_tokenizer = MagicMock() + mock_tokenizer.encode.return_value = [1] * 10 # Exactly 10 tokens + mock_encoder.return_value = mock_tokenizer + + result = clip_tokens(text, 10) + assert result == text + + def test_text_over_token_limit_with_three_dots(self): + """Test text over token limit with three dots addition.""" + text = "This is a longer text that should be clipped when it exceeds the token limit" + max_tokens = 5 + + with patch.object(TokenEncoder, 'get_token_encoder') as mock_encoder: + mock_tokenizer = MagicMock() + mock_tokenizer.encode.return_value = [1] * 20 # 20 tokens + mock_encoder.return_value = mock_tokenizer + + result = clip_tokens(text, max_tokens) + assert result.endswith("\n...(truncated)") + assert len(result) < len(text) + + def test_text_over_token_limit_without_three_dots(self): + """Test text over token limit without three dots addition.""" + text = "This is a longer text that should be clipped" + max_tokens = 5 + + with patch.object(TokenEncoder, 'get_token_encoder') as mock_encoder: + mock_tokenizer = MagicMock() + mock_tokenizer.encode.return_value = [1] * 20 # 20 tokens + mock_encoder.return_value = mock_tokenizer + + result = clip_tokens(text, max_tokens, add_three_dots=False) + assert not result.endswith("\n...(truncated)") + assert len(result) < len(text) + + def test_negative_max_tokens(self): + """Test that negative max_tokens returns empty string.""" + text = "Some text" + result = clip_tokens(text, -1) + assert result == "" + + result = clip_tokens(text, -100) + assert result == "" + + def test_zero_max_tokens(self): + """Test that zero max_tokens returns empty string.""" + text = "Some text" + result = clip_tokens(text, 0) + assert result == "" + + def test_delete_last_line_functionality(self): + """Test the delete_last_line parameter functionality.""" + text = "Line 1\nLine 2\nLine 3\nLine 4" + max_tokens = 5 + + with patch.object(TokenEncoder, 'get_token_encoder') as mock_encoder: + mock_tokenizer = MagicMock() + mock_tokenizer.encode.return_value = [1] * 20 # 20 tokens + mock_encoder.return_value = mock_tokenizer + + # Without delete_last_line + result_normal = clip_tokens(text, max_tokens, delete_last_line=False) + + # With delete_last_line + result_deleted = clip_tokens(text, max_tokens, delete_last_line=True) + + # The result with delete_last_line should be shorter or equal + assert len(result_deleted) <= len(result_normal) + + def test_pre_computed_num_input_tokens(self): + """Test using pre-computed num_input_tokens parameter.""" + text = "This is a test text" + max_tokens = 10 + num_input_tokens = 15 + + # Should not call the encoder when num_input_tokens is provided + with patch.object(TokenEncoder, 'get_token_encoder') as mock_encoder: + mock_encoder.return_value = None # Should not be called + + result = clip_tokens(text, max_tokens, num_input_tokens=num_input_tokens) + assert result.endswith("\n...(truncated)") + mock_encoder.assert_not_called() + + def test_pre_computed_tokens_under_limit(self): + """Test pre-computed tokens under the limit.""" + text = "Short text" + max_tokens = 20 + num_input_tokens = 5 + + with patch.object(TokenEncoder, 'get_token_encoder') as mock_encoder: + mock_encoder.return_value = None # Should not be called + + result = clip_tokens(text, max_tokens, num_input_tokens=num_input_tokens) + assert result == text + mock_encoder.assert_not_called() + + def test_special_characters_and_unicode(self): + """Test text with special characters and Unicode content.""" + text = "Special chars: @#$%^&*()_+ áéíóú 中문 🚀 emoji" + max_tokens = 5 + + with patch.object(TokenEncoder, 'get_token_encoder') as mock_encoder: + mock_tokenizer = MagicMock() + mock_tokenizer.encode.return_value = [1] * 20 # 20 tokens + mock_encoder.return_value = mock_tokenizer + + result = clip_tokens(text, max_tokens) + assert isinstance(result, str) + assert len(result) < len(text) + + def test_multiline_text_handling(self): + """Test handling of multiline text.""" + text = "Line 1\nLine 2\nLine 3\nLine 4\nLine 5" + max_tokens = 5 + + with patch.object(TokenEncoder, 'get_token_encoder') as mock_encoder: + mock_tokenizer = MagicMock() + mock_tokenizer.encode.return_value = [1] * 20 # 20 tokens + mock_encoder.return_value = mock_tokenizer + + result = clip_tokens(text, max_tokens) + assert isinstance(result, str) + + def test_very_long_text(self): + """Test with very long text.""" + text = "A" * 10000 # Very long text + max_tokens = 10 + + with patch.object(TokenEncoder, 'get_token_encoder') as mock_encoder: + mock_tokenizer = MagicMock() + mock_tokenizer.encode.return_value = [1] * 5000 # Many tokens + mock_encoder.return_value = mock_tokenizer + + result = clip_tokens(text, max_tokens) + assert len(result) < len(text) + assert result.endswith("\n...(truncated)") + + def test_encoder_exception_handling(self): + """Test handling of encoder exceptions.""" + text = "Test text" + max_tokens = 10 + + with patch.object(TokenEncoder, 'get_token_encoder') as mock_encoder: + mock_encoder.side_effect = Exception("Encoder error") + + # Should return original text when encoder fails + result = clip_tokens(text, max_tokens) + assert result == text + + def test_zero_division_scenario(self): + """Test scenario that could lead to division by zero.""" + text = "Test" + max_tokens = 10 + + with patch.object(TokenEncoder, 'get_token_encoder') as mock_encoder: + mock_tokenizer = MagicMock() + mock_tokenizer.encode.return_value = [] # Empty tokens (could cause division by zero) + mock_encoder.return_value = mock_tokenizer + + result = clip_tokens(text, max_tokens) + # Should handle gracefully and return original text + assert result == text + + def test_various_edge_cases(self): + """Test various edge cases.""" + # Single character + assert clip_tokens("A", 1000) == "A" + + # Only whitespace + text = " \n \t " + with patch.object(TokenEncoder, 'get_token_encoder') as mock_encoder: + mock_tokenizer = MagicMock() + mock_tokenizer.encode.return_value = [1] * 10 + mock_encoder.return_value = mock_tokenizer + + result = clip_tokens(text, 5) + assert isinstance(result, str) + + # Text with only newlines + text = "\n\n\n\n" + with patch.object(TokenEncoder, 'get_token_encoder') as mock_encoder: + mock_tokenizer = MagicMock() + mock_tokenizer.encode.return_value = [1] * 10 + mock_encoder.return_value = mock_tokenizer + + result = clip_tokens(text, 2, delete_last_line=True) + assert isinstance(result, str) + + def test_parameter_combinations(self): + """Test different parameter combinations.""" + text = "Multi\nline\ntext\nfor\ntesting" + max_tokens = 5 + + with patch.object(TokenEncoder, 'get_token_encoder') as mock_encoder: + mock_tokenizer = MagicMock() + mock_tokenizer.encode.return_value = [1] * 20 + mock_encoder.return_value = mock_tokenizer + + # Test all combinations + combinations = [ + (True, True), # add_three_dots=True, delete_last_line=True + (True, False), # add_three_dots=True, delete_last_line=False + (False, True), # add_three_dots=False, delete_last_line=True + (False, False), # add_three_dots=False, delete_last_line=False + ] + + for add_dots, delete_line in combinations: + result = clip_tokens(text, max_tokens, + add_three_dots=add_dots, + delete_last_line=delete_line) + assert isinstance(result, str) + if add_dots and len(result) > 0: + assert result.endswith("\n...(truncated)") or result == text + + def test_num_output_chars_zero_scenario(self): + """Test scenario where num_output_chars becomes zero or negative.""" + text = "Short" + max_tokens = 1 + + with patch.object(TokenEncoder, 'get_token_encoder') as mock_encoder: + mock_tokenizer = MagicMock() + mock_tokenizer.encode.return_value = [1] * 1000 # Many tokens for short text + mock_encoder.return_value = mock_tokenizer + + result = clip_tokens(text, max_tokens) + # When num_output_chars is 0 or negative, should return empty string + assert result == "" + + def test_logging_on_exception(self): + """Test that exceptions are properly logged.""" + text = "Test text" + max_tokens = 10 + + # Patch the logger at the module level where it's imported + with patch('pr_agent.algo.utils.get_logger') as mock_logger: + mock_log_instance = MagicMock() + mock_logger.return_value = mock_log_instance + + with patch.object(TokenEncoder, 'get_token_encoder') as mock_encoder: + mock_encoder.side_effect = Exception("Test exception") + + result = clip_tokens(text, max_tokens) + + # Should log the warning + mock_log_instance.warning.assert_called_once() + # Should return original text + assert result == text + + def test_factor_safety_calculation(self): + """Test that the 0.9 factor (10% reduction) works correctly.""" + text = "Test text that should be reduced by 10 percent for safety" + max_tokens = 10 + + with patch.object(TokenEncoder, 'get_token_encoder') as mock_encoder: + mock_tokenizer = MagicMock() + mock_tokenizer.encode.return_value = [1] * 20 # 20 tokens + mock_encoder.return_value = mock_tokenizer + + result = clip_tokens(text, max_tokens) + + # The result should be shorter due to the 0.9 factor + # Characters per token = len(text) / 20 + # Expected chars = int(0.9 * (len(text) / 20) * 10) + expected_chars = int(0.9 * (len(text) / 20) * 10) + + # Result should be around expected_chars length (plus truncation text) + if result.endswith("\n...(truncated)"): + actual_content = result[:-len("\n...(truncated)")] + assert len(actual_content) <= expected_chars + 5 # Some tolerance + + # Test the original basic functionality to ensure backward compatibility + def test_clip_original_functionality(self): + """Test original functionality from the existing test.""" text = "line1\nline2\nline3\nline4\nline5\nline6" max_tokens = 25 result = clip_tokens(text, max_tokens) @@ -16,4 +305,4 @@ class TestClipTokens: max_tokens = 10 result = clip_tokens(text, max_tokens) expected_results = 'line1\nline2\nline3\n\n...(truncated)' - assert result == expected_results + assert result == expected_results \ No newline at end of file From e2586cb64ad2b5328d8bddc4b15c0eeadca840f4 Mon Sep 17 00:00:00 2001 From: TaskerJang Date: Thu, 22 May 2025 14:33:45 +0900 Subject: [PATCH 25/46] docs: improve clip_tokens function docstring and add examples --- pr_agent/algo/utils.py | 60 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 57 insertions(+), 3 deletions(-) diff --git a/pr_agent/algo/utils.py b/pr_agent/algo/utils.py index 780c7953..3e3103ad 100644 --- a/pr_agent/algo/utils.py +++ b/pr_agent/algo/utils.py @@ -945,12 +945,66 @@ def clip_tokens(text: str, max_tokens: int, add_three_dots=True, num_input_token """ Clip the number of tokens in a string to a maximum number of tokens. + This function limits text to a specified token count by calculating the approximate + character-to-token ratio and truncating the text accordingly. A safety factor of 0.9 + (10% reduction) is applied to ensure the result stays within the token limit. + Args: - text (str): The string to clip. + text (str): The string to clip. If empty or None, returns the input unchanged. max_tokens (int): The maximum number of tokens allowed in the string. - add_three_dots (bool, optional): A boolean indicating whether to add three dots at the end of the clipped + If negative, returns an empty string. + add_three_dots (bool, optional): Whether to add "\\n...(truncated)" at the end + of the clipped text to indicate truncation. + Defaults to True. + num_input_tokens (int, optional): Pre-computed number of tokens in the input text. + If provided, skips token encoding step for efficiency. + If None, tokens will be counted using TokenEncoder. + Defaults to None. + delete_last_line (bool, optional): Whether to remove the last line from the + clipped content before adding truncation indicator. + Useful for ensuring clean breaks at line boundaries. + Defaults to False. + Returns: - str: The clipped string. + str: The clipped string. Returns original text if: + - Text is empty/None + - Token count is within limit + - An error occurs during processing + + Returns empty string if max_tokens <= 0. + + Examples: + Basic usage: + >>> text = "This is a sample text that might be too long" + >>> result = clip_tokens(text, max_tokens=10) + >>> print(result) + This is a sample... + (truncated) + + Without truncation indicator: + >>> result = clip_tokens(text, max_tokens=10, add_three_dots=False) + >>> print(result) + This is a sample + + With pre-computed token count: + >>> result = clip_tokens(text, max_tokens=5, num_input_tokens=15) + >>> print(result) + This... + (truncated) + + With line deletion: + >>> multiline_text = "Line 1\\nLine 2\\nLine 3" + >>> result = clip_tokens(multiline_text, max_tokens=3, delete_last_line=True) + >>> print(result) + Line 1 + Line 2 + ... + (truncated) + + Notes: + The function uses a safety factor of 0.9 (10% reduction) to ensure the + result stays within the token limit, as character-to-token ratios can vary. + If token encoding fails, the original text is returned with a warning logged. """ if not text: return text From 95c94b80a271d87a007acbc72ecfc6157c4636aa Mon Sep 17 00:00:00 2001 From: Peter Dave Hello Date: Sat, 24 May 2025 14:22:55 +0800 Subject: [PATCH 26/46] Add Grok-3 non-beta model IDs --- pr_agent/algo/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pr_agent/algo/__init__.py b/pr_agent/algo/__init__.py index e38bd713..e659e540 100644 --- a/pr_agent/algo/__init__.py +++ b/pr_agent/algo/__init__.py @@ -102,9 +102,13 @@ MAX_TOKENS = { 'xai/grok-2': 131072, 'xai/grok-2-1212': 131072, 'xai/grok-2-latest': 131072, + 'xai/grok-3': 131072, 'xai/grok-3-beta': 131072, + 'xai/grok-3-fast': 131072, 'xai/grok-3-fast-beta': 131072, + 'xai/grok-3-mini': 131072, 'xai/grok-3-mini-beta': 131072, + 'xai/grok-3-mini-fast': 131072, 'xai/grok-3-mini-fast-beta': 131072, 'ollama/llama3': 4096, 'watsonx/meta-llama/llama-3-8b-instruct': 4096, From 453f8e19f3e020d18bf34c1b4b974d44fafd7f19 Mon Sep 17 00:00:00 2001 From: Tal Date: Sat, 24 May 2025 16:12:37 +0300 Subject: [PATCH 27/46] Update docs/docs/usage-guide/changing_a_model.md Co-authored-by: qodo-merge-pro-for-open-source[bot] <189517486+qodo-merge-pro-for-open-source[bot]@users.noreply.github.com> --- docs/docs/usage-guide/changing_a_model.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/usage-guide/changing_a_model.md b/docs/docs/usage-guide/changing_a_model.md index e7f9e16a..bfeb7c3a 100644 --- a/docs/docs/usage-guide/changing_a_model.md +++ b/docs/docs/usage-guide/changing_a_model.md @@ -226,7 +226,7 @@ To use Amazon Bedrock and its foundational models, add the below configuration: model="bedrock/anthropic.claude-3-5-sonnet-20240620-v1:0" fallback_models=["bedrock/anthropic.claude-3-5-sonnet-20240620-v1:0"] -[aws] # in .secrets.toml +[aws] AWS_ACCESS_KEY_ID="..." AWS_SECRET_ACCESS_KEY="..." AWS_REGION_NAME="..." From 16b9ccd0259a8d3a32db2366516b431aa5b20153 Mon Sep 17 00:00:00 2001 From: soprue Date: Sat, 24 May 2025 23:43:16 +0900 Subject: [PATCH 28/46] feat: conditionally include diagram in output example --- pr_agent/settings/pr_description_prompts.toml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/pr_agent/settings/pr_description_prompts.toml b/pr_agent/settings/pr_description_prompts.toml index 73ec8459..d37839d5 100644 --- a/pr_agent/settings/pr_description_prompts.toml +++ b/pr_agent/settings/pr_description_prompts.toml @@ -60,6 +60,14 @@ type: - ... description: | ... +{%- if add_diagram %} +diagram: | + sequenceDiagram + participant A as ComponentA + participant B as ComponentB + A->>B: functionCall() + B-->>A: response +{%- endif %} title: | ... {%- if enable_semantic_files_types %} @@ -141,6 +149,14 @@ type: - ... description: | ... +{%- if add_diagram %} +diagram: | + sequenceDiagram + participant A as ComponentA + participant B as ComponentB + A->>B: functionCall() + B-->>A: response +{%- endif %} title: | ... {%- if enable_semantic_files_types %} From f5bb508736dfada4b1ef12608b3a8339f4a8bab7 Mon Sep 17 00:00:00 2001 From: TaskerJang Date: Sun, 25 May 2025 07:55:18 +0900 Subject: [PATCH 29/46] fix: use identity check for None comparison in clip_tokens tests - Replace `== None` with `is None` in test_empty_input_text method - Follow Python best practice for None comparisons as recommended in code review - Address feedback from PR #1816 review comment Co-authored-by: mrT23 --- tests/unittest/test_clip_tokens.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unittest/test_clip_tokens.py b/tests/unittest/test_clip_tokens.py index f791b92f..a42ef929 100644 --- a/tests/unittest/test_clip_tokens.py +++ b/tests/unittest/test_clip_tokens.py @@ -10,7 +10,7 @@ class TestClipTokens: def test_empty_input_text(self): """Test that empty input returns empty string.""" assert clip_tokens("", 10) == "" - assert clip_tokens(None, 10) == None + assert clip_tokens(None, 10) is None def test_text_under_token_limit(self): """Test that text under the token limit is returned unchanged.""" From d62cbb2fc4058e998c745f76c33b76e6717d9fe7 Mon Sep 17 00:00:00 2001 From: yujindonut Date: Sun, 25 May 2025 10:28:50 +0900 Subject: [PATCH 30/46] feat: add add_diagram flag in configuration.toml --- pr_agent/settings/configuration.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index c6437931..15c99a32 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -103,6 +103,7 @@ enable_pr_type=true final_update_message = true enable_help_text=false enable_help_comment=true +add_diagram=false # describe as comment publish_description_as_comment=false publish_description_as_comment_persistent=true From 5a0affd6cba57b8e08bbc90dc39719de7d214b33 Mon Sep 17 00:00:00 2001 From: ssunbear Date: Sun, 25 May 2025 11:08:52 +0900 Subject: [PATCH 31/46] feat: add add_diagram configuration option to PR description --- pr_agent/tools/pr_description.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pr_agent/tools/pr_description.py b/pr_agent/tools/pr_description.py index df82db67..bc8983f4 100644 --- a/pr_agent/tools/pr_description.py +++ b/pr_agent/tools/pr_description.py @@ -73,6 +73,7 @@ class PRDescription: "related_tickets": "", "include_file_summary_changes": len(self.git_provider.get_diff_files()) <= self.COLLAPSIBLE_FILE_LIST_THRESHOLD, 'duplicate_prompt_examples': get_settings().config.get('duplicate_prompt_examples', False), + "add_diagram": get_settings().config.get('pr_description.add_diagram', True), } self.user_description = self.git_provider.get_user_description() From 94e1126b003f056f0da2e53e0af591fa4d510333 Mon Sep 17 00:00:00 2001 From: Judonguk Date: Sun, 25 May 2025 11:09:11 +0900 Subject: [PATCH 32/46] add docs about Mermaid Diagram --- docs/docs/tools/describe.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/docs/tools/describe.md b/docs/docs/tools/describe.md index 0f2c45a7..4f4e2232 100644 --- a/docs/docs/tools/describe.md +++ b/docs/docs/tools/describe.md @@ -1,6 +1,18 @@ ## Overview The `describe` tool scans the PR code changes, and generates a description for the PR - title, type, summary, walkthrough and labels. +### Mermaid Diagram Support +When the `add_diagram` option is enabled in your configuration, the `/describe` tool will include a `Mermaid` sequence diagram in the PR description. + +This diagram represents interactions between components/functions based on the diff content. + +### How to enable + +In your configuration: + +toml +[pr_description] +add_diagram = true The tool can be triggered automatically every time a new PR is [opened](../usage-guide/automations_and_usage.md#github-app-automatic-tools-when-a-new-pr-is-opened), or it can be invoked manually by commenting on any PR: @@ -109,6 +121,10 @@ Everything below this marker is treated as previously auto-generated content and enable_help_text If set to true, the tool will display a help text in the comment. Default is false. + + add_diagram + If set to true, the tool will generate a Mermaid sequence diagram (in code block format) describing component interactions based on the code changes. Default is false. + ## Inline file summary 💎 From 3a385b62d6bce11841302775e56a5c601d219684 Mon Sep 17 00:00:00 2001 From: isExample Date: Sun, 25 May 2025 11:51:22 +0900 Subject: [PATCH 33/46] feat: conditionally append Mermaid sequence diagram instruction in pr_description prompt --- pr_agent/settings/pr_description_prompts.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pr_agent/settings/pr_description_prompts.toml b/pr_agent/settings/pr_description_prompts.toml index 73ec8459..1dd76d7b 100644 --- a/pr_agent/settings/pr_description_prompts.toml +++ b/pr_agent/settings/pr_description_prompts.toml @@ -44,7 +44,7 @@ class FileDescription(BaseModel): class PRDescription(BaseModel): type: List[PRType] = Field(description="one or more types that describe the PR content. Return the label member value (e.g. 'Bug fix', not 'bug_fix')") - description: str = Field(description="summarize the PR changes in up to four bullet points, each up to 8 words. For large PRs, add sub-bullets if needed. Order bullets by importance, with each bullet highlighting a key change group.") + description: str = Field(description="summarize the PR changes in up to four bullet points, each up to 8 words. For large PRs, add sub-bullets if needed. Order bullets by importance, with each bullet highlighting a key change group. {% if add_diagram %} Also, generate a Mermaid sequence diagram that focuses on the main function call flow between classes or components. {% endif %}") title: str = Field(description="a concise and descriptive title that captures the PR's main theme") {%- if enable_semantic_files_types %} pr_files: List[FileDescription] = Field(max_items=20, description="a list of all the files that were changed in the PR, and summary of their changes. Each file must be analyzed regardless of change size.") From c346d784e394c3a9580a37f7cded10d783fc5b33 Mon Sep 17 00:00:00 2001 From: chilln Date: Sun, 25 May 2025 12:47:09 +0900 Subject: [PATCH 34/46] docs:move sequence diagram section below main explanation of describe --- docs/docs/tools/describe.md | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/docs/docs/tools/describe.md b/docs/docs/tools/describe.md index 4f4e2232..03cb0c62 100644 --- a/docs/docs/tools/describe.md +++ b/docs/docs/tools/describe.md @@ -1,18 +1,6 @@ ## Overview The `describe` tool scans the PR code changes, and generates a description for the PR - title, type, summary, walkthrough and labels. -### Mermaid Diagram Support -When the `add_diagram` option is enabled in your configuration, the `/describe` tool will include a `Mermaid` sequence diagram in the PR description. - -This diagram represents interactions between components/functions based on the diff content. - -### How to enable - -In your configuration: - -toml -[pr_description] -add_diagram = true The tool can be triggered automatically every time a new PR is [opened](../usage-guide/automations_and_usage.md#github-app-automatic-tools-when-a-new-pr-is-opened), or it can be invoked manually by commenting on any PR: @@ -68,6 +56,19 @@ Everything below this marker is treated as previously auto-generated content and ![Describe comment](https://codium.ai/images/pr_agent/pr_description_user_description.png){width=512} +### Sequence Diagram Support +When the `add_diagram` option is enabled in your configuration, the `/describe` tool will include a `Mermaid` sequence diagram in the PR description. + +This diagram represents interactions between components/functions based on the diff content. + +### How to enable + +In your configuration: + +toml +[pr_description] +add_diagram = true + ## Configuration options !!! example "Possible configurations" From f58c40a6aed7cde8ee4a0032d57223bf91af10d9 Mon Sep 17 00:00:00 2001 From: chilln Date: Sun, 25 May 2025 12:48:13 +0900 Subject: [PATCH 35/46] refactor: replace single quotes with double quotes to match existing code style --- pr_agent/tools/pr_description.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pr_agent/tools/pr_description.py b/pr_agent/tools/pr_description.py index bc8983f4..1f28fb69 100644 --- a/pr_agent/tools/pr_description.py +++ b/pr_agent/tools/pr_description.py @@ -72,7 +72,7 @@ class PRDescription: "enable_semantic_files_types": get_settings().pr_description.enable_semantic_files_types, "related_tickets": "", "include_file_summary_changes": len(self.git_provider.get_diff_files()) <= self.COLLAPSIBLE_FILE_LIST_THRESHOLD, - 'duplicate_prompt_examples': get_settings().config.get('duplicate_prompt_examples', False), + "duplicate_prompt_examples": get_settings().config.get("duplicate_prompt_examples", False), "add_diagram": get_settings().config.get('pr_description.add_diagram', True), } From e57d3101e4ae2edf4164b78efdf88f0351d1e75d Mon Sep 17 00:00:00 2001 From: chilln Date: Sun, 25 May 2025 12:48:29 +0900 Subject: [PATCH 36/46] fix:set parameter default to false to make the feature opt-in by design --- pr_agent/tools/pr_description.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pr_agent/tools/pr_description.py b/pr_agent/tools/pr_description.py index 1f28fb69..84dd2b47 100644 --- a/pr_agent/tools/pr_description.py +++ b/pr_agent/tools/pr_description.py @@ -73,7 +73,7 @@ class PRDescription: "related_tickets": "", "include_file_summary_changes": len(self.git_provider.get_diff_files()) <= self.COLLAPSIBLE_FILE_LIST_THRESHOLD, "duplicate_prompt_examples": get_settings().config.get("duplicate_prompt_examples", False), - "add_diagram": get_settings().config.get('pr_description.add_diagram', True), + "add_diagram": get_settings().config.get('pr_description.add_diagram', False), } self.user_description = self.git_provider.get_user_description() From 18a8a741fa4031e99d5007762d4ef04841f96e18 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Sun, 25 May 2025 08:31:24 +0300 Subject: [PATCH 37/46] ensure_ticket_compliance --- docs/docs/tools/improve.md | 14 +++++++++++++- pr_agent/settings/configuration.toml | 1 + 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/docs/docs/tools/improve.md b/docs/docs/tools/improve.md index 2777a6d5..3d4b0336 100644 --- a/docs/docs/tools/improve.md +++ b/docs/docs/tools/improve.md @@ -435,7 +435,7 @@ To enable auto-approval based on specific criteria, first, you need to enable th enable_auto_approval = true ``` -There are two criteria that can be set for auto-approval: +There are several criteria that can be set for auto-approval: - **Review effort score** @@ -459,6 +459,18 @@ auto_approve_for_no_suggestions = true When no [code suggestion](https://www.qodo.ai/images/pr_agent/code_suggestions_as_comment_closed.png) were found for the PR, the PR will be auto-approved. +___ + +- **Ticket Compliance** + +```toml +[config] +enable_auto_approval = true +ensure_ticket_compliance = true # Default is false +``` + +If `ensure_ticket_compliance` is set to `true`, auto-approval will be disabled if a ticket is linked to the PR and the ticket is not compliant (e.g., the `review` tool did not mark the PR as fully compliant with the ticket). This ensures that PRs are only auto-approved if their associated tickets are properly resolved. + ### How many code suggestions are generated? Qodo Merge uses a dynamic strategy to generate code suggestions based on the size of the pull request (PR). Here's how it works: diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index c6437931..7d8e1e82 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -64,6 +64,7 @@ reasoning_effort = "medium" # "low", "medium", "high" enable_auto_approval=false # Set to true to enable auto-approval of PRs under certain conditions auto_approve_for_low_review_effort=-1 # -1 to disable, [1-5] to set the threshold for auto-approval auto_approve_for_no_suggestions=false # If true, the PR will be auto-approved if there are no suggestions +ensure_ticket_compliance=false # Set to true to disable auto-approval of PRs if the ticket is not compliant # extended thinking for Claude reasoning models enable_claude_extended_thinking = false # Set to true to enable extended thinking feature extended_thinking_budget_tokens = 2048 From 415817b4217e629f2648d5fc4dcaef8afe413e08 Mon Sep 17 00:00:00 2001 From: Tal Date: Sun, 25 May 2025 08:42:55 +0300 Subject: [PATCH 38/46] Update docs/docs/tools/improve.md Co-authored-by: qodo-merge-pro-for-open-source[bot] <189517486+qodo-merge-pro-for-open-source[bot]@users.noreply.github.com> --- docs/docs/tools/improve.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/tools/improve.md b/docs/docs/tools/improve.md index 3d4b0336..54ece175 100644 --- a/docs/docs/tools/improve.md +++ b/docs/docs/tools/improve.md @@ -457,7 +457,7 @@ enable_auto_approval = true auto_approve_for_no_suggestions = true ``` -When no [code suggestion](https://www.qodo.ai/images/pr_agent/code_suggestions_as_comment_closed.png) were found for the PR, the PR will be auto-approved. +When no [code suggestions](https://www.qodo.ai/images/pr_agent/code_suggestions_as_comment_closed.png) were found for the PR, the PR will be auto-approved. ___ From aa3e5b79c89600c03db79c95b8aabae93ca9cd27 Mon Sep 17 00:00:00 2001 From: chilln Date: Sun, 25 May 2025 17:55:14 +0900 Subject: [PATCH 39/46] docs:apply proper formatting to documentation --- docs/docs/tools/describe.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/docs/tools/describe.md b/docs/docs/tools/describe.md index 03cb0c62..1127c875 100644 --- a/docs/docs/tools/describe.md +++ b/docs/docs/tools/describe.md @@ -65,9 +65,11 @@ This diagram represents interactions between components/functions based on the d In your configuration: +``` toml [pr_description] add_diagram = true +``` ## Configuration options From 84f2f4fe3d8b601f55c39de1e5e03988703990d9 Mon Sep 17 00:00:00 2001 From: kkan9ma Date: Sun, 25 May 2025 18:00:38 +0900 Subject: [PATCH 40/46] Fix: use ModelTypeValidator static methods directly --- pr_agent/algo/token_handler.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pr_agent/algo/token_handler.py b/pr_agent/algo/token_handler.py index 998391b1..cb313f02 100644 --- a/pr_agent/algo/token_handler.py +++ b/pr_agent/algo/token_handler.py @@ -67,7 +67,6 @@ class TokenHandler: - user: The user string. """ self.encoder = TokenEncoder.get_token_encoder() - self.model_validator = ModelTypeValidator() if pr is not None: self.prompt_tokens = self._get_system_user_tokens(pr, self.encoder, vars, system, user) @@ -144,10 +143,10 @@ class TokenHandler: """ model_name = get_settings().config.model.lower() - if self.model_validator.is_openai_model(model_name) and get_settings(use_context=False).get('openai.key'): + if ModelTypeValidator.is_openai_model(model_name) and get_settings(use_context=False).get('openai.key'): return default_estimate - - if self.model_validator.is_anthropic_model(model_name) and get_settings(use_context=False).get('anthropic.key'): + + if ModelTypeValidator.is_anthropic_model(model_name) and get_settings(use_context=False).get('anthropic.key'): return self._calc_claude_tokens(patch) return self._apply_estimation_factor(model_name, default_estimate) From 6aac41a0dfd58c8de0e0f8607fd9845ab9189832 Mon Sep 17 00:00:00 2001 From: chilln Date: Sun, 25 May 2025 18:27:03 +0900 Subject: [PATCH 41/46] refactor:rename to --- pr_agent/settings/configuration.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index 15c99a32..5d39796b 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -103,7 +103,7 @@ enable_pr_type=true final_update_message = true enable_help_text=false enable_help_comment=true -add_diagram=false +enable_pr_diagram=false # adds a section with a diagram of the PR changes # describe as comment publish_description_as_comment=false publish_description_as_comment_persistent=true From d5dead5c7f63acd501ce1dd10b1f6175cb8d733f Mon Sep 17 00:00:00 2001 From: chilln Date: Sun, 25 May 2025 18:37:28 +0900 Subject: [PATCH 42/46] refactor: moved diagram logic to 'changes_diagram' in PRDescription and updated prompt for clarity --- pr_agent/settings/pr_description_prompts.toml | 35 +++++++++---------- pr_agent/tools/pr_description.py | 8 +++-- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/pr_agent/settings/pr_description_prompts.toml b/pr_agent/settings/pr_description_prompts.toml index 8a66f2f9..bbe55252 100644 --- a/pr_agent/settings/pr_description_prompts.toml +++ b/pr_agent/settings/pr_description_prompts.toml @@ -44,11 +44,14 @@ class FileDescription(BaseModel): class PRDescription(BaseModel): type: List[PRType] = Field(description="one or more types that describe the PR content. Return the label member value (e.g. 'Bug fix', not 'bug_fix')") - description: str = Field(description="summarize the PR changes in up to four bullet points, each up to 8 words. For large PRs, add sub-bullets if needed. Order bullets by importance, with each bullet highlighting a key change group. {% if add_diagram %} Also, generate a Mermaid sequence diagram that focuses on the main function call flow between classes or components. {% endif %}") + description: str = Field(description="summarize the PR changes in up to four bullet points, each up to 8 words. For large PRs, add sub-bullets if needed. Order bullets by importance, with each bullet highlighting a key change group.") title: str = Field(description="a concise and descriptive title that captures the PR's main theme") {%- if enable_semantic_files_types %} pr_files: List[FileDescription] = Field(max_items=20, description="a list of all the files that were changed in the PR, and summary of their changes. Each file must be analyzed regardless of change size.") {%- endif %} +{%- if enable_pr_diagram %} + changes_diagram: str = Field(description="a horizontal diagram that represents the main PR changes, in the format of a mermaid flowchart. The diagram should be concise and easy to read. Leave empty if no diagram is relevant.") +{%- endif %} ===== @@ -60,14 +63,6 @@ type: - ... description: | ... -{%- if add_diagram %} -diagram: | - sequenceDiagram - participant A as ComponentA - participant B as ComponentB - A->>B: functionCall() - B-->>A: response -{%- endif %} title: | ... {%- if enable_semantic_files_types %} @@ -84,6 +79,12 @@ pr_files: label_key_1 ... {%- endif %} +{%- if enable_pr_diagram %} + changes_diagram: | + ```mermaid + ... + ``` +{%- endif %} ``` Answer should be a valid YAML, and nothing else. Each YAML output MUST be after a newline, with proper indent, and block scalar indicator ('|') @@ -149,14 +150,6 @@ type: - ... description: | ... -{%- if add_diagram %} -diagram: | - sequenceDiagram - participant A as ComponentA - participant B as ComponentB - A->>B: functionCall() - B-->>A: response -{%- endif %} title: | ... {%- if enable_semantic_files_types %} @@ -173,6 +166,12 @@ pr_files: label_key_1 ... {%- endif %} +{%- if enable_pr_diagram %} + changes_diagram: | + ```mermaid + ... + ``` +{%- endif %} ``` (replace '...' with the actual values) {%- endif %} @@ -180,4 +179,4 @@ pr_files: Response (should be a valid YAML, and nothing else): ```yaml -""" +""" \ No newline at end of file diff --git a/pr_agent/tools/pr_description.py b/pr_agent/tools/pr_description.py index 84dd2b47..3bd4ecf7 100644 --- a/pr_agent/tools/pr_description.py +++ b/pr_agent/tools/pr_description.py @@ -73,7 +73,7 @@ class PRDescription: "related_tickets": "", "include_file_summary_changes": len(self.git_provider.get_diff_files()) <= self.COLLAPSIBLE_FILE_LIST_THRESHOLD, "duplicate_prompt_examples": get_settings().config.get("duplicate_prompt_examples", False), - "add_diagram": get_settings().config.get('pr_description.add_diagram', False), + "enable_pr_diagram": get_settings().pr_description.get("enable_pr_diagram", False), } self.user_description = self.git_provider.get_user_description() @@ -457,6 +457,10 @@ class PRDescription: self.data['labels'] = self.data.pop('labels') if 'description' in self.data: self.data['description'] = self.data.pop('description') + if 'changes_diagram' in self.data: + changes_diagram = self.data.pop('changes_diagram') + if changes_diagram.strip(): + self.data['changes_diagram'] = changes_diagram if 'pr_files' in self.data: self.data['pr_files'] = self.data.pop('pr_files') @@ -821,4 +825,4 @@ def replace_code_tags(text): parts = text.split('`') for i in range(1, len(parts), 2): parts[i] = '' + parts[i] + '' - return ''.join(parts) + return ''.join(parts) \ No newline at end of file From d2194c7ed9be09a50165e4bf52570bba2b5003be Mon Sep 17 00:00:00 2001 From: chilln Date: Sun, 25 May 2025 18:39:39 +0900 Subject: [PATCH 43/46] docs:rename parameter ('add_diagram' -> 'enable_pr_diagram') --- docs/docs/tools/describe.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/docs/tools/describe.md b/docs/docs/tools/describe.md index 1127c875..a967a6c5 100644 --- a/docs/docs/tools/describe.md +++ b/docs/docs/tools/describe.md @@ -57,7 +57,7 @@ Everything below this marker is treated as previously auto-generated content and ![Describe comment](https://codium.ai/images/pr_agent/pr_description_user_description.png){width=512} ### Sequence Diagram Support -When the `add_diagram` option is enabled in your configuration, the `/describe` tool will include a `Mermaid` sequence diagram in the PR description. +When the `enable_pr_diagram` option is enabled in your configuration, the `/describe` tool will include a `Mermaid` sequence diagram in the PR description. This diagram represents interactions between components/functions based on the diff content. @@ -68,7 +68,7 @@ In your configuration: ``` toml [pr_description] -add_diagram = true +enable_pr_diagram = true ``` ## Configuration options From f3cb4e838475386b2f87c66c700c65e85759dc57 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Sun, 25 May 2025 14:32:12 +0300 Subject: [PATCH 44/46] fix: ensure proper formatting of changes_diagram in PR description output --- pr_agent/settings/pr_description_prompts.toml | 30 +++++++++---------- pr_agent/tools/pr_description.py | 8 +++-- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/pr_agent/settings/pr_description_prompts.toml b/pr_agent/settings/pr_description_prompts.toml index bbe55252..934b30da 100644 --- a/pr_agent/settings/pr_description_prompts.toml +++ b/pr_agent/settings/pr_description_prompts.toml @@ -46,12 +46,12 @@ class PRDescription(BaseModel): type: List[PRType] = Field(description="one or more types that describe the PR content. Return the label member value (e.g. 'Bug fix', not 'bug_fix')") description: str = Field(description="summarize the PR changes in up to four bullet points, each up to 8 words. For large PRs, add sub-bullets if needed. Order bullets by importance, with each bullet highlighting a key change group.") title: str = Field(description="a concise and descriptive title that captures the PR's main theme") -{%- if enable_semantic_files_types %} - pr_files: List[FileDescription] = Field(max_items=20, description="a list of all the files that were changed in the PR, and summary of their changes. Each file must be analyzed regardless of change size.") -{%- endif %} {%- if enable_pr_diagram %} changes_diagram: str = Field(description="a horizontal diagram that represents the main PR changes, in the format of a mermaid flowchart. The diagram should be concise and easy to read. Leave empty if no diagram is relevant.") {%- endif %} +{%- if enable_semantic_files_types %} + pr_files: List[FileDescription] = Field(max_items=20, description="a list of all the files that were changed in the PR, and summary of their changes. Each file must be analyzed regardless of change size.") +{%- endif %} ===== @@ -65,6 +65,12 @@ description: | ... title: | ... +{%- if enable_pr_diagram %} + changes_diagram: | + ```mermaid + ... + ``` +{%- endif %} {%- if enable_semantic_files_types %} pr_files: - filename: | @@ -79,12 +85,6 @@ pr_files: label_key_1 ... {%- endif %} -{%- if enable_pr_diagram %} - changes_diagram: | - ```mermaid - ... - ``` -{%- endif %} ``` Answer should be a valid YAML, and nothing else. Each YAML output MUST be after a newline, with proper indent, and block scalar indicator ('|') @@ -152,6 +152,12 @@ description: | ... title: | ... +{%- if enable_pr_diagram %} + changes_diagram: | + ```mermaid + ... + ``` +{%- endif %} {%- if enable_semantic_files_types %} pr_files: - filename: | @@ -166,12 +172,6 @@ pr_files: label_key_1 ... {%- endif %} -{%- if enable_pr_diagram %} - changes_diagram: | - ```mermaid - ... - ``` -{%- endif %} ``` (replace '...' with the actual values) {%- endif %} diff --git a/pr_agent/tools/pr_description.py b/pr_agent/tools/pr_description.py index 3bd4ecf7..663c5a2d 100644 --- a/pr_agent/tools/pr_description.py +++ b/pr_agent/tools/pr_description.py @@ -458,9 +458,11 @@ class PRDescription: if 'description' in self.data: self.data['description'] = self.data.pop('description') if 'changes_diagram' in self.data: - changes_diagram = self.data.pop('changes_diagram') - if changes_diagram.strip(): - self.data['changes_diagram'] = changes_diagram + changes_diagram = self.data.pop('changes_diagram').strip() + if changes_diagram.startswith('```'): + if not changes_diagram.endswith('```'): # fallback for missing closing + changes_diagram += '\n```' + self.data['changes_diagram'] = '\n'+ changes_diagram if 'pr_files' in self.data: self.data['pr_files'] = self.data.pop('pr_files') From 2eeb9b041130fd4816e2e27026bf6a52b04c7c63 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Sun, 25 May 2025 14:59:18 +0300 Subject: [PATCH 45/46] fix: improve Mermaid diagram formatting and instructions in PR description template --- pr_agent/settings/pr_description_prompts.toml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pr_agent/settings/pr_description_prompts.toml b/pr_agent/settings/pr_description_prompts.toml index 934b30da..81cb0afb 100644 --- a/pr_agent/settings/pr_description_prompts.toml +++ b/pr_agent/settings/pr_description_prompts.toml @@ -47,7 +47,7 @@ class PRDescription(BaseModel): description: str = Field(description="summarize the PR changes in up to four bullet points, each up to 8 words. For large PRs, add sub-bullets if needed. Order bullets by importance, with each bullet highlighting a key change group.") title: str = Field(description="a concise and descriptive title that captures the PR's main theme") {%- if enable_pr_diagram %} - changes_diagram: str = Field(description="a horizontal diagram that represents the main PR changes, in the format of a mermaid flowchart. The diagram should be concise and easy to read. Leave empty if no diagram is relevant.") + changes_diagram: str = Field(description="a horizontal diagram that represents the main PR changes, in the format of a valid mermaid LR flowchart. The diagram should be concise and easy to read. Leave empty if no diagram is relevant. To create robust Mermaid diagrams, follow this two-step process: (1) Declare first the nodes: nodeID['Node Label']. (2) Then define the links: nodeID1 -- 'link text' --> nodeID2 ") {%- endif %} {%- if enable_semantic_files_types %} pr_files: List[FileDescription] = Field(max_items=20, description="a list of all the files that were changed in the PR, and summary of their changes. Each file must be analyzed regardless of change size.") @@ -68,7 +68,8 @@ title: | {%- if enable_pr_diagram %} changes_diagram: | ```mermaid - ... + flowchart LR + ... ``` {%- endif %} {%- if enable_semantic_files_types %} @@ -155,7 +156,8 @@ title: | {%- if enable_pr_diagram %} changes_diagram: | ```mermaid - ... + flowchart LR + ... ``` {%- endif %} {%- if enable_semantic_files_types %} From 16d980ec763082621ba0bfcee308705c20696e11 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Sun, 25 May 2025 15:38:08 +0300 Subject: [PATCH 46/46] fix: update Mermaid diagram instructions for clarity and consistency --- pr_agent/settings/pr_description_prompts.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pr_agent/settings/pr_description_prompts.toml b/pr_agent/settings/pr_description_prompts.toml index 81cb0afb..4c14abee 100644 --- a/pr_agent/settings/pr_description_prompts.toml +++ b/pr_agent/settings/pr_description_prompts.toml @@ -47,7 +47,7 @@ class PRDescription(BaseModel): description: str = Field(description="summarize the PR changes in up to four bullet points, each up to 8 words. For large PRs, add sub-bullets if needed. Order bullets by importance, with each bullet highlighting a key change group.") title: str = Field(description="a concise and descriptive title that captures the PR's main theme") {%- if enable_pr_diagram %} - changes_diagram: str = Field(description="a horizontal diagram that represents the main PR changes, in the format of a valid mermaid LR flowchart. The diagram should be concise and easy to read. Leave empty if no diagram is relevant. To create robust Mermaid diagrams, follow this two-step process: (1) Declare first the nodes: nodeID['Node Label']. (2) Then define the links: nodeID1 -- 'link text' --> nodeID2 ") + changes_diagram: str = Field(description="a horizontal diagram that represents the main PR changes, in the format of a valid mermaid LR flowchart. The diagram should be concise and easy to read. Leave empty if no diagram is relevant. To create robust Mermaid diagrams, follow this two-step process: (1) Declare the nodes: nodeID["node description"]. (2) Then define the links: nodeID1 -- "link text" --> nodeID2 ") {%- endif %} {%- if enable_semantic_files_types %} pr_files: List[FileDescription] = Field(max_items=20, description="a list of all the files that were changed in the PR, and summary of their changes. Each file must be analyzed regardless of change size.")