fix: Accumulate text messages for batched rendering#7231
fix: Accumulate text messages for batched rendering#7231ekoeppen wants to merge 1 commit intoblock:mainfrom
Conversation
b4bfecd to
653cf8a
Compare
In cases where the text format returned by the LLM is structured such as Markdown, streaming responses cannot reliably be rendered if the text messages arrive in separate blocks. Collect all text messages in a turn and render them at once after the turn completes. Signed-off-by: Eckhart Köppen <eck@40hz.org>
653cf8a to
4bda517
Compare
|
hi, I'm sorry, based on your comments on the other I gave the buffering suggestion you mentioned a go: #7233 - basically your approach but with entitity detection. wanna have a look if that is what you were thinking? |
No worries, thanks for taking time with this! Yes, your changes do the trick as well. I've rebased an experimental branch which uses I did think about some simpler approaches in context of your solution, e.g. just render if we get a newline char at the end (that's mostly a 60% solution), or render until we get two newlines in a row (makes it less incremental), in case you're put off by the extra complexity. This topic has however great yak shaving potential, and I'm happy to leave this up to you and the other maintainers how to approach this, even if you decide it's not worth fixing this right now :) |
that is very true. I added table rendering to my branch. right now the output that we have is somewhat syntax highlighted but still more or less standard markdown, which I think is kinda nice. if we go all fancy we would lose that (though if you switch it off for non terminal use cases maybe it doesn't matter?) if you have a pointer to the termimad version, I'd give that a spin. goose suggested that to me too as an option |
Sure, this is the branch: https://github.com/ekoeppen/goose/tree/termimad-rendering. The actual change is surprisingly small: 4836faa I also tested loading custom themes, that has a bit more impact due to passing the theme around which carries now a theme parameter: https://github.com/ekoeppen/goose/tree/termimad-custom-themes Termimad has the drawback that it doesn't do syntax highlighting at all. |
|
Closing, done in #7233 |
In cases where the text format returned by the LLM is structured such as Markdown, streaming responses cannot reliably be rendered if the text messages arrive in separate blocks. Collect all text messages in a turn and render them at once after the turn completes.
This is a minimal change for the issue at hand only, possible followup changes would be to move more module state into the renderer struct, e.g. theme or quiet/verbose output.
Summary
Introduce a struct holding the accumulated text messages, collect messages from streaming responses, and render at the end of the message stream.
Note that this changes the visual appearance of responses to appear at once instead of incrementally.
Type of Change
AI Assistance
Testing
Manual testing and
cargo testRelated Issues
Relates to #7223
Screenshots/Demos (for UX changes)
After: