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

Fixes issue #3960 [QnAMaker] [DotNet] Implementing QnAMaker's precise answering capability as an additional feature for BotFramework users #3935

Merged
merged 63 commits into from
Jun 18, 2020
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
a82c9fa
AnswerSpanRequest property in Request.
vipeketi May 14, 2020
abf369a
Answer span changes for request and response data sstructures
vipeketi May 14, 2020
f4c8c4a
Request and Result object conversion
vipeketi May 14, 2020
96df775
datatype definition change from boolean to bool for enable AnswerSpan
vipeketi May 14, 2020
9a37d25
Text
vipeketi May 15, 2020
c157ca3
spaces
vipeketi May 15, 2020
b058ddd
MRCEnable instead of AnswerSpanRequest
vipeketi May 18, 2020
202db55
Answerspan text set to answer for MRC
vipeketi May 20, 2020
e131a74
Mock changes
vipeketi May 20, 2020
57a5c53
Review comments changes
vipeketi May 21, 2020
a28e2c1
Tests for both and precise
vipeketi May 22, 2020
c244d07
Update GetQnAPromptsCard invocation in tests
vipeketi May 22, 2020
9241f3d
GetQnAPromptsContentCard method
vipeketi May 22, 2020
761ddd2
Review comments
vipeketi May 22, 2020
75c01c9
Request changes
vipeketi May 22, 2020
7bbf06a
refactoring
vipeketi May 23, 2020
7d27031
Refactoring Tests
vipeketi May 24, 2020
d1a4fb4
Tests refactoring for comments
vipeketi May 25, 2020
84233ed
Added Copyright headers
vipeketi May 26, 2020
55f3e2a
Modified comment text for review comment.
vipeketi May 26, 2020
c556fc1
Update QnAMakerDialog.cs
vipeketi May 26, 2020
2074646
document text modified.
vipeketi May 26, 2020
e97cd51
Copy right header formatting
vipeketi May 27, 2020
a5cfa1c
Remove unused import
vipeketi May 27, 2020
09c5372
Revert "Added Copyright headers"
vipeketi May 28, 2020
a19c8d4
Copyright header
vipeketi May 28, 2020
d0f51b5
Comment modified for grammar and clarity
vipeketi May 28, 2020
8b7aa4a
Review comments
vipeketi Jun 2, 2020
b04b049
For fixing assembly difference issue
vipeketi Jun 2, 2020
1e5640c
Revert "For fixing assembly difference issue"
vipeketi Jun 2, 2020
ffd0bd8
For assemblies issue
vipeketi Jun 2, 2020
a17d450
Added purpose of property in summary.
vipeketi Jun 2, 2020
9ce95ce
Comments improved
vipeketi Jun 3, 2020
b56b246
For Review comments
vipeketi Jun 3, 2020
22a3f57
Merge branch 'vipeketi/mrcsupport' of https://github.com/microsoft/bo…
vipeketi Jun 3, 2020
e651a14
For review comments
vipeketi Jun 3, 2020
7b35226
Spellings Syntax
vipeketi Jun 6, 2020
8827223
Content corrections
vipeketi Jun 8, 2020
07ee9e5
Content comments changes
vipeketi Jun 8, 2020
d9d42bb
content request
vipeketi Jun 8, 2020
be73b1e
content changes
vipeketi Jun 8, 2020
3f57d1f
Content changes for comments
vipeketi Jun 8, 2020
1a06a92
Content review updates
vipeketi Jun 8, 2020
918f816
Content changes
vipeketi Jun 8, 2020
2152ae5
Content changes
vipeketi Jun 8, 2020
d6cf4c6
content chagnes
vipeketi Jun 8, 2020
1eb0c10
Revert enablePreciseAnswer to bool
vipeketi Jun 9, 2020
7b94291
removing json property for enable preccise
vipeketi Jun 9, 2020
60d5fda
Reverting Recognizer changes
vipeketi Jun 9, 2020
757ee08
schema change and BoolExpression
vipeketi Jun 10, 2020
cb8cfcf
Review comments
vipeketi Jun 10, 2020
11c9fa1
initialization of EnablePreciseAnswer
vipeketi Jun 10, 2020
7b0c684
schema changes for Display Precise AnswerOnly
vipeketi Jun 10, 2020
d144de8
BoolExpression for DisplayPreciseAnswerOnly
vipeketi Jun 11, 2020
bf2bdc1
Merge branch 'vipeketi/mrcsupport' of https://github.com/microsoft/bo…
vipeketi Jun 11, 2020
331bfc7
bool property for displayPreciseAnswer
vipeketi Jun 16, 2020
27fe97b
Updated comments
vipeketi Jun 16, 2020
41708f8
Null checks and CardEqualityComparer comments
vipeketi Jun 16, 2020
00e5d69
Undo contenttype change.
vipeketi Jun 16, 2020
946c4b7
format
vipeketi Jun 16, 2020
b98c43c
Final Commit
vipeketi Jun 17, 2020
b23c293
Spaces formating.
vipeketi Jun 17, 2020
c99e793
Indentation for schema files.
vipeketi Jun 17, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions libraries/Microsoft.Bot.Builder.AI.QnA/AnswerSpanRequest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using System;
vipeketi marked this conversation as resolved.
Show resolved Hide resolved
using System.Collections.Generic;
using System.Text;
using Newtonsoft.Json;

namespace Microsoft.Bot.Builder.AI.QnA
{
public class AnswerSpanRequest
Copy link
Contributor

Choose a reason for hiding this comment

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

This class does not have a corresponding Unit Test class. I looked for AnswerSpanRequestTest.cs and cannot find it.

{
/// <summary>
/// Gets or sets a value indicating whether gets or sets the enablet.
vipeketi marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

@mrivera-ms mrivera-ms Jun 1, 2020

Choose a reason for hiding this comment

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

/// Gets or sets a value indicating whether gets or sets the enablet. [](start = 7, length = 70)

This summary is not clear. Please improve. #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated as per comment.


In reply to: 433508388 [](ancestors = 433508388)

/// </summary>
/// <value>
/// The answer text.
/// </value>
[JsonProperty("enable")]
public bool Enable { get; set; }
}
}
45 changes: 43 additions & 2 deletions libraries/Microsoft.Bot.Builder.AI.QnA/Dialogs/QnAMakerDialog.cs
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,7 @@ private async Task<DialogTurnResult> CallGenerateAnswerAsync(WaterfallStepContex
}

stepContext.Values[ValueProperty.QnAData] = result;

ObjectPath.SetPathValue(stepContext.ActiveDialog.State, Options, dialogOptions);

// If card is not shown, move to next step with top QnA response.
Expand Down Expand Up @@ -625,8 +626,23 @@ private async Task<DialogTurnResult> CheckForMultiTurnPromptAsync(WaterfallStepC
ObjectPath.SetPathValue(stepContext.ActiveDialog.State, PreviousQnAId, answer.Id);
ObjectPath.SetPathValue(stepContext.ActiveDialog.State, Options, dialogOptions);

int contentChoice = -1;

if (answer.AnswerSpan != null && !string.IsNullOrEmpty(answer.AnswerSpan.Text))
{
if (dialogOptions.ResponseOptions.ContentChoice.Equals(QnADialogResponseOptions.PreciseAnswer))
{
contentChoice = 0;
}

if (dialogOptions.ResponseOptions.ContentChoice.Equals(QnADialogResponseOptions.Both))
{
contentChoice = 1;
}
}

// Get multi-turn prompts card activity.
var message = QnACardBuilder.GetQnAPromptsCard(answer, dialogOptions.ResponseOptions.CardNoMatchText);
var message = QnACardBuilder.GetQnAPromptsContentCard(answer, dialogOptions.ResponseOptions.CardNoMatchText, contentChoice);
await stepContext.Context.SendActivityAsync(message).ConfigureAwait(false);
vipeketi marked this conversation as resolved.
Show resolved Hide resolved

return new DialogTurnResult(DialogTurnStatus.Waiting);
Expand All @@ -636,6 +652,13 @@ private async Task<DialogTurnResult> CheckForMultiTurnPromptAsync(WaterfallStepC
return await stepContext.NextAsync(stepContext.Result, cancellationToken).ConfigureAwait(false);
}

/// <summary>
/// Gets the options the dialog will use to display query results to the user.
vipeketi marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
/// <param name="stepContext">The <see cref="WaterfallStepContext"/> for the current turn of conversation.</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the current turn of conversation.</param>
/// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
/// <remarks>If the task is successful, the result contains the response options to use.</remarks>
Copy link
Contributor

@cleemullins cleemullins May 26, 2020

Choose a reason for hiding this comment

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

Consider improving the remarks text. The comment should have a link to the reponseOptions which are actually called DialogTurnResults. #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added Reference link.


In reply to: 430551097 [](ancestors = 430551097)

private async Task<DialogTurnResult> DisplayQnAResultAsync(WaterfallStepContext stepContext, CancellationToken cancellationToken)
vipeketi marked this conversation as resolved.
Show resolved Hide resolved
{
var dialogOptions = ObjectPath.GetPathValue<QnAMakerDialogOptions>(stepContext.ActiveDialog.State, Options);
Expand Down Expand Up @@ -667,7 +690,25 @@ private async Task<DialogTurnResult> DisplayQnAResultAsync(WaterfallStepContext
// If response is present then show that response, else default answer.
if (stepContext.Result is List<QueryResult> response && response.Count > 0)
{
await stepContext.Context.SendActivityAsync(response.First().Answer, cancellationToken: cancellationToken).ConfigureAwait(false);
var firstAnswer = response.First();
var answer = firstAnswer.Answer;
int contentChoice = -1;

if (firstAnswer.AnswerSpan != null && !string.IsNullOrEmpty(firstAnswer.AnswerSpan.Text))
{
if (dialogOptions.ResponseOptions.ContentChoice.Equals(QnADialogResponseOptions.PreciseAnswer))
{
contentChoice = 0;
}

if (dialogOptions.ResponseOptions.ContentChoice.Equals(QnADialogResponseOptions.Both))
{
contentChoice = 1;
}
}

var message = QnACardBuilder.GetAnswerSpanCard(firstAnswer, contentChoice);
await stepContext.Context.SendActivityAsync(message).ConfigureAwait(false);
vipeketi marked this conversation as resolved.
Show resolved Hide resolved
}
else
{
Expand Down
46 changes: 46 additions & 0 deletions libraries/Microsoft.Bot.Builder.AI.QnA/Models/AnswerSpan.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using Newtonsoft.Json;

namespace Microsoft.Bot.Builder.AI.QnA
{
public class AnswerSpan
vipeketi marked this conversation as resolved.
Show resolved Hide resolved
{
/// <summary>
/// Gets or sets the answer text.
/// </summary>
/// <value>
/// The answer text.
/// </value>
[JsonProperty("text")]
public string Text { get; set; }

/// <summary>
/// Gets or sets the answer score.
/// </summary>
/// <value>
/// The answer score.
/// </value>
[JsonProperty("score")]
public float Score { get; set; }

/// <summary>
/// Gets or sets the answer startIndex.
/// </summary>
/// <value>
/// The answer startIndex.
/// </value>
[JsonProperty("startIndex")]
public int StartIndex { get; set; }

/// <summary>
/// Gets or sets the answer endIndex.
/// </summary>
/// <value>
/// The answer endIndex.
/// </value>
[JsonProperty("endIndex")]
public int EndIndex { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,18 @@ public class QnAMakerTraceInfo
/// </value>
[JsonProperty("rankerType")]
public string RankerType { get; set; }

[Obsolete("This property is no longer used and will be ignored")]
[JsonIgnore]
public Metadata[] MetadataBoost { get; set; }

/// <summary>
/// Gets or sets AnswerSpanRequest of the previous turn.
/// </summary>
/// <value>
/// The AnswerSpanRequest.
/// </value>
[JsonProperty("answerSpanRequest")]
public AnswerSpanRequest AnswerSpanRequest { get; set; }
}
}
9 changes: 9 additions & 0 deletions libraries/Microsoft.Bot.Builder.AI.QnA/Models/QueryResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,14 @@ public class QueryResult
/// </value>
[JsonProperty(PropertyName = "context")]
public QnAResponseContext Context { get; set; }

/// <summary>
/// Gets or sets AnswerSpan of the previous turn.
/// </summary>
/// <value>
/// The answerspan value.
Copy link
Contributor

Choose a reason for hiding this comment

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

In the Summary "AnswerSpan". In the value here, "answerspan". Please use consistant capitalization.

Consider elaborating and adding more detail.

/// </value>
[JsonProperty("answerSpan")]
public AnswerSpan AnswerSpan { get; set; }
}
}
12 changes: 12 additions & 0 deletions libraries/Microsoft.Bot.Builder.AI.QnA/QnADialogResponseOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ namespace Microsoft.Bot.Builder.AI.QnA
/// </summary>
public class QnADialogResponseOptions
{
public const string PreciseAnswer = "PreciseAnswer";

public const string Both = "Both";

/// <summary>
/// Gets or sets get or set for Active learning card title.
/// </summary>
Expand Down Expand Up @@ -41,5 +45,13 @@ public class QnADialogResponseOptions
/// Get or set for Card no match response.
/// </value>
public Activity CardNoMatchResponse { get; set; }

/// <summary>
/// Gets or sets get or set for MRCAnswerSpanRenderingOption.
/// </summary>
/// <value>
/// Get or set for Card no match MRCAnswerSpanRenderingOption.
/// </value>
public string ContentChoice { get; set; } = PreciseAnswer;
}
}
9 changes: 9 additions & 0 deletions libraries/Microsoft.Bot.Builder.AI.QnA/QnAMakerOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,5 +92,14 @@ public QnAMakerOptions()
/// <seealso cref="RankerTypes"/>
[JsonProperty("rankerType")]
public string RankerType { get; set; }

/// <summary>
/// Gets or sets a value indicating whether MRCEnable of the previous turn.
/// </summary>
/// <value>
/// To enable MRC.
/// </value>
[JsonProperty("enableMRC")]
vipeketi marked this conversation as resolved.
Show resolved Hide resolved
public bool EnablePreciseAnswer { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ private static void ValidateOptions(QnAMakerOptions options)
private QnAMakerOptions HydrateOptions(QnAMakerOptions queryOptions)
{
var hydratedOptions = JsonConvert.DeserializeObject<QnAMakerOptions>(JsonConvert.SerializeObject(this.Options));

if (queryOptions != null)
{
if (queryOptions.ScoreThreshold != hydratedOptions.ScoreThreshold && queryOptions.ScoreThreshold != 0)
Copy link
Contributor

@cleemullins cleemullins May 26, 2020

Choose a reason for hiding this comment

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

These if statements are too complex, and they require detailed knowledge of operator precedence. Please simplify. #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simplified by removing unnecessary, if check condition.


In reply to: 430554363 [](ancestors = 430554363)

Expand All @@ -169,12 +169,13 @@ private QnAMakerOptions HydrateOptions(QnAMakerOptions queryOptions)
if (queryOptions.StrictFilters?.Length > 0)
{
hydratedOptions.StrictFilters = queryOptions.StrictFilters;
}
}

hydratedOptions.Context = queryOptions.Context;
hydratedOptions.QnAId = queryOptions.QnAId;
hydratedOptions.IsTest = queryOptions.IsTest;
hydratedOptions.RankerType = queryOptions.RankerType != null ? queryOptions.RankerType : RankerTypes.DefaultRankerType;
hydratedOptions.EnablePreciseAnswer = queryOptions.EnablePreciseAnswer;
}

return hydratedOptions;
Expand All @@ -183,17 +184,20 @@ private QnAMakerOptions HydrateOptions(QnAMakerOptions queryOptions)
private async Task<QueryResults> QueryQnaServiceAsync(Activity messageActivity, QnAMakerOptions options)
{
var requestUrl = $"{_endpoint.Host}/knowledgebases/{_endpoint.KnowledgeBaseId}/generateanswer";
var answerSpanRequest = new AnswerSpanRequest();
answerSpanRequest.Enable = options.EnablePreciseAnswer;
vipeketi marked this conversation as resolved.
Show resolved Hide resolved
var jsonRequest = JsonConvert.SerializeObject(
new
{
question = messageActivity.Text,
top = options.Top,
strictFilters = options.StrictFilters,
strictFilters = options.StrictFilters,
scoreThreshold = options.ScoreThreshold,
context = options.Context,
qnaId = options.QnAId,
isTest = options.IsTest,
rankerType = options.RankerType
rankerType = options.RankerType,
answerSpanRequest = answerSpanRequest
}, Formatting.None);

var httpRequestHelper = new HttpRequestUtils(httpClient);
Expand All @@ -206,18 +210,21 @@ private async Task<QueryResults> QueryQnaServiceAsync(Activity messageActivity,

private async Task EmitTraceInfoAsync(ITurnContext turnContext, Activity messageActivity, QueryResult[] result, QnAMakerOptions options)
{
var answerSpanRequest = new AnswerSpanRequest();
answerSpanRequest.Enable = options.EnablePreciseAnswer;
vipeketi marked this conversation as resolved.
Show resolved Hide resolved
var traceInfo = new QnAMakerTraceInfo
{
Message = (Activity)messageActivity,
QueryResults = result,
KnowledgeBaseId = _endpoint.KnowledgeBaseId,
ScoreThreshold = options.ScoreThreshold,
Top = options.Top,
StrictFilters = options.StrictFilters,
StrictFilters = options.StrictFilters,
Context = options.Context,
QnAId = options.QnAId,
IsTest = options.IsTest,
RankerType = options.RankerType
RankerType = options.RankerType,
AnswerSpanRequest = answerSpanRequest
};
var traceActivity = Activity.CreateTraceActivity(QnAMaker.QnAMakerName, QnAMaker.QnAMakerTraceType, traceInfo, QnAMaker.QnAMakerTraceLabel);
await turnContext.SendActivityAsync(traceActivity).ConfigureAwait(false);
Expand Down
69 changes: 67 additions & 2 deletions libraries/Microsoft.Bot.Builder.AI.QnA/Utils/QnACardBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public static IMessageActivity GetSuggestionsCard(List<string> suggestionsList,
throw new ArgumentNullException(nameof(suggestionsList));
}

if (cardTitle == null)
if (cardTitle == null)
{
throw new ArgumentNullException(nameof(cardTitle));
}
Expand Down Expand Up @@ -80,9 +80,21 @@ public static IMessageActivity GetSuggestionsCard(List<string> suggestionsList,
/// Get active learning suggestions card.
/// </summary>
/// <param name="result">Result to be dispalyed as prompts.</param>
vipeketi marked this conversation as resolved.
Show resolved Hide resolved
/// <param name="cardNoMatchText">No match text.</param>
/// <param name="cardNoMatchText">No match text.</param>
Copy link
Contributor

@cleemullins cleemullins May 26, 2020

Choose a reason for hiding this comment

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

I don't know what this means. Consider improving the quality of the doc comment. #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated content.


In reply to: 430557111 [](ancestors = 430557111)

/// <returns>IMessageActivity.</returns>
public static IMessageActivity GetQnAPromptsCard(QueryResult result, string cardNoMatchText)
vipeketi marked this conversation as resolved.
Show resolved Hide resolved
{
return GetQnAPromptsContentCard(result, cardNoMatchText, 0);
}

/// <summary>
/// Get active learning suggestions content card.
/// </summary>
/// <param name="result">Result to be dispalyed as prompts.</param>
Copy link
Contributor

@cleemullins cleemullins May 26, 2020

Choose a reason for hiding this comment

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

spelling. displayed.
Also, what are "prompts" in this case? Consider significantly improving the doc quality. #Resolved

Copy link
Contributor Author

@vipeketi vipeketi Jun 17, 2020

Choose a reason for hiding this comment

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

Spelling issue fixed.
Prompts are references to possible answers, displayed along with answer in Dialog. Updated description of parameter with precise answer information.


In reply to: 430558744 [](ancestors = 430558744)

/// <param name="cardNoMatchText">No match text.</param>
/// <param name="renderingOption">renderingchoice.</param>
/// <returns>IMessageActivity.</returns>
vipeketi marked this conversation as resolved.
Show resolved Hide resolved
vipeketi marked this conversation as resolved.
Show resolved Hide resolved
public static IMessageActivity GetQnAPromptsContentCard(QueryResult result, string cardNoMatchText, int renderingOption)
{
if (result == null)
{
Expand Down Expand Up @@ -115,11 +127,64 @@ public static IMessageActivity GetQnAPromptsCard(QueryResult result, string card
Buttons = buttonList
};

// For content choice Both Precise and Content
if (renderingOption == 1 && result.AnswerSpan != null)
{
plCard.Text = result.Answer;
chatActivity.Text = result.AnswerSpan.Text;
}

// For content choice Precise only
if (renderingOption == 0 && result.AnswerSpan != null)
{
chatActivity.Text = result.AnswerSpan.Text;
}

// Create the attachment.
var attachment = plCard.ToAttachment();

chatActivity.Attachments.Add(attachment);

return chatActivity;
}

/// <summary>
/// Get active learning suggestions card.
/// </summary>
/// <param name="result">Result to be dispalyed as prompts.</param>
/// <param name="renderingOption">renderingchoice.</param>
/// <returns>IMessageActivity.</returns>
public static IMessageActivity GetAnswerSpanCard(QueryResult result, int renderingOption)
{
if (result == null)
{
throw new ArgumentNullException(nameof(result));
}

var chatActivity = Activity.CreateMessageActivity();
chatActivity.Text = result.Answer;

// For content choice Precise only
if (renderingOption == 0 && result.AnswerSpan != null)
{
chatActivity.Text = result.AnswerSpan.Text;
}

var plCard = new HeroCard()
{
};

// For content choice Both Precise and Content
if (renderingOption == 1 && result.AnswerSpan != null)
{
plCard.Text = result.Answer;
chatActivity.Text = result.AnswerSpan.Text;
var attachment = plCard.ToAttachment();

// Create the attachment.
chatActivity.Attachments.Add(attachment);
}

return chatActivity;
}
}
Expand Down
Loading