search

recursecenter / pairing-bot

A Zulip bot that partners people for pair programming practice
MIT License
20 stars 13 forks source link

Deregister inactive users #75

Open cceckman opened 2 weeks ago

cceckman commented 2 weeks ago

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.

anna-hope commented 1 week ago

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 commented 6 days ago

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 commented 5 days ago

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:

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 commented 4 days ago

(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.