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

Improve parsing of responses from the AI #18201

Closed

Conversation

PankajBhojwani
Copy link
Contributor

Summary of the Pull Request

Instead of manually parsing out code blocks from the response we receive, use the cmark library.

Validation Steps Performed

Responses are parsed as expected.

PR Checklist

  • Closes #xxx
  • Tests added/passed
  • Documentation updated
    • If checked, please file a pull request on our docs repo and link it here: #xxx
  • Schema updated (if necessary)

@PankajBhojwani
Copy link
Contributor Author

PankajBhojwani commented Nov 15, 2024

I initially wanted to directly use the RichTextBlock we get from MarkdownToXaml::Convert added in #17585, but because we want to allow the user to click individual code blocks to send it to the shell we cannot just present one RichTextBlock - the palette needs to know which items are code and which items are plaintext and present them individually. The code here uses the same logic though.

Comment on lines +242 to +247
if (!currentRun.empty())
{
const auto chatMsg = winrt::make<ChatMessage>(winrt::to_hstring(currentRun), false, false);
messageParts.push_back(chatMsg);
currentRun.clear();
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't like that this is the exact same as lines 227-231 but making a helper function for 3 lines of code seems silly...

@DHowett
Copy link
Member

DHowett commented Nov 15, 2024

the palette needs to know which items are code and which items are plaintext and present them individually

does it? could we just put all the output in a single speech bubble? i'd rather avoid reimplementing the cmark->xaml parser that mike just wrote if at all possible.

@zadjii-msft
Copy link
Member

mine had a play button, chat needs a copy. that might make the reuse a little trickier

@PankajBhojwani
Copy link
Contributor Author

could we just put all the output in a single speech bubble?

I considered this but there are enough times where the AI sends a large response with multiple code blocks in it that it felt like the palette needs to do its own parsing. For example:

image

If we didn't do our own parsing, there'd be one big bubble with 2 code blocks in it and we wouldn't know which one to use when the user clicks the bubble to send the output to the shell

@lhecker
Copy link
Member

lhecker commented Nov 15, 2024

mine had a play button, chat needs a copy. that might make the reuse a little trickier

Why not both? Having a copy button in your markdown pane seems useful as well. It would also mirror GitHub which also has a copy button on each code block.

@PankajBhojwani
Copy link
Contributor Author

Closing this in favour of #18220

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.

4 participants