From 22ee08a27282ddb879a2ea443dee88209b6c1474 Mon Sep 17 00:00:00 2001 From: Utsab Dahal <134686066+utsab345@users.noreply.github.com> Date: Fri, 25 Sep 2026 00:05:05 +0545 Subject: [PATCH 1/6] fix(gitlab_webhook): degrade to shared secret when the secret provider fails (#3662) Co-authored-by: Ismael Martinez Ramos Co-authored-by: Claude Opus 5.5 (1M context) --- pr_agent/servers/gitlab_webhook.py | 23 ++--- .../test_gitlab_webhook_secret_provider.py | 88 +++++++++++++++++++ 2 files changed, 100 insertions(+), 11 deletions(-) diff --git a/pr_agent/servers/gitlab_webhook.py b/pr_agent/servers/gitlab_webhook.py index a741595d2..183c3e96d 100644 --- a/pr_agent/servers/gitlab_webhook.py +++ b/pr_agent/servers/gitlab_webhook.py @@ -252,21 +252,22 @@ def should_process_pr_logic(data) -> bool: def authenticate_gitlab_webhook(request: Request, log_context: dict): request_token = request.headers.get("X-Gitlab-Token") # Built only for a request that will actually consult it, so a cloud client that - # fails to initialize cannot drop webhooks authenticated by shared secret instead. - secret_provider = get_fork_safe_secret_provider() if request_token else None - if request_token and secret_provider: - secret = secret_provider.get_secret(request_token) - if not secret: - get_logger().warning("Empty secret retrieved for the provided webhook token") - return JSONResponse(status_code=status.HTTP_401_UNAUTHORIZED, - content=jsonable_encoder({"message": "unauthorized"})) + # fails to initialize or read cannot drop webhooks authenticated by shared secret instead. + secret = None + if request_token: + try: + secret_provider = get_fork_safe_secret_provider() + secret = secret_provider.get_secret(request_token) if secret_provider else None + except Exception as e: + get_logger().warning(f"Secret provider failed ({type(e).__name__}), falling back to the shared secret") + if secret: try: secret_dict = json.loads(secret) - gitlab_token = secret_dict["gitlab_token"] + context["settings"].gitlab.personal_access_token = secret_dict["gitlab_token"] log_context["token_id"] = secret_dict.get("token_name", secret_dict.get("id", "unknown")) - context["settings"].gitlab.personal_access_token = gitlab_token except Exception as e: - get_logger().error(f"Failed to validate the secret for the provided webhook token: {e}") + get_logger().error( + f"Failed to validate the secret for the provided webhook token: {type(e).__name__}") return JSONResponse(status_code=status.HTTP_401_UNAUTHORIZED, content=jsonable_encoder({"message": "unauthorized"})) elif get_settings().get("GITLAB.SHARED_SECRET"): diff --git a/tests/unittest/test_gitlab_webhook_secret_provider.py b/tests/unittest/test_gitlab_webhook_secret_provider.py index 3c033deb1..f740125d5 100644 --- a/tests/unittest/test_gitlab_webhook_secret_provider.py +++ b/tests/unittest/test_gitlab_webhook_secret_provider.py @@ -1,3 +1,4 @@ +import json import os import httpx @@ -10,6 +11,15 @@ class FakeSecretProvider: """Stands in for a cloud secret client, which must not be shared across a fork.""" + def __init__(self, secret=None, error=None): + self.secret = secret + self.error = error + + def get_secret(self, token): + if self.error is not None: + raise RuntimeError(self.error) + return self.secret + @pytest.fixture(autouse=True) def clean_state(): @@ -154,3 +164,81 @@ async def test_keep_the_webhook_token_out_of_the_logs(gitlab_webhook_settings): assert records, "nothing was logged, so the assertion below would be vacuous" assert not any(secret_token in record for record in records) + + +@pytest.mark.asyncio +async def test_accept_a_webhook_token_resolved_by_the_secret_provider(monkeypatch, gitlab_webhook_settings): + """Accept a delivery whose token resolves through the cloud secret provider even when it + does not match the configured shared secret.""" + secret = json.dumps({"gitlab_token": "glpat-provider", "token_name": "webhook-1"}) + monkeypatch.setattr(gitlab_webhook, "get_secret_provider", lambda: FakeSecretProvider(secret=secret)) + + assert (await _post_webhook("provider-token")).status_code == 200 + + +@pytest.mark.asyncio +async def test_fall_back_to_shared_secret_when_provider_initialization_fails(monkeypatch, gitlab_webhook_settings): + """Fall back to the shared secret when the cloud client cannot be built.""" + monkeypatch.setattr(gitlab_webhook, "get_secret_provider", + lambda: (_ for _ in ()).throw(RuntimeError("secrets manager unreachable"))) + + assert (await _post_webhook("topsecret")).status_code == 200 + assert (await _post_webhook("wrong-secret")).status_code == 401 + + +@pytest.mark.asyncio +async def test_fall_back_to_shared_secret_when_the_secret_read_fails(monkeypatch, gitlab_webhook_settings): + """Fall back to the shared secret when the provider read raises.""" + monkeypatch.setattr( + gitlab_webhook, "get_secret_provider", + lambda: FakeSecretProvider(error="secrets manager read failed")) + + assert (await _post_webhook("topsecret")).status_code == 200 + assert (await _post_webhook("wrong-secret")).status_code == 401 + + +@pytest.mark.asyncio +async def test_fall_back_to_shared_secret_when_the_provider_lookup_is_empty(monkeypatch, gitlab_webhook_settings): + """Fall back to the shared secret when the provider answers an empty lookup, as the + built-in secrets clients do during a read outage.""" + monkeypatch.setattr(gitlab_webhook, "get_secret_provider", lambda: FakeSecretProvider(secret="")) + + assert (await _post_webhook("topsecret")).status_code == 200 + assert (await _post_webhook("unseen-token")).status_code == 401 + + +@pytest.mark.asyncio +async def test_do_not_leak_provider_exception_details_in_the_fallback_warning(monkeypatch, gitlab_webhook_settings): + """Keep provider exception text out of the fallback warning, as the providers themselves + already redact it and the webhook logs are shipped to aggregators.""" + records = [] + handler_id = gitlab_webhook.get_logger().add(lambda m: records.append(str(m))) + try: + monkeypatch.setattr(gitlab_webhook, "get_secret_provider", + lambda: (_ for _ in ()).throw(RuntimeError("credential-process diagnostics"))) + assert (await _post_webhook("any-token")).status_code == 401 + finally: + gitlab_webhook.get_logger().remove(handler_id) + + assert records, "nothing was logged, so the assertion below would be vacuous" + assert not any("credential-process diagnostics" in record for record in records) + assert any("RuntimeError" in record and "falling back" in record for record in records) + + +@pytest.mark.asyncio +async def test_degrade_to_401_when_provider_fails_and_no_shared_secret(monkeypatch, gitlab_webhook_settings): + """Fail closed with 401 when the provider fails and no shared secret is configured.""" + settings = gitlab_webhook_settings + settings.set("GITLAB.SHARED_SECRET", "") + monkeypatch.setattr(gitlab_webhook, "get_secret_provider", + lambda: (_ for _ in ()).throw(RuntimeError("secrets manager unreachable"))) + + assert (await _post_webhook("any-token")).status_code == 401 + + +@pytest.mark.asyncio +async def test_reject_a_token_unknown_to_provider_and_shared_secret(monkeypatch, gitlab_webhook_settings): + """Reject a token that neither the provider nor the shared secret recognizes.""" + monkeypatch.setattr(gitlab_webhook, "get_secret_provider", lambda: FakeSecretProvider(secret="")) + + assert (await _post_webhook("unseen-token")).status_code == 401 From eaf5d97068b18cd30d50c9f8c238f5c69f327180 Mon Sep 17 00:00:00 2001 From: teachershuang <148217352+teachershuang@users.noreply.github.com> Date: Fri, 25 Sep 2026 03:15:09 +0800 Subject: [PATCH 2/6] docs: fix GitHub suggestion-count configuration examples (#3337) Signed-off-by: teachershuang --- docs/docs/installation/github.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/docs/installation/github.md b/docs/docs/installation/github.md index 98a1fc14a..f31c5c728 100644 --- a/docs/docs/installation/github.md +++ b/docs/docs/installation/github.md @@ -49,7 +49,7 @@ When you open your next PR, you should see a comment from `github-actions` bot w # ... previous environment values OPENAI.ORG: "" PR_REVIEWER.REQUIRE_TESTS_REVIEW: "false" # Disable tests review - PR_CODE_SUGGESTIONS.NUM_CODE_SUGGESTIONS: 6 # Increase number of code suggestions + PR_CODE_SUGGESTIONS.NUM_CODE_SUGGESTIONS_PER_CHUNK: 6 # Increase number of code suggestions ``` See detailed usage instructions in the [USAGE GUIDE](../usage-guide/automations_and_usage.md#github-action) @@ -380,7 +380,7 @@ Configure for specific programming languages: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Language-specific settings pr_reviewer.extra_instructions: "Focus on Python best practices, type hints, and docstrings." - pr_code_suggestions.num_code_suggestions: "8" + pr_code_suggestions.num_code_suggestions_per_chunk: "8" pr_code_suggestions.suggestions_score_threshold: "7" # Tool configuration github_action_config.auto_review: "true" @@ -459,7 +459,7 @@ fallback_models = ["anthropic/claude-opus-5"] extra_instructions = "Focus on security issues and code quality." [pr_code_suggestions] -num_code_suggestions = 6 +num_code_suggestions_per_chunk = 6 suggestions_score_threshold = 7 ``` From da625202ce840fda96a606c2dfb9d416745d3fb6 Mon Sep 17 00:00:00 2001 From: Tal <169621860+taljeon@users.noreply.github.com> Date: Fri, 25 Sep 2026 04:31:30 +0900 Subject: [PATCH 3/6] fix: classify retryable model fallback failures (#3631) Signed-off-by: taljeon <169621860+taljeon@users.noreply.github.com> Co-authored-by: Ismael Martinez Ramos Co-authored-by: Claude Opus 5.5 (1M context) --- pr_agent/algo/pr_processing.py | 15 ++- pr_agent/algo/token_budget.py | 12 +- pr_agent/tools/pr_add_docs.py | 5 +- pr_agent/tools/pr_code_suggestions.py | 5 +- pr_agent/tools/pr_description.py | 11 +- pr_agent/tools/pr_generate_labels.py | 14 ++- pr_agent/tools/pr_help_message.py | 8 +- pr_agent/tools/pr_questions.py | 5 +- pr_agent/tools/pr_reviewer.py | 9 +- pr_agent/tools/pr_update_changelog.py | 5 +- pr_agent/tools/ticket_pr_compliance_check.py | 4 +- tests/unittest/test_diff_attempt_budget.py | 2 +- tests/unittest/test_model_routing.py | 6 +- .../unittest/test_pr_code_suggestions_core.py | 2 +- tests/unittest/test_pr_description.py | 2 +- .../unittest/test_pr_help_message_fallback.py | 14 +-- .../test_pr_help_message_rendering.py | 4 +- .../test_pr_line_questions_context_budget.py | 2 +- .../test_retry_with_fallback_models.py | 103 ++++++++++++++++-- .../test_review_large_diff_chunking.py | 2 +- .../test_suggestion_chunk_recovery.py | 4 +- .../test_tool_output_reserve_forwarding.py | 31 +++++- 22 files changed, 200 insertions(+), 65 deletions(-) diff --git a/pr_agent/algo/pr_processing.py b/pr_agent/algo/pr_processing.py index 60de2ab91..f71f022a6 100644 --- a/pr_agent/algo/pr_processing.py +++ b/pr_agent/algo/pr_processing.py @@ -1,10 +1,13 @@ from __future__ import annotations +import asyncio import traceback from contextvars import ContextVar from dataclasses import dataclass, replace from typing import Callable, List, Tuple +import openai + from pr_agent.algo.git_patch_processing import ( decouple_and_convert_to_hunks_with_lines_numbers, extend_patch, @@ -13,7 +16,7 @@ from pr_agent.algo.language_handler import sort_files_by_main_languages from pr_agent.algo.model_routing import route_primary_model from pr_agent.algo.run_details import record_model_used -from pr_agent.algo.token_budget import AttemptTokenBudget, clip_tokens +from pr_agent.algo.token_budget import AttemptTokenBudget, FallbackEligibleError, clip_tokens from pr_agent.algo.token_handler import TokenHandler from pr_agent.algo.types import EDIT_TYPE from pr_agent.algo.utils import ModelType, get_model @@ -31,6 +34,7 @@ OUTPUT_BUFFER_TOKENS_HARD_THRESHOLD = 1000 MAX_EXTRA_LINES = 10 + _effective_fallback_chain: ContextVar[tuple[tuple[str, str | None], ...] | None] = ContextVar( "pr_agent_effective_fallback_chain", default=None ) @@ -667,6 +671,7 @@ async def retry_with_fallback_models(f: Callable, model_type: ModelType = ModelT effective_chain = tuple(zip(all_models, all_deployments[:len(all_models)], strict=True)) original_deployment_id = get_settings().get("openai.deployment_id", None) context_token = _effective_fallback_chain.set(effective_chain) + attempt_errors = [] try: # try each (model, deployment_id) pair until one is successful, otherwise raise exception for i, (model, deployment_id) in enumerate(effective_chain): @@ -678,12 +683,18 @@ async def retry_with_fallback_models(f: Callable, model_type: ModelType = ModelT get_settings().set("openai.deployment_id", deployment_id) result = await f(model) except Exception as e: + if not isinstance(e, (openai.APIError, asyncio.TimeoutError, FallbackEligibleError)): + raise + attempt_errors.append(f"{model}: {type(e).__name__}: {e}") get_logger().warning( f"Failed to generate prediction with {model}", artifact={"error": e}, ) if i == len(all_models) - 1: # If it's the last iteration - raise Exception(f"Failed to generate prediction with any model of {all_models}") from e + raise Exception( + f"Failed to generate prediction with any model of {all_models}. " + f"Attempts: {'; '.join(attempt_errors)}" + ) from e else: record_model_used(model, is_fallback=i > 0) return result diff --git a/pr_agent/algo/token_budget.py b/pr_agent/algo/token_budget.py index 1437c792e..da7aa8fca 100644 --- a/pr_agent/algo/token_budget.py +++ b/pr_agent/algo/token_budget.py @@ -16,6 +16,10 @@ DEFAULT_TRUNCATION_MARKER = "\n...(truncated)\n" +class FallbackEligibleError(ValueError): + """Represent a model-specific output or fit failure that another model may resolve.""" + + def _positive_int(value) -> int | None: if isinstance(value, int) and not isinstance(value, bool) and value > 0: return value @@ -388,7 +392,7 @@ def require_input_capacity( clamp=False, ) if available <= 0: - raise ValueError(f"The required prompt leaves no input capacity for {self.model}") + raise FallbackEligibleError(f"The required prompt leaves no input capacity for {self.model}") return available def count_tokens(self, text: str, *, force_accurate: bool = False) -> int: @@ -558,13 +562,13 @@ def prepare(candidate: str) -> FittedPrompt: empty_prompt = prepare("") if empty_prompt.input_tokens > input_limit: - raise ValueError(f"The required prompt exceeds the token limit for {self.model}") + raise FallbackEligibleError(f"The required prompt exceeds the token limit for {self.model}") if not optional_text: return empty_prompt marker_prompt = prepare(truncation_marker) if marker_prompt.input_tokens > input_limit: - raise ValueError( + raise FallbackEligibleError( f"The truncation marker does not fit the token limit for {self.model}" ) best_prompt = marker_prompt @@ -608,7 +612,7 @@ def retain(count: int) -> str: keep_tokens = min(keep_tokens - 1, scaled_keep) if best_prompt.input_tokens > input_limit: - raise ValueError(f"Failed to fit the optional prompt text for {self.model}") + raise FallbackEligibleError(f"Failed to fit the optional prompt text for {self.model}") return best_prompt def fit_prompt_variable( diff --git a/pr_agent/tools/pr_add_docs.py b/pr_agent/tools/pr_add_docs.py index 149e72e89..2c51147dd 100644 --- a/pr_agent/tools/pr_add_docs.py +++ b/pr_agent/tools/pr_add_docs.py @@ -7,6 +7,7 @@ from pr_agent.algo.ai_handlers.litellm_ai_handler import LiteLLMAIHandler from pr_agent.algo.pr_processing import ( OUTPUT_BUFFER_TOKENS_HARD_THRESHOLD, + FallbackEligibleError, get_pr_diff, retry_with_fallback_models, ) @@ -112,7 +113,7 @@ async def _prepare_prediction(self, model: str): output_token_reserve=output_token_reserve, ) if not patches_diff: - raise ValueError("No PR diff fits the /add_docs request") + raise FallbackEligibleError("No PR diff fits the /add_docs request") fitted = budget.fit_prompt_variable( variables, "diff", @@ -122,7 +123,7 @@ async def _prepare_prediction(self, model: str): preserve_minimum=True, ) if fitted.optional_text != patches_diff: - raise ValueError( + raise FallbackEligibleError( f"The complete packed documentation diff does not fit the token limit for {model}" ) self.patches_diff = fitted.optional_text diff --git a/pr_agent/tools/pr_code_suggestions.py b/pr_agent/tools/pr_code_suggestions.py index 0ab2da682..0fdeb7c1c 100644 --- a/pr_agent/tools/pr_code_suggestions.py +++ b/pr_agent/tools/pr_code_suggestions.py @@ -23,6 +23,7 @@ from pr_agent.algo.pr_processing import ( OUTPUT_BUFFER_TOKENS_HARD_THRESHOLD, OUTPUT_BUFFER_TOKENS_SOFT_THRESHOLD, + FallbackEligibleError, _get_all_models, add_ai_metadata_to_diff_files, get_effective_fallback_chain, @@ -917,7 +918,7 @@ def render(diff_no_line_numbers: str) -> tuple[str, str]: preserve_minimum=True, ) if fitted.optional_text != patches_diff_no_line_number: - raise ValueError( + raise FallbackEligibleError( f"The complete suggestion chunk does not fit the token limit for {model}" ) system_prompt, user_prompt = fitted.system_prompt, fitted.user_prompt @@ -2004,7 +2005,7 @@ async def prepare_prediction_main(self, model: str) -> dict: self.data = data else: get_logger().warning("Empty PR diff list") - raise ValueError(f"No PR diff fits the /improve request for {model}") + raise FallbackEligibleError(f"No PR diff fits the /improve request for {model}") return data async def convert_to_decoupled_with_line_numbers( diff --git a/pr_agent/tools/pr_description.py b/pr_agent/tools/pr_description.py index c9a5ba90e..a2f0c3f13 100644 --- a/pr_agent/tools/pr_description.py +++ b/pr_agent/tools/pr_description.py @@ -15,6 +15,7 @@ from pr_agent.algo.output_models import PRDescriptionAssembled from pr_agent.algo.pr_processing import ( OUTPUT_BUFFER_TOKENS_HARD_THRESHOLD, + FallbackEligibleError, get_pr_diff, get_pr_diff_multiple_patchs, retry_with_fallback_models, @@ -370,7 +371,7 @@ async def _prepare_prediction(self, model: str) -> None: else: get_logger().error(f"Error getting PR diff {self.pr_id}", artifact={"traceback": traceback.format_exc()}) - raise ValueError( + raise FallbackEligibleError( f"No PR diff fits the /describe request for {model}" ) else: @@ -449,7 +450,7 @@ async def _prepare_prediction(self, model: str) -> None: if isinstance(result, BaseException): raise result if not isinstance(result, str): - chunk_errors.append(ValueError(f"Description chunk {i + 1} returned no prediction")) + chunk_errors.append(FallbackEligibleError(f"Description chunk {i + 1} returned no prediction")) failed_files.extend(files_in_patch) get_logger().warning( f"Description chunk {i + 1} returned no prediction; retaining successful chunks", @@ -475,7 +476,7 @@ async def _prepare_prediction(self, model: str) -> None: prediction_files = prediction_files.removeprefix('pr_files:').strip() file_description_str_list.append(prediction_files) else: - chunk_errors.append(ValueError(f"Description chunk {i + 1} returned invalid YAML")) + chunk_errors.append(FallbackEligibleError(f"Description chunk {i + 1} returned invalid YAML")) failed_files.extend(files_in_patch) get_logger().warning( f"Failed to parse description chunk {i + 1}; retaining successful chunks", @@ -485,7 +486,7 @@ async def _prepare_prediction(self, model: str) -> None: self.description_failed_chunk_count = len(chunk_pairs) - len(file_description_str_list) self.description_failed_files = list(dict.fromkeys(failed_files)) if not file_description_str_list: - raise chunk_errors[0] if chunk_errors else ValueError("No description chunks were generated") + raise chunk_errors[0] if chunk_errors else FallbackEligibleError("No description chunks were generated") # generate files_walkthrough string, with proper token handling self.vars, token_handler_only_description_prompt = fit_related_tickets_to_prompt_budget( @@ -656,7 +657,7 @@ async def _get_prediction( preserve_minimum=True, ) if prompt != "pr_description_only_description_prompts" and fitted.optional_text != patches_diff: - raise ValueError( + raise FallbackEligibleError( f"The complete packed description diff does not fit the token limit for {model}" ) variables["diff"] = fitted.optional_text diff --git a/pr_agent/tools/pr_generate_labels.py b/pr_agent/tools/pr_generate_labels.py index 7c1aa27b7..75e9d1c28 100644 --- a/pr_agent/tools/pr_generate_labels.py +++ b/pr_agent/tools/pr_generate_labels.py @@ -2,11 +2,14 @@ from functools import partial from typing import List +from pydantic import ValidationError + from pr_agent.algo.ai_handlers.base_ai_handler import BaseAiHandler from pr_agent.algo.ai_handlers.litellm_ai_handler import LiteLLMAIHandler from pr_agent.algo.output_models import Labels from pr_agent.algo.pr_processing import ( OUTPUT_BUFFER_TOKENS_HARD_THRESHOLD, + FallbackEligibleError, get_pr_diff, retry_with_fallback_models, ) @@ -150,7 +153,7 @@ async def _prepare_prediction(self, model: str) -> None: output_token_reserve=output_token_reserve, ) if not patches_diff: - raise ValueError(f"No PR diff fits the /generate_labels request for {model}") + raise FallbackEligibleError(f"No PR diff fits the /generate_labels request for {model}") fitted = budget.fit_prompt_variable( variables, "diff", @@ -160,7 +163,7 @@ async def _prepare_prediction(self, model: str) -> None: preserve_minimum=True, ) if fitted.optional_text != patches_diff: - raise ValueError( + raise FallbackEligibleError( f"The complete packed labels diff does not fit the token limit for {model}" ) variables["diff"] = fitted.optional_text @@ -201,7 +204,12 @@ def _prepare_data(self): def _load_valid_labels_yaml(prediction: str) -> dict: """Load a usable labels response or fail the current model attempt.""" data = load_yaml(prediction.strip()) - return Labels.model_validate(data).model_dump() + try: + return Labels.model_validate(data).model_dump() + except ValidationError as error: + first_error = error.errors(include_input=False)[0] + field = ".".join(str(part) for part in first_error["loc"]) or "$" + raise FallbackEligibleError(f"Invalid labels model output at {field}: {first_error['msg']}") from error def _prepare_labels(self) -> List[str]: pr_types = self.data["labels"].copy() diff --git a/pr_agent/tools/pr_help_message.py b/pr_agent/tools/pr_help_message.py index 8f5d02081..837a8ffd3 100644 --- a/pr_agent/tools/pr_help_message.py +++ b/pr_agent/tools/pr_help_message.py @@ -11,7 +11,7 @@ from pr_agent.algo.ai_handlers.base_ai_handler import BaseAiHandler from pr_agent.algo.ai_handlers.litellm_ai_handler import LiteLLMAIHandler -from pr_agent.algo.pr_processing import retry_with_fallback_models +from pr_agent.algo.pr_processing import FallbackEligibleError, retry_with_fallback_models from pr_agent.algo.token_budget import get_max_tokens from pr_agent.algo.token_handler import TokenEncoder from pr_agent.algo.utils import ModelType, load_yaml @@ -287,11 +287,13 @@ def render(snippets): empty_prompts = render("") if self._count_prompt_tokens(model, *empty_prompts) > prompt_budget: - raise ValueError(f"The /help prompt exceeds the token limit for {model} without documentation") + raise FallbackEligibleError(f"The /help prompt exceeds the token limit for {model} without documentation") marker_prompts = render(TRUNCATION_MARKER) if self._count_prompt_tokens(model, *marker_prompts) > prompt_budget: - raise ValueError(f"The /help prompt exceeds the token limit for {model} with a truncation marker") + raise FallbackEligibleError( + f"The /help prompt exceeds the token limit for {model} with a truncation marker" + ) keep_chars = max(len(raw_snippets) - 1, 0) while keep_chars > 0: diff --git a/pr_agent/tools/pr_questions.py b/pr_agent/tools/pr_questions.py index f4bbfbdca..a3ce9b775 100644 --- a/pr_agent/tools/pr_questions.py +++ b/pr_agent/tools/pr_questions.py @@ -7,6 +7,7 @@ from pr_agent.algo.pr_processing import ( OUTPUT_BUFFER_TOKENS_HARD_THRESHOLD, OUTPUT_BUFFER_TOKENS_SOFT_THRESHOLD, + FallbackEligibleError, get_pr_diff, retry_with_fallback_models, ) @@ -195,7 +196,7 @@ async def _prepare_prediction(self, model: str): output_token_reserve=output_token_reserve, ) if not patches_diff: - raise ValueError(f"No PR diff fits the /ask request for {model}") + raise FallbackEligibleError(f"No PR diff fits the /ask request for {model}") fitted = budget.fit_prompt_variable( variables, @@ -207,7 +208,7 @@ async def _prepare_prediction(self, model: str): image_path=image_path, ) if fitted.optional_text != patches_diff: - raise ValueError( + raise FallbackEligibleError( f"The complete packed question diff does not fit the token limit for {model}" ) self.patches_diff = fitted.optional_text diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index dd949ff20..fae021b12 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -29,6 +29,7 @@ from pr_agent.algo.pr_processing import ( OUTPUT_BUFFER_TOKENS_HARD_THRESHOLD, OUTPUT_BUFFER_TOKENS_SOFT_THRESHOLD, + FallbackEligibleError, PreparedPRDiff, add_ai_metadata_to_diff_files, get_pr_diff, @@ -850,7 +851,7 @@ async def _prepare_prediction(self, model: str) -> None: self.prediction = prediction else: get_logger().warning(f"Empty diff for PR: {self.pr_url}") - raise ValueError(f"No PR diff fits the /review request for {model}") + raise FallbackEligibleError(f"No PR diff fits the /review request for {model}") async def _prepare_chunked_prediction(self, model: str, prepared_diff: PreparedPRDiff | None = None) -> bool: @@ -920,7 +921,7 @@ async def _prepare_chunked_prediction(self, model: str, if len(chunk_results) < len(patches_diff_list): if chunk_errors: raise chunk_errors[0] - raise ValueError("No valid review output was produced for one or more chunks") + raise FallbackEligibleError("No valid review output was produced for one or more chunks") return self._merge_cached_review_chunks() @@ -1047,7 +1048,7 @@ async def _get_prediction(self, model: str, patches_diff: Optional[str] = None) preserve_minimum=True, ) if fitted.optional_text != patches_diff: - raise ValueError( + raise FallbackEligibleError( f"The complete packed review diff does not fit the token limit for {model}" ) @@ -1116,7 +1117,7 @@ def _load_valid_review_yaml(cls, prediction: str, *, source: str = "model respon """Parse one prediction and require the minimum publishable review shape.""" data = cls._load_review_yaml(prediction) if not isinstance(data, dict) or not isinstance(data.get("review"), dict) or not data["review"]: - raise ValueError(f"{source} did not contain a non-empty review mapping") + raise FallbackEligibleError(f"{source} did not contain a non-empty review mapping") return data def _prepare_pr_review(self) -> str: diff --git a/pr_agent/tools/pr_update_changelog.py b/pr_agent/tools/pr_update_changelog.py index 2a4ed6575..d95b92704 100644 --- a/pr_agent/tools/pr_update_changelog.py +++ b/pr_agent/tools/pr_update_changelog.py @@ -10,6 +10,7 @@ from pr_agent.algo.pr_processing import ( OUTPUT_BUFFER_TOKENS_HARD_THRESHOLD, OUTPUT_BUFFER_TOKENS_SOFT_THRESHOLD, + FallbackEligibleError, get_pr_diff, retry_with_fallback_models, ) @@ -223,7 +224,7 @@ async def _prepare_prediction(self, model: str): output_token_reserve=output_token_reserve, ) if not patches_diff: - raise ValueError(f"No PR diff fits the /update_changelog request for {model}") + raise FallbackEligibleError(f"No PR diff fits the /update_changelog request for {model}") fitted = budget.fit_prompt_variable( variables, @@ -234,7 +235,7 @@ async def _prepare_prediction(self, model: str): preserve_minimum=True, ) if fitted.optional_text != patches_diff: - raise ValueError( + raise FallbackEligibleError( f"The complete packed changelog diff does not fit the token limit for {model}" ) self.patches_diff = fitted.optional_text diff --git a/pr_agent/tools/ticket_pr_compliance_check.py b/pr_agent/tools/ticket_pr_compliance_check.py index 690757374..448f498fb 100644 --- a/pr_agent/tools/ticket_pr_compliance_check.py +++ b/pr_agent/tools/ticket_pr_compliance_check.py @@ -9,7 +9,7 @@ from atlassian import Jira from pr_agent.algo.pr_processing import OUTPUT_BUFFER_TOKENS_SOFT_THRESHOLD -from pr_agent.algo.token_budget import AttemptTokenBudget +from pr_agent.algo.token_budget import AttemptTokenBudget, FallbackEligibleError from pr_agent.algo.token_handler import TokenHandler from pr_agent.config_loader import get_settings from pr_agent.git_providers.git_provider import GitProvider @@ -450,7 +450,7 @@ def fit_related_tickets_to_prompt_budget( lower_bound = prefix_size + 1 if best_vars is None or best_budget is None: - raise ValueError("Related-ticket omission marker exceeds the prompt token budget") + raise FallbackEligibleError("Related-ticket omission marker exceeds the prompt token budget") prompt_vars = best_vars included_tickets = len(prompt_vars["related_tickets"]) diff --git a/tests/unittest/test_diff_attempt_budget.py b/tests/unittest/test_diff_attempt_budget.py index f1cb6ac5f..2bfde9733 100644 --- a/tests/unittest/test_diff_attempt_budget.py +++ b/tests/unittest/test_diff_attempt_budget.py @@ -218,7 +218,7 @@ async def attempt(model): pr_processing.get_pr_diff(Provider(), source, model) attempts.append((model, {tag for tag, text in events})) if model != "fallback-model": - raise RuntimeError("try fallback") + raise pr_processing.FallbackEligibleError("try fallback") return "ok" result = await pr_processing.retry_with_fallback_models( diff --git a/tests/unittest/test_model_routing.py b/tests/unittest/test_model_routing.py index e5026bed2..49d298c3c 100644 --- a/tests/unittest/test_model_routing.py +++ b/tests/unittest/test_model_routing.py @@ -7,7 +7,7 @@ import pytest from pr_agent.algo.model_routing import count_hunks, route_primary_model -from pr_agent.algo.pr_processing import retry_with_fallback_models +from pr_agent.algo.pr_processing import FallbackEligibleError, retry_with_fallback_models from pr_agent.algo.run_details import get_run_details, init_run_details from pr_agent.algo.types import FilePatchInfo from pr_agent.algo.utils import ModelType @@ -71,7 +71,7 @@ def _models_tried(git_provider, model_type=ModelType.REGULAR, fail=()): async def fake_f(model): calls.append(model) if model in fail: - raise RuntimeError(f"{model} failed") + raise FallbackEligibleError(f"{model} failed") return model asyncio.run(retry_with_fallback_models(fake_f, model_type=model_type, git_provider=git_provider)) @@ -181,7 +181,7 @@ def test_fallback_deployments_stay_paired_with_their_models(self, settings): async def fake_f(model): observed.append((model, get_settings().get("openai.deployment_id"))) if model == "tiny-model": - raise RuntimeError("tiny failed") + raise FallbackEligibleError("tiny failed") return model asyncio.run(retry_with_fallback_models(fake_f, git_provider=_pr(num_files=1, hunks_per_file=1))) diff --git a/tests/unittest/test_pr_code_suggestions_core.py b/tests/unittest/test_pr_code_suggestions_core.py index 3ea064b6e..7bf78e5d7 100644 --- a/tests/unittest/test_pr_code_suggestions_core.py +++ b/tests/unittest/test_pr_code_suggestions_core.py @@ -683,7 +683,7 @@ async def test_prepare_prediction_main_keeps_outer_fallback_when_all_chunks_fail async def fake_get_prediction(model, patches_diff, patches_diff_no_line_numbers): attempted.append((model, patches_diff)) if model == "primary-model": - raise RuntimeError(f"{patches_diff} failed") + raise TimeoutError(f"{patches_diff} failed") return {"code_suggestions": [_valid_suggestion(relevant_file=f"{patches_diff}.py")]} try: diff --git a/tests/unittest/test_pr_description.py b/tests/unittest/test_pr_description.py index 99f279eb8..844832c71 100644 --- a/tests/unittest/test_pr_description.py +++ b/tests/unittest/test_pr_description.py @@ -947,7 +947,7 @@ async def test_large_pr_all_fail_then_fallback_model_keeps_partial_result(self, async def mock_get_prediction(model, patches_diff, prompt="pr_description_prompt"): calls.append((model, prompt, patches_diff)) if model == "gpt-4o": - raise RuntimeError("primary chunk failed") + raise TimeoutError("primary chunk failed") if prompt == "pr_description_only_description_prompts": return _header_prediction() if "file1" in patches_diff: diff --git a/tests/unittest/test_pr_help_message_fallback.py b/tests/unittest/test_pr_help_message_fallback.py index 95647eb81..77cae1515 100644 --- a/tests/unittest/test_pr_help_message_fallback.py +++ b/tests/unittest/test_pr_help_message_fallback.py @@ -88,7 +88,7 @@ async def test_model_prompt_preserves_question_and_documentation_markup(help_too async def test_primary_failure_uses_backup_answer(help_tool): tool, details, _ = help_tool - tool.ai_handler.chat_completion.side_effect = [RuntimeError("primary unavailable"), (ANSWER, "stop")] + tool.ai_handler.chat_completion.side_effect = [TimeoutError("primary unavailable"), (ANSWER, "stop")] await tool.run() @@ -126,7 +126,7 @@ def model_token_count(*, model, messages): monkeypatch.setattr(pr_help_message, "get_max_tokens", model_limit) monkeypatch.setattr(pr_help_message, "token_counter", model_token_count) - tool.ai_handler.chat_completion.side_effect = [RuntimeError("primary unavailable"), (ANSWER, "stop")] + tool.ai_handler.chat_completion.side_effect = [TimeoutError("primary unavailable"), (ANSWER, "stop")] await tool.run() @@ -166,7 +166,7 @@ async def test_fallback_only_publishes_sources_from_complete_fitted_documents(he monkeypatch.setattr(tool, "_get_prompt_budget", lambda model: limits[model]) monkeypatch.setattr(pr_help_message, "token_counter", count_message_characters) tool.ai_handler.chat_completion.side_effect = [ - RuntimeError("primary unavailable"), + TimeoutError("primary unavailable"), ( "response: Use the documented review settings.\n" "relevant_sections:\n" @@ -419,7 +419,7 @@ def encode(text, disallowed_special=()): monkeypatch.setattr(pr_help_message, "token_counter", Mock(side_effect=RuntimeError("counter unavailable"))) monkeypatch.setattr(pr_help_message.TokenEncoder, "get_token_encoder", lambda _model: CharacterEncoder()) tool._prepare_prediction = AsyncMock(wraps=tool._prepare_prediction) - tool.ai_handler.chat_completion.side_effect = [RuntimeError("primary unavailable"), (ANSWER, "stop")] + tool.ai_handler.chat_completion.side_effect = [TimeoutError("primary unavailable"), (ANSWER, "stop")] await tool.run() @@ -527,7 +527,7 @@ async def test_fixed_prompt_overhead_skips_model_call_and_tries_larger_fallback( async def test_all_models_fail_without_publishing_no_information(help_tool): tool, details, logger = help_tool - tool.ai_handler.chat_completion.side_effect = RuntimeError("provider unavailable") + tool.ai_handler.chat_completion.side_effect = TimeoutError("provider unavailable") assert await tool.run() == "" @@ -578,10 +578,10 @@ async def test_invalid_prompt_reaches_final_failure_boundary(help_tool): assert await tool.run() == "" - assert [call.args[0] for call in tool._prepare_prediction.await_args_list] == [PRIMARY, BACKUP] + assert [call.args[0] for call in tool._prepare_prediction.await_args_list] == [PRIMARY] tool.ai_handler.chat_completion.assert_not_called() tool.git_provider.publish_comment.assert_not_called() logger.exception.assert_called_once() - assert "Failed to generate prediction with any model" in logger.exception.call_args.args[0] + assert "missing_variable" in logger.exception.call_args.args[0] assert details.model_used is None assert get_settings().get("openai.deployment_id") == "primary-deployment" diff --git a/tests/unittest/test_pr_help_message_rendering.py b/tests/unittest/test_pr_help_message_rendering.py index 0ac557d47..1f3791a13 100644 --- a/tests/unittest/test_pr_help_message_rendering.py +++ b/tests/unittest/test_pr_help_message_rendering.py @@ -153,9 +153,9 @@ async def test_question_uses_configured_handler_error_path_without_openai_key( tool = build_question_tool(tmp_path, monkeypatch, handler) if propagate_tool_errors: - with pytest.raises(Exception, match="Failed to generate prediction with any model") as exc_info: + with pytest.raises(RuntimeError) as exc_info: await tool.run() - assert exc_info.value.__cause__ is handler.error + assert exc_info.value is handler.error else: assert await tool.run() == "" diff --git a/tests/unittest/test_pr_line_questions_context_budget.py b/tests/unittest/test_pr_line_questions_context_budget.py index 118e4c3d1..fc660d79b 100644 --- a/tests/unittest/test_pr_line_questions_context_budget.py +++ b/tests/unittest/test_pr_line_questions_context_budget.py @@ -39,7 +39,7 @@ def __init__(self, fail_models=()): async def chat_completion(self, *, model, temperature, system, user): self.requests.append({"model": model, "system": system, "user": user}) if model in self.fail_models: - raise RuntimeError(f"simulated failure for {model}") + raise TimeoutError(f"simulated failure for {model}") return "answer", "stop" diff --git a/tests/unittest/test_retry_with_fallback_models.py b/tests/unittest/test_retry_with_fallback_models.py index d4064e692..c4b96e812 100644 --- a/tests/unittest/test_retry_with_fallback_models.py +++ b/tests/unittest/test_retry_with_fallback_models.py @@ -1,8 +1,11 @@ import asyncio +import httpx +import openai import pytest +from jinja2 import UndefinedError -from pr_agent.algo.pr_processing import retry_with_fallback_models +from pr_agent.algo.pr_processing import FallbackEligibleError, retry_with_fallback_models from pr_agent.algo.run_details import get_run_details, init_run_details from pr_agent.algo.utils import ModelType from pr_agent.config_loader import get_settings @@ -61,7 +64,7 @@ def test_primary_fails_fallback_succeeds(): async def fake_f(model): calls.append(model) if model == "primary-model": - raise RuntimeError("primary failed") + raise FallbackEligibleError("primary failed") return f"ok:{model}" result = asyncio.run(retry_with_fallback_models(fake_f)) @@ -80,14 +83,14 @@ def test_all_models_fail_raises_with_aggregate_message_and_cause(): get_settings().set("openai.deployment_id", None) get_settings().set("openai.fallback_deployments", []) - last_error = ValueError("last failure") + last_error = FallbackEligibleError("last failure") attempted = [] async def fake_f(model): attempted.append(model) if model == "fallback-1": raise last_error - raise RuntimeError("primary failure") + raise FallbackEligibleError("primary failure") with pytest.raises(Exception) as exc_info: asyncio.run(retry_with_fallback_models(fake_f)) @@ -118,7 +121,7 @@ async def fake_f(model): (model, get_settings().get("openai.deployment_id", None)) ) if model != "fallback-1": - raise RuntimeError(f"fail for {model}") + raise FallbackEligibleError(f"fail for {model}") return "fallback-ok" result = asyncio.run(retry_with_fallback_models(fake_f)) @@ -145,7 +148,7 @@ def test_fallback_deployment_does_not_poison_the_next_retry(): async def fake_f(model): observed.append((model, get_settings().get("openai.deployment_id", None))) if model == "primary-model": - raise RuntimeError("primary failed") + raise FallbackEligibleError("primary failed") return "fallback-ok" assert asyncio.run(retry_with_fallback_models(fake_f)) == "fallback-ok" @@ -172,7 +175,7 @@ def test_deployment_id_is_restored_when_retry_is_cancelled(): async def fake_f(model): if model == "primary-model": - raise RuntimeError("primary failed") + raise FallbackEligibleError("primary failed") raise asyncio.CancelledError with pytest.raises(asyncio.CancelledError): @@ -284,7 +287,7 @@ def test_records_fallback_model_with_fallback_flag(): async def fake_f(model): if model == "primary-model": - raise RuntimeError("primary failed") + raise FallbackEligibleError("primary failed") return "ok" asyncio.run(retry_with_fallback_models(fake_f)) @@ -311,7 +314,7 @@ def test_fallback_flag_set_even_when_fallback_repeats_primary_model_name(): async def fake_f(model): attempts.append(model) if len(attempts) == 1: - raise RuntimeError("first attempt failed") + raise FallbackEligibleError("first attempt failed") return "ok" asyncio.run(retry_with_fallback_models(fake_f)) @@ -349,3 +352,85 @@ def boom(*_args, **_kwargs): assert calls == ["primary-model"] finally: _restore_settings(snapshot) + + +@pytest.mark.parametrize("error", [ + TypeError("bad local type"), + KeyError("missing local key"), + UndefinedError("missing prompt variable"), + ValueError("unclassified local value"), + RuntimeError("unclassified local runtime"), +]) +def test_local_errors_propagate_without_billing_fallback(error): + snapshot = _snapshot_settings() + try: + get_settings().set("config.model", "primary-model") + get_settings().set("config.fallback_models", ["fallback-1"]) + get_settings().set("openai.deployment_id", "deployment-primary") + get_settings().set("openai.fallback_deployments", ["deployment-fallback"]) + calls = [] + + async def fake_f(model): + calls.append(model) + raise error + + with pytest.raises(type(error)) as exc_info: + asyncio.run(retry_with_fallback_models(fake_f)) + + assert exc_info.value is error + assert calls == ["primary-model"] + assert get_settings().get("openai.deployment_id") == "deployment-primary" + finally: + _restore_settings(snapshot) + + +@pytest.mark.parametrize("error", [ + openai.APIError("provider failed", request=httpx.Request("POST", "https://example.invalid"), body=None), + asyncio.TimeoutError("provider timed out"), +]) +def test_provider_errors_and_timeouts_remain_fallback_eligible(error): + snapshot = _snapshot_settings() + try: + get_settings().set("config.model", "primary-model") + get_settings().set("config.fallback_models", ["fallback-1"]) + get_settings().set("openai.deployment_id", None) + get_settings().set("openai.fallback_deployments", []) + calls = [] + + async def fake_f(model): + calls.append(model) + if model == "primary-model": + raise error + return "fallback-ok" + + assert asyncio.run(retry_with_fallback_models(fake_f)) == "fallback-ok" + assert calls == ["primary-model", "fallback-1"] + finally: + _restore_settings(snapshot) + + +def test_typed_model_fit_failures_report_each_attempt_when_exhausted(): + snapshot = _snapshot_settings() + try: + get_settings().set("config.model", "primary-model") + get_settings().set("config.fallback_models", ["fallback-1"]) + get_settings().set("openai.deployment_id", None) + get_settings().set("openai.fallback_deployments", []) + last_error = FallbackEligibleError("no non-empty review mapping") + calls = [] + + async def fake_f(model): + calls.append(model) + if model == "primary-model": + raise FallbackEligibleError("No PR diff fits") + raise last_error + + with pytest.raises(Exception, match="Failed to generate prediction with any model") as exc_info: + asyncio.run(retry_with_fallback_models(fake_f)) + + assert calls == ["primary-model", "fallback-1"] + assert "primary-model: FallbackEligibleError: No PR diff fits" in str(exc_info.value) + assert "fallback-1: FallbackEligibleError: no non-empty review mapping" in str(exc_info.value) + assert exc_info.value.__cause__ is last_error + finally: + _restore_settings(snapshot) diff --git a/tests/unittest/test_review_large_diff_chunking.py b/tests/unittest/test_review_large_diff_chunking.py index 87ad227d0..510673029 100644 --- a/tests/unittest/test_review_large_diff_chunking.py +++ b/tests/unittest/test_review_large_diff_chunking.py @@ -400,7 +400,7 @@ async def test_exhausted_fallbacks_propagate_partial_review_failure_when_configu reviewer.git_provider.supports_review_comment_identity.return_value = False chunk_c = CHUNK_A.replace("a.py", "c.py").replace("the index is never checked", "the value is never checked") reviewer._get_prediction = AsyncMock(side_effect=[ - CHUNK_A, "review: {}", chunk_c, RuntimeError("context limit exceeded"), + CHUNK_A, "review: {}", chunk_c, TimeoutError("context limit exceeded"), ]) settings_values = { "config.model": "primary", diff --git a/tests/unittest/test_suggestion_chunk_recovery.py b/tests/unittest/test_suggestion_chunk_recovery.py index 5336c72e4..948b16190 100644 --- a/tests/unittest/test_suggestion_chunk_recovery.py +++ b/tests/unittest/test_suggestion_chunk_recovery.py @@ -193,7 +193,7 @@ def fit_optional_text(self, optional_text, *_args, **_kwargs): async def test_all_failed_primary_keeps_existing_outer_fallback(configured, monkeypatch): - tool, calls = make_tool(monkeypatch, {("gpt-4o", c): RuntimeError("failure") for c in "abc"}) + tool, calls = make_tool(monkeypatch, {("gpt-4o", c): TimeoutError("failure") for c in "abc"}) result = await retry_with_fallback_models(tool.prepare_prediction_main) assert [s["relevant_file"] for s in result["code_suggestions"]] == ["a.py", "b.py", "c.py"] assert [m for m, _, _, _ in calls] == ["gpt-4o"] * 3 + ["gpt-4o-mini"] * 3 @@ -220,7 +220,7 @@ def pack_for_model(_provider, _token_handler, model, **_kwargs): async def test_partial_success_on_outer_fallback_only_tries_later_models(configured, monkeypatch): - failures = {("gpt-4o", c): RuntimeError("failure") for c in "abc"} + failures = {("gpt-4o", c): TimeoutError("failure") for c in "abc"} failures[("gpt-4o-mini", "b")] = RuntimeError("secondary failed") tool, calls = make_tool(monkeypatch, failures) result = await retry_with_fallback_models(tool.prepare_prediction_main) diff --git a/tests/unittest/test_tool_output_reserve_forwarding.py b/tests/unittest/test_tool_output_reserve_forwarding.py index 53d96d769..96ab623d5 100644 --- a/tests/unittest/test_tool_output_reserve_forwarding.py +++ b/tests/unittest/test_tool_output_reserve_forwarding.py @@ -7,7 +7,8 @@ import pr_agent.tools.pr_generate_labels as generate_labels_module import pr_agent.tools.pr_questions as questions_module import pr_agent.tools.pr_update_changelog as update_changelog_module -from pr_agent.algo.pr_processing import retry_with_fallback_models +from pr_agent.algo.pr_processing import FallbackEligibleError, retry_with_fallback_models +from pr_agent.algo.token_budget import AttemptTokenBudget from pr_agent.config_loader import get_settings from tests.unittest._settings_helpers import restore_settings, snapshot_settings @@ -142,12 +143,24 @@ def fit_prompt_variable(self, _variables, _name, optional_text, **_kwargs): if tool_class is generate_labels_module.PRGenerateLabels: monkeypatch.setattr(tool_module, "set_custom_labels", lambda *_args: None) - with pytest.raises(ValueError, match="complete packed .* diff"): + with pytest.raises(FallbackEligibleError, match="complete packed .* diff"): await tool._prepare_prediction("fallback-model") tool._get_prediction.assert_not_awaited() +def test_no_input_capacity_is_model_specific(): + budget = AttemptTokenBudget( + "small-model", + object(), + SimpleNamespace(prompt_tokens=8), + context_window=10, + ) + + with pytest.raises(FallbackEligibleError, match="no input capacity"): + budget.require_input_capacity(3) + + @pytest.mark.asyncio async def test_add_docs_does_not_call_model_when_no_diff_fits(monkeypatch): tool = add_docs_module.PRAddDocs.__new__(add_docs_module.PRAddDocs) @@ -322,11 +335,13 @@ def fit_prompt_variable(self, _variables, _name, optional_text, **_kwargs): (update_changelog_module.PRUpdateChangelog, update_changelog_module), ], ) +@pytest.mark.parametrize("fit_mode", ["empty", "clipped"]) @pytest.mark.asyncio -async def test_empty_attempt_diff_advances_to_fallback_model( +async def test_model_specific_diff_failure_advances_to_fallback_model( monkeypatch, tool_class, tool_module, + fit_mode, ): settings_snapshot = snapshot_settings( ( @@ -354,24 +369,28 @@ async def test_empty_attempt_diff_advances_to_fallback_model( class FakeBudget: token_handler = object() + def __init__(self, model): + self.model = model + def require_input_capacity(self, *_args, **_kwargs): return 1 def fit_prompt_variable(self, _variables, _name, optional_text, **_kwargs): return SimpleNamespace( - optional_text=optional_text, + optional_text=optional_text[:-1] if fit_mode == "clipped" and self.model == "small-model" + else optional_text, system_prompt="system", user_prompt="user", ) def get_diff(_provider, _handler, model, **_kwargs): diff_models.append(model) - return "" if model == "small-model" else "diff" + return "" if fit_mode == "empty" and model == "small-model" else "diff" monkeypatch.setattr( tool_module.AttemptTokenBudget, "for_prompt_attempt", - lambda *_args, **_kwargs: FakeBudget(), + lambda model, *_args, **_kwargs: FakeBudget(model), ) monkeypatch.setattr(tool_module, "get_pr_diff", get_diff) From 02718aeec22324ff6f29c263e934fe4290cde6c5 Mon Sep 17 00:00:00 2001 From: Utsab Dahal <134686066+utsab345@users.noreply.github.com> Date: Fri, 25 Sep 2026 01:16:58 +0545 Subject: [PATCH 4/6] test(ai_handlers): add tripwire for guarded private LiteLLM imports (#3615) Co-authored-by: Ismael Martinez Ramos Co-authored-by: Claude Opus 5.5 (1M context) --- .../unittest/test_litellm_private_imports.py | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 tests/unittest/test_litellm_private_imports.py diff --git a/tests/unittest/test_litellm_private_imports.py b/tests/unittest/test_litellm_private_imports.py new file mode 100644 index 000000000..fba1a852f --- /dev/null +++ b/tests/unittest/test_litellm_private_imports.py @@ -0,0 +1,47 @@ +"""Tripwire tests for the guarded private LiteLLM imports. + +``cloud_auth`` and ``litellm_ai_handler`` import a few private LiteLLM symbols inside +``try: ... except ImportError: name = None`` blocks and only check them at call time. A +LiteLLM release can move or delete a private symbol without touching the proxy HTTP API +(e.g. ``litellm.utils.prompt_token_calculator`` was dropped in 1.100.0), so CI can stay +green while production raises on the first relevant call. These tests fail the moment any +guarded name resolves to ``None``. +""" + +import pytest + +import pr_agent.algo.ai_handlers.cloud_auth as cloud_auth +import pr_agent.algo.ai_handlers.litellm_ai_handler as litellm_handler + +# What breaks at runtime when each guarded LiteLLM symbol goes missing. +_CLOUD_AUTH_GUARDED = { + "AnthropicModelInfo": ( + "Anthropic API-key/auth-token resolution returns None and Claude calls lose their auth headers" + ), + "JSONProviderRegistry": ( + "openai-compatible provider registry (list_providers/get) is unavailable, breaking the JSON provider path" + ), + "_get_model_info_helper": ( + "model-info lookup trips the runtime guard RuntimeError, breaking model fallback and capability checks" + ), + "BedrockMantleAuthMixin": "Bedrock Mantle request signing is skipped and Bedrock calls lose their auth signature", +} + +_HANDLER_GUARDED = { + **_CLOUD_AUTH_GUARDED, + "MANTLE_HOST_RE": "Mantle endpoint detection is skipped, so the Mantle auth header is never applied", +} + + +@pytest.mark.parametrize("name", sorted(_CLOUD_AUTH_GUARDED)) +def test_cloud_auth_guarded_litellm_imports_are_present(name): + assert getattr(cloud_auth, name, None) is not None, ( + f"{name} is missing from cloud_auth; {_CLOUD_AUTH_GUARDED[name]}" + ) + + +@pytest.mark.parametrize("name", sorted(_HANDLER_GUARDED)) +def test_litellm_handler_guarded_litellm_imports_are_present(name): + assert getattr(litellm_handler, name, None) is not None, ( + f"{name} is missing from litellm_ai_handler; {_HANDLER_GUARDED[name]}" + ) From c5d1e599627d1183d4630ad1d9901d8ffc8e63da Mon Sep 17 00:00:00 2001 From: RAM PRASAD ATLURI <116977270+ATLURI0001@users.noreply.github.com> Date: Fri, 25 Sep 2026 11:11:47 +0530 Subject: [PATCH 5/6] Rename PR-Agent to DiffMind in README Updated README to reflect project name change from PR-Agent to DiffMind, including logo, descriptions, and usage instructions. --- README.md | 135 +++++++++++++++++++----------------------------------- 1 file changed, 48 insertions(+), 87 deletions(-) diff --git a/README.md b/README.md index b6b778d1b..1fd253e4f 100644 --- a/README.md +++ b/README.md @@ -1,55 +1,34 @@ - -
- - - - logo - + + + logo
-The Original Open-Source PR Reviewer +An Open-Source AI PR Reviewer

- -GitHub + +GitHub
--- - This repository contains the open-source PR Agent Project. - It is not the Qodo offering for open-source projects. - -PR-Agent is an open-source, AI-powered code review agent and a community-maintained legacy project of Qodo. It is distinct from Qodo's primary AI code review offering, which provides a feature-rich, context-aware experience. Qodo offers a free version for open-source projects and integrates seamlessly with GitHub, GitLab, Bitbucket, and Azure DevOps for high-quality automated reviews. +This repository contains the open-source DiffMind project. +DiffMind is an open-source, AI-powered code review agent that helps teams ship better pull requests faster. It plugs into your existing Git workflow and gives you automated, context-aware reviews without locking you into any single vendor. ## Sponsors -PR-Agent is a community-maintained open-source project, with its ongoing development supported by our sponsors. If you'd like to support the project, consider [becoming a sponsor](https://github.com/sponsors/naorpeled). - -

-

πŸ₯‡ Gold Sponsor

-

- -

- - Qodo β€” Gold sponsor - -

- -

- Free version of Qodo for open-source projects -

- +DiffMind is a community-maintained open-source project. If you'd like to support the project, consider [becoming a sponsor](https://github.com/sponsors/your-org). ## Table of Contents - [Getting Started](#getting-started) -- [Why Use PR-Agent?](#why-use-pr-agent) +- [Why Use DiffMind?](#why-use-diffmind) - [Features](#features) - [See It in Action](#see-it-in-action) - [How It Works](#how-it-works) @@ -59,68 +38,67 @@ PR-Agent is a community-maintained open-source project, with its ongoing develop ## Getting Started > [!NOTE] -> **Docker Hub namespace migration.** Releases `0.34.2` and later are published under [`pragent/pr-agent`](https://hub.docker.com/r/pragent/pr-agent). Older releases (up to and including `v0.31`) remain available at the legacy [`codiumai/pr-agent`](https://hub.docker.com/r/codiumai/pr-agent) namespace as a frozen archive β€” no new images are pushed there. Update any pinned `image:` / `docker pull` / `uses: docker://` references when upgrading to `0.34.2+`. +> **Docker Hub namespace.** Images are published under [`your-org/diffmind`](https://hub.docker.com/r/your-org/diffmind). Update any pinned `image:` / `docker pull` / `uses: docker://` references when upgrading. -### πŸš€ Quick Start for PR-Agent +### πŸš€ Quick Start for DiffMind #### 1. GitHub Action (Recommended) Add automated PR reviews to your repository with a simple workflow file: ```yaml -# .github/workflows/pr-agent.yml -name: PR Agent +# .github/workflows/diffmind.yml +name: DiffMind on: pull_request: types: [opened, synchronize] jobs: - pr_agent_job: + diffmind_job: runs-on: ubuntu-latest steps: - - name: PR Agent action step - uses: the-pr-agent/pr-agent@main + - name: DiffMind action step + uses: your-org/DiffMind@main env: OPENAI_KEY: ${{ secrets.OPENAI_KEY }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} ``` -[Full GitHub Action setup guide](https://docs.pr-agent.ai/installation/github/#run-as-a-github-action) +[Full GitHub Action setup guide](https://docs.diffmind.dev/installation/github/#run-as-a-github-action) #### 2. CLI Usage (Local Development) -Run PR-Agent locally on your repository: +Run DiffMind locally on your repository: ```bash -pip install pr-agent +pip install diffmind export OPENAI_KEY=your_key_here -pr-agent --pr_url https://github.com/owner/repo/pull/123 review +diffmind --pr_url https://github.com/owner/repo/pull/123 review ``` -[Complete CLI setup guide](https://docs.pr-agent.ai/usage-guide/automations_and_usage/#local-repo-cli) +[Complete CLI setup guide](https://docs.diffmind.dev/usage-guide/automations_and_usage/#local-repo-cli) #### 3. Other Platforms -- [GitLab webhook setup](https://docs.pr-agent.ai/installation/gitlab/) -- [BitBucket app installation](https://docs.pr-agent.ai/installation/bitbucket/) -- [Azure DevOps setup](https://docs.pr-agent.ai/installation/azure/) +- [GitLab webhook setup](https://docs.diffmind.dev/installation/gitlab/) +- [BitBucket app installation](https://docs.diffmind.dev/installation/bitbucket/) +- [Azure DevOps setup](https://docs.diffmind.dev/installation/azure/) ## News and Updates -Full notes for every release are on the [Releases page](https://github.com/the-pr-agent/pr-agent/releases). - +Full notes for every release are on the [Releases page](https://github.com/your-org/DiffMind/releases). -## Why Use PR-Agent? +## Why Use DiffMind? ### 🎯 Built for Real Development Teams **Fast & Affordable**: Each tool (`/review`, `/improve`, `/ask`) uses a single LLM call (~30 seconds, low cost) -**Handles Any PR Size**: Our [PR Compression strategy](https://docs.pr-agent.ai/core-abilities/#pr-compression-strategy) effectively processes both small and large PRs +**Handles Any PR Size**: Our PR compression strategy effectively processes both small and large PRs -**Highly Customizable**: JSON-based prompting allows easy customization of review categories and behavior via [configuration files](pr_agent/settings/configuration.toml) +**Highly Customizable**: JSON-based prompting allows easy customization of review categories and behavior via configuration files (`diffmind/settings/configuration.toml`) **Platform Agnostic**: - **Git Providers**: GitHub, GitLab, BitBucket, Azure DevOps, Gitea - **Deployment**: CLI, GitHub Actions, Docker, self-hosted, webhooks -- **AI Models**: OpenAI GPT, Anthropic Claude, Google Gemini, DeepSeek, Mistral, and any other model reachable through LiteLLM (Azure OpenAI, AWS Bedrock, Vertex AI, Databricks, OpenRouter, Ollama, and more) β€” see [Changing a model](https://docs.pr-agent.ai/usage-guide/changing_a_model/) +- **AI Models**: OpenAI GPT, Anthropic Claude, Google Gemini, DeepSeek, Mistral, and any other model reachable through LiteLLM (Azure OpenAI, AWS Bedrock, Vertex AI, Databricks, OpenRouter, Ollama, and more) β€” see [Changing a model](https://docs.diffmind.dev/usage-guide/changing_a_model/) **Open Source Benefits**: - Full control over your data and infrastructure @@ -132,39 +110,37 @@ Full notes for every release are on the [Releases page](https://github.com/the-p
-See the current [feature and git provider support matrix](https://docs.pr-agent.ai/#features) in the PR-Agent documentation. - -⚠️ `/help_docs` is temporarily disabled since `v0.36.1` pending a fix for a credential-exposure issue ([#2445](https://github.com/the-pr-agent/pr-agent/issues/2445)). +See the current feature and git-provider support matrix in the DiffMind documentation. -[//]: # (- Support for additional git providers is described in [here](./docs/Full_environments.md)) ___ ## See It in Action
-

/describe

+ +

/describe

- +

-

/review

+

/review

- +

-

/improve

+

/improve

- +

@@ -173,7 +149,7 @@ ___ ### Usage Examples -PR-Agent tools run as a comment on a PR or from the CLI. A few common ones: +DiffMind tools run as a comment on a PR or from the CLI. A few common ones: ```bash # Comment on a PR (GitHub/GitLab/Bitbucket/…): @@ -186,48 +162,33 @@ PR-Agent tools run as a comment on a PR or from the CLI. A few common ones: /similar_issue # find similar issues in the repository # Or locally via the CLI: -pr-agent --pr_url review -pr-agent --issue_url similar_issue +diffmind --pr_url review +diffmind --issue_url similar_issue ``` -See the [Tools docs](https://docs.pr-agent.ai/tools/#usage-examples) for the full list of tools with example commands, and each tool's page for screenshots and options. +See the Tools docs for the full list of tools with example commands, and each tool's page for screenshots and options.
## How It Works -The following diagram illustrates PR-Agent tools and their flow: +The following diagram illustrates DiffMind tools and their flow: -![PR-Agent Tools](https://www.qodo.ai/images/pr_agent/diagram-v0.9.png) +![DiffMind Tools](https://your-org.dev/images/diffmind/diagram-v0.1.png) ## Data Privacy -### Self-hosted PR-Agent +### Self-hosted DiffMind -- If you host PR-Agent with your OpenAI API key, it is between you and OpenAI. You can read their API data privacy policy here: +- If you host DiffMind with your own OpenAI API key, data handling is between you and OpenAI. You can read their API data privacy policy here: https://openai.com/enterprise-privacy ## Contributing -To contribute to the project, get started by reading our [Contributing Guide](https://github.com/the-pr-agent/pr-agent/blob/main/CONTRIBUTING.md). +To contribute to the project, get started by reading our [Contributing Guide](https://github.com/your-org/DiffMind/blob/main/CONTRIBUTING.md). For local verification, run `PYTHONPATH=. uv run pytest` from the repository root; it discovers the unit-test suite under `tests/unittest` by default. End-to-end tests under `tests/e2e_tests` require provider credentials and should be invoked explicitly, for example `PYTHONPATH=. uv run pytest tests/e2e_tests/test_github_app.py`. - -## Big News for PR-Agent - -PR-Agent has a new home! - -After years of building this tool alongside the community, Qodo has donated PR-Agent to the open-source community - and we couldn't be more excited about what comes next. - -The project now lives in the PR-Agent org on GitHub, is fully community-owned, and is open for contributions and additional maintainers. - -What else changed: -- Docs moved to - [docs.pr-agent.ai](https://docs.pr-agent.ai/) -- Qodo Merge (Qodo 1.0), the hosted URL, which was the enterprise version of PR-Agent, has been rebranded and evolved into Qodo (Qodo 2.0), a full AI code review platform. - ## ❀️ Community -This open-source release remains here as a community contribution from Qodo β€” the origin of modern AI-powered code collaboration. We’re proud to share it and inspire developers worldwide. - -The project now has its first external maintainer, Naor ([@naorpeled](https://github.com/naorpeled)), and is currently in the process of being donated to an open-source foundation. +DiffMind is community-owned and open for contributions and additional maintainers. If you'd like to get involved, open an issue or a PR! From e3227557a2692ff67a1e0dcbf79cf3f44b679643 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 25 Sep 2026 05:47:56 +0000 Subject: [PATCH 6/6] chore(ci): bump astral-sh/setup-uv in the actions-minor-and-patch group Bumps the actions-minor-and-patch group with 1 update: [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv). Updates `astral-sh/setup-uv` from 10.1.0 to 10.2.0 - [Release notes](https://github.com/astral-sh/setup-uv/releases) - [Commits](https://github.com/astral-sh/setup-uv/compare/bec219d24cd3e171d82865faccec33120bb574f4...c18668ad3cf93ea998bef934396af7bb5c839dc7) --- updated-dependencies: - dependency-name: astral-sh/setup-uv dependency-version: 10.2.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: actions-minor-and-patch ... Signed-off-by: dependabot[bot] --- .github/workflows/build-and-test.yaml | 2 +- .github/workflows/pre-commit.yml | 2 +- .github/workflows/provider-smoke.yaml | 2 +- .github/workflows/publish.yml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build-and-test.yaml b/.github/workflows/build-and-test.yaml index 5c1596ded..bb753cc00 100644 --- a/.github/workflows/build-and-test.yaml +++ b/.github/workflows/build-and-test.yaml @@ -17,7 +17,7 @@ jobs: uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - id: uv - uses: astral-sh/setup-uv@bec219d24cd3e171d82865faccec33120bb574f4 # v10.1.0 + uses: astral-sh/setup-uv@c18668ad3cf93ea998bef934396af7bb5c839dc7 # v10.2.0 - name: Check uv.lock run: uv lock --check diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index 72a3dd19f..0cfb33859 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -14,6 +14,6 @@ jobs: steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 - - uses: astral-sh/setup-uv@bec219d24cd3e171d82865faccec33120bb574f4 # v10.1.0 + - uses: astral-sh/setup-uv@c18668ad3cf93ea998bef934396af7bb5c839dc7 # v10.2.0 # SEE https://github.com/pre-commit/action - uses: pre-commit/action@2c7b3805fd2a0fd8c1884dcaebf91fc102a13ecd # v3.0.1 diff --git a/.github/workflows/provider-smoke.yaml b/.github/workflows/provider-smoke.yaml index 862742d8c..3714f41a7 100644 --- a/.github/workflows/provider-smoke.yaml +++ b/.github/workflows/provider-smoke.yaml @@ -21,7 +21,7 @@ jobs: uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Setup uv - uses: astral-sh/setup-uv@bec219d24cd3e171d82865faccec33120bb574f4 # v10.1.0 + uses: astral-sh/setup-uv@c18668ad3cf93ea998bef934396af7bb5c839dc7 # v10.2.0 - name: Install dependencies run: uv sync diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 142b13f0f..d09e0baa2 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -255,7 +255,7 @@ jobs: with: python-version: '3.12' - - uses: astral-sh/setup-uv@bec219d24cd3e171d82865faccec33120bb574f4 # v10.1.0 + - uses: astral-sh/setup-uv@c18668ad3cf93ea998bef934396af7bb5c839dc7 # v10.2.0 - name: Flag missing PyPI publish if: needs.publish-pypi.result != 'success'