Begin new dialog ActivityProcessed does not work as expected

[chrimc62]

Describe the bug

When you call a child AdaptiveDialog there is a flag for whether the current activity has been processed or not. If it is set to false then the child gets an opportunity to process it. The problem is that if there is a BeginDialog trigger in the child dialog the activity will not get processed by the child dialog. To fix it, you either have to remove the child BeginDialog trigger or add complicated logic to check the flag and emit the event. (Which I could not figure out.) This is made especially prevalent in Composer because creating a new dialog always adds a BeginDialog trigger. You have to choose either to have a BeginDialog trigger or process an optional incoming activity--this is not a choice I should be forced to make, I should be able to get both.

To Reproduce

Steps to reproduce the behavior:

1. Open the attached bot in composer and run it. It has a root dialog and 3 triggered dialogs:
   • NoChildProcessing where you get a begin dialog message and the child trigger does not fire.
   • ChildProcessingOnly where the child trigger fires, but there is no begin dialog message.
   • BeginAndChildProcessing is an attempt to get both a begin dialog and recognizing the intent.
2. Enter "NoChildProcessing" and you will not see the "Recognized NoChildProcessing from parent message", i.e. BeginDialog got executed but the RecognizedNoChildProcessing trigger fired.
3. Enter "ChildProcessingOnly" and you will see "Recognized ChildProcessingOnly from parent" which means the RecognizedChildProcessingOnly trigger fired, but there is no BeginDialog.
4. Enter "BeginAndChildProcessing" and you will not see the "Recognized BeginAndChildProcessing from parent" message. The logic in the bot does not work.

Expected behavior

I should be able to both have a begin dialog and process a parent activity. At the very least the last sample should be made to work and documented. We should consider building in some SDK behavior to control this.

SimpleCallTest.zip

[Stevenic]

@chrimc62 and I chatted about this and this is somewhat "by design".  If you call a dialog that has  BeginDialog actions those actions should be run when the dialog starts.  That will result in the intent recognition being intercepted making the activity processed flag worthless.  The core issue here is that when we're calling into a dialog (either via consultation or an explicit begin) we make one and only one decision of which actions to execute.  To truly honor that flag we would have to decide to do two sets of actions (begin + intent) and then append both onto the plan.  While we could do that I personally don't think we should.

First off for Chris's scenario it wouldn't have even done what he wanted.  He wanted to do the intents actions instead of the begin dialog actions.  While that OR case works for him how do we also support the AND case that others might want?  If anything I think we'd lean towards always just doing the AND otherwise you end up with begin actions that only "sometimes" run.

Personally I'd prefer the developer to have more control over the order in which things execute. I propose we execute the BeginDilaog actions as we do today but then add a new RecognizeIntent action that the developer can call to explicitly try to dispatch the current utterance.  This would return a flag indicating whether it was handled or not letting the BeginDialog flow decide what it wants to do next.

Another thing I'll point out is that when we discussed adding that flag my pushback was that I didn't like that it allowed the caller to change the execution behavior of the dialog being called.  That flag makes things less predictable for the dialog author as they know have to be prepared for two different execution sequences.  The dialog author knows how they're wanting their dialog to work and the caller shouldn't be able to make decisions that attempt to change that.

[Stevenic]

@tomlm @cwhitten @carlosscastro adding you guys to weigh in

[tomlm]

I don't understand this.  The reason the caller sets the flag is because the caller is the only one who knows that the activity has been "consumed". This was added specifically to allow the caller to initiate a sub-dialog without the dialog processing the activity that triggered the start of that dialog. If the sub-dialog has a BeginDialog it is specifically saying "I know what I want to do when I start, regardless what you sent me."If it doesn't, then recognition is appropriate and the flag is appropriate.  Why are we mixing BeginDialog and recgonition triggers together?

[tomlm]

I prefer processing BeginDialog trigger to set up the initial plan, and after run recognizer if ActivityProcessed==false and APPEND the intent actions to the plan.  This gives the effect of intial plan is always BeginDialog (If Defined) + Recognition (if needed)

The problem with a Recognize action is that it can only be validly invoked if you haven't released the turn...that's super problematic.

[chrimc62]

The only amendment I would make to @tomlm's proposal is that you could either put it before or after the begin dialog actions. I agree after is the most important one if we only are going to do one. I'd like to get this into the plan--Melanie and Xi have definitely been running into this. Before:

"order a ham and swiss on wheat"
did you mean [whole wheat, multigrain]?
"whole wheat"
Assigned whole wheat to bread
Welcome: you can fill in meat, cheese, toppings, bread and add/remove to it.

After

"order a ham and swiss on wheat"
Welcome: you can fill in meat, cheese, toppings, bread and add/remove to it.
did you mean [whole wheat, multigrain]?
"whole wheat"
Assigned whole wheat to bread

[mrivera-ms]

Reviewed by the SDK team. @tomlm, @chrimc62, @Stevenic is this something that requires any action from the SDK team now?

[Stevenic]

This would be a fairly significant change to the way Adaptive Dialogs work. We need to at least have a phone call to discuss. Is that something you can coordinate @mrivera-ms ?