[DCR] Make ValueRecognizer internal to AdaptiveDialog

[tomlm]

Currently we use a ValueRecognizer which recognizes values sent back from a card by looking for

Activity.Type == Message & String.IsNullOrEmpty(Activity.Text) && activity.Value != null

When that is seen, we return an "Intent" and map the value into entities. The issue with this approach is that it a. forces you to always use RecognizerSet b. always using recognizerSet forces Composer to build UX for recognizerSet c. There is ambiguity for Composer to deal with multiple recognizers d. It is something the developer now needs to know to add.

The much simpler solution to the problem is to add a new trigger type OnValueReceived which has the constraint built in.

OnValueSubmitted
    Condition = define extra constraint

Usage would be something like this:

{
    "$kind": "Microsoft.OnValueReceived ",
     Condition = "turn.activity.value.form == 'MyForm'"
    actions = [ ... ]
}

This aligns with existing Composer IA and is more natural with the rest of the triggers.

We should also add Microsoft.OnAttachmentReceived for modeling receiving attachments.

[vishwacsena]

@tomlm @Stevenic Not a fan of this proposal. My thoughts -

1. I'm not sure how using triggers make it easy for users to understand what's going on. One of the use cases for doing what we do today with value recognizer is for input actions. With the current solution, we always run the parents recognizer because we need that to be done before evaluating the AllowInterruption expression. In this pre-bubble phase, we do not evaluate triggers unless the allow interruption expression evaluates to true. Removing the value recognizer and instead doing it as triggers would mean the user would need to know to set AllowInterruption to true to even hit this trigger.
2. The value recognizer is only enabled for adaptive cards AFAIK and it is super easy to consume the card payload via intent + entities. If you are supporting entity extraction through an LU recognizer, then an adaptive card interaction also flows through the same OnIntent path. Having a separate trigger makes it harder to understand and use only when there is a card.
3. Do we have feedback from Composer team that this is hard? I had worked with Lu Han to have composer always write out a recognizer set with value recognizer and a cross-trained recognizer set. In fact, luis:build can just automatically do this and we can always enable the value recognizer via a recognizer set even if you use a regex recognizer.
4. We should try and keep recognition and triggers as separate phases. Processing responses from cards is just one other input rather than textual responses. Soon enough we will have inputs from other modalities that need to be processed into a recognition result (e.g. gaze, geolocation data etc). Having a model where we have them all come through triggers seem to be overloading triggers and their purpose.

[vishwacsena]

@cwhitten can you comment from composer side as well? Is using a recognizer set an issue for composer?

[tomlm]

There is nothing to recognize. ValueRecognizer is artificially mapping activity.value => intent, entities=value.

It's artifical. We don't need it.

This issue came from Chris, ValueRecognizer forces composer to have to deal with multiple recognizers with cascading problems.

[cwhitten]

@vishwacsena the RecognizerSet requires non-trivial change to Composer's IA in both assigning recognizers, managing the list, and configuring different authoring views. A user needs this context to be effective in using the concept, and we just haven't worked through the design yet. We will probably get to this in R10 as part of the inline Qna authoring feature.

What we discussed is that we could have the "default" template for a new recognizer on a dialog include the ValueRecognizer, but again it's not immediately clear to a user what this is for or how it works, and going through the scenario that it supports (rich card form submissions) it wasn't clear why this particular form submission required running through the recognition phase. It all seems very deterministic and static. The consensus on the call was that a trigger would do the job.

I see your point about consistency around other modalities: gaze, geolocation data, etc. You feel strongly that form submissions be a part of that?

[vishwacsena]

@tomlm I still cannot get the non-value recognizer route to work as a trigger and in general I still dislike the idea of removing the value recognizer. I just tried 3 different scenarios with adaptive cards

With ValueRecognizer

1. Using adaptive card buttons to route to different OnIntent triggers. See here
2. Using adaptive cards to get single input bound to a property on an input action. See here
3. Using adaptive card to get multiple inputs bound to properties on more than one input action. See here

With the trigger route proposed in this DCR

1. Using adaptive card buttons to route to different OnIntent triggers. See here
2. Using adaptive cards to get single input bound to a property on an input action. See here
3. Using adaptive card to get multiple inputs bound to properties on more than one input action. See here

Observations:

1. The simplest of these cases (1) above would require users to understand concept of EmitEvent to get simple routing to an existing OnIntent trigger. Do we think this will fly for composer users? The push back would be that we could tuck away this as a template (like we do for OnConversationUpdate) but this feels more involved than doing a loop and an if condition to me. Even so, @tomlm I still cannot get this block to work. I suspect RecoginzedIntent event needs a full recognition result?

new EmitEvent()
{
    EventName = AdaptiveEvents.RecognizedIntent,
    EventValue = "=json(`{\"intent\", ${turn.activity.value.intent}}`)"
}

2. In the case where you are looking to accept either the card input or LU recognizer picking up an entity, you need to do this

With ValueRecognizer	Without
Value = "=@email"	Value = "=coalesce(@email, turn.activity.value.email)"

3. In cases where you need to handle multiple inputs coming back from the card, you need to add more than one eventTrigger each pegged on an intent name which anyways is in the adaptive card immaterial of using value recognizer or not.

new OnMessageActivity()
{
    Condition = "turn.activity.text == null && turn.activity.value != null && turn.activity.value.intent == 'userProfileAdaptiveCardInput'"
    Actions = new List<Dialog>()
    {
        new SendActivity("Setting values..."),
        new SetProperties()
        {
            Assignments = new List<PropertyAssignment>()
            {
                new PropertyAssignment()
                {
                    Property = "user.email",
                    Value = "=turn.activity.value.email"
                },
                new PropertyAssignment()
                {
                    Property = "user.age",
                    Value = "=turn.activity.value.number"
                }
            }
        }
    }
}

@tomlm @stevenic I have done due diligence to actually render what the end user will experience with or without ValueRecognizer would be. If we can manage setting up the ValueRecognizer in composer (which I thought Lu Han had a proposal for), then my recommendation would be to not take this DCR. Even if we have a different solution for composer, I'd still vote to keep ValueRecognizer.

@cwhitten is it a goal for composer user to understand what's persisted as recognizer? If so, do we plan to explain what we do for luis:build and the generated folder etc? I do not view card input as being separate from any other type of input. I believe the right frame of mind here is to have all inputs (type, speak, card, geolocation, gaze etc) have their own recognizer and have dialog only deal with a recognition event.

[tomlm]

Thank you for all of the due diligence you have done on this.

Some comments:

Regarding 1 EmitEvent

EmitEvent is bad pattern for reuse.

You are trying to use EmitEvent for code reuse.  Why?  For any case more complex then sending a single response you would create a dialog for it.  aka BookAFlight dialog.  

This would then simply be:

OnIntent()  intent:  "BookADialog"=> 
              BookADialog(@start, @end,...);

OnValueReceived() => value.action = "BookADialog" =>
              BookADialog(value.start, value.end,...);

That feels clean and natural.  Emitting a Recognized event feels 500 level and probably even bad practice.

Put another way...in early windows programming people would write code immediately on the switch statement:

switch(message.type)
{
     case WM_CLICK: 
        ....code...
        break;
}

Over time that pattern has been completely replaced with simple mapping

switch(message.type)
{
     case WM_CLICK:  OnClick()
                 break;
}

This separates the eventing from the logic and allows you to call the logic independent of the eventing, Put another way, you don't and shouldn't pump an artificial event into the message pump to trigger OnClick().

InputDialog.Value case For the InputDialog value case I get the simplicity of the model you are suggesting.  One of the motivators of moving the ValueRecognizer out of AdaptiveDialog was to not have to have it baked into AdaptiveDialog for the rest of eternity.

Alternate Idea

Perhaps we should build ValueRecognition into the base Recognizer class, and it would recognize Entities, Values, Attachments and other things and could evolve over time.

RegExRecognizer/LuisRecognizer then would do something like this.

override RecognizerResult Recognize(Activity activity, ...)
{
   if (String.IsNullOrEmpty(activity.Text))
   {
          return base.Recognize(activity);
   }
   ...

The default recognizer for an AdaptiveDialog would be `new Recognizer()

RecognizerResult Recognizer.Recognize() 
{
  new RecognizerResult()
 {   
     Intent = activity.Name ?? activity.semanticAction ?? activity.value.intent ?? "None"
   Entities = union(activity.Entities, activity.value)
   };
}

Definitely we need to update the composer to handle recognizerSet concepts, but we don't need to introduce that complexity just for this feature.

[vishwacsena]

Thanks @tomlm! With use case 1, my intention as the author of a bot is very simple. I have already a working bot with LU. How can have the card input take the same route with minimal work from my side. You can also flip this on the head and say users typically want to start with a simple card based bot and then want to add LU. I do agree that in cases where I have logic tucked away in a dialog, BeginDialog would work but hopefully we are not saying that you must always tuck any set of actions (apart from a single send activity) into a dialog. Even so, we still need to solve for cases where the card needs to dispatch to a single sendActivity action (e.g. Help scenario). Hopefully I do not need to wrap each one of those into its own dialog.

I like the alternate idea in that we need a default recognizer for adaptive dialogs in composer. By default, could composer just include the ValueRecognizer? This way you can build bots with cards and not have to worry about managing or changing any recognizer configuration.. We'd also not have to deal with a recognizer set in the default case in composer. If and when user adds either a LU or Regex recognizer, then we can simply under the cover swap the default ValueRecognizer with a RecognizerSet. Would that work?

May be we can rename ValueRecognizer to AdaptiveCardInputRecognizer to be more clear on its function?

Also, attachments AFAIK do not come through Value property on the activity but instead via Attachments property - right?

[boydc2014]

late on the party, just my two cents

1. Generally, though i know how recognizer set will introduce non-trivial change to Composer, i don't think making Composer's life easier is a good motivation to some fundamental changes in SDK. SDK could definitely use Composer as a valuable source of feedback, but if i'm about to make significant design change, i would ask myself, am i designing for making Composer easier or am i using Composer's feedback to drive me rethink a better model that makes more sense. Those two are different goals sometimes.

2. Recognition phrase is supposed to positioned different from the selection phrase (both the pre-pre-recognition selection and after recognition selection, where triggers are designed for). So the first and perhaps the only question is is card processing a recognition problem? I would vote for yes,

   The simplest scenario i have, when i try to build bot is, i definitely want to treat user text, voice and button clicking in the same way, it's more clean to have one single place to define "what my bot want to do if user's intention is bookflight" (creating a onIntent for this), then have somewhere else to decide "how do i know user's intention is bookflight". In the simplest case, i want when user say "book flight" have the same effect with "clicking book flight button", creating a different trigger seems not a reasonable requirement.

3. I like the alternative, if i understand correctly, is that embed the value recognizer into certain recognizer. I like because we still model the problem is a recognition problem.

4. Just some brain storming, i would vision Composer will create a certain level of abstraction on top of SDK in this recognition problem. I mean, Composer not have to real expose the same structure it write down to disk. I would image perhaps in the Managed-Lu experience, where an input edited, we could provide some check box, let's say "enabling card", and composer take over the serialization and feeding SDK.

   By saying that, the direction makes most sense to me, is Composer should be free to pursue higher-lever abstraction that make close to user, and convert that into SDK implementation. We don't have to tight Composer close to SDK, perhaps we shouldn't. To be specific, SDK have a ValueRecognizer don't automatically equals to Composer's complexity is increased.

[cwhitten]

Perhaps we should build ValueRecognition into the base Recognizer class, and it would recognize Entities, Values, Attachments and other things and could evolve over time.

RegExRecognizer/LuisRecognizer then would do something like this.

override RecognizerResult Recognize(Activity activity, ...)
{
   if (String.IsNullOrEmpty(activity.Text))
   {
          return base.Recognize(activity);
   }
   ...

The default recognizer for an AdaptiveDialog would be `new Recognizer()

RecognizerResult Recognizer.Recognize() 
{
  new RecognizerResult()
 {   
     Intent = activity.Name ?? activity.semanticAction ?? activity.value.intent ?? "None"
   Entities = union(activity.Entities, activity.value)
   };
}

Definitely we need to update the composer to handle recognizerSet concepts, but we don't need to introduce that complexity just for this feature.

I like this idea and think we should pull on this a bit. Can we model the adaptive card form submission directly into the recognizer? It begs the question if we want to provide it by default for all dialogs with recognition configured. When would one not want set up for them?

Something we should also consider is other Adaptive Card behavior that isn't just form submissions. How will this work with Adaptive Cards v2 and the invoke/refresh capabilities? We will want to allow users to hook into that processing as well. I feel like this may fall into triggers/events, however we should have line of sight on this before we make a decision.

@boydc2014, some comments:

1. Generally, though i know how recognizer set will introduce non-trivial change to Composer, i don't think making Composer's life easier is a good motivation to some fundamental changes in SDK. SDK could definitely use Composer as a valuable source of feedback, but if i'm about to make significant design change, i would ask myself, am i designing for making Composer easier or am i using Composer's feedback to drive me rethink a better model that makes more sense. Those two are different goals sometimes.

I'm not sure is this if directed at something more broad or if your are criticizing this particular discussion. I wouldn't characterize this as a fundamental change in the SDK. In fact, the proposed trigger behavior is more like the current behavior as I hear it from Steve and Tom. "am i using Composer's feedback to drive me rethink a better model that makes more sense" is exactly what we are doing.

By saying that, the direction makes most sense to me, is Composer should be free to pursue higher-lever abstraction that make close to user, and convert that into SDK implementation. We don't have to tight Composer close to SDK, perhaps we shouldn't. To be specific, SDK have a ValueRecognizer don't automatically equals to Composer's complexity is increased.

This is the position we've held since day 1, however the resulting abstractions need to be a direct composition of the SDK primitives. We aim to reduce friction and make the simple things easy. Card form submission is a simple thing, and it should be easy.

To capture more succinctly what I'd like to understand better if we move forward with the ValueRecognizer:

1. Can we hide it completely from the user and bake it into the base recognition logic?
2. How will additional Adaptive Card v2 behavior ie. refreshing impact the model?
3. How will non-Adaptive Card value selections benefit from this, i.e. HeroCard? If this is positioned as a mechanism to handle card value submissions, it makes sense that HeroCard's should benefit from it.

[boydc2014]

Perhaps we should build ValueRecognition into the base Recognizer class, and it would recognize Entities, Values, Attachments and other things and could evolve over time. RegExRecognizer/LuisRecognizer then would do something like this.

override RecognizerResult Recognize(Activity activity, ...)
{
   if (String.IsNullOrEmpty(activity.Text))
   {
          return base.Recognize(activity);
   }
   ...

The default recognizer for an AdaptiveDialog would be `new Recognizer()

RecognizerResult Recognizer.Recognize() 
{
  new RecognizerResult()
 {   
     Intent = activity.Name ?? activity.semanticAction ?? activity.value.intent ?? "None"
   Entities = union(activity.Entities, activity.value)
   };
}

Definitely we need to update the composer to handle recognizerSet concepts, but we don't need to introduce that complexity just for this feature.

I like this idea and think we should pull on this a bit. Can we model the adaptive card form submission directly into the recognizer? It begs the question if we want to provide it by default for all dialogs with recognition configured. When would one not want set up for them?

Something we should also consider is other Adaptive Card behavior that isn't just form submissions. How will this work with Adaptive Cards v2 and the invoke/refresh capabilities? We will want to allow users to hook into that processing as well. I feel like this may fall into triggers/events, however we should have line of sight on this before we make a decision.

@boydc2014, some comments:

1. Generally, though i know how recognizer set will introduce non-trivial change to Composer, i don't think making Composer's life easier is a good motivation to some fundamental changes in SDK. SDK could definitely use Composer as a valuable source of feedback, but if i'm about to make significant design change, i would ask myself, am i designing for making Composer easier or am i using Composer's feedback to drive me rethink a better model that makes more sense. Those two are different goals sometimes.

I'm not sure is this if directed at something more broad or if your are criticizing this particular discussion. I wouldn't characterize this as a fundamental change in the SDK. In fact, the proposed trigger behavior is more like the current behavior as I hear it from Steve and Tom. "am i using Composer's feedback to drive me rethink a better model that makes more sense" is exactly what we are doing.

By saying that, the direction makes most sense to me, is Composer should be free to pursue higher-lever abstraction that make close to user, and convert that into SDK implementation. We don't have to tight Composer close to SDK, perhaps we shouldn't. To be specific, SDK have a ValueRecognizer don't automatically equals to Composer's complexity is increased.

This is the position we've held since day 1, however the resulting abstractions need to be a direct composition of the SDK primitives. We aim to reduce friction and make the simple things easy. Card form submission is a simple thing, and it should be easy.

@cwhitten, just to be clear by my #1 comment or any comments, not meant to criticizing any body or any behavior here, we are definitely having a good conversation that result into solid follow ups , by all means i'm glad we have it, and i also get chance to learn from the code sample @vishwacsena have, and all the discussions you and @tomlm posted here.

I was just broadly expressing my impression from the original motivation of this DCR which seems mostly about Composer will be made complicated, plus my personal judgement of this is a "significant change". Like i listed in other comments, doing triggers vs recognizer is different, and even though we do use very simple conversion behind the ValueRecoginzer, it's still an explicit Recognizer that works in recognition phase. Moving to use trigger will have visible, if not significant, change on how user will use that (like in Vishwac's cases and in my simple example), and the message we deliver to user about how we model that behavior.

So I characterize it as a problem about where we model "card processing", and any problem of "where", to me, feels critical to a library like SDK, because it matters whether we are delivering a unified design, which keep things to where it belongs. I'm only speaking from my feeling and judgement, so i try to make it general, instead of pointing directly to any specific behavior.

Since those are all speaking from myself, I totally expect you or any participation of this conversation will feel different about

• whether we are designing for Composer specific or we are designing for better abstraction of bot developing
• whether this change is significant

And that's also i try to make it board and list it as an self-checking question.

Actually, design motivations are sometimes hard to distinguish, and sometimes different motivations are by nature overlapped (if we have a design that great for Composer, that's at least one solid justification of it works in this aspect). From my personal experience, sometimes when i'm designing some features based on certain feekback, even if i convinced myself and everybody else at the design time that this is a generic solution that should work, when things are moving to implementation phase, i would also keep asking me, is this really generic and good abstraction, or i just be able to found some argument to convince myself and\or others to believe it's generic. In several cases, i did realize the design decision was not good, i reverted that at the cost of time, effort and my reputation.

Maybe that's the nature of design. Anyhow, i'm definitely trying to constructive, or at least to be constructive criticism, not to be that "blindly attacking" kind of criticism. Sorry for my wordings if it feels that way, i understand nobody like someone jump in between the conversation without too much context and start questioning the fundamental approach, which i tried to avoid to be.

[cleemullins]

per @tomlm, we need to resovle this before taking Adaptive to GA.

[vishwacsena]

@cwhitten

We aim to reduce friction and make the simple things easy. Card form submission is a simple thing, and it should be easy.

To capture more succinctly what I'd like to understand better if we move forward with the ValueRecognizer:

1. Can we hide it completely from the user and bake it into the base recognition logic?
2. How will additional Adaptive Card v2 behavior ie. refreshing impact the model?
3. How will non-Adaptive Card value selections benefit from this, i.e. HeroCard? If this is positioned as a mechanism to handle card value submissions, it makes sense that HeroCard's should benefit from it.

Totally on the same page that we should do the necessary abstractions where possible so it is easy for developers to get set up!

1. @tomlm and I discussed your first point over the weekend. I already commented on this as well, specifically can we not default ValueRecognizer in composer by default? The issue with baking it into core recognizer is that we know we will need a multi-recognizer solution for LUIS + QnA. At the moment recognizerSet just does a union of entities recognized across recognizers. By baking value recognition into each recognizer, we will now need to know specific entities that are from a card and be able to de-dupe them. I'm not sure if you have thought through an elegant solution for that de-duping?
2. I do not know how this works for v2 since I do not know what is in v2.
3. The unique thing about Adaptive card is that they do not support imBack whereas the default for other card types typically is imBack (or postBack) which instead of relying on a separate recognizer, just relies on the LU recognizer. AFAIK, all other BF card types except adaptive cards support card actions. To this effect, I think it makes sense to rename ValueRecognizer to AdaptiveCardRecognizer because this is not required for any other card types except for adaptive cards.

Given this context, here's what I'm proposing as the update to the DCR -

1. The recommendation that Tom made makes it hard for developers to consume adaptive card responses. Also unclear how to get cards seamlessly integrated with other LU triggers
2. We should consider renaming ValueRecognizer to AdaptiveCardsRecognizer
3. I'm open to the idea of a default recognizer for dialog.
   • We cannot bake the value logic into the recognizer since that does not work for multi-recognizer scenario.
   • In composer, can we automatically set the AdaptiveCardRecognizer as the default? We could further optimize this to only do this if we know users have an adaptive card tied to the LG for that dialog.

@cwhitten @Stevenic thoughts?

[vishwacsena]

@tomlm @Stevenic @cwhitten and @vishwacsena discussed this today. Here's the updated proposal.

• Make ValueRecognizer private
• Update adaptive dialog to call the ValueRecognizer if
  • recognizers returned a null intent .or. if no recognizer was configured on the dialog
  • turn.activity.value is not null or empty

This would help us

• achieve the right short term fix to have value recognition always work without requiring additional configuration from the developer
• help us provide a property on adaptive dialog to turn this default behavior off.
• In the future, we could also make the ValueRecognizer if there is a need to make this public for varied composition needs per customer feedback.

@tomlm signed up to implement the updated DCR proposal for R9.

Tagging Approved, assigned to @tomlm