elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.64k stars 8.22k forks source link

[GenAI] Model as a property of the connector #162204

Closed dgieselaar closed 1 year ago

dgieselaar commented 1 year ago

Currently, the GenAI connector has three properties, apiKey, apiUrl, and apiProvider. model is not part of this. For OpenAI, this is a required parameter for the createChatCompletion API request. For Azure, it's part of the deployment. In the current implementation, model is separately chosen by the user, outside of the context of the connector, allowing them to pick a model without having to edit the connector. However, there are certain limitations with this approach:

To that end, I want to suggest making the model a property of the connector. We can perhaps offer the user a list of suggestions when configuring the model in the connector. To limit the impact on existing connectors and the current Security Solution implementation we can perhaps make it an optional property (although I'd prefer a required property).

elasticmachine commented 1 year ago

Pinging @elastic/kibana-security (Team:Security)

elasticmachine commented 1 year ago

Pinging @elastic/response-ops (Team:ResponseOps)

azasypkin commented 1 year ago

Pinging @elastic/kibana-security (Team:Security)

Is there any input needed from us (Kibana Platform Security), or did you mean to ping the Security Solution team (Team:SecuritySolution)?

dgieselaar commented 1 year ago

@azasypkin the latter :) coffee withdrawal. thanks and sorry for the ping!

elasticmachine commented 1 year ago

Pinging @elastic/security-solution (Team: SecuritySolution)

dgieselaar commented 1 year ago

Something I ran into today: it looks like connector.config is not exposed for preconfigured connectors, which means that I can't add the model to the request body, because I don't know the value of connector.config.apiProvider, so one of the providers won't work (depending on what way the if/else goes)

spong commented 1 year ago

As a future use case, say you're creating a chain and want to use different models for different parts of the chain (for cost/capability reasons). In that instance, it would be nice if the connector had a list of 'usable' models (and perhaps a default) instead of just a single model, as that would make it cumbersome to build multi-model chains. Then that way we can at least validate a connector configuration against a chain and let the user know if they have the ability to use the feature/run the chain. This ignores Azure being deployment-locked, in which case a user would need to set up multiple connectors for different models and then the chain would use those different connectors.

I guess in that instance you could make the argument that if we need to support 1:1 connector:model workflows (as mentioned above) then it's fine for the OpenAI apiProvider as well, but perhaps the ease in UX of only setting up a single connector is worth it (I would vote yes)... 🤷‍♂️


For Azure, every model has its own deployment, so there is a 1:1 relationship with a connector anyway. (I'm not entirely sure how the model selection works for Azure, perhaps @spong can clarify).

I haven't done it personally, but from the docs it looks like it's just a configuration when creating a new deployment.

dgieselaar commented 1 year ago

@spong:

I haven't done it personally, but from the docs it looks like it's just a configuration when creating a new deployment.

Sorry, I meant in the UI. Suppose you have one Azure connector, and one OpenAI connector, what is the list of options (apiUrl+apiKey+model) that is available to the user in the current Elastic Security Assistant?

I took a stab at implementing this in https://github.com/elastic/kibana/pull/162754. @spong I like your suggestion, but I think it's something we can implement separately. Note that the implementation I put up should allow the model selection to work as-is for the Elastic Security AI Assistent - it only sets the model value to defaultModel if it has not been overridden in the body.

spong commented 1 year ago

Sorry, I meant in the UI. Suppose you have one Azure connector, and one OpenAI connector, what is the list of options (apiUrl+apiKey+model) that is available to the user in the current Elastic Security Assistant?

Ahh, in that case it's pretty simple 🙂, we just don't show the model selector for GenAI Connectors where provider === OpenAiProviderType.AzureAi.

image image

That said, it would be nice to show additional information to the user for Azure providers like what model is configured and what it's context window size is.


I took a stab at implementing this in https://github.com/elastic/kibana/pull/162754. @spong I like your suggestion, but I think it's something we can implement separately. Note that the implementation I put up should allow the model selection to work as-is for the Elastic Security AI Assistent - it only sets the model value to defaultModel if it has not been overridden in the body.

Nice! And that sounds good to me for the time being 👍. Thanks for the fallback to help with backwards compatibility as well!

lcawl commented 1 year ago

It seems like there's a new defaultModel property that will need to be added to the documentation (e.g. in https://www.elastic.co/guide/en/kibana/8.10/gen-ai-action-type.html and the open API specifications). Is that right? I noticed the changes to the UI while generating screenshots in https://github.com/elastic/kibana/pull/165420

spong commented 1 year ago

That is correct @lcawl, those resources will need to be updated for this new defaultModel property.

WRT the OpenAPI spec update, is that something that could have been handled as part of the original PR (https://github.com/elastic/kibana/pull/162754), or was that not possible at time? Just want to understand where the responsibilities are split here so it can all be handled at the same time going forward. Thanks for catching this! 🙂

dgieselaar commented 1 year ago

@spong I missed the fact that there was an OpenAPI spec, my bad :)

lcawl commented 1 year ago

WRT the OpenAPI spec update, is that something that could have been handled as part of the original PR (#162754), or was that not possible at time? ...

Yes, ideally until we get them auto-generated the specs should be updated in conjunction with the code updates. In particular, I think this change affects https://github.com/elastic/kibana/blob/main/x-pack/plugins/actions/docs/openapi/components/schemas/config_properties_genai.yaml

There are also some doc and screenshot impacts, so I've create an issue to follow-up

spong commented 1 year ago

No worries @dgieselaar! I didn't realize it either, and so was curious what the process would be in the interim while we finish out the OpenAPI gen work. Thank you for clarifying @lcawl!