Streaming error propagation contract

[JMAN730]

Part of #25

Blocked by #28

Question

streamComplete catches exceptions and emits them as chat content (OpenAIProvider.kt:99), so a failure reads as a model answer. What is the contract instead — the Flow throws the classified error, or emits a typed terminal event? What happens to partial content already emitted before the failure (keep and mark truncated, or discard)? How do collectors in the agent/chat path distinguish an error from a completed stream? Real SSE streaming is out of scope.

[JMAN730]

Resolution

A stream is a Flow<String> that either completes normally — meaning the provider observed its terminal sentinel and emitted at least one chunk — or completes exceptionally with LLMException. There is no third outcome, and no error text ever enters a chunk.

Grilled self-directed on the maintainer's delegation. Below: every question, the decision, and what would reverse it.

Ground truth read first

• streamComplete is fake streaming everywhere in scope: eleven providers call complete(request), split the string on spaces, and emit words with a 50 ms delay (OpenAIProvider.kt:89-104, ClaudeProvider.kt:116-127, GeminiProvider.kt:151-162, and eight more of identical shape). Real SSE is out of scope for API reliability & error surfacing map (upstream issue 13) #25, so today a failure can only happen before the first token. The contract below is therefore written for a stream that can fail mid-flight, and is satisfied trivially by the current fake implementation.
• Every one of those eleven ends catch (e: Exception) { emit("Error streaming X: ${e.localizedMessage}") } — the bug in this ticket's framing.
• There is exactly one collector: AgentLoop.executeSimpleQuery, AgentLoop.kt:432-443. Nothing else calls streamComplete.
• The second stream API, AIProvider.generate(): Flow<StreamChunk> (LLMProvider.kt:22-51), has zero consumers — the only calls are HybridOnDeviceProvider.kt:162,168 delegating to itself.
• ConversationDao.insertMessage is @Insert(onConflict = REPLACE) (ConversationDao.kt:15), so AgentLoop.kt:442 re-inserting the same id per chunk is an upsert. Partial text is already persisted per chunk today.
• ChatMessage (ChatMessage.kt:6-14) and ConversationEntity (ConversationEntity.kt:12-20) have no error or completeness field. A transcript row cannot currently say "this answer is cut off".

---

Q1. Does a mid-stream failure throw, emit a terminal error element, or complete silently?

Decision: it throws. streamComplete fails the Flow with LLMException (#28). A terminal StreamEvent.Failed element is rejected.

Reasoning, argued against my own preference: the terminal-element case is real — it puts the partial text and the error in one ordered sequence, and a collector cannot forget to look at an element the way it can forget a try. But an in-band error element is what we have today, one type down: emit("Error streaming OpenAI: …") is a terminal error element whose type is String, and it fails exactly because a collector that doesn't special-case it renders it as an answer. Giving it a distinct type raises the cost of the mistake without removing it — a when with no Failed branch, or an if (chunk is Content), and the error is gone again. The exception channel is the one channel a collector cannot silently drop: an unhandled one crashes the coroutine and reaches AgentLoop.kt:469.

It also isn't a free choice. OkHttp read failures and JSON decode failures inside a real SSE loop are exceptions. Option B keeps the exception channel and adds a second one, so every provider converts, and consumers must handle both. One channel, not one and a half.

Silent completion is banned outright. This is the sharper half of the decision. A real SSE stream that ends because the socket closed cleanly mid-generation, with no [DONE]/message_stop, completes normally under a naive implementation — the truncated answer renders as a whole one, with no error anywhere. So: a provider that does not observe its own terminal sentinel MUST throw, and normal completion is thereby a positive assertion that the answer is whole. The success signal is the absence of an exception, and it is only reachable deliberately.

finish_reason: "length" (our own maxTokens at AgentLoop.kt:437) is a normal completion, not an error — the model stopped where we told it to. Left open below.

Reverses if: a second consumer appears that must survive a stream error without an exception crossing its boundary — a background prefetch, or a Flow exposed straight to Compose. Then a terminal element earns its keep. One collector today, so it does not.

Q2 / Q5. What happens to tokens already emitted — kept or discarded, and is the partial kept in the transcript?

Decision: kept in the transcript, marked incomplete, and the error rendered separately — never appended into the partial's text.

Discarding is defensible: a truncated answer can invert its own meaning, and deleting the row is one deleteMessage call. But the text has already been on screen for ten seconds (AgentLoop.kt:442 writes per chunk), so discarding is a visible retraction of something the user already read, and it destroys the only evidence for a bug report.

The load-bearing consequence is that the partial is not cosmetic. AgentLoop.kt:413 builds the next prompt from conversationRepository.getLastMessages(sessionId, 10) — the conversation table, not memory. So the partial row goes into the next turn's context whatever we do with memoryManager. A truncated assistant turn presented to the model as a complete one is a silent context poison, and skipping memoryManager.storeMessage (AgentLoop.kt:464) does not prevent it. Therefore the row must be distinguishable, not merely present.

Consequence: ConversationEntity gains a nullable completeness marker — a new column, e.g. incomplete: Boolean? = null, surfaced on ChatMessage the same way, plus a Room migration. Explicitly not overloaded onto modelBadge: that is a display string the badge UI reads, and stuffing state into it is the same category of mistake as stuffing an error into text, moved one field over.

incomplete means cut off, not errored. Cancellation sets it too (Q6). The error surface is separate and is #31's.

Reverses if: #31's prototype finds a carrier that renders the partial-plus-error pair without a schema change — the flag is one column and a migration, cheap to drop. Note #42 already flags Room migrations as untested by CI.

Q3. How does a non-200 before the first token differ from a break after 500 tokens?

Decision: not by position. The split is "is there classifiable material?"

The tempting rule — pre-first-token gets a real category, post-first-token gets MalformedResponse — is wrong, and #27 says why: OpenAI and others emit an in-band data: {"error": {...}} frame after HTTP 200, carrying a full, parseable envelope. Under a position rule, a mid-stream rate limit is reported as "the provider sent us garbage" and the user is told to file a bug instead of to wait. Three cases:

1. HTTP response unsuccessful — no tokens can have been emitted. Classify from status + headers + body exactly as Error taxonomy and where classification lives #28 (LLMErrorMapper.classify(...)). Real category, real user action.
2. HTTP 200, then an in-band error frame that parses — same mapper, same dialect, status = null, body-only branch. Real category. Error taxonomy and where classification lives #28's mapper already has to tolerate a body-only classification because of the non-JSON tiers.
3. HTTP 200, then the stream breaks — see Q4.

The collector's own accumulator distinguishes "I have a partial" from "I have nothing"; the exception does not need to say so.

Sub-decision: no emittedTokens field on LLMException. It would duplicate state the collector already holds (currentReplyText, AgentLoop.kt:422) and the two can disagree. Reverses if: a collector appears that does not accumulate — the Benchmark path (#32) or a voice pipeline — and still needs to phrase "partial answer" copy.

Q4. How are SSE parse errors, truncated JSON, and socket timeouts distinguished?

Decision: by cause, not by position — which narrows #28.

#28 reserved MalformedResponse for "HTTP 200, unparseable or post-200 stream failure". Taken literally that folds a dropped socket into a category whose user action is "report a bug", for what is the user's own flaky wifi. #28's own test for a category is a distinct user action, and these are distinct. So:

what happened	category	retryable
no HTTP response at all (DNS, connect, socket timeout pre-response)	Network(cause)	yes
socket reset / read timeout mid-stream, after 200	Network(cause)	yes, gated by Q7
a frame arrives but is unparseable / truncated JSON	MalformedResponse	no
stream ends cleanly, no terminal sentinel	MalformedResponse	yes, gated by Q7
in-band error frame that parses	per #28 mapper	per mapper

The distinction is mechanically available at the read site and is not a judgement call: OkHttp's BufferedSource.readUtf8Line() returns null at clean EOF and throws SocketTimeoutException/IOException on transport failure. Clean-EOF-without-sentinel is MalformedResponse because nothing broke at transport level — the provider handed us a truncated document.

retryable for a truncated stream is true (transient); retryable for an unparseable frame is false (retrying gets the same garbage). This refines #28's per-category verdicts for stream-origin errors only; MalformedResponse from complete()'s Gson path stays false.

Reverses if: the first real-SSE implementation finds a provider whose clean-EOF is a normal, sentinel-free termination — then case 4 has to become per-dialect rather than universal.

Q6. How is cancellation told apart from failure?

Decision: CancellationException propagates untouched. It is never classified, never rendered, never written.

Today it is not distinguished, and the failure is concrete, not theoretical. catch (e: Exception) catches CancellationException. The sharpest case is HybridOnDeviceProvider.kt:129-136: on any Exception it retries the whole generation on the fallback backend — so pressing Stop mid-stream starts a second generation on another backend and emits "[Switched to …]" into the transcript. The other ten turn Stop into an "Error streaming X: … was cancelled" chat bubble, or try to. AgentLoop.kt:444's catch (streamError: CancellationException) { throw streamError } is correct and currently unreachable from inside a provider, because the provider ate it first.

Implementation: the eleven catch (e: Exception) blocks are deleted, not amended. Under "the Flow throws", there is nothing left for them to do — the classified throw from LLMErrorMapper is the whole error path. Cancellation then propagates by default, which is the Kotlin default we stopped fighting.

Where an operator does sit in the chain (Q7), the rethrow is written explicitly rather than relying on Flow.catch's treatment of cancellation:

.catch { t -> if (t is CancellationException) throw t else throw classify(t) }

A cancelled stream marks its partial row incomplete (Q2) and shows no error — state returns to Idle via the existing cancelCurrentTask path (AgentLoop.kt:676).

Reverses if: nothing plausible. This one is not close.

Q7. Where does #28's classification plug into the stream?

Decision: the provider classifies at the throw site (unchanged from #28); WrappedLLMProvider nets the rest with an operator chain, not a try/catch.

This is the hole #28 could not have seen. #28's safety net is a try/catch in the wrapper — correct for complete(), which is suspend. But streamComplete returns a cold Flow (LLMProviderFactory.kt:147-150): the delegate's body never runs inside the wrapper method, it runs in the collector's coroutine at collection time. A try/catch around delegate.streamComplete(...) catches literally nothing. The net has to be operators.

WrappedLLMProvider.streamComplete (LLMProviderFactory.kt:147) becomes, in shape:

override fun streamComplete(request: LLMRequest): Flow<String> {
    val rewritten = rewriteRequestIfNeeded(request)   // + #29's resolved model
    var emitted = false
    return delegate.streamComplete(rewritten)
        .onEach { emitted = true }
        .catch { t ->
            if (t is CancellationException) throw t
            throw LLMErrorMapper.wrap(t, provider = delegate.name, model = rewritten.resolvedModel,
                                      allowRetry = !emitted)
        }
        .onCompletion { cause ->
            if (cause == null && !emitted) throw LLMException(
                LLMError.MalformedResponse("empty completion"), /* … */)
        }
}

Three jobs in one place, and it is the seam #28 chose for classification and #29 chose for model resolution — the same layer, now doing a third thing on the same request. Signature of the interface method is unchanged: fun streamComplete(request: LLMRequest): Flow<String> (LLMProvider.kt:33). See Q8.

allowRetry = !emitted encodes the constraint for #30: a stream is retryable only before its first emission. After the first token, retryable is forced false regardless of the mapper's verdict, because replaying a stream that already put 500 tokens on screen either duplicates them or requires a rollback the transcript cannot express. #30 gets this as a given, not a choice. The wrapper counts locally — it is in the operator chain, so it can, and the exception stays field-free (Q3).

Reverses if: real SSE lands with resumption tokens (some providers support continuing from an offset), which would make post-emission retry expressible. None of the eleven offer it today.

Q8. Does the type signature change?

Decision: no. Flow<String> stays. This ticket is a behavioural contract plus a wrapper, not a type change.

The case for Flow<StreamChunk> is real — the interface carries two streaming APIs, generate() is a defaulted wrapper around streamComplete (LLMProvider.kt:36-51), and merging them deletes a duplicate. But generate() has zero consumers, the merge buys nothing for error propagation, and it touches the one collector at AgentLoop.kt:432 for a reason unrelated to this map. Everything this ticket needs — throw vs complete, cancellation vs failure, partial retention — is expressible on Flow<String> because Kotlin already gives a Flow an exception channel and a completion signal.

The same rule binds generate(): Flow<StreamChunk>: throw, never emit(StreamChunk.Content("Error: …")). That deletes GemmaProvider.kt:137,141,176, LiteRTLMProvider.kt:257, HybridOnDeviceProvider.kt:156,172 — except those four are on-device and #28 held them out of scope. They stay as-is, covered by the wrapper net only, exactly as #28 ruled. The contract is stated for both APIs; enforcement is the eleven cloud providers.

Reverses if: #16 or a tool-calling effort merges the two APIs anyway — then the contract text carries over verbatim, since it constrains behaviour, not shape.

Q9. Does an empty-but-successful stream count as success?

Decision: no — zero emissions with a normal completion is MalformedResponse("empty completion"). Enforced in the wrapper's onCompletion (Q7), so it costs nothing per provider.

Otherwise: no chunks, so (with Q10) no transcript row, so the state machine goes Speaking("") (AgentLoop.kt:465) and TTS speaks silence. The user sees a request that did nothing and reports "it hangs". #26 already found one bug of exactly this shape — the benchmark's silently-untouched third state — so a second render-as-nothing state is not one to leave in.

Reverses if: a provider is found that legitimately streams zero tokens for a valid request (a pure tool-call turn, once tool streaming is real). Then the gate moves from "zero emissions" to "zero emissions and zero tool calls".

Q10. Does AgentLoop's non-streaming re-issue survive?

Decision: delete AgentLoop.kt:446-460.

Today, if the stream fails with nothing emitted, AgentLoop silently re-issues the whole request through complete(). That branch is vestigial scaffolding for the bug this ticket removes: since the eleven providers never throw, it is currently reachable only by a cancellation-shaped exception. The moment the contract makes streams throw, it becomes a hidden, unconditional retry — a wrong API key now costs two full round trips and double the wait before the user is told the key is wrong, and it retries AuthInvalid and QuotaExhausted, which are never worth retrying. It is also un-guarded: if the re-issue throws, it falls to AgentLoop.kt:469, so the user waits twice to see one error.

Retry belongs to #30, over retryable, in one place.

Reverses if: #30 adopts "on a streaming failure with nothing emitted, fall back once to non-streaming" as policy — plausible, and then it returns as policy in the wrapper, keyed off retryable, not as an ad-hoc branch at the call site.

Q11. What happens to the placeholder row?

Decision: insert the agent row lazily, on the first chunk. Move AgentLoop.kt:429's insertMessage inside the collect.

Found by composing Q10 with Q1: an empty ChatMessage is inserted before streaming. With the re-issue deleted, every pre-first-token failure — i.e. every auth, quota, and rate-limit error, i.e. the entire class of failure #25 exists to surface — now leaves a blank agent bubble in the transcript next to the error, permanently, because Room REPLACE never removes it.

Lazy insertion also makes Q2 clean: a transcript row exists iff at least one token arrived, so "partial kept" and "nothing to keep" are structurally distinct rather than distinguished by an empty string. Nothing is lost — the busy indicator is AgentState, set at AgentLoop.kt:298, not the empty bubble.

Reverses if: the UI turns out to need a placeholder to anchor a typing animation. It does not today; the status pill (ChatScreen.kt:634-672) carries it.

---

Implementation consequences, named

• LLMProvider.kt:33 — signature unchanged; the KDoc becomes the contract: completes normally ⇒ terminal sentinel observed and ≥1 chunk emitted; otherwise throws LLMException; never emits error text; CancellationException propagates.
• LLMProviderFactory.kt:147-150 — WrappedLLMProvider.streamComplete gains the onEach/catch/onCompletion chain of Q7. This is the only structural code this ticket adds.
• Eleven providers — OpenAIProvider.kt:89-104, ClaudeProvider.kt:116-127, GeminiProvider.kt:151-162, GroqProvider.kt:84, MistralProvider.kt:84, CohereProvider.kt:80, DeepSeekProvider.kt:84, OpenRouterProvider.kt:86, TogetherAIProvider.kt:84, CustomOpenAIProvider.kt:90, CopilotProvider.kt:102: the try/catch (e: Exception) { emit("Error streaming …") } wrapper is deleted. The flow body keeps only the emit loop; complete()'s classified throw (Error taxonomy and where classification lives #28) propagates.
• OllamaProvider.kt:91-102 is the same shape and is on-device — left as-is per Error taxonomy and where classification lives #28's scope line, netted by the wrapper.
• AgentLoop.kt:429 — insert lazily, on first chunk (Q11).
• AgentLoop.kt:444-460 — the CancellationException rethrow stays and becomes reachable; the complete() re-issue is deleted (Q10). The remaining catch (streamError: Exception) marks the row incomplete and re-throws to the outer handler, which is AgentLoop response to each error category #43's to shape per category.
• AgentLoop.kt:1310-1314 — formatStreamedReply is deleted. It exists solely to detect the string prefix "Error streaming" in the model's own reply text and re-humanise it. Once errors are never content, string-sniffing the transcript for an error is dead — and it is a nice marker for whether this contract actually landed.
• ConversationEntity.kt / ChatMessage.kt — nullable incomplete flag plus a Room migration (Q2). Rendering is Chat error surfacing: message, details block, redaction #31's.
• NetworkErrorFormatter (core/util/NetworkErrorFormatter.kt) — its whole job is lowercase substring-matching a raw message to guess a category. With LLMError in hand it is superseded on the LLM path; retiring it is Chat error surfacing: message, details block, redaction #31's call, not decided here.

Handed to neighbours

• Retry policy for transient provider failures #30 — streams are retryable only before the first emission; the gate is !emitted in the wrapper, not a policy choice.
• Chat error surfacing: message, details block, redaction #31 — the partial and the error are two surfaces, never one string; a transcript row can be incomplete for either a failure or a cancellation, and only the former shows an error.
• AgentLoop response to each error category #43 — the contract guarantees an LLMException reaches AgentLoop; what AgentLoop does per category, mid-plan, is entirely AgentLoop response to each error category #43's.
• Fate of executeWithFallback: silent cross-provider fallback already ships #39 — no interaction. executeWithFallback (LLMProviderFactory.kt:104) uses complete() only.

Deliberately left open

• finish_reason: "length" — a completion truncated by our own maxTokens = 500 (AgentLoop.kt:437) is a normal completion, not an error, and is not marked incomplete. Whether to surface it as a non-error hint ("answer cut short — ask for more") needs a metadata channel this ticket does not open.
• Where the terminal sentinel per dialect is defined — [DONE] vs message_stop vs Gemini's stream shape. It belongs with the real-SSE work, which API reliability & error surfacing map (upstream issue 13) #25 rules out of scope; this ticket only requires that one exist and be checked.
• Dispatcher and buffering for a real SSE read loop. Not an error-propagation matter.
• Retiring NetworkErrorFormatter. Named above, left to Chat error surfacing: message, details block, redaction #31.

Confidence

Self-grilled on the maintainer's explicit delegation — no live confirmation on any of the above. The three most reversible, in order: the incomplete column on ConversationEntity (Q2 — a schema change #31 may carry better), deleting the non-streaming re-issue (Q10 — #30 may want it back as policy), and treating a mid-stream socket drop as Network rather than MalformedResponse (Q4 — a deliberate narrowing of #28's wording). The two I would defend hardest: throw rather than a terminal element (Q1), and cancellation never being classified (Q6).