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

Add persistent synonyms with pdictng #134

Merged
merged 10 commits into from
Mar 18, 2022
Merged

Add persistent synonyms with pdictng #134

merged 10 commits into from
Mar 18, 2022

Conversation

deepfates
Copy link
Contributor

A bot with a thesaurus!

We want to be able to easily define synonyms for the various commands within bots: "hi" for "hello", "picture" for "imagine", etc.

I pulled the relevant logic out of #127 and put it into SynonymBot class, using the powerful aPersistDict for stateful storage of synonyms.

Devs can still add lists of synonyms to methods in development, but now admins and users can adjust the bot's vocabulary directly in Signal. The interface is roughly based on the syntax of ln in the command line, like so:

link hello wazzup

SynonymBot has commands for linking, unlinking, listing synonyms, and more. It also has dialogue branches for various error states.

Still wondering:

Design/abstraction

  • Should this be integrated lower, into core?
  • Or should it inherit from a higher level bot?
    • Features from QuestionBot might make error handling more natural

Technical hurdles

NLP questions

  • Some commands are restricted to admin, but currently link and unlink are available to users. This opens possibilities for abuse.
    • Should they also be admin-only?
    • Or have a restricted-words list, like berduck and bouncebot?
  • Unclear how synonyms will interact with fuzzy matching from string_dist
  • Currently devs can add the same synonym to two different commands. The first one alphabetically gets priority, i think. Users will be rebuffed from doing this though. (see sample dialogue below)

Happy to accept feedback on any of the above, or anything I missed.

Sample dialogue

image

@deepfates deepfates requested a review from itdaniher February 22, 2022 00:18
@technillogue
Copy link
Contributor

maybe a decorator, like requires_admin / hide? at least fewer # type: hides

in the link it looks like a protocol might be the way to go, or a Command class that inherits from Callable but has more attributes....

mobfriend issue should be fixed, run pip install --upgrade prometheus_client prometheus_async locally; they added type hints in a new release

@deepfates
Copy link
Contributor Author

Okay @technillogue , I pulled your fix from main (thx!) and then updated to a decorator and protocol. This fixes most of the issues with mypy, except for adding synonyms to an inherited method, because the signatures no longer match. Don't know how common this use case is though? Definitely reduced the amount of #type: ignore although not total LOC

@itdaniher
Copy link
Contributor

this is so exciting.

@itdaniher
Copy link
Contributor

I'm going to add the pdictng_readme.md document requested by @technillogue in #112

synonymbot.py Outdated
Comment on lines 10 to 15
class SynonymAttributes(Protocol):
syns: list


def has_synonyms(func: Any) -> SynonymAttributes:
return func
Copy link
Contributor

@technillogue technillogue Feb 25, 2022

Choose a reason for hiding this comment

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

oh FUCK YEAH

SynonymAttributes might need a __call__?

synonymbot.py Outdated
Comment on lines 31 to 34
"""Build synonyms from in-code definitions.

Run this command as admin when bot is first deployed.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm slightly confused about this workflow. Why not do this offline? examining pdict requires auth. (not saying don't do this, just curious)

Copy link
Contributor

Choose a reason for hiding this comment

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

oh nvm i think i get it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

apartly because accessing pdict requires await and I think you can't do in init.

It also allows to clear and reset the thesaurus while the bot is live w/o rebooting.

does that track?

Copy link
Contributor

Choose a reason for hiding this comment

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

there's the grungy internals of manipulating aPersistDict.dict_ which can be done in a synchronous context

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, or maybe just create_task inside of init? i was thinking about this today.

should the bot re-up on dev-installed commands when it reboots?

@technillogue technillogue merged commit 7a613a6 into main Mar 18, 2022
@technillogue technillogue deleted the synonyms-pdictng branch March 18, 2022 00:48
jgreat pushed a commit that referenced this pull request Apr 10, 2023
* persistent synonyms working

* synonyms in match_command

* deal with edge cases for synonyms

* move synonymbot to forest and update for new pdictng

* switch from a protocol to only setting the synonyms in a decorator, similarly to requires_admin, hide, and group_help_text on the imogen branch

Co-authored-by: technillogue <technillogue@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants