Skip to content

Send the message ID in the writer state when the user edit a message #208

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

brichet
Copy link
Collaborator

@brichet brichet commented Apr 11, 2025

This PR improves the handling of the writers, by sending also the message being edited.

Related to #202, see #202 (comment) for context (only the last change was required by #202).

Changes

  • when editing a message, the input model of the message edition is added to the chat model => only one message can be edited at the same time
  • jupyterlab-chat also listen to the message edition to update the writer list
  • the writer list can optionally contain a message ID if this is a message edition

Copy link
Contributor

Binder 👈 Launch a Binder on branch brichet/jupyter-chat/improving_writers

@brichet brichet added the enhancement New feature or request label Apr 11, 2025
Copy link
Member

@dlqqq dlqqq left a comment

Choose a reason for hiding this comment

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

Thanks for opening this PR! Just curious: I thought we already only allow a user to update their own messages. What is the motivation for this PR?

As long as a user can only update their own messages, I don't see a use-case for only letting a user update one message at a time. I think it makes sense to allow users to have the "edit message" UI open for multiple messages, even if it doesn't make sense. Figuring out a way to do this will probably be easier than managing the canEdit state correctly 100% of the time.

We also don't need to broadcast the ID of the message being edited through the awareness layer to lock it, because users should never be editing the same message anyways. As long as we have frontend controls for ensuring users only update their own messages, that should be sufficient.

Great E2E testing BTW. We really need to catch up with you all in Jupyter AI. 😅

@brichet
Copy link
Collaborator Author

brichet commented Apr 17, 2025

Before this PR, the input model was created in the message component itself. The model was not aware of this new input model, and could not listen to it to update the awareness.
So one of the idea of this PR was to add the "message edition input model" to the chat model when message is edited. That way we are able to listen for this new input model changes, and update the awareness about the user currently writing in it (or not).

Indeed, currently it allows only one message edition at the time, to avoid multiple input models.
It makes also sense to allow multiple current edition, we probably just need to have a list of IChatModel.IMessageEdition. 👍

@brichet brichet marked this pull request as draft April 18, 2025 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants