-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Refinement of folder import #426
Refinement of folder import #426
Conversation
Hi, great job, and thank you for adding this feature—it’s fantastic! |
I think its related to size of context, that is one of next big things to fix. |
There is one issue with this PR ignore of files, we still get whole trough file input list. Which could be large. I could instead use window.showDirectoryPicker but that is not available in Firefox and Safari. So I could add code that uses that and falls back on input in Safari and Firefox, but don't really want to yet. There bigger issues with bigger projects anyways for now. |
I like this a lot except for the skipping of cretin files, like images. Maybe we could add a tab in the settings modal PR I have and the user could whitelist file types? |
Reason images are ignored is that it was not implemented, they need to be handled dufferently. Currently all files are added in to chat as text. If you allow images they will end up in chat as base64 and overload context. To support non textual files you will need to invent a fute proof way to add binnary files to chat that are not sent to AI. |
Ooo I just realized this we can put base64 content in to the chat and it will not overload the context with this PR added |
toast.info(`Skipping ${binaryFilePaths.length} binary files`); | ||
} | ||
|
||
const { userMessage, assistantMessage } = await createChatFromFolder(textFiles, binaryFilePaths, folderName); |
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.
can this return an array of messages instead of object, so that we can directly pass them.
this will help if we want to return multiple messages instead of one from user and one from assistant
I like the shell command to be in a separate message with its own artifact. so that I can bundle the files artifact into one single action.. its already present the main repo
updated function output
added fix
@wonderwhy-er , please check the adjustments |
Yeah, I was thinking to allow images, and then just filter them from context, replace just with path when sent to AI |
yes we need addition tags on the action tags |
Motivation
PR for refinement of folder import based on feedback from here:
#413 (comment)
Improvements
But even 1000 seems to big. Will not fit in to LLM context anyways...