From fb61c48227fb68e06d10cedb2007a74c83d04e16 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 18 Mar 2026 20:43:50 +0200 Subject: [PATCH] fix: remove omitempty from required ChatMessagePart fields (#23250) ChatMessagePart uses a flat struct with omitempty on all fields, but some fields are required in their TypeScript variant (no ? suffix in the variants struct tag). When Go omits a zero-valued required field, the frontend receives undefined where it expects a concrete value. Remove omitempty from fields that are required in at least one variant: Text, URL, MediaType, FileName, StartLine, EndLine, Content. Fields where all variants use ? keep omitempty. Add a sub-test to TestChatMessagePartVariantTags that enforces this invariant via reflection so future additions cannot reintroduce the mismatch. Supersedes #23249 --- codersdk/chats.go | 23 +++++++---- codersdk/chats_test.go | 40 +++++++++++++++++++ site/src/api/typesGenerated.ts | 11 ++++- .../AgentsPage/AgentDetail/messageParsing.ts | 5 +-- 4 files changed, 68 insertions(+), 11 deletions(-) diff --git a/codersdk/chats.go b/codersdk/chats.go index f14cedeab5..ce2466fcbe 100644 --- a/codersdk/chats.go +++ b/codersdk/chats.go @@ -125,9 +125,18 @@ func AllChatMessagePartTypes() []ChatMessagePartType { // name = required, ? suffix = optional. Fields without a variants // tag are excluded from the generated union. See // scripts/apitypings/main.go for the codegen that reads these. +// +// omitempty rules (enforced by TestChatMessagePartVariantTags): +// - If a field is required (no ? suffix) in ANY variant, it +// must NOT use omitempty. Go would silently drop zero values +// that TypeScript expects to always be present. +// - If a field is optional (? suffix) in ALL of its variants, +// it MUST use omitempty. Sending zero values for fields that +// the frontend does not expect adds noise to the wire format +// and wastes space in persisted chat_messages rows. type ChatMessagePart struct { Type ChatMessagePartType `json:"type"` - Text string `json:"text,omitempty" variants:"text,reasoning?"` + Text string `json:"text" variants:"text,reasoning"` Signature string `json:"signature,omitempty"` ToolCallID string `json:"tool_call_id,omitempty" variants:"tool-call?,tool-result?"` ToolName string `json:"tool_name,omitempty" variants:"tool-call?,tool-result?"` @@ -137,16 +146,16 @@ type ChatMessagePart struct { ResultDelta string `json:"result_delta,omitempty"` IsError bool `json:"is_error,omitempty" variants:"tool-result?"` SourceID string `json:"source_id,omitempty" variants:"source?"` - URL string `json:"url,omitempty" variants:"source"` + URL string `json:"url" variants:"source"` Title string `json:"title,omitempty" variants:"source?"` - MediaType string `json:"media_type,omitempty" variants:"file"` + MediaType string `json:"media_type" variants:"file"` Data []byte `json:"data,omitempty" variants:"file?"` FileID uuid.NullUUID `json:"file_id,omitempty" format:"uuid" variants:"file?"` - FileName string `json:"file_name,omitempty" variants:"file-reference"` - StartLine int `json:"start_line,omitempty" variants:"file-reference"` - EndLine int `json:"end_line,omitempty" variants:"file-reference"` + FileName string `json:"file_name" variants:"file-reference"` + StartLine int `json:"start_line" variants:"file-reference"` + EndLine int `json:"end_line" variants:"file-reference"` // The code content from the diff that was commented on. - Content string `json:"content,omitempty" variants:"file-reference"` + Content string `json:"content" variants:"file-reference"` // ProviderMetadata holds provider-specific response metadata // (e.g. Anthropic cache control hints) as raw JSON. Internal // only: stripped by db2sdk before API responses. diff --git a/codersdk/chats_test.go b/codersdk/chats_test.go index a698b14505..1d919f2f0e 100644 --- a/codersdk/chats_test.go +++ b/codersdk/chats_test.go @@ -259,6 +259,46 @@ func TestChatMessagePartVariantTags(t *testing.T) { assert.True(t, coveredTypes[pt], "ChatMessagePartType %q is not referenced by any variants tag; %s", pt, editHint) } + + // Enforce the omitempty <-> variants invariant: + // required in any variant => must NOT have omitempty + // optional in all variants => MUST have omitempty + // See the struct comment on ChatMessagePart for rationale. + t.Run("omitempty must match variant optionality", func(t *testing.T) { + t.Parallel() + + typ := reflect.TypeOf(codersdk.ChatMessagePart{}) + for i := range typ.NumField() { + f := typ.Field(i) + varTag := f.Tag.Get("variants") + if varTag == "" { + continue + } + + allOptional := true + for _, entry := range strings.Split(varTag, ",") { + if !strings.HasSuffix(entry, "?") { + allOptional = false + break + } + } + + jsonTag := f.Tag.Get("json") + hasOmitEmpty := strings.Contains(jsonTag, "omitempty") + + if !allOptional { + assert.False(t, hasOmitEmpty, + "field %s is required in at least one variant but has omitempty in its json tag; "+ + "remove omitempty so Go does not silently drop the zero value that TypeScript expects to always be present", + f.Name) + } else { + assert.True(t, hasOmitEmpty, + "field %s is optional in all variants but is missing omitempty in its json tag; "+ + "add omitempty to avoid sending zero values for fields the frontend does not expect", + f.Name) + } + } + }) } func TestModelCostConfig_LegacyNumericJSON(t *testing.T) { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 5c49f82474..928318c50e 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1320,6 +1320,15 @@ export interface ChatMessage { * name = required, ? suffix = optional. Fields without a variants * tag are excluded from the generated union. See * scripts/apitypings/main.go for the codegen that reads these. + * + * omitempty rules (enforced by TestChatMessagePartVariantTags): + * - If a field is required (no ? suffix) in ANY variant, it + * must NOT use omitempty. Go would silently drop zero values + * that TypeScript expects to always be present. + * - If a field is optional (? suffix) in ALL of its variants, + * it MUST use omitempty. Sending zero values for fields that + * the frontend does not expect adds noise to the wire format + * and wastes space in persisted chat_messages rows. */ export type ChatMessagePart = | ChatTextPart @@ -1682,7 +1691,7 @@ export interface ChatQueuedMessage { // From codersdk/chats.go export interface ChatReasoningPart { readonly type: "reasoning"; - readonly text?: string; + readonly text: string; } // From codersdk/chats.go diff --git a/site/src/pages/AgentsPage/AgentDetail/messageParsing.ts b/site/src/pages/AgentsPage/AgentDetail/messageParsing.ts index d12ad253e5..15e2dd5e8f 100644 --- a/site/src/pages/AgentsPage/AgentDetail/messageParsing.ts +++ b/site/src/pages/AgentsPage/AgentDetail/messageParsing.ts @@ -131,9 +131,8 @@ export const parseMessageContent = ( break; } case "reasoning": { - const text = part.text ?? ""; - parsed.reasoning = appendText(parsed.reasoning, text); - parsed.blocks = appendTextBlock(parsed.blocks, "thinking", text); + parsed.reasoning = appendText(parsed.reasoning, part.text); + parsed.blocks = appendTextBlock(parsed.blocks, "thinking", part.text); break; } case "tool-call": {