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

Allow streaming chunks to HTML panes #7125

Merged
merged 8 commits into from
Aug 12, 2024
Merged

Allow streaming chunks to HTML panes #7125

merged 8 commits into from
Aug 12, 2024

Conversation

philippjfr
Copy link
Member

@philippjfr philippjfr commented Aug 10, 2024

Often, especially for the chat interfaces you are slowly streaming chunks to the frontend. Currently however, we send the entire text every time any of it is updated. This PR proposes to add a enable_streaming parameter to HTML based components, e.g. like HTML itself or Markdown. When enabled the pane will find the length of overlap and then simply sends the non-overlapping chunk to to the frontend using a server-side event, massively reducing the amount of text sent to the frontend. Since this can lead to super rapid updates we currently batch the incoming messages and apply the update at most every 10 ms.

Unfortunately for large messages this quickly runs into this issue: #6603

@philippjfr philippjfr marked this pull request as draft August 10, 2024 23:00
@MarcSkovMadsen
Copy link
Collaborator

MarcSkovMadsen commented Aug 11, 2024

Here are some ideas for the design

  • You could call the stream parameter append instead. Would make it even more clear what its about.
  • As an alternative to a new stream parameter you could have a mode parameter to set how data is updated. Values could be "replace" or "append". Then we could keep using the value parameter which might be more simple. And the mode parameter makes it possible to support other, more general diffing algorithms in the future (see below).
  • As an alternative to appending by position you might implement a more general diffing. There are some algorithms out there to use, but would probably introduce more complexity.

@philippjfr
Copy link
Member Author

I think you're misunderstanding the PR, the stream parameter is a boolean toggle which determines whether to send diffs or not.

@philippjfr
Copy link
Member Author

Okay, sorry reading back your reply I misread it. Generally I agree and I'd be okay with a mode or diff_mode, that said streaming is really the main scenario where you send tons of small individual updates. If you're editing a large document you might want to send smaller patches but finding the patches becomes computationally expensive and in many cases may not be worth it.

@MarcSkovMadsen
Copy link
Collaborator

MarcSkovMadsen commented Aug 11, 2024

Makes sense.

If stream is the only additional mode we will ever support then having a boolean parameter stream is fine.

I think my context is coming from streamlit. My understanding is that they have a virtual document on the python side and only send diff patches using protobuf.

@philippjfr
Copy link
Member Author

I think my context is coming from streamlit. My understanding is that they have a virtual document on the python side and only send diff patches using protobuf.

Do you know of a good writeup of this? This sounds more analogous to the bokeh document than diffing of the contents of an individual update but I'd love to know more.

Copy link

codecov bot commented Aug 11, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 8 lines in your changes missing coverage. Please review.

Project coverage is 81.82%. Comparing base (f7d7f58) to head (0cc0ea5).

Files Patch % Lines
panel/tests/ui/pane/test_markup.py 41.66% 7 Missing ⚠️
panel/chat/icon.py 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7125      +/-   ##
==========================================
- Coverage   82.26%   81.82%   -0.44%     
==========================================
  Files         327      327              
  Lines       48828    48874      +46     
==========================================
- Hits        40168    39993     -175     
- Misses       8660     8881     +221     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@philippjfr philippjfr marked this pull request as ready for review August 11, 2024 16:26
@philippjfr philippjfr merged commit 9b198c8 into main Aug 12, 2024
15 of 16 checks passed
@philippjfr philippjfr deleted the stream_html branch August 12, 2024 11:59
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