JSON-encode structured tool results for OpenAI instead of Go %v#525
Open
PratikDhanave wants to merge 3 commits into
Open
JSON-encode structured tool results for OpenAI instead of Go %v#525PratikDhanave wants to merge 3 commits into
PratikDhanave wants to merge 3 commits into
Conversation
Both the Chat Completions and Responses paths rendered a non-string
FunctionResultContent.Result with fmt.Sprintf("%v", ...). A struct or map
result — exactly what a typed functool.New[In,Out] with a struct Out
returns — was therefore sent to the model as Go's rendering (e.g.
"{Paris 20}") instead of JSON ("{\"city\":\"Paris\",\"temp_c\":20}").
Add a shared toolResultText helper that JSON-encodes structured results
(passing strings/raw JSON/[]byte through and rendering errors via Error()),
mirroring the anthropic and gemini providers which already json.Marshal.
Adds a black-box test asserting a struct tool result is JSON-encoded in the
request body, not rendered with %v.
There was a problem hiding this comment.
Pull request overview
This PR fixes how OpenAI request builders serialize function/tool results by JSON-encoding structured outputs (e.g., structs/maps) instead of using Go’s %v formatting, aligning OpenAI provider behavior with the Anthropic/Gemini providers and improving model-side parseability.
Changes:
- Introduces a shared
toolResultText(ret any) stringhelper to render tool results as JSON where appropriate. - Uses
toolResultTextin both Chat Completions (chat.go) and Responses API (responses.go) tool-result paths. - Adds a non-streaming chat test covering struct tool results serialization.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| provider/openaiprovider/responses.go | Switches default tool-result serialization from fmt.Sprintf("%v", ret) to toolResultText(ret) for Responses API inputs. |
| provider/openaiprovider/chat.go | Adds toolResultText helper and uses it for RoleTool messages in Chat Completions requests. |
| provider/openaiprovider/chat_test.go | Adds a test to ensure structured tool results are JSON-encoded in Chat Completions requests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+749
to
754
| // Default case - convert to string (JSON-encode structured | ||
| // results rather than rendering them with Go's %v). | ||
| resp = append(resp, responses.ResponseInputItemParamOfFunctionCallOutput( | ||
| funcResult.CallID, | ||
| fmt.Sprintf("%v", ret), | ||
| toolResultText(ret), | ||
| )) |
Comment on lines
+1559
to
+1565
| var captured string | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| b, _ := io.ReadAll(r.Body) | ||
| captured = string(b) | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _, _ = io.WriteString(w, `{"id":"x","object":"chat.completion","created":1,"model":"gpt-4o-mini","choices":[{"index":0,"message":{"role":"assistant","content":"ok"},"finish_reason":"stop"}]}`) | ||
| })) |
Contributor
Author
There was a problem hiding this comment.
Done — the captured request body now passes through a buffered channel instead of a shared variable, so the assertions are race-free. Also added a Responses-path structured-result test (the other review point).
Comment on lines
+1585
to
+1590
| if strings.Contains(captured, "{Paris 20}") { | ||
| t.Errorf("tool result rendered with Go %%v instead of JSON:\n%s", captured) | ||
| } | ||
| if !strings.Contains(captured, "temp_c") { | ||
| t.Errorf("tool result was not JSON-encoded (missing field temp_c):\n%s", captured) | ||
| } |
Address review feedback: - Add TestResponsesToolResult_StructSerializedAsJSON covering a struct FunctionResultContent.Result through the Responses API path (the fix changed responses.go too, but only the chat path had a test). - Pass the captured request body through a channel instead of a shared variable so the non-streaming chat tool-result test is race-free.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Both OpenAI request-building paths render a non-string
FunctionResultContent.Resultwithfmt.Sprintf("%v", ret):provider/openaiprovider/chat.go(RoleToolhandling)provider/openaiprovider/responses.go(function-call-output default case)So a struct or map result — exactly what a typed
functool.New[In, Out]with a structOutreturns (the handler returns the rawOutvalue, not a JSON string) — is sent to the model as Go's%vrendering instead of JSON:weather{City: "Paris", TempC: 20}→"{Paris 20}"sent to the model, instead of{"city":"Paris","temp_c":20}.The
anthropicandgeminiproviders alreadyjson.Marshalsuch results; only the two OpenAI paths don't.Fix
Add a shared
toolResultText(ret any) stringhelper that JSON-encodes structured results, while passingstring/json.RawMessage/[]bytethrough unchanged and renderingerrorvia.Error()(andnil→"", which also removes a stray"<nil>"on the Responses path). Use it in both paths.Public API
No exported symbols change.
Tests
Adds
TestChatToolResult_StructSerializedAsJSON_NonStreaming(black-box, inchat_test.go): aRoleToolmessage with a struct result. The request body contains"{Paris 20}"(Go%v) before this change and the JSON-encodedtemp_cfield after. Full./provider/openaiprovidersuite passes.