Skip to content

Conversation

@cpsievert
Copy link
Collaborator

@cpsievert cpsievert commented Jul 8, 2024

With this PR:

  1. .append_message() and .append_message_stream() now (correctly) scroll to the bottom of the chat window (after the message gets appended/updated).
  2. During a .append_message_stream(), the scroll stays at the bottom (unless the user scrolls up).
  3. During a resize, the scroll stays at the bottom (unless the user scrolls up).
  4. When a scroll is triggered outside of a streaming/resizing context, it's done with a smooth transition.
Testing app
import os

from langchain_openai import ChatOpenAI

from shiny.express import input, ui

# Provide your API key here (or set the environment variable)
llm = ChatOpenAI(
    api_key=os.environ.get("OPENAI_API_KEY"),
)

# Set some Shiny page options
ui.page_opts(
    title="Enter a message; the chat should scroll to bottom",
    fillable=True,
    fillable_mobile=True,
)

ipsum = """
Elit fames nascetur cubilia vulputate donec habitant pulvinar? Tempor
nam laoreet est pellentesque, nostra hendrerit mus id torquent –
molestie habitant turpis dis felis. Praesent laoreet montes neque
volutpat nulla pellentesque non pretium turpis dapibus nibh. Faucibus
interdum nascetur quis: semper facilisis fusce, quis felis? Felis dis
lectus, fringilla integer facilisis, felis ridiculus parturient purus
venenatis. Vivamus euismod.

Adipiscing mauris ullamcorper rutrum justo dis lectus. Felis habitasse
dui habitant neque ornare tempor velit placerat tellus. Sed turpis
laoreet, nunc pellentesque, maecenas commodo quis. Auctor tempor
quisque nisl eget urna quis mollis commodo. Odio ultrices dui fermentum
fermentum id conubia proin nulla habitant penatibus netus vestibulum
etiam urna duis gravida pharetra.

Ipsum inceptos sociis, fusce magna potenti quisque pretium blandit a
volutpat nascetur. Cursus potenti mattis erat phasellus, rhoncus
fermentum placerat, interdum mus tempor eleifend! Aliquet placerat
massa quis phasellus cubilia euismod luctus tempus. Etiam magnis dis
phasellus, potenti netus volutpat.
"""

chat = ui.Chat(
    id="chat",
    messages=[ipsum],
)

chat.ui()

chat.set_user_message("Tell me a story")


@chat.on_user_submit
async def _():

    if input.stream():
        msg = llm.astream(chat.user_input())
        await chat.append_message_stream(msg)
    else:
        msg = await llm.ainvoke(chat.user_input())
        await chat.append_message(msg)


ui.input_switch("stream", "Stream messages", True)

shiny-chat-input {
margin-top: auto;
position: sticky;
background-color: var(--bs-body-bg, white);
Copy link
Collaborator Author

@cpsievert cpsievert Jul 8, 2024

Choose a reason for hiding this comment

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

This part is a bit tangental to the rest of this PR -- the intention here is to prevent message content from being visible below the input

Before

Screenshot 2024-07-08 at 10 47 31 AM

After

Screenshot 2024-07-08 at 10 49 43 AM

@cpsievert cpsievert force-pushed the chat-append-user-msg branch from e5f48df to 6e04ce4 Compare July 8, 2024 17:58
@cpsievert cpsievert changed the title Fix Chat()'s scroll-on-append behavior Fix and improve Chat()'s scroll-on-append behavior Jul 8, 2024
@wch
Copy link
Collaborator

wch commented Jul 8, 2024

I haven't tried these changes yet, but I think that it would be good if the behavior were as follows:

  • If it is currently scrolled to the bottom of the chat, then whenever new content is added (even just a single word), it should auto-scroll to the bottom.
  • If it is not currently at the bottom of the chat, then do not auto-scroll at all.

@cpsievert cpsievert force-pushed the chat-append-user-msg branch from 427e719 to b95bf81 Compare July 8, 2024 20:16
@cpsievert
Copy link
Collaborator Author

cpsievert commented Jul 8, 2024

If it is not currently at the bottom of the chat, then do not auto-scroll at all.

I'm not sure I like this is better than the current behavior. Here are a few points to consider:

  1. When first loading a Chat(), if there are already overflowing messages, should submitting an input message not scroll to the bottom? If we don't in that case, it might not be clear to the user that a message has been appended.
  2. If other code/controls (other than the chat's input) are allowed to .append_message(), it could also lead to a situation where the user is left wondering what effect their action has made?
  3. This behavior deviates from what ChatGPT does.

All that considered, I think I could come around your suggestion, but in that case, we definitely need a better visual cue that messages have been added to the chat.

@wch
Copy link
Collaborator

wch commented Jul 8, 2024

The behavior of ChatGPT is what I had in mind. Notice that when it's scrolled to the bottom, the chat auto-scrolls as text is added. When it's not scrolled to the bottom, then the chat does not auto scroll.

chatgpt-scroll.mp4

@cpsievert
Copy link
Collaborator Author

Ohhhh, I thought you meant that, when a new call to .append_message()/.append_message_stream() happens, then we shouldn't scroll to the bottom (unless we're already at the bottom). That's different than what happens during a stream.

@cpsievert cpsievert marked this pull request as ready for review July 8, 2024 21:14
@cpsievert cpsievert requested a review from wch July 8, 2024 21:14
@cpsievert cpsievert force-pushed the chat-append-user-msg branch from 4aa8481 to 6609fbd Compare July 8, 2024 21:54
…s, but if we do in fact support it, we should have some sort of visual cue that their is overflow -- i.e., a scroll button)
@cpsievert cpsievert force-pushed the chat-append-user-msg branch from 6609fbd to a88a3cb Compare July 8, 2024 21:55
@cpsievert cpsievert added this to the v1.0.0 milestone Jul 9, 2024
@wch
Copy link
Collaborator

wch commented Jul 9, 2024

Note that there is an issue with the z-index of the clipboard icon in code blocks.

image (27)

@cpsievert cpsievert changed the title Fix and improve Chat()'s scroll-on-append behavior Improve Chat()'s auto-scroll behavior Jul 9, 2024
@cpsievert cpsievert enabled auto-merge July 9, 2024 18:52
@cpsievert cpsievert added this pull request to the merge queue Jul 9, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 9, 2024
@cpsievert cpsievert merged commit 6d95314 into main Jul 9, 2024
@cpsievert cpsievert deleted the chat-append-user-msg branch July 9, 2024 19:18
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.

3 participants