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

Fixing Issue#3568, bot receives a utterances smaller than the entity indexes #4456

Conversation

arielcintra
Copy link

@arielcintra arielcintra commented Aug 14, 2020

Fixes #

Description

I've got the same issue as described on https://github.com/microsoft/botframework-solutions/issues/3568 and as analyzing the bot framework code and my LUIS applications, my team could get to the result that the code is trying to peform a substring on a utterance with bigger entities indexes.

Specific Changes

The change performed was to verirfy if the utterance length is smaller or equals to the entity size (endIndex - startIndex) and, as a suggestion, return the utterance, if not continues to perform the substring as always.
I putted as suggestion so you guys analyze if it does not impact other parts of the framework. None of the unit test were broken with the change.

Testing

@arielcintra arielcintra requested a review from a team as a code owner August 14, 2020 15:52
@ghost
Copy link

ghost commented Aug 14, 2020

CLA assistant check
All CLA requirements met.

Copy link
Contributor

@chrimc62 chrimc62 left a comment

Choose a reason for hiding this comment

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

:shipit:

@mrivera-ms mrivera-ms changed the base branch from master to main August 20, 2020 21:49
@mrivera-ms mrivera-ms added the Area: AI-LUIS The issue is related to LUIS label Aug 26, 2020
@peterinnesmsft peterinnesmsft linked an issue Sep 1, 2020 that may be closed by this pull request
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.

🕐

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.

:shipit:

@tdurnford
Copy link
Contributor

@arielcintra Can you make a no-op change to the PR so it reruns the checks? Not sure why we're not able to rerun them on our end.

@EricDahlvang
Copy link
Member

@tdurnford they can be re-run from the Azure devops Pipeline, using the commit hash. They are running now.

@EricDahlvang
Copy link
Member

Oh, sorry ... that did not work. Presumably because this PR is from a fork?

@tdurnford
Copy link
Contributor

@arielcintra It looks like your branch is based on master instead of main. Can you please rebase your PR to the main branch?

Thanks @BruceHaley for investigating!

@mrivera-ms
Copy link
Contributor

@arielcintra , could you please look at the message above to resolve the failing checks? Thank you.

@cleemullins
Copy link
Contributor

Close this PR out:

  1. It's a code change w/o Unit Tests.
  2. It's been inactive / unresponded for quite some time.

@arielcintra This looks like a great fix, and we would like to take it. Please update / re-open the PR with a Unit Test and address the merge issue so we can take it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: AI-LUIS The issue is related to LUIS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ExtractEntityMetadata crashes on specific entity
7 participants