fix(runtime): ask a local backend before killing its stalled stream - #343
Merged
Merged
Conversation
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: Claude Opus 5 <noreply@anthropic.com>
10 tasks
OmarB97
added a commit
that referenced
this pull request
Aug 2, 2026
…ll (#347) `interruptible_api_call`'s stale detector was the last watchdog in this file still deciding on a stopwatch alone. Both streaming ones already ask `_local_backend_generation_active()` — the DFlash pre-first-chunk wait since #278, the shared stale-stream branch since #343 — and a backend reporting an in-flight generation gets its deadline extended instead of its socket closed. The non-streaming kill never asked. It matters more here than in either streaming branch. A stream killed mid-generation has at least delivered the chunks it already produced; a non-streaming call delivers nothing until it delivers everything, so the kill throws away the whole prefill AND the whole generation and the retry restarts from zero — the kill -> re-prefill -> kill spiral with nothing salvaged. The reachable population is narrower than the streaming case, and deliberately so: a generic local endpoint left on the implicit default resolves to `inf` in `_compute_non_stream_stale_timeout`, so the branch cannot fire for it and there is nothing to ask about. Three configurations do reach it: * a local DFlash model (~180s + context scaling); * a local model in the reasoning-floor allowlist — deepseek-r1, qwq, qwen3, nemotron-3, the o-series — at 300-600s. This one is the least obvious: the model is not DFlash, so it looks like the generic local case, but the floor returns uses_implicit_default=False, and that flag is exactly what the `inf` short-circuit tests; * any endpoint where the operator pinned `stale_timeout_seconds` or `HERMES_API_CALL_STALE_TIMEOUT`. Probe in the stale branch for any local endpoint, mirroring the shape #343 established. Extensions are bounded by the same `HERMES_DFLASH_FIRST_CHUNK_CEILING`, measured over the current silent stretch — which for a non-streaming call is the whole request, the same span the pre-first-chunk streaming branch bounds, and the reason no reset is needed here (there is no chunk that can end a stretch). An operator-pinned threshold above the ceiling is honoured unchanged: the guard is already false the first time the branch fires, so the probe never runs and never shortens a deadline. 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. The `_codex_silent_hang_hint` messaging on the kill path is untouched. The docs claimed the non-stream detector is simply "auto-disabled for local providers"; the two local families that keep a finite budget are now named alongside the probe. Co-authored-by: Omar Baradei <omar@kostudios.io> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
The stale-stream watchdog now asks a local backend whether it is still generating before it kills the connection, instead of trusting a client-side stopwatch on its own.
interruptible_streaming_api_callhas two watchdog branches. The no-first-chunk branch has asked the server since #278 —_local_backend_generation_active()reads/_gate/status, and a backend that reports an in-flight generation gets the deadline extended rather than killed. The stale branch, a few lines below it, killed unconditionally.That asymmetry was harmless while generic local endpoints resolved to
float("inf"): the branch could not fire for them at all, so there was nothing to ask about. #338 wiredagent.local_stream_stale_timeoutback in and gave them a finite 900s ceiling (widened by context-scaled prefill cost, capped at 1800s) — which is what makes the question live. A healthy-but-slow local server that overruns the ceiling now has its socket killed and reconnected, which throws away the prefill it had already paid for and is the first step of a kill → re-prefill → kill spiral.Killing a request the server says it is serving is exactly the failure #278 named. This PR applies the same answer to the branch that actually does the killing.
Why the shared branch rather than a generic-local-only fix. The obvious narrow change — probe only when the endpoint is generic local — would leave the DFlash stale branch inconsistent with the DFlash first-chunk branch directly above it: the same backend's answer would be authoritative for one phase of a request and ignored for the next. There is only one stale branch, so gating the probe on
is_local_endpoint(base_url)covers both cases with less code and a single rule: a local stall is never killed while the backend reports it is working.Scope is unchanged for everyone else. The probe is only consulted for a local
base_url, and it is deliberately conservative — any error, timeout, non-200, missing endpoint or unparseable body yieldsNone, so a bare Ollama or llama.cpp is decided by the timer exactly as before and a remote API is never asked at all. Only a positive, parsed signal from a gated lane can extend anything.The bound. Extensions are capped by the same
HERMES_DFLASH_FIRST_CHUNK_CEILING(1800s) the first-chunk path uses, applied to one silent stretch rather than to total request time. Before the first chunk those are the same span, which keeps this identical to the branch above; mid-stream, "how long the backend has said nothing" is the quantity a stall watchdog is about — bounding total request time instead would kill a long, healthy, chunk-producing generation. Grants are reset the moment a chunk lands, so they cannot accumulate across stalls and drift the watchdog toward never firing. No new configuration is introduced.The socket underneath allows it. An extended wait is only real if httpx does not tear the connection down first. A local endpoint already gets
read=HERMES_API_TIMEOUT(1800s) on its stream (agent/chat_completion_helpers.py:3333) rather than the 120s cloud default, which is the same figure as the ceiling — so the longest stretch this can grant is exactly the longest the socket was already prepared to sit through. Nothing needed widening.Known sibling, deliberately not in this PR. The non-streaming path (
interruptible_api_call, agent/chat_completion_helpers.py:1501) still kills a local DFlash call without asking. It is not the same regression: generic local endpoints still resolve toinfthere (_compute_non_stream_stale_timeout, run_agent.py:1445), so #338's new ceiling does not reach it and nothing there changed recently. It deserves its own verified fix rather than a drive-by.Related Issue
No filed issue — this is the sibling call path of #278, made reachable by #338.
Type of Change
Changes Made
agent/chat_completion_helpers.py— ininterruptible_streaming_api_call:_stale_probe_localonce before the poll loop (base_urldoes not change for the life of a call);_stale_probe_extension, the seconds the probe has granted to the current silent stretch, reset whenever_stale_elapsedfalls back inside the plain threshold (i.e. a chunk landed, or a kill reset the clock);_local_backend_generation_active(base_url, model); on a positive answer extend, log, surface a status line, touch activity, and keep the socket. The next look is scheduled one stale-timeout later, clamped to land on the ceiling rather than past it — without the clamp a wide threshold (900s generic local, or a prefill-scaled DFlash one) would schedule its next check beyond the bound and silently outlive it.tests/agent/test_local_stream_timeout.py— newTestLocalStaleWatchdogAsksTheBackend(7 tests), plus a probe stub in the existing fix(runtime): wire the local stream stale ceiling the config already promises #338 watchdog driver so a unit test no longer makes a real request to port 11434.website/docs/reference/environment-variables.md,website/docs/user-guide/configuration.md, and the zh-Hans translation — describe the liveness question and the single-stretch bound.How to Test
The new tests drive the real poll loop in
interruptible_streaming_api_callwith a fake clock and a stubbed probe:test_busy_backend_extends_instead_of_killingtest_idle_backend_is_still_killed_promptlytest_endpoint_with_no_gate_keeps_the_plain_stopwatchtest_remote_endpoint_is_never_askedNonedefaulttest_a_backend_that_never_stops_saying_busy_is_still_boundedtest_dflash_mid_stream_stall_is_asked_about_tootest_extension_does_not_carry_into_the_next_silent_stretchReverting only
agent/chat_completion_helpers.pyfails 5 of the 7; the 2 that still pass are the unchanged-behavior guards (no-gate and remote), which is the point of them.Collateral — every other test file that drives
interruptible_streaming_api_call, plus the two the #338 ceiling touched. 794 passed, 0 failed:Not a full-suite run: this checkout has a standing set of unrelated macOS/Linux-tooling failures on
origin/main, so a green full suite is not a signal here.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleuntouched; this reusesHERMES_DFLASH_FIRST_CHUNK_CEILING)