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

command_endpoints: sanitize result #59

Merged
merged 1 commit into from
Dec 14, 2023

Conversation

stintel
Copy link
Collaborator

@stintel stintel commented Dec 12, 2023

A command endpoint that inserts newlines in the speech result causes the result to be printed outside of the display. We do not want to deal with this in the Willow C code, and as we will eventually make WAS command mode the default and rip out endpoint support from Willow, we can simply do this in WAS.

As we're using a pydantic model for the command endpoint result, we can easily do this with a sanitize function in the CommandEndpointResult model, and call this when we initialize the CommandEndpointResponse. We can't do it when we initialize the CommandEndpointResult, as we sometimes change the speech attribute after init.

@stintel stintel force-pushed the feature/endpoint_response_strip_newlines branch from dfac24a to 636652d Compare December 12, 2023 16:05
A command endpoint that inserts newlines in the speech result causes the
result to be printed outside of the display. We do not want to deal with
this in the Willow C code, and as we will eventually make WAS command
mode the default and rip out endpoint support from Willow, we can simply
do this in WAS.

As we're using a pydantic model for the command endpoint result, we can
easily do this with a sanitize function in the CommandEndpointResult
model, and call this when we initialize the CommandEndpointResponse.
We can't do it when we initialize the CommandEndpointResult, as we
sometimes change the speech attribute after init.
@stintel stintel force-pushed the feature/endpoint_response_strip_newlines branch from 636652d to c0d5e73 Compare December 12, 2023 16:37
@kristiankielhofner
Copy link
Contributor

This is great! I've noticed this most with LLM support. I added (more or less) the exact same code to WAC but this is a much better approach!

@kristiankielhofner kristiankielhofner merged commit 5f3fe38 into main Dec 14, 2023
3 checks passed
@kristiankielhofner kristiankielhofner deleted the feature/endpoint_response_strip_newlines branch December 14, 2023 12:41
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.

2 participants