Add file attachment directly to JupyternautPersona when file is included in message #1419
Conversation
- Add _process_attachments() method to read and include file content in chat context - Add _resolve_attachment_to_path() to resolve attachment IDs to file paths - Add _get_attachment_from_ychat() to retrieve attachment data from YDoc This enables the Jupyternaut AI assistant to process file attachments in chat messages and include their content as context for better responses.
srdas
left a comment
srdas
left a comment
There was a problem hiding this comment.
Thanks for this PR, the functionality is very useful. Good to see that it works for multiple files in the same context query:
Also, nice error handling for non-supported files:
Comments:
- Suggest moving the new methods added by this PR to the base class in
base_persona.py. - Since it is working well, remove all non-essential DEBUG statements, and change DEBUG to FILE or some other tag?
These changes should be straightforward.
What the PR does not do is take in a folder and nested subfolders and process all acceptable files within it as context and this would be a larger improvement and handled in a separate PR. I tried it as shown:
Appreciate the neat work on this PR! TY.
… code for automated Typing test
|
Thank you for the quick feedback! I've gone ahead and included the changes you suggested. I also ran I didn't think to include folders (and nested subfolders), which is an interesting idea. This is probably not too difficult to implement, but I think that would balloon the context passed to the LLM pretty quickly. Maybe I will explore this further in another PR :) |
dlqqq
left a comment
dlqqq
left a comment
There was a problem hiding this comment.
@joadoumie Thank you for this contribution! 🤗 This PR looks great, just left a couple of suggestions below. Sanjiv had called out an issue with supporting directories, but that can be addressed later.
BTW, most likely, by v3.0, Jupyternaut will be an agent that is capable of reading files at a given path. This means that we will likely pass the path of the file & some metadata (e.g. file size) into the context instead of the entire file by then. Letting the agent decide how to read a file helps prevent token limit issues. 👍
packages/jupyter-ai/jupyter_ai/personas/jupyternaut/jupyternaut.py
Outdated
Show resolved
Hide resolved
packages/jupyter-ai/jupyter_ai/personas/jupyternaut/jupyternaut.py
Outdated
Show resolved
Hide resolved
|
I finished my review right before these changes were committed. My feedback should still be applicable, but let me know if I can help clarify anything. 😅 |
|
Perfect! I'll take a look :) Thanks for reviewing!!! |
This is a great point, and this is exactly why we are seeking to provide Jupyternaut tools. Instead of "shoving all the context in", the agent should know how to retrieve a reasonable amount of context. Based on my observations, this is what Claude Code does. Small files get read directly, large files get pattern-searched via However, this strategy is perfectly fine for a reasonable number of reasonably-sized files, so we can merge this once the feedback is addressed. 👍 If you're interested in contributing more, let me know so I can share some ideas on things to work on. The directory attachments feature will probably have to wait until tools are available to Jupyternaut. |
…ents() from ychat
|
I've incorporated the feedback you gave to this PR. I'd love to keep contributing to the project, so please do let me know what ideas you have for things that I might be able to tackle as I familiarize myself with the codebase 😊 |
srdas
left a comment
srdas
left a comment
There was a problem hiding this comment.
I rebased and retested this PR, all looks good. Nice work @joadoumie !!
@dlqqq I'll approve and you can merge after taking a look.
|
Will merge this PR once CI passes. I just pushed a commit that should hopefully fix the CI; We should drop Python 3.9 support soon, but it should be fairly easy to fix it here. |
|
Sweet! CI is passing (except for the |
|
Thanks for the review and help on this one! Please let me know where else I may be able to contribute 😀 |
3 participants
This PR explores automatically adding the file context to the Jupyternaut Persona (for v3) when a file is attached to a message in a chat.
Here is a gif of the functionality (notice how Jupyternaut can read the content from the file).
Related to #1299
This is my first PR to the project, so please let me know how I can improve anything to make the process smoother!
😊