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

Add Markdown Render Support to GPT completions #56

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

Conversation

praveen-palanisamy
Copy link

@praveen-palanisamy praveen-palanisamy commented Mar 27, 2023

Purpose

The frontend on the main branch currently doesn't support rendering Markdown in the model's responses/completions. This PR adds support for rendering Markdown while retaining support for raw HTML code in the completions.
Fixes #30

Current This PR
without-markdown-support markdown-support_cropped

Does this introduce a breaking change?

[ ] Yes
[x] No

Pull Request Type

What kind of change does this Pull Request introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

How to Test

  • Get the code
git clone https://github.com/Azure-Samples/azure-search-openai-demo
cd azure-search-openai-demo
git checkout -b praveen-palanisamy-add-markdown-render-support main
git pull https://github.com/praveen-palanisamy/azure-search-openai-demo add-markdown-render-support
  • Test the code
    Run backend & frontend on Azure or locally and test with a prompt that would result in GPT's response/completion containing Markdown syntax. Example prompt:
Showcase markdown syntax features

What to Check

Verify that the UI renders Markdown syntax as expected.

Other Information

  • Uses react-markdown which is safe by default (no dangerouslySetInnerHTML or XSS attacks)

@vrajroutu
Copy link

@praveen-palanisamy approved this pull request. It is working as expected.

@praveen-palanisamy
Copy link
Author

praveen-palanisamy commented Mar 28, 2023

Hi @kishorerv93 : Thank you for reviewing, testing and confirming that this PR works as expected and resolves your issue. We need a review from @jongio / @chuwik / @pablocastro or another core dev with write access to approve/merge this.

@ToadRider
Copy link

bumping this so that @jongio / @chuwik / @pablocastro can review this to get it implemented.

@pamelafox pamelafox self-requested a review October 18, 2023 05:15
@pamelafox pamelafox self-assigned this Oct 18, 2023
@M0inUddin
Copy link

@praveen-palanisamy for me it doesn't seem to work properly.
image

@pamelafox
Copy link
Collaborator

I suspect that the actual Markdown returned by the second question included backticks, and thats why the Markdown renders as code. Here's a deployed version which includes the markdown renderer:

https://app-backend-dsqwnx2uoc57g.azurewebsites.net/

@praveen-palanisamy
Copy link
Author

@pamelafox : Resolved the merge conflicts with the latest main branch. Please let me know if there's anything pending on this PR or if it's ready to finalize/merge.

@pamelafox
Copy link
Collaborator

@praveen-palanisamy Thanks for updating to main! I've integrated this change in this demo app here to test it out:
https://app-backend-dsqwnx2uoc57g.azurewebsites.net/

Now that we've added streaming to the repo, this change does introduce a slight visual hiccup when it's interpreting certain incremental Markdown, and some of those can be jarring. I screen capped it happening:

Glitch - turned list item into a heading:
Screenshot 2023-11-01 at 1 44 53 PM

Fixed a moment after - back into a normal list item:
Screenshot 2023-11-01 at 1 45 11 PM

I haven't dug into precisely the underlying Markdown causing the glitch, but I think we'd want some solution before merging into main, to avoid that jarring effect. It looks like it may be related to citations, and we might need to make sure that citations get streamed a whole citation at a time, but I'm not certain that's the case.

If you have time to look into it, let us know what you find out.

@pamelafox
Copy link
Collaborator

cc @SteveJSteiner who authored the streaming change (see comment above on interaction with this PR)

@praveen-palanisamy
Copy link
Author

praveen-palanisamy commented Nov 1, 2023

That's a good observation! If we have Markdown rendering enabled while streaming (partial fragments/chunks of) output text, that's an inevitable artifact, I think. I have observed this behavior with the official ChatGPT webapp and also with the Bing (Enterprise) Chat. A sample repro below where the partial markdown bold text takes a few more bytes to be complete and render properly. This output didn't include Markdown headers to match with your above screenshot, but you can see the behavior:

StreamingMarkdownRender-BingChatEnterprise.mp4

If this is an unacceptable UX, one way to handle this is to add another post-processing step in AnswerParser.tsx to the run_with_streaming output fragments (from chat.completion.chunk) to contain complete/closed Markdown tags before displaying on the UI.

@SteveJSteiner
Copy link
Contributor

SteveJSteiner commented Nov 2, 2023

Yes, I have seen chatGPT with some visual ticks due to re-rendering of markdown. Table as the output hits this. I can also reproduce some oddities in Bing as well as it creates annotated lists.

@pamelafox - what kind of bar do you wish to apply for these cases? Adding markdown support seems highly worthwhile.
Perhaps fix the 'worst' of the visual hiccups as one-offs?

Can the markdown support be disabled if a user customizing the demo repo finds it too jarring for their end customers? Turing off the streaming in those cases is also an option.

Copy link

github-actions bot commented Jan 2, 2024

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed.

@github-actions github-actions bot added the Stale label Jan 2, 2024
@praveen-palanisamy
Copy link
Author

@pamelafox : Could you please provide your final comments to finalize this PR soon?

@microsoft-github-policy-service agree

@github-actions github-actions bot removed the Stale label Jan 20, 2024
@pamelafox
Copy link
Collaborator

Okay, so I think what we could do is just change our CSS so that headings inside the chat boxes aren't actually very large, that way it isn't such a jarring visual glitch. How about that?

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.

Markdown is not rendered as HTML in the frontend
6 participants