Skip to content
Open
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
30 changes: 30 additions & 0 deletions agent/conversation_loop.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,27 @@ def _should_skip_model_call_for_reference_handoff(
"awaiting your next message."
)

# jiter (the Rust JSON parser the openai SDK >=1.x uses for SSE stream
# chunks) raises a plain ``ValueError`` on a truncated/corrupted ``data:``
# payload — not a ``json.JSONDecodeError`` subclass, so it isn't caught by
# the #14271/#14782 exclusion below (see #65147). A bare "ends in at line N
# column N" match is not jiter-specific — an unrelated local ValueError
# could coincidentally share that suffix and get misclassified as
# retryable. jiter wraps Rust's serde_json parser, whose error messages are
# a small, stable, well-known vocabulary (verified directly against the
# installed jiter package): "EOF while parsing a value/list/string",
# "trailing characters", "trailing comma", "key must be a string",
# "expected value", "invalid type/escape/unicode", "control character",
# "number out of range", "recursion limit exceeded", "duplicate field",
# "unknown field". Require the message to start with one of these, not
# just end in the line/column suffix.
_JITER_PARSE_ERROR_RE = re.compile(
r"^(?:eof while parsing|trailing (?:characters|comma)|key must be a string|"
r"expected value|invalid (?:type|escape|unicode|length)|control character|"
r"number out of range|recursion limit exceeded|duplicate field|unknown field)"
r".* at line \d+ column \d+$",
re.IGNORECASE,
)

# Stable prefix of the local interrupt status string emitted when a turn is
# cancelled while waiting on the provider. Surfaces (ACP, TUI) match on this
Expand Down Expand Up @@ -5417,6 +5438,15 @@ def _perform_api_call(next_api_kwargs):
and "nonetype" in str(api_error).lower()
and "not iterable" in str(api_error).lower()
)
# jiter parse failures on a malformed/truncated SSE chunk
# are a transient provider/network issue, the same class
# as the json.JSONDecodeError exclusion above — but jiter
# raises a plain ValueError with no dedicated exception
# class, so match by its message shape instead (#65147).
and not (
type(api_error) is ValueError
and _JITER_PARSE_ERROR_RE.search(str(api_error))
)
)
# ``FailoverReason.billing`` (HTTP 402) is NOT in this
# exclusion set. By the time we reach this block:
Expand Down
163 changes: 163 additions & 0 deletions tests/run_agent/test_jsondecodeerror_retryable.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,25 @@
from __future__ import annotations

import json
import re
from types import SimpleNamespace
from unittest.mock import MagicMock, patch

import pytest

from run_agent import AIAgent

# Kept in lock-step with agent/conversation_loop.py's _JITER_PARSE_ERROR_RE —
# jiter/serde_json's small, stable error-message vocabulary, verified
# directly against the installed jiter package, not just the line/column
# suffix (an unrelated ValueError could coincidentally share that suffix).
_JITER_PARSE_ERROR_RE = re.compile(
r"^(?:eof while parsing|trailing (?:characters|comma)|key must be a string|"
r"expected value|invalid (?:type|escape|unicode|length)|control character|"
r"number out of range|recursion limit exceeded|duplicate field|unknown field)"
r".* at line \d+ column \d+$",
re.IGNORECASE,
)


def _mirror_agent_predicate(err: BaseException) -> bool:
Expand All @@ -42,6 +61,13 @@ def _mirror_agent_predicate(err: BaseException) -> bool:
and "nonetype" in str(err).lower()
and "not iterable" in str(err).lower()
)
# jiter (openai SDK's Rust JSON parser) raises a plain ValueError
# on a truncated/corrupted SSE chunk — a transient provider/network
# issue, not a local bug. See #65147.
and not (
type(err) is ValueError
and _JITER_PARSE_ERROR_RE.search(str(err))
)
)


Expand Down Expand Up @@ -97,3 +123,140 @@ def test_unrelated_type_error_remains_local_validation(self):
assert _mirror_agent_predicate(TypeError("expected str, got int"))



class TestJiterParseErrorIsRetryable:
"""Regression for #65147: jiter (openai SDK's Rust SSE JSON parser)
raises a plain ValueError on a truncated/corrupted stream chunk — not a
json.JSONDecodeError subclass, so the #14782 carve-out alone doesn't
catch it. Its messages consistently end in "at line N column N"."""

def test_jiter_style_value_error_is_not_local_validation(self):
err = ValueError("expected value at line 1 column 223")
assert not _mirror_agent_predicate(err), (
"A ValueError shaped like jiter's parse-failure message must be "
"excluded from is_local_validation_error — it is a transient "
"provider/network stream corruption, not a local bug. See #65147."
)

def test_unrelated_value_error_remains_local_validation(self):
"""A bare ValueError without jiter's message shape still aborts."""
assert _mirror_agent_predicate(ValueError("bad arg"))
assert _mirror_agent_predicate(ValueError("invalid literal for int()"))

def test_value_error_subclass_is_not_matched(self):
"""Only a bare ValueError (type(err) is ValueError) matches — a
subclass like json.JSONDecodeError is already excluded above via
isinstance, so this carve-out must not double-match/broaden scope."""
try:
json.loads("{not valid json at line 1 column 5")
except json.JSONDecodeError as exc:
# Already excluded by the JSONDecodeError isinstance check —
# confirm the jiter carve-out doesn't need to fire for it.
assert not _mirror_agent_predicate(exc)

def test_unrelated_valueerror_with_same_line_column_suffix_is_not_matched(self):
"""A same-shaped SUFFIX alone must not be enough to match — only
jiter/serde_json's actual message vocabulary should. An app-level
validation error that happens to end in "at line N column N" (e.g.
a config-file parser reporting its own location) must still abort
as a local programming bug, not silently retry."""
assert _mirror_agent_predicate(
ValueError("custom field validation failed at line 1 column 223")
)
assert _mirror_agent_predicate(
ValueError("unexpected indentation at line 4 column 10")
)


_TEST_AGENT_KWARGS = {
"api_key": "-".join(["not", "a", "real", "credential", "placeholder"]),
"base_url": "https://openrouter.ai/api/v1",
"quiet_mode": True,
"skip_context_files": True,
"skip_memory": True,
}


def _agent_with_mocked_client():
"""Minimal AIAgent with a mocked OpenAI client, ready for run_conversation."""
with (
patch("run_agent.get_tool_definitions", return_value=[]),
patch("run_agent.check_toolset_requirements", return_value={}),
patch("run_agent.OpenAI"),
):
a = AIAgent(**_TEST_AGENT_KWARGS)
a.client = MagicMock()
a._cached_system_prompt = "You are helpful."
a._use_prompt_caching = False
a.tool_delay = 0
a.compression_enabled = False
a.save_trajectories = False

return a


def _mock_success_response(content="Done"):
msg = SimpleNamespace(
content=content, tool_calls=None, reasoning=None,
reasoning_content=None, reasoning_details=None,
)
choice = SimpleNamespace(message=msg, finish_reason="stop")
resp = SimpleNamespace(choices=[choice], model="test/model", usage=None)

return resp


class TestJiterParseErrorRealLoopRetry:
"""Real end-to-end coverage through agent.run_conversation() — not a
source scan. A source-presence check (inspect.getsource) passes when
the implementation is subtly broken and fails on a pure refactor with
identical behavior; it also can't run against a built/bundled artifact.
Drive the actual retry classifier instead, mirroring the pattern in
tests/run_agent/test_streaming.py's failing-first-call/succeeding-
second-call tests."""

def test_jiter_style_valueerror_retries_then_succeeds(self):
agent = _agent_with_mocked_client()
agent.client.chat.completions.create.side_effect = [
ValueError("expected value at line 1 column 223"),
_mock_success_response("Done"),
]

with (
patch.object(agent, "_persist_session"),
patch.object(agent, "_save_trajectory"),
patch.object(agent, "_cleanup_task_resources"),
patch("run_agent.time.sleep"),
):
result = agent.run_conversation("hello")

assert agent.client.chat.completions.create.call_count == 2, (
"A jiter-shaped ValueError on the first call must be retried, not "
"aborted as a local programming bug — see #65147."
)
assert result.get("completed") is True
assert result.get("failed") is not True

def test_unrelated_valueerror_with_same_suffix_aborts_without_retry(self):
"""Sanity check for the negative case: a same-suffix but non-jiter
ValueError must still abort immediately as a local validation
error, proving the carve-out is jiter-specific and not just
matching the line/column suffix shape."""
agent = _agent_with_mocked_client()
agent.client.chat.completions.create.side_effect = ValueError(
"custom field validation failed at line 1 column 223"
)

with (
patch.object(agent, "_persist_session"),
patch.object(agent, "_save_trajectory"),
patch.object(agent, "_cleanup_task_resources"),
patch("run_agent.time.sleep"),
):
result = agent.run_conversation("hello")

assert agent.client.chat.completions.create.call_count == 1, (
"A non-jiter ValueError sharing only the line/column suffix must "
"abort immediately, not retry."
)
assert result.get("failed") is True
Loading