Bug causing .optionally regex to not execute, but it works with .required. IntentBuilder

[InconsolableCellist]

I've observed that when doing a relatively simple regex with the IntentBuilder, the regex doesn't match when I include it with .optionally, but it works with .required.

This is most easily demonstrated with this example code: https://github.com/InconsolableCellist/test-skill/tree/optionally_bug

Steps to reproduce:

1. Clone repo into mycroft skills directory; make sure you're on the optionally_bug branch
2. Make sure mycroft loads it
3. Note line 26 init.py
4. Enter the following utterance to mycroft's debug console: test a by artist
5. Change line init.py:26 to use "optionally" instead of "required"
6. Enter test a by artist again

Expected behavior The skill prints an INFO message: "Artist found! artist" on both steps 4 AND 6

Observed behavior The INFO message is only printed on step 4, not on step 6, indicating that the regex doesn't seem to match when you build the intent by optionally including the regex rather than requiring it.

[MatthewScholefield]

Would you mind creating this issue on the Adapt issue page instead? Also, it sounds similar, but the opposite to this issue.

[clusterfudge]

Hey, sorry for the super delayed response here. I believe that this issue was addressed by a bug in optionally some time ago. A recreation of your issue (using adapt only) yields the following results:

from adapt.intent import IntentBuilder
from adapt.engine import IntentDeterminationEngine

def run_test(required):
    engine = IntentDeterminationEngine()
    engine.register_entity("Test", "TestKeyword")
    engine.register_regex_entity(r"by (?P<Artist>.*)")

    test_intent = IntentBuilder("TestIntent"). \
        require("TestKeyword")
    if required:
        test_intent = test_intent.require('Artist').build()
    else:
        test_intent = test_intent.optionally('Artist').build()
    engine.register_intent_parser(test_intent)

    print(list(engine.determine_intent("test by Artist")))

run_test(True)
run_test(False)

> [{'intent_type': 'TestIntent', 'TestKeyword': 'Test', 'Artist': 'artist', 'target': None, 'confidence': 0.375}]
> [{'intent_type': 'TestIntent', 'TestKeyword': 'Test', 'Artist': 'artist', 'target': None, 'confidence': 1.0}]

[david-morris]

I'm still experiencing this intermittently with adapt-parser 0.4.0. Here's my code (__init__.py excerpt) in my local version of pcwii's cpkodi skill:

...
    @intent_handler(IntentBuilder('WatchPVRChannelNumber').require("WatchKeyword")
                    .require("PVRKeyword").optionally("ChannelNumber").build())
    def handle_channel(self, message):
        if 'ChannelNumber' in message.data:
            self.dLOG("Adapt successfully recognized an optional regex.")
            play_channel_number(self.kodi_path, int(message.data['ChannelNumber']))
            return
        if channel_no := self._match_adapt_regex(message.data['utterance'], "ChannelNumber") is not None:
            self.dLOG("Adapt failed to recognize an optional regex.")
            play_channel_number(self.kodi_path, int(channel_no))
            return
...
    def _match_adapt_regex(self, string, rxfile):
        with open(os.path.join(os.path.dirname(__file__), "regex", self.lang, rxfile + ".rx")) as f:
            self.dLOG("Matching " + string)
            for line in f.readlines():
                self.dLOG(line.strip())
                match = re.match(line.strip(), string)
                if match is not None:
                    return match[rxfile]
            return None

And here's the regex file I'm using, ChannelNumber.rx in the regex/en-us directory:

.*channel (?P<ChannelNumber>\d+)
.*channel number (?P<ChannelNumber>\d+)

And that gives me the following output:

 12:49:31.894 | INFO     | 719574 | __main__:handle_utterance:76 | Utterance: ['watch channel 5']
 12:49:31.922 | INFO     | 719538 | cpkodi-skill_pcwii:dLOG:96 | Matching watch channel 5
 12:49:31.923 | INFO     | 719538 | cpkodi-skill_pcwii:dLOG:96 | .*channel (?P<ChannelNumber>\d+)
 12:49:31.926 | INFO     | 719538 | cpkodi-skill_pcwii:dLOG:96 | Adapt failed to recognize an optional regex.

Then I tried it with this decorator and it worked, but I don't see why:

    @intent_handler(IntentBuilder('').require("WatchKeyword").require("PVRKeyword").optionally("ChannelNumber"))

Is this user error? What can I do to help debug this? Is adapt being deprecated in favor of padatious?

[forslund]

Is channel registered as a keyword as well?

[david-morris]

Ah yes, that is user error then, that's part of PVRKeyword! I suppose I need to use one-of or have the optionally term before the PVRKeyword if regexes still don't consume parts of the utterance.

I still don't understand why it works now, though.

[clusterfudge]

hey @david-morris , I've opened https://github.com/MycroftAI/mycroft-core/issues/2883 against mycroft to request some better tools to help us debug issues like this.

Regex entities in particular are thorny in how they influence ranking, as are optional fields on intents. The combination of the two can result in unexpected behaviors, and without the ability to see the full context of the environment (i.e. mycroft state) I don't believe that they can be fully reproduced or debugged.

[oblitum]

Hey folks, I'm facing a very similar problem and I can't figure how to fix. I have a locale/pt-br/Percent.rx file with (?P<Percent>\d+%). Then I have an intent decorated with @intent_handler(IntentBuilder('BrightnessIntent').one_of('Brighten', 'Darken').require('Target').optionally('Percent')). Inside the intent handler, I never get the matching percent value in message.data, but changing optionally for require makes it present, for the same utterance.

[david-morris]

Hi @oblitum , Could you check out the issue I filed against the Mycroft documentation and add your perspective on the information I posted? I don't see any obvious source of your problem, but I think it's good to have that info when developing intents.

Additionally, as someone new to this community, I'm a bit uncertain about whether we should be posting on Adapt issues when mycroft-core currently uses adapt 0.3.7 and adapt 0.4.0 has been released. Could someone with more experience comment on that?

[forslund]

Good question, I'm not sure what is best. maybe it's better to open on mycroft-core and if it can be determined that it's adapt and not mycroft-core that is at fault it can be moved by maintainers...

But if the issue / question is specific to adapt it may be better to go here directly.

So I'd say...it depends :P

[oblitum]

@david-morris thanks for your input regarding the docs, but I don't think it's a match issue, because actually the one I posted formerly was just one of the attempts, originally it was .*?(?P<Percent>\d+%), then I changed it to .*?(?P<Percent>\d+%).*, then .*(?P<Percent>\d+%).*, .*?(?P<Percent>\d+)%, etc, nothing worked to change the adapt behavior that simply never captured the "Percent" in message.data when it was optionally, and always captured it when it was require.

[oblitum]

I've given up trying to use .optionally and I'm regex matching the message.data['utterance'] directly, it works.

[david-morris]

@oblitum me too. I wrote a method that's been merged into cpkodi for doing this consistently with multiline .rx files.

[clusterfudge]

@oblitum and @david-morris , in a lengthy explanation of some dark corners of adapt to @chrisveilleux , I concluded that there is a failure to meet the principle of least astonishment. I'm finally tackling this with #137 (and fixing a few other latent bugs), and I'd encourage you to try the latest version when it gets merged!