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

New Amazon Rules and Intents #1

Merged
merged 4 commits into from Dec 20, 2016

Conversation

Projects
None yet
2 participants
@bleonard
Contributor

bleonard commented Dec 2, 2016

Some changes have been made by Amazon

  • Slots now have AMAZON prefix
  • Only AMAZON.LITERAL are supposed to include sample utterances
  • They have also added new intents like "CancelIntent" and such
@sidoh

This is awesome! Thanks for doing this.

Left a few minor suggestions (nothing that'd affect the interface).

Couple of questions re: Amazon's update. I've been a little out of the loop.

  • Seems like they're deprecating LITERAL in February 2017 in favor of custom slot types. Seems like this would: (1) obviate the need to expand the cartesian product of slot values, (2) require support to spit out values for added slot types. Does this sound right?
  • Seems like they added a bunch of builtin slot types (like AMAZON.US_FIRST_NAME).

It probably makes most sense to make these changes seperately, but wanted to get your thoughts.

@@ -3,6 +3,26 @@
module AlexaGenerator
class Intent
module IntentType

This comment has been minimized.

@sidoh

sidoh Dec 3, 2016

Owner

What do you think about naming this AmazonIntentType? Might make it more obvious that this isn't an exhaustive list of possible intent types.

@sidoh

sidoh Dec 3, 2016

Owner

What do you think about naming this AmazonIntentType? Might make it more obvious that this isn't an exhaustive list of possible intent types.

This comment has been minimized.

@bleonard

bleonard Dec 13, 2016

Contributor

Sure. I want following the SlotType pattern.
I also saw there might be other intent types and maybe there would be different namespaces.
https://developer.amazon.com/blogs/post/Tx2EWC85F6H422/Introducing-the-ASK-Built-in-Library-Developer-Preview-Pre-Built-Models-for-Hund

    "intent": {

      "name": "SearchAction<object@WeatherForecast>",

      "slots": {}

    }
@bleonard

bleonard Dec 13, 2016

Contributor

Sure. I want following the SlotType pattern.
I also saw there might be other intent types and maybe there would be different namespaces.
https://developer.amazon.com/blogs/post/Tx2EWC85F6H422/Introducing-the-ASK-Built-in-Library-Developer-Preview-Pre-Built-Models-for-Hund

    "intent": {

      "name": "SearchAction<object@WeatherForecast>",

      "slots": {}

    }
}
end
}
out = { intents: [] }

This comment has been minimized.

@sidoh

sidoh Dec 3, 2016

Owner

Any reason to not leave this as a map over @intents.values?

@sidoh

sidoh Dec 3, 2016

Owner

Any reason to not leave this as a map over @intents.values?

This comment has been minimized.

@bleonard

bleonard Dec 13, 2016

Contributor

we tend to prefer doing it the old fashioned way. been burned by the collects and maps and rejects and such with minor misunderstandings and magic.
Do whatever you want, of course.

@bleonard

bleonard Dec 13, 2016

Contributor

we tend to prefer doing it the old fashioned way. been burned by the collects and maps and rejects and such with minor misunderstandings and magic.
Do whatever you want, of course.

This comment has been minimized.

@sidoh

sidoh Dec 20, 2016

Owner

Fair enough. This is probably at least bordering excess nested ruby magic. I'll leave it split out.

@sidoh

sidoh Dec 20, 2016

Owner

Fair enough. This is probably at least bordering excess nested ruby magic. I'll leave it split out.

}
end
if slots.size > 0 || !intent.name =~ /^AMAZON/

This comment has been minimized.

@sidoh

sidoh Dec 3, 2016

Owner

What do you think about adding a amazon_intent? method to Intent instead of regexing? Could use the following enum-ish pattern for AmazonIntentType to make it a one-liner:

module AmazonIntentType
  VALUES = [
    CANCEL = :"AMAZON.CancelIntent",
    ...
  ]
end

(can then reference AmazonIntentType::VALUES for all possible values).

@sidoh

sidoh Dec 3, 2016

Owner

What do you think about adding a amazon_intent? method to Intent instead of regexing? Could use the following enum-ish pattern for AmazonIntentType to make it a one-liner:

module AmazonIntentType
  VALUES = [
    CANCEL = :"AMAZON.CancelIntent",
    ...
  ]
end

(can then reference AmazonIntentType::VALUES for all possible values).

utterances = Set.new
templates.each do |template|
# Consider only the slots that are referenced in this template
relevant_slots = template.referenced_slots
# Amazon wants only the LITERAL ones
relevant_slots.select! { |s| slot_types[s.to_sym] =~ /LITERAL/ }

This comment has been minimized.

@sidoh

sidoh Dec 3, 2016

Owner

Might suggest a literal? helper on Slot::SlotType instead of the regex.

@sidoh

sidoh Dec 3, 2016

Owner

Might suggest a literal? helper on Slot::SlotType instead of the regex.

This comment has been minimized.

@bleonard

bleonard Dec 13, 2016

Contributor

👍

@bleonard

bleonard Dec 13, 2016

Contributor

👍

}
end
if slots.size > 0 || !intent.name =~ /^AMAZON/

This comment has been minimized.

@sidoh

sidoh Dec 3, 2016

Owner

Just tested on one of my dev skills -- it doesn't seem like Amazon considers it an error to specify an empty slots array for Amazon intent types (or any slot-less intent). If that's the case, I think it makes sense to return a hash with an empty array for the :slots key when there are no slots.

@sidoh

sidoh Dec 3, 2016

Owner

Just tested on one of my dev skills -- it doesn't seem like Amazon considers it an error to specify an empty slots array for Amazon intent types (or any slot-less intent). If that's the case, I think it makes sense to return a hash with an empty array for the :slots key when there are no slots.

This comment has been minimized.

@bleonard

bleonard Dec 13, 2016

Contributor

all the samples just had the string. and no slots. I thought it was nice to keep it tight.

@bleonard

bleonard Dec 13, 2016

Contributor

all the samples just had the string. and no slots. I thought it was nice to keep it tight.

}
end
if slots.size > 0 || !intent.name =~ /^AMAZON/

This comment has been minimized.

@sidoh

sidoh Dec 3, 2016

Owner

Might make sense to have Intent::Builder fail loudly when trying to add a slot for an Amazon slot type. That way we could avoid the conditional here. I think it could be confusing to have this code silently filter out slots that were added erroneously.

@sidoh

sidoh Dec 3, 2016

Owner

Might make sense to have Intent::Builder fail loudly when trying to add a slot for an Amazon slot type. That way we could avoid the conditional here. I think it could be confusing to have this code silently filter out slots that were added erroneously.

This comment has been minimized.

@bleonard

bleonard Dec 13, 2016

Contributor

that sounds fine too.

@bleonard

bleonard Dec 13, 2016

Contributor

that sounds fine too.

}
end
if slots.size > 0 || !intent.name =~ /^AMAZON/

This comment has been minimized.

@sidoh

sidoh Dec 3, 2016

Owner

If we're OK with those suggestions, think this particular bit of code would remain the same. Changes would be pushed into Intent::Builder.

@sidoh

sidoh Dec 3, 2016

Owner

If we're OK with those suggestions, think this particular bit of code would remain the same. Changes would be pushed into Intent::Builder.

@sidoh

This comment has been minimized.

Show comment
Hide comment
@sidoh

sidoh Dec 13, 2016

Owner

Unless there are objections, I'll merge these changes into a branch, apply the few changes in the comments and merge. Thanks again for adding support for the updates!

Owner

sidoh commented Dec 13, 2016

Unless there are objections, I'll merge these changes into a branch, apply the few changes in the comments and merge. Thanks again for adding support for the updates!

@bleonard

This comment has been minimized.

Show comment
Hide comment
@bleonard

bleonard Dec 13, 2016

Contributor

Sorry for just dumping it and running @sidoh - I'm completely fine with whatever you want to do. I'll respond to some of the above in case it's helpful.

Contributor

bleonard commented Dec 13, 2016

Sorry for just dumping it and running @sidoh - I'm completely fine with whatever you want to do. I'll respond to some of the above in case it's helpful.

@bleonard

as you like, just noting thought processes during dev.

@@ -3,6 +3,26 @@
module AlexaGenerator
class Intent
module IntentType

This comment has been minimized.

@bleonard

bleonard Dec 13, 2016

Contributor

Sure. I want following the SlotType pattern.
I also saw there might be other intent types and maybe there would be different namespaces.
https://developer.amazon.com/blogs/post/Tx2EWC85F6H422/Introducing-the-ASK-Built-in-Library-Developer-Preview-Pre-Built-Models-for-Hund

    "intent": {

      "name": "SearchAction<object@WeatherForecast>",

      "slots": {}

    }
@bleonard

bleonard Dec 13, 2016

Contributor

Sure. I want following the SlotType pattern.
I also saw there might be other intent types and maybe there would be different namespaces.
https://developer.amazon.com/blogs/post/Tx2EWC85F6H422/Introducing-the-ASK-Built-in-Library-Developer-Preview-Pre-Built-Models-for-Hund

    "intent": {

      "name": "SearchAction<object@WeatherForecast>",

      "slots": {}

    }
}
end
}
out = { intents: [] }

This comment has been minimized.

@bleonard

bleonard Dec 13, 2016

Contributor

we tend to prefer doing it the old fashioned way. been burned by the collects and maps and rejects and such with minor misunderstandings and magic.
Do whatever you want, of course.

@bleonard

bleonard Dec 13, 2016

Contributor

we tend to prefer doing it the old fashioned way. been burned by the collects and maps and rejects and such with minor misunderstandings and magic.
Do whatever you want, of course.

}
end
if slots.size > 0 || !intent.name =~ /^AMAZON/

This comment has been minimized.

@bleonard

bleonard Dec 13, 2016

Contributor

all the samples just had the string. and no slots. I thought it was nice to keep it tight.

@bleonard

bleonard Dec 13, 2016

Contributor

all the samples just had the string. and no slots. I thought it was nice to keep it tight.

}
end
if slots.size > 0 || !intent.name =~ /^AMAZON/

This comment has been minimized.

@bleonard

bleonard Dec 13, 2016

Contributor

that sounds fine too.

@bleonard

bleonard Dec 13, 2016

Contributor

that sounds fine too.

@sidoh sidoh merged commit 44ced6e into sidoh:master Dec 20, 2016

@sidoh

This comment has been minimized.

Show comment
Hide comment
@sidoh

sidoh Dec 20, 2016

Owner

Released the changes in 0.2.0. Thanks again @bleonard!

Owner

sidoh commented Dec 20, 2016

Released the changes in 0.2.0. Thanks again @bleonard!

@bleonard

This comment has been minimized.

Show comment
Hide comment
@bleonard

bleonard Dec 20, 2016

Contributor

💥

Contributor

bleonard commented Dec 20, 2016

💥

@bleonard

This comment has been minimized.

Show comment
Hide comment
@bleonard

bleonard Dec 20, 2016

Contributor

PR #1
Congratulations!

Contributor

bleonard commented Dec 20, 2016

PR #1
Congratulations!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment