-
-
Notifications
You must be signed in to change notification settings - Fork 29
[Demo] Add an example with streaming #118
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
Conversation
INstead of an image, could you paste the mermaid code for the diagram to the PR header? |
This looks great - even tho I'm not sure I get it all - but will give it a try 👍 Thanks already! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
demo/templates/index.html.twig
Outdated
</div> | ||
<div class="card-body"> | ||
<h5 class="card-title">Turbo Stream Bot</h5> | ||
<p class="card-text">Simple demonstration of text streaming capabilities.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<p class="card-text">Simple demonstration of text streaming capabilities.</p> | |
<p class="card-text">Simple demonstration of text streaming capabilities based on Turbo and SSE.</p> |
demo/templates/index.html.twig
Outdated
<div class="col-md-4"> | ||
<div class="card stream bg-body shadow-sm"> | ||
<div class="card-img-top py-2"> | ||
{{ ux_icon('tabler:activity', { height: '150px', width: '150px' }) }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's use the same icon on both pages:
{{ ux_icon('tabler:activity', { height: '150px', width: '150px' }) }} | |
{{ ux_icon('mdi:car-turbocharger', { height: '150px', width: '150px' }) }} |
I did consider that, but not sure if it's a good idea since the LLM calls don't show up in the profiler when using streaming. Maybe you know if there's something to do about that? |
oh, wasn't aware, but true, i think having that profiler insights is important for the demo - let's keep it as a separate one - good call 👍 |
Did the suggested changes Created an issue for the profiler bug and I have an idea for the fix too, if it'll work, then maybe we could use streaming for all demos instead of this separate one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work again, thanks!
I'm proposing to add an example which uses streaming via Turbo Streams / SSE since
symfony/ux-turbo
is already included, yet no demo app is using streaming.This demo does not use a custom Stimulus controller at all, but solely relies on vendor-provided
live
&turbo_stream
controllers, making it ideal for those afraid of JS :)A couple of observations made while doing this:
Symfony\AI\Agent\Chat
is not compatible at all with async/streaming flow.AssistantMessage
. Or alternatively,AssistantMessage
would need to be non-readonly (or havewithContent
) so that the message object could be created earlier and appended with content later. At the end, I didn't end up needing the IDs here since they didn't solve the "duplicate messages" (one from turbo, another from live component) problem I had at some point.