OpenAI support by hanorik · Pull Request #1178 · google/adk-go

hanorik · 2026-07-20T15:33:37Z

This PR duplicates #242 to apply required fixes keeping full ownership and clean history as there's no movement in the original one.

This PR enables basic support for OpenAI models (and endpoints that expose a OpenAI API compatible API interface). I am stressing on *basic* support because we will leave tool calling to the next PR :)

…propagate response metadata in streams

karolpiotrowicz

Thanks for picking this up and carrying #242 forward with a clean history — the package structure and the genai↔OpenAI conversion layering are in good shape, and it's close to the adk-python labs/openai implementation.

I built the package locally and verified behavior against the openai-go/v3 and google.golang.org/genai types directly. A heads-up worth calling out up front: CI is green but the tests are fully offline (inline httptest mocks that don't validate the outgoing request), so several real issues aren't exercised by the suite. I found a few blocking correctness bugs in exactly those un-exercised paths:

Tool/structured-output JSON schemas are emitted with uppercase types ("OBJECT", "STRING"), which OpenAI rejects.
Function tools are always sent strict: true, which 400s on typical genai schemas.
Streaming function calls return the item id instead of the call_id, so streamed tool-call round-trips are inconsistent with the non-streaming path.
The streaming iterator can panic (yields from a deferred Close()), and streamed responses drop token usage.
The package doc example doesn't compile against the current NewModel signature.

Details in the inline comments below (from blockers to nits). Most have small, self-contained fixes, and adk-python is a handy parity reference for a couple of them. Happy to help with any of these. Since it's still a draft, marking as request-changes for tracking rather than as a gate.

karolpiotrowicz · 2026-07-21T09:53:12Z

+	}
+}
+
+func schemaToMap(schema *genai.Schema) (map[string]any, error) {


BLOCKER. JSON Schema types are emitted in uppercase here. genai.Schema serializes its Type field using the genai enum values, which are uppercase (genai.TypeObject == "OBJECT", genai.TypeString == "STRING"), and schemaToMap marshals the schema through verbatim. OpenAI (and JSON Schema generally) expect lowercase types ("object", "string"), so any real tool declaration or structured-output schema built with the genai constants will be rejected with a 400.

This isn't caught by the tests because TestBuildOpenAIParams_JSONSchema hand-writes Type: "object" as a lowercase literal rather than using genai.TypeObject.

Suggested fix: recursively lowercase type (handling both the scalar and array-of-types cases) on the map returned by schemaToMap — applied to both tool parameters and the structured-output path (newJSONSchemaFormat). adk-python does exactly this in _openai_responses_llm.py (value['type'] = schema_type.lower()).

karolpiotrowicz · 2026-07-21T09:53:13Z

+		Name:       fn.Name,
+		Type:       constant.Function("function"),
+		Parameters: paramsMap,
+		Strict:     param.NewOpt(true),


BLOCKER. Strict: param.NewOpt(true) is forced on every function tool. OpenAI strict function calling requires the schema to set additionalProperties: false and list all properties in required. genai-derived schemas won't satisfy that, so strict mode will 400 for typical tools (this is independent of the uppercase-type issue).

Suggested fix: don't hard-code strict. Either omit it (default non-strict), make it configurable, or transform the schema to satisfy strict requirements before enabling it. For reference, adk-python sends strict=False for function tools and only enforces strict for structured output.

karolpiotrowicz · 2026-07-21T09:53:13Z

+//
+//	ctx := context.Background()
+//	client := openai.NewClient(option.WithAPIKey(os.Getenv("OPENAI_API_KEY")))
+//	llm, err := openaimodel.NewModel(ctx, openai.ChatModelGPT4oMini, client)


BLOCKER. The package doc example won't compile against the current API:

llm, err := openaimodel.NewModel(ctx, openai.ChatModelGPT4oMini, client)

NewModel's third parameter is *ClientConfig (see openai.go:50), not an *openai.Client. The example also uses option without importing it. Related: the first doc line says // Package openai, but the package is openaimodel.

Suggested fix:

cfg := &openaimodel.ClientConfig{APIKey: os.Getenv("OPENAI_API_KEY")} llm, err := openaimodel.NewModel(ctx, openai.ChatModelGPT4oMini, cfg)

and update the // Package openaimodel ... sentence to match the package name.

karolpiotrowicz · 2026-07-21T09:53:13Z

+	return &genai.Part{
+		FunctionCall: &genai.FunctionCall{
+			Name: done.Name,
+			ID:   done.ItemID,


MAJOR. In streaming, the function-call ID is set from done.ItemID, but ResponseFunctionCallArgumentsDoneEvent only carries the item id (fc_...), not the model's call_id (call_...). The non-streaming path (response.go:121) correctly uses item.CallID, so the two paths return different kinds of IDs for the same concept. When a tool result is sent back on the next turn, OpenAI expects the original call_id; the item id won't match its server-side record.

The real call_id is available on the response.output_item.added event, which the translator currently ignores (stream.go:94). Suggested fix: capture the item_id -> call_id mapping from output_item.added and use it in emitFunctionCall. adk-python resolves the call_id from the output-item events the same way.

karolpiotrowicz · 2026-07-21T09:53:13Z

+			yield(nil, ErrStreamingUnavailable)
+			return
+		}
+		defer func() {


MAJOR. Calling yield from this deferred stream.Close() can violate Go's range-over-func protocol. If the consumer breaks early, yield has already returned false; the deferred Close() erroring then calls yield again, which panics with "range function continued iteration after loop body returned false." On a successful stream where Close() returns an error, it also yields a spurious trailing (nil, error), making a good response look failed. (ssestream.Stream.Close() can return a non-nil error, e.g. on the underlying body close.)

Suggested fix: don't yield from the defer — defer func() { _ = stream.Close() }(). If the close error must surface, capture it and yield once at the single normal exit, guarded by a stopped/active bool so it's never yielded after the consumer stopped.

karolpiotrowicz · 2026-07-21T09:53:13Z

+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package openaimodel


MINOR. The directory is model/openai but the package is openaimodel, which diverges from the sibling convention (model/gemini -> gemini, model/apigee -> apigee) and means importers can't refer to it as openai. This looks deliberate to avoid colliding with the SDK's package openai (imported in the same files), which is reasonable — just flagging it so it's an intentional, documented choice. One option is to rename the directory to model/openaimodel so dir and package match.

karolpiotrowicz · 2026-07-21T09:53:13Z

+func (m *openAIModel) generateStream(ctx context.Context, params responses.ResponseNewParams) iter.Seq2[*model.LLMResponse, error] {
+	return func(yield func(*model.LLMResponse, error) bool) {
+		stream := m.client.Responses.NewStreaming(ctx, params)
+		if stream == nil {


MINOR. if stream == nil { ... ErrStreamingUnavailable } is unreachable — NewStreaming returns a non-nil *ssestream.Stream even on init failure (the error surfaces via stream.Err(), which is already checked just below). Safe to remove along with the now-unused ErrStreamingUnavailable.

karolpiotrowicz · 2026-07-21T09:53:13Z

+
+func convertUsage(usage responses.ResponseUsage) *genai.GenerateContentResponseUsageMetadata {
+	return &genai.GenerateContentResponseUsageMetadata{
+		PromptTokenCount:        int32(usage.InputTokens),


NIT. Token counts are narrowed int64 -> int32. Practically fine, but a large count would wrap silently; consider guarding or widening if the target field allows.

karolpiotrowicz · 2026-07-21T09:53:13Z

+	name   string
+}
+
+func NewModel(_ context.Context, modelName string, cfg *ClientConfig) (model.LLM, error) {


NIT. NewModel takes ctx but discards it (_ context.Context), whereas gemini.NewModel uses it. Minor consistency point — either use it for client construction or keep the parameter purely for signature parity (a short comment would help).

karolpiotrowicz · 2026-07-21T09:53:13Z

+		Contents: []*genai.Content{genai.NewContentFromText("respond JSON", genai.RoleUser)},
+		Config: &genai.GenerateContentConfig{
+			ResponseMIMEType: "application/json",
+			ResponseSchema: &genai.Schema{


Test coverage. A few gaps worth closing, since they're why the issues above pass CI:

TestBuildOpenAIParams_JSONSchema uses lowercase literals (Type: "object"); switching to genai.TypeObject/genai.TypeString would surface the uppercase-type bug and assert the output is lowercased.

No test covers a streaming tool-call round-trip (call -> result -> next turn), which would catch the call_id vs item-id issue.

No tests for the error/refusal mapping or the unsupported-config rejections.

Per AGENTS.md, model packages record traffic via testdata/*.httprr; these use inline httptest mocks instead. Coverage is ~56%.

…n handling for OpenAI responses

levigross and others added 4 commits November 11, 2025 09:35

Add support for the OpenAPI models

f942078

This PR enables basic support for OpenAI models (and endpoints that expose a OpenAI API compatible API interface). I am stressing on *basic* support because we will leave tool calling to the next PR :)

Merge branch 'main' into main

b9c6ee4

Run go mod tidy

2846ff8

Merge remote-tracking branch 'upstream/main' into openai_support

8ddc5fb

hanorik force-pushed the openai_support branch from 11a396e to c809bb8 Compare July 20, 2026 17:05

refactor: rename package to openaimodel, introduce ClientConfig, and …

290e9aa

…propagate response metadata in streams

hanorik force-pushed the openai_support branch from c809bb8 to 290e9aa Compare July 20, 2026 17:21

karolpiotrowicz requested changes Jul 21, 2026

View reviewed changes

refactor: add support for logprobs conversion and update finish reaso…

8c3cbff

…n handling for OpenAI responses

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

OpenAI support#1178

OpenAI support#1178
hanorik wants to merge 6 commits into
mainfrom
openai_support

hanorik commented Jul 20, 2026

Uh oh!

karolpiotrowicz left a comment

Uh oh!

karolpiotrowicz Jul 21, 2026

Uh oh!

karolpiotrowicz Jul 21, 2026

Uh oh!

karolpiotrowicz Jul 21, 2026

Uh oh!

karolpiotrowicz Jul 21, 2026

Uh oh!

karolpiotrowicz Jul 21, 2026

Uh oh!

karolpiotrowicz Jul 21, 2026

Uh oh!

karolpiotrowicz Jul 21, 2026

Uh oh!

karolpiotrowicz Jul 21, 2026

Uh oh!

karolpiotrowicz Jul 21, 2026

Uh oh!

karolpiotrowicz Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

hanorik commented Jul 20, 2026

Uh oh!

karolpiotrowicz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants