fix: improve rate limit handling with exponential backoff

[cabana8471-arch]

Summary

• Detects rate limits from HTTP 429 exceptions (not just response text)
• Implements exponential backoff for rate limits: 60s → 120s → 240s... (max 1 hour)
• Improves generic error backoff: 30s → 60s → 90s... (max 5 minutes)
• Expands rate limit detection patterns for better coverage

Problem

When Claude API hits rate limits via HTTP 429 exceptions, the agent would:

1. Catch the exception generically
2. Retry after just 3 seconds
3. Fail again immediately, causing rapid retry loops

Solution

• Add parse_retry_after() to extract wait times from error messages
• Add is_rate_limit_error() for comprehensive pattern matching
• Return new "rate_limit" status from exception handler
• Track retry counts with exponential backoff

Test plan

• Unit tests added (test_agent.py - 11 tests)
• Ruff lint passes
• Manual test: Set invalid API key, observe exponential backoff in logs

Fixes #41
Fixes #111

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Adaptive rate-limit handling with parsed retry delays, clamped wait times, and exponential/linear backoff strategies for more reliable retries.

• Bug Fixes

  • Better detection and recovery from rate-limited responses, reducing stalled sessions and unnecessary retry storms; clearer retry/wait messages.

• Tests

  • Comprehensive tests for rate-limit detection, retry parsing, clamping, and backoff calculations.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a new rate-limit utilities module and updates the agent loop to detect rate-limit errors, parse retry hints, and apply clamped exponential/linear backoff with separate counters for rate-limit and general errors; unit tests for the utilities are included.

Changes

Cohort / File(s)	Summary
Rate Limit Utilities rate_limit_utils.py	New module: regex patterns for rate-limit detection, is_rate_limit_error(), parse_retry_after(), calculate_rate_limit_backoff(), calculate_error_backoff(), and clamp_retry_delay() with caps/bounds.
Agent Resilience Enhancement agent.py	Integrates rate-limit utilities into the main loop: detects rate limits, parses retry-after, uses exponential (rate-limit) and linear (error) backoffs, tracks rate_limit_retries and error_retries, clamps delays, and adjusts logging/sleep behavior for continue/rate_limit/error statuses.
Unit Tests test_rate_limit_utils.py	New tests for parsing various Retry-After/retry formats, is_rate_limit_error() matching and false positives, backoff calculations (caps), and clamp_retry_delay() behavior.

Sequence Diagram

sequenceDiagram
    participant Agent
    participant API
    participant RateLimitUtils
    participant Backoff

    Agent->>API: perform request/operation
    API-->>Agent: response (success or error)
    alt error returned
        Agent->>RateLimitUtils: is_rate_limit_error(error_message)
        RateLimitUtils-->>Agent: true/false
        alt rate limit detected
            Agent->>RateLimitUtils: parse_retry_after(error_message)
            RateLimitUtils-->>Agent: retry_seconds or None
            alt retry_seconds present
                Agent->>Backoff: clamp_retry_delay(retry_seconds)
                Backoff-->>Agent: clamped_delay
            else retry unknown
                Agent->>Backoff: calculate_rate_limit_backoff(rate_limit_retries)
                Backoff-->>Agent: exponential_delay
            end
        else not a rate limit
            Agent->>Backoff: calculate_error_backoff(error_retries)
            Backoff-->>Agent: linear_delay
        end
        Agent->>Agent: sleep(delay) and adjust retry counters
        Agent->>API: retry or continue loop
    else success
        Agent->>Agent: reset retry counters and proceed
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰
I smelled the throttle on the line,
I paused my hops, I counted time.
A gentle backoff, clamped and neat,
Then onward light on nimble feet. 🥕

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR addresses the core coding requirements from both #41 and #111: rate-limit detection [#41, #111], exponential backoff implementation [#41], pausing/rescheduling instead of failing [#41, #111], and preventing rapid retry loops [#41]. However, reviewer comments reveal critical blockers: non-existent import (clear_stuck_features), overly broad regex patterns risking false positives, and missing error handling/bounds on retry-after parsing.	Remove the non-existent clear_stuck_features import and psutil detection block; narrow rate-limit regex patterns; add try/except and clamp bounds to retry-after parsing (max 3600s); use is_rate_limit_error helper consistently.
Out of Scope Changes check	⚠️ Warning	The import of non-existent clear_stuck_features and related psutil parent-process detection block are out of scope and unrelated to rate-limit handling improvements specified in #41 and #111.	Remove the clear_stuck_features import and associated psutil parent-process detection logic from agent.py as they are unrelated to rate-limit handling objectives.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main focus: improved rate limit handling with exponential backoff, which aligns with the core changes adding backoff logic and detection patterns.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

agent.py (1)

319-367: Rate‑limit response text can still retry too quickly.
In the "continue" path, counters are reset before checking response text. If a rate‑limit message lacks a reset time, the delay stays at 3s and the backoff counter is cleared, risking rapid retry loops. Consider applying parse_retry_after or exponential backoff here and only resetting rate_limit_retries when no rate‑limit signal is present.

💡 One possible adjustment

-        # Reset retry counters on success
-        rate_limit_retries = 0
-        error_retries = 0
+        # Reset error retries on success; rate-limit retries only reset if no signal
+        error_retries = 0
+        reset_rate_limit_retries = True

         delay_seconds = AUTO_CONTINUE_DELAY_SECONDS
         target_time_str = None

         # Check for rate limit indicators in response text
         response_lower = response.lower()
         if any(pattern in response_lower for pattern in RATE_LIMIT_PATTERNS):
             print("Claude Agent SDK indicated rate limit reached.")
+            reset_rate_limit_retries = False
+            retry_seconds = parse_retry_after(response)
+            if retry_seconds is not None:
+                delay_seconds = retry_seconds
+            else:
+                delay_seconds = min(60 * (2 ** rate_limit_retries), 3600)
+                rate_limit_retries += 1
             ...
+        if reset_rate_limit_retries:
+            rate_limit_retries = 0

🤖 Fix all issues with AI agents

In `@agent.py`:
- Around line 165-172: The rate-limit handling block incorrectly treats a parsed
retry value of 0 as falsy; update the conditional in the agent.py rate-limit
branch (the code using is_rate_limit_error and parse_retry_after) to check
retry_seconds is not None (explicitly) instead of using a truthy check so that
"Retry-After: 0" is honored and returned as ("rate_limit", "0") rather than
falling back to "unknown".

In `@test_agent.py`:
- Around line 12-54: Extract the duplicated rate-limit logic
(RATE_LIMIT_PATTERNS, parse_retry_after, is_rate_limit_error) into a small
shared module (e.g., rate_limit_utils) and change both test_agent.py and
agent.py to import those symbols from that module; ensure the new module exports
RATE_LIMIT_PATTERNS, parse_retry_after, and is_rate_limit_error with the same
signatures and behavior, update tests to import from it, and remove the copied
definitions from test_agent.py so production and tests use the single source of
truth.

🧹 Nitpick comments (2)

test_agent.py (1)

129-147: Backoff tests re‑encode formulas instead of exercising production behavior.
These assertions will still pass if run_autonomous_agent changes its backoff rules. Consider extracting backoff calculations into helpers (e.g., calc_rate_limit_backoff, calc_error_backoff) and testing those directly.

agent.py (1)

415-418: Backoff comment says “exponential” but formula is linear.
The code uses 30 * retries, so the comment should match to avoid confusion.

✏️ Suggested edit

-            # Non-rate-limit errors: shorter backoff but still exponential
+            # Non-rate-limit errors: linear backoff capped at 5 minutes

[leonvanzyl]

Hey @cabana8471-arch! Thanks for tackling rate limit handling — this is a real pain point (issues #41 and #111), and the core design here is solid. The exponential backoff approach, the shared rate_limit_utils.py module, and the clean "rate_limit" status separation are all well thought out.

I ran through a detailed review and found a few things that need attention before we can merge. Flagging them below roughly by priority.

---

🚫 Blocker: clear_stuck_features import will crash at startup

The PR imports clear_stuck_features from progress.py, but that function doesn't exist anywhere in the codebase. This means agent.py will fail with an ImportError the moment it's loaded.

The entire clear_stuck_features + psutil parent-process detection block (lines 175–195 in the diff) also seems unrelated to rate limiting — could you remove it from this PR and submit it separately if needed?

---

⚠️ Overly broad detection patterns

Some entries in RATE_LIMIT_PATTERNS risk false positives when matched against Claude's full response text:

Pattern	Concern
"please wait"	Claude naturally says "please wait while I..." during normal coding
"try again later"	Could appear in error-handling discussions or npm output
"429"	Matches port numbers (4293), line numbers (line 429), counts, etc.

Since these are substring-matched on response.lower(), any natural language output triggers exponential backoff. The other patterns ("rate limit", "rate_limit", "too many requests", "quota exceeded", "limit reached") are specific enough to be reliable. Could you remove the broad ones, or replace bare "429" with something like "http 429" or "429 too many"?

---

⚠️ Missing bounds check on parsed retry delay

In the elif status == "rate_limit": block:

if response != "unknown":
    delay_seconds = int(response)

Two issues here:

1. No try/except — a malformed value would crash the loop
2. No upper bound — a weird error message like "retry after 999999 seconds" would sleep for days

Suggestion: delay_seconds = min(int(response), 3600) wrapped in a try/except that falls back to the exponential backoff path.

---

💡 Smaller suggestions

• Use is_rate_limit_error() consistently — The "continue" path inlines any(pattern in response_lower for pattern in RATE_LIMIT_PATTERNS) instead of calling the function you already wrote. Using the function keeps detection logic in one place.

• Rename test_agent.py → test_rate_limit_utils.py — The tests only cover rate_limit_utils.py functions, and the project convention is test_{module}.py (e.g., test_security.py).

• Backoff tests don't test production code — TestExponentialBackoff independently computes the formula rather than importing/calling anything from agent.py. If the formula changes in production, these tests still pass. Consider extracting the backoff calculation into a function in rate_limit_utils.py and testing that.

• False-positive test cases — Would be great to add tests confirming that strings like "please wait for the build" or "see line 429" do NOT trigger rate-limit detection (especially if the broad patterns stay).

---

📝 For awareness (not blocking)

• Parallel mode: The orchestrator runs agents with --max-iterations 1, so backoff never accumulates across subprocess restarts. Rate-limited agents burn through MAX_FEATURE_RETRIES without benefit of the backoff. Worth documenting as a known limitation or opening a follow-up issue.

• SDK behavior: Rate limits (HTTP 429) may arrive as AssistantMessage objects rather than exceptions, so the except Exception path in run_agent_session may not fire for the most common rate-limit scenario. The response-text detection in the "continue" path is the actual safety net — worth confirming which path fires in practice.

---

Overall the direction is great — the backoff logic, utility extraction, and test coverage of the parsing functions are all good work. Just need to sort out the import crash, tighten the patterns, and add the bounds check. Looking forward to the updated version! 🙌

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@agent.py`:
- Around line 373-389: The rate-limit branch should reset the error counter so
backoffs remain consecutive: when handling status == "rate_limit" (in the block
using clamp_retry_delay and calculate_rate_limit_backoff and updating
rate_limit_retries), set error_retries = 0 before incrementing/using
rate_limit_retries; likewise, mirror this change in the error-handling branch
(where error_retries is updated) to reset rate_limit_retries = 0. Ensure both
places reference the existing variables rate_limit_retries and error_retries so
mixed events don't inflate delays.

In `@rate_limit_utils.py`:
- Around line 48-52: The current patterns list in rate_limit_utils.py (the regex
entries starting with "retry.?after" and similar) will capture bare digits even
when the text says "minutes" or "hours"; update those regexes to prohibit
minute/hour units after the captured number by adding a negative lookahead for
minute/hour variants (e.g. (?!\s*(?:m|min|minute|minutes|h|hr|hour|hours)) )
immediately after the digit capture, or alternatively require an explicit
seconds unit when a unit is present (so only seconds/s are accepted); adjust the
pattern strings in the patterns list accordingly so "retry after 5 minutes" no
longer returns 5 while keeping valid "5 seconds" matches.

In `@test_rate_limit_utils.py`:
- Around line 53-57: Add a negative assertion to the test_minutes_not_supported
test to ensure strings like "retry after 5 minutes" are not parsed;
specifically, call parse_retry_after("retry after 5 minutes") and assert it
returns None so the existing "retry after" regex doesn't accept minutes. Update
test_minutes_not_supported to include this additional assert alongside the
existing "wait 5 minutes" and "try again in 2 minutes" checks.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

agent.py (1)

277-371: ⚠️ Potential issue | 🟠 Major

Clamp reset‑time delays to the 1‑hour cap.
Line 328 allows up to 24 hours, which bypasses the 1‑hour cap used by clamp_retry_delay/calculate_rate_limit_backoff and the PR objectives. Align this path with the same cap (or update the objective if 24h is intended).

🔧 Suggested fix (use shared clamp)

-                        delay_seconds = int(min(
-                            delta.total_seconds(), 24 * 60 * 60
-                        ))  # Clamp to 24 hours max
+                        delay_seconds = clamp_retry_delay(int(delta.total_seconds()))

[leonvanzyl]

Thank you