From 45f5a6e659ec8bb17b963ab0775043bf6a9bf7ba Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Tue, 7 Jul 2026 11:49:34 +0530 Subject: [PATCH] refactor(retry): single-source Z.AI overload short-attempts + drop change-detector assert Follow-up on the salvage of #59523. Two low-risk cleanups surfaced by review: - Extract _ZAI_CODING_OVERLOAD_SHORT_ATTEMPTS as a module constant so adaptive_rate_limit_backoff() and zai_coding_overload_retry_ceiling() share one source of truth. Previously both hardcoded short_attempts=3 independently; tuning one without the other would silently desync the retry ceiling from the backoff schedule. - Replace the tautological formula-mirroring assert in test_zai_overload_retry_ceiling_exceeds_short_attempts with a behavior invariant (ceiling leaves headroom for every long-backoff entry), per the repo's contracts-over-snapshots testing rule. --- agent/conversation_loop.py | 17 +++++++---------- agent/retry_utils.py | 12 ++++++++++-- tests/test_retry_utils.py | 11 +++++++---- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 122cf76883d..cbcb8561701 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -3147,16 +3147,13 @@ def run_conversation( FailoverReason.timeout, FailoverReason.overloaded, } - # Z.AI Coding Plan GLM-5.2 signals server overload as HTTP 429 - # code 1305 ("temporarily overloaded"). classify_api_error routes - # that to `overloaded` (not `rate_limit`) so the credential pool - # isn't burned on a valid key — but `overloaded` is excluded from - # `is_rate_limited`, which is exactly what gates the adaptive Z.AI - # backoff below. Detect the overload 429 directly so its adaptive - # long-backoff schedule still runs, and raise the retry ceiling so - # the long tier (30/60/90/120s) is actually reachable: the default - # ceiling equals the short-retry threshold, so the loop otherwise - # gives up after a few quick retries and the long tier is dead code. + # Z.AI Coding Plan GLM-5.2 overload 429s classify as + # `overloaded` (to spare the credential pool), but `overloaded` + # is excluded from `is_rate_limited` — the gate for the adaptive + # Z.AI backoff below. Detect the overload directly so its + # long-backoff schedule runs, and raise the retry ceiling so the + # long tier (30/60/90/120s) is reachable. See + # zai_coding_overload_retry_ceiling() for the ceiling rationale. _is_zai_coding_overload = is_zai_coding_overload_error( base_url=str(_base), model=_model, error=api_error ) diff --git a/agent/retry_utils.py b/agent/retry_utils.py index 3fc5645edb6..c4971122394 100644 --- a/agent/retry_utils.py +++ b/agent/retry_utils.py @@ -24,6 +24,14 @@ _jitter_lock = threading.Lock() # not sit silent for 20+ minutes. _ZAI_CODING_OVERLOAD_LONG_BACKOFF = (30.0, 60.0, 90.0, 120.0) +# Number of initial short retries before the adaptive long-backoff tier kicks +# in. Shared by ``adaptive_rate_limit_backoff`` (which walks the long table +# starting at attempt ``short_attempts + 1``) and +# ``zai_coding_overload_retry_ceiling`` (which sizes the retry loop so every +# long-tier entry is reachable). Keeping it a single module constant prevents +# the two from silently desyncing if the short-retry count is ever tuned. +_ZAI_CODING_OVERLOAD_SHORT_ATTEMPTS = 3 + def jittered_backoff( attempt: int, @@ -104,7 +112,7 @@ def adaptive_rate_limit_backoff( model: str | None, error: Any, default_wait: float, - short_attempts: int = 3, + short_attempts: int = _ZAI_CODING_OVERLOAD_SHORT_ATTEMPTS, ) -> tuple[float, str | None]: """Provider-aware rate-limit backoff. @@ -129,7 +137,7 @@ def adaptive_rate_limit_backoff( return jittered_backoff(1, base_delay=base_delay, max_delay=base_delay, jitter_ratio=0.2), "zai_coding_overload_long" -def zai_coding_overload_retry_ceiling(short_attempts: int = 3) -> int: +def zai_coding_overload_retry_ceiling(short_attempts: int = _ZAI_CODING_OVERLOAD_SHORT_ATTEMPTS) -> int: """Retry-loop ceiling needed for the full Z.AI overload backoff schedule. The adaptive policy runs ``short_attempts`` short retries, then walks the diff --git a/tests/test_retry_utils.py b/tests/test_retry_utils.py index 460a9b8c0f3..49bdb09ec0b 100644 --- a/tests/test_retry_utils.py +++ b/tests/test_retry_utils.py @@ -224,10 +224,13 @@ def test_zai_overload_retry_ceiling_exceeds_short_attempts(): short_attempts = 3 ceiling = zai_coding_overload_retry_ceiling(short_attempts) assert ceiling > short_attempts - # Room for every long-backoff entry to run before giving up: the loop's - # give-up check (retry_count >= ceiling) runs before the attempt's backoff, - # so the ceiling sits one past the final long entry. - assert ceiling == short_attempts + len(_ZAI_CODING_OVERLOAD_LONG_BACKOFF) + 1 + # Invariant (not a formula mirror): the loop's give-up check + # (retry_count >= ceiling) runs *before* the attempt's backoff, so the + # ceiling must leave headroom for every long-backoff entry to execute — + # i.e. the largest attempt the loop still computes backoff for + # (ceiling - 1) must reach the final long-tier index. + last_attempt_with_backoff = ceiling - 1 + assert last_attempt_with_backoff - short_attempts >= len(_ZAI_CODING_OVERLOAD_LONG_BACKOFF) def test_zai_overload_ceiling_makes_long_tier_reachable(monkeypatch):