ros/autocoder
1
0
Fork 0
mirror of https://github.com/leonvanzyl/autocoder.git synced 2026-02-01 15:03:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: address PR #109 review feedback from leonvanzyl

Browse Source 
- BLOCKER: Remove clear_stuck_features import and psutil block (doesn't exist in upstream)
- Fix overly broad rate limit patterns to avoid false positives
  - Remove "please wait", "try again later", "limit reached", "429" (bare)
  - Convert to regex-based detection with word boundaries
  - Add patterns for "http 429", "status 429", "error 429"
- Add bounds checking (1-3600s) for parsed retry delays
- Use is_rate_limit_error() consistently instead of inline pattern matching
- Extract backoff functions to rate_limit_utils.py for testability
  - calculate_rate_limit_backoff() for exponential backoff
  - calculate_error_backoff() for linear backoff
  - clamp_retry_delay() for safe range enforcement
- Rename test_agent.py to test_rate_limit_utils.py (matches module)
- Add comprehensive false-positive tests:
  - Version numbers (v14.29.0)
  - Issue/PR numbers (#429)
  - Line numbers (file.py:429)
  - Port numbers (4293)
  - Legitimate wait/retry messages

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
cabana8471
2026-01-30 21:20:52 +01:00
parent dcf8b99dca
commit f018b4c1d8
3 changed files with 164 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files

55
agent.py
View File

@@ -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)

81
rate_limit_utils.py
View File

@@ -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)

93
test_agent.py → test_rate_limit_utils.py
View File

@@ -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()