feat(model): bound silence instead of the whole call #883

Merged

coilysiren merged 1 commit from aos/claude/hs68-model-idle-timeout into main

2026-08-17 04:18:27 +00:00

coilyco-ops commented

2026-08-17 04:09:26 +00:00

Member

A total request deadline ignores heartbeats by construction. [...] so the consuming side must move to an idle or read timeout for any of this to change behaviour. That half lives in sirens-echo. This is the emitting side, and it is inert until the consuming side lands.

The defect, stated precisely

A total deadline fires on schedule however many bytes arrived. So a turn that was queued behind other work and a turn whose backend had hung died at the same second, and the member was told turn timed out for both. There were two total deadlines, not one - the turn context and http.Client.Timeout, both set to cfg.RequestTimeout.

What changed

stream: true. Heartbeats have nowhere to go on a non-streaming request, which is why the proxy documents them as streaming-only.

Two bounds instead of one.

The turn context stays the ceiling. That number does not move here - see the last section.
SIRENS_ECHO_MODEL_IDLE_TIMEOUT (new, 45s) bounds silence. Any line resets it, and a heartbeat comment is a line.
http.Client.Timeout is now unset. Leaving it would have cut a streaming completion on schedule and reintroduced the defect one layer down.

Silence became its own failure. ErrModelSilent, cause model_silent, span sirens_echo.model.silent, and the member is told the backend went quiet rather than that the turn timed out.

That split is acceptance criterion 3 and it is the part that matters most: retrying silence can work, and retrying a turn that ran the ceiling out while making progress cannot. Telling a member to retry the second one is the true-sentence-that-misdirects shape from #449.

Assembly

Content and reasoning concatenate. Tool calls arrive as fragments keyed by index, so name and argument pieces are joined per call - a truncated argument string would reach the executor and fail for a reason that is not the model's.

Calls are returned in the order their indexes first appeared, not sorted by index, because that is the order the model asked for them in. Pinned by a test that streams index 1 before index 0.

Reasoning stays unnamed when the stream carried none. reasoning_content absent and reasoning_content: "" are echoed back differently on the next round, so inventing one would corrupt the roundtrip that docs/sirens-echo-reasoning-roundtrip.md exists to protect.

The non-streamed shape still works

Selected on Content-Type. Agent Proxy's own non-streaming surface still exists, and a backend that answers a streaming request with a whole body is read as one rather than failing a turn that arrived intact. That path gets no heartbeats, so the turn ceiling is its only bound - unchanged from today.

This is also why the existing test fixtures still pass without being rewritten. One assertion did change and had to: TestProxyClientSendsBoundedCommunityRequest asserted stream must be false, which is now wrong by design.

Acceptance, checked

criterion	how
a heartbeating turn survives past what a total deadline cut	`TestHeartbeatsKeepAStreamAliveThroughSilence` - three beats at 60ms under a 120ms idle bound, total read outlives it
no bytes at all fails at the idle timeout, faster than the ceiling	`TestASilentBackendFailsAtTheIdleTimeout`
the two failures are different sentences	`TestSilenceAndTheCeilingAreDifferentSentences`, plus `TestTheTurnCeilingStillEndsAStream` proving the ceiling does not collapse into silence
`1 + 1` does not fail	not verifiable from here - it needs the live lane. See below.

Also pinned: fragmented tool calls, two-call ordering, an unparsable heartbeat being skipped rather than fatal, a stream ending without [DONE], and reasoning staying unnamed.

What I deliberately did not do

The ceiling number. Kai's sizing note here says to clear litellm_request's 233.71s p99 with margin, and the guidance was to read #577's result before choosing. I read it, and it argues against picking a number in this PR: after Echo's lane went to 5m, 4 of 8 turns still landed within 1.5s of the new ceiling, and round 0 alone ranged 133.5s to 255.4s. A bigger number did not stop turns reaching the wall. #577 is open and consult, so the number stays Kai's and this PR ships the mechanism it was waiting on.

The fourth acceptance criterion is unverified. 1 + 1 dying at 181s needs the live lane to confirm, and nothing here reaches it. The mechanism that killed it is gone; whether it stays gone is a measurement someone should take against real traffic, which is currently about four turns a day (#577).

just gate passes.

closes #171

Kai approved this on 2026-08-15 and named one thing to read first: #577's investigation, which landed 2026-08-16. This is the consuming half. The emitting half shipped in agent-proxy `d8277f4` and has been inert since, exactly as its own doc predicted: > A **total** request deadline ignores heartbeats by construction. [...] so the consuming side must move to an idle or read timeout for any of this to change behaviour. That half lives in `sirens-echo`. This is the emitting side, and it is **inert until the consuming side lands**. ## The defect, stated precisely A total deadline fires on schedule however many bytes arrived. So a turn that was queued behind other work and a turn whose backend had hung died at the same second, and the member was told `turn timed out` for both. There were **two** total deadlines, not one - the turn context and `http.Client.Timeout`, both set to `cfg.RequestTimeout`. ## What changed **`stream: true`.** Heartbeats have nowhere to go on a non-streaming request, which is why the proxy documents them as streaming-only. **Two bounds instead of one.** * The turn context stays the ceiling. **That number does not move here** - see the last section. * `SIRENS_ECHO_MODEL_IDLE_TIMEOUT` (new, 45s) bounds *silence*. Any line resets it, and a heartbeat comment is a line. * `http.Client.Timeout` is now unset. Leaving it would have cut a streaming completion on schedule and reintroduced the defect one layer down. **Silence became its own failure.** `ErrModelSilent`, cause `model_silent`, span `sirens_echo.model.silent`, and the member is told the backend went quiet rather than that the turn timed out. That split is acceptance criterion 3 and it is the part that matters most: **retrying silence can work, and retrying a turn that ran the ceiling out while making progress cannot.** Telling a member to retry the second one is the true-sentence-that-misdirects shape from #449. ## Assembly Content and reasoning concatenate. Tool calls arrive as fragments keyed by `index`, so name and argument pieces are joined per call - a truncated argument string would reach the executor and fail for a reason that is not the model's. Calls are returned **in the order their indexes first appeared**, not sorted by index, because that is the order the model asked for them in. Pinned by a test that streams index 1 before index 0. Reasoning stays unnamed when the stream carried none. `reasoning_content` absent and `reasoning_content: ""` are echoed back differently on the next round, so inventing one would corrupt the roundtrip that `docs/sirens-echo-reasoning-roundtrip.md` exists to protect. ## The non-streamed shape still works Selected on `Content-Type`. Agent Proxy's own non-streaming surface still exists, and a backend that answers a streaming request with a whole body is read as one rather than failing a turn that arrived intact. That path gets no heartbeats, so the turn ceiling is its only bound - unchanged from today. This is also why the existing test fixtures still pass without being rewritten. One assertion did change and had to: `TestProxyClientSendsBoundedCommunityRequest` asserted `stream must be false`, which is now wrong by design. ## Acceptance, checked | criterion | how | | --- | --- | | a heartbeating turn survives past what a total deadline cut | `TestHeartbeatsKeepAStreamAliveThroughSilence` - three beats at 60ms under a 120ms idle bound, total read outlives it | | no bytes at all fails at the idle timeout, faster than the ceiling | `TestASilentBackendFailsAtTheIdleTimeout` | | the two failures are different sentences | `TestSilenceAndTheCeilingAreDifferentSentences`, plus `TestTheTurnCeilingStillEndsAStream` proving the ceiling does not collapse into silence | | `1 + 1` does not fail | not verifiable from here - it needs the live lane. See below. | Also pinned: fragmented tool calls, two-call ordering, an unparsable heartbeat being skipped rather than fatal, a stream ending without `[DONE]`, and reasoning staying unnamed. ## What I deliberately did not do **The ceiling number.** Kai's sizing note here says to clear `litellm_request`'s 233.71s p99 with margin, and the guidance was to read #577's result before choosing. I read it, and it argues against picking a number in this PR: after Echo's lane went to 5m, **4 of 8 turns still landed within 1.5s of the new ceiling**, and round 0 alone ranged 133.5s to 255.4s. A bigger number did not stop turns reaching the wall. **#577 is open and `consult`, so the number stays Kai's** and this PR ships the mechanism it was waiting on. **The fourth acceptance criterion is unverified.** `1 + 1` dying at 181s needs the live lane to confirm, and nothing here reaches it. The mechanism that killed it is gone; whether it stays gone is a measurement someone should take against real traffic, which is currently about four turns a day (#577). `just gate` passes. closes #171

coilyco-ops added 1 commit

2026-08-17 04:09:26 +00:00

feat(model): bound silence instead of the whole call

ci / image-build (pull_request) Successful in 24s

Details

ci / test (pull_request) Successful in 43s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

092e1ad65b

A total deadline fires on schedule however many bytes arrived, so a queued turn
and a hung one died at the same second and the member was told "timed out" for
both. Agent Proxy has emitted SSE heartbeats since d8277f4 on its side and
nothing here could see them, because the request asked for a whole body.

The call now streams. Any line resets an idle timer, a heartbeat comment counts
as a line, and the turn context stays the ceiling. `http.Client.Timeout` is
unset: it is a total timeout and would have reintroduced the same defect one
layer down.

Silence is now its own failure. `ErrModelSilent` carries it, `model_silent`
labels it, `sirens_echo.model.silent` spans it, and the member is told the
backend went quiet rather than that the turn timed out. Retrying silence can
work; retrying a turn that ran the ceiling out while making progress cannot,
and telling a member to do it is the sirens-echo#449 shape.

Content and reasoning concatenate. Tool calls arrive fragmented and are joined
per index, returned in the order their indexes first appeared rather than
sorted, because that is the order the model asked for them in.

A backend answering a streaming request with a whole JSON body is still read as
one, selected on Content-Type. Agent Proxy's non-streaming surface still exists
and failing a turn that arrived intact is the worse trade.

The ceiling itself does not move. Choosing that number is sirens-echo#577's,
which is open and reserved.

closes #171

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops requested review from coilysiren

2026-08-17 04:09:26 +00:00