Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Make getTopScoringIntent return '' instead of undefined #3870

Merged
merged 1 commit into from
Jul 15, 2021

Conversation

taicchoumsft
Copy link
Contributor

@taicchoumsft taicchoumsft commented Jul 15, 2021

Fixes Composer#8368. fixes #minor.

Description

There is a minor drift between the dotnet and nodeJs implementation of getTopScoringIntent.

  1. In the NodeJS implementation, topIntent is declared but not initialized. In the case of empty intents bag, can return undefined.

    NodeJS Implementation

         let topIntent: string;

    In the dotnet implementation, the topIntent is initialized.
    Dotnet Implementation

          var topIntent = (string.Empty, 0.0d);
  2. This later causes an issue in crossTrainedRecognizerSet line 110, where an undefined intent can be set.
    The isRedirect function will eventually be called on the undefined intent and cause the 500 error in the bot.

Note: this entire chain of events is started at the end of a text input, when the recognizers are fired with an empty utterance (not sure why). Orchestrator recognizer will return a RecognizerResult with this shape in the case of empty utterance:

        let recognizerResult: RecognizerResult = {
            text,
            intents: {},
            entities: {},
        };

Both LUIS and QnA fill the intents bag.

The dotnet Orchestrator implementation also returns an empty intents dictionary in the case of an empty utterance so we follow that behavior.

Testing

UT added, manual testing to test for correct behavior.

@coveralls
Copy link

coveralls commented Jul 15, 2021

Pull Request Test Coverage Report for Build 1035028336

  • 2 of 2 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 84.342%

Totals Coverage Status
Change from base Build 1027112788: 0.2%
Covered Lines: 19685
Relevant Lines: 22143

💛 - Coveralls

@joshgummersall joshgummersall merged commit 285f3e9 into main Jul 15, 2021
@joshgummersall joshgummersall deleted the tachou/fixTopScoringIntentDrift branch July 15, 2021 19:23
stevengum pushed a commit that referenced this pull request Jul 15, 2021
* port: beginskill uischema fix (#3854)

Fixes #3853

* make getTopScoringIntent return '' instead of undefined (#3870)

Co-authored-by: taicchoumsft <61705609+taicchoumsft@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unexpected error: Cannot read property ‘startsWith’ of undefined
3 participants