Emit SSE heartbeats carrying attempt and backend state #123

Closed
coilyco-ops wants to merge 0 commits from sse-heartbeats into main
Member

Summary

Closes #104, the emitting half.

A caller waiting on a streaming completion cannot tell four states apart: admitted-and-queued, retrying, generating, and hung. All four look like silence followed by either a response or a deadline.

Wire shape

: {"state":"attempt","n":1,"of":2,"backend":"tower-3026","regime":"idle"}
: {"state":"upstream_started","backend":"tower-3026"}
: {"state":"attempt","n":2,"of":2,"backend":"litellm","regime":"hosted"}
data: {"id":"chatcmpl-...","choices":[{"delta":{"content":"Paris"}}]}

Lines beginning with : are SSE comments. Every spec-compliant client ignores them, so a consumer that does not parse them sees the same data: frames as before - which is the reason the issue preferred comments over empty-delta chunks.

PROXY_HEARTBEAT_INTERVAL (default 10s, 0 disables) repeats the current state so a state that persists stays visible without a transition.

Retry visibility is what earns this

sirens-echo#137 records three attempts burning ~9s before a turn 502s, and from outside that is indistinguishable from one slow attempt. attempt n of N turns a silent fallback into something a caller can log, display, and alert on. test_attempt_number_surfaces_a_silent_fallback is that case end to end.

Queue position is not carried

#105 measured admission delay at 0.7-4.5ms across 29 traces while queue.wait itself varied 25-fold, so there is nothing to report at current traffic. It is also moot on this path: the streaming surface calls dispatch_stream directly and never touches the queue. Whether position is worth carrying under concurrency is #107's question, and the payload has room for it when that is answered.

This does mean the issue's first acceptance line - "a streaming request that waits in admission receives at least one heartbeat" - is vacuously true rather than demonstrated, because a streaming request does not wait in admission today. Flagging that rather than claiming it.

How to verify

  1. ward exec test - tests/test_heartbeats.py, 8 cases.
  2. test_attempt_state_precedes_the_first_content_delta asserts the comment arrives before any content.
  3. test_a_consumer_ignoring_comments_sees_unchanged_output asserts the data: frames are unchanged against a heartbeats-disabled run.
  4. test_keepalives_fire_while_a_state_persists asserts the shielded read is never lost to a tick.

Test plan

  • New suite for the behaviour
  • Existing tests still pass (338 passed)
  • ward exec format-check, lint, typecheck, pre-commit all clean

Risk

Low, and bounded by the SSE spec. The one real exposure is a non-spec-compliant client that chokes on comment lines; PROXY_HEARTBEAT_INTERVAL=0 still leaves the state markers, so a deployment that needs the old byte stream exactly wants the emission off rather than the interval at zero. Say the word and I will add that switch.

This changes nothing a user sees on its own

The issue says so plainly and I am repeating it: a total request deadline ignores heartbeats by construction. sirens-echo must move to an idle or read timeout before any of this alters an outcome. That half lives in that repository.

Note

Branched off backend-regime-spans (#122), which sits on #120, #119, #118. The regime field on attempt comes from #122. Review the chain in order.

## Summary Closes #104, the emitting half. A caller waiting on a streaming completion cannot tell four states apart: admitted-and-queued, retrying, generating, and hung. All four look like silence followed by either a response or a deadline. ## Wire shape ``` : {"state":"attempt","n":1,"of":2,"backend":"tower-3026","regime":"idle"} : {"state":"upstream_started","backend":"tower-3026"} : {"state":"attempt","n":2,"of":2,"backend":"litellm","regime":"hosted"} data: {"id":"chatcmpl-...","choices":[{"delta":{"content":"Paris"}}]} ``` Lines beginning with `:` are SSE **comments**. Every spec-compliant client ignores them, so a consumer that does not parse them sees the same `data:` frames as before - which is the reason the issue preferred comments over empty-delta chunks. `PROXY_HEARTBEAT_INTERVAL` (default 10s, `0` disables) repeats the current state so a state that persists stays visible without a transition. ## Retry visibility is what earns this `sirens-echo#137` records three attempts burning ~9s before a turn 502s, and from outside that is indistinguishable from one slow attempt. `attempt n of N` turns a silent fallback into something a caller can log, display, and alert on. `test_attempt_number_surfaces_a_silent_fallback` is that case end to end. ## Queue position is not carried #105 measured admission delay at 0.7-4.5ms across 29 traces while `queue.wait` itself varied 25-fold, so there is nothing to report at current traffic. It is also **moot on this path**: the streaming surface calls `dispatch_stream` directly and never touches the queue. Whether position is worth carrying under concurrency is #107's question, and the payload has room for it when that is answered. This does mean the issue's first acceptance line - "a streaming request that waits in admission receives at least one heartbeat" - is vacuously true rather than demonstrated, because a streaming request does not wait in admission today. Flagging that rather than claiming it. ## How to verify 1. `ward exec test` - `tests/test_heartbeats.py`, 8 cases. 2. `test_attempt_state_precedes_the_first_content_delta` asserts the comment arrives before any content. 3. `test_a_consumer_ignoring_comments_sees_unchanged_output` asserts the `data:` frames are unchanged against a heartbeats-disabled run. 4. `test_keepalives_fire_while_a_state_persists` asserts the shielded read is never lost to a tick. ## Test plan - [x] New suite for the behaviour - [x] Existing tests still pass (338 passed) - [x] `ward exec format-check`, `lint`, `typecheck`, `pre-commit` all clean ## Risk Low, and bounded by the SSE spec. The one real exposure is a non-spec-compliant client that chokes on comment lines; `PROXY_HEARTBEAT_INTERVAL=0` still leaves the state markers, so a deployment that needs the old byte stream exactly wants the emission off rather than the interval at zero. Say the word and I will add that switch. ## This changes nothing a user sees on its own The issue says so plainly and I am repeating it: a **total** request deadline ignores heartbeats by construction. `sirens-echo` must move to an idle or read timeout before any of this alters an outcome. That half lives in that repository. ## Note Branched off `backend-regime-spans` (#122), which sits on #120, #119, #118. The `regime` field on `attempt` comes from #122. Review the chain in order.
Emit SSE heartbeats carrying attempt and backend state
Some checks failed
ci / quality (pull_request) Failing after 21s
ci / smoke (pull_request) Successful in 6s
d8277f41c9
Issue #104: a caller waiting on a streaming completion cannot tell four states
apart. Admitted and queued, retrying after a failure, generating, and hung all
look like silence followed by either a response or a deadline. sirens-echo
resolved that by giving up at ~179.5s and reporting a timeout, including in
cases where the proxy was still doing legitimate work.

Lines beginning with a colon are SSE comments, which every spec-compliant client
ignores, so a consumer that does not parse them sees the same data frames it saw
before. That is why the issue preferred comments to empty-delta chunks, which
some OpenAI-compatible clients mishandle.

dispatch_stream yields a state marker before each chain entry and once the first
real chunk arrives, and the streaming surface turns those into comment lines
rather than deltas. attempt carries n, of, backend, and the backend's regime from
#109. A keepalive repeats the current state every PROXY_HEARTBEAT_INTERVAL
seconds, default 10, so a state that persists stays visible without a
transition.

Retry visibility is what earns this. sirens-echo#137 records three attempts
burning ~9s before a turn 502s, and from outside that is indistinguishable from
one slow attempt. attempt n of N turns a silent fallback into something a caller
can log, display, and alert on.

Queue position is not carried. Issue #105 measured admission delay between
0.7ms and 4.5ms across 29 traces while queue.wait itself varied 25-fold, so
there is nothing to report at current traffic. It is also moot on this path: the
streaming surface calls dispatch_stream directly and never touches the queue.
Whether position is worth carrying under concurrency is #107's question, and the
payload has room for it when that is answered.

_with_keepalives shields the in-flight read while it waits, so a keepalive tick
never cancels the chunk it was waiting for. Setting the interval to 0 removes
the keepalives and leaves the state markers.

Each emission increments llm_stream_heartbeats_total and adds a stream.heartbeat
event to the request span, because the issue asked that a silent heartbeat path
not be able to regress unnoticed.

This changes nothing a user sees on its own, and the issue says so plainly. A
total request deadline ignores heartbeats by construction, so sirens-echo must
move to an idle or read timeout before any of this alters an outcome. That half
lives in that repository. This is the emitting side.

closes #104

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>
Close the wrapped stream cleanly when a caller stops reading
All checks were successful
ci / quality (pull_request) Successful in 28s
ci / smoke (pull_request) Successful in 8s
0bbf45932d
The keepalive wrapper advances its source through a task so a tick can fire
while a read is outstanding. When the consumer stops early, that task was
cancelled and abandoned, which leaves the source async generator mid-step. The
generator's own close then raises from a state it cannot unwind, and Python
reports it as an unraisable error at collection time - so whether it surfaces
depends on when garbage collection runs, which is exactly the kind of failure
that reproduces on a loaded runner and not on a laptop.

The wrapper now awaits the cancelled read before dropping it, then closes the
source explicitly rather than leaving it to the collector.

The keepalive test also waited on a fixed sleep for a tick that is only
guaranteed to be prompt on an idle machine. It waits for the behaviour now.

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>
coilysiren closed this pull request 2026-08-13 19:41:57 +00:00
All checks were successful
ci / quality (pull_request) Successful in 28s
ci / smoke (pull_request) Successful in 8s

Pull request closed

Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
coilyco-flight-deck/agent-proxy!123
No description provided.