feat(acp): stream an agent's reply into the channel as it is written - #3263
feat(acp): stream an agent's reply into the channel as it is written#3263noah-shipley-centric wants to merge 1 commit into
Conversation
ACP delivers an answer as a run of `agent_message_chunk` updates. The harness logged each one to `acp::stream` and dropped it, so a channel saw nothing until the agent posted a finished message. Every token was already arriving — it just never left the log. Publish it instead: post once on the first meaningful text, then edit that same message as more arrives. A stream is one message that grows, not a wall of fragments. This works cleanly because edits (kind 40003) are already excluded from CHANNEL_MESSAGE_EVENT_KINDS, so a streamed answer does not turn into hundreds of unread pings. Relaying every chunk would be worse than not streaming, so `stream_flush` holds a coalescing policy: - the first publish ignores the throttle — delaying the first sign of life is exactly backwards, that is when a reader most needs to know the agent is working; - mid-turn edits need both a minimum delta and a throttle interval; - end-of-turn always flushes, so the tail is never dropped; - an empty turn publishes nothing and never blanks an existing message; - bodies are clamped under build_edit's 64 KiB limit, truncating on a char boundary — a rejected edit would strand the message mid-answer, and a naive byte slice would panic on a multi-byte codepoint. The sender is scope-local in the turn driver on purpose: dropping it IS the end-of-turn signal, so success, cancel, idle timeout and agent exit all flush without any path having to remember to. The driver awaits the streamer so the complete reply is on the relay before the turn reports its outcome. Off by default (BUZZ_ACP_STREAM_REPLIES) — this changes what every channel sees. BUZZ_ACP_STREAM_THROTTLE_MS and BUZZ_ACP_STREAM_MIN_DELTA tune it. 18 tests: every policy branch, plus the post-once-then-edit contract driven through an injected publisher, covering the empty turn, the dropped tail, and a failed post — which must not leave the policy believing there is a message to edit, or the reply would vanish into orphan edits. Signed-off-by: Noah Shipley <[email protected]>
|
Update from running this in production, since it changes what a reviewer should expect. It works end to end — enabled against a live relay, and a kind:40003 edit landed against the turn's posted message, targeting the right event id. The lifecycle behaved: sender dropped at turn end, final flush applied, no orphan edits. But the visible effect depends on the harness. With That is still useful — the room stops looking idle during a 2-minute turn, which was the original complaint — but it is not the same as streaming the final answer, and I would rather say so than let the PR title imply more than it delivers. Worth deciding upstream: for harnesses where the agent replies through the ACP result rather than a side-channel CLI, this streams the answer proper. For No code change proposed here — flagging the semantics so the default-off flag is judged on what it actually does. |
Problem
ACP delivers an answer as a run of
agent_message_chunkupdates.handle_session_updatelogged each one toacp::streamand returned:So a channel sees nothing until the agent posts a finished message. Every token was already arriving — it just never left the log. From the room, the agent goes quiet for a minute and then a whole answer appears at once.
Approach
Post once on the first meaningful text, then edit that same message as more arrives. A stream is one message that grows, not a wall of fragments.
This works cleanly because kind 40003 edits are already excluded from
CHANNEL_MESSAGE_EVENT_KINDS— the existing comment says edits "can land after the last human-visible message and would otherwise create phantom unreads". That is exactly the property streaming needs: a streamed answer does not become hundreds of badge pings.The policy (
stream_flush.rs, pure)Relaying every chunk would be worse than not streaming, so publishing is coalesced:
no_tokens_are_lost_between_the_last_edit_and_turn_end).build_edit's 64 KiB limit, truncating on a char boundary — a rejected edit would strand the message mid-answer, and a naive byte slice would panic on a multi-byte codepoint.No clock, no I/O: the caller supplies
now_msand performs the publish.Lifecycle
The sender is scope-local in the turn driver on purpose — dropping it is the end-of-turn signal, so success, cancel, idle timeout and agent exit all flush without any path having to remember to. This mirrors the existing
ReactionGuardpattern.The driver awaits the streamer before reporting the outcome, so the complete reply is on the relay before anything observes "turn complete" — otherwise a caller could see a finished turn against a message still missing its last edit.
Only channel turns stream; a heartbeat has no channel to write into.
Safety
Off by default (
BUZZ_ACP_STREAM_REPLIES) — this changes what every channel sees, so it is opted into rather than inherited.BUZZ_ACP_STREAM_THROTTLE_MS(1000) andBUZZ_ACP_STREAM_MIN_DELTA(24) tune it. With it unset, behaviour is byte-for-byte what it was: chunks are logged and dropped.Tests
18 tests — every policy branch, plus the post-once-then-edit contract driven through an injected
StreamPublisherfake:Notes
Independent of #3192 and #3196 — different files, no shared commits.
Not yet exercised against a live relay; the publish path is covered by the injected-fake seam rather than an end-to-end run. Happy to add an env-gated integration test if you would rather see one before merging.