Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ to follow semantic versioning.

## [Unreleased]

- CodeAct and PurePython `max_retries` now count only consecutive LLM API
errors; a successful LLM call resets the count. Other errors, such as
invalid tool calls and failed result validation, still add up across the
call. Previously a few scattered transient endpoint errors could end a long
turn even though every retry succeeded.
- Dependency security update: the locked versions of Pillow, PyJWT,
cryptography, aiohttp, urllib3, AnyIO, filelock, multidict, Werkzeug,
setuptools, certifi, OpenTelemetry, and mcp now include their published
Expand Down
2 changes: 1 addition & 1 deletion skills/nooa-codeact-advanced/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class Notifier(Agent, llm=llm):
| Knob | Default | Verified semantics |
|---|---|---|
| `max_iterations` | `None` | **Unlimited.** The loop then stops only on completion, the error budget, or a hard abort. |
| `max_retries` | `3` | **Cumulative session error budget, not consecutive** (the counter is never reset). LLM API errors, bad tool JSON, empty code, and `return_result` validation failures all count. |
| `max_retries` | `3` | Error budget with two counters. **LLM API errors count only while consecutive**: any model response (including a malformed one) resets them. **Other errors are cumulative** across the call (never reset): bad tool JSON, unknown tools, empty code, empty replies, and `return_result` validation failures. Either counter reaching `max_retries` fails the call. |
| `max_consecutive_text_only` | `3` | Consecutive no-tool-call text replies before hard abort; `0` disables. Any real tool call resets the counter. |
| `cell_timeout` | `None` | Per-cell `asyncio.wait_for` limit in seconds; `None` = unlimited. Cannot interrupt a truly blocking sync syscall — that's what the blocking-call AST validation is for. |
| `max_tokens` / `temperature` / `top_p` | `None` | Passed to every generation call when set (model defaults otherwise). On empty responses with `finish_reason="length"` CodeAct aborts and tells you to raise `max_tokens` (16384+ for reasoning models). |
Expand Down
3 changes: 3 additions & 0 deletions src/nooa/config/strategy_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ class CodeActConfig(BaseModel):
model_config = ConfigDict(frozen=True, arbitrary_types_allowed=True)

max_iterations: int | None = None
# Error budget per call. LLM API errors are counted only while
# consecutive (a successful LLM call resets them); other errors such as
# invalid tool calls and failed result validation accumulate.
max_retries: int = 3
# Maximum consecutive turns where the LLM returns plain text instead of a
# tool call before the run is aborted. A real tool call resets the counter.
Expand Down
34 changes: 28 additions & 6 deletions src/nooa/strategies/codeact.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ class CodeActSession:
event_manager: Any # EventManager reference for Out[n] access
iteration: int = 0
error_count: int = 0
consecutive_api_errors: int = 0
consecutive_text_only: int = 0
session_locals: dict[str, Any] = field(default_factory=dict)
out_accessor: Any = field(default=None) # OutAccessor instance, created lazily
Expand Down Expand Up @@ -302,6 +303,18 @@ def record_execution(self) -> int:
def record_error(self) -> None:
self.error_count += 1

def record_api_error(self) -> None:
"""Count an LLM API error toward the consecutive-failure budget."""
self.consecutive_api_errors += 1

def reset_api_errors(self) -> None:
"""Clear the API-error count after the model responds."""
self.consecutive_api_errors = 0

def api_errors_exhausted(self) -> bool:
"""Whether consecutive LLM API errors have used up ``max_retries``."""
return self.consecutive_api_errors >= self.max_retries

def record_text_only(self) -> None:
self.consecutive_text_only += 1

Expand Down Expand Up @@ -410,7 +423,7 @@ class CodeActStrategy(CompositeStrategy):

Configuration (fields of CodeActConfig, passed via config=):
max_iterations: Maximum number of tool call iterations
max_retries: Maximum consecutive errors before failure
max_retries: Error budget; LLM API errors count only while consecutive

Example:
@strategy(CodeActStrategy())
Expand Down Expand Up @@ -977,7 +990,9 @@ async def _run_generation(

logger.debug(
f"[CODEACT] Loop iteration: iter={session.iteration}/{session.max_iterations}, "
f"err={session.error_count}/{session.max_retries}, tool_choice={tool_choice}"
f"err={session.error_count}/{session.max_retries}, "
f"api_err={session.consecutive_api_errors}/{session.max_retries}, "
f"tool_choice={tool_choice}"
)

response = None
Expand All @@ -994,6 +1009,8 @@ async def _run_generation(
turn_state.is_final = True
raise
except BlockSyntaxError as e:
# Raised while building the prompt, before any LLM request,
# so it neither counts as nor clears an API error.
self._handle_block_syntax_error(e, session, runtime)
continue

Expand All @@ -1002,7 +1019,10 @@ async def _run_generation(
# Note: unifiedllm already has retry logic with exponential backoff
# for 429, 500, 502, 503, 504 errors. This handles cases where
# all retries are exhausted or the error is not retryable.
session.record_error()
# Only consecutive failures count: a transient endpoint error
# followed by a successful call must not eat into the budget
# for the rest of a long turn.
session.record_api_error()
error_name = type(e).__name__
cause_parts = []
exc: BaseException | None = e
Expand All @@ -1013,18 +1033,19 @@ async def _run_generation(
exc = exc.__cause__ or exc.__context__
cause_chain = " <- ".join(cause_parts)
error_msg = (
f"LLM API error (attempt {session.error_count}/{session.max_retries}): "
f"LLM API error (attempt {session.consecutive_api_errors}/{session.max_retries}): "
f"{cause_chain}"
)
get_harness_metrics().llm_api_error(cause_chain[:500])
runtime.event_manager.add(Error(content=error_msg))
logger.warning(
f"[CODEACT] LLM API error (iter={session.iteration}, err={session.error_count}): "
f"[CODEACT] LLM API error (iter={session.iteration}, "
f"api_err={session.consecutive_api_errors}): "
f"{cause_chain}",
exc_info=True,
)
turn_state.exception = error_name
if session.is_exhausted():
if session.api_errors_exhausted():
turn_state.is_final = True
raise GenerationError(
f"LLM API error after {session.max_retries} retries. "
Expand All @@ -1034,6 +1055,7 @@ async def _run_generation(
# Skip rest of turn if LLM call failed
if response is None:
continue
session.reset_api_errors()

# Output-limit responses are incomplete even when they carry
# partial text. Preserve the exact LLMResponse for diagnostics,
Expand Down
52 changes: 37 additions & 15 deletions src/nooa/strategies/pure_python.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ class GenerationSession:
iteration: int = 0
execution_count: int = 0
error_count: int = 0
consecutive_api_errors: int = 0
session_locals: dict[str, Any] = field(default_factory=dict)
task_event_id: str = ""
out_accessor: Any = field(default=None) # OutAccessor instance, created lazily
Expand Down Expand Up @@ -112,6 +113,18 @@ def record_execution(self) -> int:
def record_error(self) -> None:
self.error_count += 1

def record_api_error(self) -> None:
"""Count an LLM API error toward the consecutive-failure budget."""
self.consecutive_api_errors += 1

def reset_api_errors(self) -> None:
"""Clear the API-error count after the model responds."""
self.consecutive_api_errors = 0

def api_errors_exhausted(self) -> bool:
"""Whether consecutive LLM API errors have used up ``max_retries``."""
return self.consecutive_api_errors >= self.max_retries

def record_output(self, execution_count: int, value: Any) -> None:
"""Record an execution output for Out[n] access."""
if self.out_accessor is not None:
Expand Down Expand Up @@ -311,27 +324,31 @@ async def execute(self, runtime: RuntimeServices, call: "CurrentCall") -> Any:
turn_exception = type(e).__name__
raise
except _HTTPX_TIMEOUT_EXCEPTIONS as e:
# Catch httpx timeout exceptions and preserve them
session.record_error()
# Catch httpx timeout exceptions and preserve them. Like other
# LLM API errors, timeouts count only while consecutive.
session.record_api_error()
error_msg = (
f"LLM request timed out (attempt {session.error_count}/{session.max_retries}): "
f"{type(e).__name__}: {e}"
f"LLM request timed out (attempt {session.consecutive_api_errors}/"
f"{session.max_retries}): {type(e).__name__}: {e}"
)
runtime.event_manager.add(Error(content=error_msg))
logger.warning(
f"[PURE_PYTHON] LLM timeout (iter={session.iteration}, err={session.error_count}): {e}"
f"[PURE_PYTHON] LLM timeout (iter={session.iteration}, "
f"api_err={session.consecutive_api_errors}): {e}"
)
turn_exception = type(e).__name__

if session.is_exhausted():
if session.api_errors_exhausted():
turn_final = True
turn_exception = "GenerationError"
raise GenerationError(
f"LLM request timed out after {session.max_retries} retries. "
f"Original error: {type(e).__name__}: {e}"
) from e
except XMLFormatError as e:
# XML format error - already logged in _generate_code, just count as error
# XML format error - already logged in _generate_code, just count as error.
# The model responded, so the endpoint is healthy.
session.reset_api_errors()
session.record_error()
logger.warning(
f"[PURE_PYTHON] XML format error (iter={session.iteration}, err={session.error_count})"
Expand All @@ -344,8 +361,11 @@ async def execute(self, runtime: RuntimeServices, call: "CurrentCall") -> Any:
self._sync_session_locals(call, session)
raise session.build_failure_error() from None
except Exception as e:
# Catch other LLM API errors (rate limits, connection errors, etc.)
session.record_error()
# Catch other LLM API errors (rate limits, connection errors, etc.).
# Only consecutive failures count: a transient endpoint error
# followed by a successful call must not eat into the budget
# for the rest of a long run.
session.record_api_error()
error_name = type(e).__name__
cause_parts = []
exc: BaseException | None = e
Expand All @@ -356,18 +376,18 @@ async def execute(self, runtime: RuntimeServices, call: "CurrentCall") -> Any:
exc = exc.__cause__ or exc.__context__
cause_chain = " <- ".join(cause_parts)
error_msg = (
f"LLM API error (attempt {session.error_count}/{session.max_retries}): "
f"{cause_chain}"
f"LLM API error (attempt {session.consecutive_api_errors}/"
f"{session.max_retries}): {cause_chain}"
)
runtime.event_manager.add(Error(content=error_msg))
logger.warning(
f"[PURE_PYTHON] LLM API error (iter={session.iteration}, err={session.error_count}): "
f"{cause_chain}",
f"[PURE_PYTHON] LLM API error (iter={session.iteration}, "
f"api_err={session.consecutive_api_errors}): {cause_chain}",
exc_info=True,
)
turn_exception = error_name

if session.is_exhausted():
if session.api_errors_exhausted():
turn_final = True
turn_exception = "GenerationError"
raise GenerationError(
Expand All @@ -378,6 +398,7 @@ async def execute(self, runtime: RuntimeServices, call: "CurrentCall") -> Any:
# Skip rest of turn if code generation failed (caught exception above)
if code is None:
continue
session.reset_api_errors()

if not code:
session.record_error()
Expand Down Expand Up @@ -596,7 +617,8 @@ async def _generate_code(
"""
logger.debug(
f"[PURE_PYTHON] Loop iteration: iter={session.iteration}/{session.max_iterations}, "
f"err={session.error_count}/{session.max_retries}"
f"err={session.error_count}/{session.max_retries}, "
f"api_err={session.consecutive_api_errors}/{session.max_retries}"
)

response, event_id = await runtime.generate(tools=None)
Expand Down
98 changes: 98 additions & 0 deletions tests/strategies/test_error_recovery_gl106.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,24 @@ async def acall(
return await super().acall(messages, tools, output_model, **kwargs)


class FlakyFakeLLM(FakeLLMClient):
"""FakeLLMClient that raises RuntimeError on the given 0-based call indices."""

def __init__(self, scripted_responses: list[LLMResponse], failing_calls: set[int]):
super().__init__(scripted_responses=scripted_responses)
self._failing_calls = failing_calls
self.attempts = 0

async def acall(
self, messages: list[dict], tools=None, output_model=None, **kwargs
) -> LLMResponse:
index = self.attempts
self.attempts += 1
if index in self._failing_calls:
raise RuntimeError("Simulated transient LLM API failure")
return await super().acall(messages, tools, output_model, **kwargs)


class ReasoningReplayFailingLLM(FakeLLMClient):
"""Raise a framework replay error and count attempts."""

Expand Down Expand Up @@ -182,6 +200,33 @@ async def compute(self, x: int) -> int:
# Must mention "LLM API error after 2 retries" — the exact phrase from codeact.py line 697
assert "LLM API error after 2 retries" in err

@pytest.mark.asyncio
async def test_successful_call_resets_api_error_budget(self):
"""Non-consecutive API errors must not add up across a long turn.

Calls 0, 2 and 4 fail and each retry succeeds. With max_retries=2 the
turn must still finish: only consecutive API errors count.
"""

class TestAgent(Agent, llm=_DUMMY_LLM):
@strategy(CodeActStrategy(config=CodeActConfig(max_retries=2, max_iterations=10)))
async def compute(self, x: int) -> int:
"""Compute {x}."""
...

fake_llm = FlakyFakeLLM(
scripted_responses=[
_resp(tool_calls=[_exec_python("y = 1", call_id="c1")]),
_resp(tool_calls=[_exec_python("y = 2", call_id="c2")]),
_resp(tool_calls=[_return_result(42)]),
],
failing_calls={0, 2, 4},
)

agent = TestAgent(llm=fake_llm)
assert await agent.compute(5) == 42
assert fake_llm.attempts == 6


# ---------------------------------------------------------------------------
# CodeActStrategy — max_iterations exceeded (codeact.py: loop exhaust path)
Expand Down Expand Up @@ -474,6 +519,59 @@ async def analyze(self, data: str) -> str:
# pure_python.py line 343: "LLM API error after {max_retries} retries."
assert "llm api error after 2 retries" in err

@pytest.mark.asyncio
async def test_successful_call_resets_api_error_budget(self):
"""Non-consecutive API errors must not add up across a long run.

Calls 0, 2 and 4 fail and each retry succeeds. With max_retries=2 the
run must still finish: only consecutive API errors count.
"""

class TestAgent(Agent, llm=_DUMMY_LLM):
@strategy(PurePythonStrategy(max_iterations=10, max_retries=2))
async def compute(self, x: int) -> int:
"""Compute {x}."""
...

fake_llm = FlakyFakeLLM(
scripted_responses=[
_resp("y = 1"),
_resp("y = 2"),
_resp("return 42"),
],
failing_calls={0, 2, 4},
)

agent = TestAgent(llm=fake_llm)
assert await agent.compute(5) == 42
assert fake_llm.attempts == 6

@pytest.mark.asyncio
async def test_xml_format_error_resets_api_error_budget(self):
"""An XML format error is a model reply, so it clears the API-error count.

API error, XMLFormatError, API error, success: with max_retries=2 the two
API errors are not consecutive and the run must finish.
"""

class TestAgent(Agent, llm=_DUMMY_LLM):
@strategy(PurePythonStrategy(max_iterations=10, max_retries=2))
async def compute(self, x: int) -> int:
"""Compute {x}."""
...

fake_llm = FlakyFakeLLM(
scripted_responses=[
_resp("<tool_code><code>return 1</code></tool_code>"),
_resp("return 42"),
],
failing_calls={0, 2},
)

agent = TestAgent(llm=fake_llm)
assert await agent.compute(5) == 42
assert fake_llm.attempts == 4


# ---------------------------------------------------------------------------
# PurePythonStrategy — code validation errors (pure_python.py lines 671-679)
Expand Down
Loading