-
Notifications
You must be signed in to change notification settings - Fork 15
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
decompose class hierarchy of LLM implementations #397
decompose class hierarchy of LLM implementations #397
Conversation
@raulraja For the moment, I feel like the PR is ready for first feedback. |
Since this is quite a large PR, it would be good if we could get a walk-through tomorrow or early this week to see the most relevant aspects and things you changed together. |
core/src/commonTest/kotlin/com/xebia/functional/xef/conversation/ConversationSpec.kt
Outdated
Show resolved
Hide resolved
openai/src/commonMain/kotlin/com/xebia/functional/xef/conversation/llm/openai/AutoClose.kt
Show resolved
Hide resolved
openai/src/commonMain/kotlin/com/xebia/functional/xef/conversation/llm/openai/OpenAI.kt
Show resolved
Hide resolved
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.
Looks great @Intex32 !
tokenizer/src/commonMain/kotlin/com/xebia/functional/tokenizer/ModelType.kt
Show resolved
Hide resolved
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.
Really great job!
I have added some suggestions. Please, let me know your opinion about them
core/src/commonMain/kotlin/com/xebia/functional/xef/llm/MemoryManagement.kt
Outdated
Show resolved
Hide resolved
core/src/commonMain/kotlin/com/xebia/functional/xef/llm/MemoryManagement.kt
Outdated
Show resolved
Hide resolved
.../commonMain/kotlin/com/xebia/functional/xef/llm/models/functions/FunChatCompletionRequest.kt
Show resolved
Hide resolved
core/src/commonTest/kotlin/com/xebia/functional/xef/conversation/ConversationSpec.kt
Outdated
Show resolved
Hide resolved
openai/src/commonMain/kotlin/com/xebia/functional/xef/conversation/llm/openai/Conversions.kt
Show resolved
Hide resolved
* storing full prompt and response messages * updated the rest of the functions * fixed problem in messages order and added tests
PR ready, quillos? |
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.
🚀
decompose class hierarchy of LLM implementations:
from one class per provider → one class per model and it's respective capabilities
Current problem:
you may call functions on a model that the model does not support, as there is only one class per provider that all models use
in particular, ChatWithFunctions extends Chat, whereby ChatWithFunctions is not necessarily compatible with Chat, thus modelled incorrectly which can lead to unsupported operations at runtime