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

Add useChat() hook and ChatProvider context #822

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

humphd
Copy link
Collaborator

@humphd humphd commented Feb 9, 2025

Fixes #818, adding a new <ChatProvider /> context and associated useChat() hook. We can use this to access the current chat anywhere in the React tree without props.

After this lands, it would be nice to refactor our code to not bother passing the chat via props all the time.

You can test this by trying to have the LLM write SQL that references one of the files or tables in DuckDB (e.g., select * from chatcraft.chats) and then clicking the run button to execute it.

Copy link
Collaborator

@mulla028 mulla028 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small fix required. Overall, LGTM!

@@ -62,7 +64,7 @@ function CodeHeader({
setIsRunning(true);

try {
let { logs, ret } = await runCode(code, language);
let { logs, ret } = await runCode(code, language, chat);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you pass code, language, and chat to the runCode.
We want to re-render handleRunBrowser with the latest value of chat. It re-renders on change of dependencies.
Therefore, you have a missing dependency: chat on line 114.
It should look like:

}, [onPrompt, code, language, chat]);

vs.

}, [onPrompt, code, language]);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, it doesn't work as expected, but it works with the oldest value of chat. As you can see messageids aren't updating for the new chats, since chat value isn't updating:
image

Fixed locally, now the same query returns current data table, since runCode receives updated chat value:
image

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.

Add useChat() hook
2 participants