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

Chaining with slots creates a loop #684

Closed
6 tasks done
amendlik opened this issue Jan 26, 2024 · 9 comments
Closed
6 tasks done

Chaining with slots creates a loop #684

amendlik opened this issue Jan 26, 2024 · 9 comments
Assignees
Labels

Comments

@amendlik
Copy link

Describe the bug
If I set up 2 QNA documents, with the first chaining to the second, there are no issues.
When I add a slot to the first document (and perform a Lex Rebuild), the chain loops on the first document and never reaches the second.

To Reproduce
Import the following test configuration:

{
  "qna": [
    {
      "a": "ChainHead",
      "slots": [
        {
          "slotName": "Number",
          "slotPrompt": "Give me a number",
          "slotRequired": true,
          "slotType": "AMAZON.Number"
        }
      ],
      "enableQidIntent": true,
      "conditionalChaining": "'QID::TST-ChainTarget'",
      "type": "qna",
      "qid": "TST-ChainHead",
      "q": [
        "ChainHead"
      ]
    },
    {
      "a": "ChainTarget",
      "type": "qna",
      "qid": "TST-ChainTarget",
      "q": [
        "ChainTarget"
      ]
    }
  ]
}

Expected behavior
The bot should elicit the slot, then chain to the second document.

Please complete the following information about the solution:

  • Version: v5.4.5
  • Region: **us-west-2
  • Was the solution modified from the version published on this repository? No
  • If the answer to the previous question was yes, are the changes available on GitHub? N/A
  • Have you checked your service quotas for the services this solution uses? Yes
  • Were there any errors in the CloudWatch Logs? No

Screenshots
image

Additional context
Add any other context about the problem here.

@jangidms
Copy link
Member

Hi @amendlik Thanks for reporting this
We'll look into this and get back to you

@amendlik
Copy link
Author

Hello @jangidms - any updates on this? I can work on the fix if you can point me in the right direction.

@jangidms
Copy link
Member

Hi @amendlik
I looked into this scenario and I see that currently Document Chaining is not supported when using intent & slots. It is working as expected.
When processing request with QnaIntent, QnABot injects qid in the request (Link to Request Parsing Code)
When evaluating conditional chaining, QnABot updates the question in the request to chaining question (Conditional Chaining Code)
But since request also has qid field, that takes precedence over question when making OpenSearch request (OpenSearch Request logic)

You could try to customize QnABot as per your use case, or open a new enhancement request which we'll add to our backlog

@amendlik
Copy link
Author

@bobpskier I've been testing a solution that just deletes qid from the structure by adding this code to evaluateConditionalChaining()

    if(req.hasOwnProperty('qid')) {
        delete req['qid'];
    }

It's working fine so far. I have not encountered any negative effects.

@bobpskier
Copy link
Contributor

@amendlik I'd probably implement something similar in evaluateConditionalChaining.js starting at line 100

if (next_q) {
        req.question = next_q;
        if (next_q.toLowerCase().startsWith('qid::')) {
            _.set(req,'qid', next_q.substr(5));
        } else {
            _.set(req,'qid', undefined);
        }
        [req, res, hit2, errors] = await getHit(req, res);
    }

This updates req.qid to be the actual target qid of the chaining config if it is specified as a "qid::" or removes qid if not specified in that format.

The intent/slot mechanism is not a good citizen in the word of QnABot chaining. It looks like it can only be used in the first qid that kicks off a series of chained qids. If in the middle of the chain of questions, a qid is specified that also uses intents/slots, the chaining mechanism will skip right over asking to resolve the slot values. The intent/slot mechanism only works at the beginning of the chain when Lex can call QnABot fulfillment using a DialogCodeHook with the intent/slots as the context.

If you want to conditionally ask for values in one of more qids via chaining, it is better to use ElicitResponse bots as they support the concept of chaining.

@amendlik
Copy link
Author

amendlik commented Feb 9, 2024

This fix is working well for us. @bobpskier, do you want to upstream that, or would you like me to do it?

@fhoueto-amz
Copy link
Member

fhoueto-amz commented Feb 22, 2024

@amendlik can you please submit a PR if you want us to consider as improvementÉ
Closing this

@amendlik
Copy link
Author

I have opened a PR on this: #721

@fhoueto-amz
Copy link
Member

Great. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants