Skip to content
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

Unify message clearing & broadcast logic #1038

Merged
merged 8 commits into from
Oct 18, 2024
Merged

Conversation

dlqqq
Copy link
Member

@dlqqq dlqqq commented Oct 18, 2024

Description

  • Gathers all message clearing logic into a single method, RootChatHandler.on_clear_request(). Now, the backend history & memory models are cleared before a ClearMessage is broadcast to all clients.
  • ClearChatHandler now just calls RootChatHandler.on_clear_request().
  • Adds new BaseChatHandler.broadcast_message() method, which removes duplicate logic for broadcasting messages scattered across different methods.
  • Removes unused StopMessage backend model accidentally merged with Allow users to stop message streaming #1022.
  • Removes unused ClearRequest.after field in frontend & backend models that should have been reverted before merging Add ability to delete messages + start new chat session #951.
    • If we want to implement bulk message clearing in a future release, a new ClearRequest.targets: Optional[List[str]] field can be added. Some of the methods involved with clearing messages still accept a list of message IDs, so this change shouldn't be too difficult if needed.

Demo

Screen.Recording.2024-10-18.at.8.45.24.AM.mov

@dlqqq dlqqq added the enhancement New feature or request label Oct 18, 2024
Copy link
Collaborator

@michaelchia michaelchia left a comment

Choose a reason for hiding this comment

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

I don't see any obvious issues since it is mostly moving code around. Thanks for cleaning up the code with this refactor.

@dlqqq dlqqq merged commit b3a5f8c into jupyterlab:main Oct 18, 2024
10 checks passed
Marchlak pushed a commit to Marchlak/jupyter-ai that referenced this pull request Oct 28, 2024
* remove unused `StopMessage` backend model

* fixup

* remove unused `after` attr on `ClearRequest`

* unify message clearing logic into `on_clear_request()`

* unify message broadcast logic into `broadcast_message()`

* pre-commit

* fix typing issues raised by mypy

* pre-commit
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