Add ui.MarkdownStream

[cpsievert]

This PR extracts the streaming markdown capabilities of ui.Chat() into another component: ui.MarkdownStream(). See the new files for example usage:

• shiny/api-examples/MarkdownStream/app-express.py
• shiny/api-examples/MarkdownStream/app-core.py

While extracting out the markdown streaming portion, I also improved the 'auto-scroll' behavior. Now it should be easier to disable/enable auto scroll by scrolling away/towards the bottom of a scrollable parent of the stream. Also, this is done in such a way that MarkdownStream() will automatically find the nearest scrollable parent and scroll that element (so that, it just works in something like a card with a set height).

Follow up

• Handle fallout for shinychat Add markdown_stream() shinychat#23

[gadenbuie]

Not something to handle in this PR, but we should anticipate highlighting styles being something that people will want to configure.

[gadenbuie]

Would the Shiny Express pattern work in Core?

md = ui.MarkdownStream("readme")

ui = ui.page_fluid(
    md.ui()
)

def server(input, output, session):
    # Read in the README.md file from the py-shiny repository
    # ... snip ...

    @reactive.effect
    async def _():
        await md.stream(chunk_generator())


app = App(app_ui, server)

[cpsievert]

No, I don't think we want to support that since, if there are multiple user sessions, they would be mutating the same object

[gadenbuie]

Could we make it work?

If not, I think the documentation needs to be a bit stronger: like don't use this in Core! But I'd rather not solve this with documentation; if we can save people from self-inflicted issues we should do that. Even better if we can have a consistent interface that works the same way in both modes.

[gadenbuie]

If we use md.ui(), we wouldn't have the "is it an output or not?" problem.

[cpsievert]

I get where you're coming from. I think it could technically work since there's not currently any important state management going on, but it makes me feel uneasy that it would handcuff future design/engineering. I also like that the usage constraints will be consistent with ui.Chat(), and potentially other components that need a similar design.

Also, the tone of the docs...

This method is only relevant for Shiny Express. In Shiny Core, use :func:~shiny.ui.markdown_stream_ui for placing the markdown stream in the UI.

... feels appropriate to me since there is no inherent danger in using .ui() in Core if you know what you're doing

[gadenbuie]

I've been thinking about this while traveling and my opinion distills down to thinking that we should do whatever we can to make sure that the Core and Express usage and implementation are as unified as possible. If we can write a component such that you use it the same way in both syntaxes and the implementations are the same, it's worth the reduced complexity for future support.

The one shortcut I think we should allow ourselves is to automatically create UI elements when we can in Express – like not needing ui.output_plot() because it's obviated by @render.plot. For MarkdownStream, however, users still need to insert the UI in both cases. My objection is that how that UI is created is different between the two (instance method in Express, separate function in Core).

Another thing to consider is that the class-based approach is very natural in Python, but slightly at odds with how things work in Shiny in general. My sense is that many people want our components to work more like the Express version in this PR, but there are reasons why that's difficult.

My feeling is that we should either figure out how to make a class-based component work all the way in all the places, or we should stick with an interface that's common to the rest of Shiny. Here's a patch that enables using the class-based approach in both Core and Express:
md-stream-core.patch. I'm not sure how this pattern will work out with modules, though.

Speaking of modules, I think the component needs to use resolve_id() throughout (added in the patch above) to support modules. But that got me thinking: maybe we could use Shiny modules for this component instead. That would look like this:

ui = ui.page_fluid(
    ui.markdown_stream_ui("readme")
)

def server(input, output, session):
	md_readme = ui.markdown_stream_server("readme")

	# ... chunk generator code ...

    @reactive.effect
    async def _():
        await md_readme.stream(chunk_generator())

In other words, the server function would return the MarkdownStream instance, with the appropriate classes. In that case, because we're building on a syntax and pattern that already exists, we could considering having the express version of markdown_stream_server() emit the UI as well (or maybe just called markdown_stream()) in Express.

from shiny.express import ui

```python
md_readme = ui.markdown_stream_server("readme")

# ... chunk generator code ...

@reactive.effect
async def _():
    await md_readme.stream(chunk_generator())

[gadenbuie]

Summarized in #1837

[gadenbuie]

I think it's worth considering having either a .set() method (or better named) or at least including in the docs that .stream([content]) can be used to set the content. Primarily because we allow setting the initial content and you'll likely want to be able to restore that initial state if you called the .clear() method.

Relatedly, I can see utility in having an .append() that you could use synchronously. AFAICT that's entirely supported by the component but would be hard to access as a user.

[cpsievert]

Addressed as a note in d091ebd

If you feel strongly about .append(), would you mind filing an issue with some thoughts on it?

[gadenbuie]

Is it possible to change this choice after you've created the UI element? It's worth considering.

Or maybe it's that this feels like it's in the wrong place, since the message is the thing that's in html, text or markdown. I can see this being less relevant for the chat use case but as a standalone component it seems feasible to want to switch between html/text/markdown.

I think we should leave content_type here, to pair with content, but we should also have content_type as an argument in stream(). That argument could default to None – i.e. don't change the content type – but if provided we'd change the content type with the first message. (Just brainstorming out loud...)

[cpsievert]

but we should also have content_type as an argument in stream()

I agree, but unfortunately it wouldn't be easy to support that when .stream(clear=False) since you could end up with a mix of content types. That said, I guess we could just not allow content_type to be specified when clear=False?

Maybe a better way to go to get around this is to have .stream() always clear/replace the content, and then a .append_stream() that wouldn't have content_type?

[cpsievert]

Merging so this available to others at work week. Will file issues to handle remaining feedback