Adapt: Road to 1.0.0

road-to-1.0.0.md

Overview

Adapt has been largely stable over the last 5 years, with the bulk of effort going into either mycroft-core, or padatious (a c++ ML-based intent parser). Given this stability, it makes sense to finally make a push for 1.0, opening the way to begin development on 2.0, without the baggage of maintaining backwards compatibility.

As part of the effort to get to 1.0, we'll have 3 major workstreams

Open PRs

There are currently 4 open PRs, languishing as far back as 2017. We should be able to complete or close these out quickly.

Open Issues

There are currently 30 open issues against Adapt. I'd like to triage these on the following dimensions

• Needs reproduction
• Can't reproduce, won't fix
• Reproducible, won't fix
• Reproducible, scheduled|in progress
• Fixed
• Deferred post-1.0

I'd also like to add the label of Adapt: Road to 1.0.0 to each of these issues, once they've been triaged.

Document the transition

Speficially, how/why we're transitioning to 1.0.0 to stable/supported, what the future looks like for 1.x, and decide where to branch for 2.0 development.

A couple of things I would like to see addressed...

First, the comments and docstrings need some work. Example,

    Returns key matches to metadata
        This will check every key in query for a matching key in metadata
        returning true if every key is in metadata.  query without keys
        return false.
        Args:
            query(object): metadata for matching
        Returns:
            bool:
                True: when key count in query is > 0 and all keys in query in
                    self.metadata
                False: if key count in query is <= 0 or any key in query not
                    found in self.metadata

What is a key? what is metadata? how are they related? Comments should not be pseudocode. We can all read the code. They should give insight into what the function is doing from a business perspective, and why? This is a somewhat complex library. Some of the code is hard to follow without knowing the context of what it is doing.

The IntentBuilder in its current form results in a lot of duplicate code. Look at the weather skill. there are several intents with "one_of('Forecast', 'Weather')", "optional('Location')", "require('Query')", etc. Isn't there a way this can be done without all the duplication? Also, some of the bigger intents can get very large and somewhat complex. Trying to follow the chain of function calls can be difficult.

1. I'm supportive of seeing some better docstrings here; it looks like there was an effort to improve them about 4 years ago, though they still leave a lot to be desired. My original pass focused on the public-facing interfaces, and the deeper docstrings (in my opinion) don't add a ton of value.
2. Do you have a proposal here? I don't have a substantially better idea for the builder pattern right now, except perhaps to adopt something like the buildPartial that protobuf builder pattern uses. The nature of how mycroft uses the annotations for binding parsers to skills makes it a little less-straightforward how that would look.

If we think there's something pretty trivial that makes it easier to share parts of Intents, I'm open to bringing it in, however I'm reluctant to take on a large refactor as part of trying to cut a v1 release (of otherwise largely stable interfaces).

Oof, reading back into MycroftAI/adapt#57, I'm tempted to revert the whole thing and start over. I must not have looked very closely when I originally approved this PR, but many of the comments are full of typos and/or flat out wrong. They demonstrate a real lack of understanding of the code, which is more problematic than the complete absence of comments.

Sounds like a good approach.

Have you got a preferred place for ideas to be posted if they are deemed to be for v2.0? It sounded like you already had some high level ideas of what that might look like, and potentially ideas here will be fed into that rather than making it into this v1.0

Have you got a preferred place for ideas to be posted if they are deemed to be for v2.0?

v2.0 is not really at a point that's open to community input just yet. My goal is to get 1.0 to a stable state, to allow for 2.0 to break those APIs cleanly. The suggestions that we've heard so far I would consider candidates for a 1.1 branch.

I don't yet have a prototype for the internals of 2.0, just a rough mental model. Until I have that prototype functional, I'm not willing to commit to any externally facing APIs.

An alternative might be to fork to a different library for v2.0, and bring it back iff they seem compatible. As I'm considering internals that are not as cleanly synchronous as adapt is today, that might make more sense. I still see the value in closing out what has been a largely stable interface for 5 years and calling it such. What are your thoughts, @krisgesling?

I'm adding a label for Deferred post 1.0 for a couple of things I've come across; MycroftAI/adapt#17 is old but fits the bill. I'm thinking the biggest chunk of work here right now is the docs revamp, and the more of them I've read the more work I realize there is to be done.