Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit 68e0fb0

Browse files
authored
fix/openai: non-streaming LLM response (#64473)
Fixes CODY-3194 Previously, using `/chat/completions` with OpenAI models always returned an empty completion because we were reading a non-existent `"text"` property instead of the nested `"message": { "content": ...}` property. This PR fixes the bug and adds a test case to demonstrate how we parse a real-world OpenAI response. <!-- PR description tips: https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e --> ## Test plan See new test cases. I'm failing to get a locally running setup to manually test this e2e, see https://sourcegraph.slack.com/archives/C04MYFW01NV/p1723664513193939 <!-- REQUIRED; info at https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles --> ## Changelog * Fix bug where requests to `/.api/completions/stream` for OpenAI models returned an empty completion when using `stream: false`. <!-- OPTIONAL; info at https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c -->
1 parent 5a3366e commit 68e0fb0

File tree

3 files changed

+64
-24
lines changed

3 files changed

+64
-24
lines changed

internal/completions/client/openai/openai.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func (c *openAIChatCompletionStreamClient) Complete(
7474
logger.Warn("Failed to count tokens with the token manager %w ", log.Error(err))
7575
}
7676
return &types.CompletionResponse{
77-
Completion: response.Choices[0].Text,
77+
Completion: response.Choices[0].Message.Content,
7878
StopReason: response.Choices[0].FinishReason,
7979
}, nil
8080
}
@@ -138,7 +138,7 @@ func (c *openAIChatCompletionStreamClient) Stream(
138138

139139
if len(event.Choices) > 0 {
140140
if request.Feature == types.CompletionsFeatureCode {
141-
content += event.Choices[0].Text
141+
content += event.Choices[0].Message.Content
142142
} else {
143143
content += event.Choices[0].Delta.Content
144144
}

internal/completions/client/openai/openai_test.go

Lines changed: 57 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -25,32 +25,35 @@ func (c *mockDoer) Do(r *http.Request) (*http.Response, error) {
2525
return c.do(r)
2626
}
2727

28-
func TestErrStatusNotOK(t *testing.T) {
29-
tokenManager := tokenusage.NewManager()
30-
mockClient := NewClient(&mockDoer{
28+
var compRequest = types.CompletionRequest{
29+
Feature: types.CompletionsFeatureChat,
30+
Version: types.CompletionsVersionLegacy,
31+
ModelConfigInfo: types.ModelConfigInfo{
32+
Provider: modelconfigSDK.Provider{
33+
ID: modelconfigSDK.ProviderID("xxx-provider-id-xxx"),
34+
},
35+
Model: modelconfigSDK.Model{
36+
ModelRef: modelconfigSDK.ModelRef("provider::apiversion::test-model"),
37+
},
38+
},
39+
Parameters: types.CompletionRequestParameters{
40+
RequestedModel: "xxx-requested-model-xxx",
41+
},
42+
}
43+
44+
func NewMockClient(statusCode int, response string) types.CompletionsClient {
45+
return NewClient(&mockDoer{
3146
func(r *http.Request) (*http.Response, error) {
3247
return &http.Response{
33-
StatusCode: http.StatusTooManyRequests,
34-
Body: io.NopCloser(bytes.NewReader([]byte("oh no, please slow down!"))),
48+
StatusCode: statusCode,
49+
Body: io.NopCloser(bytes.NewReader([]byte(response))),
3550
}, nil
3651
},
37-
}, "", "", *tokenManager)
52+
}, "", "", *tokenusage.NewManager())
53+
}
3854

39-
compRequest := types.CompletionRequest{
40-
Feature: types.CompletionsFeatureChat,
41-
Version: types.CompletionsVersionLegacy,
42-
ModelConfigInfo: types.ModelConfigInfo{
43-
Provider: modelconfigSDK.Provider{
44-
ID: modelconfigSDK.ProviderID("xxx-provider-id-xxx"),
45-
},
46-
Model: modelconfigSDK.Model{
47-
ModelRef: modelconfigSDK.ModelRef("provider::apiversion::test-model"),
48-
},
49-
},
50-
Parameters: types.CompletionRequestParameters{
51-
RequestedModel: "xxx-requested-model-xxx",
52-
},
53-
}
55+
func TestErrStatusNotOK(t *testing.T) {
56+
mockClient := NewMockClient(http.StatusTooManyRequests, "oh no, please slow down!")
5457

5558
t.Run("Complete", func(t *testing.T) {
5659
logger := log.Scoped("completions")
@@ -74,3 +77,36 @@ func TestErrStatusNotOK(t *testing.T) {
7477
assert.True(t, ok)
7578
})
7679
}
80+
81+
func TestNonStreamingResponseParsing(t *testing.T) {
82+
mockClient := NewMockClient(http.StatusOK, `{
83+
"id": "chatcmpl-9wEJ9hnLdPcCLrfdZLrRPGOz48Pmo",
84+
"object": "chat.completion",
85+
"created": 1723665051,
86+
"model": "gpt-4o-mini-2024-07-18",
87+
"choices": [
88+
{
89+
"index": 0,
90+
"message": {
91+
"role": "assistant",
92+
"content": "yes",
93+
"refusal": null
94+
},
95+
"logprobs": null,
96+
"finish_reason": "stop"
97+
}
98+
],
99+
"usage": {
100+
"prompt_tokens": 15,
101+
"completion_tokens": 1,
102+
"total_tokens": 16
103+
},
104+
"system_fingerprint": "fp_48196bc67a"
105+
}`)
106+
logger := log.Scoped("completions")
107+
resp, err := mockClient.Complete(context.Background(), logger, compRequest)
108+
require.NoError(t, err)
109+
assert.NotNil(t, resp)
110+
autogold.Expect(&types.CompletionResponse{Completion: "yes", StopReason: "stop"}).Equal(t, resp)
111+
112+
}

internal/completions/client/openai/types.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,14 @@ type openaiChoiceDelta struct {
5050
Content string `json:"content"`
5151
}
5252

53+
type openaiMessage struct {
54+
Content string `json:"content"`
55+
}
56+
5357
type openaiChoice struct {
5458
Delta openaiChoiceDelta `json:"delta"`
59+
Message openaiMessage `json:"message"`
5560
Role string `json:"role"`
56-
Text string `json:"text"`
5761
FinishReason string `json:"finish_reason"`
5862
}
5963

0 commit comments

Comments
 (0)