Page MenuHomePhabricator

Add Link: Ensure user does not see invalid tasks in task queue
Closed, ResolvedPublicBUG 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 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

kostajh renamed this task from Ensure user does not see invalid tasks in task queue to Add Link: Ensure user does not see invalid tasks in task queue.Nov 15 2021, 7:39 AM
kostajh added a project: Add-Link.

Change 722593 merged by jenkins-bot:

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

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

Etonkovidova subscribed.

On betalabs there are quite few tasks that are stuck in the SE feed. Even if I open them and click on "Back to suggested edits", the tasks don't get removed (e.g. "Technology+Add a suggested image" filters). I guess it's a specifically betalabs problems with testing data set. Otherwise, normal tasks that were accepted/rejected get filtered out from the feed.

Moving to Test in Production.

On betalabs there are quite few tasks that are stuck in the SE feed. Even if I open them and click on "Back to suggested edits", the tasks don't get removed (e.g. "Technology+Add a suggested image" filters). I guess it's a specifically betalabs problems with testing data set. Otherwise, normal tasks that were accepted/rejected get filtered out from the feed.

Moving to Test in Production.

They will only be removed from your feed if you accept/reject (and for rejection, the reasons of "undecided" or "foreign language" will not remove from your feed). AFAIK, there is not a business rule that says to remove the task from the feed if the user taps on it.

On betalabs there are quite few tasks that are stuck in the SE feed. Even if I open them and click on "Back to suggested edits", the tasks don't get removed (e.g. "Technology+Add a suggested image" filters). I guess it's a specifically betalabs problems with testing data set. Otherwise, normal tasks that were accepted/rejected get filtered out from the feed.

Moving to Test in Production.

They will only be removed from your feed if you accept/reject (and for rejection, the reasons of "undecided" or "foreign language" will not remove from your feed). AFAIK, there is not a business rule that says to remove the task from the feed if the user taps on it.

Ahhh I think this may be an oversight/miscommunication about the rejection reasons, but my expectation is that for the rejection reasons where it is not removed from the pool (covered in T290046), it is still no longer shown in the specific person's tasks but remain available for others. Perhaps something for @MMiller_WMF to clarify?

@RHo one fairly simple thing to do (@Tgr proposed this in T290315#7331733) is to refresh the user's task feed after accept or reject action. Should we do that? That would take care of the problem, I think.

@RHo one fairly simple thing to do (@Tgr proposed this in T290315#7331733) is to refresh the user's task feed after accept or reject action. Should we do that? That would take care of the problem, I think.

Thanks @kostajh , yes that would be good because at the moment seeing it again would be pretty confusing.

@RHo one fairly simple thing to do (@Tgr proposed this in T290315#7331733) is to refresh the user's task feed after accept or reject action. Should we do that? That would take care of the problem, I think.

Thanks @kostajh , yes that would be good because at the moment seeing it again would be pretty confusing.

Sorry, I just realized this task is about link recommendations and not image recommendations.

For link recommendations, once you've done an accept/reject action, the link recommendation is invalidated for everyone and you shouldn't see it again in your feed.

For image recommendations, I'll follow up in T295410: [betalabs] Add image - suggested articles are not removed from Add image SE feed