-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Supported model names #133
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can define the constants only once and use the union of LLM Access + orchestration. And then use Omit<.>
within the specific clients?
While there will be exceptions, probably most models will be the same across both, so we would only have to omit a few models, instead of duplicating a lot..
@MatKuhr @deekshas8 Thanks for your feedback! Still a bit hesitating if excluding models would be a good idea as per default, as per default, adding a new model in a reused model type file and then exclude is a bit error-prone as we might simply forget to exclude it. But anyways since it is now quite simple to do so, I weight reusability a bit higher than maintainability, and refactored accordingly. |
Equally, we might forget to add a model in two places, i.e. by accident only adding it in one. For now we can assume 9/10 models will be available in both LLM access & orchestration, so I'd rather take the chance on exclusion 😉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Context
AI/gen-ai-hub-sdk-js-backlog#113.
Model name autocompletion
openai
infoundation-models
orchestration
Model types are defined centrally in
core
and then separately imported into these two packages.Definition of Done
Error handling created / updated & covered by the tests aboveDocumentation updated(Optional) Aligned changes with the Java SDK(Optional) Release notes updated