Page MenuHomePhabricator

Chinese conversion no longer work in the table of content
Open, HighPublicBUG REPORT

Description

List of steps to reproduce:

What happens?:
Since yesterday, multiple users have reported that Chinese conversion no longer functions in ToC on Chinese Wikipedia.

image.png (1×662 px, 179 KB)

What should have happened instead?:
ToC is converted.

Software version (if not a Wikimedia wiki), browser information, screenshots, other information, etc:
1.38.0-wmf.7 (7dc8191)
2021-11-02T17:55:02

Event Timeline

Neither https://zh.wikipedia.org/zh-hans/任天堂Switch nor https://zh.wikipedia.org/zh-hant/任天堂Switch produces the correct conversion. The main text, however, is converted correctly.

@Jdlrobson @cscott FYI. I suppose this is related to the TOC work that got rolled out recently.

Could someone link me to the code that is responsible for this conversion?

Legoktm triaged this task as Unbreak Now! priority.Fri, Nov 5, 7:41 PM
Legoktm added a subscriber: Legoktm.

Could someone link me to the code that is responsible for this conversion?

It's the language converter.

Mentioned in SAL (#wikimedia-operations) [2021-11-05T20:09:40Z] <dduvall> rolling back 1.38.0-wmf.7 from all wikis due to UBN T295187 (refs T293948)

I've rolled back group2. However, there are Chinese language wikis present in group1 that are still on wmf.7. Please advice whether group1 should also be rolled back.

I would assume this affects all language converter-enabled languages, not just Chinese fwiw.

Mentioned in SAL (#wikimedia-operations) [2021-11-05T22:19:24Z] <dduvall> rolling back 1.38.0-wmf.7 from group1 and group0 due to UBN T295187 (refs T293948)

Tldr: rolling back has resulted in table of contents disappearing from many pages.

Screenshot_20211105-152222_Firefox.jpg (2×1 px, 746 KB)

I understand this is LanguageConverter. I am just having difficulty locating the exact line of code that does the actual conversion. My guess is that it's running on ParserOutput on content before the table of contents is generated. If that's the case we need to make sure the converter runs on the table of contents as well which is now stored separately as of wmf7.

I'm AFK this weekend but rolling back resulted in the table of contents disappearing from pages so please monitor WVP. Example page https://en.wikipedia.org/wiki/Wagon_Wheel_(song) has no table of contents at time of writing for anonymous users.

Cached parser output will have the TOC placeholder marker in it, but no method to inject table of contents. ParserOutput generated in wmf6 is not the same as wmf7. I can't verify that now as I'm heading out for the weekend so can't check this now. Clearing parser cache is now needed.

There were also a few unbreak nows broken in last week that probably should be backported now to wmf.6.

Will check in on Monday when I'm back but hope somebody can clear this up.

Would suggest restoring wmf7. The language converter issue is bad but surely no table of contents is even worse?

Yes, I think so. Let us move i forward to wmf7. Broken TOC on a a subset of pages on lang-converter wikis is better than no TOC on many wikis.

I can work on a patch to restore language conversion in toc, but only purging pages from the parsercache can fix the tocs disappeared by the rollback. (So I agree that the best fix is a roll-forward and patch.)

Probably we should have done this in two phases, so we had the code present in <version-1> to handle both types of parsercache content.

The Table of Contents of several big Wikipedia articles (in english) are no longer visible for me. I was told it may be related to attempts to fix the problem at hand here.

Screenshot 2021-11-05 at 23-04-46 Fumio Kishida - Wikipedia.png (1×1 px, 238 KB)

Mentioned in SAL (#wikimedia-operations) [2021-11-05T22:32:25Z] <dduvall> re-rolling 1.38.0-wmf.7 to all wikis due to a better of two evil regressions UBN T295187 (refs T293948)

For future reference, if there is something that has an effect such as "ParserOutput generated in wmf6 is not the same as wmf7", please list it as a risky change in the deployment blockers task so we are aware. If we cannot safely rollback upon regression, that puts deployers in a pretty tight spot to say the least.

Change 737150 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] Regression fix: Ensure ToC is properly language-converted

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

I can work on a patch to restore language conversion in toc, but only purging pages from the parsercache can fix the tocs disappeared by the rollback. (So I agree that the best fix is a roll-forward and patch.)

Probably we should have done this in two phases, so we had the code present in <version-1> to handle both types of parsercache content.

There's no #Content-Transformers tag yet, where should this live, team project-wise?

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/737150 is the quick and dirty fix, but doesn't do anything for pages which are already stored with a non-language-converted TOC in the ParserCache (which is all of them). It will fix the issue as pages are purged from ParserCache, though.

I'll try to fix this "properly" as well, but it's worth reviewing this patch and seeing if it's "good enough for the weekend".

Change 737153 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] WIP: put language conversion for ToC in the 'right' place

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

Tested 737150 locally and it seems to work. Tested 737153 as well, and it works. I think 737153 is preferable as it fixes even "old" parser cache entries.

Need to get a review & vote on which is preferable, and perhaps a cherry-pick to production if we don't want to wait for the next train.

Tested locally by setting $wgLanguageCode='crh'; in my LocalSettings.php and switching to Cyrillic; for someone illiterate in Chinese this is a much easier script to use to verify that language conversion is working.

Change 737079 had a related patch set uploaded (by Dduvall; author: C. Scott Ananian):

[mediawiki/core@wmf/1.38.0-wmf.7] Regression fix: do language conversion on ToC in ParserOutput::getText()

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

Change 737153 merged by jenkins-bot:

[mediawiki/core@master] Regression fix: do language conversion on ToC in ParserOutput::getText()

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

Change 737079 merged by jenkins-bot:

[mediawiki/core@wmf/1.38.0-wmf.7] Regression fix: do language conversion on ToC in ParserOutput::getText()

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

Mentioned in SAL (#wikimedia-operations) [2021-11-06T01:43:43Z] <dduvall@deploy1002> Synchronized php-1.38.0-wmf.7/includes/parser/ParserOutput.php: Backport: [[gerrit:737079|Regression fix: do language conversion on ToC in ParserOutput::getText() (T295187)]] (duration: 00m 56s)

Change 737150 abandoned by C. Scott Ananian:

[mediawiki/core@master] Regression fix: Ensure ToC is properly language-converted

Reason:

Abandoned in favor of Ic14b3a49a8ee7ed600485d4f8a363a206035a847

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

Legoktm lowered the priority of this task from Unbreak Now! to High.Sat, Nov 6, 1:54 AM
Legoktm added a project: Wikimedia-Incident.

The issue should be fixed now, but I'm leaving this open since there's some follow-up work to be done with parser tests to prevent this kind of regression in the future. I think an incident doc would be nice to have since there seem to be things that could be learned from this, like how to make a patch like this safe to revert without requiring parser cache invalidation.

T295227 has a bit different outcome than T295187, however it's the same underlying issue due to reverting those changes which originally triggered T295003.

SD_hehua raised the priority of this task from High to Needs Triage.Mon, Nov 8, 2:25 PM

Please resolve the problem on zh wikivoyage (T295227 )!

Please resolve the problem on zh wikivoyage (T295227 )!

This is unrelated. Please see my advice on T295227#7489787

Thanks all for handling this so late on Friday and +1 to writing an incident report. @cscott and I will be looking into any remaining issues today. Please ping us directly on Phab/IRC/Slack if you see any other bugs that could be related.

For future reference, if there is something that has an effect such as "ParserOutput generated in wmf6 is not the same as wmf7", please list it as a risky change in the deployment blockers task so we are aware. If we cannot safely rollback upon regression, that puts deployers in a pretty tight spot to say the least.

Unfortunately, I only realized this was risky while spending the early part of Friday heads down debugging this after realizing the LanguageConverter operated on table of contents, and alerting @cscott we'd introduced a bug. In hindsight, I think any parser change should be flagged as risky going forward given a lot of its hidden complexities regardless of whether we know that or not. I'll make sure that goes into the incident report.

Unfortunately, I did know prior to the rollback happening, so the follow-up issue of the missing table of contents could have been prevented. I wasn't expecting us to roll back the train, so I didn't raise the alarm. I'll make sure this is noted in the incident report as well. Seems like we could improve processes here.

Also for the incident report: In terms of code matters, we definitely need to cover this with some kind of test going forward and probably some tests for cached parser content. Having a project that uses LanguageConverter in group 1 wikis and having a beta cluster version enabled would have also helped flag this earlier.

Having a project that uses LanguageConverter in group 1 wikis and having a beta cluster version enabled would have also helped flag this earlier.

Actually, we do have a group 1 wiki that uses LanguageConverter in beta cluster since 2020: Chinese Wikivoyage Beta Cluster

Technically the bug should have been visible on all non-wikipedia chinese language sites, which are all in group 1. But unfortunately they don't have very high visibility.

I'm going to rephrase and repost this later today