From c3b8f53619aa5d8f7c84a987c55f33eceaef9140 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Tue, 23 Jun 2026 14:13:39 -0700 Subject: [PATCH] [eric] agents: error_classify convention-clean (6 is_* funcs + 2 pattern consts off leading-_, @typechecked) --- backend/apps/agents/agent_manager.py | 10 ++--- backend/apps/agents/core/error_classify.py | 30 ++++++++------ backend/tests/test_free_trial.py | 12 +++--- backend/tests/test_v2_invariants.py | 46 +++++++++++----------- 4 files changed, 52 insertions(+), 46 deletions(-) diff --git a/backend/apps/agents/agent_manager.py b/backend/apps/agents/agent_manager.py index 2f98243e..325b80ee 100644 --- a/backend/apps/agents/agent_manager.py +++ b/backend/apps/agents/agent_manager.py @@ -20,11 +20,11 @@ from backend.apps.tools_lib.tools_lib import ( from backend.apps.agents.core.error_classify import ( CAPACITY_BACKOFFS, capacity_retry_wait, - _is_auth_error as is_auth_error, - _is_free_trial_exhausted as is_free_trial_exhausted, - _is_long_context_error as is_long_context_error, - _is_transient_capacity_error as is_transient_capacity_error, - _is_unknown_model_error as is_unknown_model_error, + is_auth_error, + is_free_trial_exhausted, + is_long_context_error, + is_transient_capacity_error, + is_unknown_model_error, parse_retry_after, redact_for_telemetry, ) diff --git a/backend/apps/agents/core/error_classify.py b/backend/apps/agents/core/error_classify.py index 6d7a7039..05830c9a 100644 --- a/backend/apps/agents/core/error_classify.py +++ b/backend/apps/agents/core/error_classify.py @@ -31,7 +31,7 @@ def redact_for_telemetry(text: str, *, limit: int = 2000) -> str: # Patterns that indicate an upstream transient problem (overload / rate limit / # infra blip), safe to silently retry with backoff. Checked against the # stringified exception from claude_agent_sdk / Claude CLI. -_TRANSIENT_CAPACITY_PATTERNS = re.compile( +TRANSIENT_CAPACITY_PATTERNS = re.compile( r"(?:\b(?:429|500|502|503|504|529)\b" r"|overloaded" r"|service\s+(?:temporarily\s+)?unavailable" @@ -70,7 +70,7 @@ _TRANSLATION_ERROR_PATTERNS = re.compile( # Anthropic returns when an OAuth Pro/Max account ships a request whose input # exceeds the 200K standard tier and would need the "extra usage" tier; the # user can't recover by waiting, so we surface it instead of looping. -_NON_TRANSIENT_PATTERNS = re.compile( +NON_TRANSIENT_PATTERNS = re.compile( r"(?:usage\s+cap\s+exceeded" r"|reached\s+your\s+OpenSwarm.*plan\s+limit" r"|no\s+active\s+subscription" @@ -85,7 +85,8 @@ _NON_TRANSIENT_PATTERNS = re.compile( ) -def _is_long_context_error(exc: BaseException, extra_text: str = "") -> bool: +@typechecked +def is_long_context_error(exc: BaseException, extra_text: str = "") -> bool: """True when the upstream error is the 'long context tier required' 429. Used by the catch-all error path to emit a friendly context-overflow @@ -102,7 +103,8 @@ def _is_long_context_error(exc: BaseException, extra_text: str = "") -> bool: )) -def _is_free_trial_exhausted(exc: BaseException, extra_text: str = "") -> bool: +@typechecked +def is_free_trial_exhausted(exc: BaseException, extra_text: str = "") -> bool: """True when the cloud says the machine's free runs are spent (a 402 with type free_trial_exhausted). The catch-all path uses this to flip back to own_key and show a friendly connect-a-model upsell instead of a raw error. @@ -117,7 +119,8 @@ def _is_free_trial_exhausted(exc: BaseException, extra_text: str = "") -> bool: )) -def _is_translation_error(exc: BaseException, extra_text: str = "") -> bool: +@typechecked +def is_translation_error(exc: BaseException, extra_text: str = "") -> bool: """True when the upstream 400 is a tool-schema / protocol translation failure (9Router rewriting Anthropic tools into Gemini function_declarations or OpenAI params), not auth or capacity. Kept distinct so the catch-all @@ -128,7 +131,8 @@ def _is_translation_error(exc: BaseException, extra_text: str = "") -> bool: return bool(_TRANSLATION_ERROR_PATTERNS.search(combined)) -def _is_auth_error(exc: BaseException, extra_text: str = "") -> bool: +@typechecked +def is_auth_error(exc: BaseException, extra_text: str = "") -> bool: """True when the upstream error is a 401/403 auth failure. Used by the catch-all error path to surface a friendly "subscription @@ -141,7 +145,7 @@ def _is_auth_error(exc: BaseException, extra_text: str = "") -> bool: return False # A tool-schema translation 400 can carry provider/connection wording that # trips the auth regex below; it isn't auth, so don't claim it is. - if _is_translation_error(exc, extra_text): + if is_translation_error(exc, extra_text): return False return bool(re.search( r"\b(401|403)\b" @@ -156,7 +160,8 @@ def _is_auth_error(exc: BaseException, extra_text: str = "") -> bool: )) -def _is_unknown_model_error(exc: BaseException, extra_text: str = "") -> bool: +@typechecked +def is_unknown_model_error(exc: BaseException, extra_text: str = "") -> bool: """True when the upstream rejects the model code itself (e.g. a ChatGPT/Codex subscription whose plan doesn't expose the GPT model id we send: code 1211 'Unknown Model, please check the model code'). The fix isn't retry, it's a @@ -194,7 +199,8 @@ def parse_retry_after(exc: BaseException, extra_text: str = "") -> int | None: return None -def _is_transient_capacity_error(exc: BaseException, extra_text: str = "") -> bool: +@typechecked +def is_transient_capacity_error(exc: BaseException, extra_text: str = "") -> bool: # The Claude CLI's underlying ProcessError stringifies to a generic # "Command failed with exit code 1 / Check stderr output for details"; # the real cause (rate_limit_error / No pool capacity available / 429 @@ -205,9 +211,9 @@ def _is_transient_capacity_error(exc: BaseException, extra_text: str = "") -> bo combined = f"{exc!s}\n{extra_text}".strip() if not combined: return False - if _NON_TRANSIENT_PATTERNS.search(combined): + if NON_TRANSIENT_PATTERNS.search(combined): return False - if _TRANSIENT_CAPACITY_PATTERNS.search(combined): + if TRANSIENT_CAPACITY_PATTERNS.search(combined): return True # Pool-exhaustion copy from the OpenSwarm proxy ("No pool capacity # available. Try again shortly."), matches the capacity family too. @@ -226,6 +232,6 @@ def capacity_retry_wait(exc: BaseException, attempt: int, extra_text: str = "") """Seconds to wait before retrying a transient upstream capacity error (429 / overload / 5xx / network blip), or None when the error isn't transient or the backoff budget for this turn is already spent. Keeps the retry DECISION testable; the loop owns the wait.""" - if _is_transient_capacity_error(exc, extra_text=extra_text) and 0 <= attempt < len(CAPACITY_BACKOFFS): + if is_transient_capacity_error(exc, extra_text=extra_text) and 0 <= attempt < len(CAPACITY_BACKOFFS): return CAPACITY_BACKOFFS[attempt] return None diff --git a/backend/tests/test_free_trial.py b/backend/tests/test_free_trial.py index bc4cc522..93cd4d81 100644 --- a/backend/tests/test_free_trial.py +++ b/backend/tests/test_free_trial.py @@ -7,8 +7,8 @@ import pytest from backend.apps.settings.models import AppSettings from backend.apps.settings.credentials import proxy_auth from backend.apps.agents.core.error_classify import ( - _is_free_trial_exhausted, - _is_transient_capacity_error, + is_free_trial_exhausted, + is_transient_capacity_error, ) from backend.apps.agents.providers.registry import resolve_model_id_for_sdk from backend.apps.subscription import free_trial as ft @@ -44,11 +44,11 @@ def test_free_trial_resolves_to_a_bare_anthropic_id(): def test_exhaustion_is_classified_and_not_retried(): - assert _is_free_trial_exhausted(Exception("error type free_trial_exhausted")) - assert _is_free_trial_exhausted(Exception("You've used your free OpenSwarm runs")) - assert not _is_free_trial_exhausted(Exception("overloaded, try again")) + assert is_free_trial_exhausted(Exception("error type free_trial_exhausted")) + assert is_free_trial_exhausted(Exception("You've used your free OpenSwarm runs")) + assert not is_free_trial_exhausted(Exception("overloaded, try again")) # Must NOT look transient, or the agent loop would retry a spent trial forever. - assert not _is_transient_capacity_error(Exception("free_trial_exhausted")) + assert not is_transient_capacity_error(Exception("free_trial_exhausted")) def test_has_own_model_never_shadows_a_real_provider(): diff --git a/backend/tests/test_v2_invariants.py b/backend/tests/test_v2_invariants.py index 9df8ba65..ae8313d1 100644 --- a/backend/tests/test_v2_invariants.py +++ b/backend/tests/test_v2_invariants.py @@ -600,25 +600,25 @@ def test_error_classify_schema_translation_400_is_not_auth(): 'reconnect your subscription' card for what is really a schema bug. The translation guard must win: schema 400 -> not auth; a real auth failure with no translation signature still reads as auth.""" - from backend.apps.agents.core.error_classify import _is_auth_error, _is_translation_error + from backend.apps.agents.core.error_classify import is_auth_error, is_translation_error both = Exception("provider not connected: 400 INVALID_ARGUMENT at " "tools[0].function_declarations[0].parameters") - assert _is_translation_error(both) - assert not _is_auth_error(both), "schema-400 must not be classified as auth" + assert is_translation_error(both) + assert not is_auth_error(both), "schema-400 must not be classified as auth" # Pure auth failures (no translation signature) still classify as auth. - assert _is_auth_error(Exception("provider not connected: gemini")) - assert _is_auth_error(Exception("401 invalid authentication credentials")) - assert not _is_translation_error(Exception("401 invalid authentication credentials")) + assert is_auth_error(Exception("provider not connected: gemini")) + assert is_auth_error(Exception("401 invalid authentication credentials")) + assert not is_translation_error(Exception("401 invalid authentication credentials")) def test_error_classify_gemini_resource_exhausted_is_transient(): """gemini-cli's free-tier 429 surfaces as RESOURCE_EXHAUSTED; it must count as transient so the existing backoff/retry catches it instead of dying as a hard first-message error. A 403 (hard auth/quota) must still NOT retry.""" - from backend.apps.agents.core.error_classify import _is_transient_capacity_error - assert _is_transient_capacity_error(Exception("429 RESOURCE_EXHAUSTED: Quota exceeded")) - assert _is_transient_capacity_error(Exception("RESOURCE_EXHAUSTED")) - assert not _is_transient_capacity_error(Exception("403 permission denied")) + from backend.apps.agents.core.error_classify import is_transient_capacity_error + assert is_transient_capacity_error(Exception("429 RESOURCE_EXHAUSTED: Quota exceeded")) + assert is_transient_capacity_error(Exception("RESOURCE_EXHAUSTED")) + assert not is_transient_capacity_error(Exception("403 permission denied")) @pytest.mark.asyncio @@ -761,8 +761,8 @@ def test_router_auth_pattern_does_not_falsely_match_normal_text(): def test_is_auth_error_classifier(): - """The classifier at agent_manager.py:_is_auth_error covers many shapes.""" - from backend.apps.agents.core.error_classify import _is_auth_error + """The classifier at agent_manager.py:is_auth_error covers many shapes.""" + from backend.apps.agents.core.error_classify import is_auth_error # Real shapes that must be caught matches = [ @@ -775,7 +775,7 @@ def test_is_auth_error_classifier(): Exception("Provider not configured: gemini"), ] for e in matches: - assert _is_auth_error(e), f"should match: {e}" + assert is_auth_error(e), f"should match: {e}" # Non-auth errors must not match non_matches = [ @@ -785,15 +785,15 @@ def test_is_auth_error_classifier(): Exception("File not found"), ] for e in non_matches: - assert not _is_auth_error(e), f"should NOT match: {e}" + assert not is_auth_error(e), f"should NOT match: {e}" def test_is_auth_error_with_stderr_tail(): """The classifier also reads stderr buffer text.""" - from backend.apps.agents.core.error_classify import _is_auth_error + from backend.apps.agents.core.error_classify import is_auth_error e = Exception("Command failed with exit code 1") stderr = "...\n[codex/gpt-5.5] [401]: Provided authentication token is expired" - assert _is_auth_error(e, extra_text=stderr) + assert is_auth_error(e, extra_text=stderr) # =========================================================================== @@ -892,19 +892,19 @@ def test_active_mcps_persistence_on_session(): def test_long_context_pattern_caught(): """The 'extra usage required' 429 must NOT silently retry.""" - from backend.apps.agents.core.error_classify import _NON_TRANSIENT_PATTERNS + from backend.apps.agents.core.error_classify import NON_TRANSIENT_PATTERNS cases = [ "Extra usage is required for long context requests", "extra usage is required for long context", "EXTRA USAGE IS REQUIRED FOR LONG CONTEXT", ] for case in cases: - assert _NON_TRANSIENT_PATTERNS.search(case), f"missed: {case!r}" + assert NON_TRANSIENT_PATTERNS.search(case), f"missed: {case!r}" def test_transient_capacity_patterns(): """Real transient errors that SHOULD retry.""" - from backend.apps.agents.core.error_classify import _TRANSIENT_CAPACITY_PATTERNS, _NON_TRANSIENT_PATTERNS + from backend.apps.agents.core.error_classify import TRANSIENT_CAPACITY_PATTERNS, NON_TRANSIENT_PATTERNS transients = [ "Error 429: rate_limit_error", "503 Service Unavailable", @@ -916,18 +916,18 @@ def test_transient_capacity_patterns(): "overloaded", ] for t in transients: - assert _TRANSIENT_CAPACITY_PATTERNS.search(t), f"transient missed: {t!r}" + assert TRANSIENT_CAPACITY_PATTERNS.search(t), f"transient missed: {t!r}" # Importantly: must NOT also match non-transient (no double-classification) # except for the fuzzy edge cases. Spot-check a couple: if "429" in t and "rate_limit" in t.lower(): # rate_limit_error is transient; non-transient should not match this exact text - assert not _NON_TRANSIENT_PATTERNS.search(t) + assert not NON_TRANSIENT_PATTERNS.search(t) def test_long_context_does_not_match_normal_429(): """Generic 429 is transient, only the long-context variant is non-transient.""" - from backend.apps.agents.core.error_classify import _NON_TRANSIENT_PATTERNS - assert not _NON_TRANSIENT_PATTERNS.search("Error 429: rate_limit_error") + from backend.apps.agents.core.error_classify import NON_TRANSIENT_PATTERNS + assert not NON_TRANSIENT_PATTERNS.search("Error 429: rate_limit_error") # ===========================================================================