[Go] adding metadata to models, etc. #71
Conversation
| // NewAction creates a new Action with the given name and non-streaming function. | ||
| func NewAction[I, O any](name string, fn func(context.Context, I) (O, error)) *Action[I, O, struct{}] { | ||
| return NewStreamingAction(name, func(ctx context.Context, in I, cb NoStream) (O, error) { | ||
| func NewAction[I, O any](name string, metadata map[string]any, fn func(context.Context, I) (O, error)) *Action[I, O, struct{}] { |
There was a problem hiding this comment.
How do you see metadata being used? Who is the producer and who is the consumer? Typically in Go we would use specific types here (and make the metadata parameter type any), but if this is truly general, or user-controlled, than a map makes sense.
There was a problem hiding this comment.
At the action level metadata is not strictly defined, it is an arbitrary map. But at the specific action type level (model, embedder, retriever, etc.) metadata is expected to have specific structure (if it wants to be understood). This is an example of model metadata:
It is important because, for example, the model playground needs to know whether the model supports multiturn input (if it does then it will render a chat style interface) or whether it support multimodal input (it will allow inserting images).
but it's not just the Dev UI, for example the generate function will check model supported features and throw errors if the developer is trying to do something that's not supported by that specific model:
Line 547 in 9445f92
There was a problem hiding this comment.
BTW, for the generator I defined structs: https://github.com/firebase/genkit/pull/71/files#diff-c8c738aff67a5f8e34971b78f75941a99020e41e6ac1c535bef6e83b4c998278
There was a problem hiding this comment.
That suggests that the metadata parameter to NewAction should be type any, and the actual action will type assert to the desired value.
There was a problem hiding this comment.
In the action schema it is defined as map[string]any (Record<string, any>):
genkit/genkit-tools/reflectionApi.yaml
Line 68 in 9445f92
There was a problem hiding this comment.
Without getting into too much detail, I think the question here comes down to whether this value will ever be used in Go code, or will it just be written to JSON for the benefit of the tooling?
If no Go code needs to access the metadata except to JSON-marshal it, then Ian's way is more type-safe and Go-like. With an any parameter, a user can still pass in a map[string]any, or they can pass in a struct that is JSON-marshalable (like the GeneratorCapabilities one you've defined). Both a map[string]any and a struct with the right field names or struct tags will marshal to the same JSON, but the latter will be easier for Go programmers to write and won't need to be hand-converted to a map in the code.
There was a problem hiding this comment.
Using specific types for generators, as you've already written, is consistent with having the metadata parameter of NewAction by type any. Generators can type assert the metadata as needed. Perhaps I misunderstand.
There was a problem hiding this comment.
Yes, it should be possible to get the GeneratorCapabilities from a Generator without going into the action.
There was a problem hiding this comment.
I tried changing the metadata type on NewAction to any, but I don't think it's possible/easy... I'll leave that to you. :)
Basically:
https://github.com/firebase/genkit/blob/main/go/genkit/action.go#L62
and
https://github.com/firebase/genkit/blob/main/go/genkit/action.go#L173
jba
left a comment
jba
left a comment
There was a problem hiding this comment.
Looks basically fine to me.
I agree with Ian's comment, and it will also simplify the code.
But I think you've shown the way here, so I'm fine with you submitting as is and me fixing it up.
3 participants
No description provided.