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 template response by ActionQueryKnowledgeBase to correctly use string representation of kb item. #407

Merged
merged 5 commits into from
Mar 5, 2021

Conversation

kedz
Copy link
Contributor

@kedz kedz commented Feb 17, 2021

When using ActionQueryKnowldgeBase, the action utters a templated response to queries about the knowledge base items of the form: "'{NAME}' has the value '{VALUE}' for attribute '{ATTRIBUTE}'." NAME is supposed to be the string representation of the knowledge base item, however, it is currently returning the string representation of the function used to generate the string representation of the kb item. E.g.:

'<function ActionMyKB.init.. at 0x7fb23b7fddd0>' has the value 'True' for attribute 'breakfast-included'.

instead of

'Hilton (Berlin)' has the value 'True' for attribute 'breakfast-included'.

This pull request fixes this bug so that templated responses produced by the action uses the string representation of the kb item.
In addition to fixing the bug in rasa_sdk/knowledge_base/actions.py, this pull request also modifies tests/knowledge_base/test_actions.py to check that templated responses are generated correctly.

Note: When I ran make lint, black suggested that the call signature for run (line 103 in rasa_sdk/knowledge_base/actions.py) be on one line so I made that change as well.

To Reproduce the Bug

cd rasa/examples/knowledgebasebot
rasa train
rasa run actions &
rasa shell
# Your input -> Hotels
# bot: Found the following objects of type 'hotel':
#    1: City Hotel (Frankfurt am Main)
#    2: Jugendherberge (Berlin)
#    3: Hilton (Frankfurt am Main)
#    4: Berlin Wall Hostel (Berlin)
#    5: B&B (Berlin)
# Your input -> does the last one offer breakfast?
# bot: Did not find a valid value for attribute 'breakfast-included' for object '<function ActionMyKB.__init__.<locals>.<lambda> at 0x7fb23b7fddd0>'.

Status (please check what you already did):

  • made PR ready for code review
  • added some tests for the functionality
  • updated the documentation (I don't think this is necessary?)
  • updated the changelog (please check changelog for instructions)
  • reformat files using black (please check Readme for instructions)

…e string representation of the knowledge base item retrieved by the action. Add corresponding test.
@kedz kedz added type:bug 🐛 Something isn't working area:rasa-sdk 🧑‍💻 Everything that touches our python Rasa SDK labels Feb 17, 2021
@kedz kedz requested a review from wochinge February 17, 2021 19:09
@kedz kedz self-assigned this Feb 17, 2021
@CLAassistant
Copy link

CLAassistant commented Feb 17, 2021

CLA assistant check
All committers have signed the CLA.

@kedz kedz linked an issue Feb 17, 2021 that may be closed by this pull request
@kedz
Copy link
Contributor Author

kedz commented Feb 17, 2021

FYI: Looks like this has actually been proposed to be fixed before: #365

@sara-tagger
Copy link

Thanks for submitting a pull request 🚀 @rgstephens will take a look at it as soon as possible ✨

@wochinge
Copy link
Contributor

@kedz Sorry, I'm on vacation from tomorrow till next Friday. Could you please request review by somebody else? 🙌🏻

@kedz kedz requested review from m-vdb and removed request for rgstephens and wochinge February 18, 2021 21:12
@alwx alwx self-requested a review February 24, 2021 17:27
@alwx
Copy link
Contributor

alwx commented Mar 1, 2021

Hey @kedz !
Your PR looks good but there is a small issue with the code style. Could you please run make formatter to fix the issue with the code style?

@kedz
Copy link
Contributor Author

kedz commented Mar 1, 2021

Thanks @alwx ! I made the code style fix and added a changelog entry. I think this is good to merge?

@kedz kedz requested a review from alwx March 4, 2021 19:31
@alwx
Copy link
Contributor

alwx commented Mar 5, 2021

Yes, perfect, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:rasa-sdk 🧑‍💻 Everything that touches our python Rasa SDK type:bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ActionQueryKnowledgeBase: utter_attribute_value wrong function argument
5 participants