OpenAI support - #1178
OpenAI support#1178
Conversation
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
left a comment
There was a problem hiding this comment.
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
NewModelsignature.
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.
…n handling for OpenAI responses
…rehensive unit test coverage for OpenAI model converters
Adds a runnable example showing an ADK llmagent backed by the OpenAI model from #1178 (openaimodel.NewModel) instead of Gemini. The only difference from the Gemini quickstart is the model constructor; agents, the functiontool, the runner, and the launcher are unchanged. The sample registers a get_weather function tool to exercise OpenAI tool calling, defaults to gpt-4o-mini, and supports OpenAI-compatible endpoints via OPENAI_BASE_URL. A README follows the examples/workflow sample layout.
066b300 to
8093c15
Compare
|
One remaining gap in the structured-output path that I think is worth a look before this lands (the schema type-case handling you added in the last commit already cleared the earlier blocker; this is the next one behind it). Symptom: requesting structured output — either via Minimal repro: m, _ := openaimodel.NewModel(ctx, "gpt-4o-mini",
&openaimodel.ClientConfig{APIKey: os.Getenv("OPENAI_API_KEY")})
req := &model.LLMRequest{
Contents: []*genai.Content{{Role: genai.RoleUser, Parts: []*genai.Part{
{Text: "Paris is 22C. Extract city and temperature_c."}}}},
Config: &genai.GenerateContentConfig{
ResponseMIMEType: "application/json",
ResponseSchema: &genai.Schema{
Type: genai.TypeObject,
Properties: map[string]*genai.Schema{
"city": {Type: genai.TypeString},
"temperature_c": {Type: genai.TypeInteger},
},
Required: []string{"city", "temperature_c"},
},
},
}
for _, err := range m.GenerateContent(ctx, req, false) {
// err -> 400 ... 'additionalProperties' is required to be supplied and to be false
}This also reproduces through a normal agent: Root cause: the Responses API How adk-python handles it: its native OpenAI Responses integration does exactly this transform in Possible direction (your call): mirror that helper — recursively inject Scope / not affected:
Happy to share the exact schemas I tested if useful. Thanks again! |
|
Found a bug in streaming + tool calling that's worth fixing before this lands (it builds on your streaming refactor). Symptom: with an agent that has a function tool (e.g. the Root cause: in the OpenAI Responses streaming API, the function name is delivered on the
Non-streaming is unaffected (the name comes straight from the response output items), and plain-text streaming is unaffected (no function call). The broken combination is specifically streaming + tools. The Proposed fix (mirrors the existing type streamTranslator struct {
functionArgs map[string]*strings.Builder
itemToCallID map[string]string
itemToName map[string]string // add
}
// in newStreamTranslator():
itemToName: make(map[string]string),
// in the responseOutputItemAdded case, alongside the CallID capture:
if added.Item.ID != "" && added.Item.Name != "" {
t.itemToName[added.Item.ID] = added.Item.Name
}
// in emitFunctionCall(), before building the part:
name := done.Name
if name == "" {
name = t.itemToName[done.ItemID]
}
delete(t.itemToName, done.ItemID)
// ...then use `Name: name` in the returned genai.FunctionCallWith this applied, streaming tool calling works end-to-end — function call → tool executes → final text streams token-by-token: Might be worth a unit test with a fake stream where the |
…s in done events and cleanup unused schema test code.
* Add support for the OpenAPI models 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 :) * Run go mod tidy * refactor: rename package to openaimodel, introduce ClientConfig, and propagate response metadata in streams * refactor: add support for logprobs conversion and update finish reason handling for OpenAI responses * refactor: replace raw error strings with sentinel errors and add comprehensive unit test coverage for OpenAI model converters * docs(examples/openai): add OpenAI model integration sample Adds a runnable example showing an ADK llmagent backed by the OpenAI model from google#1178 (openaimodel.NewModel) instead of Gemini. The only difference from the Gemini quickstart is the model constructor; agents, the functiontool, the runner, and the launcher are unchanged. The sample registers a get_weather function tool to exercise OpenAI tool calling, defaults to gpt-4o-mini, and supports OpenAI-compatible endpoints via OPENAI_BASE_URL. A README follows the examples/workflow sample layout. * refactor: move openai model package to openaimodel and reorganize internal structure * feat: enable strict mode for OpenAI structured outputs by enforcing schema requirements * docs: mark openaimodel package as experimental * fix: track function names in stream translator to handle missing names in done events and cleanup unused schema test code. ---------
* Add support for the OpenAPI models 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 :) * Run go mod tidy * refactor: rename package to openaimodel, introduce ClientConfig, and propagate response metadata in streams * refactor: add support for logprobs conversion and update finish reason handling for OpenAI responses * refactor: replace raw error strings with sentinel errors and add comprehensive unit test coverage for OpenAI model converters * docs(examples/openai): add OpenAI model integration sample Adds a runnable example showing an ADK llmagent backed by the OpenAI model from google#1178 (openaimodel.NewModel) instead of Gemini. The only difference from the Gemini quickstart is the model constructor; agents, the functiontool, the runner, and the launcher are unchanged. The sample registers a get_weather function tool to exercise OpenAI tool calling, defaults to gpt-4o-mini, and supports OpenAI-compatible endpoints via OPENAI_BASE_URL. A README follows the examples/workflow sample layout. * refactor: move openai model package to openaimodel and reorganize internal structure * feat: enable strict mode for OpenAI structured outputs by enforcing schema requirements * docs: mark openaimodel package as experimental * fix: track function names in stream translator to handle missing names in done events and cleanup unused schema test code. --------- Co-authored-by: Levi Gross <levi@levigross.com> Co-authored-by: Levi Gross <levigross@users.noreply.github.com> Co-authored-by: wolo <wolo@google.com>
This PR duplicates #242 to apply required fixes keeping full ownership and clean history as there's no movement in the original one.