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.

Sign up for GitHub

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

Jump to bottom

Rich markdown formatting (including streaming) in any mode with --rich #571

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Rich markdown formatting (including streaming) in any mode with --rich #571

wants to merge 11 commits into from

Conversation

gianlucatruda
Copy link

@gianlucatruda gianlucatruda commented Sep 12, 2024 •
edited
Loading

Overview

Fixes #12

This builds on the excellent foundation that @juftin laid down in #278 and

  1. fixes a mysterious bug that was failing some tests and
  2. resolves merge conflicts caused by changes since rich printing #278 was proposed.

I love llm and use it constantly. My only gripe has been the lack of rich formatting in the terminal. I recently used rich for a project and found it excellent, so I was excited to add this to llm. I found #278 was open but dormant, so I decided to nudge things along.

@simonw thanks for your amazing tools and awsome blog!

Screenshots

SCR-20240912-rpqh SCR-20240912-rpxr SCR-20240912-rqws

@gianlucatruda
Copy link
Author

Here's a demo gif of streaming working with rich output:
llm-with-rich-demo

@gianlucatruda gianlucatruda mentioned this pull request Sep 12, 2024
Open
@gianlucatruda
Copy link
Author

Update: I've added pytest tests for making sure that --rich mode works as intended. I also factored in release 0.16 commits. All 185 tests pass.

@simonw is there anything else this needs in order to be merged? That would allow you to close #12

@gianlucatruda gianlucatruda mentioned this pull request Sep 16, 2024
Open
@irthomasthomas
Copy link

What is the benefit versus piping to something? I like the idea of keeping the main project as light as possible.

Screenshot_20240916_180124-1

@dzmitry-kankalovich
Copy link

@irthomasthomas likely the difference is in syntax highlighting of a partial / streamed LLM response.

I've been piping llms output to glow for the past months, but the drawback is that you see the result only when streaming is completed, and until that there is like no output. It's a subpar UX when you need to wait some dozens of seconds to see the result.

As I get it from @gianlucatruda examples here this particular problem was solved.

@gianlucatruda
Copy link
Author

gianlucatruda commented Sep 16, 2024 •
edited
Loading

I like the idea of keeping the main project as light as possible.

I normally would agree, @irthomasthomas. But as @dzmitry-kankalovich correctly points out, piping breaks streaming, which is a major drawback to usability. I think this justifies the choice.

@irthomasthomas
Copy link

irthomasthomas commented Sep 16, 2024 via email

@gianlucatruda
Copy link
Author

gianlucatruda commented Sep 16, 2024 •
edited
Loading

Are you talking about the ansi codes being injected?

@irthomasthomas When you pipe the output of llm to another application that renders markdown (which may do ANSI code injection), you have to wait for the entire LLM response, which could be several seconds or even minutes. And in chat mode, it's not possible at all. So it's not a viable solution.

This PR enables llm to do the rich textual rendering itself in a way that supports response streaming. That means the user sees the llm output in realtime, rendered prettily, as it arrives from the LLM. It also allows this rich text streaming to work in chat mode (as seen in my screenshots).

Overall, this PR adds functionality to llm that is not possible when piping to other tools. It's a massive upgrade to the user experience and something that has been requested by many people for a long time.

@irthomasthomas
Copy link

irthomasthomas commented Sep 16, 2024 via email

@dzmitry-kankalovich
Copy link

@irthomasthomas I just checked, and indeed highlight unlike glow does process stream responses.

however it... does not render markdown?

it just highlights (I guess hence the name) markdown blocks, but it does not render them - at least not like glow or rich.

It is somewhat better than just plain text, but it does not feel as convenient as these other alternatives.

@gianlucatruda
Copy link
Author

That's not true. The example I gave, I'm using highlight and that displays the rendered markdown as it streams in.

@irthomasthomas

  1. can you link to the source for installing highlight? If it's cli-highlight, then I'm unable to replicate the streaming you claim.
  2. your screenshots with piping to highlight are showing syntax highlighting, not markdown rendering as Markdown renderer support  #12 is asking for and this PR provides.
  3. can you provide an example showing evidence that streaming works with piping in both normal and chat modes?

@gianlucatruda
Copy link
Author

@simonw let me know if you have any feedback on this PR. Happy to make any changes necessary.

sa-
sa- reviewed Oct 31, 2024
View reviewed changes
llm/cli.py

def print_response(response, stream=True, rich=False):
# These nested ifs are necessary!? Only way this works.
if stream:
Copy link

Choose a reason for hiding this comment

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

This could also work

live.update(
    Markdown(full_response)
    if rich
    else Text(full_response)
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sa- sa- sa- left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Markdown renderer support
6 participants
@gianlucatruda @irthomasthomas @dzmitry-kankalovich @sa- @juftin @simonw