Page MenuHomePhabricator

Pending changes: autoreview randomly fails
Open, HighPublic

Description

https://en.wikipedia.org/w/index.php?title=J.S.S._Academy_of_Technical_Education,_Noida&diff=917234861&oldid=914475481&diffmode=source should have been accepted automatically, but wasn't

https://en.wikipedia.org/w/index.php?title=International_Space_Station&dir=prev&offset=20190921172729&limit=20&action=history includes a series of edits by the same user, the first half of which were automatically accepted, before it suddenly stopped being automatically accepted

Event Timeline

Restricted Application added a project: User-DannyS712. · View Herald TranscriptSep 22 2019, 10:37 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
DannyS712 renamed this task from Pending changes: AutoAccept randomly fails to Pending changes: autoreview randomly fails.Sep 24 2019, 4:31 PM
DannyS712 added subscribers: MBH, Base.
MBH added a subscriber: TerraCodes.Sep 29 2019, 9:29 AM

@TerraCodes can you do something? Flagged revs based reviewing is COMPLETELY BROKEN for a week in any wiki where it's installed, including big wikis such as ruwiki and enwiki.

This seems like a permission handling problem, it also results occasionally in users with autoreview right getting a permission denied error on Special:UnreviewedPages.

Jarash added a subscriber: Jarash.Sep 29 2019, 4:36 PM
Zache added a subscriber: Zache.EditedSep 29 2019, 6:12 PM

Based on huwiki configuration I think special:unreviewedpages is allowed only for Administrators and Editors. Users with only autoreview right should see a permission denied error on Special:UnreviewedPages.

Tgr added a comment.Sep 30 2019, 7:00 AM

@Zache maybe autoreview also randomly fails then. In any case the page returns permission denied on some random subset of pageviews.

Zache added a subscriber: Reedy.Sep 30 2019, 7:14 AM

@Reedy do you have any educated guess what is going on here?

Tgr added a comment.Sep 30 2019, 7:51 AM

Based on timing (first huwiki report was Sept 21) and that this doesn't seem to affect core permissions, T223602 would be my first guess - maybe that introduced some kind of indeterminism, which affects FlaggedRevs config which is loaded in a weird unusual way (via flaggedrevs.php)?

Tgr added a comment.Sep 30 2019, 7:54 AM

My other guess would be something session handling related. I get the permission denied screen fairly frequently, but never immediately after a previous (successful or unsuccessful) page load. So maybe it only triggers when the local session has expired and MediaWiki has to renew it? I have no idea how session handling would interfere with permissions, though.

Tgr added a comment.Sep 30 2019, 7:57 AM

I get the permission denied screen fairly frequently, but never immediately after a previous (successful or unsuccessful) page load.

OTOH in the International Space Station example there was very little time between the last successful autoreview and the first failed one...

Zache added a comment.Oct 1 2019, 7:19 AM

I made a small script for myself to see if the UI (javascript) code thinks that I am autoreviewed or not. Script colorifies the usarname on the top of the page and prints a list of the groups on the end of the page.

I would be interested if user is in editor OR sysop group when Special:UnreviewedPages fails .

Copy of the script in huwiki.

Tgr triaged this task as High priority.Oct 1 2019, 9:31 AM
Tgr added a project: Release-Engineering-Team.

I see all the groups I should be seeing (["checkuser", "sysop", "*", "user", "autoconfirmed"]).

The error message says The action you have requested is limited to users in one of the groups: Editors, Reviewers.
What it should say (and does if I load the page in incognito mode) is Administrators, Editors. So this does seem like an issue with site configuration being messed up occasionally.

Bumping priority - while the flagrev issue is not so severe, we don't know if anything else is affected, and if configuration is not loaded consistently, there is potential for worse fallout.

Neolexx added a subscriber: Neolexx.EditedOct 1 2019, 9:46 AM

To the best of my knowledge en-wiki doesn't use FlaggedRevs extension (as a replacement for the native patrol mechanics). Compare

This way either the problem caused by come recent core patrolling bug or these are two very different bugs (like say this one and T234121) with similar end effects.

As the patrolling mechanics of two (at least) major wiki projects is currently completely crewed (to avoid stronger expressions) - I would suggest to raise this bug priority.

Update: oops, did not notice it is already High, sorry.

Zache added a comment.EditedOct 1 2019, 9:52 AM

To the best of my knowledge en-wiki doesn't use FlaggedRevs extension (as a replacement for the native patrol mechanics). Compare

In enwiki it is named as "Pending changes" and in flagged revision configuration name is "protection setup" which means that only small subset of the pages are in FlaggedRevs review loop and it is used for protecting the pages.

Traditional setup like in dewiki, ruwiki, huwiki, fiwiki etc is that whole namespace is included.

In enwiki it is named as "Pending changes" and in flagged revision configuration name is "protection setup" which means that only small subset of the pages are in FlaggedRevs review loop.

Ah, OK.
The current FlaggedRevs usage is highly twisty and very far from what the extension has been written years ago. At least at ru-wiki.
Basically when someone "patrolling" a page, she sets its quality criterion "accuracy" from 0 to 1. There are 3 criteria (accuracy, depth, tone) and 4 levels for each (0, 1, 2, 3) - but it all has been blocked ages ago as not used.
And an added level of logic interprets accuracy==1 as patrolled, accuracy==0 as unpatrolled,
It seems that with some recent changes the system got some kind of A.I. so started to sort out edits to 1) not affecting "accuracy" and 2) affecting "accuracy". Or started to set "accuracy" higher than 1. So expecting user rights no lesser than reviewer. And there are close to none of such users because this side of the project is long time ago abandonned (ru-wiki has maybe 20 of them kept for historical purposes).

Just brainstorming, don't get upset if gibberish.

Zache added a comment.EditedOct 1 2019, 10:30 AM

I tried to check yesterday if the patrolling is working or not too, but there are not many wikis where wgUseRCPatrol and FlaggedRews are both enabled. Also when revision is manually reviewed then it will be patrolled too which will mask the problem.

In de.wiktionary.org there was edit by user with autoreview and autopatrol user rights which was still autopatrolled but not autoreviewed.

select rc_id, rc_title, rc_cur_id, rc_this_oldid, fp_stable, rc_timestamp, rc_patrolled, fr_rev_id FROM flaggedpages, recentchanges LEFT JOIN flaggedrevs ON fr_rev_id=rc_this_oldid WHERE rc_cur_id=fp_page_id AND rc_this_oldid = 7237740;
rc_idrc_titlerc_cur_idrc_this_oldidfp_stablerc_timestamprc_patrolledfr_rev_id
9874103geldi10369272377406070691201909170653442NULL

However the result of my investigation was that even if it looks like that patrolling worked here we cannot know it using the available information.

Tgr added a comment.Oct 1 2019, 11:26 AM

In de.wiktionary.org there was edit by user with autoreview and autopatrol user rights which was still autopatrolled but not autoreviewed.

Nothing seems out of the ordinary there:

08:54, 17 September 2019 Peter Gröbner talk contribs block reviewed a version of geldi (changes reviewed) (revision: 13:52, 30 June 2017)
08:54, 17 September 2019 Peter Gröbner talk contribs block deprecated a version of geldi (changes deprecated) (revision: 08:53, 17 September 2019)
08:53, 17 September 2019 Peter Gröbner talk contribs block deprecated a version of geldi (revision: 13:52, 30 June 2017)
08:53, 17 September 2019 Peter Gröbner talk contribs block automatically reviewed a version of geldi (changes reviewed) (revision: 08:53, 17 September 2019)

In general autoreviews are hard to verify since you have to reconstruct the review state at the time of the edit. Eg. an edit might not get autoreviewed because the parent revision was not reviewed, and then the parent revision might get reviewed later...

Long time ago just for fun I wrote a patrolling script that works straight through the wrapping interfaces: https://ru.wikipedia.org/wiki/User:Neolexx/review.js

So it literally sets article's "accuracy" criterion to 1 or 0, which is reflected in the top interface as patrolled or not. So (tested) in normal circumstances it was not matter to use the script or to press conventional patrol button in the web-interface.

Now it could worth to see this script results on different edits, if fails and where.

And - in continuation of my first mad idea that the engine somehow got a mind of its own - to check ORES scores of edits. To see if there is any connection between edits' scores and their autopatrol failures.

Zache added a comment.EditedOct 1 2019, 1:26 PM

I see all the groups I should be seeing (["checkuser", "sysop", "*", "user", "autoconfirmed"]).
The error message says The action you have requested is limited to users in one of the groups: Editors, Reviewers.
What it should say (and does if I load the page in incognito mode) is Administrators, Editors. So this does seem like an issue with site configuration being messed up occasionally.

"The action you have requested is limited to users in one of the groups: Editors, Reviewers" would happen if wmf-config/flaggedrevs.php configuration for huwiki is not loaded. (this is FR default groups for unreviewedpages)

Also, my guess is that if the file wmf-config/flaggedrevs.php is not loaded at all then data expected by default wgFlaggedRevsTags settings is different compared to previous revision with huwiki settings in db so it would not do an autoreview because that. This is however guessing.

JJMC89 added a subscriber: JJMC89.Oct 1 2019, 2:51 PM

I also saw this on enwiki (protection configuration) on September 23. As a sysop, it told me that I could not accept a pending change. After a hard refresh, I was able to accept it.

Neolexx added a comment.EditedOct 2 2019, 7:03 AM

In general autoreviews are hard to verify since you have to reconstruct the review state at the time of the edit. Eg. an edit might not get autoreviewed because the parent revision was not reviewed, and then the parent revision might get reviewed later...

To be honest, I am in the project since 2008 and patroller since 2011 - but the patrolling part remains beyond my intellectual capabilities. I just know what to do (say patrol the article after removing DR request which is autopatrolled and the like). But what do I do (in the logical sense) - I still don't know.

Any way and leaving the theoretical questions behind - the engine allows to have articles which are 1) not patrolled but 2) having patrolled edits in them in any random order, including the latest edit.

From the other side FlaggedRevs by design is a "whole thing" so applies to the entire article (entire revision version). No one planned to review and set quality levels of accuracy, depth or tone of a separate edit while disregarding the article state in whole. If FlaggedRevs later has been learned to act in patrol-like way - then it is a hack (or OK - added feature). And then at some point all this set of added features just stopped to work.

I do agree that we need more show cases. And actual (unfixed) show cases, because looking in the page history and event journals doesn't help too much. Same with test-wiki - only an actual project with full set of added features may help. The topic is discussed at ru-wiki techforum, I may propose there to allocate a set of guinea pig articles. Something what is not too painful to temporary screw up in the name of science. If goes and new actual samples - I link it here.

MBH added a comment.Oct 2 2019, 7:12 AM

To all discussion participants: don't listen Neolexx. This user can't understand Flagged Revs mechanics, makes false statements about it, confuses himself and others. Two years ago all ruwiki's technically competent users in the amount of more than five people were trying to explain him simple FR mechanics, but failed. Don't listen him and his false statements, for example about "engine allows to have articles which are not patrolled but having patrolled last edit" (this statement is clearly wrong).

Neolexx added a comment.EditedOct 2 2019, 8:45 AM

Don't listen him and his false statements, for example about "engine allows to have articles which are not patrolled but having patrolled last edit" (this statement is clearly wrong).

It is funny to read clearly wrong statement with a "clearly wrong" comment about the right statement. But we should excuse MBH as he's one of ru-wiki users with reviewer rights (left for historical purposes, 26 users in total). So his "patrolling" interface and mechanics differs to others but it is easy to forget.

Also if there is any behind logic in this bug and it is connected with FlaggedRews features - then the bug should never happen with edits of users from the group reviewer If they are sometimes affected as well the the situation even more mysterious.

MBH added a comment.Oct 2 2019, 11:43 AM

Of course, this bug should never happen for users with "reviewer" rights, 'cause it happens only for users with "autoeditor" right, not "editor" and not "reviewer".

As so far there are very few of "of course" things in it. Let's us take fact that "patrolling" is just an imaginary thing existing only as a social contract and nothing else.
The only real thing is "reviewing" the whole page (not a given edit) accuracy for its states "not acceptable" (0) and "acceptable" (1). And any "patroller" has right to set it to 0 or 1 - but only reviewer may set it to 2, 3 or 1001. So now what may happen if reviewer "patrolled" a page as accuracy==2 and some other "patroller" made an edit after that? Technically it is a request to change the "accuracy" criterion level beyond the allowance (0 or 1) - so the refusal. Just keep brainstorming, don't get too upset.

but only reviewer may set it to 2, 3 or 1001.

OK, purely technically wrong, even reviewer cannot set it higher than 2(?) The point is that the underlying engine recognizes only zero accuracy state ("not patrolled") and non-zero accuracy state ("patrolled"). And anything else falls under the current editor rights and so we come.

Fresh to whatever UTC-related time (you figure out) problematic article states: https://ru.wikipedia.org/wiki/Википедия:Форум/Технический#samples01

In general, please follow the Phabricator etiquette: "Criticize ideas, not people." Thanks for keeping discussions respectful and without getting personal here.

I got an idea of what may happen. If I'm right then all problematic articles have been once "patrollled" by setting their "quality" to 0 like
https://ru.wikipedia.org/w/api.php?format=xmlfm&action=query&list=reviewedpages&rpnamespace=0&rpfilterlevel=0&rplimit=1&rpstart=884521
or
https://ru.wikipedia.org/w/api.php?format=xmlfm&action=query&list=reviewedpages&rpnamespace=0&rpfilterlevel=0&rplimit=1&rpstart=5325503

If not and there ara also problems with "quality" 1 - then I'm wrong and no use to go into details.

FlaggedRevs currently doesn't have a clear maintainer (T185664) making things even more complicated.

DannyS712 moved this task from Unsorted to Others on the User-DannyS712 board.Oct 4 2019, 1:11 AM
Stryn added a subscriber: Stryn.Oct 4 2019, 4:35 AM
Neolexx added a comment.EditedOct 5 2019, 2:45 PM

Since Sep.23 (this bag open by en-wiki user) it seems that there is not any progress. At ru-wiki Землеройкин summed up some facts: "Кратко суммирую факты" ("Summing up some facts").

IMHO more properly would be to say not facts (like some definitely proven things) but like educated guesses made by observing a large set of affected articles: https://ru.wikipedia.org/wiki/Википедия:Форум/Технический#samples01
The majority of them for the moment of this post left unfixed for the purpose of further testing if needed. So the facts are (and we remember that FlaggedRevs installation and usage is different between ru-wiki and en-wiki):

That's all for now.

Neolexx added a comment.EditedOct 5 2019, 3:00 PM

I got an idea of what may happen.

That idea appeared to be plain wrong.
As a round #2 :-) - illogical and random character of the problem reminds me T224491 Which is marked as fixed but maybe that PHP 7 quirk came back here? So sometimes some edit context gets corrupted by one char (say "review" becomes "reviev" or something)?

Samat added a subscriber: Samat.Oct 6 2019, 8:36 AM
MBH added a comment.Oct 13 2019, 7:52 AM

@DannyS712 @Tgr @Zache I wrote a bot that reviews all edit sequences that was not reviewed due to this bug. I wrote it for ruwiki, but if you want, I can launch it for your wikis too, if your wikis grant "editor" flag to my bot.

Zache added a comment.Oct 13 2019, 8:17 AM

@DannyS712 @Tgr @Zache I wrote a bot that reviews all edit sequences that was not reviewed due to this bug. I wrote it for ruwiki, but if you want, I can launch it for your wikis too, if your wikis grant "editor" flag to my bot.

Thank you for your offer. However, in fiwiki SeulojaBot does this already as part of it's approval workflow.

@DannyS712 @Tgr @Zache I wrote a bot that reviews all edit sequences that was not reviewed due to this bug. I wrote it for ruwiki, but if you want, I can launch it for your wikis too, if your wikis grant "editor" flag to my bot.

Is the source code public?

https://tools.wmflabs.org/mbh/patrol.txt (C#)
Code is dirty, but working.

Question: the resulting API call seems to just be written to the console; is there no validation to ensure the call was a success?
It should be pretty easy to get approval, although you will need to localize the summary message. Also, on WMF wikis using [[phab:T233561]] as the link should work instead of the full hyperlink, and it is generally (imo) clearer

binbot added a subscriber: binbot.Mon, Oct 21, 5:40 AM
binbot added a comment.EditedMon, Oct 21, 5:52 AM

Other phenomena in huwiki not mentioned by this time:

  • the "undo" link is randomly missing
  • the Accept revision, Reject changes buttons, comment field and the whole box around them are missing.

In my experience reloading the page always helps.

Just a thought: for me it seems like the beginning of the problems in time may be close to the beginning of frequent HTTP 503 server errors. I can't prove it, this is just an idea. May there be any connection?