Skip to content

fix(agent): classify jiter SSE parse failures as retryable (#65147) - #65154

Open
wesleysimplicio wants to merge 3 commits into
NousResearch:mainfrom
wesleysimplicio:fix/jiter-parse-error-retryable
Open

fix(agent): classify jiter SSE parse failures as retryable (#65147)#65154
wesleysimplicio wants to merge 3 commits into
NousResearch:mainfrom
wesleysimplicio:fix/jiter-parse-error-retryable

Conversation

@wesleysimplicio

Copy link
Copy Markdown
Contributor

Summary

A transient provider/network failure (a truncated/corrupted data: SSE
event) surfaces from jiter — the Rust JSON parser the openai SDK
>=1.x uses to parse streamed chunks — as a plain ValueError. The
non-retryable classifier in agent/conversation_loop.py only carves out
json.JSONDecodeError (added for #14271/#14782), so a jiter parse error
still gets misclassified as a local programming bug and aborts on
attempt 1/3 instead of retrying — even though an identical request
retried seconds later succeeds.

Changes

  • agent/conversation_loop.py: added _JITER_PARSE_ERROR_RE (matches
    jiter's consistent "... at line N column N" message shape, since it
    has no dedicated exception class) and excluded that shape — only for a
    bare ValueError, not any subclass already covered by other carve-outs
    — from is_local_validation_error.
  • tests/run_agent/test_jsondecodeerror_retryable.py: extended the
    existing mirror-predicate test file (same pattern as the [Bug]: JSONDecodeError bypasses retry logic due to ValueError inheritance #14782 and
    Treat NoneType provider shape errors as retryable #33136 carve-outs) with TestJiterParseErrorIsRetryable (positive case,
    a bare unrelated ValueError still aborts, and a JSONDecodeError
    isn't double-matched) and TestAgentLoopSourceHasJiterCarveOut (belt-
    and-suspenders check that the production source actually contains the
    carve-out).

Validation

Command Result
pytest tests/run_agent/test_jsondecodeerror_retryable.py -q 13 passed
Fail-before (stashed the conversation_loop.py change) TestAgentLoopSourceHasJiterCarveOut fails as expected — carve-out absent
Pass-after (restored the change) 13 passed
pytest tests/run_agent/ -q -k "retryable or jsondecode or jiter" --ignore=tests/run_agent/test_strip_reasoning_tags_cli.py 16 passed
ruff check agent/conversation_loop.py tests/run_agent/test_jsondecodeerror_retryable.py All checks passed

(test_strip_reasoning_tags_cli.py is excluded — it fails to collect on
a clean main checkout too, due to a missing local prompt_toolkit
dependency unrelated to this change.)

Closes #65147

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing this to the local-validation gate. The premise remains valid on current main: agent/conversation_loop.py:3764-3792 still treats a plain ValueError as non-retryable, while #65147 reports the OpenAI-compatible jiter failure in that form.

Problems

  • The added at line N column N suffix matcher is not jiter-specific. It will retry any bare local ValueError with that suffix, widening the exception to the local-programming-error rule in agent/conversation_loop.py:3753-3792.
  • TestAgentLoopSourceHasJiterCarveOut reads production source via inspect.getsource. AGENTS.md:1370 explicitly bans source-reading tests, and the mirror predicate does not execute the real retry path.

Suggested changes

  • Use a verified jiter-specific signature or relevant streaming-path context, and test a same-suffix non-jiter ValueError remains non-retryable.
  • Replace the source/mirror coverage with a real loop retry test; tests/run_agent/test_streaming.py:1206-1263 already demonstrates a failing-first-stream, succeeding-second-stream pattern.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 16, 2026
@wesleysimplicio

Copy link
Copy Markdown
Contributor Author

Both fixed:

  1. Narrowed _JITER_PARSE_ERROR_RE to require the message start with one of jiter's real, stable error phrases (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/length, control character, number out of range, recursion limit exceeded, duplicate field, unknown field — jiter wraps Rust's serde_json parser, whose error templates are this small, stable set), not just the trailing line/column suffix.
  2. Replaced the inspect.getsource belt-and-suspenders test with two real tests driving agent.run_conversation() through a mocked client.chat.completions.create: a jiter-shaped ValueError on the first call retries and succeeds on the second; a same-suffix but non-jiter ValueError aborts immediately without retrying.
uv run --extra dev pytest tests/run_agent/test_jsondecodeerror_retryable.py -q
15 passed

Fail-before verified: reverting only the regex to the old bare-suffix version makes the new negative test fail exactly as expected — call_count == 3 (retried twice, should have aborted on attempt 1) instead of 1, with the actual retry log lines confirming the misclassification.

Broader retry-classifier suite: uv run --extra dev pytest tests/run_agent/ -q -k 'retry or retryable or jiter or jsondecode' → 61 passed, no regressions.

Simplicio, Wesley (ext) added 3 commits August 10, 2026 17:19
…rch#65147)

jiter (the Rust JSON parser the openai SDK >=1.x uses for streamed SSE
chunks) raises a plain ValueError on a truncated/corrupted data: payload
- not a json.JSONDecodeError subclass, so the NousResearch#14271/NousResearch#14782 carve-out
doesn't catch it. The classifier in conversation_loop.py treated this as
a local programming bug and aborted on attempt 1/3 instead of retrying,
even though the same transient failure recovers fine seconds later.

jiter has no dedicated exception class, so match its consistent message
shape ("... at line N column N") instead, and only for a bare ValueError
(not a subclass already covered by other carve-outs).
… real loop test

Maintainer review (hermes-sweeper) on this PR found two real gaps:

1. _JITER_PARSE_ERROR_RE only checked the trailing 'at line N column N'
   suffix, which is not jiter-specific -- an unrelated local ValueError
   coincidentally ending in that suffix would be misclassified as
   retryable, widening the carve-out beyond jiter's actual failures.
   Fixed by requiring the message to also start with one of jiter's
   real, stable error phrases (verified directly against the installed
   jiter package).

2. The belt-and-suspenders test read production source via
   inspect.getsource, which AGENTS.md bans. Replaced it with two real
   tests driving the actual conversation loop through
   agent.run_conversation() with a mocked client: one where a
   jiter-shaped ValueError on the first call retries and succeeds on
   the second, and one where a same-suffix but non-jiter ValueError
   aborts immediately without retrying.

Validation: uv run --extra dev pytest tests/run_agent/test_jsondecodeerror_retryable.py -q -> 15 passed. Fail-before: reverted only the regex to the old bare-suffix version; the new negative real-loop test failed exactly as expected (retried 3 times instead of aborting on attempt 1). ruff check: clean. Broader retry suite (uv run --extra dev pytest tests/run_agent/ -q -k 'retry or retryable or jiter or jsondecode'): 61 passed, no regressions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: jiter ValueError from corrupted SSE stream chunk classified as non-retryable — gap in the #14271 fix

3 participants