adding rate limit tests

tests/unit/resolver/test_guess_success.py

@@ -7,21 +7,6 @@
 from openhands.resolver.issue_definitions import IssueHandler, PRHandler
 
 
-class MockLLMResponse:
-    """Mock LLM Response class to mimic the actual LLM response structure."""
-
-    class Choice:
-        class Message:
-            def __init__(self, content):
-                self.content = content
-
-        def __init__(self, content):
-            self.message = self.Message(content)
-
-    def __init__(self, content):
-        self.choices = [self.Choice(content)]
-
-
 def test_guess_success_multiline_explanation():
     # Mock data
     issue = GithubIssue(
@@ -37,7 +22,11 @@ def test_guess_success_multiline_explanation():
     llm_config = LLMConfig(model='test', api_key='test')
 
     # Create a mock response with multi-line explanation
-    mock_response = """--- success
+    mock_response = MagicMock()
+    mock_response.choices = [
+        MagicMock(
+            message=MagicMock(
+                content="""--- success
 true
 
 --- explanation
@@ -47,28 +36,28 @@ def test_guess_success_multiline_explanation():
 - Updated documentation C
 
 Automatic fix generated by OpenHands 🙌"""
+            )
+        )
+    ]
 
-    # Mock the litellm.completion call
-    mock_llm = MagicMock(spec=LLM)
-    mock_llm.completion.return_value = MockLLMResponse(mock_response)
-
-    # Create a handler instance
-    llm_config = LLMConfig(model='test', api_key='test')
-    handler = IssueHandler('test', 'test', 'test', llm_config)
-    handler.llm = mock_llm
-
-    # Call guess_success
-    success, _, explanation = handler.guess_success(issue, history)
+    # Use patch to mock the LLM completion call
+    with patch.object(LLM, 'completion', return_value=mock_response) as mock_completion:
+        # Create a handler instance
+        handler = IssueHandler('test', 'test', 'test', llm_config)
 
-    # Verify the results
-    assert success is True
-    assert 'The PR successfully addressed the issue by:' in explanation
-    assert 'Fixed bug A' in explanation
-    assert 'Added test B' in explanation
-    assert 'Updated documentation C' in explanation
-    assert 'Automatic fix generated by OpenHands' in explanation
+        # Call guess_success
+        success, _, explanation = handler.guess_success(issue, history)
 
-    mock_llm.completion.assert_called_once()
+        # Verify the results
+        assert success is True
+        assert 'The PR successfully addressed the issue by:' in explanation
+        assert 'Fixed bug A' in explanation
+        assert 'Added test B' in explanation
+        assert 'Updated documentation C' in explanation
+        assert 'Automatic fix generated by OpenHands' in explanation
+
+        # Verify that LLM completion was called exactly once
+        mock_completion.assert_called_once()
 
 
 def test_pr_handler_guess_success_with_thread_comments():

tests/unit/resolver/test_issue_handler_error_handling.py

@@ -1,9 +1,34 @@
 from unittest.mock import MagicMock, patch
 
+import pytest
 import requests
+from litellm.exceptions import RateLimitError
 
 from openhands.core.config import LLMConfig
-from openhands.resolver.issue_definitions import PRHandler
+from openhands.events.action.message import MessageAction
+from openhands.llm.llm import LLM
+from openhands.resolver.github_issue import GithubIssue
+from openhands.resolver.issue_definitions import IssueHandler, PRHandler
+
+
+@pytest.fixture(autouse=True)
+def mock_logger(monkeypatch):
+    # suppress logging of completion data to file
+    mock_logger = MagicMock()
+    monkeypatch.setattr('openhands.llm.debug_mixin.llm_prompt_logger', mock_logger)
+    monkeypatch.setattr('openhands.llm.debug_mixin.llm_response_logger', mock_logger)
+    return mock_logger
+
+
+@pytest.fixture
+def default_config():
+    return LLMConfig(
+        model='gpt-4o',
+        api_key='test_key',
+        num_retries=2,
+        retry_min_wait=1,
+        retry_max_wait=2,
+    )
 
 
 def test_handle_nonexistent_issue_reference():
@@ -100,3 +125,141 @@ def test_successful_issue_reference():
 
         # The method should return a list with the referenced issue body
         assert result == ['This is the referenced issue body']
+
+
+class MockLLMResponse:
+    """Mock LLM Response class to mimic the actual LLM response structure."""
+
+    class Choice:
+        class Message:
+            def __init__(self, content):
+                self.content = content
+
+        def __init__(self, content):
+            self.message = self.Message(content)
+
+    def __init__(self, content):
+        self.choices = [self.Choice(content)]
+
+
+class DotDict(dict):
+    """
+    A dictionary that supports dot notation access.
+    """
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        for key, value in self.items():
+            if isinstance(value, dict):
+                self[key] = DotDict(value)
+            elif isinstance(value, list):
+                self[key] = [
+                    DotDict(item) if isinstance(item, dict) else item for item in value
+                ]
+
+    def __getattr__(self, key):
+        if key in self:
+            return self[key]
+        else:
+            raise AttributeError(
+                f"'{self.__class__.__name__}' object has no attribute '{key}'"
+            )
+
+    def __setattr__(self, key, value):
+        self[key] = value
+
+    def __delattr__(self, key):
+        if key in self:
+            del self[key]
+        else:
+            raise AttributeError(
+                f"'{self.__class__.__name__}' object has no attribute '{key}'"
+            )
+
+
+@patch('openhands.llm.llm.litellm_completion')
+def test_guess_success_rate_limit_wait_time(mock_litellm_completion, default_config):
+    """Test that the retry mechanism in guess_success respects wait time between retries."""
+
+    with patch('time.sleep') as mock_sleep:
+        # Simulate a rate limit error followed by a successful response
+        mock_litellm_completion.side_effect = [
+            RateLimitError(
+                'Rate limit exceeded', llm_provider='test_provider', model='test_model'
+            ),
+            DotDict(
+                {
+                    'choices': [
+                        {
+                            'message': {
+                                'content': '--- success\ntrue\n--- explanation\nRetry successful'
+                            }
+                        }
+                    ]
+                }
+            ),
+        ]
+
+        llm = LLM(config=default_config)
+        handler = IssueHandler('test-owner', 'test-repo', 'test-token', default_config)
+        handler.llm = llm
+
+        # Mock issue and history
+        issue = GithubIssue(
+            owner='test-owner',
+            repo='test-repo',
+            number=1,
+            title='Test Issue',
+            body='This is a test issue.',
+            thread_comments=['Please improve error handling'],
+        )
+        history = [MessageAction(content='Fixed error handling.')]
+
+        # Call guess_success
+        success, _, explanation = handler.guess_success(issue, history)
+
+        # Assertions
+        assert success is True
+        assert explanation == 'Retry successful'
+        assert mock_litellm_completion.call_count == 2  # Two attempts made
+        mock_sleep.assert_called_once()  # Sleep called once between retries
+
+        # Validate wait time
+        wait_time = mock_sleep.call_args[0][0]
+        assert (
+            default_config.retry_min_wait <= wait_time <= default_config.retry_max_wait
+        ), f'Expected wait time between {default_config.retry_min_wait} and {default_config.retry_max_wait} seconds, but got {wait_time}'
+
+
+@patch('openhands.llm.llm.litellm_completion')
+def test_guess_success_exhausts_retries(mock_completion, default_config):
+    """Test the retry mechanism in guess_success exhausts retries and raises an error."""
+    # Simulate persistent rate limit errors by always raising RateLimitError
+    mock_completion.side_effect = RateLimitError(
+        'Rate limit exceeded', llm_provider='test_provider', model='test_model'
+    )
+
+    # Initialize LLM and handler
+    llm = LLM(config=default_config)
+    handler = PRHandler('test-owner', 'test-repo', 'test-token', default_config)
+    handler.llm = llm
+
+    # Mock issue and history
+    issue = GithubIssue(
+        owner='test-owner',
+        repo='test-repo',
+        number=1,
+        title='Test Issue',
+        body='This is a test issue.',
+        thread_comments=['Please improve error handling'],
+    )
+    history = [MessageAction(content='Fixed error handling.')]
+
+    # Call guess_success and expect it to raise an error after retries
+    with pytest.raises(RateLimitError):
+        handler.guess_success(issue, history)
+
+    # Assertions
+    assert (
+        mock_completion.call_count == default_config.num_retries
+    )  # Initial call + retries