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

49.qnamaker-all-features [nodejs]: make QnAMakerBaseDialog a component dialog again. #2959

Closed
HesselWellema opened this issue Nov 26, 2020 · 19 comments
Assignees
Labels
Bot Services Required for internal Azure reporting. Do not delete. Do not change color. customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. customer-reported Issue is created by anyone that is not a collaborator in the repository.

Comments

@HesselWellema
Copy link

HesselWellema commented Nov 26, 2020

Before #2289, QnAMakerBaseDialog was a componentdialog with a four step waterfall dialog. This allowed me for instance to start a dialog in stead of showing an answercard.

In the most recent version of this sample, QnAMakerBaseDialog is 'extending' QnAMakerDialog which is in a module called botbuilder-ai. As a result it is now hard (at least for me) to modidy the behavior of a Qna-bot.

Also, in the utils folder of the sample, there is a qnaCardBuilder.js. I do not think this specific file is used by the QnAMakerDialog in bobuilder-ai.

@hcyang hcyang added Bot Services Required for internal Azure reporting. Do not delete. Do not change color. customer-reported Issue is created by anyone that is not a collaborator in the repository. Support labels Nov 26, 2020
@HesselWellema
Copy link
Author

Just to clearify. Modifications that I need to be able to implement are for instance:

-1 In stead of providing an answer, startt a dialog
-2 Add the source to the answers given by QNAMaker
-3 Make adjustments to suggestions card

@hcyang hcyang assigned mrivera-ms and unassigned jwiley84 Nov 30, 2020
@tonyanziano
Copy link

@mrivera-ms can you comment on this?

@mrivera-ms mrivera-ms assigned stevengum and unassigned mrivera-ms Dec 2, 2020
@stevengum
Copy link
Member

@HesselWellema could you clarify on the three scenarios you mentioned:

Just to clearify. Modifications that I need to be able to implement are for instance:

-1 In stead of providing an answer, startt a dialog
-2 Add the source to the answers given by QNAMaker
-3 Make adjustments to suggestions card

  1. Are you trying to start another dialog and then call QnAMaker? This should be possible through modifying the RootDialog
  2. I'm not sure I understand this, are you referring to modifying the answers provided by QnAMaker before forwarding them to the user?
  3. Unfortunately, the implementation of QnAMakerDialog doesn't provide a way to inject your own card builder or modify the suggestions card. You could create a SendActivityHandler to modify the outgoing activity.

@stevengum stevengum added the customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. label Dec 3, 2020
@HesselWellema
Copy link
Author

HesselWellema commented Dec 4, 2020

Hi @stevengum,

Thanks for your reply.

wrt 1. For now I use QnaMaker for most of my bots. My customers usually need a qna bot with some additional, customer specific, dialogs. Lets say they need a complaint registration process). Working with the old sample, I would implement this by creating a question-answer pair for filing a complaint. The answer would be something like #complaint. In the checkForMultiTurnPrompt step I would check for this answer and start a component dialog from there (and end the QnAMakerBaseDialog. With the current sample, I don't know how to addres this. I tried to use the old sample with the new Managed QNA service (I need that because I am working on a multi-lingual bot)but that doesn't work. This results in an error as soon as this._qnaMakerService is called (I don't recall the exact error message).

wrt2. I implemented a chatbot to help people figure out what the latest measures are wrt to the Covid virus. Since the information is scattered over multiple websites anc changes a lot, this is a perfect usecase for QnaMaker. With the old sample, It was easy to add the source to an answer so people could check in the original source. With the new sample, I do not know how to do this.

wrt 3. Good one. I will look into middleware.

Hope this all makes sense.

@stevengum
Copy link
Member

stevengum commented Dec 9, 2020

Thank you for clarifying @HesselWellema.

For item 2, does it make sense to modify the QnA Knowledge Base or are you saying that the answers remain the same but the location of information changes?

For item 1, I'll have to investigate the new implementation to see if it's possible to do what you're asking for.

@HesselWellema
Copy link
Author

tnx. Wrt your suggestion for item 2. I'm afraid that will not work in my case. The bot is using 100+ different ur's and I retrain the database every three days (covid measures change a lot over here) resulting in over 2.500 qna pairs. This is not a hazzle-free process as you might guess (I can only train 10 url at a time). Adding the source to each pair manually takes to much time.

@stevengum
Copy link
Member

@HesselWellema at the moment I'm cleaning up the sample to hopefully address some of the extensibility issues, but I fear a deeper rewrite of the QnAMakerDialog class in the SDKs is necessary. I'll keep you updated.

@stevengum
Copy link
Member

@HesselWellema, did the middleware/SendActivityHandler suggestion help with addressing the 3rd requirement?

@HesselWellema
Copy link
Author

@stevengum. Thanks for asking. I did not implement it but trust that I will be ok. Did something like that in another project

@stevengum
Copy link
Member

@HesselWellema, sorry for the back and forth, thank you for your patience and understanding.

For requirements 1 & 2 in your comment it sounds like you need an "QnAMakerDialog.onAnswer" hook in the existing QnAMakerDialog. This hook(s) would allow you to modify the dialog's outgoing activity/answer(s). Does this sound accurate

For requirement 3, passing in a custom QnACardBuilder to the SDK's QnAMakerDialog instead of the SDK using a static helper would suffice.

To aid in the design process, are there other specific benefits that you would get in your scenarios by having a QnAMakerDialog class that subclasses ComponentDialog rather than WaterfallDialog?

@sahithikkss FYI

@HesselWellema
Copy link
Author

For requirements 1 & 2: Yes that sounds accurate.
wrt the designprocess: not sure yet. The waterfall helps me to understand what is actually happening and provides more room for customization (adding my own stuff). Guess that is part of the purpose of a sample: providing a head start.

@HesselWellema
Copy link
Author

@stevengum Hi, do I need to create a feature request for the QnAMakerDialog.onAnswer hook you suggested?

@gabog gabog assigned stevkan and unassigned stevengum Jan 4, 2021
@srinaath srinaath assigned mrivera-ms and unassigned stevkan Feb 4, 2021
@srinaath
Copy link

srinaath commented Feb 4, 2021

@mrivera-ms Could you take a look at  this ticket?

@compulim
Copy link
Contributor

@mrivera-ms @stevengum can you advise who should handle this ticket?

@mrivera-ms mrivera-ms removed their assignment Feb 17, 2021
@stevengum
Copy link
Member

Sorry about the delay, I dropped the ball on this... I'm putting together a proposal for cleaning up the QnAMakerDialog and the sample. I'll update this ticket ASAP once the proposal is done.

@munozemilio munozemilio added this to the R12 milestone Mar 4, 2021
@carlosscastro
Copy link
Member

Hello @stevengum -- are we including this proposal in R13? Or R14? Lets get the item in the right milestone

@stevengum
Copy link
Member

stevengum commented Mar 28, 2021

@HesselWellema I modified the current sample to address your first two scenarios you listed:

-1 In stead of providing an answer, startt a dialog
-2 Add the source to the answers given by QNAMaker
-3 Make adjustments to suggestions card

These updated sample can be found on the stgum/2959/demo branch (compare with main). If it doesn't address the scenarios, please let me know where it missed the target so I can reconsider how to extend the sample and/or update QnAMakerDialog in the SDK.

For the third scenario, a PR is in progress. The idea is for developers to pass in their own "QnASuggestionsActivityFactory" via a new QnAMakerDialog constructor overload.

/**
 * Returns an activity with active learning suggestions.
 * 
 * Important: The activity returned should relay the noMatchesText as an option to the end user. 
 *
 * @param suggestionsList List of suggestions.
 * @param noMatchesText If this text is received by the bot during a prompt, 
 */
 export type QnASuggestionsActivityFactory = (suggestionsList: string[], noMatchesText: string) => Partial<Activity>;

const isSuggestionsFactory: Test<QnASuggestionsActivityFactory> = (val): val is QnASuggestionsActivityFactory => {
    return tests.isFunc(val);
}

@HesselWellema
Copy link
Author

@stevengum. This is exactly what I needed. Thanks for you help!

@HesselWellema
Copy link
Author

HesselWellema commented Mar 29, 2021

Closing this question. Thanks for your help all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bot Services Required for internal Azure reporting. Do not delete. Do not change color. customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. customer-reported Issue is created by anyone that is not a collaborator in the repository.
Projects
None yet
Development

No branches or pull requests