From 72862ef3bef62deed0a0731dd7fa4d1bad7450a0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Louis-F=C3=A9lix=20Nothias?= <louis-felix.nothias@cnrs.fr>
Date: Fri, 3 Jul 2026 21:58:13 +0200
Subject: [PATCH] fix(llm): enforce max_retries cap on the unbounded retry loop

---
 sources/core/llm_provider.py | 18 ++++++++--
 tests/llm_provider_test.py   | 69 ++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+), 2 deletions(-)
 create mode 100644 tests/llm_provider_test.py

diff --git a/sources/core/llm_provider.py b/sources/core/llm_provider.py
index 1fd9576..d884d1c 100644
--- a/sources/core/llm_provider.py
+++ b/sources/core/llm_provider.py
@@ -193,6 +193,10 @@ def __init__(
         self.agent_name = agent_name
         self.memory_path = memory_path
         self.use_flat_cache = use_flat_cache
+        # Hard cap on transient-error retries in __call__. The retry loop is
+        # otherwise unbounded (`while True`), so a persistently overloaded or
+        # rate-limited provider would retry forever, re-sending the full prompt
+        # each time. Both retryable paths raise once this ceiling is reached.
         self.max_retries = 3
         self.logger = logging.getLogger(__name__)
 
@@ -479,7 +483,11 @@ def __call__(self, prompt: str, timeout: int = 180, use_cache: bool = True) -> s
                 break
 
             except TimeoutError as e:
-                # Timeout is retryable
+                # Timeout is retryable, up to the max_retries ceiling.
+                if attempt >= self.max_retries:
+                    raise RuntimeError(
+                        f"❌ LLM API error: timed out after {self.max_retries} retries"
+                    ) from e
                 wait_time = self._calculate_backoff_wait(attempt, max_wait)
                 self.logger.warning(
                     f"⌛ Timeout on attempt {attempt + 1}. Retrying in {wait_time:.1f}s..."
@@ -517,7 +525,13 @@ def __call__(self, prompt: str, timeout: int = 180, use_cache: bool = True) -> s
                             )
                         attempt += 1
                     else:
-                        # Regular retry with backoff for other retryable errors
+                        # Regular retry with backoff for other retryable errors,
+                        # bounded by the max_retries ceiling so a persistently
+                        # failing provider cannot loop forever.
+                        if attempt >= self.max_retries:
+                            raise RuntimeError(
+                                f"❌ LLM API error after {self.max_retries} retries: {str(e)}"
+                            ) from e
                         wait_time = self._calculate_backoff_wait(attempt, max_wait)
                         self.logger.warning(
                             f"⚠️  Retryable error on attempt {attempt + 1}: {str(e)[:512]}. "
diff --git a/tests/llm_provider_test.py b/tests/llm_provider_test.py
new file mode 100644
index 0000000..ed5b790
--- /dev/null
+++ b/tests/llm_provider_test.py
@@ -0,0 +1,69 @@
+"""Tests for the LLMProvider transient-error retry loop.
+
+Focus: the retry loop in ``LLMProvider.__call__`` is bounded by
+``self.max_retries`` on both retryable paths (timeout and generic retryable
+errors) and can no longer loop forever on a persistently failing provider.
+"""
+
+import sys
+from pathlib import Path
+from unittest.mock import patch
+
+import pytest
+
+sys.path.append(str(Path(__file__).parent.parent))
+
+from sources.core.llm_provider import LLMConfig, LLMProvider
+
+
+def _make_provider() -> LLMProvider:
+    """Build a provider with no network dependencies and no on-disk cache."""
+    return LLMProvider(agent_name="test", config=LLMConfig(model="anthropic/claude-sonnet-4-5"))
+
+
+def test_retry_loop_raises_after_max_retries():
+    """A persistently retryable error terminates instead of looping forever."""
+    provider = _make_provider()
+    retryable = Exception("provider overloaded, please retry")
+
+    with patch("litellm.completion", side_effect=retryable) as completion, patch(
+        "sources.core.llm_provider.time.sleep"
+    ):
+        with pytest.raises(RuntimeError, match="retries"):
+            provider("hello", use_cache=False)
+
+    # attempts 0..max_retries are tried, then the ceiling raises.
+    assert completion.call_count == provider.max_retries + 1
+
+
+def test_timeout_retry_is_bounded():
+    """Repeated timeouts terminate at the max_retries ceiling."""
+    provider = _make_provider()
+
+    with patch("litellm.completion", side_effect=TimeoutError("slow")) as completion, patch(
+        "sources.core.llm_provider.time.sleep"
+    ):
+        with pytest.raises(RuntimeError, match="timed out"):
+            provider("hello", use_cache=False)
+
+    assert completion.call_count == provider.max_retries + 1
+
+
+def test_non_retryable_error_raises_immediately():
+    """A non-retryable error is raised on the first attempt, with no retry."""
+    provider = _make_provider()
+
+    with patch("litellm.completion", side_effect=ValueError("bad request")) as completion, patch(
+        "sources.core.llm_provider.time.sleep"
+    ):
+        with pytest.raises(RuntimeError):
+            provider("hello", use_cache=False)
+
+    assert completion.call_count == 1
+
+
+if __name__ == "__main__":
+    test_retry_loop_raises_after_max_retries()
+    test_timeout_retry_is_bounded()
+    test_non_retryable_error_raises_immediately()
+    print("llm_provider retry tests passed")