Skip to content
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

Refactor chatbase to use the new chat completion method in ChatCraftChat class. #832

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

aldrin312
Copy link
Collaborator

@aldrin312 aldrin312 commented Feb 18, 2025

Added a new method for chat completion the ChatCraftChat class and refactored ChatCraftBase to use the new method.
The new method doesn't implement UI elements like forceScrolling. May need to refactor again to include it.

Currently all of the hooks are being passed in from ChatCraftBase. Wasn't sure how to use them directly in ChatCraftChat class or if they are possible to use.

Issues: #819 and #820

Copy link

cloudflare-workers-and-pages bot commented Feb 18, 2025

Deploying chatcraft-org with  Cloudflare Pages  Cloudflare Pages

Latest commit: 7daf6cc
Status: ✅  Deploy successful!
Preview URL: https://bc3ac6d8.console-overthinker-dev.pages.dev
Branch Preview URL: https://refactorchatbase.console-overthinker-dev.pages.dev

View logs

@aldrin312 aldrin312 requested review from humphd and mulla028 February 18, 2025 20:35
@aldrin312
Copy link
Collaborator Author

Create a new hook for chat completion. I find it doing it like this is much more easier and cleaner.
Using it now would be just await chatCompletion(prompt ?? "", chat); and would be easily accessible anywhere in the code.

I'm not sure if the completion method in the ChatCraftChat class should be removed now.

@humphd
Copy link
Collaborator

humphd commented Feb 20, 2025

You don't want to limit the ability to do chat completions to React Components, which a hook-only approach would do. We should have both ways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants