[Request] Let the validator (get_response) drive the response return value

[emphasize]

By now you validate the response - lets say you require a number in the response - and if so, you return the response.

So - to give an example - if someone says: "The second title", the return (if validated) is "The second title" In this context you would require to extract_number in the validator to validate the answer is inside the boundaries, but - depending on your code - would need to do it again outside the get_response call.

code example: here the validator excepts a number, sentence containing a number or a fitting song name -> int

song = message.data.get("song", False)
if not song:
        num = self.get_response("ask.specific.song",
                                validator=self.validate_specific_song,
                                on_fail="nothing.found", num_retries=1)
...
def validate_specific_song(self, response):
        # is response a number or contains a number?
        num = extract_number(response)
        # fuzzy match additionally (song name contains a number?)
        matches = [fuzzy_match(response, item["name"])
                   for item in self.songs.playlist]
        best = max(matches)
        if best > 0.5:
            num = matches.index(best)
        # if not affirmative, dont ask again
        if self.voc_match(response, "no"):
            num = 1
        return num if isinstance(num, int) and \
            num <= len(self.songs.playlist) else False

I think it would be in favour of slim code if instead of response core returns validator(response)

core code: #

#new
else:
        validated = validator(response)
        if validated:
              return response if validated == True else validated

        # catch user saying 'cancel'
        if is_cancel(response):
              return None

changed original, see #

If one requires the original utterance, just pass response (or whatever is set) in the validator

[JarbasAl]

unrelated to the validator proposal, but for your specific example i recommend you look into

    def ask_selection(self, options, dialog='',
                      data=None, min_conf=0.65, numeric=False):
        """Read options, ask dialog question and wait for an answer.
        This automatically deals with fuzzy matching and selection by number
        e.g.
        * "first option"
        * "last option"
        * "second option"
        * "option number four"
        Args:
              options (list): list of options to present user
              dialog (str): a dialog id or string to read AFTER all options
              data (dict): Data used to render the dialog
              min_conf (float): minimum confidence for fuzzy match, if not
                                reached return None
              numeric (bool): speak options as a numeric menu
        Returns:
              string: list element selected by user, or None
        """

also related https://github.com/MycroftAI/mycroft-core/pull/2854

    def ask_yesno(self, prompt, data=None):
        """Read prompt and wait for a yes/no answer
        This automatically deals with translation and common variants,
        such as 'yeah', 'sure', etc.
        Args:
              prompt (str): a dialog id or string to read
              data (dict): response data
        Returns:
              string:  'yes', 'no' or whatever the user response if not
                       one of those, including None
        """

[emphasize]

@offtopic Thanks, but both not fitting the use case. Imagine an album with 20 title spoken to you. In most cases you know the media you're playing, ie the options are known. If necessary i might add {playlistcount} to the initial dialog, to tell the max number.

The point is you use a validator to narrow down the response options, it would make sense that the validator passes the processed validation and not require a do-over.

[krisgesling]

Hey, this is something that has bugged me as well and your proposal sounds pretty good to me personally.

If a validator explicitly returned True (not a "truthy value"), would it be better to return the entire utterance? If you were only interested in an affirmative response then I'd assume you would use ask_yesno() or similar - but this might be a bad assumption to make.

[emphasize]

Not exactly following, but you wouldn't need True in if validated: And you are not reusing the validator return value later in the code (would have to check is True anyway)

If the original response is wanted one can return response in the validator

I'm exactly advocating the "truthy value" since you could do multiple things at once in/with the validator and not sort things out later (again). (Like in the example given "validator excepts a number, sentence containing a number, a fitting song name or a not affirmative answer" and in this case spits out a track number ... or None)

[krisgesling]

I was thinking partly about backwards compatibility because making this change will break anything that currently uses the validator function and returns a boolean, so something like:

return False or None   # validation failed
return True            # validation succeeded - returns whole utterance - matches current behaviour
return truthy_value    # validation succeeded - returns truthy_value

[emphasize]

Gotcha (in the case of returning True in the face of backwards compatability).

Tested it on a repl and the False or None scenario is the same in the former and the new code - it cancels out get_response with None depending on num_retries. (Or one could say or None is neglectable).

so

validated = validator(response)
if validated:
        return response if validated is True else validated

should cover all the bases

[krisgesling]

Yeah I think that works - at least it does in my head.

Interested if anyone else has opinions on this proposal? :+1: Hell yeah :-1: No way :bulb: I like the idea but not sure about the implementation :shrug: Don't really understand or not sure what I think

[NeonDaniel]

I like the backwards-compatible implementation, this does make the return type of get_response unknown (currently Optional[str]). Should validator return be enforced as a str, or left up to the skill dev and just update the spec for get_response?

[krisgesling]

Hmmm - good point.

Personally my first reaction is not to type cast it to a string and leave it up to the person defining the validator. This does break the current spec, but theoretically only for people who are intentionally using it.

[NeonDaniel]

Hmmm - good point.

Personally my first reaction is not to type cast it to a string and leave it up to the person defining the validator. This does break the current spec, but theoretically only for people who are intentionally using it.

I think that makes sense; it makes it a little more difficult to infer types, but it leaves more options open to the skill dev.

[emphasize]

cough HELL YEAH! Should i write a PR to push this?