feat(go): relax option merging to accumulate or last-win - #5875
Open
apascal07 wants to merge 1 commit into
Open
Conversation
Every option in ai/option.go returned an error when it was set more than once or combined with a sibling variant (WithMessages with WithMessagesFn, WithModel with WithModelName, ...). That made options impossible to compose: a caller could not build a request up from several helpers, and GenerateData could not inject its output type without conflicting with a caller-supplied WithOutputSchema. Options now follow the standard Go functional-options pattern, merging left to right under two rules: - Options carrying multiple items accumulate across repeats and across their variants: WithMessages/WithMessagesFn, WithTools, WithResources, WithUse, WithMiddleware, WithDocs/WithTextDocs, WithDataset, WithToolResponses, and WithToolRestarts. - Options filling a single slot take the last one set: WithConfig, WithModel/WithModelName, WithSystem/WithPrompt, the output schema and format options, WithStreaming, WithInput*, WithStrictSchema, and the tool restart/respond options. Applying an option can therefore no longer fail, so the unexported apply* methods drop their error return; only genuinely invalid arguments (a type WithInputType cannot turn into a schema) still panic at the call site. GenerateData prepends its inferred WithOutputType so an explicit WithOutputSchema or WithOutputSchemaName wins the schema slot while typed output extraction keeps working, matching what GenerateDataStream and DefineDataPrompt already did. Message accumulation composes the MessagesFn closures and copies into a fresh slice rather than appending in place, since WithMessages(history...) hands the caller's slice straight through and appending onto it could scribble over the array backing their history. This is not a breaking change. The apply* methods are unexported and take unexported parameter types, so the option interfaces were never implementable outside package ai, and every option list that is valid today produces the same request afterward: with a slot set at most once, both merge rules reduce to that one value. Only calls that error today behave differently.
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the functional options pattern across the Go SDK to support option composition. Instead of returning errors when options are set multiple times, single-value options now follow a 'last-wins' approach, while collection-based options (such as messages, tools, documents, resources, and middleware) accumulate. Additionally, GenerateData has been updated to prepend the inferred output type option, allowing caller-supplied schemas to take precedence. Comprehensive unit tests have been added and updated to verify these new behaviors. No review comments were provided, so there is no additional feedback.
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.
Every option in
ai/option.goreturned an error when it was set more than once, or combined with a sibling variant (WithMessageswithWithMessagesFn,WithModelwithWithModelName, and so on). That made options impossible to compose: a caller could not build a request up from several helpers, andGenerateDatacould not inject its output type without conflicting with a caller-suppliedWithOutputSchema.Options now follow the standard Go functional-options pattern, merging left to right under two rules, and applying them never fails.
Options carrying multiple items accumulate across repeats and across their variants:
WithMessages/WithMessagesFn,WithTools,WithResources,WithUse,WithMiddleware,WithDocs/WithTextDocs,WithDataset,WithToolResponses,WithToolRestarts.Options filling a single slot take the last one set:
WithConfig,WithModel/WithModelName,WithSystem/WithPrompt, the output schema and format options,WithStreaming,WithInput*,WithStrictSchema, and the tool restart/respond options.Only genuinely invalid arguments still fail, and they panic at the call site (a type
WithInputTypecannot turn into a schema). The unexportedapply*methods drop theirerrorreturn accordingly.The two rules are documented in
go/README.mdbeside the generate examples, since composability is the reason to reach for them and nothing else in the docs said repeats were allowed.Not a breaking change
Compile-time. Every
apply*method is unexported and takes unexported parameter types, soai.GenerateOptionand its siblings were only ever implementable insidepackage ai. No consumer can have implemented them. Every exported constructor keeps its signature. This was verified by building a package outsidegenkit/gothat holds each option interface, constructs each one, directly constructs the two exported option structs (ai.RestartOptions,ai.RespondOptions), and embeds one in a user type.Runtime. The change is a relaxation, not a redefinition. "Valid today" means each slot was set at most once, and with exactly one setter both merge rules reduce to that same one value. The only inputs whose behavior changes are the ones that return an error today.
Two deltas worth naming:
GenerateDatanow prepends its inferredWithOutputTypeinstead of appending it, so a caller-suppliedWithOutputSchemawins the schema slot while typed extraction keeps working. This matches whatGenerateDataStreamandDefineDataPromptalready did. It changes one currently-valid combination:GenerateData[T](..., ai.WithOutputFormat("text"))previously ended up as JSON because the injected option applied last; the caller's format now wins.WithTools(a), WithTools(a)now accumulates and trips the existing duplicate check, so the message moves fromcannot set tools more than oncetoduplicate tool "a".Examples
Compose a request from several helpers:
Override a base configuration by appending:
Keep typed extraction while supplying your own schema:
Notes
Message accumulation composes the
MessagesFnclosures and copies into a fresh slice rather than appending in place.ai.WithMessages(history...)hands the caller's slice straight through, so appending onto it could scribble over the array backing their history. There is a regression test for this.Options outside
go/aiare unchanged and still reject repeats:genkit.Initoptions (WithPlugins,WithDefaultModel,WithPromptDir),ai/expagent options, and the firebase/localstore plugin options. Whether those should follow, particularly whetherWithPluginsshould accumulate, is a separate design question.Testing
Go 1.26.3:
go build ./...andgo vet ./...clean across the module, andgo test ./...passes all 39 packages with tests.go test -race ./ai/passes.New coverage: collection options accumulate (messages across both variants, tools, docs, resources, middleware, tool responses/restarts, dataset), single-value options take last-win, the caller's slice is not aliased by message accumulation, and
GenerateDatareaches the model with a caller-supplied schema while still extracting into the typed output.