Core Assistant: Cannot disambiguate similar intents in the same LUIS recognizer

darrenj commented 3 years ago

Describe the bug

Basic Assistant template has a duplicated intents recognized capability which I'm unable to trigger. Some of this is related to microsoft/BotFramework-Composer#7404 but after inspecting trace it would appear that IncludeAllIntents isn't being set on the call to LUIS meaning your only going to get the topIntent back not the other intents and scores meaning this feature cannot work when LUIS is being used.


{
  "activity": {
    "type": "trace",
    "timestamp": "2021-04-27T14:12:01.477Z",
    "serviceUrl": "http://localhost:5000",
    "channelId": "emulator",
    "from": {
      "id": "ec66a842-b822-4078-92c4-d2bf4890942d",
      "name": "Bot",
      "role": "bot"
    },
    "conversation": {
      "id": "04b1056c-dc37-44ad-8764-ea2ff77add8d|livechat"
    },
    "recipient": {
      "id": "91b9c1ca-4273-4517-87a4-6cf4ffe6a8ab",
      "role": "user"
    },
    "locale": "en-us",
    "replyToId": "1d2ffde0-5652-4b7a-a5d9-3c7f8702e5d0",
    "label": "LuisV3 Trace",
    "valueType": "https://www.luis.ai/schemas/trace",
    "value": {
      "recognizerResult": {
        "text": "book",
        "alteredText": null,
        "intents": {
          "None": {
            "score": 0.79196495
          }
        },
        "entities": {}
      },
      "luisModel": {
        "ModelID": "44388ba7-de06-46e6-9c98-090b6219f5e9"
      },
      "luisOptions": {
        "IncludeAllIntents": false,
        "IncludeInstanceData": false,
        "IncludeAPIResults": false,
        "Log": true,
        "DynamicLists": null,
        "ExternalEntities": null,
        "PreferExternalEntities": true,
        "DateTimeReference": null,
        "Slot": "production",
        "Version": null
      },
      "luisResult": {
        "query": "book",
        "prediction": {
          "topIntent": "None",
          "intents": {
            "None": {
              "score": 0.79196495
            }
          },
          "entities": {}
        }
      }
    },
    "name": "LuisRecognizer",
    "id": "bfc80184-d739-470e-9d93-dbd10d6cabfa",
    "localTimestamp": "2021-04-27T15:12:01+01:00"
  },
  "id": "0f59b4bf-598d-4d26-9621-11fac3acbffc",
  "timestamp": 1619532721477
}

Version

2.0.0-nightly-239644

Browser

[x] Electron distribution
[ ] Chrome
[ ] Safari
[ ] Firefox
[ ] Edge

OS

[ ] macOS
[x] Windows
[ ] Ubuntu

To Reproduce

Steps to reproduce the behavior:

Create Basic Assistant
Create two dialogs with overlapping intents (be aware of microsoft/BotFramework-Composer#7404)
Observe LUIS trace in Composer and see that only topIntent is being returned

Expected behavior

Disambiguation to work!

darrenj commented 3 years ago

UPDATE

Did some digging as per this:

https://github.com/microsoft/BotFramework-Composer/blob/ef424c5583d9c2813f4b7255201d912ccc770696/Composer/packages/server/schemas/sdk.en-US.schema

By hand I updated my recognizer configuration in recognizers folder to include following (not clear if Composer surfaces this anywhere)

  "predictionOptions" : 
    {
        "includeAllIntents": true
    }

This results with multiple intents coming back but not able to prove disambiguation works due to microsoft/BotFramework-Composer#7404. If it does we'll need to figure out if we can have this set in the template?

cwhitten commented 3 years ago

@darrenj Composer does not (yet) surface the the entire interface for a specific recognizer.

lauren-mills commented 3 years ago

This will impact EA as well so @ryanlengel and I can tag team it

taicchoumsft commented 3 years ago

@darrenj . even if we set includeAllIntents to true, and even if we solved the lu de-dupe problem in Composer#7404, the disambig template will not work with LUIS.

The new template logic takes into account child recognizers that fire their own ChooseIntent intents, like Orchestrator (CrossTrainedRecognizerSet, the parent recognizer in Composer, already does this).

i.e. To see this template disambiguate on intents, the intent-based child recognizers must themselves be able to fire the "ChooseIntent" intent, and put what it thinks are duplicates into the candidate property. When the trigger is hit, the template takes all these candidates from all child recognizers of CrossTrainedRecognizerSet and flattens them into one list, that's all it does.

LUIS does not fire this intent, If you set the includeAllIntents property to true, it'll simply populate the intents property, not the candidates property.

To do intent disambiguation for LUIS (more than disambiguating between LUIS and QNA like the old template), we also need to change the template logic, and set the includeAllIntents flag for LUIS in Composer.

ryanisgrig commented 3 years ago

I've validated that we can include the includeAllIntents property in the Basic Assistant's recognizer and LUIS does in fact return all intents after having configured it. While the Enterprise Assistant is configured with the Orchestrator recognizer, the Basic Assistant uses the LUIS recognizer (since there are no prebuilt skills included).

Based on what @taicchoumsft described, the disambiguation dialog used by both Composer and these templates is configured only for Orchestrator and will not work in the same way for LUIS. That being said I'd like to hold on assistant template changes until the end to end scenario is fully understood and agreed upon, the changes needed to make work here will have to bubble upstream. Will wait for @darrenj's reply.

darrenj commented 3 years ago

Hmmm :-) So, the disambiguation trigger has existed since at least Composer 1.2 released in November last year (so pre-orchestrator). From the discovery of includeAllIntents not being set it would lead to me to believe that we've uncovered a long-term bug whereby disambiguation hasn't worked for Composer bots at any point as the multiple intents and scores would not have been returned?

With the introduction of Orchestrator we happily found that the same disambiguation trigger worked so we had a one size fits all solution. However it seems whilst Orchestrator works, LUIS doesn't and disambiguation for LUIS is a key conversational ai scenario especially where a developer isn't using Skills (quite reasonably).

Given this disambiguation feature has been available for a while, and because Composer allows you to add this trigger to any old bot (with a default recognizer of LUIS) it feels to me that we need to do whatever work is required to ensure the disambiguation trigger works with LUIS?

taicchoumsft commented 3 years ago

The original trigger design was to disambiguate between the top results of LUIS and QnA in the same dialog. It has absolutely no effect if only one recognizer is being used.

It uses the cross-training mechanism described here. In case 4 of the table where the two recognizers defer to each other, CrossTrainedRecognizerSet emits the ChooseIntent intent and triggers this template.

Yes the implication is this template never worked to detect ambiguous intents coming from the same recognizer. It only worked to detect ambiguity between two recognizers' top intents. This was by design.

It is probably better if LUIS can detect ambiguity at the recognizer level and fire the ChooseIntent intent, then this template will work for LUIS even if QnA is not being used.

If we redesign this template now, we'll have to re-jigger quite a bit of template code and perhaps rethink the in-dialog CrossTrain concept.

darrenj commented 3 years ago

@cwhitten Wasn't entirely sure precisely where to put the settings so tried both approaches:

  "luis": {
    "authoringEndpoint": "",
    "authoringRegion": "westus",
    "defaultLanguage": "en-us",
    "endpoint": "",
    "environment": "composer",
    "name": "djCoreWithLanguage",
    "authoringKey": "REDACTED",
    "endpointKey": ""
  },
  "predictionOptions": {
    "includeAllIntents" : true
  },

and

  "luis": {
    "authoringEndpoint": "",
    "authoringRegion": "westus",
    "defaultLanguage": "en-us",
    "endpoint": "",
    "environment": "composer",
    "name": "djCoreWithLanguage",
    "authoringKey": "redacted",
    "endpointKey": "",
    "predictionOptions": {
      "includeAllIntents" : true
    }

Unfortunately neither seem to change the behaviour (via config). LUIS trace shows that includeAllIntents is set to false (default) despite setting this.

"luisOptions": {
        "IncludeAllIntents": false,
        "IncludeInstanceData": false,
        "IncludeAPIResults": false,

sgellock commented 3 years ago

Shiproom - lets spike this out and see what it looks like.

taicchoumsft commented 3 years ago

We have two major options here:

1) High Cost, High Risk (but right thing to do given current design): Copy OrchestratorRecognizer's disambiguation logic to LuisRecognizer.

This is done at SDK level - must make changes in Node and C# SDK, also comes with LUIS Recognizer schema changes.  No Composer changes necessary, no need to enable LUIS's `includeAllIntents`.

2) Low Cost, Medium Risk: Shift the disambiguation logic from SDK into Composer.

Mimic SDK disambiguation logic inside Composer's OnIntent trigger template. If there are ambiguous intents, then the OnIntent trigger calls the disambiguation trigger. This means we add a lot of template code (visible to Composer user) to the OnIntent trigger.

taicchoumsft commented 3 years ago

2) turns out to not be straight-forward. After some exploration for path 2, these are the findings:

enable includeAllIntents from the recognizers/<botname>.en-us.lu.dialog file:

{
  "$kind": "Microsoft.LuisRecognizer",
  "id": "LUIS_DisambiguationForLuisBot",
  "applicationId": "=settings.luis.DisambiguationForLuisBot_en_us_lu.appId",
  "version": "=settings.luis.DisambiguationForLuisBot_en_us_lu.version",
  "endpoint": "=settings.luis.endpoint",
  "endpointKey": "=settings.luis.endpointKey",
  "predictionOptions": {
    "includeAllIntents": true
  }
}

This allows LuisRecognizer to return all intents in the results. We need this to perform disambiguation at Composer level. This works and is verified from trace events in Composer

However, there is code in AdaptiveDialog to remove all but the top intent from the recognizer output
The end result is, after the OnIntent trigger, we have lost all but the top intent in turn.recognized (turn.recognized/turn.dialogEvent.value are the special values in AdaptiveDialog containing the copied recognizerResult):

So far, I haven't found an easy Composer-only method to solve this problem, still investigating.

sgellock commented 3 years ago

Shiproom - I discussed this issue with @tomlm and I've asked him to weigh in on approach

tomlm commented 3 years ago

Tai is correct that this event is fired currently only with Cross trained QnA/LUIS models.

I do have an ThresholdRecognizer implementation in iciclecreek.bot/ThresholdRecognizer.cs at master · tomlm/iciclecreek.bot (github.com) which applies a threshold to any recognizer.

In this package

This example will fire OnChooseIntent for intents that are within .1 of each other from the inner recognizer.

{
    "recognizer": {
        "$kind":"Iciclecreek.ThresholdRecognizer",
        "threshold": 0.1, 
        "recognizer": { 
            ...luis recognizer with all intents returned...
         }
    }
    ...
}

sgellock commented 3 years ago

Shiproom - moving out to R14 as we need time to design this correctly

microsoft / botframework-components