Repository: sashabaranov/go-openai
Pull request #180 (2023-03-20 02:38:00)

refactor: Refactor endpoint and model compatibility check
- Add model check for `CreateCompletionStream` and `CreateChatCompletionStream`.
- Correct model check in `CreateCompletion`.

codecov[bot]: ## [Codecov](https://codecov.io/gh/sashabaranov/go-openai/pull/180?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#180](https://codecov.io/gh/sashabaranov/go-openai/pull/180?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (fda2e7a) into [master](https://codecov.io/gh/sashabaranov/go-openai/commit/d529d13ba15d31928a414f83f9ae9730d0a9223b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (d529d13) will **increase** coverage by `0.04%`.
> The diff coverage is `76.92%`.

```diff
@@            Coverage Diff             @@
##           master     #180      +/-   ##
==========================================
+ Coverage   70.65%   70.69%   +0.04%     
==========================================
  Files          18       18              
  Lines         535      546      +11     
==========================================
+ Hits          378      386       +8     
- Misses        121      124       +3     
  Partials       36       36              
```


| [Impacted Files](https://codecov.io/gh/sashabaranov/go-openai/pull/180?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [stream.go](https://codecov.io/gh/sashabaranov/go-openai/pull/180?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3RyZWFtLmdv) | `69.23% <40.00%> (-3.11%)` | :arrow_down: |
| [chat\_stream.go](https://codecov.io/gh/sashabaranov/go-openai/pull/180?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y2hhdF9zdHJlYW0uZ28=) | `80.39% <100.00%> (+2.13%)` | :arrow_up: |
| [completion.go](https://codecov.io/gh/sashabaranov/go-openai/pull/180?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29tcGxldGlvbi5nbw==) | `100.00% <100.00%> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

sashabaranov: Hey, thank you for the PR!
j178: Hi, I am wondering if it is necessary to have compatibility checks between the endpoint and model, or if it can be removed and let it fail. Here are some points to consider:

1. OpenAI will regularly release new models, which requires us to update the checks here in a timely manner, or else it will hinder users from using the new models.
2. If an incompatible model is used in the endpoint, OpenAI API will return a clear error message, which should be enough to inform users of the error.
sashabaranov: @j178 What if we check for "incompability" — basically return error only when we are sure that the model is not supported?
j178: > basically return error only when we are sure that the model is not supported?

While technically possible, this approach would be very long and difficult to maintain. We would need to explicitly list all unsupported models for each endpoint check, like this:

```go
func checkEndpointSupportsModel(endpoint, model string) error {
	switch endpoint {
	case "/completions":
		switch model {
		case GPT3Dot5Turbo0301, GPT3Dot5Turbo, GPT4, GPT40314, GPT432K, GPT432K0314:
			return ErrCompletionUnsupportedModel
		}
	case "/chat/completions":
		switch model {
		case GPT3TextDavinci003, GPT3TextDavinci002, GPT3TextCurie001, GPT3TextBabbage001, GPT3TextAda001, GPT3TextDavinci001, GPT3DavinciInstructBeta, GPT3Davinci, GPT3CurieInstructBeta, GPT3Curie, GPT3Ada, GPT3Babbage, CodexCodeDavinci002, CodexCodeCushman001, CodexCodeDavinci001:
			return ErrChatCompletionInvalidModel
		}
	}
	return nil
}
```
sashabaranov: @j178 here's a simpler approach:

```go
type endpointModelPair struct {
	endpoint string
	model    string
}

var disabledPairs = map[endpointModelPair]bool{
	{"/completions", "gpt-3.5-turbo"}: true,
}

func main() {
	pairIsDisabled := disabledPairs[endpointModelPair{"/completions", "gpt-3.5-turbo"}]
	fmt.Println("Pair is disabled:", pairIsDisabled)
}
```

We can also use `map[string]map[string]bool` to have endpoint->model->disabled flag mapping.
j178: I applied your suggestion, but I also had to introduce a breaking change: I deleted `ErrChatCompletionInvalidModel` and `ErrCompletionUnsupportedModel`, and replaced them with a unified `ErrUnsupportedModel`. Do you have any better suggestions?
j178: The current CI failure is quite peculiar(I was able to pass the test locally). The panic stack trace appears at `stream_test.go:167` and `stream_test.go:169`, even though the `stream_test.go` file only has 165 lines in total.

```
=== RUN   TestCreateCompletionStreamError
    stream_test.go:167: CreateCompletionStream returned error: this model is not supported with this method, please use CreateChatCompletion client method instead
--- FAIL: TestCreateCompletionStreamError (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x88aac1]

goroutine 171 [running]:
testing.tRunner.func1.2({0x8c6f80, 0xbd9fc0})
	/opt/hostedtoolcache/go/1.19.7/x64/src/testing/testing.go:1396 +0x372
testing.tRunner.func1()
	/opt/hostedtoolcache/go/1.19.7/x64/src/testing/testing.go:1399 +0x5f0
panic({0x8c6f80, 0xbd9fc0})
	/opt/hostedtoolcache/go/1.19.7/x64/src/runtime/panic.go:890 +0x262
github.com/sashabaranov/go-openai_test.TestCreateCompletionStreamError(0xc00027fd40)
	/home/runner/work/go-openai/go-openai/stream_test.go:169 +0x581
testing.tRunner(0xc00027fd40, 0x954fa0)
	/opt/hostedtoolcache/go/1.19.7/x64/src/testing/testing.go:1446 +0x217
created by testing.(*T).Run
	/opt/hostedtoolcache/go/1.19.7/x64/src/testing/testing.go:1493 +0x75e
FAIL	github.com/sashabaranov/go-openai	0.196s
FAIL
Error: Process completed with exit code 1.
```
sashabaranov: In terms of panic: I don't even see the `TestCreateCompletionStreamError` in your fork, could you please sync your form with the master branch?

Patch:
diff --git a/chat.go b/chat.go
index 0f56216..c09861c 100644
--- a/chat.go
+++ b/chat.go
@@ -14,8 +14,8 @@ const (
 )
 
 var (
-	ErrChatCompletionInvalidModel       = errors.New("currently, only gpt-3.5-turbo and gpt-3.5-turbo-0301 are supported")                 //nolint:lll
-	ErrChatCompletionStreamNotSupported = errors.New("streaming is not supported with this method, please use CreateChatCompletionStream") //nolint:lll
+	ErrChatCompletionInvalidModel       = errors.New("this model is not supported with this method, please use CreateCompletion client method instead") //nolint:lll
+	ErrChatCompletionStreamNotSupported = errors.New("streaming is not supported with this method, please use CreateChatCompletionStream")              //nolint:lll
 )
 
 type ChatCompletionMessage struct {
@@ -71,14 +71,12 @@ func (c *Client) CreateChatCompletion(
 		return
 	}
 
-	switch request.Model {
-	case GPT3Dot5Turbo0301, GPT3Dot5Turbo, GPT4, GPT40314, GPT432K0314, GPT432K:
-	default:
+	urlSuffix := "/chat/completions"
+	if !checkEndpointSupportsModel(urlSuffix, request.Model) {
 		err = ErrChatCompletionInvalidModel
 		return
 	}
 
-	urlSuffix := "/chat/completions"
 	req, err := c.requestBuilder.build(ctx, http.MethodPost, c.fullURL(urlSuffix), request)
 	if err != nil {
 		return
diff --git a/chat_stream.go b/chat_stream.go
index 26c4bfc..8211292 100644
--- a/chat_stream.go
+++ b/chat_stream.go
@@ -37,8 +37,14 @@ func (c *Client) CreateChatCompletionStream(
 	ctx context.Context,
 	request ChatCompletionRequest,
 ) (stream *ChatCompletionStream, err error) {
+	urlSuffix := "/chat/completions"
+	if !checkEndpointSupportsModel(urlSuffix, request.Model) {
+		err = ErrChatCompletionInvalidModel
+		return
+	}
+
 	request.Stream = true
-	req, err := c.newStreamRequest(ctx, "POST", "/chat/completions", request)
+	req, err := c.newStreamRequest(ctx, "POST", urlSuffix, request)
 	if err != nil {
 		return
 	}
diff --git a/completion.go b/completion.go
index 22211d3..6617e5a 100644
--- a/completion.go
+++ b/completion.go
@@ -45,6 +45,38 @@ const (
 	CodexCodeDavinci001 = "code-davinci-001"
 )
 
+var disabledModelsForEndpoints = map[string]map[string]bool{
+	"/completions": {
+		GPT3Dot5Turbo:     true,
+		GPT3Dot5Turbo0301: true,
+		GPT4:              true,
+		GPT40314:          true,
+		GPT432K:           true,
+		GPT432K0314:       true,
+	},
+	"/chat/completions": {
+		CodexCodeDavinci002:     true,
+		CodexCodeCushman001:     true,
+		CodexCodeDavinci001:     true,
+		GPT3TextDavinci003:      true,
+		GPT3TextDavinci002:      true,
+		GPT3TextCurie001:        true,
+		GPT3TextBabbage001:      true,
+		GPT3TextAda001:          true,
+		GPT3TextDavinci001:      true,
+		GPT3DavinciInstructBeta: true,
+		GPT3Davinci:             true,
+		GPT3CurieInstructBeta:   true,
+		GPT3Curie:               true,
+		GPT3Ada:                 true,
+		GPT3Babbage:             true,
+	},
+}
+
+func checkEndpointSupportsModel(endpoint, model string) bool {
+	return !disabledModelsForEndpoints[endpoint][model]
+}
+
 // CompletionRequest represents a request structure for completion API.
 type CompletionRequest struct {
 	Model            string         `json:"model"`
@@ -105,12 +137,12 @@ func (c *Client) CreateCompletion(
 		return
 	}
 
-	if request.Model == GPT3Dot5Turbo0301 || request.Model == GPT3Dot5Turbo {
+	urlSuffix := "/completions"
+	if !checkEndpointSupportsModel(urlSuffix, request.Model) {
 		err = ErrCompletionUnsupportedModel
 		return
 	}
 
-	urlSuffix := "/completions"
 	req, err := c.requestBuilder.build(ctx, http.MethodPost, c.fullURL(urlSuffix), request)
 	if err != nil {
 		return
diff --git a/stream.go b/stream.go
index 322d27f..944546a 100644
--- a/stream.go
+++ b/stream.go
@@ -22,8 +22,14 @@ func (c *Client) CreateCompletionStream(
 	ctx context.Context,
 	request CompletionRequest,
 ) (stream *CompletionStream, err error) {
+	urlSuffix := "/completions"
+	if !checkEndpointSupportsModel(urlSuffix, request.Model) {
+		err = ErrCompletionUnsupportedModel
+		return
+	}
+
 	request.Stream = true
-	req, err := c.newStreamRequest(ctx, "POST", "/completions", request)
+	req, err := c.newStreamRequest(ctx, "POST", urlSuffix, request)
 	if err != nil {
 		return
 	}


Test patch:
diff --git a/chat_stream_test.go b/chat_stream_test.go
index de604fa..a21ceee 100644
--- a/chat_stream_test.go
+++ b/chat_stream_test.go
@@ -13,6 +13,28 @@ import (
 	"testing"
 )
 
+func TestChatCompletionsStreamWrongModel(t *testing.T) {
+	config := DefaultConfig("whatever")
+	config.BaseURL = "http://localhost/v1"
+	client := NewClientWithConfig(config)
+	ctx := context.Background()
+
+	req := ChatCompletionRequest{
+		MaxTokens: 5,
+		Model:     "ada",
+		Messages: []ChatCompletionMessage{
+			{
+				Role:    ChatMessageRoleUser,
+				Content: "Hello!",
+			},
+		},
+	}
+	_, err := client.CreateChatCompletionStream(ctx, req)
+	if !errors.Is(err, ErrChatCompletionInvalidModel) {
+		t.Fatalf("CreateChatCompletion should return ErrChatCompletionInvalidModel, but returned: %v", err)
+	}
+}
+
 func TestCreateChatCompletionStream(t *testing.T) {
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "text/event-stream")
diff --git a/chat_test.go b/chat_test.go
index 8866ff2..2d569a4 100644
--- a/chat_test.go
+++ b/chat_test.go
@@ -34,7 +34,7 @@ func TestChatCompletionsWrongModel(t *testing.T) {
 	}
 	_, err := client.CreateChatCompletion(ctx, req)
 	if !errors.Is(err, ErrChatCompletionInvalidModel) {
-		t.Fatalf("CreateChatCompletion should return wrong model error, but returned: %v", err)
+		t.Fatalf("CreateChatCompletion should return ErrChatCompletionInvalidModel, but returned: %v", err)
 	}
 }
 
diff --git a/request_builder_test.go b/request_builder_test.go
index f0f99ee..0f14f93 100644
--- a/request_builder_test.go
+++ b/request_builder_test.go
@@ -61,7 +61,7 @@ func TestClientReturnsRequestBuilderErrors(t *testing.T) {
 		t.Fatalf("Did not return error when request builder failed: %v", err)
 	}
 
-	_, err = client.CreateChatCompletionStream(ctx, ChatCompletionRequest{})
+	_, err = client.CreateChatCompletionStream(ctx, ChatCompletionRequest{Model: GPT3Dot5Turbo})
 	if !errors.Is(err, errTestRequestBuilderFailed) {
 		t.Fatalf("Did not return error when request builder failed: %v", err)
 	}
diff --git a/stream_test.go b/stream_test.go
index ce560c6..7d01ebd 100644
--- a/stream_test.go
+++ b/stream_test.go
@@ -12,6 +12,23 @@ import (
 	"testing"
 )
 
+func TestCompletionsStreamWrongModel(t *testing.T) {
+	config := DefaultConfig("whatever")
+	config.BaseURL = "http://localhost/v1"
+	client := NewClientWithConfig(config)
+
+	_, err := client.CreateCompletionStream(
+		context.Background(),
+		CompletionRequest{
+			MaxTokens: 5,
+			Model:     GPT3Dot5Turbo,
+		},
+	)
+	if !errors.Is(err, ErrCompletionUnsupportedModel) {
+		t.Fatalf("CreateCompletion should return ErrCompletionUnsupportedModel, but returned: %v", err)
+	}
+}
+
 func TestCreateCompletionStream(t *testing.T) {
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "text/event-stream")
@@ -140,7 +157,7 @@ func TestCreateCompletionStreamError(t *testing.T) {
 
 	request := CompletionRequest{
 		MaxTokens: 5,
-		Model:     GPT3Dot5Turbo,
+		Model:     GPT3TextDavinci003,
 		Prompt:    "Hello!",
 		Stream:    true,
 	}