From e2bbd121372be47b5b1a6ae24b22b94673e47e13 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Tue, 31 Mar 2026 23:04:56 +0200 Subject: [PATCH] test(coderd/x/chatd): remove flaky OpenAI round-trip tests (#23877) --- coderd/x/chatd/integration_test.go | 322 ----------------------------- 1 file changed, 322 deletions(-) diff --git a/coderd/x/chatd/integration_test.go b/coderd/x/chatd/integration_test.go index c53e2d8eb6..f0e80dde32 100644 --- a/coderd/x/chatd/integration_test.go +++ b/coderd/x/chatd/integration_test.go @@ -1,24 +1,14 @@ package chatd_test import ( - "bytes" "context" - "encoding/json" - "io" - "net/http" - "net/http/httptest" "os" - "strconv" - "strings" - "sync" - "sync/atomic" "testing" "github.com/stretchr/testify/require" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/util/ptr" - "github.com/coder/coder/v2/coderd/x/chatd/chattest" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" ) @@ -597,315 +587,3 @@ func partTypeSet(parts []codersdk.ChatMessagePart) map[codersdk.ChatMessagePartT } return set } - -type openAIStoreMode string - -const ( - openAIStoreModeTrue openAIStoreMode = "store_true" - openAIStoreModeFalse openAIStoreMode = "store_false" -) - -func TestOpenAIReasoningWithWebSearchRoundTrip(t *testing.T) { - t.Parallel() - runOpenAIReasoningWithWebSearchRoundTripTest(t, openAIStoreModeTrue) -} - -func TestOpenAIReasoningWithWebSearchRoundTripStoreFalse(t *testing.T) { - t.Parallel() - runOpenAIReasoningWithWebSearchRoundTripTest(t, openAIStoreModeFalse) -} - -func runOpenAIReasoningWithWebSearchRoundTripTest(t *testing.T, storeMode openAIStoreMode) { - t.Helper() - - ctx := testutil.Context(t, testutil.WaitLong) - store := storeMode == openAIStoreModeTrue - - type capturedOpenAIRequest struct { - Stream bool `json:"stream,omitempty"` - Store *bool `json:"store,omitempty"` - PreviousResponseID *string `json:"previous_response_id,omitempty"` - Prompt []interface{} `json:"input,omitempty"` - } - - var ( - streamRequestCount atomic.Int32 - firstReq *capturedOpenAIRequest - secondReq *capturedOpenAIRequest - mu sync.Mutex - ) - upstreamOpenAIURL := chattest.NewOpenAI(t, func(req *chattest.OpenAIRequest) chattest.OpenAIResponse { - if !req.Stream { - return chattest.OpenAINonStreamingResponse("reasoning + web search title") - } - - switch req.Header.Get("X-Request-Ordinal") { - case "1": - return chattest.OpenAIResponse{ - ResponseID: "resp_first_test", - StreamingChunks: chattest.OpenAIStreamingResponse( - chattest.OpenAITextChunks("Here is what I found.")..., - ).StreamingChunks, - Reasoning: &chattest.OpenAIReasoningItem{ - Summary: "thinking about the question", - EncryptedContent: "encrypted_data_here", - }, - WebSearch: &chattest.OpenAIWebSearchCall{ - Query: "latest AI news", - }, - } - default: - return chattest.OpenAIStreamingResponse( - chattest.OpenAITextChunks("Follow-up answer.")..., - ) - } - }) - captureServer := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { - body, err := io.ReadAll(r.Body) - if err != nil { - t.Errorf("read OpenAI request body: %v", err) - http.Error(rw, err.Error(), http.StatusInternalServerError) - return - } - _ = r.Body.Close() - - if r.URL.Path == "/responses" { - var captured capturedOpenAIRequest - if err := json.Unmarshal(body, &captured); err != nil { - t.Errorf("decode OpenAI request body: %v", err) - http.Error(rw, err.Error(), http.StatusBadRequest) - return - } - if captured.Stream { - requestCount := streamRequestCount.Add(1) - r.Header.Set("X-Request-Ordinal", strconv.Itoa(int(requestCount))) - - mu.Lock() - switch requestCount { - case 1: - firstReq = &captured - default: - secondReq = &captured - } - mu.Unlock() - } - } - - upstreamReq, err := http.NewRequestWithContext( - r.Context(), - r.Method, - upstreamOpenAIURL+r.URL.RequestURI(), - bytes.NewReader(body), - ) - if err != nil { - t.Errorf("create upstream OpenAI request: %v", err) - http.Error(rw, err.Error(), http.StatusInternalServerError) - return - } - upstreamReq.Header = r.Header.Clone() - - resp, err := http.DefaultClient.Do(upstreamReq) - if err != nil { - t.Errorf("forward OpenAI request: %v", err) - http.Error(rw, err.Error(), http.StatusBadGateway) - return - } - defer resp.Body.Close() - - for key, values := range resp.Header { - for _, value := range values { - rw.Header().Add(key, value) - } - } - rw.WriteHeader(resp.StatusCode) - if _, err := io.Copy(rw, resp.Body); err != nil { - t.Errorf("copy OpenAI response body: %v", err) - } - })) - t.Cleanup(captureServer.Close) - openAIURL := captureServer.URL - - deploymentValues := coderdtest.DeploymentValues(t) - deploymentValues.Experiments = []string{string(codersdk.ExperimentAgents)} - client := coderdtest.New(t, &coderdtest.Options{ - DeploymentValues: deploymentValues, - }) - _ = coderdtest.CreateFirstUser(t, client) - expClient := codersdk.NewExperimentalClient(client) - - _, err := expClient.CreateChatProvider(ctx, codersdk.CreateChatProviderConfigRequest{ - Provider: "openai", - APIKey: "test-api-key", - BaseURL: openAIURL, - }) - require.NoError(t, err) - - contextLimit := int64(200000) - isDefault := true - reasoningEffort := "medium" - reasoningSummary := "auto" - _, err = expClient.CreateChatModelConfig(ctx, codersdk.CreateChatModelConfigRequest{ - Provider: "openai", - Model: "o4-mini", - ContextLimit: &contextLimit, - IsDefault: &isDefault, - ModelConfig: &codersdk.ChatModelCallConfig{ - ProviderOptions: &codersdk.ChatModelProviderOptions{ - OpenAI: &codersdk.ChatModelOpenAIProviderOptions{ - Store: ptr.Ref(store), - ReasoningEffort: &reasoningEffort, - ReasoningSummary: &reasoningSummary, - WebSearchEnabled: ptr.Ref(true), - }, - }, - }, - }) - require.NoError(t, err) - - t.Logf("Creating chat with reasoning + web search query (store=%t)...", store) - chat, err := expClient.CreateChat(ctx, codersdk.CreateChatRequest{ - Content: []codersdk.ChatInputPart{{ - Type: codersdk.ChatInputPartTypeText, - Text: "Search for the latest AI news and summarize it briefly.", - }}, - }) - require.NoError(t, err) - - events, closer, err := expClient.StreamChat(ctx, chat.ID, nil) - require.NoError(t, err) - defer closer.Close() - - waitForChatDone(ctx, t, events, "step 1") - - chatData, err := expClient.GetChat(ctx, chat.ID) - require.NoError(t, err) - chatMsgs, err := expClient.GetChatMessages(ctx, chat.ID, nil) - require.NoError(t, err) - require.Equal(t, codersdk.ChatStatusWaiting, chatData.Status, - "chat should be in waiting status after step 1") - - assistantMsg := findAssistantWithText(t, chatMsgs.Messages) - require.NotNil(t, assistantMsg, - "expected an assistant message with text content after step 1") - - partTypes := partTypeSet(assistantMsg.Content) - require.Contains(t, partTypes, codersdk.ChatMessagePartTypeReasoning, - "assistant message should contain reasoning parts") - require.Contains(t, partTypes, codersdk.ChatMessagePartTypeToolCall, - "assistant message should contain a provider-executed web search tool call") - require.Contains(t, partTypes, codersdk.ChatMessagePartTypeToolResult, - "assistant message should contain a provider-executed web search tool result") - require.Contains(t, partTypes, codersdk.ChatMessagePartTypeText, - "assistant message should contain a text part") - - var foundReasoning, foundWebSearchCall, foundText bool - for _, part := range assistantMsg.Content { - switch part.Type { - case codersdk.ChatMessagePartTypeReasoning: - // fantasy emits a leading newline when the reasoning summary part is - // added, so match the persisted summary text after trimming whitespace. - if strings.TrimSpace(part.Text) == "thinking about the question" { - foundReasoning = true - } - case codersdk.ChatMessagePartTypeToolCall: - if part.ToolName == "web_search" { - require.True(t, part.ProviderExecuted, - "web search tool-call should be marked provider-executed") - foundWebSearchCall = true - } - case codersdk.ChatMessagePartTypeText: - if part.Text == "Here is what I found." { - foundText = true - } - } - } - require.True(t, foundReasoning, "expected reasoning summary text to be persisted") - require.True(t, foundWebSearchCall, "expected persisted web_search tool call") - require.True(t, foundText, "expected streamed assistant text to be persisted") - - t.Log("Sending follow-up message...") - _, err = expClient.CreateChatMessage(ctx, chat.ID, codersdk.CreateChatMessageRequest{ - Content: []codersdk.ChatInputPart{{ - Type: codersdk.ChatInputPartTypeText, - Text: "What is the follow-up takeaway?", - }}, - }) - if !store && err != nil { - require.NotContains(t, err.Error(), - "Items are not persisted when store is set to false.", - "follow-up should reconstruct store=false responses without stale provider item IDs") - } - require.NoError(t, err) - - events2, closer2, err := expClient.StreamChat(ctx, chat.ID, nil) - require.NoError(t, err) - defer closer2.Close() - - waitForChatDone(ctx, t, events2, "step 2") - - chatData2, err := expClient.GetChat(ctx, chat.ID) - require.NoError(t, err) - chatMsgs2, err := expClient.GetChatMessages(ctx, chat.ID, nil) - require.NoError(t, err) - require.Equal(t, codersdk.ChatStatusWaiting, chatData2.Status, - "chat should be in waiting status after step 2") - require.Greater(t, len(chatMsgs2.Messages), len(chatMsgs.Messages), - "follow-up should have added more messages") - require.NotNil(t, findLastAssistantWithText(t, chatMsgs2.Messages), - "expected an assistant message with text after the follow-up") - // waitForChatDone returns on the chat status event, which can arrive - // before the capture server records the second streamed request. - captureCtx, cancel := context.WithTimeout(ctx, testutil.WaitShort) - defer cancel() - require.True(t, testutil.Eventually(captureCtx, t, func(context.Context) bool { - mu.Lock() - defer mu.Unlock() - return secondReq != nil - }, testutil.IntervalFast), "expected second streaming request to be captured") - require.GreaterOrEqual(t, streamRequestCount.Load(), int32(2), - "expected at least two streamed OpenAI responses") - - mu.Lock() - defer mu.Unlock() - - require.NotNil(t, firstReq, "expected first streaming request to be captured") - if store { - require.NotNil(t, firstReq.Store, "first request should have store field") - require.True(t, *firstReq.Store, "store should be true") - } else if firstReq.Store != nil { - require.False(t, *firstReq.Store, "store should be false") - } - - require.NotNil(t, secondReq, "expected second streaming request to be captured") - foundAssistantReplay := false - for _, item := range secondReq.Prompt { - m, ok := item.(map[string]interface{}) - if !ok { - continue - } - role, _ := m["role"].(string) - if role == "assistant" { - foundAssistantReplay = true - } - if store { - require.NotEqual(t, "assistant", role, - "store=true chain-mode prompt should not replay assistant messages") - require.NotEqual(t, "tool", role, - "store=true chain-mode prompt should not replay tool messages") - } - } - - if store { - require.NotNil(t, secondReq.PreviousResponseID, - "store=true follow-up should set previous_response_id") - require.Equal(t, "resp_first_test", *secondReq.PreviousResponseID, - "previous_response_id should match the first response's ID") - } else { - if secondReq.PreviousResponseID != nil { - require.Empty(t, *secondReq.PreviousResponseID, - "store=false follow-up should not set previous_response_id") - } - require.True(t, foundAssistantReplay, - "store=false follow-up should replay prior assistant history") - } -}