From a48aedd924fe5aa824db40194cc893bf6b578aac Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:09:00 +0200 Subject: [PATCH] fix(coderd/x): surface Gemini malformed-function-call stream deaths as retryable errors (#28470) ## Problem When Gemini's OpenAI-compatible endpoint rejects a model-generated function call server-side, it ends the SSE stream cleanly with the nonstandard finish reason `function_call_filter: MALFORMED_FUNCTION_CALL` after streaming only thought summaries; the rejected call never reaches the wire. chatd treated this as a normal completion: fantasy maps the unrecognized finish reason to `unknown`, the reasoning block never closes (the only non-thought delta is the `` marker, which the transport seam strips to an empty string, and the openaicompat hook only ends reasoning on a non-empty content delta), so the step accumulates no content and the generation loop finishes the turn as complete. The user sees the model think for ~40 seconds and then nothing: no assistant message, no `last_error`, chat status `waiting`. Observed twice in a row in production on gemini-3.7-flash, with "Resume" reproducing it identically. ## Fix Two independent layers: - `coderd/x/googleopenai`: the stream rewrite now converts any chunk whose `finish_reason` starts with `function_call_filter` into an OpenAI-style SSE `{"error": ...}` event embedding the raw reason. openai-go turns error-bearing events into stream errors, so the failure rides the existing stream-error path instead of ending the stream cleanly. - `coderd/x/chatd/chaterror`: classifies that injected error as retryable (kind `generic`, provider `google`) with a clear user-facing message, so the existing generation retry machinery re-runs the step and persists a `last_error` if retries exhaust. - `coderd/x/chatd/chatloop`: provider-agnostic guard: a step that produced no user-visible content and no tool calls under a finish reason of `unknown`, `error`, `other`, or `tool-calls` (a tool-calls finish that delivered zero calls) now returns a retryable error instead of silently completing the turn. `stop` and `length` finishes keep their existing semantics. Tests cover the seam rewrite (live-capture SSE shape plus standard finish reason passthrough), the new classification, and the chatloop guard (error cases plus preserved stop, length, text, and tool-call behavior). Each layer was red-green verified independently. Remote dogfood UAT ran against this exact commit: normal reasoning and tool-call chats on a real model complete cleanly with no spurious guard errors and no retry loops. The Google-side failure itself is not deterministically triggerable against live Gemini and is owned by the unit tests. > Xum acted on Mike's (@ibetitsmike) behalf. --- coderd/x/chatd/chaterror/classify.go | 35 ++++ coderd/x/chatd/chaterror/classify_test.go | 27 +++ coderd/x/chatd/chatloop/chatloop.go | 45 ++++- .../chatd/chatloop/nooutput_internal_test.go | 157 ++++++++++++++++++ coderd/x/googleopenai/thoughts.go | 51 +++++- coderd/x/googleopenai/thoughts_test.go | 62 +++++++ 6 files changed, 371 insertions(+), 6 deletions(-) create mode 100644 coderd/x/chatd/chatloop/nooutput_internal_test.go diff --git a/coderd/x/chatd/chaterror/classify.go b/coderd/x/chatd/chaterror/classify.go index eb15093d036..b3a87269aaf 100644 --- a/coderd/x/chatd/chaterror/classify.go +++ b/coderd/x/chatd/chaterror/classify.go @@ -9,6 +9,7 @@ import ( "golang.org/x/net/http2" "golang.org/x/xerrors" + "github.com/coder/coder/v2/coderd/x/googleopenai" "github.com/coder/coder/v2/codersdk" ) @@ -177,6 +178,15 @@ func Classify(err error) ClassifiedError { return classified } + if classified, ok := functionCallFilterClassification( + lower, + provider, + statusCode, + structured, + ); ok { + return classified + } + retryableHTTP2StreamReset, hasHTTP2StreamReset := classifyHTTP2StreamReset(err) // combinedText merges the transport wrapper text with the structured // provider response body so signal patterns in either are detected. @@ -375,6 +385,31 @@ func streamIncompleteMessage(provider string) string { return providerSubject(provider) + " stream closed unexpectedly before the response completed." } +// Match only the adapter's exact error prefix so unrelated provider errors +// mentioning function_call_filter retain their normal classification. +func functionCallFilterClassification( + lowerMessage string, + provider string, + statusCode int, + structured providerErrorDetails, +) (ClassifiedError, bool) { + if !strings.Contains(lowerMessage, googleopenai.MalformedFunctionCallMessagePrefix) { + return ClassifiedError{}, false + } + if provider == "" { + provider = "google" + } + return normalizeClassification(ClassifiedError{ + Message: "Gemini rejected the model's generated function call as malformed.", + Detail: structured.detail, + Kind: codersdk.ChatErrorKindGeneric, + Provider: provider, + Retryable: true, + StatusCode: statusCode, + RetryAfter: structured.retryAfter, + }), true +} + func responsesAPIDiagnostic(lowerMessage, detail string) (string, bool) { lowerDetail := strings.ToLower(detail) for _, match := range responsesAPIDiagnosticMatches { diff --git a/coderd/x/chatd/chaterror/classify_test.go b/coderd/x/chatd/chaterror/classify_test.go index 42947a11914..f310f2d662f 100644 --- a/coderd/x/chatd/chaterror/classify_test.go +++ b/coderd/x/chatd/chaterror/classify_test.go @@ -79,6 +79,33 @@ func TestClassify(t *testing.T) { StatusCode: 0, }, }, + { + name: "FunctionCallFilterMentionKeepsOwnClassification", + err: xerrors.New(`status 401: function_call_filter is not supported for this endpoint`), + want: chaterror.ClassifiedError{ + Message: "Authentication with the AI provider failed. Check the API key and permissions.", + Kind: codersdk.ChatErrorKindAuth, + Provider: "", + Retryable: false, + StatusCode: 401, + }, + }, + { + name: "GeminiFunctionCallFilter", + err: xerrors.New( + `stream response: received error while streaming: ` + + `{"message":"gemini dropped the model's generated function call ` + + `(finish_reason \"function_call_filter: MALFORMED_FUNCTION_CALL\")",` + + `"type":"invalid_response_error","code":"malformed_function_call"}`, + ), + want: chaterror.ClassifiedError{ + Message: "Gemini rejected the model's generated function call as malformed.", + Kind: codersdk.ChatErrorKindGeneric, + Provider: "google", + Retryable: true, + StatusCode: 0, + }, + }, { name: "AuthBeatsConfig", err: xerrors.New("authentication failed: invalid model"), diff --git a/coderd/x/chatd/chatloop/chatloop.go b/coderd/x/chatd/chatloop/chatloop.go index 2800c2dae98..3e14b8943e3 100644 --- a/coderd/x/chatd/chatloop/chatloop.go +++ b/coderd/x/chatd/chatloop/chatloop.go @@ -51,6 +51,9 @@ var ( // classifiers blocked the response and the model produced no // content, e.g. Anthropic's stop_reason "refusal". ErrContentFiltered = xerrors.New("response blocked by provider content filter") + // ErrNoModelOutput is returned when a stream ends without visible content or + // tool calls and its finish reason indicates an incomplete response. + ErrNoModelOutput = xerrors.New("model stream finished without output") errStreamSilenceTimeout = xerrors.New( "chat stream was silent for longer than the configured timeout", @@ -462,6 +465,12 @@ func GenerateAssistant(ctx context.Context, opts GenerateAssistantOptions) (Assi if result.finishReason == fantasy.FinishReasonContentFilter && !hasUserVisibleContent(result.content) { return AssistantOutcome{}, contentFilterError(errorProvider, result.providerMetadata) } + // Treat discarded responses as retryable so an empty turn is not persisted. + if silentNoOutputFinish(result.finishReason) && !hasUserVisibleContent(result.content) && len(result.toolCalls) == 0 { + noOutputErr := noModelOutputError(errorProvider, result.finishReason) + opts.Metrics.RecordStreamRetry(provider, modelName, chaterror.Classify(noOutputErr)) + return AssistantOutcome{}, noOutputErr + } step := PersistedStep{ Content: result.content, Usage: result.usage, @@ -496,13 +505,20 @@ func wrapProviderStreamError(provider string, err error) error { return xerrors.Errorf("stream response: %w", chaterror.WithClassification(err, classified)) } -// hasUserVisibleContent reports whether any content part carries output the -// user can see. Reasoning parts do not count: they stream transiently and are -// not a substitute for a response. +// hasUserVisibleContent ignores reasoning and blank text because neither can +// complete a user-facing response. func hasUserVisibleContent(content []fantasy.Content) bool { for _, part := range content { - switch part.(type) { + switch value := part.(type) { case fantasy.ReasoningContent, *fantasy.ReasoningContent: + case fantasy.TextContent: + if strings.TrimSpace(value.Text) != "" { + return true + } + case *fantasy.TextContent: + if value != nil && strings.TrimSpace(value.Text) != "" { + return true + } default: return true } @@ -510,6 +526,27 @@ func hasUserVisibleContent(content []fantasy.Content) bool { return false } +func silentNoOutputFinish(reason fantasy.FinishReason) bool { + switch reason { + case fantasy.FinishReasonUnknown, fantasy.FinishReasonError, + fantasy.FinishReasonOther, fantasy.FinishReasonToolCalls: + return true + default: + return false + } +} + +func noModelOutputError(provider string, reason fantasy.FinishReason) error { + classified := chaterror.ClassifiedError{ + Message: "The model ended its response without producing any output.", + Detail: "finish reason: " + string(reason), + Kind: codersdk.ChatErrorKindGeneric, + Provider: provider, + Retryable: true, + } + return chaterror.WithClassification(ErrNoModelOutput, classified) +} + func contentFilterError(provider string, metadata fantasy.ProviderMetadata) error { classified := chaterror.ClassifiedError{ Kind: codersdk.ChatErrorKindContentFilter, diff --git a/coderd/x/chatd/chatloop/nooutput_internal_test.go b/coderd/x/chatd/chatloop/nooutput_internal_test.go new file mode 100644 index 00000000000..d7f9622ca7b --- /dev/null +++ b/coderd/x/chatd/chatloop/nooutput_internal_test.go @@ -0,0 +1,157 @@ +package chatloop + +import ( + "context" + "testing" + + "charm.land/fantasy" + "github.com/prometheus/client_golang/prometheus" + promtestutil "github.com/prometheus/client_golang/prometheus/testutil" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/x/chatd/chaterror" + "github.com/coder/coder/v2/coderd/x/chatd/chattest" + "github.com/coder/coder/v2/codersdk" +) + +func TestGenerateAssistant_SilentNoOutputFinish(t *testing.T) { + t.Parallel() + + generate := func(t *testing.T, parts []fantasy.StreamPart) (AssistantOutcome, *Metrics, error) { + t.Helper() + model := &chattest.FakeModel{ + ProviderName: "google", + ModelName: "test-model", + StreamFn: func(_ context.Context, _ fantasy.Call) (fantasy.StreamResponse, error) { + return streamFromParts(parts), nil + }, + } + metrics := NewMetrics(prometheus.NewRegistry()) + outcome, err := GenerateAssistant(context.Background(), GenerateAssistantOptions{ + Model: model, + Metrics: metrics, + Messages: []fantasy.Message{ + textMessage(fantasy.MessageRoleUser, "hello"), + }, + }) + return outcome, metrics, err + } + + unterminatedReasoning := func(finish fantasy.FinishReason) []fantasy.StreamPart { + return []fantasy.StreamPart{ + {Type: fantasy.StreamPartTypeReasoningStart, ID: "reasoning-1"}, + {Type: fantasy.StreamPartTypeReasoningDelta, ID: "reasoning-1", Delta: "planning"}, + {Type: fantasy.StreamPartTypeFinish, FinishReason: finish}, + } + } + + t.Run("UnknownFinishWithoutOutputErrors", func(t *testing.T) { + t.Parallel() + + outcome, metrics, err := generate(t, unterminatedReasoning(fantasy.FinishReasonUnknown)) + require.ErrorIs(t, err, ErrNoModelOutput) + require.Empty(t, outcome.Step.Content) + + classified := chaterror.Classify(err) + require.True(t, classified.Retryable) + require.Equal(t, codersdk.ChatErrorKindGeneric, classified.Kind) + require.Equal(t, "google", classified.Provider) + require.Equal(t, "The model ended its response without producing any output.", classified.Message) + require.Equal(t, "finish reason: unknown", classified.Detail) + + retries := promtestutil.ToFloat64(metrics.StreamRetriesTotal.WithLabelValues( + "google", "test-model", string(codersdk.ChatErrorKindGeneric), + )) + require.Equal(t, float64(1), retries) + }) + + t.Run("ReasoningOnlyContentWithUnknownFinishErrors", func(t *testing.T) { + t.Parallel() + + _, _, err := generate(t, []fantasy.StreamPart{ + {Type: fantasy.StreamPartTypeReasoningStart, ID: "reasoning-1"}, + {Type: fantasy.StreamPartTypeReasoningDelta, ID: "reasoning-1", Delta: "planning"}, + {Type: fantasy.StreamPartTypeReasoningEnd, ID: "reasoning-1"}, + {Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonUnknown}, + }) + require.ErrorIs(t, err, ErrNoModelOutput) + }) + + t.Run("ToolCallsFinishWithoutToolCallsErrors", func(t *testing.T) { + t.Parallel() + + _, _, err := generate(t, []fantasy.StreamPart{ + {Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonToolCalls}, + }) + require.ErrorIs(t, err, ErrNoModelOutput) + require.Equal(t, "finish reason: tool-calls", chaterror.Classify(err).Detail) + }) + + t.Run("StopFinishWithoutOutputCompletes", func(t *testing.T) { + t.Parallel() + + outcome, metrics, err := generate(t, unterminatedReasoning(fantasy.FinishReasonStop)) + require.NoError(t, err) + require.Empty(t, outcome.Step.Content) + require.True(t, outcome.ModelStopped) + + retries := promtestutil.ToFloat64(metrics.StreamRetriesTotal.WithLabelValues( + "google", "test-model", string(codersdk.ChatErrorKindGeneric), + )) + require.Equal(t, float64(0), retries) + }) + + t.Run("LengthFinishWithoutOutputCompletes", func(t *testing.T) { + t.Parallel() + + _, _, err := generate(t, unterminatedReasoning(fantasy.FinishReasonLength)) + require.NoError(t, err) + }) + + t.Run("EmptyTextWithUnknownFinishErrors", func(t *testing.T) { + t.Parallel() + + _, _, err := generate(t, []fantasy.StreamPart{ + {Type: fantasy.StreamPartTypeTextStart, ID: "text-1"}, + {Type: fantasy.StreamPartTypeTextEnd, ID: "text-1"}, + {Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonUnknown}, + }) + require.ErrorIs(t, err, ErrNoModelOutput) + }) + + t.Run("WhitespaceTextWithUnknownFinishErrors", func(t *testing.T) { + t.Parallel() + + _, _, err := generate(t, []fantasy.StreamPart{ + {Type: fantasy.StreamPartTypeTextStart, ID: "text-1"}, + {Type: fantasy.StreamPartTypeTextDelta, ID: "text-1", Delta: " \n"}, + {Type: fantasy.StreamPartTypeTextEnd, ID: "text-1"}, + {Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonUnknown}, + }) + require.ErrorIs(t, err, ErrNoModelOutput) + }) + + t.Run("UnknownFinishWithTextCompletes", func(t *testing.T) { + t.Parallel() + + outcome, _, err := generate(t, []fantasy.StreamPart{ + {Type: fantasy.StreamPartTypeTextStart, ID: "text-1"}, + {Type: fantasy.StreamPartTypeTextDelta, ID: "text-1", Delta: "answer"}, + {Type: fantasy.StreamPartTypeTextEnd, ID: "text-1"}, + {Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonUnknown}, + }) + require.NoError(t, err) + require.NotEmpty(t, outcome.Step.Content) + }) + + t.Run("ToolCallsFinishWithToolCallCompletes", func(t *testing.T) { + t.Parallel() + + outcome, _, err := generate(t, []fantasy.StreamPart{ + {Type: fantasy.StreamPartTypeToolCall, ID: "call-1", ToolCallName: "do_thing", ToolCallInput: "{}"}, + {Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonToolCalls}, + }) + require.NoError(t, err) + require.Len(t, outcome.ToolCalls, 1) + }) +} diff --git a/coderd/x/googleopenai/thoughts.go b/coderd/x/googleopenai/thoughts.go index 613e71a785a..ea92be9d257 100644 --- a/coderd/x/googleopenai/thoughts.go +++ b/coderd/x/googleopenai/thoughts.go @@ -3,6 +3,7 @@ package googleopenai import ( "bufio" "bytes" + "encoding/json" "errors" "io" "net/http" @@ -145,6 +146,9 @@ func (b *thoughtStreamBody) rewriteLine(line []byte) []byte { if !choices.IsArray() { return line } + if errPayload := functionCallFilterErrorPayload(choices); errPayload != nil { + return assembleDataLine(errPayload, suffix) + } out := payload for index, choice := range choices.Array() { delta := choice.Get("delta") @@ -186,9 +190,52 @@ func (b *thoughtStreamBody) rewriteLine(line []byte) []byte { b.inThought[index] = false } } - rewritten := make([]byte, 0, len(streamDataPrefix)+len(out)+len(suffix)) + return assembleDataLine(out, suffix) +} + +func assembleDataLine(payload, suffix []byte) []byte { + rewritten := make([]byte, 0, len(streamDataPrefix)+len(payload)+len(suffix)) rewritten = append(rewritten, streamDataPrefix...) - rewritten = append(rewritten, out...) + rewritten = append(rewritten, payload...) rewritten = append(rewritten, suffix...) return rewritten } + +// Gemini uses this nonstandard finish-reason prefix when it discards a +// malformed generated function call. +const functionCallFilterFinishReasonPrefix = "function_call_filter" + +// MalformedFunctionCallMessagePrefix distinguishes adapter-generated errors +// from unrelated provider errors. +const MalformedFunctionCallMessagePrefix = "gemini dropped the model's generated function call" + +type streamErrorEvent struct { + Error streamErrorDetail `json:"error"` +} + +type streamErrorDetail struct { + Message string `json:"message"` + Type string `json:"type"` + Code string `json:"code"` +} + +// functionCallFilterErrorPayload turns the nonstandard finish reason into an +// SSE error so consumers do not accept the empty stream as successful. +func functionCallFilterErrorPayload(choices gjson.Result) []byte { + for _, choice := range choices.Array() { + reason := choice.Get("finish_reason") + if reason.Type != gjson.String || !strings.HasPrefix(reason.Str, functionCallFilterFinishReasonPrefix) { + continue + } + payload, err := json.Marshal(streamErrorEvent{Error: streamErrorDetail{ + Message: MalformedFunctionCallMessagePrefix + " (finish_reason " + strconv.Quote(reason.Str) + ")", + Type: "invalid_response_error", + Code: "malformed_function_call", + }}) + if err != nil { + return nil + } + return payload + } + return nil +} diff --git a/coderd/x/googleopenai/thoughts_test.go b/coderd/x/googleopenai/thoughts_test.go index 62e4e7b8957..938ccb7ebd1 100644 --- a/coderd/x/googleopenai/thoughts_test.go +++ b/coderd/x/googleopenai/thoughts_test.go @@ -81,6 +81,68 @@ func TestRewriteThoughtResponse_StreamWithoutThoughts(t *testing.T) { require.Equal(t, body, string(rewritten)) } +func TestRewriteThoughtResponse_StreamFunctionCallFilter(t *testing.T) { + t.Parallel() + + lines := []string{ + `data: {"choices":[{"delta":{"role":"assistant","content":"**Planning**\n\nStep one.","extra_content":{"google":{"thought":true}}},"index":0}],"object":"chat.completion.chunk"}`, + ``, + `data: {"choices":[{"delta":{"content":"","extra_content":null},"index":0}],"object":"chat.completion.chunk"}`, + ``, + `data: {"choices":[{"delta":{"content":null},"finish_reason":"function_call_filter: MALFORMED_FUNCTION_CALL","index":0}],"usage":{"completion_tokens":3392,"prompt_tokens":100,"total_tokens":3492}}`, + ``, + `data: [DONE]`, + ``, + } + resp := &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{"Content-Type": []string{"text/event-stream"}}, + Body: io.NopCloser(strings.NewReader(strings.Join(lines, "\n"))), + } + + googleopenai.RewriteThoughtResponse(resp) + rewritten, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + out := strings.Split(string(rewritten), "\n") + require.Len(t, out, len(lines)) + + first := gjson.Get(strings.TrimPrefix(out[0], "data: "), "choices.0.delta") + require.Equal(t, "**Planning**\n\nStep one.", first.Get("reasoning_content").String()) + + require.True(t, strings.HasPrefix(out[4], "data: ")) + errPayload := gjson.Get(strings.TrimPrefix(out[4], "data: "), "error") + require.True(t, errPayload.Exists()) + require.Contains(t, errPayload.Get("message").String(), `"function_call_filter: MALFORMED_FUNCTION_CALL"`) + require.Equal(t, "invalid_response_error", errPayload.Get("type").String()) + require.Equal(t, "malformed_function_call", errPayload.Get("code").String()) + + require.Equal(t, `data: [DONE]`, out[6]) +} + +func TestRewriteThoughtResponse_StreamStandardFinishReasonsPassThrough(t *testing.T) { + t.Parallel() + + for _, reason := range []string{"stop", "tool_calls", "length", "content_filter"} { + t.Run(reason, func(t *testing.T) { + t.Parallel() + + line := `data: {"choices":[{"delta":{"content":null},"finish_reason":"` + reason + `","index":0}]}` + "\n" + resp := &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{"Content-Type": []string{"text/event-stream"}}, + Body: io.NopCloser(strings.NewReader(line)), + } + + googleopenai.RewriteThoughtResponse(resp) + rewritten, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + require.Equal(t, line, string(rewritten)) + }) + } +} + func TestRewriteThoughtResponse_StreamToolCallsEndThought(t *testing.T) { t.Parallel()