Skip to content

Conversation

@laileni-aws
Copy link
Contributor

@laileni-aws laileni-aws commented Apr 20, 2024

Problem

  • The Q chat telemetry event recordAddMessage is missing the required parameter cwsprChatResponseCodeSnippetCount for AB testing.

Solution

  • Counting the query of pre > code to find out total number of code blocks in the generated message response.

TODO:

This is a temporary solution (until backend response contains the number of code blocks with the last chunk), revert this change once backend is ready and use data from the backend.

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

@dogusata dogusata left a comment

Choose a reason for hiding this comment

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

Please replace the code block calculation method with ui endstream method return.

@laileni-aws laileni-aws marked this pull request as ready for review April 22, 2024 16:55
@laileni-aws laileni-aws requested review from a team as code owners April 22, 2024 16:55
Comment on lines 101 to 102
// Count the number of <pre> tags in the HTML to find the total number of code blocks
const totalNumberOfCodeBlocks = (html.match(/<pre>/g) || []).length

Choose a reason for hiding this comment

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

This is not specific enough - it could match unrelated <pre> tags.

Copy link
Contributor

Choose a reason for hiding this comment

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

My suggestion would be jsdom and the method querySelectorAll with a query of pre > code to find all code blocks

@laileni-aws laileni-aws requested review from mitroviv and ptrucks April 22, 2024 21:17
@laileni-aws laileni-aws requested a review from mitroviv April 23, 2024 02:22
@justinmk3 justinmk3 merged commit c749059 into aws:master Apr 23, 2024
@laileni-aws laileni-aws deleted the feature/abtesting branch May 3, 2024 17:10
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.

5 participants