fix(review-feedback-1002): address latest review comments

Author: ZhuLinsen

src/agent/llm_adapter.py

@@ -327,7 +327,13 @@ def call_completion(
                 hit_rate_limit = True
                 # Brief backoff before trying the next model; avoids hammering
                 # the same provider when multiple models share one account.
-                time.sleep(min(2.0, (time.time() - started_at) * 0.1 + 0.5))
+                backoff_sleep = min(2.0, (time.time() - started_at) * 0.1 + 0.5)
+                if timeout is not None and timeout > 0:
+                    remaining_timeout = max(0.0, float(timeout) - (time.time() - started_at))
+                    if remaining_timeout > 0:
+                        time.sleep(min(backoff_sleep, remaining_timeout))
+                else:
+                    time.sleep(backoff_sleep)
                 continue
             except litellm.ContextWindowExceededError as e:
                 logger.warning("Agent LLM context window exceeded on %s: %s", model, e)
@@ -338,7 +344,7 @@ def call_completion(
                 last_error = e
                 continue
 
-        suffix = " (rate-limited)" if hit_rate_limit else ""
+        suffix = " (rate-limit encountered during fallback)" if hit_rate_limit else ""
         error_msg = f"All LLM models failed{suffix}. Last error: {last_error}"
         logger.error(error_msg)
         return LLMResponse(content=error_msg, provider="error")

tests/test_agent_pipeline.py

@@ -861,6 +861,134 @@ def fake_call(_messages, _tools, model, **kwargs):
         self.assertEqual(timeouts[0], ("openai/gpt-4o-mini", 10.0))
         self.assertEqual(timeouts[1], ("anthropic/claude-3-5-sonnet-20241022", 3.0))
 
+    @patch("src.agent.llm_adapter.Router")
+    def test_llm_adapter_rate_limit_backoff_is_bounded_by_remaining_timeout(self, _mock_router):
+        """Rate-limit backoff should sleep, but never longer than the remaining timeout budget."""
+        mock_cfg = MagicMock()
+        mock_cfg.agent_litellm_model = "gpt-4o-mini"
+        mock_cfg.litellm_model = None
+        mock_cfg.litellm_fallback_models = ["anthropic/claude-3-5-sonnet-20241022"]
+        mock_cfg.llm_model_list = []
+        mock_cfg.llm_temperature = 0.7
+        mock_cfg.gemini_api_keys = []
+        mock_cfg.anthropic_api_keys = []
+        mock_cfg.openai_api_keys = []
+        mock_cfg.deepseek_api_keys = []
+        mock_cfg.openai_base_url = None
+
+        from src.agent.llm_adapter import LLMToolAdapter
+        adapter = LLMToolAdapter(config=mock_cfg)
+
+        class FakeRateLimitError(Exception):
+            pass
+
+        timeouts = []
+
+        def fake_call(_messages, _tools, model, **kwargs):
+            timeouts.append((model, kwargs.get("timeout")))
+            if model == "openai/gpt-4o-mini":
+                raise FakeRateLimitError("rate limited")
+            return MagicMock(content="ok")
+
+        adapter._call_litellm_model = MagicMock(side_effect=fake_call)
+
+        with patch("src.agent.llm_adapter.litellm.RateLimitError", FakeRateLimitError), \
+             patch("src.agent.llm_adapter.logger.warning"), \
+             patch("src.agent.llm_adapter.time.sleep") as mock_sleep, \
+             patch("src.agent.llm_adapter.time.time", side_effect=[0.0, 0.0, 8.0, 8.8, 9.2]):
+            result = adapter.call_completion(
+                messages=[{"role": "user", "content": "hi"}],
+                tools=[],
+                timeout=10.0,
+            )
+
+        self.assertEqual(result.content, "ok")
+        self.assertEqual(timeouts[0], ("openai/gpt-4o-mini", 10.0))
+        self.assertEqual(timeouts[1][0], "anthropic/claude-3-5-sonnet-20241022")
+        self.assertAlmostEqual(timeouts[1][1], 0.8)
+        mock_sleep.assert_called_once()
+        self.assertAlmostEqual(mock_sleep.call_args.args[0], 1.2)
+
+    @patch("src.agent.llm_adapter.Router")
+    def test_llm_adapter_context_window_error_skips_sleep(self, _mock_router):
+        """Context-window errors should continue fallback immediately without backoff."""
+        mock_cfg = MagicMock()
+        mock_cfg.agent_litellm_model = "gpt-4o-mini"
+        mock_cfg.litellm_model = None
+        mock_cfg.litellm_fallback_models = ["anthropic/claude-3-5-sonnet-20241022"]
+        mock_cfg.llm_model_list = []
+        mock_cfg.llm_temperature = 0.7
+        mock_cfg.gemini_api_keys = []
+        mock_cfg.anthropic_api_keys = []
+        mock_cfg.openai_api_keys = []
+        mock_cfg.deepseek_api_keys = []
+        mock_cfg.openai_base_url = None
+
+        from src.agent.llm_adapter import LLMToolAdapter
+        adapter = LLMToolAdapter(config=mock_cfg)
+
+        class FakeContextWindowExceededError(Exception):
+            pass
+
+        def fake_call(_messages, _tools, model, **_kwargs):
+            if model == "openai/gpt-4o-mini":
+                raise FakeContextWindowExceededError("window exceeded")
+            return MagicMock(content="ok")
+
+        adapter._call_litellm_model = MagicMock(side_effect=fake_call)
+
+        with patch(
+            "src.agent.llm_adapter.litellm.ContextWindowExceededError",
+            FakeContextWindowExceededError,
+        ), patch("src.agent.llm_adapter.time.sleep") as mock_sleep:
+            result = adapter.call_completion(messages=[{"role": "user", "content": "hi"}], tools=[])
+
+        self.assertEqual(result.content, "ok")
+        mock_sleep.assert_not_called()
+
+    @patch("src.agent.llm_adapter.Router")
+    def test_llm_adapter_reports_rate_limit_suffix_when_any_fallback_hit_limit(self, _mock_router):
+        """Final error should note earlier rate limiting even if the last error differs."""
+        mock_cfg = MagicMock()
+        mock_cfg.agent_litellm_model = "gpt-4o-mini"
+        mock_cfg.litellm_model = None
+        mock_cfg.litellm_fallback_models = ["anthropic/claude-3-5-sonnet-20241022"]
+        mock_cfg.llm_model_list = []
+        mock_cfg.llm_temperature = 0.7
+        mock_cfg.gemini_api_keys = []
+        mock_cfg.anthropic_api_keys = []
+        mock_cfg.openai_api_keys = []
+        mock_cfg.deepseek_api_keys = []
+        mock_cfg.openai_base_url = None
+
+        from src.agent.llm_adapter import LLMToolAdapter
+        adapter = LLMToolAdapter(config=mock_cfg)
+
+        class FakeRateLimitError(Exception):
+            pass
+
+        class FakeContextWindowExceededError(Exception):
+            pass
+
+        def fake_call(_messages, _tools, model, **_kwargs):
+            if model == "openai/gpt-4o-mini":
+                raise FakeRateLimitError("rate limited")
+            raise FakeContextWindowExceededError("window exceeded")
+
+        adapter._call_litellm_model = MagicMock(side_effect=fake_call)
+
+        with patch("src.agent.llm_adapter.litellm.RateLimitError", FakeRateLimitError), \
+             patch(
+                 "src.agent.llm_adapter.litellm.ContextWindowExceededError",
+                 FakeContextWindowExceededError,
+             ), \
+             patch("src.agent.llm_adapter.time.sleep"):
+            result = adapter.call_completion(messages=[{"role": "user", "content": "hi"}], tools=[])
+
+        self.assertEqual(result.provider, "error")
+        self.assertIn("All LLM models failed (rate-limit encountered during fallback).", result.content)
+        self.assertIn("window exceeded", result.content)
+
 
 # ============================================================
 # _safe_int tests