refactor(go/plugins/googlegenai): migrate to the typed-config constructors - #5869
refactor(go/plugins/googlegenai): migrate to the typed-config constructors#5869apascal07 wants to merge 8 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the googlegenai plugin to leverage Genkit's typed models, embedders, and background models, delegating request configuration validation and deserialization to the framework. This simplifies model initialization and removes redundant manual configuration conversion logic. The review feedback focuses on performance optimization, recommending that large configuration structs (such as genai.GenerateImagesConfig and genai.GenerateContentConfig) be passed by pointer rather than by value to avoid unnecessary copying.
| if mt == ModelTypeImagen { | ||
| return ai.NewTypedModel(api.NewName(provider, name), &opts, | ||
| func(ctx context.Context, input *ai.ModelRequest, config genai.GenerateImagesConfig, cb ai.ModelStreamCallback) (*ai.ModelResponse, error) { | ||
| return generateImage(ctx, client, name, input, config, cb) |
| } | ||
|
|
||
| gcc, err := toGeminiRequest(input, cache, model) | ||
| gcc, err := toGeminiRequest(input, config, cache, model) |
| func toGeminiRequest(input *ai.ModelRequest, config genai.GenerateContentConfig, cache *genai.CachedContent, modelName ...string) (*genai.GenerateContentConfig, error) { | ||
| gcc := &config |
There was a problem hiding this comment.
Change the signature of toGeminiRequest to accept config as a pointer *genai.GenerateContentConfig instead of a value. This avoids copying the large configuration struct on every request.
| func toGeminiRequest(input *ai.ModelRequest, config genai.GenerateContentConfig, cache *genai.CachedContent, modelName ...string) (*genai.GenerateContentConfig, error) { | |
| gcc := &config | |
| func toGeminiRequest(input *ai.ModelRequest, config *genai.GenerateContentConfig, cache *genai.CachedContent, modelName ...string) (*genai.GenerateContentConfig, error) { | |
| gcc := config |
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return toGeminiRequest(input, config, cache, modelName...) |
| client *genai.Client, | ||
| model string, | ||
| input *ai.ModelRequest, | ||
| gic genai.GenerateImagesConfig, |
| } | ||
|
|
||
| resp, err := client.Models.GenerateImages(ctx, model, userPrompt, gic) | ||
| resp, err := client.Models.GenerateImages(ctx, model, userPrompt, &gic) |
…as once Reflecting the genai SDK config structs is expensive, and every model of a modality advertises the same read-only schema, so the three schemas move to package-level values shared by the default options and by GetModelOptions instead of being rebuilt per model. ModelType gains an unexported configSchema accessor that resolves unrecognized names to the Gemini schema, since those speak generateContent too; the exported DefaultConfig is unchanged.
…validation The curated schema hides fields the plugin manages (systemInstruction, cachedContent, responseSchema, responseMimeType, responseJsonSchema, tools[].functionDeclarations, candidateCount) so the dev UI does not offer them. That is a presentation choice, but once the typed constructors wire the config schema into the request input schema it is enforced on every call, and a hidden field would be rejected as an unknown property before the plugin could answer with the primitive to use instead. candidateCount would fare worse: setting it to 1 is legal today and would start failing. Objects that lost a property are now reopened with additionalProperties: true. Everything else stays strict, so mistyped values and typos in untouched objects are still caught, and the Imagen and Veo schemas, which hide nothing, are unaffected.
NewTypedEmbedder hands the embedder function a genai.EmbedContentConfig the framework deserialized from the request, replacing a pointer type assertion that silently dropped anything else: a map, which is what the dev UI and every other JSON caller sends, produced an embed call with no config at all. The explicit ConfigSchema is gone too, since inference over the same type produces it. newEmbedder returns the concrete *ai.EmbedderAction, so the caller no longer asserts its way back to api.Action.
NewTypedBackgroundModel hands the start function a genai.GenerateVideosConfig, replacing toVeoParameters, whose type switch duplicated what the framework now does. It also copies: the old pointer case handed the user's own config to the SDK and then wrote the NumberOfVideos default into it. The flat TypedBackgroundModelOptions carries the model descriptor, so the video config schema is advertised the same way as before, with a fallback for callers that do not supply one. The test that covered the deleted conversion now pins the config type the model is defined with.
… and imagen models NewTypedModel infers the config type from the model function, so newModel picks the type per modality: image models take a genai.GenerateImagesConfig, everything else speaks generateContent and takes a genai.GenerateContentConfig. That deletes configFromRequest and imagenConfigFromRequest, whose type switches are now the framework's job, and it pins the advertised schema to the type the function receives, so the two can no longer drift. The config travels by value from there down: toGeminiRequest amends the request's own copy rather than a pointer whose provenance the reader has to trace, and the media-download middleware wraps a closure over that copy so the built-in chain order is unchanged. The tests that covered the deleted conversions now go through the framework's deserialization before calling toGeminiRequest, which is what a real request does.
…ewActionOf The registry-free core.NewAction is deprecated in favor of NewActionOf, which leads with the action type and takes an options struct, so the check action's description becomes a field instead of a metadata entry.
…rtise The typed constructors turn the advertised config schema into something enforced on every request, so these tests check both directions at the action boundary: the configs callers send today are accepted, including the fields hidden from the dev UI, and configs meant for another modality are not. They also cover the embedder's options slot, which used to ignore anything that was not a pointer.
… plugin's errors Walks the whole path for each field hidden from the dev UI: past input validation, through the framework's deserialization, into the check that names the primitive to use instead. Without this the two halves are pinned separately and nothing catches a schema change that turns a targeted error into a schema violation.
ce13bac to
08a0657
Compare
Migrates the googlegenai plugin (Google AI and Vertex AI) to the typed-config constructors from #5849, the third PR of the train. Every primitive the plugin defines now declares its config type and receives it deserialized, so the four hand-rolled config mappers are gone:
configFromRequest,imagenConfigFromRequest,toVeoParameters, and the embedder's pointer type assertion.Before: each modality re-implements the same type switch.
After: the type parameter is inferred from the function and the framework fills it.
genai.GenerateImagesConfig, Veo takes agenai.GenerateVideosConfig, the embedder takes agenai.EmbedContentConfig, and everything else (Gemini, Gemma, TTS, tuned endpoints, unrecognized names) speaks generateContent and takes agenai.GenerateContentConfig.toGeminiRequestamends the request's own copy. The old pointer path let a Veo request write itsNumberOfVideosdefault into the caller's config struct.newModel,newVeoModel, andnewEmbedderreturn the concrete*ai.ModelAction,*ai.BackgroundModelAction, and*ai.EmbedderAction, solistActionsandResolveActionno longer assert their way back toapi.Action. The exportedDefineModel/DefineEmbeddermethods keep their interface returns.core.NewActiontocore.NewActionOf, which turns the check action's description into a field instead of a metadata entry.Config schemas are now enforced
The plugin supplies
ConfigSchemaexplicitly (the SDK structs need a custom reflector to avoid recursion panics, plus curated descriptions for the dev UI), and the typed constructors wire that schema into the request input schema, which is validated on every call. Two consequences:The curated schema hides fields the plugin manages, and hiding is a presentation choice, not a narrowing of what callers may send. Left alone,
candidateCount: 1would start failing even though the plugin pins it to 1 itself, andsystemInstruction,cachedContent,responseSchema,responseMimeType,responseJsonSchema, andtools[].functionDeclarationswould be rejected as unknown properties instead of reaching the plugin's own check, which names the primitive to use instead (ai.WithSystemPrompt,ai.WithCacheTTL,ai.WithOutputType,ai.WithTools). So an object that loses a property is reopened withadditionalProperties: true. Objects that kept every field stay strict, and the Imagen and Veo schemas, which hide nothing, are untouched.The three modality schemas are now built once at package level instead of being reflected per model.
GetModelOptionswas doing that work on every call, whichlistActionsmakes once per model.Fixes
Embedder options were read with a
*genai.EmbedContentConfigtype assertion, so amap[string]any(what the dev UI and every other JSON caller sends) was silently dropped and the embed call went out with no config at all. The framework now deserializes it.Verification
Go 1.26.3,
go test ./plugins/googlegenai/...: passes, 56 top-level tests and 222 including subtests, 2 skipped (they need live credentials), no failures.New coverage: the config schema each action advertises is pinned per modality, every field the curated schemas hide still reaches the plugin's own error naming the Genkit primitive to use, and a
map[string]anyconfig reaches the embedder deserialized.