diff --git a/agent.py b/agent.py index 46f1f34..265a702 100644 --- a/agent.py +++ b/agent.py @@ -24,7 +24,6 @@ if sys.platform == "win32": from client import create_client from progress import ( - clear_stuck_features, count_passing_tests, has_features, print_progress_summary, @@ -38,7 +37,9 @@ from prompts import ( get_testing_prompt, ) from rate_limit_utils import ( - RATE_LIMIT_PATTERNS, + calculate_error_backoff, + calculate_rate_limit_backoff, + clamp_retry_delay, is_rate_limit_error, parse_retry_after, ) @@ -173,28 +174,6 @@ async def run_autonomous_agent( # Create project directory project_dir.mkdir(parents=True, exist_ok=True) - # IMPORTANT: Do NOT clear stuck features in parallel mode! - # The orchestrator manages feature claiming atomically. - # Clearing here causes race conditions where features are marked in_progress - # by the orchestrator but immediately cleared by the agent subprocess on startup. - # - # For single-agent mode or manual runs, clearing is still safe because - # there's only one agent at a time and it happens before claiming any features. - # - # Only clear if we're NOT in a parallel orchestrator context - # (detected by checking if this agent is a subprocess spawned by orchestrator) - import psutil - try: - parent_process = psutil.Process().parent() - parent_name = parent_process.name() if parent_process else "" - - # Only clear if parent is NOT python (i.e., we're running manually, not from orchestrator) - if "python" not in parent_name.lower(): - clear_stuck_features(project_dir) - except Exception: - # If parent process check fails, err on the safe side and clear - clear_stuck_features(project_dir) - # Determine agent type if not explicitly set if agent_type is None: # Auto-detect based on whether we have features @@ -304,18 +283,17 @@ async def run_autonomous_agent( 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): + if is_rate_limit_error(response): print("Claude Agent SDK indicated rate limit reached.") reset_rate_limit_retries = False # Try to extract retry-after from response text first retry_seconds = parse_retry_after(response) if retry_seconds is not None: - delay_seconds = retry_seconds + delay_seconds = clamp_retry_delay(retry_seconds) else: # Use exponential backoff when retry-after unknown - delay_seconds = min(60 * (2 ** rate_limit_retries), 3600) + delay_seconds = calculate_rate_limit_backoff(rate_limit_retries) rate_limit_retries += 1 # Try to parse reset time from response (more specific format) @@ -347,9 +325,9 @@ async def run_autonomous_agent( target += timedelta(days=1) delta = target - now - delay_seconds = min( + delay_seconds = int(min( delta.total_seconds(), 24 * 60 * 60 - ) # Clamp to 24 hours max + )) # Clamp to 24 hours max target_time_str = target.strftime("%B %d, %Y at %I:%M %p %Z") except Exception as e: @@ -395,20 +373,25 @@ async def run_autonomous_agent( elif status == "rate_limit": # Smart rate limit handling with exponential backoff if response != "unknown": - delay_seconds = int(response) - print(f"\nRate limit hit. Waiting {delay_seconds} seconds before retry...") - else: - # Use exponential backoff when retry-after unknown - delay_seconds = min(60 * (2 ** rate_limit_retries), 3600) # Max 1 hour + try: + delay_seconds = clamp_retry_delay(int(response)) + except (ValueError, TypeError): + # Malformed value - fall through to exponential backoff + response = "unknown" + if response == "unknown": + # Use exponential backoff when retry-after unknown or malformed + delay_seconds = calculate_rate_limit_backoff(rate_limit_retries) rate_limit_retries += 1 print(f"\nRate limit hit. Backoff wait: {delay_seconds} seconds (attempt #{rate_limit_retries})...") + else: + print(f"\nRate limit hit. Waiting {delay_seconds} seconds before retry...") await asyncio.sleep(delay_seconds) elif status == "error": # Non-rate-limit errors: linear backoff capped at 5 minutes error_retries += 1 - delay_seconds = min(30 * error_retries, 300) # Max 5 minutes + delay_seconds = calculate_error_backoff(error_retries) print("\nSession encountered an error") print(f"Will retry in {delay_seconds}s (attempt #{error_retries})...") await asyncio.sleep(delay_seconds) diff --git a/rate_limit_utils.py b/rate_limit_utils.py index 6d817f3..de70b24 100644 --- a/rate_limit_utils.py +++ b/rate_limit_utils.py @@ -3,25 +3,31 @@ Rate Limit Utilities ==================== Shared utilities for detecting and handling API rate limits. -Used by both agent.py (production) and test_agent.py (tests). +Used by both agent.py (production) and test_rate_limit_utils.py (tests). """ import re from typing import Optional -# Rate limit detection patterns (used in both exception messages and response text) -RATE_LIMIT_PATTERNS = [ - "limit reached", - "rate limit", - "rate_limit", - "too many requests", - "quota exceeded", - "please wait", - "try again later", - "429", - "overloaded", +# Regex patterns for rate limit detection (used in both exception messages and response text) +# These patterns use word boundaries to avoid false positives like "PR #429" or "please wait while I..." +RATE_LIMIT_REGEX_PATTERNS = [ + r"\brate[_\s]?limit", # "rate limit", "rate_limit", "ratelimit" + r"\btoo\s+many\s+requests", # "too many requests" + r"\bhttp\s*429\b", # "http 429", "http429" + r"\bstatus\s*429\b", # "status 429", "status429" + r"\berror\s*429\b", # "error 429", "error429" + r"\b429\s+too\s+many", # "429 too many" + r"\boverloaded\b", # "overloaded" + r"\bquota\s*exceeded\b", # "quota exceeded" ] +# Compiled regex for efficient matching +_RATE_LIMIT_REGEX = re.compile( + "|".join(RATE_LIMIT_REGEX_PATTERNS), + re.IGNORECASE +) + def parse_retry_after(error_message: str) -> Optional[int]: """ @@ -57,7 +63,8 @@ def is_rate_limit_error(error_message: str) -> bool: """ Detect if an error message indicates a rate limit. - Checks against common rate limit patterns from various API providers. + Uses regex patterns with word boundaries to avoid false positives + like "PR #429", "please wait while I...", or "Node v14.29.0". Args: error_message: The error message to check @@ -65,5 +72,49 @@ def is_rate_limit_error(error_message: str) -> bool: Returns: True if the message indicates a rate limit, False otherwise. """ - error_lower = error_message.lower() - return any(pattern in error_lower for pattern in RATE_LIMIT_PATTERNS) + return bool(_RATE_LIMIT_REGEX.search(error_message)) + + +def calculate_rate_limit_backoff(retries: int) -> int: + """ + Calculate exponential backoff for rate limits. + + Formula: min(60 * 2^retries, 3600) - caps at 1 hour + Sequence: 60s, 120s, 240s, 480s, 960s, 1920s, 3600s... + + Args: + retries: Number of consecutive rate limit retries (0-indexed) + + Returns: + Delay in seconds (clamped to 1-3600 range) + """ + return int(min(max(60 * (2 ** retries), 1), 3600)) + + +def calculate_error_backoff(retries: int) -> int: + """ + Calculate linear backoff for non-rate-limit errors. + + Formula: min(30 * retries, 300) - caps at 5 minutes + Sequence: 30s, 60s, 90s, 120s, ... 300s + + Args: + retries: Number of consecutive error retries (1-indexed) + + Returns: + Delay in seconds (clamped to 1-300 range) + """ + return min(max(30 * retries, 1), 300) + + +def clamp_retry_delay(delay_seconds: int) -> int: + """ + Clamp a retry delay to a safe range (1-3600 seconds). + + Args: + delay_seconds: The raw delay value + + Returns: + Delay clamped to 1-3600 seconds + """ + return min(max(delay_seconds, 1), 3600) diff --git a/test_agent.py b/test_rate_limit_utils.py similarity index 51% rename from test_agent.py rename to test_rate_limit_utils.py index f672ecb..eb1f01c 100644 --- a/test_agent.py +++ b/test_rate_limit_utils.py @@ -1,13 +1,16 @@ """ Unit tests for rate limit handling functions. -Tests the parse_retry_after() and is_rate_limit_error() functions -from rate_limit_utils.py (shared module). +Tests the parse_retry_after(), is_rate_limit_error(), and backoff calculation +functions from rate_limit_utils.py (shared module). """ import unittest from rate_limit_utils import ( + calculate_error_backoff, + calculate_rate_limit_backoff, + clamp_retry_delay, is_rate_limit_error, parse_retry_after, ) @@ -64,10 +67,15 @@ class TestIsRateLimitError(unittest.TestCase): assert is_rate_limit_error("Too many requests") is True assert is_rate_limit_error("HTTP 429 Too Many Requests") is True assert is_rate_limit_error("API quota exceeded") is True - assert is_rate_limit_error("Please wait before retrying") is True - assert is_rate_limit_error("Try again later") is True assert is_rate_limit_error("Server is overloaded") is True - assert is_rate_limit_error("Usage limit reached") is True + + def test_specific_429_patterns(self): + """Test that 429 is detected with proper context.""" + assert is_rate_limit_error("http 429") is True + assert is_rate_limit_error("HTTP429") is True + assert is_rate_limit_error("status 429") is True + assert is_rate_limit_error("error 429") is True + assert is_rate_limit_error("429 too many requests") is True def test_case_insensitive(self): """Test that detection is case-insensitive.""" @@ -86,26 +94,83 @@ class TestIsRateLimitError(unittest.TestCase): assert is_rate_limit_error("") is False -class TestExponentialBackoff(unittest.TestCase): - """Test exponential backoff calculations.""" +class TestFalsePositives(unittest.TestCase): + """Verify non-rate-limit messages don't trigger detection.""" - def test_backoff_sequence(self): - """Test that backoff follows expected sequence.""" - # Simulating: min(60 * (2 ** retries), 3600) + def test_version_numbers_with_429(self): + """Version numbers should not trigger.""" + assert is_rate_limit_error("Node v14.29.0") is False + assert is_rate_limit_error("Python 3.12.429") is False + assert is_rate_limit_error("Version 2.429 released") is False + + def test_issue_and_pr_numbers(self): + """Issue/PR numbers should not trigger.""" + assert is_rate_limit_error("See PR #429") is False + assert is_rate_limit_error("Fixed in issue 429") is False + assert is_rate_limit_error("Closes #429") is False + + def test_line_numbers(self): + """Line numbers in errors should not trigger.""" + assert is_rate_limit_error("Error at line 429") is False + assert is_rate_limit_error("See file.py:429") is False + + def test_port_numbers(self): + """Port numbers should not trigger.""" + assert is_rate_limit_error("port 4293") is False + assert is_rate_limit_error("localhost:4290") is False + + def test_legitimate_wait_messages(self): + """Legitimate wait instructions should not trigger.""" + # These would fail if "please wait" pattern still exists + assert is_rate_limit_error("Please wait for the build to complete") is False + assert is_rate_limit_error("Please wait while I analyze this") is False + + def test_retry_discussion_messages(self): + """Messages discussing retry logic should not trigger.""" + # These would fail if "try again later" pattern still exists + assert is_rate_limit_error("Try again later after maintenance") is False + assert is_rate_limit_error("The user should try again later") is False + + def test_limit_discussion_messages(self): + """Messages discussing limits should not trigger (removed pattern).""" + # These would fail if "limit reached" pattern still exists + assert is_rate_limit_error("File size limit reached") is False + assert is_rate_limit_error("Memory limit reached, consider optimization") is False + + +class TestBackoffFunctions(unittest.TestCase): + """Test backoff calculation functions from rate_limit_utils.""" + + def test_rate_limit_backoff_sequence(self): + """Test that rate limit backoff follows expected exponential sequence.""" expected = [60, 120, 240, 480, 960, 1920, 3600, 3600] # Caps at 3600 for retries, expected_delay in enumerate(expected): - delay = min(60 * (2 ** retries), 3600) + delay = calculate_rate_limit_backoff(retries) assert delay == expected_delay, f"Retry {retries}: expected {expected_delay}, got {delay}" def test_error_backoff_sequence(self): - """Test error backoff follows expected sequence.""" - # Simulating: min(30 * retries, 300) + """Test that error backoff follows expected linear sequence.""" expected = [30, 60, 90, 120, 150, 180, 210, 240, 270, 300, 300] # Caps at 300 for retries in range(1, len(expected) + 1): - delay = min(30 * retries, 300) + delay = calculate_error_backoff(retries) expected_delay = expected[retries - 1] assert delay == expected_delay, f"Retry {retries}: expected {expected_delay}, got {delay}" + def test_clamp_retry_delay(self): + """Test that retry delay is clamped to valid range.""" + # Values within range stay the same + assert clamp_retry_delay(60) == 60 + assert clamp_retry_delay(1800) == 1800 + assert clamp_retry_delay(3600) == 3600 + + # Values below minimum get clamped to 1 + assert clamp_retry_delay(0) == 1 + assert clamp_retry_delay(-10) == 1 + + # Values above maximum get clamped to 3600 + assert clamp_retry_delay(7200) == 3600 + assert clamp_retry_delay(86400) == 3600 + if __name__ == "__main__": unittest.main()