mirror of
https://github.com/coder/coder.git
synced 2026-09-21 12:44:32 +08:00
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
This commit is contained in:
+16
-7
@@ -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.
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Generated
+10
-1
@@ -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
|
||||
|
||||
@@ -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": {
|
||||
|
||||
Reference in New Issue
Block a user