A chat turn should end in exactly one earned terminal

[WaylandYang]

The family

Four open PRs each close one hole, and they are the same hole:

PR	What was reported as success
#845	The model's final text was tool-control markup, streamed and persisted as the answer
#850	A database failure during fallback retrieval became "no evidence found", then done
#852	The assistant message failed to save, and done was emitted anyway
#851	The stream ended without a terminal — or ended normally and fired onDone twice

Each fix is correct and each should land. But four guards written separately means the fifth leak is written the same way, by whoever next adds a path that can fail after the first byte is out.

The rule underneath

A chat turn ends in exactly one terminal, and the terminal is earned.

Today "ended" is inferred in several places from several things: the loop finished, the stream closed, the provider said stop, no error was raised. None of those is the same as the answer exists and was kept. The four fixes above are each a local repair of one inference.

What would make the fifth leak hard to write is a single representation the producer must return — a value that is either the answer, or a reason — with the SSE layer emitting exactly one terminal from it and the client accepting exactly one. Then "the loop ended" stops being a code path that can reach done at all, because done is not reachable without the value.

The client parser is a separate problem, and it should be a library

streamSse in web/src/api.ts hand-rolls SSE framing, and it is wrong in ways the spec is explicit about:

while ((idx = buf.indexOf("\n\n")) >= 0) {      // CRLF and bare CR are also frame separators
  ...
  else if (line.startsWith("data:")) data += line.slice(5).trim();   // multiple data: lines join with \n
}                                                                     // and trim() eats meaningful whitespace

• Frames are split on \n\n only. The spec allows \r\n\r\n and \r\r.
• Multiple data: lines in one event must be joined with \n. Here they are concatenated with nothing, so a multi-line payload is silently corrupted.
• .trim() on each data line destroys leading and trailing whitespace that belongs to a delta.
• Split UTF-8 across chunk boundaries is handled (decoder.decode(value, {stream: true})), which is the one part that is right.

The server side is already on a framework — axum::response::sse::{Event, KeepAlive, Sse} — so this is the one hand-written half. The browser's native EventSource is not a candidate because these streams are POSTs with a body, but the ecosystem has small focused parsers for exactly this shape (eventsource-parser is the common one; whoever picks should check its maintenance status rather than take this as a recommendation of a version). Feed it the chunks, keep our own event dispatch and reattachment policy on top.

web/package.json currently has no SSE dependency at all.

These two are complementary, not alternatives. A spec-correct parser fixes the framing bugs and none of the terminal-semantics bugs — onDone firing twice and an unterminated EOF reporting success are our logic, not the parser's. #851 is where that discipline belongs; the framing underneath it is what should stop being ours.

Suggested order

1. Land Hand off exhausted tool runs to an evidence-only final answer #845, Keep fallback retrieval failures distinct from empty results #850, Preserve exact chat outcomes across stream closure and reattachment #851, Do not report fallback chat completion when saving fails #852 as they are. Each is a real fix with a regression behind it.
2. Replace the framing in streamSse with a parser library, keeping the terminal handling Preserve exact chat outcomes across stream closure and reattachment #851 establishes.
3. Then introduce the single producer terminal, and delete the guards that become unreachable.

Step 3 is the one that stops this list from growing, and it is only worth doing after 1 and 2 have settled what the terminal has to carry.

Background

Noticed while reviewing the batch on 2026-09-21. Related: #844 (the report behind #845), and the same shape appeared earlier this week on the export side — #821, #823, #824, #831 — where the store held something and the serializer dropped it without an error. Silent success is the recurring failure mode in both places.

[WaylandYang]

Two corrections to the framing above, one of emphasis and one that is the actual missing piece.

This is mostly a backend problem

The parser discussion takes up more room above than it earns. By what the four PRs actually change:

PR	backend	frontend
#845	agent.rs, chat.rs, chat_finalization.rs, utopia-llm	none
#850	chat.rs	none
#852	chat.rs	none
#851	live.rs, chat.rs — a reattachment between the terminal broadcast and registry removal receives a snapshot without the terminal	api.ts framing

Three of the four are entirely server-side, and #851 has a genuine server race that has nothing to do with how the client parses bytes.

The three backend ones are one Rust habit:

retrieval fails    → treated as an empty result      (#850)
the insert fails   → return value ignored            (#852)
the model returns markup → accepted as the answer    (#845)

A failure turned into an empty-but-valid value. No library changes that. A spec-correct SSE parser fixes framing and fixes none of these: it governs whether a sentence was heard intact, not whether the speaker reported failure as success.

The missing piece is an end-to-end acceptance test of the rule

Each of these PRs ships a regression that exercises real handlers against a real Postgres with a local model substitute — that part is done well. But each test catches its own leak. Four tests that each catch one leak do not catch the fifth, which is the thing this issue exists to prevent.

What would is a single acceptance test that injects a failure at each point the chat path can fail after the first byte is out, and asserts the same three things every time:

• no done reaches the client
• exactly one terminal is observed
• the terminal names a reason

Injection points that exist today, each already reachable in a test fixture because these PRs built the fixtures:

where	how it is already injected somewhere
provider returns tool-control markup	#845's scripted upstream
provider returns empty text	existing empty-reply tests
retrieval errors mid-request	#850 closes the fixture pool after the tool request lands
assistant insert rejected	#852's fixture-scoped trigger
stream cut mid-answer	#851's disconnect case
terminal broadcast lost before registry removal	#851's race

Written as a table-driven test rather than six tests, a seventh row is what a new failure mode costs — and a new path that can fail without a row is what review looks for. That is the difference between the list stopping and the list growing.

The cheapest version does not wait for step 3 in the plan above: the assertions hold against today's code for the rows that are fixed, and fail for any that are not, so it can land alongside these PRs and grow with them.

[WaylandYang]

Closing: the family has landed and so has the rule underneath it. #845, #850, #851 and #852 closed the four holes, and #903 put the single representation in place: the producer returns one outcome, the stream layer emits exactly one terminal from it, and the client reads frames with eventsource-parser instead of the hand-rolled splitter. The terminal table in chat_terminal_tests.rs is where a new failing path adds its row.