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

Support pending/loading message while waiting for response #821

Merged
merged 11 commits into from
Jun 18, 2024

Conversation

michaelchia
Copy link
Collaborator

@michaelchia michaelchia commented Jun 7, 2024

Description

Add support for a pending or loading message to be displayed to let the user know that the request is being processed. Provides updates for the progress on multi-step pipelines.

Fixes #176.

Demo

Screen.Recording.2024-06-07.at.6.46.31.PM.mov

Features

  • Is controlled from the respective ChatHandler on the server side.
  • Text can be configured
  • Supports sequential step-by-step messages before a response
    • E.g. for a RAG chain is may show "Retrieving documents" followed by "Generating answer" once the retrieval step is done.
  • Supports multiple concurrent messages
    • E.g. showing a message for each pending concurrent async task

Usage

This first draft contains some example usage in a few default chat handlers. I would expect the team to modify the message to their liking.

To display a pending message, you can use the BaseChatHandler.pending(text: str, ellipsis: bool = True) -> PendingMessage context manager method to start and remove the pending message.

This was used in default to provide a 'Thinking...' message
https://github.com/michaelchia/jupyter-ai/blob/d499edaab60e60192712ab96e4331c96e07deab1/packages/jupyter-ai/jupyter_ai/chat_handlers/default.py#L61-L65

Similarly in /ask
https://github.com/michaelchia/jupyter-ai/blob/d499edaab60e60192712ab96e4331c96e07deab1/packages/jupyter-ai/jupyter_ai/chat_handlers/ask.py#L75-L78

Used in /learn to replace the static reply message
https://github.com/michaelchia/jupyter-ai/blob/d499edaab60e60192712ab96e4331c96e07deab1/packages/jupyter-ai/jupyter_ai/chat_handlers/learn.py#L154-L167

Instead of the context manager, users can manually start the pending message with BaseChatHandler.start_pending(text: str, ellipsis: bool = True) -> PendingMessage and close it by calling BaseChatHandler.close_pending(message: PendingMessage) -> None.

Implemenation summary

  • 2 new message types were added PendingMessage and ClosePendingMessage.
  • pending start_pending and close_pending where added to BaseChatHandler
  • components/chat.tsx modified to handle the new message types
    • array of PendingMessage maintained following how chat messages was done.
    • PendingMessages element added just below ChatMessages
  • PendingMessages element implemented in compontents/pending-messages.tsx as a modified version of compontents/chat-messages.tsx

Next steps

If this idea is accepted by the team I would need some help cleaning up so it adheres to the product standards.

  • Feel free to rename "pending message" to something else.
    • I chose amongst "pending", "loading" and "temp"
  • You would probably want to reimplement the JSX Element stuff.
    • I just hacked it together to get a working demo. I have very limited experience with React or any frontend stuff.
    • I did some hacky stuff like creating a dummy AgentChatMessage so that I could reuse ChatMessageHeader
    • Consider doing some styling on pending text so it looks different from normal chat text
    • Consider changing from animated ellipsis to some kind of spinner or loading widget if it looks better.

Let me know what you guys think and if there is anything you would like me to add.

@JasonWeill
Copy link
Collaborator

Thank you so much for opening this! When it's ready for review, please add "Fixes #176" to the description, so that we can close this long-running enhancement request.

@JasonWeill JasonWeill added the enhancement New feature or request label Jun 7, 2024
@michaelchia
Copy link
Collaborator Author

I've done whatever I want to for now but definitely a bit needs work for the fontend part. I would need help with that. Would you like me a mark it ready for review first before you guys take a look or would you guys want to provide some comments for me to fix stuff and only mark it when it is at a more ready state?

@dlqqq
Copy link
Member

dlqqq commented Jun 7, 2024

@michaelchia Thank you for this contribution! Feel free to mark it as ready for review if you're seeking reviews. I'll have time to read through this on Monday.

@michaelchia michaelchia marked this pull request as ready for review June 7, 2024 17:30
@krassowski
Copy link
Member

Maybe the frontend part should be contributed to https://github.com/jupyterlab/jupyter-chat and reused here?

@dlqqq
Copy link
Member

dlqqq commented Jun 12, 2024

@michaelchia Thank you for this PR! This would be a wonderful addition to Jupyter AI. I circled this PR around our team, and we are in alignment with the UX you chose. I will review this tomorrow as soon as I get an opportunity and fast-track this PR for merge & release.

Apologies for the delay in my review; our team is mostly out-of-office and I am currently cutting a new Jupyter AI release.

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.

@michaelchia Awesome work! I love the context manager implementation you built in the Python backend. I left a few points of feedback concerning the frontend implementation below.

packages/jupyter-ai/jupyter_ai/models.py Show resolved Hide resolved
packages/jupyter-ai/src/components/pending-messages.tsx Outdated Show resolved Hide resolved
packages/jupyter-ai/src/components/pending-messages.tsx Outdated Show resolved Hide resolved
packages/jupyter-ai/src/components/pending-messages.tsx Outdated Show resolved Hide resolved
packages/jupyter-ai/src/components/pending-messages.tsx Outdated Show resolved Hide resolved
packages/jupyter-ai/style/pending-message.css Outdated Show resolved Hide resolved
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.

@michaelchia Thank you for working on this so quickly! I have a few more minor callouts and this PR should be good-to-go.

packages/jupyter-ai/src/components/pending-messages.tsx Outdated Show resolved Hide resolved
packages/jupyter-ai/src/components/pending-messages.tsx Outdated Show resolved Hide resolved
@dlqqq
Copy link
Member

dlqqq commented Jun 17, 2024

@michaelchia I left a couple more points of feedback above. No rush at all, just writing this as a reminder in case you missed my update on Friday. 😄

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.

@michaelchia Awesome work, thank you very much for contributing and addressing my review feedback! I pushed a couple of commits that 1) simplify the rendered element and 2) match the font size of Generating response... to other Jupyternaut messages. Tested locally and everything is working as expected.

Really excited to include this in the next release! 🎉

Demo screenshot Screenshot 2024-06-18 at 8 53 12 AM

@dlqqq dlqqq merged commit 02d1966 into jupyterlab:main Jun 18, 2024
8 checks passed
@michaelchia michaelchia deleted the pending-msg branch September 4, 2024 05:49
@brichet brichet mentioned this pull request Sep 12, 2024
10 tasks
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.

Provide feedback that Jupyternaut is working
5 participants