Page MenuHomePhabricator

Wikispeech doesn't work on SVWP
Closed, ResolvedPublicBUG REPORT

Description

In most, maybe 80–90 percent, of a large number of randomly chosen articles can't be read, and the user don't even get an error message when clicking the play button. For a smaller share, maybe 10–20 percent, the page title is read aloud, but instead of the body text then being read out, a dialog box pops up with the following error message:

"Could not load audio
This can happen due to a poor internet connection or a server error. Would you like to try again or stop playback?"

Event Timeline

There are a few things I've discovered now that I've been troubleshooting, and I've found where the biggest root problem was:

  1. The issue where you basically couldn't play back any articles at all, i.e. the most urgent part, should now be fixed. That is, Wikispeech wasn't playing back anything at all on SVWP. Some other users have tested this and it seems to be working.
  • The solution: Users who had turned parts of content off got it silently switched on, which also disabled word highlighting. Deleting their preferences page made Wikispeech work again. That last part is the big part: we store wikispeechPartOfContent as the string '1'/'0', but the action API's 'boolean' type is PresenceBooleanDef, which reads any present value as true, including "0". With no saved preferences the value falls back to the boolean default false, which api drops from the request entirely, so the parameter is absent and the feature is genuinely off. ApiWikispeechSegment now reads the raw request value and interprets it against BooleanDef::FALSEVALS, and ext.wikispeech.highlighter.js compares explicitly against true/'1' rather than relying on js-truthiness, where "0" is also true.

This fix is now an open patch for review, but I had to make this a quick fix, and added the little code needed on the producer server to make this acute fix. The code added on the server is the code within that patch (except for the test-file)

  1. The second part is that certain characters or character combinations seem to cause playback to stop such as in the article: Valleydale_500_1981. I've tested this against the new speech synthesis, where it works. The rollout of the new speech synthesis to SVWP has already been underway since last week. Once that's complete, the character-combination issue should resolve itself. See: T435966 and T435967
  1. Another part concerns reading aloud other types of content (POC), such as links. That functionality works on both the demo server and the test server, but not on the production server, and I might have a small theory about what's causing that. I consider this part less urgent, since playback works fine if that setting is switched off, but here is a theory:
  • wikispeech-listen is never told whether parts of content is enabled, so it re segments the page without POC and can't resolve the hashes the client was given, hence a RuntimeException.

Feel free to check out the full discussion at the Bybrunnen from today: Bybrunnen: What's happening with Wikispeech?

Change #1332194 had a related patch set uploaded (by Viktoria Hillerud WMSE; author: Viktoria Hillerud WMSE):

[mediawiki/extensions/Wikispeech@master] Handle part-of-content consistently as a boolean

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

wikispeech-listen was never told whether parts of content was enabled. Segment hashes are decided from the text of the segment, and POC changes that text by inserting announcements like "länk:". A hash created with the setting on therefore does not exist in a segmentation done with it off. getUtteranceForRevisionAndSegment() always segmented without the setting, so it could never find the hash the client had been given. That is the "No such segment" RuntimeException.

The setting is now sent with the listen request, accepted by the API and passed into the segmentation. Both wikispeech-segment and wikispeech-listen read it through a new shared ValueBoolean::fromRequest(), so the boolean problem from the first part cannot reappear on the new parameter.

Why then did it work on our demo and testwiki...?

  1. The segments cache key did not include parts of content, so both variants shared one entry. wikispeech-listen picked up the "parts of content on" segments that segment had just stored, never resegmented the page, and the hash matched.
  1. On production that cache hit does not happen: it's somewhat handled differently or the entry has expired. The page is segmented again without the setting and the hashes do not match.
  1. partOfContent is now part of the cache key. It's possible that the demo and testwiki would have started failing if the cache key had been fixed on its own, without sending the setting to the listen API.

Still broken: parts of content crashes on any article containing a table, this will be handled separately in T424044.

It could be worth it to split this into two separate taks and/or patches. While both contribute to things not working they're not really related. At least I think so?

Starting with the API parameters (T436431#12268126). This solution would create an exception to how the action API works, which we want to avoid. Judging by the documentation the behaviour is intentional.

I think it would be better to solve the issue with writing, reading and handling parameters on the config wiki page. It's a bit messy since we deal with different formats at different stages. Something like:

  • Save wikispeechPartOfContent as a JSON boolean in sharedUserOptionSettings.js. Now it's either saved as "0" or "1".
  • When requests are made to wikispeech-segment and wikispeech-listen convert the parameter value. If it's true set it to a non-empty string. The API sandbox uses 1 so probably go with that. If it's anything else (false, any other type or missing) leave it out. Even the empty string is truthy here.
  • Change how the option dialogue reads the value. It should be enough to remove the conversion from string to boolean. Maybe needs to handle anything other than true as false.

That way we don't need to touch the API, which is behaving as intended. Legacy config pages should be handled fine (and I don't think they're an issue at this point). If you had changed POC in the preferences dialogue it will be read as false until you set it again.

Yes you're right. The parameter handling and listen not being told about the setting are unrelated, even if they touch the same grounds.

And you're right about the API. I read PARAM_TYPE => 'boolean' as an ordinary boolean and never checked what it maps to, so I took the behaviour for a bug and worked around it.. The client is totally the place to handle it.

Is this the split you meant?:

  • One patch does the parameter handling client side and drops ValueBoolean, leaving the API alone.
  • A second one adds part-of-content to wikispeech-listen and puts partOfContent in the cache key. Those two belong together since the cache key is why it worked on demo and testwiki, fixing it alone would just have made them fail too.

Yes, that was the split I was thinking of. I haven't looked to closely at the code, but I got the impression that they could be separated.

Starting with the first patch, the parameter handling (JSON boolean in sharedUserOptionSettings.js) :

It works on the consumer but not on the producer, and it seems to come down to where the value is stored.

On the consumer it's the JSON page. JSON has real booleans, so true comes back as true and === true matches.

On the producer it's user preferences, and core casts every option with (string) before saving it (UserOptionsManager::saveOptionsInternal). So true is stored as 1 and read back as '1'. === true is never true there, and saving again doesn't help, and whatever you write comes back as text.

So i think I'll convert the value to a real boolean when the options are loaded, in addUserOptions. Then it doesn't matter which of the two it came from, and the rest of the code can just check === true.

Change #1332194 abandoned by Viktoria Hillerud WMSE:

[mediawiki/extensions/Wikispeech@master] Fix part-of-content boolean handling and playback

Reason:

Will be divided into separate patches

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

Change #1337823 had a related patch set uploaded (by Viktoria Hillerud WMSE; author: Viktoria Hillerud WMSE):

[mediawiki/extensions/Wikispeech@master] Only send POC when it is turned on

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

Change #1337874 had a related patch set uploaded (by Viktoria Hillerud WMSE; author: Viktoria Hillerud WMSE):

[mediawiki/extensions/Wikispeech@master] Tell wikispeech-listen whether POC is on

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

For the second part, wikispeech-listen:

It now takes part-of-content and passes it into the segmentation, and the client sends it with the listen request. POC is also part of the segments cache key, so the two segmentations no longer share an entry.

Tested on top of the first patch but uploaded against master, so it has to go in after that one. Until then the client won't actually send the parameter, since the setting isn't stored as a real boolean yet.

Change #1337823 merged by jenkins-bot:

[mediawiki/extensions/Wikispeech@master] Only send POC when it is turned on

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

I encountered a strange thing when setting POC on the preferences page (not our dialogue). When you check the box for POC its value in options (checked in browser console with mw.user.options) is "1". When you uncheck it it's false. However if you do the same with the setting for enabling highlighting you get true and "" respectively.

I see no reason why they should be different or why neither of them isn't just true or false. With the extra conversion we do it works and I don't think it should cause any issues. It does seem like very odd behaviour in MW.

I encountered a strange thing when setting POC on the preferences page (not our dialogue). When you check the box for POC its value in options (checked in browser console with mw.user.options) is "1". When you uncheck it it's false. However if you do the same with the setting for enabling highlighting you get true and "" respectively.

I see no reason why they should be different or why neither of them isn't just true or false. With the extra conversion we do it works and I don't think it should cause any issues. It does seem like very odd behaviour in MW.

Could it be the defaults? POC defaults to false, highlighting to true. When a value matches its default MW deletes the row and you get the default back as a real boolean, when it differs you get the string?


The poc-listen patch is now rebased on top of master (which holds the poc-parameterpatch), and is up for review.

Could it be the defaults? POC defaults to false, highlighting to true. When a value matches its default MW deletes the row and you get the default back as a real boolean, when it differs you get the string?

I don't think so. I tried removing $out->addModules( [ 'ext.wikispeech' ] ); in onBeforePageDisplay() and still get the same.

The poc-listen patch is now rebased on top of master (which holds the poc-parameterpatch), and is up for review.

Were you able to recreate the issue on the producer and verify that this solved it? I tried to run the same version of Wikispeech locally and using that as producer, but then I didn't encounter the issue on the page. I'm a bit worried that there's something else going on since neither the demo nor test wiki had the same issue.

Change #1337874 merged by jenkins-bot:

[mediawiki/extensions/Wikispeech@master] Tell wikispeech-listen whether POC is on

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

With this merged, I think its time to apply it to the producer wiki, or is it better to wait until this is done?: https://phabricator.wikimedia.org/T435966 @Sebastian_Berlin-WMSE

Though I think it could be good to do it and test it now, so we know it works when the restructuring is done..

If we apply it now, is it best to do a new version to upload? E.g v0.1.16. I will also have to remove the code applied directly on the server a few weeks ago, and I know what to remove and to restore it.

Since we're switching server anyway in T435966 I think we may as well wait for that and keep the current server as it is. As for testing we can use the (new) development wiki for that. You can do this by setting the producer URL to https://wikispeech-dev.wmcloud.org/w/. It's running from a Git repo and I just updated it.

When we're happy with testing we should make a new version to use on the new producer wiki once it's up.

Ok, but just to keep in mind that this was never reproducible on the test or demo wiki either.
So would it make sense to force a cache miss on the dev wiki, like I did locally by lowering the segments TTL from an hour to 30 seconds? Otherwise I'm not sure the testing will tell us much.

Feel free to make any changes to the dev wiki you need to test. That's what it's there for 🙂

Tested against the dev wiki as producer onto svwp, with CACHE_NONE so the cache can't hide anything. POC is now sent with both wikispeech-segment and wikispeech-listen, the segment is found and the page plays. The same setup gave "No such segment" every time before the patch. so this should be working!

(The voices on the dev wiki have different names than on the old producer, so you have to pick the default voice in the dialogue first)