From af9bdaac8dde09c1f93f77355311af3af7221ddb Mon Sep 17 00:00:00 2001 From: Don Garrett Date: Tue, 8 Sep 2026 00:18:32 +0000 Subject: [PATCH] Stop CI running two suites at once, and retry rate limits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI failed on the delegation branch with a 429 from the proxy: token limit 200000, 17399 remaining. Not a defect — the live delegation tests are token-hungry, and every push to a PR branch fired two identical suites concurrently, since the push and pull_request events both matched. - Push now only triggers on main. A PR branch is already covered by the pull_request event, so this halves both the wall time and the tokens spent per branch - A 429 is retried once before being surfaced. Losing a ten-turn delegation to a transient limit wastes far more than waiting out the window, and retrying forever would hide a real ceiling. The sleep is injectable, so the existing 429 test no longer waits 20 seconds for real Co-Authored-By: Claude Opus 5 --- .gitea/workflows/ci.yml | 5 +++++ nisse-dev/src/nisse_dev/llm.py | 41 +++++++++++++++++++++++++++------- nisse-dev/tests/test_llm.py | 32 +++++++++++++++++++++++--- 3 files changed, 67 insertions(+), 11 deletions(-) diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index cad44d9..6f7d82c 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -1,6 +1,11 @@ name: CI on: + # Only main on push. A PR branch is already covered by the pull_request + # event, and running both fired two identical suites concurrently — which + # doubled the LLM tokens the live tests spend and tripped the proxy's + # per-minute limit. push: + branches: [main] pull_request: # Manual re-run, e.g. after rotating a credential or raising a budget cap. workflow_dispatch: diff --git a/nisse-dev/src/nisse_dev/llm.py b/nisse-dev/src/nisse_dev/llm.py index 269eece..55f6ac1 100644 --- a/nisse-dev/src/nisse_dev/llm.py +++ b/nisse-dev/src/nisse_dev/llm.py @@ -12,6 +12,7 @@ config change cannot split a task across two planners. import json import os +import time from collections.abc import Mapping from dataclasses import asdict, dataclass from typing import Any, ClassVar, Protocol @@ -33,6 +34,14 @@ class LLMError(RuntimeError): pass +# A rate limit is a transient condition, not a bad request. Losing a +# ten-turn delegation to one wastes far more tokens than waiting out the +# window, so retry once before surfacing it (A13's caps still bound the +# total). +RATE_LIMIT_STATUS = 429 +RATE_LIMIT_RETRY_S = 20.0 + + class LLMClient(Protocol): def complete( self, @@ -82,6 +91,7 @@ class LiteLLMClient: *, timeout_s: float = 120.0, transport: httpx.BaseTransport | None = None, + sleep: Any = time.sleep, ) -> None: self._http = httpx.Client( base_url=base_url.rstrip("/"), @@ -90,6 +100,7 @@ class LiteLLMClient: transport=transport, ) self._renderer = OpenAIChatRenderer() + self._sleep = sleep @classmethod def from_env(cls, env: Mapping[str, str] = os.environ) -> "LiteLLMClient": @@ -107,10 +118,9 @@ class LiteLLMClient: system: str | None = None, ) -> list[CanonicalEvent]: body = self._renderer.render(events, tools, model, system=system) - try: - response = self._http.post("/chat/completions", json=body) - except httpx.HTTPError as exc: - raise LLMError(f"LLM request failed: {exc}") from exc + response = _post_with_rate_limit_retry( + self._http, "/chat/completions", body, self._sleep + ) if response.status_code != 200: raise LLMError( f"LLM request returned {response.status_code}: {response.text[:500]}" @@ -176,6 +186,7 @@ class AnthropicPassthroughClient: timeout_s: float = 300.0, max_tokens: int = 4096, transport: httpx.BaseTransport | None = None, + sleep: Any = time.sleep, ) -> None: self._http = httpx.Client( base_url=base_url.rstrip("/"), @@ -189,6 +200,7 @@ class AnthropicPassthroughClient: ) self._renderer = AnthropicMessagesRenderer() self._max_tokens = max_tokens + self._sleep = sleep @classmethod def from_env( @@ -210,10 +222,9 @@ class AnthropicPassthroughClient: body = self._renderer.render( events, tools, model, system=system, max_tokens=self._max_tokens ) - try: - response = self._http.post("/v1/messages", json=body) - except httpx.HTTPError as exc: - raise LLMError(f"passthrough request failed: {exc}") from exc + response = _post_with_rate_limit_retry( + self._http, "/v1/messages", body, self._sleep + ) if response.status_code != 200: raise LLMError( f"passthrough returned {response.status_code}: {response.text[:500]}" @@ -251,3 +262,17 @@ class AnthropicPassthroughClient: def close(self) -> None: self._http.close() + + +def _post_with_rate_limit_retry( + http: httpx.Client, path: str, body: dict[str, Any], sleep: Any +) -> httpx.Response: + for attempt in (1, 2): + try: + response = http.post(path, json=body) + except httpx.HTTPError as exc: + raise LLMError(f"LLM request failed: {exc}") from exc + if response.status_code != RATE_LIMIT_STATUS or attempt == 2: + return response + sleep(RATE_LIMIT_RETRY_S) + raise AssertionError("unreachable") diff --git a/nisse-dev/tests/test_llm.py b/nisse-dev/tests/test_llm.py index 5f5dc41..25706f9 100644 --- a/nisse-dev/tests/test_llm.py +++ b/nisse-dev/tests/test_llm.py @@ -11,16 +11,17 @@ from nisse_dev.canonical import ( UserMessage, ) from nisse_dev.envfile import load_env_file -from nisse_dev.llm import LiteLLMClient, LLMError, RoleMap +from nisse_dev.llm import RATE_LIMIT_RETRY_S, LiteLLMClient, LLMError, RoleMap EVENTS = [StoredEvent(id=1, ts="t", event=UserMessage(text="hi", source="m:1"))] -def make_client(handler) -> LiteLLMClient: +def make_client(handler, sleep=lambda _s: None) -> LiteLLMClient: return LiteLLMClient( "http://litellm.test/v1", "sk-test", transport=httpx.MockTransport(handler), + sleep=sleep, # never wait in unit tests ) @@ -109,9 +110,34 @@ def test_non_object_tool_arguments_raise_llm_error() -> None: def test_http_error_status_raises_llm_error() -> None: - client = make_client(lambda request: httpx.Response(429, text="rate limited")) + client = make_client(lambda request: httpx.Response(500, text="boom")) + with pytest.raises(LLMError, match="500"): + client.complete(EVENTS, tools=None, model="local/qwen") + + +def test_rate_limit_is_retried_once_then_surfaced() -> None: + # A 429 is transient. Losing a long delegation to one costs far more + # than waiting out the window, but retrying forever would hide a real + # ceiling, so it is retried exactly once. + attempts: list[int] = [] + waited: list[float] = [] + + def handler(request: httpx.Request) -> httpx.Response: + attempts.append(1) + return httpx.Response(429, text="rate limited") + + client = make_client(handler, sleep=waited.append) with pytest.raises(LLMError, match="429"): client.complete(EVENTS, tools=None, model="local/qwen") + assert len(attempts) == 2 + assert waited == [RATE_LIMIT_RETRY_S] + + +def test_rate_limit_that_clears_succeeds() -> None: + responses = [httpx.Response(429, text="slow down"), completion({"content": "hi"})] + client = make_client(lambda request: responses.pop(0), sleep=lambda _s: None) + [event] = client.complete(EVENTS, tools=None, model="local/qwen") + assert isinstance(event, AssistantText) def test_connection_failure_raises_llm_error() -> None: