From 3ebcc6729ae5b80f568980b5499c1a26cc6f4286 Mon Sep 17 00:00:00 2001 From: Marius Ciepluch <11855163+norandom@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:41 +0200 Subject: [PATCH] fix: a body that stops mid-read is transient, and a round that dies still says so Round 5 died on `http.client.IncompleteRead(0 bytes read)` and left no record at all. Two defects, both ours. The retry classified only connection-level failures. IncompleteRead and RemoteDisconnected arrive while the *body* is being read, after the connection succeeded, so they are not URLError -- they escaped both the retry and the error wrapper and walked out of the round. The network is allowed to be unreliable; losing a paid round to it is not the provider's defect. And the exception path was wrapped around the stages that come *after* the proposal is applied, because those are the ones with something to revert. The stages before it had already spent money, and a round that leaves no record is a round nobody can account for. Every outcome is journalled now, with the exception type in the reason. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG --- src/openultrasast/learning/rounds.py | 25 +++++++++--- src/openultrasast/provider/openrouter.py | 17 +++++--- tests/test_learning_round_evidence.py | 22 ++++++++++- tests/test_provider_retry.py | 50 ++++++++++++++++++++++++ 4 files changed, 102 insertions(+), 12 deletions(-) diff --git a/src/openultrasast/learning/rounds.py b/src/openultrasast/learning/rounds.py index b91003e..89b8bd0 100644 --- a/src/openultrasast/learning/rounds.py +++ b/src/openultrasast/learning/rounds.py @@ -406,10 +406,25 @@ def run_learning_round( spent = spent_usd() if spent_usd is not None else 0.0 return not within_budget(spent, cost_cap_usd) - before_scan = scan_factory(config) - before_train = _score(batch, before_scan, family) - before_hold = _score(holdout, before_scan, family) record = _round_record(round_number, family, None, taxonomy, config) + try: + before_scan = scan_factory(config) + before_train = _score(batch, before_scan, family) + before_hold = _score(holdout, before_scan, family) + except Exception as exc: + # These stages run before anything is applied, so there is nothing to revert — but they have already spent + # money, and a round that leaves no record is a round nobody can account for. Measured: round 5 died on a + # network read during its first stage and vanished. + _finish( + journal, + directory, + record, + outcome="reverted", + reason=f"stage_failed: {type(exc).__name__}", + cost=spent_usd() if spent_usd is not None else 0.0, + trajectories=trajectories, + ) + raise if _over_cap(): return _finish( journal, directory, record, outcome="reverted_cost", reason="cost_cap_exceeded", cost=spent, trajectories=trajectories @@ -551,7 +566,7 @@ def run_learning_round( version=version, trajectories=trajectories, ) - except Exception: + except Exception as exc: # A stage that raises has already spent money and already changed the tree. Both facts belong in the # journal before the exception leaves, or the next round starts from a state nothing describes. snapshot.restore() @@ -560,7 +575,7 @@ def run_learning_round( directory, record, outcome="reverted", - reason="stage_failed", + reason=f"stage_failed: {type(exc).__name__}", proposal=proposal, cost=spent, trajectories=trajectories, diff --git a/src/openultrasast/provider/openrouter.py b/src/openultrasast/provider/openrouter.py index ab26fd2..c2e6551 100644 --- a/src/openultrasast/provider/openrouter.py +++ b/src/openultrasast/provider/openrouter.py @@ -1,5 +1,6 @@ from __future__ import annotations +import http.client import json import os import re @@ -20,10 +21,14 @@ _T = TypeVar("_T") def _is_transient(exc: Exception) -> bool: - """A network failure worth retrying (rate-limit, 5xx, connection error, timeout).""" + """A network failure worth retrying: rate-limit, 5xx, connection error, timeout, or a body that stopped. + + `http.client.IncompleteRead` and `RemoteDisconnected` arrive while the *body* is being read, after the + connection succeeded, so they are not `URLError`. Measured: one of them ended an evolve round that had already + been paid for.""" if isinstance(exc, urllib.error.HTTPError): return exc.code in _TRANSIENT_STATUS - return isinstance(exc, urllib.error.URLError | TimeoutError) + return isinstance(exc, urllib.error.URLError | TimeoutError | http.client.HTTPException) def call_with_retry( @@ -108,8 +113,8 @@ class OpenRouterChatClient: try: return call_with_retry(_do, attempts=self.max_attempts, base_delay=self.retry_base_delay) - except (urllib.error.URLError, TimeoutError, json.JSONDecodeError) as exc: - raise OpenRouterError(f"OpenRouter chat request failed: {exc}") from exc + except (urllib.error.URLError, TimeoutError, http.client.HTTPException, json.JSONDecodeError) as exc: + raise OpenRouterError(f"OpenRouter chat request failed: {type(exc).__name__}: {exc}") from exc def complete_json(self, *, model: str, messages: list[dict[str, str]], timeout_seconds: int = 60) -> object: message = self.complete_chat(model=model, messages=messages, timeout_seconds=timeout_seconds) @@ -151,8 +156,8 @@ class OpenRouterEmbeddingClient: try: response_payload = call_with_retry(_do, attempts=self.max_attempts, base_delay=self.retry_base_delay) - except (urllib.error.URLError, TimeoutError, json.JSONDecodeError) as exc: - raise OpenRouterError(f"OpenRouter embedding request failed: {exc}") from exc + except (urllib.error.URLError, TimeoutError, http.client.HTTPException, json.JSONDecodeError) as exc: + raise OpenRouterError(f"OpenRouter embedding request failed: {type(exc).__name__}: {exc}") from exc return parse_embedding_response(response_payload) diff --git a/tests/test_learning_round_evidence.py b/tests/test_learning_round_evidence.py index b6a1d1c..5775f25 100644 --- a/tests/test_learning_round_evidence.py +++ b/tests/test_learning_round_evidence.py @@ -224,7 +224,7 @@ def test_a_stage_that_raises_is_reverted_journalled_and_re_raised(world) -> None _run(world, _proposal(), build) assert (world["configs"] / "injection" / "checklist.md").read_text() == "- old\n" outcomes = [(item.outcome, item.reason) for item in world["journal"].rounds()] - assert outcomes == [("reverted", "stage_failed")] + assert outcomes == [("reverted", "stage_failed: RuntimeError")] # the exception type is part of the record assert (world["out"] / "rounds" / "1" / "scores.json").is_file() @@ -406,3 +406,23 @@ def test_a_round_journals_why_the_proposer_had_nothing_to_say(world) -> None: # ) assert record.outcome == "rejected" assert record.reason == "no_proposal: meta_agent_failed: RuntimeError" + + +def test_a_round_that_dies_before_it_applies_anything_is_still_journalled(world) -> None: # type: ignore[no-untyped-def] + """Measured: round 5 died on a network read during its *first* stage and left no record at all. + + The exception path was wrapped around the stages after the proposal is applied, because those are the ones that + have something to revert. The ones before it had already spent money.""" + + def build(config: FamilyConfig): # type: ignore[no-untyped-def] + def scan(root: Path) -> list[StaticFinding]: + raise ConnectionError("the body stopped arriving") + + return scan + + with pytest.raises(ConnectionError): + _run(world, _proposal(), build) + outcomes = [(item.outcome, item.reason) for item in world["journal"].rounds()] + assert outcomes == [("reverted", "stage_failed: ConnectionError")] + assert (world["out"] / "rounds" / "1").is_dir() + assert (world["configs"] / "injection" / "checklist.md").read_text() == "- old\n" diff --git a/tests/test_provider_retry.py b/tests/test_provider_retry.py index f4fc1c9..eadf149 100644 --- a/tests/test_provider_retry.py +++ b/tests/test_provider_retry.py @@ -63,3 +63,53 @@ def test_non_retryable_value_error_propagates_immediately() -> None: with pytest.raises(OpenRouterError): call_with_retry(op, attempts=3, base_delay=0, sleep=_no_sleep) + + +def test_a_body_that_stops_mid_read_is_transient() -> None: + """Measured: an evolve round died on `http.client.IncompleteRead(0 bytes read)`. + + The retry classified only connection-level failures, so a response whose *body* stopped arriving escaped both + the retry and the error wrapper, and the exception walked out of the round. The network is allowed to be + unreliable; losing a paid round to it is our defect, not the provider's.""" + import http.client + + from openultrasast.provider.openrouter import OpenRouterError, _is_transient, call_with_retry + + assert _is_transient(http.client.IncompleteRead(b"")) is True + assert _is_transient(http.client.RemoteDisconnected("closed")) is True + assert _is_transient(ValueError("a bug in our own code")) is False + + attempts = [] + + def flaky() -> str: + attempts.append(1) + if len(attempts) < 3: + raise http.client.IncompleteRead(b"") + return "ok" + + assert call_with_retry(flaky, attempts=3, base_delay=0.0, sleep=lambda _s: None) == "ok" + assert len(attempts) == 3 + assert issubclass(OpenRouterError, Exception) + + +def test_a_read_failure_is_reported_as_a_provider_error_not_a_raw_http_exception() -> None: + import http.client + import urllib.error + + from openultrasast.provider.openrouter import OpenRouterChatClient, OpenRouterError + + client = OpenRouterChatClient(api_key="k", base_url="https://example.invalid", max_attempts=1) + + def boom(*_args: object, **_kwargs: object) -> object: + raise http.client.IncompleteRead(b"") + + import openultrasast.provider.openrouter as module + + original = module.urllib.request.urlopen + module.urllib.request.urlopen = boom # type: ignore[assignment] + try: + with pytest.raises(OpenRouterError, match="IncompleteRead"): + client.complete_chat_raw(model="m", messages=[{"role": "user", "content": "x"}]) + finally: + module.urllib.request.urlopen = original # type: ignore[assignment] + assert urllib.error.URLError is not None