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

Dedicate one set of chat handlers per room #9

Merged
merged 5 commits into from
Dec 5, 2024

Conversation

dlqqq
Copy link

@dlqqq dlqqq commented Dec 4, 2024

Description

  • Makes chat handlers non-singletons in Jupyter AI. Each chat handler is dedicated to at most 1 YChat instance.
  • Makes YChat an instance attribute instead of a local variable that gets forwarded across methods.
  • Reverts most changes to chat handlers (except for the base class).

Demo

Screen.Recording.2024-12-04.at.11.03.06.AM.mov

Copy link
Member

@brichet brichet left a comment

Choose a reason for hiding this comment

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

Thanks @dlqq, this looks great and should considerably reduce the diffs in jupyterlab#1043.

I have some comments/questions below.

packages/jupyter-ai/jupyter_ai/extension.py Outdated Show resolved Hide resolved
packages/jupyter-ai/jupyter_ai/extension.py Show resolved Hide resolved
packages/jupyter-ai/jupyter_ai/extension.py Show resolved Hide resolved
packages/jupyter-ai/jupyter_ai/extension.py Show resolved Hide resolved
Co-authored-by: Nicolas Brichet <[email protected]>
@dlqqq
Copy link
Author

dlqqq commented Dec 5, 2024

@brichet Addressed your comments. As for the ychats_by_room dict, we can remove that later. It has a tiny memory footprint, and there could be a future use-case for caching get_room() calls.

@brichet brichet merged commit c1b3d5d into QuantStack:jupyter-chat Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants