Repository: sashabaranov/go-openai
Pull request #699 (2024-03-29 08:31:31)

[BREAKING_CHANGES] Fix update message payload
**Describe the change**
Resolves https://github.com/sashabaranov/go-openai/issues/692
The ModifyMessage endpoint improperly wraps the payload. Including the expected map[string]any object into the method will result in an API error as the API requires it to be wrapped in "metadata".

This is a breaking change because to have successfully used this endpoint, developers would have run into this API error, debugged it and realized that the payload isn't being wrapped by the underlying code as expected by the API. 

**Provide OpenAI documentation link**
https://platform.openai.com/docs/api-reference/messages/modifyMessage

**Describe your solution**
This simply wraps the payload as expected by the API and as assumed by the interface. The interface accepts a metadata object, either the interface needs to change to accept a generic payload object (which would be equally misleading). Or we update the underlying code to properly wrap the payload as expected

**Tests**
Unit tests

**Additional context**
We will need to inform that anyone who figured out how to use this endpoint will need to update their code to remove the manual wrapping of the payload.

Issue: #692


Alternatively I can avoid the breaking change by only wrapping the payload if it's not already wrapped. I think this will create complexity down the road, but it is an option


qhenkart: I'm not sure why the Linter is breaking randomly. Did something change in the underlying build process? It doesn't look like it's due to any changes in this PR

```
golangci-lint run ./...
chat_test.go:330:1: cognitive complexity 30 of func `TestMultipartChatMessageSerialization` is high (> 20) (gocognit)
func TestMultipartChatMessageSerialization(t *testing.T) {
^
fine_tunes.go:118:2: directive `//nolint:goconst // Decreases readability` is unused for linter "goconst" (nolintlint)
	//nolint:goconst // Decreases readability
	^
messages.go:21:49: directive `//nolint:revive //backwards-compatibility` is unused for linter "revive" (nolintlint)
	FileIds     []string         `json:"file_ids"` //nolint:revive //backwards-compatibility
	                                               ^
messages.go:57:54: directive `//nolint:revive // backwards-compatibility` is unused for linter "revive" (nolintlint)
	FileIds  []string       `json:"file_ids,omitempty"` //nolint:revive // backwards-compatibility
	                                                    ^
run.go:27:54: directive `//nolint:revive // backwards-compatibility` is unused for linter "revive" (nolintlint)
	FileIDS        []string           `json:"file_ids"` //nolint:revive // backwards-compatibility
```
qhenkart: Also worth noting. metadata should be a map[string]string. Non string types are not accepted and can also lead to an error. Would you like me to include that in this PR? It will lead to more breaking changes, but also enforce the requirements of the API which are currently unenforced and lead to error
sashabaranov: @qhenkart thank you for the PR! The master branch contains the fix for linter, you can merge/rebase it here so the sanity check would pass!
qhenkart: @sashabaranov Okay thanks, this PR is ready for your final review

Patch:
diff --git a/messages.go b/messages.go
index 8614632..6fd0adb 100644
--- a/messages.go
+++ b/messages.go
@@ -139,11 +139,11 @@ func (c *Client) RetrieveMessage(
 func (c *Client) ModifyMessage(
 	ctx context.Context,
 	threadID, messageID string,
-	metadata map[string]any,
+	metadata map[string]string,
 ) (msg Message, err error) {
 	urlSuffix := fmt.Sprintf("/threads/%s/%s/%s", threadID, messagesSuffix, messageID)
 	req, err := c.newRequest(ctx, http.MethodPost, c.fullURL(urlSuffix),
-		withBody(metadata), withBetaAssistantV1())
+		withBody(map[string]any{"metadata": metadata}), withBetaAssistantV1())
 	if err != nil {
 		return
 	}


Test patch:
diff --git a/messages_test.go b/messages_test.go
index 9168d6c..a18be20 100644
--- a/messages_test.go
+++ b/messages_test.go
@@ -68,6 +68,10 @@ func TestMessages(t *testing.T) {
 				metadata := map[string]any{}
 				err := json.NewDecoder(r.Body).Decode(&metadata)
 				checks.NoError(t, err, "unable to decode metadata in modify message call")
+				payload, ok := metadata["metadata"].(map[string]any)
+				if !ok {
+					t.Fatalf("metadata payload improperly wrapped %+v", metadata)
+				}
 
 				resBytes, _ := json.Marshal(
 					openai.Message{
@@ -86,8 +90,9 @@ func TestMessages(t *testing.T) {
 						FileIds:     nil,
 						AssistantID: &emptyStr,
 						RunID:       &emptyStr,
-						Metadata:    metadata,
+						Metadata:    payload,
 					})
+
 				fmt.Fprintln(w, string(resBytes))
 			case http.MethodGet:
 				resBytes, _ := json.Marshal(
@@ -212,7 +217,7 @@ func TestMessages(t *testing.T) {
 	}
 
 	msg, err = client.ModifyMessage(ctx, threadID, messageID,
-		map[string]any{
+		map[string]string{
 			"foo": "bar",
 		})
 	checks.NoError(t, err, "ModifyMessage error")