refactor: type both chat message parsers#23176
Merged
Merged
Conversation
ac518b6 to
8c96ab4
Compare
f88b532 to
4e9c9ae
Compare
e2835a1 to
c4e0f14
Compare
c4e0f14 to
4077eb8
Compare
4077eb8 to
cff66c6
Compare
4e9c9ae to
77fe83d
Compare
Both parsers (historical REST and streaming SSE) previously accepted untyped input and relied on scattered asRecord/asString calls to extract fields at runtime. With the discriminated ChatMessagePart union from PR 1, the parsers can accept typed input directly. Historical parser (parseMessageContent): - Signature changes from (content: unknown) to (content: readonly ChatMessagePart[] | undefined) - Removes legacy input shape handling (plain string, null, bare object, non-object array entries) that the Go backend normalizes away - switch narrows on part.type automatically, no asRecord/asString needed Streaming parser (applyMessagePartToStreamState): - Signature changes from (part: Record<string, unknown>) to (part: ChatMessagePart) - SSE type guards no longer widen with & Record<string, unknown>, so streamEvent.message_part?.part is typed directly ChatContext.ts: - Removes asRecord/asString import entirely - All stream event handlers use generated types directly codersdk/chats.go: - Marks tool_call_id and tool_name as optional in variant tags (Go code guards against empty values, omitempty omits them) - Relaxes test requiring every variant to have a required field
cff66c6 to
719c40f
Compare
Member
Author
|
Ok looks like the Streamed Reasoning changed, need to look into that. |
DanielleMaywood
approved these changes
Mar 18, 2026
Contributor
DanielleMaywood
left a comment
There was a problem hiding this comment.
this makes me happy
kylecarbs
approved these changes
Mar 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR 3 of 3 in a series re-architecting
ChatMessagePartfrom a flat interface into a discriminated union.Both message parsers previously accepted untyped input (
unknown/Record<string, unknown>) and used scatteredasRecord/asStringcalls to extract fields at runtime. With the discriminated union from PR 1, both accept typedChatMessagePartdirectly and narrow viaswitch (part.type).The historical parser (
parseMessageContent) signature narrows from(content: unknown)to(content: readonly ChatMessagePart[] | undefined), removing legacy input shape handling the Go backend normalizes away. The streaming parser (applyMessagePartToStreamState) narrows fromRecord<string, unknown>toChatMessagePart.The SSE event type guards had a
& Record<string, unknown>intersection that widened everything untyped downstream. Since the SSE data comes from our own API serializing the same Go structs the types are generated from, the intersection was removed. All handlers inChatContext.tsnow use the generated types directly, removingasRecord/asStringfrom the file entirely.Fixes
tool_call_idandtool_namevariant tags incodersdk/chats.go: they were marked required but Go code guards against them being empty in multiple places (persistInterruptedStep,ConvertMessagesWithFiles, etc.) andomitemptyomits them at the wire level. Changed to optional so the generated TypeScript matches reality. Relaxed the test that required every variant to have at least one required field.Both parser switches use
satisfies neverexhaustiveness checks so the compiler catches missing cases when new part types are added.Refs #23168, #23175