mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
test(coderd/x/chatd): remove flaky OpenAI round-trip tests (#23877)
This commit is contained in:
Michael Suchacz
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user