Skip to content
This repository was archived by the owner on Jan 5, 2026. It is now read-only.

Conversation

@chrimc62
Copy link

@chrimc62 chrimc62 commented Apr 14, 2021

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.

@coveralls
Copy link
Collaborator

coveralls commented May 4, 2021

/// <summary>
/// The context label for a LUIS trace activity.
/// </summary>
public const string LuisTraceLabel = "LuisV3 Trace";
Copy link
Contributor

@tomlm tomlm May 12, 2021

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)
{
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wha? Why?

Copy link
Author

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") });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new EntityDescription("boolean")

Do we need to capture the forms of boolean? Yes/NO/OK/Cancel/Nope, etc.?

Copy link
Author

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; }
Copy link
Contributor

@tomlm tomlm May 12, 2021

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"

Copy link
Author

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RecognizerDescription

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.

Copy link
Author

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/>
Copy link
Contributor

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?

Copy link
Author

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?

Copy link
Contributor

@tomlm tomlm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🕐

@gabog
Copy link
Contributor

gabog commented Mar 22, 2022

We will not address this at the moment.

@gabog gabog closed this Mar 22, 2022
@tracyboehrer tracyboehrer deleted the chrimc/priming branch July 14, 2022 15:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable speech priming in adaptive dialogs

4 participants