Skip to content

Make the SSE reader emit exactly one terminal outcome; add ModelUsage parity - #14

Merged
HartBrook merged 1 commit into
mainfrom
feature/sse-terminal-state-machine
Aug 10, 2026
Merged

HartBrook merged 1 commit into
mainfrom
feature/sse-terminal-state-machine

Conversation

@HartBrook

Copy link
Copy Markdown
Owner

Second of the two P0 follow-up PRs. Independent of #13 — this touches only packages/llmpane-react, so the two can merge in either order.

The SSE reader had no terminal state

readSSEStream fired callbacks opportunistically as it walked the buffer, which produced four distinct defects:

Scenario Old behaviour
Chunk with both error and done onComplete and onError — the same failed run reported as success and failure
Body closes after a done chunk onComplete fires a second time
Stream aborted No callback at all — optimistic messages pending forever
Body closes with no terminal event Reported as success, though the outcome is unknown

Every exit path now settles through a single guard that delivers exactly one of onComplete, onError, or the new onAbort, and reading stops as soon as a terminal chunk arrives. A body ending without a terminal event is an error. The terminal chunk is still delivered to onChunk first, since consumers read messageId and conversationId off it.

useChat had matching problems

It classified error chunks inside onChunk while also receiving onError from the reader, so consumers saw every failure twice. That duplicate is removed.

Failed and cancelled runs also left both optimistic messages pending forever. They now settle deliberately:

  • User message is kept and finalized — the server persists it before streaming begins.
  • Partial assistant message is dropped — the server discards partial output, so keeping it on screen would make it silently vanish on the next reload.

Two subtleties the tests forced out:

  • The settle helper is keyed off the temp IDs rather than streamingMessageRef, because stopStream() clears that ref before the abort reaches the handler. Keying off the ref left the assistant bubble pending — caught by the cancellation test.
  • It is declared before the request, so an abort landing during fetch (rather than mid-stream) settles too.

Protocol parity

Python's StreamChunk has carried a usage: ModelUsage field that the TypeScript contract never exposed. ModelUsage is now part of the React types and exported from the package entry point.

total_tokens is a computed property on the Python model and does not cross the wire, so it ships as a totalTokens(usage) helper rather than a phantom field.

Verification

  • 151 React tests pass (was 130); lint, typecheck, and build clean; verified ModelUsage and totalTokens actually appear in the built dist/index.d.ts.
  • New coverage asserts callback counts and ordering — not merely "was it called" — across success, terminal error, structured errorInfo, early EOF, empty body, non-ok response, malformed events, [DONE] marker, a terminal split across a trailing partial line, and cancellation.

One existing test changed, deliberately. "calls onComplete when stream ends" fed a stream with no terminal done chunk and asserted onComplete — it encoded the bug. It is replaced by two tests: one for the real success path, one asserting an early close is an error.

Compatibility

onAbort is additive and optional. The behavioural changes are the point of the PR, and worth noting for anyone upgrading:

  • If you handled error chunks yourself inside onChunk, you were previously getting onError as well; that double-call is gone.
  • A stream that closes without a terminal event now reports an error instead of success.
  • Cancelled runs now invoke onAbort instead of nothing.

Checked against a production consumer: it passes neither onError nor onComplete, reading only the returned error state and calling stopStream(), so this is a strict improvement there with no API break — cancelled runs stop leaving permanently pending bubbles in its message list.

readSSEStream fired callbacks opportunistically rather than modelling a
terminal state, which produced four distinct defects:

- A terminal chunk carrying both `error` and `done` invoked onComplete and
  onError, reporting the same failed run as both success and failure.
- When the body closed after a `done` chunk, onComplete fired a second
  time.
- An aborted stream broke out of the loop and invoked no callback at all,
  leaving optimistic messages pending forever.
- A stream that closed without any terminal event was reported as success,
  even though the run's outcome was unknown.

Every exit path now settles through a single guard that delivers exactly
one of onComplete, onError, or the new onAbort, and reading stops as soon
as a terminal chunk arrives. A body that ends without a terminal event is
an error. The terminal chunk is still handed to onChunk first, since
consumers read messageId and conversationId from it.

useChat had matching problems. It classified error chunks in onChunk while
also receiving onError from the reader, so consumers saw every failure
twice; that duplicate is removed. Failed and cancelled runs now settle
both optimistic messages instead of leaving them pending: the user message
is finalized, because the server persists it before streaming, and the
partial assistant message is dropped, because the server discards partial
output and keeping it would vanish on reload.

The settle helper is keyed off the temp IDs rather than
streamingMessageRef, since stopStream() clears that ref before the abort
reaches the handler, and it is declared before the request so an abort
during fetch settles too.

Also adds ModelUsage to the React types to close a protocol gap: Python's
StreamChunk has carried a `usage` field that the TypeScript contract never
exposed. total_tokens is a computed property that does not cross the wire,
so it ships as a totalTokens() helper.

One existing test asserted that a stream closing with no terminal event
calls onComplete. That encoded the bug, so it is replaced by tests for
both the correct success path and the early-close error.
@HartBrook
HartBrook merged commit 409443d into main Aug 10, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant