Create/update AI Configuration#4147
Create/update AI Configuration#4147nelsonkopliku wants to merge 6 commits intoexpose-ai-configuration-in-profilefrom
Conversation
0970032 to
64eb0e8
Compare
0b8c6b2 to
41dc558
Compare
64eb0e8 to
b3734fb
Compare
arbulu89
left a comment
There was a problem hiding this comment.
Looks good!
I left some comments as I'm unsure about some of the business logic we pretend to implement.
I recommend letting somebody else who is more aware of things to double-check hehe
| type: :object, | ||
| additionalProperties: false, | ||
| properties: %{ | ||
| model: %Schema{ |
There was a problem hiding this comment.
question:
Don't we need the AI provider field here?
The same for the update.
Or do you extract the provide in the backend somehow?
Is it possible to have the same model name for multiple providers at any given time in the future?
There was a problem hiding this comment.
The idea is to extract the provider based on the model. See here
Is it possible to have the same model name for multiple providers at any given time in the future?
Good question. Technically it might be possible, although it's not in the immediate scope.
Anyway, in the current scheme of things where the provider/model map is defined as follows
config :trento, :ai,
enabled: true,
providers: [
googleai: [
models: [
"gemini-2.5-pro",
"gemini-2.5-flash",
"gemini-2.5-flash-lite",
"gemini-3.1-flash-preview",
"gemini-3.1-flash-lite-preview",
"gemini-3.1-pro-preview"
]
],
openai: [
models: [
"o3-mini",
"o3",
"gpt-4.1",
"gpt-4",
"gpt-5-mini",
"gpt-5.4"
]
],
anthropic: [
models: [
"claude-opus-4-6",
"claude-sonnet-4-6",
"claude-haiku-4-5"
]
]
]we could have the same model in many providers by for instace prepending the provider to the model name openai/modelname and googleai/modelname
This could be either at config level or extracted when provided as a request parameter.
I think we can extend when the need arises.
There was a problem hiding this comment.
Ok, I made provider to be explicitly set upon creation and update in this commit 700aa78.
It was a bit more tedious than I hoped and made the PR changes rise by more than 50% 🙈 (it is fair to say that it is mainly tests, tho)
Now, in total honesty I found the previous version simpler, and it was easily extendable as well to support the same model over many providers scenario by changing the pattern of the model name (the openai/modelname and googleai/modelname option).
However, in any case it is very likely that things around this topic will change because we aim to be able to get the model list from providers' APIs, but we're not sure it will be in the very first version.
If you ask me, I would revert the last commit 700aa78 and go ahead from there, but I want to hear your opinion as well.
test/trento_web/controllers/v1/ai_configuration_controller_test.exs
Outdated
Show resolved
Hide resolved
d795031 to
b06dc2b
Compare
926d411 to
9aff1c6
Compare
ebc92a2 to
a89a153
Compare
b06dc2b to
8c061b2
Compare
a89a153 to
700aa78
Compare
700aa78 to
26a569d
Compare
Description
This PR adds the endpoints to create and update own AI configuration.
More than half of the changes are tests.
Comes after #4140 and #4143
Depends on #4143
How was this tested?
Automated and IRL tests.