-
Notifications
You must be signed in to change notification settings - Fork 45
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
Initial support for files table in Dexie and DuckDB #789
Conversation
Everything is solvable with more layers of indirection :) This is solved by adding another 'file-content' table |
): Promise<ChatCraftFile> { | ||
let options: ChatCraftFileOptions; | ||
|
||
if (input instanceof File) { |
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.
findOrCreate only makes sense to do via sha
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.
That's exactly what it will do. Below this, I calculate the sha based on the file.
This lets us receive a File
object in the app and use it to do a search, handling the hashing internally.
* @param options Listing options for sorting and filtering | ||
* @returns Array of matching ChatCraftFile instances | ||
*/ | ||
export async function ls( |
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.
Why not use duckdb glob here? Not a fan of making a non-standard glob
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.
This is not part of duckdb, it's the files in Dexie. We also need it for duckdb, but not in this file.
src/lib/commands/DuckCommand.ts
Outdated
|
||
// Include files if there are any, including files in the chat | ||
const files = await chat.files(); | ||
if (files.length) { |
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.
love it
* from the chat's files. | ||
* @param chat the ChatCraftChat, potentially with files | ||
*/ | ||
export async function getFiles(chat: ChatCraftChat) { |
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.
How about we do queryToMarkdown() here..and add a filter predicate to queryToMarkdown to allow filtering items...might be useful elsewhere
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.
Let's enhance this in a follow up. I wonder if we should be returning an object like this:
const result = query('select ...');
// result can now be used in various ways:
const markdown = result.toMarkdown();
const json = result.toJSON();
...
We could add some way to do further sub-queries on it too, by passing the Arrow Table result back into DuckDB and running another sql query.
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.
I like your proposal of an object with multiple "projections"...ok with followup
Overall, nice stuff. What's left to land this? Just my review? |
Deploying chatcraft-org with
|
Latest commit: |
cb4d691
|
Status: | ✅ Deploy successful! |
Preview URL: | https://ce30ec4e.console-overthinker-dev.pages.dev |
Branch Preview URL: | https://humphd-files-table.console-overthinker-dev.pages.dev |
@tarasglek I think this is good to go. If you're ok with it, let's merge and keep improving in follow-ups. |
Fantastic work here |
This begins the journey to implement #788. I've done the minimum to get a workable thing. There is lots more to do, but it's already amazing!
files
table, which tracks files by their hashed content id and includes metadataChatCraftFile
class to make it easier to work with files and convert to/from various thingsfileIds
Array to thechats
table to track files associated with a chatChatCraftFile
andfiles
to theuse-file-import.ts
hook so attaching, pasting, dragging files into a chat all add them to thefiles
table and associate with the current chat's files.duckdb-chatcraft.ts
to know aboutChatCraftFile
, bridging the filesystem from Dexie -> DuckDB (we still need to go the other way).chatcraft.*
tables, auto-injects files into DuckDB when they are referenced in a SQL query (by name or id). Ive you go toOptions > Attach Files...
and add a CSV file, doing something likeselect * from 'some-file-name.csv'
just works./duck
command to be able to show all tables and all files, including virtual files in the chat we could inject./ls
slash command to list files in a chatimageUrls
work at all.For follow-ups--I don't want to bother with this stuff now, but so I don't forget:
runCode
working with file injection yet, since it requires thechat
to be passed all the way through to the inner component. We should probably make a React Context and Hook for this, so you can doconst chat = useChat()
imageUrls
out ofmessages
and intofiles