fix(ENG-887): agent thoughts stream as one unbroken paragraph - #244
fix(ENG-887): agent thoughts stream as one unbroken paragraph#244hazemahmedx0 wants to merge 2 commits into
Conversation
- arm a paragraph break when an LLM round completes (StreamComplete), consume it on the next round's first text delta - the break rides the delta itself, so the live stream, the persisted message, and replays all get the same paragraphing from one place - skip the break after a max_tokens truncation with no tool calls: its continuation round resumes mid-sentence - skip the break when the previous round already ends with a blank line Refs ENG-887
No PR environment for this pull requestAdd the Updated on every push to this PR. |
hazemahmedx0
left a comment
There was a problem hiding this comment.
Traced this through anton's session loop and the responses handler. The fix is in the right place.
Putting the break on the delta itself is what makes it hold. responses.py builds the persisted message from the response.output_text.delta payloads via event_sink, and stashes those same payloads in the events log for replay — so the live stream, the stored message, and reload all pick up the paragraphing from one spot. Nothing to drift out of sync.
The max_tokens skip is a faithful mirror of anton's own check: stop_reason in ("max_tokens", "length") and not tool_calls appears verbatim at session.py:1716 and session.py:2117. So the formatter can't disagree with anton about whether a continuation round is coming. That holds on the OpenAI Responses path too, where stop_reason is the response status rather than a finish reason — anton doesn't detect truncation there either, so there's no continuation round, so breaking is correct.
Keeping text_tail instead of inspecting collected_text[-1] is the right call: a round ending "…\n" + "\n" across two deltas would otherwise get a second blank line. Test 5 covers exactly that.
Two nits, neither blocking:
-
A round that streams no text and ends
max_tokenswith no tool calls disarms a break armed by the round before it, so those two narration rounds fuse again. Reachable when a round spends its whole output budget on thinking tokens. Checked it:"Sentence one."→ complete(tool_use) → complete(max_tokens, no text) →"Sentence two."gives"Sentence one.Sentence two.". Rare, and the failure is just the old behaviour, but if you want it: only evaluate the exception when the round actually streamed text. -
if collected_textis a truthiness check on the list, so a leading empty-string delta (anthropic'stext_deltabranch doesn't guard"") makes it truthy and the first break lands at index 0 — message starts with"\n\n".any(collected_text)covers it. Cosmetic; markdown eats it.
Tests are good — real anton dataclasses instead of mocks, and "".join(deltas) == completed pins the invariant that actually matters. I poked the uncovered shapes too (back-to-back StreamCompletes, a tool-only first round, stop_reason=None, single trailing newline); all behave.
tests/test_stream_round_breaks.py: 5 passed. Approving.
Review follow-ups: - a truncated round that streamed no visible text (thinking-token budget exhaustion) no longer disarms a break armed by the previous narration round — the truncation belongs to text the user never saw - gate the break on text_tail instead of collected_text so an empty leading delta can't put a blank line before the first visible text Refs ENG-887
hazemahmedx0
left a comment
There was a problem hiding this comment.
Re-reviewed a344ee7. Both fixes are right.
Splitting out truncated and gating on round_had_text reads better than the single negation it replaced — the three cases are explicit now, and "leave the armed break alone" is the one that needed a name. Good detail that round_had_text reads event.text and not the possibly-prefixed text: a round whose only output was the synthetic break hasn't said anything, and the break has already landed by that point, so nothing double-fires.
Overloading text_tail as the "has anything visible streamed yet" flag rather than adding a second bool is the right trade. Once it's non-empty it can never go back, so it's a monotone flag, and the comment says as much.
Checked 15 shapes against the new logic, including ones the tests don't cover:
- truncated-with-text round followed by a zero-text truncated round → stays seamless (
"A.\n\nwin is Avatar.") - empty delta that consumes a break mid-message → break still lands (
"A.\n\nB.") - zero-text truncated first round → no leading blank line
- back-to-back
StreamCompletes,stop_reason=None,"length", and the OpenAI Responses"incomplete"status
All as expected, and every case holds "".join(deltas) == completed.
7 tests pass. Also ran the other files that touch the delta and persistence path (channels smoke, cell status, inline artifacts, turn capture, tool-call history, tenancy, delete cascade): 85 passed.
Still approving.
|
Verified before/after in the real desktop app: replayed the same multi-round turn through the formatter at staging and at this branch, persisted each as a conversation against an isolated local server, and opened both in the app. Before (staging): After (this branch): the same content renders as four paragraphs, one per thinking round. Screenshots are on ENG-887 (Linear-hosted images don't render durably here). |
Fixes ENG-887
What
During a tool-heavy turn, anton streams narration in several LLM rounds (narrate → run tools → narrate …). The formatter pushed every round's text into the same output item with no separator, so rounds fused mid-sentence:
The glued text hit all three surfaces at once: the live SSE stream, the assistant message we persist (
"".join(collected_text)), and every replay when a conversation is reopened.Fix
format_responses_streamalready receives the round boundary —StreamComplete— and ignored it. Now it arms a paragraph break there and prepends\n\nto the next round's first text delta. Because the break rides the delta itself, the live stream, the persisted message, and replays all pick it up from the same place. No wire-format change, no client change; the renderer's markdown turns the blank line into a paragraph.Two boundaries deliberately stay seamless:
max_tokenswith no tool calls — anton resumes it mid-sentence with a continuation round, so a break there would split a word in halfTesting
uv run python -m pytest tests/test_stream_round_breaks.pycovers the round break (live deltas and final text), the untouched single-round case, both max_tokens shapes, and the existing-blank-line case.Full suite: 667 passed. The one failure (
test_comments_layer.py::test_serve_injects_only_with_flag) also fails on clean staging — unrelated to this change.Glossary
StreamCompletestop_reasonand tool calls — the signal this fix keys on.max_tokensmid-sentence with no tool calls, anton asks the model to continue exactly where it left off. Text across that boundary is one sentence, so it must not get a paragraph break.