Skip to content

Fix v0.14.0 bugs #242

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

Merged
merged 9 commits into from
Jun 25, 2025
Merged

Fix v0.14.0 bugs #242

merged 9 commits into from
Jun 25, 2025

Conversation

dlqqq
Copy link
Member

@dlqqq dlqqq commented Jun 25, 2025

Description

This PR fixes each of the small bugs reported in #241. The commit history addresses each in the order they appear in #241.

The most notable changes are:

  1. The user.mention_name property has been made consistent across the frontend & backend. It is now computed using the same procedure and is always defined in the LabChatModel.

  2. The inputModel.currentWord property has been modified to allow for spaces escaped with a preceding backslash \ in the current word. This allows commands to contain spaces, allowing @file in JAI to accept file paths that contain spaces.

@dlqqq dlqqq requested a review from brichet June 25, 2025 13:44
@dlqqq dlqqq self-assigned this Jun 25, 2025
@dlqqq dlqqq added the bug Something isn't working label Jun 25, 2025
Copy link
Contributor

Binder 👈 Launch a Binder on branch dlqqq/jupyter-chat/0.14-fixes

@brichet
Copy link
Collaborator

brichet commented Jun 25, 2025

@dlqqq thanks for working on this.

  1. The user.mention_name property has been made consistent across the frontend & backend. It is now computed using the same procedure and is always defined in the LabChatModel.

It may be a naive question, but I wonder what would be the drawback to just build this mention_name every time the setUser method is called (frontend and backend). That would ensure that every user has the property and that the same pattern is used.

And and we want backward compatibility, we can probably add it on first load.

@dlqqq
Copy link
Member Author

dlqqq commented Jun 25, 2025

@brichet Great question. The benefit of adding a LabChatUser with a mention_name computed property is that it allows getting a mention_name from any object with display_name, name, username fields. We're not just getting user objects from the YChat, we are also getting it in:

  1. From options.user in LabChatModel.constructor(), and
  2. From the awareness states in get ChatContext.users().

These objects may not have the mention_name property since they come from other code not defined by Jupyter Chat. Just making changes to YChat.setUser() doesn't add mention_name to these 2 objects, so we have to add this by wrapping these objects in LabChatUser. I tried just changing YChat.setUser() first, but that led to runtime errors since user.mention_name was not always defined.

The implementation probably can be simplified, but the changes should be safe as-is since LabChatUser is only used in LabChatModel.

@dlqqq
Copy link
Member Author

dlqqq commented Jun 25, 2025

@andrii-i BTW, this PR can be tested by:

I would recommend doing this in a new environment to avoid breaking an existing one.

@dlqqq dlqqq requested a review from andrii-i June 25, 2025 17:34
@dlqqq dlqqq changed the title Fix v0.14 bugs Fix v0.14.0 bugs Jun 25, 2025
@dlqqq
Copy link
Member Author

dlqqq commented Jun 25, 2025

We plan to publish a v0.15.0 release after this PR is merged so we can integrate the latest version w/ JAI v3.

Copy link
Collaborator

@andrii-i andrii-i left a comment

Choose a reason for hiding this comment

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

Looks and works well, improves user experience. Thank you @dlqqq.

I've tested this separately and with Jupyter AI v3, mentions and commands/filenames (including with spaces) work well in different scenarios (couldn't break it).

@dlqqq
Copy link
Member Author

dlqqq commented Jun 25, 2025

@andrii-i Thank you!

Our team is working really hard on Jupyter AI v3 right now & needs this change to use the main branch. I'm going to merge this & publish a v0.15 release right now. This should be safe since we're still on v0, and everything that depends on v0 should be using strict version ceilings.

@jtpio @brichet If our changes / releases ever cause an issue, please let us know. We're happy to take responsibility for our PRs & fix any issues they may cause.

@dlqqq dlqqq merged commit ce0e13d into jupyterlab:main Jun 25, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minor bugs in Jupyter Chat v0.14.0 w/ JAI v3
3 participants