Skip to content

fix: harden the chat call against malformed events and config - #21

Merged
RohanReddy44 merged 4 commits into
mainfrom
fix/chat-call-review
Sep 24, 2026
Merged

RohanReddy44 merged 4 commits into
mainfrom
fix/chat-call-review

Conversation

@RohanReddy44

Copy link
Copy Markdown
Collaborator

What changed

The four findings from the review of #19, one commit each.

  • started without ids is a broken stream. It was coerced with String(), so a missing assistantMessageId became the bubble id "undefined" and a second such turn overwrote the first bubble. Now upstream_error, like a malformed completed or error.
  • api.baseUrl may end in a slash. /experience/api/ produced /experience/api//v1/chat, a 404 with nothing pointing at the cause. Trailing slashes are dropped where the value is read, in the client and the dev proxy.
  • completed is validated before the store acts on it. An error field that is not the contract's object, or empty content with no error, is now upstream_error instead of a silently hidden Retry or an empty bubble with a Copy row.
  • Suggestions are fetched in one place. The last duplicated block left over from the turn-runner refactor.

Verified

  • Lint, typecheck, knip, tests (48, three new) and build pass at each commit via the pre-commit hook.
  • Not run in Actions.

🤖 Generated with Claude Code

RohanReddy44 and others added 4 commits September 24, 2026 14:55
`started` was the one event read without checking its shape: a missing
assistantMessageId or traceId was coerced with String(), so the store
got the literal text "undefined" as the bubble id. A second such turn
would then overwrite the first turn's bubble in place, and "undefined"
would be kept as the trace id for support.

It is now held to the same rule as completed and error: a started
event without both ids is a broken stream, reported as upstream_error
before any handler runs.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`api.baseUrl` is used as written and every path is appended as
`/v1/chat`, so a value of `/experience/api/` produced
`/experience/api//v1/chat`. The API treats that as a different, unknown
route and answers 404, which the client showed as an unknown error on
every turn with nothing pointing at the cause. The dev proxy had the
matching problem: its key matched, but the stripped path lost its
leading slash.

Trailing slashes are now dropped where the value is read, in the
client and in vite.config.ts alike. The old default ended in a slash,
so this is a natural thing to type.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`completed` was accepted once `content` was an array. Two malformed
shapes then reached the store: an `error` field that was not the
contract's object, which made the turn count as failed with Retry
silently hidden because `retryable` was undefined; and empty `content`
with no `error`, which drew an empty assistant bubble with a Copy row
and counted as answered.

Both are now a broken stream, reported as upstream_error, the same as
a completed with no content array. The rule is §5.2: content is a
list, error when present has our shape, and empty content only comes
with error.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The turn runner removed the two copies of the streaming and error
code from sendText and sendImage, but each still carried its own copy
of "fetch suggestions and map them" afterwards. One helper now, so a
change to the suggestion shape is made once.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@RohanReddy44
RohanReddy44 merged commit 28aec77 into main Sep 24, 2026
4 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