Change 730414 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/GrowthExperiments@master] Add an Image: Finalize log entry text

https://gerrit.wikimedia.org/r/730414

@MMiller_WMF is logging skips important? Currently we don't save the page when skipping so the server doesn't even learn that it happens. IIRC we also don't log skipping (when all links are skipped) in Add Link.

Change 730414 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Add an Image: Finalize log entry text

https://gerrit.wikimedia.org/r/730414

@Tgr -- it is okay not to log skips because we'll be picking it up in EventLogging. I'll change the task description.

Checked on betalbs and testwiki wmf.7 - accepted & rejected image actions are recorded in Special:Log - GrowthExperiments. Special:RecentChanges and Special:Contributions won't display rejected image actions.

@MMiller_WMF, @Tgr - is it acceptable? Also, presently testwiki would not add any tags at all to the rejected image actions, so it's better to test in betalabs for now.
Note rejected image action won't have any tags associated with Structured tasks:

After speaking to @Tgr, we decided that we should remove the tags from all log entries.

So what's happening is:

• the revision gets saved
• the log entry gets saved, with associated_rev_id set
• the recentchanges entry gets saved; RecentChanges::save() calls ChangeTags::updateTags() with the rc_id and rev_id; the logic in that method looks up the corresponding log_id using associated_rev_id, and uses all three IDs to save the change tag record. That means the tag will be associated with both the revision and the log entry.

So in essence it is not possible to create a log entry that has an associated revision but separate change tags. This is clearly intentional - it was added in rMW68cc94540d1d: ChangeTags: Teach updateTags() to derive log_id from rev_id (and the other way), although that doesn't have much in way of explanation.

I see three ways to prevent tagging the log entry:

1. change the behavior of ChangeTags::updateTags() to not look up the matching log record
2. delay the creation of the log entry until after the recentchanges entry has been published
3. do not associate the revision ID with the log ID

The first is scary, this code is used for all edits and log entries, it could break many things; and probably unwanted.
The second seems like a fragile hack, quite possibly the tags might be readded over time (change tags can be edited or otherwise changed in a subsequent request, and that would trigger the same lines of code) and moves the logging to a separate transaction, making it more fragile. (Although it probably has some performance benefits.)
The third just feels conceptually incorrect (after all the log entry *is* associated to the revision) and would make using the log entries for analytics harder.

Overall I'm not sure this is worth doing. @kostajh what do you think?

Change 738493 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/GrowthExperiments@master] Structured tasks: Document setAssociatedRevId side effects

https://gerrit.wikimedia.org/r/738493

Provisionally added some documentation about it.

(There is also a fifth option, besides the three above + not doing anything: manually tag rejections as well. I doubt people would care about tags though, since these entries are not included in recent changes.)

In T293169#7496787, @MMiller_WMF wrote:

After speaking to @Tgr, we decided that we should remove the tags from all log entries.

Can you provide more context for this, please – why do we need to remove the tags for the log entries? I'm asking because the options we have aren't great, and "do nothing" about this seems like the best option given our current backlog.

Change 738493 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Structured tasks: Document setAssociatedRevId side effects

https://gerrit.wikimedia.org/r/738493

In T293169#7502872, @kostajh wrote:

In T293169#7496787, @MMiller_WMF wrote:

After speaking to @Tgr, we decided that we should remove the tags from all log entries.

Can you provide more context for this, please – why do we need to remove the tags for the log entries? I'm asking because the options we have aren't great, and "do nothing" about this seems like the best option given our current backlog.

@kostajh -- I never understood this well enough to explain. @Tgr, could you? Or perhaps you already talked about it offline.

In T293169#7505648, @MMiller_WMF wrote:

@kostajh -- I never understood this well enough to explain. @Tgr, could you? Or perhaps you already talked about it offline.

I first thought this is a simple bug in our code (that the hook for adding the tags would run for both the edit and the log entry and we don't check which), so it would have been more of a "why not?" than a "why?". But per T293169#7497897 that's not actually the case - our hook handling is fine, but MediaWiki core kind of handles an edit and a log entry referencing that edit as a single unit. So we don't have an easy fix (it wouldn't even really be a "fix" in the sense that we aren't doing anything incorrect).

Not sure if we had a stronger reason than that. The tags certainly aren't useful (in general change tags are for filtering changes lists - recent changes, watchlist, user contributions etc - and Special:Log, but we don't send our log events to recentchanges/watchlist on account of them not really being informative; user contributions don't include logs; and Special:Log can already be filtered by log type so the tags are somewhat superfluous) but they don't do any harm either.

Reading back, my comments about this weren't clear, sorry. @MMiller_WMF I'm recommending we leave this as is. What first seemed like a bug in our code turned out to be a weird edge case in MediaWiki core - log events sort of inherit change tags from the associated page history revisions. We don't have a good way to prevent it from happening, and I don't think it matters enough to try to prevent it in a bad way.