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

Deregister inactive users #75

Open
cceckman opened this issue Aug 31, 2024 · 4 comments
Open

Deregister inactive users #75

cceckman opened this issue Aug 31, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@cceckman
Copy link
Collaborator

Pairing Bot de-registers users at the end of their batch, along with a message saying they're welcome to re-register. If they do, they're in until their next batch.

If an alum re-registers, and then drops off the face of the earth, they'll still get matches indefinitely. This is a poor experience for current Recursers / active pairing-bot users.

It would be Nice to have some "eviction policy" for Pairing Bot: if a registered PB user misses enough pairings, they get one or more messages from PB poking them, and eventually get booted.

Detecting and defining inactivity

We don't currently have a definition for "inactive", or any state reflecting PB use beyond "configured schedule".

When Pairing bot makes a pairing thread, it's also a party to that thread. If either party responds to that thread, PB will also get a message. (Currently, PB ignores it.)

This suggests a definition for inactivity: if Pairing Bot receives a message from a user, in a private thread or a thread with two other users (i.e. humans), that counts as "an interaction with PB".

PB can keep, for each user, a count of the number of times it has paired that user since the last time the user interacted. That is, an interaction resets the count to 0.

We can then add a cron that checks this count for each user. If it's greater than some threshold (what?), we can send a message to that user that says "we've de-registered you for inactivity, but you're welcome to re-register".

Note this doesn't require a user to actually do the pairings: a response of "sorry, I can't pair today" still counts. I think this is fine, even desirable; we still trust Recursers who are active on Zulip to remove themselves or alter their schedule if they consistently can't make pairings. We're only trying to take care of the case where someone isn't checking / acting on Zulip.

@cceckman cceckman added the enhancement New feature or request label Aug 31, 2024
@anna-hope
Copy link

anna-hope commented Sep 1, 2024

If it's greater than some threshold (what?), we can send a message to that user that says "we've de-registered you for inactivity, but you're welcome to re-register".

I feel like 3 (a good magic number?) pairing chats in a row where a user didn't engage with Pairing Bot would be a good starting threshold to evict them from the pairing roster until the user proactively opts back in. A clear message from PB, like the one @cceckman is proposing, would help make sure the deregistered user knows what's going on. It would be ideal to give the user a heads up + chance to engage before kicking them off, but I don't know if it's necessary.

I can see some weird edge cases where PB might kick a user off the roster even if that user is going through with the pairing sessions — e.g. they always just DM the other person they were paired with instead of responding to the PB thread. However, I don't have a good idea of how likely that is. Right now, my intuition is to place a higher priority on making sure the pairing roster includes only users who want/can pair, so as to reduce the incidence of failed pairings.

@ChrisRenfrow
Copy link
Collaborator

This sounds like a good idea. I do agree with Anna however that folks who just DM the person they're pairing with (me) might be in for a rude surprise!

As a suggestion, perhaps "interaction" could include reacting to pairing bot's message. This could even just be the recommended path, e.g. "react or respond to confirm this pairing"

@cceckman
Copy link
Collaborator Author

folks who just DM the person they're pairing with (me) might be in for a rude surprise!

I agree that this would be surprising! I'd hope that we could do user education for this in two ways:

  • Warning message before kicking out
  • That message says something like: "Pairing Bot has not seen you participating (though she may have missed it!). Please say something in each pairing thread (like 'Thanks, Pairing Bot!') so she knows you're interested in future pairings!"

Re: reactions: I agree that this would be sufficient if you're also spinning off a DM to start pairing, or if the other party has sent a DM, etc. I think "some activity in the pairing thread* would work.

(I don't know how reacts show up in the Zulip API- same as messages? Similar enough to run most of the same logic?)

I say all the above before doing a thorough review of @jdkaplan 's PR so let's see how that works :-)

@jdkaplan
Copy link
Collaborator

(I don't know how reacts show up in the Zulip API- same as messages? Similar enough to run most of the same logic?)

Adding some links because I happened to be browsing these sections of the docs recently:

It looks like we get a reactions: {user_id: number, ...}[] field in the webhook body. A quick test seems to say that we don't get updates to existing messages this way, though.

Another option seems to be to set up an event queue for op:add. That seems a little more complicated (but worth it!), so I'll consider "emoji reactions as interaction" to be a nice-to-have feature rather than a requirement for now.

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

No branches or pull requests

4 participants