Skip to content

feat(agent): finite first-chunk timeout + failover for local OpenAI-compatible streams - #37168

Closed
OmarB97 wants to merge 1 commit into
NousResearch:mainfrom
OmarB97:codex/pr35642-local-ttfb-failover
Closed

feat(agent): finite first-chunk timeout + failover for local OpenAI-compatible streams#37168
OmarB97 wants to merge 1 commit into
NousResearch:mainfrom
OmarB97:codex/pr35642-local-ttfb-failover

Conversation

@OmarB97

@OmarB97 OmarB97 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

What

Local OpenAI-compatible backends currently get an unbounded wait in two failure modes: a stream that is accepted but never emits its first SSE chunk, and a non-streaming call whose server sends no response headers until the entire completion finishes. Both wedge the turn instead of failing over.

How

  • HERMES_LOCAL_FIRST_CHUNK_TIMEOUT (default 90s, scales with estimated request context) bounds time-to-first-chunk for local streams; on expiry the connection is killed, the error is marked, and FailoverReason.local_first_chunk_timeout routes straight to the fallback chain instead of rebuilding the same wedged client.
  • HERMES_LOCAL_NON_STREAM_STALE_TIMEOUT (default 120s, context-scaled) makes local non-streaming calls finite.
  • Streaming stale timeouts for local endpoints keep the historical unbounded default; HERMES_LOCAL_STALE_TIMEOUT is an opt-in bound for backends known to park requests on open sockets.

Tests

tests/agent/test_local_stream_timeout.py (boundaries, opt-in, env precedence), tests/agent/test_error_classifier.py (marker classification), tests/hermes_cli/test_timeouts.py (finite local non-stream default), plus an ordering assertion that first-chunk fallback precedes primary-transport recovery. 230+ tests pass locally.

This is part 4 of the former #35642 split; rewritten per review feedback to be model-name agnostic (no model-specific env vars or heuristics) and to drop the unrelated oneshot logging changes.

@OmarB97
OmarB97 marked this pull request as ready for review June 2, 2026 03:26
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have labels Jun 2, 2026
@OmarB97
OmarB97 force-pushed the codex/pr35642-local-ttfb-failover branch from f03ceec to f105b3b Compare June 10, 2026 03:11
@OmarB97 OmarB97 changed the title [Hermes Agent][hermes-pr35642-split-snowball][4/n] Split local TTFB failover feat(agent): finite first-chunk timeout + failover for local OpenAI-compatible streams Jun 10, 2026
@OmarB97
OmarB97 force-pushed the codex/pr35642-local-ttfb-failover branch from f105b3b to c4aad3f Compare July 5, 2026 19:11
Reapply the local-provider stale/TTFB timeout bounding (stream, first-chunk,
and non-stream resolvers; local_first_chunk_timeout tracked through the
streaming worker result; the reasoning-floor and explicit-config interplay
fix) onto the post-history-replacement mainline.

Conflict resolution (agent/chat_completion_helpers.py, two spots): upstream
grew its own independent, unrelated addition in the same locations since this
branch's last refresh — the cross-turn stale-call circuit breaker (NousResearch#58962:
_stale_streak/_bump_stale_streak/_reset_stale_streak/_check_stale_giveup).
Both features are additive; kept upstream's circuit-breaker block ahead of
the local-provider timeout resolvers, and folded _check_stale_giveup(agent)
into the same result-dict block that carries local_first_chunk_timeout
rather than picking one side. Everything else (agent_runtime_helpers.py,
conversation_loop.py, error_classifier.py, run_agent.py, and the four test
files) auto-merged with zero conflict.

Ran the full touched-module suite locally: tests/agent/test_local_stream_timeout.py,
tests/agent/test_error_classifier.py, tests/hermes_cli/test_timeouts.py,
tests/run_agent/test_run_agent.py — 684 passed.

Refresh of PR NousResearch#37168; prior head 0440c4c625ccc57fb1aad1eb1eb5e919f3d8726f
(a follow-up fix commit on top of an earlier orphan-commit refresh,
c4aad3f1fc, which itself carries no original-head trailer — PR timeline
shows force-pushes on 2026-06-10 and 2026-07-05 for anyone reconstructing
provenance further back).
@OmarB97
OmarB97 force-pushed the codex/pr35642-local-ttfb-failover branch from 0440c4c to 82d4649 Compare July 10, 2026 15:57
@OmarB97

OmarB97 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed onto current upstream/main via rebase (real merge-base — an earlier "no common ancestor" read on a sibling PR in this wave turned out to be a shallow-clone artifact, not true history divergence). Two commits replayed (an earlier orphan refresh + a follow-up bug-fix that patched two regressions in it), then squashed to one. One real conflict, in agent/chat_completion_helpers.py at two spots: upstream independently grew the cross-turn stale-call circuit breaker (#58962) in the same locations since this branch's last refresh. Both features are additive — kept upstream's circuit-breaker block ahead of the local-provider timeout resolvers, and folded _check_stale_giveup(agent) into the same result-dict block that carries local_first_chunk_timeout. Everything else (5 files + the 4 test files) auto-merged clean. Ran the full touched-module suite locally: test_local_stream_timeout.py, test_error_classifier.py, test_timeouts.py, test_run_agent.py — 684 passed. Prior head: 0440c4c625; new head: 82d4649e9f. Force-pushed with lease; checks re-running.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the careful refresh, focused tests, and preservation of the existing stale-call circuit-breaker work. The local no-first-chunk problem is real, but this implementation cannot land in its current configuration form.

  • Automated hermes-sweeper review found that the PR's user-facing mechanism adds several non-secret timeout variables (HERMES_LOCAL_FIRST_CHUNK_TIMEOUT, HERMES_LOCAL_NON_STREAM_STALE_TIMEOUT, and aliases) in agent/chat_completion_helpers.py.
  • The standing policy in AGENTS.md:102-106 requires behavioral settings such as timeouts and thresholds to use config.yaml, not new HERMES_* environment variables.
  • Existing provider timeout configuration already lives under providers.<id>.stale_timeout_seconds / per-model settings in cli-config.yaml.example:108-139; a future focused proposal should use that config.yaml mechanism rather than introduce new environment-variable surfaces.

This is an automated hermes-sweeper review.


Closed as not-planned per standing maintainer policy (env-var-for-config). This is a design-direction decision, not a code-quality judgment — see the Contribution Rubric in AGENTS.md for what the project is looking for. If you believe this policy was misapplied to your change, comment here and a maintainer will take a look.

@teknium1 teknium1 closed this Jul 13, 2026
@teknium1 teknium1 added sweeper:not-planned Sweeper: closed per standing maintainer policy (design direction) sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
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 comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:not-planned Sweeper: closed per standing maintainer policy (design direction) 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.

3 participants