Skip to content

refactor(go/plugins/anthropic): migrate to the typed-config constructors - #5874

Open
apascal07 wants to merge 4 commits into
ap/go-googlegenai-typed-configfrom
ap/go-anthropic-typed-config
Open

refactor(go/plugins/anthropic): migrate to the typed-config constructors#5874
apascal07 wants to merge 4 commits into
ap/go-googlegenai-typed-configfrom
ap/go-anthropic-typed-config

Conversation

@apascal07

@apascal07 apascal07 commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Migrates the Anthropic plugin and the Vertex AI Model Garden Claude models to the typed-config constructors from #5849, the fourth PR of the train. Both surfaces are backed by the same plugins/internal/anthropic package, so they move together and the hand-rolled configFromRequest type switch is gone.

Before: the plugin re-implements what the framework now does.

func configFromRequest(input *ai.ModelRequest) (*anthropic.MessageNewParams, error) {
    switch config := input.Config.(type) {
    case anthropic.MessageNewParams:  return &config, nil
    case *anthropic.MessageNewParams: return config, nil
    case map[string]any:              return base.MapToStruct[anthropic.MessageNewParams](config)
    case nil:                         return &anthropic.MessageNewParams{}, nil
    default:                          return nil, fmt.Errorf("unexpected config type: %T", input.Config)
    }
}
ai.NewModel(name, meta, fn)

After: the type parameter is inferred from the function and the framework fills it.

ai.NewTypedModel(api.NewName(provider, name), &opts,
    func(ctx context.Context, input *ai.ModelRequest, config anthropic.MessageNewParams, cb ai.ModelStreamCallback) (*ai.ModelResponse, error) {
        return Generate(ctx, client, provider, apiModel, input, config, cb)
    })
  • The config travels by value from the model function down, so toAnthropicRequest amends the request's own copy of MessageNewParams rather than the caller's struct.
  • The config schema is reflected once at package level instead of per model. newModel was reflecting the whole SDK params struct on every call, which ListActions makes once per discovered model.
  • Requests that carry a config type the model was not defined with now fail with the framework's error instead of the plugin's unexpected config type: %T.

One constructor for both plugins

ant.DefineModel and the Anthropic plugin's own newModel built the same action from the same options with the same generate closure, so the two collapse into ant.NewModel. The old name registered nothing despite the Define prefix, which is what #5849 removed elsewhere in the framework.

It returns the concrete *ai.ModelAction, so ListActions, ResolveAction, and the Model Garden Init no longer assert their way back to api.Action. Two options fields that the old shared constructor dropped on the floor, Stage and Metadata, now reach the descriptor because the options struct is passed through whole rather than copied field by field.

Labels are the caller's to set: ant.NewModel fills one in only when the caller leaves it empty, deriving "Anthropic - <name>" or "Vertex AI - <name>" from the provider. The old constructor overwrote whatever it was given with "<Provider>-<model id>", so the curated labels in AnthropicModels never reached the dev UI. Model Garden Claude models now show Claude Opus 4.7 rather than Vertex AI-claude-opus-4-7, matching how the Llama models in the same plugin present their labels.

Two bugs found while moving the config by value

A shared config had its tool list scribbled over by concurrent requests. toAnthropicRequest appends the Genkit tools onto the config's Tools rather than assigning, so that server-side tools (web search, code execution) set through the config survive. The config it now receives is a shallow copy, so that slice header still points at the caller's backing array, and a config hoisted into a package-level var or a ModelRef is shared by every request made with it. The append wrote into that array's spare capacity, so two concurrent requests raced over the same slots and one request's tools surfaced in another's. Clipping before the append forces an allocation. The test that claimed to pin this only compared lengths, which an in-place append leaves untouched while overwriting the slots past them; it now asserts that the result does not share the caller's array and that the caller's spare capacity is still empty, and it fails on the old append.

A nil ModelOptions panicked DefineModel. The pointer was dereferenced unguarded. A nil now takes the capabilities the plugin already resolves by name, curated for a known model and the Claude defaults for the rest, rather than an empty ModelOptions that would advertise a model supporting neither tools nor multiturn nor a system role.

Behavior notes

The config schema the plugin supplies is unchanged, so what requests are accepted is unchanged with it. The schema is explicit rather than inferred because the SDK wraps optional primitives in param.Opt[T], which reflects as an object but marshals as a bare value; the custom reflector maps those back to their underlying types, and the tests pin that everything the schema accepts deserializes into MessageNewParams.

Verification

Go 1.26.3, go test ./plugins/anthropic/... ./plugins/internal/anthropic/... ./plugins/vertexai/modelgarden/...: 3 packages pass, 28 top-level tests and 88 including subtests, no failures.

New coverage: the config schema reaches the request input schema so the framework rejects a config the SDK type cannot hold, everything the schema advertises deserializes into MessageNewParams, the curated labels survive to the descriptor for both providers, the tool append leaves the caller's array untouched, and DefineModel with a nil opts resolves the curated capabilities instead of panicking.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the Anthropic plugin to leverage Genkit's typed model support, delegating configuration validation and deserialization to the framework and simplifying the codebase. Feedback highlights a potential nil pointer dereference in DefineModel when dereferencing the opts parameter without a prior nil check.

Comment thread go/plugins/anthropic/anthropic.go
…odel

Migrates the Anthropic plugin and the Vertex AI Model Garden Claude models
to the typed-config constructors: the model function receives an
anthropic.MessageNewParams the framework has already validated and
deserialized, so the hand-rolled configFromRequest type switch is gone.

Both plugins built their own model action with the same options and the
same generate closure, so they now share one constructor. It replaces
ant.DefineModel, which registered nothing despite its name, returns the
concrete *ai.ModelAction so the callers stop asserting their way back to
api.Action, and honors the caller's label instead of overwriting it with
a provider-prefixed model ID.

The config schema is reflected once at package level rather than per
model, which ListActions builds once per discovered model.
The config schema a model advertises is now enforced on every request, so
what it accepts has to match what anthropic.MessageNewParams can hold:
every config form the action boundary accepts is checked to deserialize
into the SDK type, including the wrapper types the SDK uses for optional
primitives and the thinking union. The forms it rejects (unknown fields,
camelCase spellings of snake_case wire names, mistyped values) are pinned
too, since the model function never sees them.

Labels move with the constructor: curated ones are honored as given and
the rest are derived from the provider and the model name.
… opts

DefineModel dereferenced its ModelOptions pointer unguarded, so a nil
panicked. A nil now takes the capabilities the plugin already resolves by
name, curated for a known model and the Claude defaults for the rest,
rather than an empty ModelOptions that would advertise a model supporting
neither tools nor multiturn nor a system role.
…er's array

The request's config is a shallow copy, so its Tools slice header still
points at the caller's backing array. Appending the request's tools in
place writes into that array's spare capacity, so a config hoisted into a
package-level var or a ModelRef, which every request made with it shares,
has concurrent requests writing the same slot and reading each other's
tools. Clipping before the append forces an allocation.

The test that claimed to pin this only compared lengths, which an in-place
append leaves untouched while overwriting the slots past them. It now
checks that the result does not share the caller's array and that the
caller's spare capacity is still empty; it fails on the old append.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant