-
-
Notifications
You must be signed in to change notification settings - Fork 432
Load local personas from .jupyter/personas
instead of .jupyter/
#1443
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
Conversation
11f8581
to
d13ee16
Compare
Kicking CI |
d619004
to
8fa0bbf
Compare
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.
Tested this as follows:
- Added the emoji persona files to the
.jupyter/personas/
subfolder and started a new chat session. The persona displays with the@EmojiPersona
prefix. The commands also execute as expected with this persona. - Deleting these files and opening a new chat reveals no emoji persona as expected.
- Also checked that using
/refresh-personas
in chat either adds or deletes the persona as expected.
The code looks good to me.
One thing to be handled in a separate PR is to allow subfolders for each persona under the .jupyter/personas/
subfolder. This is because every persona contains a persona.py
file and hence all personas cannot be in one single .jupyter/personas/
subfolder. This is true for any personas that have common filenames. We also need to add documentation to (i) explain where the files need to be placed, (ii) explain that the dependencies also need to be installed if the persona is just being "dropped in".
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.
Changes look good, thank you Andrii!
My only concerns w/ this area of the code are pre-existing issues, so they don't need to be addressed here. Just calling them out:
-
There is an added security risk with loading any module matching the glob
.jupyter/personas/*persona*.py
. We should explore whether we can be more secure by default. -
We are using the
os
andglob
module to find persona files on the server's local filesystem. This will not work for Jupyter users with a custom/remoteContentsManager
configured in their Jupyter Server.
.jupyter/personas
instead of .jupyter/
References
Fixes #1424
Code changes
Local personas are now loaded from
personas
subderectory of the.jupyter
directory.User-facing changes
If there are personas in
.jupyter
directory, send system chat message warning users that they should be moved to thepersonas
sub-directory.Other considerations
Taking into account that
find_dot_dir
function navigates up from the chat file location in search of.jupyter
folder and therefore there could be multiple potential.jupyter
locations, I considered showing the absolute path to.jupyter
as a part of system message.I decided against it to not overload casual users working in managed instances, power users already know which .jupyter directory contains their personas, and detailed information about where AiExtension is searching for persona files is available in terminal logs: