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

Feature/83694 streaming markdown handling #106

Merged
merged 29 commits into from
Jan 28, 2025

Conversation

dshire
Copy link
Member

@dshire dshire commented Jan 20, 2025

This PR adds support for markdown rendering and streaming messages (animation + message chunk collation) to the chat components. There is a new view on the demo page as well.

For testing, checkout the Webchat branch of the same name with the PR linked here: (Cognigy/Webchat#55)
npm ci && npm pack in this repo,
then npm i ../chat-components/cognigy-chat-components-0.38.0.tgz && npm run dev in the Webchat repo (correct path if it is somewhere else for you)

In the local Webchat index.html you need to enable
behavior: {
collateStreamedOutputs: true,
progressiveStreaming: true,
renderMarkdown: false,
}

Please test directly with the related PR in cognigy AI (coming up)

content={text}
className={`webchat-${classType}-template-header`}
id={webchatButtonTemplateTextId}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this removed? This could potentially break some tests in the webchat repo

Copy link
Member Author

Choose a reason for hiding this comment

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

you are right, I will revert this!

@sushmi21
Copy link
Contributor

Some markdown syntax did not work as expected
Screenshot from 2025-01-24 18-43-08

@sushmi21
Copy link
Contributor

sushmi21 commented Jan 24, 2025

Collation of streamed outputs did not work for me as expected. I had collateStreamedOutputs and progressiveStreaming set to true. Are these two settings dependent. Should collation work when collateStreamedOutputs is true but progressiveStreaming is false

Screenshot from 2025-01-24 18-44-47

@sushmi21
Copy link
Contributor

When I have two say nodes one after the other and if the first one has a longer text than the second one, I would expect the streaming animation of the first one to be completed if the next one is complete. See the below recording: The third message has stopped streaming, but the second message is still streaming which I found out after the scrollbar slightly flickered.
Screencast from 24.01.2025 21:35:55.webm

@sushmi21
Copy link
Contributor

Code looks clean! Great work on the animation :)

@dshire
Copy link
Member Author

dshire commented Jan 27, 2025

When I have two say nodes one after the other and if the first one has a longer text than the second one, I would expect the streaming animation of the first one to be completed if the next one is complete. See the below recording: The third message has stopped streaming, but the second message is still streaming which I found out after the scrollbar slightly flickered. Screencast from 24.01.2025 21:35:55.webm

Yes I have seen this as well, I added it originally just for streamed messages, but then we decided to also enable it for say outputs. So you would see it if you have several say outputs behind each other, especially if there are longer ones. I think if someone sends outputs like that, they should not use this setting to be honest. But we can add some more logic to handle the animation message after message in the future if requested.

@dshire
Copy link
Member Author

dshire commented Jan 27, 2025

Some markdown syntax did not work as expected Screenshot from 2025-01-24 18-43-08

fixed by adding a plugin for tables and some other markdown (https://github.com/remarkjs/react-markdown#use-a-plugin). please remove the triple backticks in your output so that is rendered

src/messages/Text/Text.tsx Fixed Show fixed Hide fixed
src/messages/Text/Text.tsx Fixed Show fixed Hide fixed
@sushmi21
Copy link
Contributor

Markdown contents exceed the bubble width. Not sure if we need to handle this scenario though

Screenshot from 2025-01-27 16-38-26

@dshire
Copy link
Member Author

dshire commented Jan 28, 2025

Markdown contents exceed the bubble width. Not sure if we need to handle this scenario though

Screenshot from 2025-01-27 16-38-26

It's an edge case. I think in that case we can recommend turning off the bubbles. Let's leave this as it is for now

@sushmi21
Copy link
Contributor

I just released v0.39.0 of chat-components. Chat-components should be updated to 0.40.0

@dshire dshire merged commit 6c84e53 into main Jan 28, 2025
6 checks passed
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