Page MenuHomePhabricator

Ensure user does not see invalid tasks in task queue
Open, HighPublicBUG REPORT

Description

While doing suggested edits on my phone, I noticed an issue:

  • Do a link recommendation edit
  • Go back to the homepage
  • The task you've just done is there, or when going through the task queue, you see the same task again

I would expect to *not* see the same task again.

This type of thing was probably rare before T260522: Optimize number of results requested from API was done, but now is more likely, as there are at most 20 items in the cached task queue for the user at any given time.

I would propose that in a DeferredUpdate after save, we refresh the cache for the user. We don't necessarily need to do it before the PostEdit dialog, because we already have frontend code to ensure the user doesn't see the task they just did. T279025: Add a link: maintain task feed and position on reload/back button is related to this task, in that if a user returns to their task feed without having completed it (e.g., by selecting browser 'back'), the task should be considered still valid.

One option is to use a cache to keep track of tasks that are going to be invalidated: T289854#7341932

Event Timeline

kostajh triaged this task as Medium priority.Sep 3 2021, 10:20 AM
kostajh created this task.
kostajh renamed this task from Saving a suggested edit should refresh cache to Refresh per-user cache of newcomer tasks after a suggested edit is saved.Sep 3 2021, 10:21 AM
kostajh changed the subtype of this task from "Task" to "Bug Report".

This is not related to caching, SearchTaskSuggester::filter() should remove these results and that's called on result sets read from cache. Either the filtering logic is broken or (more likely, per T289854) search index invalidation, which the filtering is based on, is broken.

We might want to refresh the cache nevertheless, it's nice to keep the cache fresh. I think it comes down to a design choice - how much do we want to keep the task list stable? (Ie. do we think the user would prefer or find less confusing if, after doing a task and returning to the home page, they'd see the same results they saw before picking the task?) From that POV, I think this is a good change - previously we relied on randomizing the 200 cached results to pretend a random selection, but with only 20 results cached that doesn't work so well, so rotating them more often would be good.

We might want to refresh the cache nevertheless, it's nice to keep the cache fresh. I think it comes down to a design choice - how much do we want to keep the task list stable? (Ie. do we think the user would prefer or find less confusing if, after doing a task and returning to the home page, they'd see the same results they saw before picking the task?) From that POV, I think this is a good change - previously we relied on randomizing the 200 cached results to pretend a random selection, but with only 20 results cached that doesn't work so well, so rotating them more often would be good.

Yeah, I'm not sure. I believe @RHo wanted to explore having a stable and non-randomized task queue per user; I think we discussed that in another phab task early on in the Add-Link project.

We might want to refresh the cache nevertheless, it's nice to keep the cache fresh. I think it comes down to a design choice - how much do we want to keep the task list stable? (Ie. do we think the user would prefer or find less confusing if, after doing a task and returning to the home page, they'd see the same results they saw before picking the task?) From that POV, I think this is a good change - previously we relied on randomizing the 200 cached results to pretend a random selection, but with only 20 results cached that doesn't work so well, so rotating them more often would be good.

Yeah, I'm not sure. I believe @RHo wanted to explore having a stable and non-randomized task queue per user; I think we discussed that in another phab task early on in the Add-Link project.

Hiya, yes the task is T279025. Should the tasks be merged together for discussion and/or is there a separate bug per @Tgr's comment T290315#7331733 that should be fixed separately?

We might want to refresh the cache nevertheless, it's nice to keep the cache fresh. I think it comes down to a design choice - how much do we want to keep the task list stable? (Ie. do we think the user would prefer or find less confusing if, after doing a task and returning to the home page, they'd see the same results they saw before picking the task?) From that POV, I think this is a good change - previously we relied on randomizing the 200 cached results to pretend a random selection, but with only 20 results cached that doesn't work so well, so rotating them more often would be good.

Yeah, I'm not sure. I believe @RHo wanted to explore having a stable and non-randomized task queue per user; I think we discussed that in another phab task early on in the Add-Link project.

Hiya, yes the task is T279025. Should the tasks be merged together for discussion and/or is there a separate bug per @Tgr's comment T290315#7331733 that should be fixed separately?

Thanks for digging that out. I think we can discuss the stable task queue on T279025 -- even if we implement that, this is a separate issue about ensuring that the user doesn't see invalid tasks in the task queue.

This is not related to caching, SearchTaskSuggester::filter() should remove these results and that's called on result sets read from cache. Either the filtering logic is broken or (more likely, per T289854) search index invalidation, which the filtering is based on, is broken.

I don't know if T289854: Consider removing link recommendation from search index in PageSaveComplete hook instead of SearchDataForIndex means something is broken, just that the job queue processing isn't instantaneous. Per T289854#7341932 maybe we need some kind of cache to keep track of tasks before they're removed from the search index.

kostajh renamed this task from Refresh per-user cache of newcomer tasks after a suggested edit is saved to Ensure user does not see invalid tasks in task queue.Sep 17 2021, 4:28 PM
kostajh updated the task description. (Show Details)
MMiller_WMF raised the priority of this task from Medium to High.Sep 21 2021, 2:19 AM

Change 722593 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/GrowthExperiments@master] Filter out invalid link recommendation tasks

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

Change 725764 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/GrowthExperiments@master] SuggestedEdits: Introduce InvalidTaskTracker

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

Change 725764 abandoned by Kosta Harlan:

[mediawiki/extensions/GrowthExperiments@master] [WIP] SuggestedEdits: Introduce TemporarilyHiddenFilter

Reason:

I can't reproduce the issue I saw earlier that was covered by this and not by the LinkRecommendationFilter, so abandoning for now, and maybe will bring it back of the LinkRecommendationFilter doesn't suffice.

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