feat(agents): support OpenAI Agents SDK#353
Open
SantiagoDePolonia wants to merge 14 commits into
Open
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR extends GoModel's OpenAI-compatible Responses API to support OpenAI Agents SDK portable flows by adding conversation references, expanded request controls, structured-output translation, and validated chat conversion with comprehensive documentation and runnable examples. ChangesOpenAI Agents SDK Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
|
Greptile SummaryThis PR adds OpenAI Agents SDK support on top of GoModel's Responses API. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Reviews (7): Last reviewed commit: "fix(agents): tighten conversation refere..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/gomodel/docs/docs.go`:
- Around line 5088-5106: The generated OpenAPI schema for
core.ConversationCreateRequest currently types "items" as array<array<int>>
(byte arrays) and the update schema does not mark "metadata" as required; fix
the source model used to generate docs so that
core.ConversationCreateRequest.items is defined as an array of opaque JSON
values (e.g., array of string/JSON) rather than array<array<int>>, and ensure
the ConversationUpdateRequest (or the update contract/type) marks the "metadata"
property as required; then re-run the Swagger/Docs generator to regenerate
cmd/gomodel/docs/docs.go so the core.ConversationCreateRequest,
ConversationUpdateRequest, and their "items" and "metadata" properties are
correctly emitted.
In `@docs/openapi.json`:
- Around line 7131-7140: The OpenAPI schema for core.ConversationUpdateRequest
is missing a required declaration for the metadata field; update the
ConversationUpdateRequest schema to include required: ["metadata"] so the API
spec matches internal/core/conversations.go (which expects a *map[string]string
to detect omitted vs empty) and ensures clients know metadata must be present on
update requests.
- Around line 7099-7107: The OpenAPI schema currently documents
ConversationCreateRequest.Items as nested arrays of integers; update the schema
for the JSON field backing ConversationCreateRequest.Items (which is
[]json.RawMessage) so each entry is treated as opaque JSON rather than []int.
Locate the block that currently nests "type":"array" with inner
"type":"array"/"type":"integer" and replace the inner item schema with an opaque
JSON schema (e.g. an unconstrained item or a oneOf covering
object/array/string/number/boolean/null) so codegen clients will emit arbitrary
JSON values instead of byte arrays.
In `@internal/providers/responses_adapter.go`:
- Around line 69-108: The Conversation field on core.ResponsesRequest is
currently typed as any which prevents compile-time checks; change
core.ResponsesRequest.Conversation to a concrete type (e.g., *string or a
dedicated struct representing the conversation shape) and update all call sites
and JSON unmarshalling logic accordingly so validation in
validateResponsesRequestForChatTranslation can remain a nil-check; ensure
references in validateResponsesRequestForChatTranslation, any request
decoding/parsing code, and downstream handlers are updated to use the new
concrete type and that unsupportedResponsesChatTranslationField continues to be
used for rejected fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 576fd7ec-8f1e-44b7-8554-57af88e2fb7d
📒 Files selected for processing (20)
cmd/gomodel/docs/docs.godocs/dev/agents-sdk-support.mddocs/dev/claude-agent-sdk-support.mddocs/docs.jsondocs/examples/openai-agents-sdk/README.mddocs/examples/openai-agents-sdk/javascript_basic.mjsdocs/examples/openai-agents-sdk/python_basic.pydocs/examples/openai-agents-sdk/python_streaming_tool.pydocs/guides/openai-agents-sdk.mdxdocs/openapi.jsoninternal/core/chat_json.gointernal/core/responses.gointernal/core/responses_json.gointernal/core/responses_json_test.gointernal/core/types.gointernal/providers/responses_adapter.gointernal/providers/responses_adapter_test.gointernal/providers/responses_input.gointernal/server/handlers_test.gointernal/server/translated_inference_service.go
Comment on lines
+69
to
+108
| func validateResponsesRequestForChatTranslation(req *core.ResponsesRequest) error { | ||
| if strings.TrimSpace(req.PreviousResponseID) != "" { | ||
| return unsupportedResponsesChatTranslationField("previous_response_id") | ||
| } | ||
| if req.Conversation != nil { | ||
| return unsupportedResponsesChatTranslationField("conversation") | ||
| } | ||
| if len(req.Include) > 0 { | ||
| return unsupportedResponsesChatTranslationField("include") | ||
| } | ||
| if req.Prompt != nil { | ||
| return unsupportedResponsesChatTranslationField("prompt") | ||
| } | ||
| if strings.TrimSpace(req.Truncation) != "" { | ||
| return unsupportedResponsesChatTranslationField("truncation") | ||
| } | ||
| if req.Text != nil { | ||
| return unsupportedResponsesChatTranslationField("text") | ||
| } | ||
| if strings.TrimSpace(req.PromptCacheRetention) != "" { | ||
| return unsupportedResponsesChatTranslationField("prompt_cache_retention") | ||
| } | ||
| if req.ContextManagement != nil { | ||
| return unsupportedResponsesChatTranslationField("context_management") | ||
| } | ||
| if req.TopLogprobs != nil { | ||
| return unsupportedResponsesChatTranslationField("top_logprobs") | ||
| } | ||
| if strings.TrimSpace(req.SafetyIdentifier) != "" { | ||
| return unsupportedResponsesChatTranslationField("safety_identifier") | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func unsupportedResponsesChatTranslationField(field string) error { | ||
| return core.NewInvalidRequestError( | ||
| fmt.Sprintf("responses field %q is only supported by native Responses providers; use an OpenAI-compatible provider or passthrough for this request", field), | ||
| nil, | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the type definition of ResponsesRequest.Conversation
ast-grep --pattern $'type ResponsesRequest struct {
$$$
Conversation $FIELD_TYPE
$$$
}'Repository: ENTERPILOT/GoModel
Length of output: 3108
Type ResponsesRequest.Conversation instead of any
The req.Conversation != nil check is correct because internal/core.ResponsesRequest.Conversation is typed as any; the issue is that the API payload field is untyped. Change Conversation to a concrete type (e.g., *string or a dedicated struct) so validation and downstream handling are type-safe.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/providers/responses_adapter.go` around lines 69 - 108, The
Conversation field on core.ResponsesRequest is currently typed as any which
prevents compile-time checks; change core.ResponsesRequest.Conversation to a
concrete type (e.g., *string or a dedicated struct representing the conversation
shape) and update all call sites and JSON unmarshalling logic accordingly so
validation in validateResponsesRequestForChatTranslation can remain a nil-check;
ensure references in validateResponsesRequestForChatTranslation, any request
decoding/parsing code, and downstream handlers are updated to use the new
concrete type and that unsupportedResponsesChatTranslationField continues to be
used for rejected fields.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/openapi.json`:
- Around line 7655-7662: The "conversation" schema's description says it accepts
either a conversation ID string or an object with id, but the schema currently
only references core.ResponsesConversationRef (object), breaking clients that
send strings; update the "conversation" schema (and the other occurrences noted)
to allow both by using a oneOf/anyOf that includes the string type and the
existing $ref (e.g., oneOf: [{ "type": "string" }, { "$ref":
"`#/components/schemas/core.ResponsesConversationRef`" }]) so generated clients
can send either a plain ID string or the object with id.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4d27f74b-707a-4fc2-8b9c-84abd2f26abb
📒 Files selected for processing (11)
cmd/gomodel/docs/docs.godocs/dev/agents-sdk-support.mddocs/openapi.jsoninternal/core/conversations.gointernal/core/responses.gointernal/core/responses_json.gointernal/core/responses_json_test.gointernal/providers/openai/compatible_provider.gointernal/providers/openai/openai_test.gointernal/providers/responses_adapter.gointernal/providers/responses_adapter_test.go
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# Conflicts: # cmd/gomodel/docs/docs.go # docs/openapi.json
… items Unknown Responses input item types stored the full payload in Raw and also extracted every non-type field into ExtraFields. Marshaling then merged both, emitting duplicate JSON keys (e.g. id, summary) to native Responses providers. Preserve the verbatim Raw on decode and reserve ExtraFields for metadata added after decoding, so a round trip emits each field once. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Chat-translated providers previously rejected any Responses request carrying structured text output settings. Map text.format onto the Chat Completions response_format (json_schema fields are nested under a json_schema member; json_object passes through) and forward text.verbosity, so the Agents SDK can request structured outputs on non-native providers. Unknown text formats still return a clear error. Adds core.MergeUnknownJSONFields to inject the derived passthrough fields, reusing the same extras path providers already read for response_format. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/core/responses_json.go (1)
448-505:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset
ResponsesInputElementbefore repopulating it.
UnmarshalJSONonly assigns fields that are present in the new payload. Reusing the sameResponsesInputElementleaves staleStatus,CallID,Content,Raw, and especiallyExtraFieldsbehind; with the new unknown-type fast path, a second marshal can merge unrelated keys into the preserved raw item.Suggested fix
func (e *ResponsesInputElement) UnmarshalJSON(data []byte) error { var raw map[string]json.RawMessage if err := json.Unmarshal(data, &raw); err != nil { return err } + + *e = ResponsesInputElement{} if v, ok := raw["type"]; ok { _ = json.Unmarshal(v, &e.Type) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/core/responses_json.go` around lines 448 - 505, UnmarshalJSON for ResponsesInputElement must clear any existing state before repopulating to avoid carrying over stale fields (Status, CallID, Content, Raw, ExtraFields); after successfully decoding into raw (the local variable map[string]json.RawMessage) reset the receiver with *e = ResponsesInputElement{} and then proceed with the existing logic that reads raw and assigns to e.Type, e.Name, e.CallID, e.Status, e.Arguments, e.Output, e.Role, e.Content or sets e.Raw; this ensures ExtraFields and other fields are not preserved across calls to ResponsesInputElement.UnmarshalJSON.
♻️ Duplicate comments (1)
docs/openapi.json (1)
7913-7919:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
conversationis still modeled as object-only.The description says these fields accept either a conversation ID string or an object with
id, but the schema still only referencescore.ResponsesConversationRef, and that ref still does not requireid. Generated clients will reject valid string IDs and may allow{}for the object form.Also applies to: 8086-8092, 8240-8246, 8296-8302
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi.json` around lines 7913 - 7919, The "conversation" schema currently only references core.ResponsesConversationRef and that ref does not require id, so clients reject valid string IDs and may accept empty objects; update the schemas (e.g., the "conversation" property and the referenced core.ResponsesConversationRef) to allow either a string ID or an object with a required id by changing the "conversation" schema to a oneOf (or anyOf) that accepts { type: "string" } or an object reference to core.ResponsesConversationRef, and modify core.ResponsesConversationRef to require the id property (add "required": ["id"] or ensure the object schema enforces id) so both forms (string or { id: "..." }) validate correctly; apply the same change to the other occurrences mentioned (lines referencing the same "conversation" schema at 8086-8092, 8240-8246, 8296-8302).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/guides/openai-agents-sdk.mdx`:
- Around line 133-157: The snippet uses top-level await and an undefined agent;
wrap the Runner.run call inside an async function (e.g., async def main()) and
call it with asyncio.run(main()), and either provide a minimal agent definition
or add a comment that agent is defined earlier; specifically update the block
that constructs AsyncOpenAI, RunConfig/MultiProvider and the Runner.run(...)
invocation so Runner.run is called from within main() (use asyncio.run) and
ensure agent is declared or referenced as continuing from a prior example.
In `@internal/core/json_fields.go`:
- Around line 83-92: The current code unmarshals base.raw into a map which
rewrites/loses the original raw formatting; instead build a merged raw object
without decoding: if base.IsEmpty() start with "{" otherwise scan/copy
base.raw's object members while skipping any keys present in additions (so
additions win), then append the additions as raw fields (encode keys with
json.Marshal to escape) separated by commas and close the object; replace the
json.Unmarshal/merged map logic in the block using base.raw, additions, and
returning UnknownJSONFieldsFromMap/UnknownJSONFields with this raw-object
merging using bytes.Buffer/json.RawMessage to preserve original byte layout for
unchanged fields.
In `@internal/providers/anthropic/anthropic_test.go`:
- Around line 1345-1380: The test
TestConvertToAnthropicRequest_RejectsUnsupportedChatExtras currently only checks
the error text; update it to assert the error shape too: after calling
convertToAnthropicRequest, ensure err is a *core.GatewayError, cast it (e.g. ge
:= err.(*core.GatewayError) or use errors.As), then assert ge.Code ==
"invalid_request_error" and ge.Status == 400 in addition to verifying the error
message contains tt.field; keep the existing message substring check as well so
the test matches the neighboring validation tests.
In `@internal/providers/bedrock/bedrock_test.go`:
- Around line 273-290: Update the test to cover precedence: add a case to
TestBuildConverseParts_TopPFromTypedField (or a new test in
internal/providers/bedrock/bedrock_test.go) where both the typed field req.TopP
and a conflicting value in req.ExtraFields (e.g., map with "top_p": 0.3) are
set, call buildConverseParts(req), and assert parts.infCfg.TopP equals the typed
value (0.8) not the ExtraFields value; reference buildConverseParts and
parts.infCfg.TopP/awssdk.ToFloat32 for locating assertions.
In `@internal/providers/responses_adapter.go`:
- Around line 106-108: validateResponsesToolsForChatTranslation blocks hosted
tool definitions but req.ToolChoice is not validated, allowing hosted
tool_choice objects to slip through; update
validateResponsesRequestForChatTranslation to also call
validateResponsesToolsForChatTranslation (or add a new validation step) on
req.ToolChoice (and ensure normalizeResponsesToolChoiceForChat does not return
hosted definitions unchanged) so any hosted tool_choice like
{"type":"file_search"} is rejected the same way as hosted entries in req.Tools;
apply the same fix for the equivalent logic path handled around
normalizeResponsesToolChoiceForChat (the other occurrence mentioned in the
comment).
---
Outside diff comments:
In `@internal/core/responses_json.go`:
- Around line 448-505: UnmarshalJSON for ResponsesInputElement must clear any
existing state before repopulating to avoid carrying over stale fields (Status,
CallID, Content, Raw, ExtraFields); after successfully decoding into raw (the
local variable map[string]json.RawMessage) reset the receiver with *e =
ResponsesInputElement{} and then proceed with the existing logic that reads raw
and assigns to e.Type, e.Name, e.CallID, e.Status, e.Arguments, e.Output,
e.Role, e.Content or sets e.Raw; this ensures ExtraFields and other fields are
not preserved across calls to ResponsesInputElement.UnmarshalJSON.
---
Duplicate comments:
In `@docs/openapi.json`:
- Around line 7913-7919: The "conversation" schema currently only references
core.ResponsesConversationRef and that ref does not require id, so clients
reject valid string IDs and may accept empty objects; update the schemas (e.g.,
the "conversation" property and the referenced core.ResponsesConversationRef) to
allow either a string ID or an object with a required id by changing the
"conversation" schema to a oneOf (or anyOf) that accepts { type: "string" } or
an object reference to core.ResponsesConversationRef, and modify
core.ResponsesConversationRef to require the id property (add "required": ["id"]
or ensure the object schema enforces id) so both forms (string or { id: "..." })
validate correctly; apply the same change to the other occurrences mentioned
(lines referencing the same "conversation" schema at 8086-8092, 8240-8246,
8296-8302).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 98038e4d-ea4b-43f3-90f5-ecd2d8721da6
📒 Files selected for processing (25)
cmd/gomodel/docs/docs.godocs/advanced/responses-api.mdxdocs/advanced/responses-compatibility.mdxdocs/dev/agents-sdk-support.mddocs/docs.jsondocs/examples/openai-agents-sdk/README.mddocs/examples/openai-agents-sdk/anthropic_agents_probe.pydocs/examples/openai-agents-sdk/anthropic_responses_probe.pydocs/guides/openai-agents-sdk.mdxdocs/openapi.jsoninternal/core/json_fields.gointernal/core/json_fields_test.gointernal/core/responses_json.gointernal/core/responses_json_test.gointernal/core/types.gointernal/providers/anthropic/anthropic_test.gointernal/providers/anthropic/request_translation.gointernal/providers/bedrock/bedrock_test.gointernal/providers/bedrock/chat.gointernal/providers/gemini/gemini_test.gointernal/providers/gemini/native.gointernal/providers/gemini/native_schema_test.gointernal/providers/openai/compatible_provider.gointernal/providers/responses_adapter.gointernal/providers/responses_adapter_test.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/providers/anthropic/request_translation.go (1)
360-367:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t reject noop chat extras just because the key is present.
validateAnthropicUnsupportedChatExtrascurrently fails any request whereresponse_formatorverbosityexists inExtraFields, including explicitnullvalues. That turns valid OpenAI-compatible no-op inputs like{"response_format":null}into 400s.response_format: {"type":"text"}is another default/no-op case that Anthropic can safely ignore rather than reject.Minimal fix
func validateAnthropicUnsupportedChatExtras(extra core.UnknownJSONFields) error { for _, field := range []string{"response_format", "verbosity"} { - if extra.Lookup(field) != nil { + raw := bytes.TrimSpace(extra.Lookup(field)) + if len(raw) == 0 || bytes.Equal(raw, []byte("null")) { + continue + } + if field == "response_format" { + var cfg struct { + Type string `json:"type"` + } + if err := json.Unmarshal(raw, &cfg); err == nil && (cfg.Type == "" || cfg.Type == "text") { + continue + } + } + if len(raw) != 0 { return core.NewInvalidRequestError("chat field "+field+" is not supported by Anthropic translation", nil) } } return nil }As per coding guidelines: Accept requests generously and adapt them to each provider's requirements; preserve the OpenAI-compatible public API when changing provider behavior.
internal/core/responses_json.go (1)
163-169:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't let
Rawoverride a laterConversation.IDupdate.Line 164 always re-emits
c.Rawwhen it exists, so decodingconversation, changingreq.Conversation.ID, and then marshaling still sends the old ID. That can forward the wrong conversation reference or persist stale state. Preserve only the original shape (string vs object), or rewrite the stored raw value whenIDchanges, before marshaling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/core/responses_json.go` around lines 163 - 169, ResponsesConversationRef.MarshalJSON currently always returns cloneRawMessage(c.Raw) when Raw is non-empty, which can re-emit a stale ID; change it to preserve the original shape but honor an updated c.ID: if c.Raw is empty behave as today (json.Marshal(c.ID) when ID != ""), otherwise detect whether c.Raw is a JSON string or object (e.g., trim and inspect first non-space char); if it was a string, return json.Marshal(c.ID) so the shape remains a string but with the new ID; if it was an object, unmarshal c.Raw into a map[string]any, set or replace the "id" field with c.ID and re-marshal that map; only fall back to cloneRawMessage(c.Raw) when c.ID is empty or when parsing fails, and keep using cloneRawMessage to preserve raw bytes when appropriate — update the function ResponsesConversationRef.MarshalJSON and reuse cloneRawMessage/json.Marshal accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/core/json_fields_test.go`:
- Around line 113-141: Add table-driven tests for MergeUnknownJSONFields to
cover its error paths: include cases where base.raw is malformed JSON (e.g.,
truncated/invalid bytes) expecting an error, where base.raw is valid JSON but
not an object (e.g., array or string) causing appendUnknownJSONMembers to fail,
and where one of the additions contains invalid JSON so MergeUnknownJSONFields
returns an error; reference the UnknownJSONFields struct for constructing base,
call MergeUnknownJSONFields with those inputs, and assert that an error is
returned (and optionally assert that the error message or type indicates a JSON
unmarshal/non-object failure) rather than a successful merge so these 400-paths
remain tested.
In `@internal/providers/responses_adapter_test.go`:
- Around line 471-488: Add table-driven test cases in the same test table in
responses_adapter_test.go that cover object-form aliases for ToolChoice: add
entries named like "tool_choice none alias", "tool_choice auto alias", and
"tool_choice required alias" each using req: &core.ResponsesRequest{Model:
"test-model", Input: "Hello", ToolChoice: map[string]any{"type":"none"}} (and
similarly for "auto" and "required") and set want to the normalized
chat-compatible string ("none", "auto", "required"). This ensures the code paths
in the ResponsesRequest -> response adapter that read ToolChoice (look for the
test harness and the core.ResponsesRequest usage in this table) continue to
accept object-form inputs and normalize them to the expected string values.
In `@internal/providers/responses_adapter.go`:
- Around line 125-136: The validator
validateResponsesToolChoiceForChatTranslation currently rejects map-shaped
tool_choice unless type == "function"; instead, accept and normalize
OpenAI-style aliases by allowing "none", "auto", "required" (and "function")
when choice is a map and treat them as valid (i.e., return nil) rather than
calling unsupportedResponsesChatTranslationTool; update the logic to read
choiceMap["type"], trim it, switch/compare against
"function","none","auto","required" and only call
unsupportedResponsesChatTranslationTool for truly unsupported values; apply the
same normalization change to the analogous validator referenced around lines
284-306.
---
Outside diff comments:
In `@internal/core/responses_json.go`:
- Around line 163-169: ResponsesConversationRef.MarshalJSON currently always
returns cloneRawMessage(c.Raw) when Raw is non-empty, which can re-emit a stale
ID; change it to preserve the original shape but honor an updated c.ID: if c.Raw
is empty behave as today (json.Marshal(c.ID) when ID != ""), otherwise detect
whether c.Raw is a JSON string or object (e.g., trim and inspect first non-space
char); if it was a string, return json.Marshal(c.ID) so the shape remains a
string but with the new ID; if it was an object, unmarshal c.Raw into a
map[string]any, set or replace the "id" field with c.ID and re-marshal that map;
only fall back to cloneRawMessage(c.Raw) when c.ID is empty or when parsing
fails, and keep using cloneRawMessage to preserve raw bytes when appropriate —
update the function ResponsesConversationRef.MarshalJSON and reuse
cloneRawMessage/json.Marshal accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e45845dd-1351-4f51-85cb-b55b5387a5ac
📒 Files selected for processing (11)
docs/guides/openai-agents-sdk.mdxinternal/core/json_fields.gointernal/core/json_fields_test.gointernal/core/responses_json.gointernal/core/responses_json_test.gointernal/providers/anthropic/anthropic_test.gointernal/providers/anthropic/request_translation.gointernal/providers/anthropic/types.gointernal/providers/bedrock/bedrock_test.gointernal/providers/responses_adapter.gointernal/providers/responses_adapter_test.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/core/json_fields.go`:
- Around line 84-86: Remove the redundant call to validateUnknownJSONObject on
additionFields.raw immediately after UnknownJSONFieldsFromMap builds it: since
UnknownJSONFieldsFromMap guarantees a well-formed JSON object (or returns empty)
and appendUnknownJSONMembers performs necessary validation later, delete the
validateUnknownJSONObject(additionFields.raw) check and its error path; keep
UnknownJSONFieldsFromMap, the UnknownJSONFields construction, and the later
validation in appendUnknownJSONMembers intact to avoid double-validation.
In `@internal/core/responses_json.go`:
- Around line 164-181: The current marshaling returns cloneRawMessage(trimmed)
when c.ID == "" which preserves an old ID in c.Raw and hides JSON corruption by
returning raw on unmarshal errors; update the logic so that you no longer
short-circuit on c.ID == "" — always inspect trimmed and handle each shape: for
'"' return json.Marshal(c.ID) (which will produce an empty string when ID is
cleared); for '{' attempt json.Unmarshal into obj and if c.ID == "" delete
obj["id"] otherwise set obj["id"]=c.ID, then json.Marshal(obj); and on
json.Unmarshal errors return the error (propagate it) instead of silently
returning cloneRawMessage(trimmed); use the existing symbols c.Raw, c.ID,
cloneRawMessage, json.Unmarshal/json.Marshal to locate and modify the code.
In `@internal/providers/responses_adapter.go`:
- Around line 293-299: The switch on choiceType (variable names choiceType and
choice) contains an empty case "function" that intentionally continues to the
normalization logic; make this explicit by adding a clear fallthrough comment
(e.g. "// fallthrough: allow 'function' to be handled by the default
normalization below") inside the case "function": block so future maintainers
understand the intent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8eea4024-8a25-43b2-88cf-9af11570a73f
📒 Files selected for processing (8)
internal/core/json_fields.gointernal/core/json_fields_test.gointernal/core/responses_json.gointernal/core/responses_json_test.gointernal/providers/anthropic/anthropic_test.gointernal/providers/anthropic/request_translation.gointernal/providers/responses_adapter.gointernal/providers/responses_adapter_test.go
Comment on lines
+84
to
+86
| if err := validateUnknownJSONObject(additionFields.raw); err != nil { | ||
| return UnknownJSONFields{}, err | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Remove redundant validation of additions.
Line 84 validates additionFields.raw immediately after UnknownJSONFieldsFromMap constructs it at line 83. Since UnknownJSONFieldsFromMap always builds a well-formed JSON object (or returns empty), this validation will never fail and is redundant with the validation performed later in appendUnknownJSONMembers at line 148.
♻️ Optional simplification
func MergeUnknownJSONFields(base UnknownJSONFields, additions map[string]json.RawMessage) (UnknownJSONFields, error) {
if len(additions) == 0 {
return base, nil
}
additionFields := UnknownJSONFieldsFromMap(additions)
- if err := validateUnknownJSONObject(additionFields.raw); err != nil {
- return UnknownJSONFields{}, err
- }
if base.IsEmpty() {
return additionFields, nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if err := validateUnknownJSONObject(additionFields.raw); err != nil { | |
| return UnknownJSONFields{}, err | |
| } | |
| func MergeUnknownJSONFields(base UnknownJSONFields, additions map[string]json.RawMessage) (UnknownJSONFields, error) { | |
| if len(additions) == 0 { | |
| return base, nil | |
| } | |
| additionFields := UnknownJSONFieldsFromMap(additions) | |
| if base.IsEmpty() { | |
| return additionFields, nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/core/json_fields.go` around lines 84 - 86, Remove the redundant call
to validateUnknownJSONObject on additionFields.raw immediately after
UnknownJSONFieldsFromMap builds it: since UnknownJSONFieldsFromMap guarantees a
well-formed JSON object (or returns empty) and appendUnknownJSONMembers performs
necessary validation later, delete the
validateUnknownJSONObject(additionFields.raw) check and its error path; keep
UnknownJSONFieldsFromMap, the UnknownJSONFields construction, and the later
validation in appendUnknownJSONMembers intact to avoid double-validation.
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
2 participants
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
store: falsefor local response snapshots and return clear errors for unsupported chat-translated Responses statedocs/examples/openai-agents-sdkTests
Summary by CodeRabbit
Release Notes
New Features
service_tier,user,top_p,store,conversation,previous_response_id,prompt, structured text output (text), and context management controls.Documentation