ChatCompletionRequest discards stream field when set to false

[odannyc]

Your issue may already be reported!
Please search on the issue tracker before creating one.

Describe the bug
A clear and concise description of what the bug is. If it's an API-related bug, please provide relevant endpoint(s).

I'd like to explicitly set the stream field to false and have that sent over to the server. Unfortunately, that field has the omitempty json tag which causes it to be discarded from the final request payload. Can we remove the omitempty json tag?

Source:

go-openai/chat.go

Line 51 in c3b2451

Stream bool `json:"stream,omitempty"`

[vvatanabe]

@odannyc The 'stream' field is optional, and its default value is false. Therefore, even if we don't remove 'omitempty', it will be set to false unless explicitly specified otherwise.

Refs:

• https://platform.openai.com/docs/api-reference/chat/create#chat/create-stream
• https://platform.openai.com/docs/api-reference/completions/create#completions/create-stream

[odannyc]

Thank you for the quick response @vvatanabe - that is true for open AI. But there are other services to might default to true in the case where it's not sent. In my specific case, we have an internal service which defaults it to true if a null is sent or that field isn't included in the request.

IMO if I set it to the false in the struct of this library, I expect it to be sent as false and not discarded (via omitempty).

[vvatanabe]

I have summarized the pros and cons for both cases: removing the omitempty and not removing it.

Removing omitempty

Pros:

• Even if a field is set to its default value, it will always be included in the request payload. This is useful for services that have a different default value when a field is omitted.
• If a particular field is set with a specific intent, its value is assuredly sent to the server. This helps avoid accidental applications of default values.

Cons:

• The payload may increase in size, potentially leading to wasteful consumption of network bandwidth. This is because all fields, including those with default values, are sent.

Not removing omitempty

Pros:

• The payload size decreases, helping conserve network bandwidth. Fields with default values are omitted, and only necessary data is sent.
• If the receiving service properly sets default values, there should be no issues even if fields are omitted.

Cons:

• If a field is set to its default value, it won't be sent and its existence may be ignored. This is particularly problematic when the default value for an omitted field differs between the sender and receiver.
• Even if the sender explicitly sets a field to its default value, this setting may not be communicated to the receiver.

[vvatanabe]

Given that the Stream field is optional according to the OpenAI API specifications, it might be better to change it to a pointer as follows:

Stream *bool json:"stream,omitempty"

With this change, the value won't be sent unless the developer explicitly sets the Stream field. The value is sent only when it's explicitly set. This behavior aligns with the OpenAI API specifications.

[vvatanabe]

This behavior is not actually a bug, so I change the label.

[odannyc]

Thank you! Let me know if you need help with implementing this.

[ealvar3z]

@vvatanabe @odannyc This will indeed work, but be advised that it'll be hacky as you cannot pass untyped bool constants in struct literals "cleanly". Meaning that for every request that will require a change, the code will be littered w/ the following:

val := true // this is a hack to pass a pointer to a bool to the stream field 
stream, err := client.CreateChatCompletionStream(context.Background(), ChatCompletionRequest{
	MaxTokens: 5,
	Model:     GPT3Dot5Turbo,
	Messages: []ChatCompletionMessage{
		{
			Role:    ChatMessageRoleUser,
			Content: "Hello!",
		},
	},
	Stream: &val,
})

Or

stream, err := client.CreateChatCompletionStream(context.Background(), ChatCompletionRequest{
	MaxTokens: 5,
	Model:     GPT3Dot5Turbo,
	Messages: []ChatCompletionMessage{
		{
			Role:    ChatMessageRoleUser,
			Content: "Hello!",
		},
	},
        // another hack: this creates an array with a single bool element set to true, 
       // and then takes the address of the first element.
	Stream: &[]bool{true}[0],
})

Or

We can build a "cleaner" hack and create a function instead:

func BoolPtr(b bool) *bool { return &b }

// then call it in the struct literal
Stream: BoolPtr(true)

There's also a way to do this by creating default structs or the builder pattern, etc. With all of that being said. I still think that the trade-off may be worth it (and I would choose the first example), but it is important to note it for codebase readability.

Please advise