-
Notifications
You must be signed in to change notification settings - Fork 492
Conversation
Add recognizerSet.
Start hooking up adaptive inputs.
Changed LUIS to include canonicalForm in synonyms.
…ce, but am going to change to put it directly in the activity.
| /// <summary> | ||
| /// The context label for a LUIS trace activity. | ||
| /// </summary> | ||
| public const string LuisTraceLabel = "LuisV3 Trace"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
most definitely NOT here. #Resolved
| /// <param name="activity">Activity to modify.</param> | ||
| protected virtual void AddContext(DialogContext dc, Activity activity) | ||
| { | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wha? Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because Ask derives from this and I needed a way to add the context. The other alternative was to duplicate BeginDialogAsync or pull all of it out into a common method. Do you have a preference?
|
|
||
| /// <inheritdoc/> | ||
| public override RecognizerDescription GetRecognizerDescription(DialogContext dialogContext, string expectedLocale) | ||
| => new RecognizerDescription(entities: new[] { new EntityDescription("boolean") }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. This corresponds to the boolean prebuilt in luis or the confirmation in speech.
| /// Gets the expected locale. | ||
| /// </summary> | ||
| /// <value>Locale.</value> | ||
| public string Locale { get; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be here? How does this relate to activity.locale, turn.locale, user.locale, etc?
Or is the intent that I say in english,
"OK, let's talk in spanish" and so activity.locale ="en" and activity.inputcontext.locale = 'es' ?
Seems like I would simply say
"Yo habo en espanol", activity.locale = "es"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the expected locale of the input which priming needs and is the simplest kind of priming. It is usually the dialog context locale, but happy to use another one. There was also some desire to have a self-contained package.
| /// <summary> | ||
| /// Description of the intents and entities a recognizer can return. | ||
| /// </summary> | ||
| public partial class RecognizerDescription |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a fan of "Description" name...RecognitionReference? Does it require it to be broken down into intents and entities? Isn't the name + source enough for it to know that? That would simplify and also not bake in LU concepts of intents and entities that may change in the fuiture.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have intents and entities everywhere in the SDK, recognizers, composer, ... You want to introduce a new concept? Why is Reference better than description? You would have a single list of Recognitions?
| [JsonProperty("possibleEntities")] | ||
| public ArrayExpression<EntityDescription> PossibleEntities { get; set; } | ||
|
|
||
| /// <inheritdoc/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to break down to intents and entities?
Why not just an array of ids?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the ids appear in intents or entities when you do recognition so if you want to find them you have to look in both places. Why the allergy to intents/entities?
tomlm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🕐
|
We will not address this at the moment. |
Fixes: #3402
DO NOT REVIEW THIS ONE FOR NOW. Look at chrimc/priming2 PR instead. I'm keeping this in case people want to look at the prior approach.
This is the PR for adding dynamic priming to SDK.