Skip to content

fix(runtime): wire the local stream stale ceiling the config already promises - #338

Merged
OmarB97 merged 1 commit into
mainfrom
fix/local-stream-stale-ceiling-fork-20260802
Aug 2, 2026
Merged

fix(runtime): wire the local stream stale ceiling the config already promises#338
OmarB97 merged 1 commit into
mainfrom
fix/local-stream-stale-ceiling-fork-20260802

Conversation

@OmarB97

@OmarB97 OmarB97 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

agent.local_stream_stale_timeout (default 900) and its env twin
HERMES_LOCAL_STREAM_STALE_TIMEOUT were read by no code. A whole-tree grep
found exactly four references: the default in DEFAULT_CONFIG, the docs row in
website/docs/reference/environment-variables.md, and a test whose name
mentions the knob but which asserted float("inf") — i.e. it locked in the
absence of the ceiling.

The reader was dropped by #263 (d8aced717a), which replaced the inline
load_config() + env_float("HERMES_LOCAL_STREAM_STALE_TIMEOUT", ...) block
with resolve_stream_stale_timeout() and never reinstated the ceiling. That
commit touched only agent/, run_agent.py and tests — the config default and
the docs row were left behind, still promising a bound that no longer existed.
So resolve_stream_stale_timeout returned float("inf") for every generic
(non-DFlash) local endpoint: exactly the infinite disable the 900s default was
introduced to replace.

Why it matters

A knob that is defaulted, documented, and inert is worse than no knob. This one
misdirected a root-cause investigation into deep-context turn timeouts on a
local lane, because the operator and the investigator both reasonably believed a
900s ceiling was in force.

The runtime consequence is worse than a stale doc. With the threshold at inf,
_stale_elapsed > _stream_stale_timeout can never be true, so the stale branch
of the streaming poll loop is unreachable for a generic local endpoint. A
crashed or deadlocked local server that keeps the socket alive with SSE pings
parks the session forever: no reconnect, no fallback, no error the user can act
on.

The fix

resolve_stream_stale_timeout now resolves the ceiling instead of returning
inf, highest precedence first:

  1. HERMES_LOCAL_STREAM_STALE_TIMEOUT — the documented escape hatch;
  2. agent.local_stream_stale_timeout in config.yaml — the canonical setting;
  3. 900s.

Non-positive disables the watchdog, so inf stays reachable as a deliberate
choice for an exotically slow local model. A providers.<id>.stale_timeout_seconds
or an explicit HERMES_STREAM_STALE_TIMEOUT still wins outright, and the local
DeepSeek Flash family keeps its own tighter budget — this branch is only reached
when nothing more specific is set.

The resolved ceiling is then widened — never narrowed — by the request's
context-scaled prefill cost, reusing _dflash_prefill_scaled_timeout. That
helper is already the portable half of the DFlash budget: the per-1k prefill
term is arithmetic (a bigger prompt takes proportionally longer on any backend),
while the cold-start allowance stays gated to the one lane it was measured on. A
flat deadline is precisely how the old ladder killed healthy prefills
mid-flight (#278, #334: 240s at ~95k tokens, 240s at ~89k, 180s at ~33k, all on
a server that was still working). Trading "hangs forever" for "kills work that
was fine" would be the worse of the two. The same
HERMES_DFLASH_FIRST_CHUNK_CEILING bound (1800s) caps the result, so a wedged
endpoint is bounded at any prompt size.

Behavior change and the tradeoff

A previously unbounded wedged local server now trips at 900s + 4s/1k of prompt.
That is the intended change, and it is what the config default and the docs have
promised all along.

The risk in the other direction is real and worth stating: this path has no
liveness probe (_local_backend_generation_active stays on the DFlash
first-chunk branch), so a genuinely healthy generic local server that needs
longer than the scaled ceiling gets its connection killed and reconnected, which
costs a re-prefill. The mitigations are the generous 900s base (5× the DFlash
family's), the widen-only context scaling, and a one-line escape hatch
(local_stream_stale_timeout: 0). Extending the liveness probe to this branch
is a reasonable follow-up, but it is a separate behavior change from wiring the
knob.

Tests

tests/agent/test_local_stream_timeout.py::test_generic_local_stream_stale_timeout_still_disables_by_default
asserted float("inf"). That assertion pinned the regression, not the design —
the config default and the docs both describe a finite ceiling, and #263 changed
neither. It is rewritten as
test_generic_local_stream_stale_timeout_is_bounded_but_looser (900s, with the
DFlash family's 180s asserted alongside it on the same endpoint so the contrast
is explicit), and the docstring records why the new number is the right one.

Added TestGenericLocalStreamStaleCeiling, which resolves through the real
config loader and a real AIAgent: default from DEFAULT_CONFIG, config.yaml
value honored, env override, unparseable env falling through to config rather
than being treated as an override, 0/negative disabling, context scaling
(1260s at 90k, 904s at 1k), the 1800s clamp at 400k, and explicit
HERMES_STREAM_STALE_TIMEOUT / providers.<id>.stale_timeout_seconds still
winning.

Added TestGenericLocalStaleWatchdogActuallyFires, which drives the real poll
loop in interruptible_streaming_api_call against a local endpoint that accepts
the request and never sends a chunk. It asserts the kill actually fires at 960s
(first poll past the ceiling on a 60s-per-poll clock) and counts toward the
cross-turn give-up breaker — and that with the ceiling disabled, 1200s of
silence produces no kill at all, which is both the escape hatch and a record of
the pre-fix behavior.

Two test-local mirrors of this resolution still said local means inf. The one
in test_stream_read_timeout_floor.py is exercised (its local case now asserts
900s, which makes the test stronger: it proves the local read-timeout branch
wins outright even when the stale value is finite). The one in
test_reasoning_stale_timeout_floor.py is never called with a local URL, so it
now raises rather than carrying a second, drifting copy of the rule.

Docs

  • HERMES_STREAM_RETRIES was documented as defaulting to 3; the code default is
    2, which yields 3 total attempts. Corrected in the EN and zh-Hans env-var
    tables and in the tips corpus (hermes_cli/tips.py).
  • Three surfaces still said stale-stream detection is "disabled entirely" for
    local providers: the HERMES_STREAM_STALE_TIMEOUT row, the API-timeouts table
    and prose in user-guide/configuration.md, and the table in
    guides/local-llm-on-mac.md. All corrected to describe the ceiling, the
    context scaling, and the 0 disable.
  • Added the knob to cli-config.yaml.example and expanded its DEFAULT_CONFIG
    comment, since config.yaml is the canonical surface and the env var is the
    escape hatch.

Unrelated drift left alone: the zh-Hans env-var table also has stale defaults
for HERMES_API_CALL_STALE_TIMEOUT (300 vs 90) and HERMES_AGENT_TIMEOUT
(900 vs 1800). Those belong to a translation sweep, not to this fix.

…promises

`agent.local_stream_stale_timeout` (default 900) and its env twin
`HERMES_LOCAL_STREAM_STALE_TIMEOUT` were read by no code. A whole-tree grep
found exactly four references: the default in DEFAULT_CONFIG, the docs row, and
a test whose NAME mentions the knob but which asserted `float("inf")` — pinning
the absence of the ceiling.

The reader was dropped by #263 (d8aced7), which replaced the inline
`load_config()` + `env_float("HERMES_LOCAL_STREAM_STALE_TIMEOUT", ...)` block
with `resolve_stream_stale_timeout()` and never reinstated it. That commit
touched only agent/, run_agent.py and tests — the config default and the docs
row were left behind, still promising a bound that no longer existed. So every
generic (non-DFlash) local endpoint got `float("inf")`: exactly the infinite
disable the 900s default was introduced to replace.

A knob that is defaulted, documented, and inert is worse than no knob. This one
misdirected a root-cause investigation into deep-context turn timeouts on a
local lane, because the operator and the investigator both reasonably believed
a 900s ceiling was in force. The runtime cost is worse than a stale doc: with
the threshold at inf, `_stale_elapsed > _stream_stale_timeout` can never be
true, so the poll loop's stale branch is unreachable for a generic local
endpoint. A crashed or deadlocked local server that keeps the socket alive with
SSE pings parks the session forever — no reconnect, no fallback, no error.

`resolve_stream_stale_timeout` now resolves the ceiling instead of returning
inf: `HERMES_LOCAL_STREAM_STALE_TIMEOUT` (the documented escape hatch), then
`agent.local_stream_stale_timeout` (canonical), then 900s. Non-positive
disables the watchdog, so an unbounded wait stays reachable as a deliberate
choice for an exotically slow local model. A `providers.<id>.stale_timeout_seconds`
or an explicit `HERMES_STREAM_STALE_TIMEOUT` still wins outright, and the local
DeepSeek Flash family keeps its own tighter budget — this branch is reached
only when nothing more specific is set.

The ceiling is then widened — never narrowed — by the request's context-scaled
prefill cost, reusing `_dflash_prefill_scaled_timeout`. That helper is already
the portable half of the DFlash budget: the per-1k prefill term is arithmetic
(a bigger prompt takes proportionally longer on any backend), while the
cold-start allowance stays gated to the lane it was measured on. A FLAT
deadline is precisely how the old ladder killed healthy prefills mid-flight
(#278, #334: 240s at ~95k tokens, 240s at ~89k, 180s at ~33k, on a server that
was still working); trading "hangs forever" for "kills work that was fine"
would be the worse of the two. The same HERMES_DFLASH_FIRST_CHUNK_CEILING
(1800s) caps the result, so a wedged endpoint stays bounded at any prompt size.

Behavior change, stated plainly: a previously unbounded wedged local server now
trips at 900s + 4s/1k of prompt. The risk in the other direction is that this
path has no liveness probe (`_local_backend_generation_active` stays on the
DFlash first-chunk branch), so a healthy-but-very-slow generic local server
that exceeds the scaled ceiling is reconnected rather than waited out. The
mitigations are the generous base (5x the DFlash family's), the context
scaling, and a one-line escape hatch. Extending the probe to this branch is a
separate behavior change from wiring the knob.

test_generic_local_stream_stale_timeout_still_disables_by_default asserted inf.
That assertion pinned the regression, not the design — the config default and
the docs both describe a finite ceiling and #263 changed neither. Rewritten as
..._is_bounded_but_looser (900s, with the DFlash family's 180s asserted
alongside it on the same endpoint so the contrast is explicit).

TestGenericLocalStreamStaleCeiling resolves through the real config loader and
a real AIAgent: DEFAULT_CONFIG default, config.yaml value, env override,
unparseable env falling through to config rather than counting as an override,
0/negative disabling, context scaling (1260s at 90k, 904s at 1k), the 1800s
clamp at 400k, and explicit stale-timeout settings still winning.

TestGenericLocalStaleWatchdogActuallyFires drives the real poll loop in
`interruptible_streaming_api_call` against an endpoint that accepts the request
and never sends a chunk: the kill fires at 960s and counts toward the
cross-turn give-up breaker, and with the ceiling disabled 1200s of silence
produces no kill at all — the escape hatch, and a record of the pre-fix
behavior.

Two test-local mirrors of this resolution still said local means inf. The one
in test_stream_read_timeout_floor.py is exercised, so its local case now
asserts 900s — which makes it stronger, since it proves the local read-timeout
branch wins outright even when the stale value is finite. The one in
test_reasoning_stale_timeout_floor.py is never called with a local URL, so it
now raises instead of carrying a second, drifting copy of the rule.

Docs: HERMES_STREAM_RETRIES was documented as defaulting to 3; the code default
is 2, which yields 3 total attempts (corrected in the EN and zh-Hans env-var
tables and in the tips corpus). Three surfaces still said stale-stream
detection is "disabled entirely" for local providers — the
HERMES_STREAM_STALE_TIMEOUT row, the API-timeouts table and prose in
user-guide/configuration.md, and the table in guides/local-llm-on-mac.md — all
corrected to describe the ceiling, the scaling, and the 0 disable. Added the
knob to cli-config.yaml.example and expanded its DEFAULT_CONFIG comment, since
config.yaml is the canonical surface and the env var is the escape hatch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@OmarB97
OmarB97 merged commit 3afe992 into main Aug 2, 2026
35 checks passed
OmarB97 pushed a commit that referenced this pull request Aug 2, 2026
…anch

main advanced while this branch was in flight, and #338 ("wire the local
stream stale ceiling the config already promises") landed in the same
stale-timeout logic. Three conflicts, none of them a behavior collision:

* agent/chat_completion_helpers.py — docstring only. Both sides appended to
  the same paragraph in _dflash_prefill_scaled_timeout; main's text is a
  superset of ours, adding a cross-reference to _generic_local_stale_timeout.
  Took main's. Verified that function exists in the merged tree (line 630) and
  that it really is widened by the per-1k prefill term, so the reference it
  adds is accurate rather than aspirational.

* website/docs/user-guide/configuration.md — same sentence on both sides,
  differing only in the example provider name. Took main's `my-local-lane`:
  it is the entry actually used in cli-config.yaml.example, whereas
  `ai-router` appeared nowhere else in the docs.

* tests/agent/test_local_stream_timeout.py — a tail add/add. Our side
  contributes no lines at the conflict point; main appends
  TestGenericLocalStreamStaleCeiling. Kept both bodies by dropping the
  markers, so our first-chunk resolver tests above the seam and main's new
  class below it both survive.

No production logic was resolved away — the only code hunk was a docstring.

Verified on the merge result, not on either parent:
  pytest tests/hermes_cli/test_timeouts.py \
         tests/agent/test_request_prefix_stability.py \
         tests/run_agent/test_provider_parity.py \
         tests/agent/test_local_stream_timeout.py \
         tests/agent/test_reasoning_stale_timeout_floor.py \
         tests/agent/test_stream_read_timeout_floor.py -q
  343 passed

Merged rather than rebased on purpose: another session is active on this
branch with uncommitted work, and a force-push would have destroyed it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
OmarB97 added a commit that referenced this pull request Aug 2, 2026
…343)

The stale-stream watchdog in `interruptible_streaming_api_call` killed a
local connection on a stopwatch alone. The no-first-chunk branch a few
lines above it has asked the server since #278 —
`_local_backend_generation_active()` reads `/_gate/status`, and a backend
reporting an in-flight generation gets its deadline extended instead of
its socket closed — but the stale branch, the one that actually kills a
generic local stream, never asked.

That asymmetry was harmless while generic local endpoints resolved to
`inf`: the branch could not fire for them, so there was nothing to ask
about. #338 gave them a finite 900s ceiling, which is what makes the
question live. A healthy-but-slow local server that overruns it now has
its socket killed and reconnected, throwing away the prefill it already
paid for — the first step of a kill -> re-prefill -> kill spiral.

Probe in the shared stale branch for any local endpoint rather than only
the generic one, so a DFlash mid-stream stall is judged the same way its
pre-first-chunk wait already is. Extensions are bounded by the same
`HERMES_DFLASH_FIRST_CHUNK_CEILING` the first-chunk path uses, applied to
one silent stretch rather than to total request time, and are reset the
moment a chunk lands so they cannot accumulate across stalls. The probe
still returns None for any endpoint without a gate, so a bare Ollama or
llama.cpp is decided by the timer exactly as before and a remote API is
never asked at all.

Co-authored-by: Omar Baradei <omar@kostudios.io>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant