Edits which have the Bot flag should never be reverted by Automoderator - they are edits performed automatically by trusted individuals and therefore Revert Risk's judgement of them is not something we should care about.
Acceptance criteria
| Subject | Author | Repo | Branch | Lines +/- | |
|---|---|---|---|---|---|
| Skip bot accounts | Scardenasmolinar | mediawiki/extensions/AutoModerator | master | +26 -2 |
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Samwalton9-WMF | T336934 Enable communities to configure automated reversion of bad edits | |||
| Resolved | Samwalton9-WMF | T358228 Deploy AutoModerator as a pilot on one Wikimedia project | |||
| Resolved | Samwalton9-WMF | T349598 Create Automoderator extension | |||
| Resolved | None | T352437 Create Automoderator's edit reverting functionality | |||
| Resolved | • Scardenasmolinar | T352440 Automoderator should not revert bot edits |
We have a problem: the bot flag is stored in the recentchanges table, and at the moment we make the pre-check, I don't think the table has been updated. Let's use the user group way to check for a bot because it will be as accurate. For example, if an account is a bot account when the edit is made, it will be skipped, but if they change the account to be a non-bot, then the next time they make an edit, it will not be skipped.
Change 1007751 had a related patch set uploaded (by Scardenasmolinar; author: Scardenasmolinar):
[mediawiki/extensions/AutoModerator@master] Skip bot accounts
I'm thinking this through: this works in practice because we're running this really close to edit time. Meaning that for now the window for user rights to change between the edit save and our check is very small. Ultimately we're going to run these out of a job, so maybe the job code could block on a check for the rev being present in the rc table, or we could be firing from RecentChange_save.
We're using the RevisionFromEditComplete hook because it was a handy place to start with the prototype. I think rc rows have most of what we care about (including that bot flag) with the exception of the tags. I think a lot of implementations of this hook set tags, so we'd need to check to see where the tags we care about (eg. revert tags) are set, so that we aren't in the same situations with tags in recent changes as we are with the bot flag in revision edit complete.
Should we create a new spike about changing hooks or should this be addressed in this ticket?
Let's do that in another task. I think the solution in the patch is reasonable for the current hook.
Change 1007751 merged by jenkins-bot:
[mediawiki/extensions/AutoModerator@master] Skip bot accounts
Why should this exception use the bot flag rather than bot membership? If admins are exempted based on group membership, why should this be changed for bots? As for the window, it’s quite uncommon to add a user to the bot group or remove it from the group in the middle of an editing session. If a user gets added to the bot group, it usually happens after at least a few hours of inactivity after the last test edit, and it also means that it’s generally trusted – both its edits that it makes after being added and the ones that it has made before that. If a bot is malfunctioning or the community doesn’t trust it for some other reason anymore, it’s blocked, not removed from the group. Removal from the group usually happens after months or years of complete inactivity – well, if the jobs haven’t been processed for months, we have bigger problems than whether the edit gets automoderated. 🙂
We are looking at using recent changes as our starting point rather than revisions, and the bot flag is stored with recent changes. It looks like the most reliable way to make the determination. Even if it's only better by a small margin, why not?
If admins are exempted based on group membership, why should this be changed for bots? As for the window, it’s quite uncommon to add a user to the bot group or remove it from the group in the middle of an editing session. If a user gets added to the bot group, it usually happens after at least a few hours of inactivity after the last test edit, and it also means that it’s generally trusted – both its edits that it makes after being added and the ones that it has made before that. If a bot is malfunctioning or the community doesn’t trust it for some other reason anymore, it’s blocked, not removed from the group. Removal from the group usually happens after months or years of complete inactivity – well, if the jobs haven’t been processed for months, we have bigger problems than whether the edit gets automoderated. 🙂
100% agree, which is why we're using the user group now.
Because it’s inconsistent, and inconsistency is confusing. (E.g. “Why did AutoModerator revert this bot edit? Bots and admins are exempted, aren’t they? ― Oh, so because it wasn’t marked as a bot edit? But admins’ edits aren’t marked as bot edits either, yet they’re still exempted.”)
This is a good point. I think we're getting into product territory here, so I'll loop @Samwalton9-WMF into the conversation.
Thanks for this conversation. It brought up something I'd missed when initially scoping this ticket, which is that in the front-end we likely want to communicate this behaviour via user groups (alongside the admin group) for simplicity. I agree that user groups is the right way to approach this from a UX perspective.