Tools for debugging intent parsing issues in mycroft

[clusterfudge]

There are currently a number of issues open against Adapt that reference mycroft-core concepts (skills, vocab files, etc). It's extremely difficult to diagnose (or even reproduce) these issues outside of the context of a fully spun-up mycroft instance, including all skills installed, all vocab registered, reload/restart state, and interaction with padatious.

In order to better help users of Adapt (and mycroft!), we need to build some better logging (and potentially tooling).

As a first pass, there should be an intent-parsing debug mode that logs the following:
All tagged entities from the utterance
All context state
All possible parse results
All valid parse results (and the intents they matched to)
All potential intents (in order that they would be generated from IntentDeterminationEngines)
Which intent was selected (if any) and its source parser (Adapt vs Padatious).

A longer term goal might be build a state-dump tool that can be shared for easier debugging across developers. There are potentially some data privacy concerns with this tool, and I don't immediately want to unpack that can of worms.

Assuming there's no objections to the existence of this tool, there's a few Adapt issues that I'll mark as blocked by this.

[forslund]

Most of these would be fairly easy to implement as logging. Could technically be setup into a separate log file but maybe the skill log would suffice?

By "All possible results" do you mean all registered intent handlers?

The ones that I'd consider to be tricky to implement are "All valid parse results (and the intents they matched to)" and "All possible parse results". These would require some instrumentation from Adapt and Padatious to know what they'd considered and discarded.

[clusterfudge]

As a start, it looks like the adapt Parser emits all of the tagged entities, which would include matching span info. Since the expander is stateless, it should be straight forward to recreate the rest of the parse result from there (will require some adapt/mycroft tooling to wire it all together).

That wouldn't provide insight when there are multiple intents or parsers in play (and padatious may not even have similar internal concepts).

[forslund]

A right that looks usable enough. I expect we'd need to ask each of the different types to provide their own set of debug info in their match response, since the capabilities varies between adapt, padatious, converse and fallback. converse and fallback aren't quite as complete and will be a bit harder to get good debug info from but adapt and padatious could easily append a well-formatted debug structure to the match response.

[forslund]

Did start to collect some info from adapt:

Utterance: what's the time
 [Context is empty]

Tagged entities:
what
- mycroft_pandora_mycroftaiQuery
- mycroft_configuration_mycroftaiQuery
- mycroft_npr_news_mycroftaiGive
- mycroft_timer_mycroftaiQuery
- mycroft_alarm_mycroftaiQuery
- mycroft_ip_mycroftaiquery
- skill_queryQuestion
- mycroft_date_time_mycroftaiQuery
- mycroft_volume_mycroftaiQuery
the
- mycroft_timer_mycroftaiConnector
time
- mycroft_date_time_mycroftaiTime
- mycroft_timer_mycroftaiTime
Matched intents:
Match: mycroft-date-time.mycroftai:handle_query_time Confidence: 0.6666666666666666


Utterance: what is the time
 [Context is empty]

Tagged entities:
what
- mycroft_pandora_mycroftaiQuery
- mycroft_configuration_mycroftaiQuery
- mycroft_npr_news_mycroftaiGive
- mycroft_timer_mycroftaiQuery
- mycroft_alarm_mycroftaiQuery
- mycroft_ip_mycroftaiquery
- skill_queryQuestion
- mycroft_date_time_mycroftaiQuery
- mycroft_volume_mycroftaiQuery
what is
- mycroft_version_checker_mycroftaiCheck
- fallback_unknown_mycroftaiquestion
- mycroft_alarm_mycroftaiQuery
- mycroft_configuration_mycroftaiQuery
the
- mycroft_timer_mycroftaiConnector
time
- mycroft_date_time_mycroftaiTime
- mycroft_timer_mycroftaiTime
Matched intents:
Match: mycroft-date-time.mycroftai:handle_query_time Confidence: 0.6666666666666666

[forslund]

Some interesting things turned up from the weather:

Utterance: what is the weather like in stockholm
[...]
stockholm
- mycroft_weather_mycroftaiLocation
- mycroft_weather_mycroftaiLocation
- mycroft_weather_mycroftaiLocation
- mycroft_weather_mycroftaiLocation
- mycroft_weather_mycroftaiLocation
- mycroft_date_time_mycroftaiLocation
- mycroft_weather_mycroftaiLocation

Lots of entity matches...for the location regexp

Edit: the removal on skill reload PR hasn't been merged so it can be an artifact of me reloading the skill)

[clusterfudge]

I think this is a mistake with the Location regex, probably in a few different skills.

Regex entities are run against all possible subsequences of an utterance, meaning that they explicitly should not need leading and/or trailing wildcards. I believe the trailing wildcard is necessary here (as we're using that to match on location). This probably negatively impacts accuracy across a wide variety of skills. I'd recommend changing this regex (and other language-specific ones) to something like the following:
\b(at|in) (?P<Location>(?!\bcelsius\b|\bfahrenheit\b).*)

I'll try to craft a proof-of-concept directly in adapt to confirm that regex is correct tomorrow if I have a moment, otherwise I'd recommend you do so @forslund .

[clusterfudge]

It's also worth calling out the en-us reged in date-time skill does not have this overmatching problem:

https://github.com/MycroftAI/skill-date-time/blob/fdec596ec93638f30d04d01d7b2d3ce432fe732a/regex/en-us/location.rx

[forslund]

Ah yes, the leading .* there. I wonder if it would be even better to have

\b(at|in) (?P<Location>(?!\bcelsius\b|\bfahrenheit\b).+)

with .+ so the empty string can't be matched. Just tested and with either of above changes it is reduced to the expected 1 match.

Anyways, the debug tool already finding issues, so a success I'd say :)