Page MenuHomePhabricator

Add Links: Cannot read property 'link_target' of undefined
Closed, ResolvedPublic

Description

Go to beta cswiki, select the link recommendation task for Emma Hewitt (or just click on https://cs.wikipedia.beta.wmflabs.org/wiki/Speci%C3%A1ln%C3%AD:Homepage/newcomertask/241?geclickid=5036q73hc4c8nk8jvji0vsegkierfgep&getasktype=link-recommendation ) and open the article for editing. VisualEditor will die halfway with this error message:

jQuery.Deferred exception: Cannot read property 'link_target' of undefined TypeError: Cannot read property 'link_target' of undefined
    at AddLinkDesktopArticleTarget.AddLinkArticleTarget.annotateSuggestions (<anonymous>:6:647)
    at AddLinkDesktopArticleTarget.AddLinkArticleTarget.beforeLoadSuccess (<anonymous>:3:218)
    at AddLinkDesktopArticleTarget.loadSuccess (<anonymous>:2:303)
    at fire (https://cs.wikipedia.beta.wmflabs.org/w/load.php?lang=en&modules=jquery%2Coojs-ui-core%2Coojs-ui-widgets&skin=vector&version=1vuvy:46:209)
    at fireWith (https://cs.wikipedia.beta.wmflabs.org/w/load.php?lang=en&modules=jquery%2Coojs-ui-core%2Coojs-ui-widgets&skin=vector&version=1vuvy:47:402)
    at mightThrow (https://cs.wikipedia.beta.wmflabs.org/w/load.php?lang=en&modules=jquery%2Coojs-ui-core%2Coojs-ui-widgets&skin=vector&version=1vuvy:49:751)
    at process (https://cs.wikipedia.beta.wmflabs.org/w/load.php?lang=en&modules=jquery%2Coojs-ui-core%2Coojs-ui-widgets&skin=vector&version=1vuvy:49:808)
    at https://cs.wikipedia.beta.wmflabs.org/w/load.php?lang=en&modules=jquery%2Coojs-ui-core%2Coojs-ui-widgets&skin=vector&version=1vuvy:50:37
    at mightThrow (https://cs.wikipedia.beta.wmflabs.org/w/load.php?lang=en&modules=jquery%2Coojs-ui-core%2Coojs-ui-widgets&skin=vector&version=1vuvy:49:149)
    at process (https://cs.wikipedia.beta.wmflabs.org/w/load.php?lang=en&modules=jquery%2Coojs-ui-core%2Coojs-ui-widgets&skin=vector&version=1vuvy:49:808)

At a glance the DB record seems OK:

MariaDB [cswiki]> select * from growthexperiments_link_recommendations where gelr_page = 241 \G
*************************** 1. row ***************************
gelr_revision: 245
    gelr_page: 241
    gelr_data: {"links":[{"link_text":"Finsku","link_target":"Finsko","match_index":0,"wikitext_offset":1360,"score":0.567171037197113,"context_before":" m\u00edsto ve ","context_after":". Singl se","link_index":0},{"link_text":"Decade","link_target":"Decade","match_index":0,"wikitext_offset":3268,"score":0.6700974106788635,"context_before":"pres. Sun ","context_after":" feat. Emm","link_index":1},{"link_text":"Berlin","link_target":"Berl\u00edn","match_index":1,"wikitext_offset":3520,"score":0.8429474830627441,"context_before":"11 - Dash ","context_after":" feat. Emm","link_index":2},{"link_text":"Miss You","link_target":"Miss You","match_index":0,"wikitext_offset":3869,"score":0.9231995940208435,"context_before":" Hewitt - ","context_after":" Paradise\n","link_index":3}]}
1 row in set (0.00 sec)

Event Timeline

On the other hand https://en.wikipedia.beta.wmflabs.org/wiki/Special:Homepage/newcomertask/182198?geclickid=25be7l7i5gbi24bqfvd0p3iokr5l54q9&getasktype=link-recommendation works, so this isn't about the code being generally broken. At a glance the data for the two has the same structure.

Hmm, I get jQuery.Deferred exception: suggestion is undefined for the Emma Hewitt link https://cs.wikipedia.beta.wmflabs.org/wiki/Speci%C3%A1ln%C3%AD:Homepage/newcomertask/241?geclickid=5036q73hc4c8nk8jvji0vsegkierfgep&getasktype=link-recommendation

mw.config.get( 'wgGESuggestedEditData' ) shows:

{
  "links": [
    {
      "link_text": "Finsku",
      "link_target": "Finsko",
      "match_index": 0,
      "wikitext_offset": 1360,
      "score": 0.567171037197113,
      "context_before": " místo ve ",
      "context_after": ". Singl se",
      "link_index": 0
    },
    {
      "link_text": "Decade",
      "link_target": "Decade",
      "match_index": 0,
      "wikitext_offset": 3268,
      "score": 0.6700974106788635,
      "context_before": "pres. Sun ",
      "context_after": " feat. Emm",
      "link_index": 1
    },
    {
      "link_text": "Berlin",
      "link_target": "Berlín",
      "match_index": 1,
      "wikitext_offset": 3520,
      "score": 0.8429474830627441,
      "context_before": "11 - Dash ",
      "context_after": " feat. Emm",
      "link_index": 2
    },
    {
      "link_text": "Miss You",
      "link_target": "Miss You",
      "match_index": 0,
      "wikitext_offset": 3869,
      "score": 0.9231995940208435,
      "context_before": " Hewitt - ",
      "context_after": " Paradise\n",
      "link_index": 3
    }
  ]
}

Ok, this is where it breaks:

image.png (270×2 px, 182 KB)

because phrase.suggestions[phrase.occurrencesSeen] is indeed undefined.

image.png (476×824 px, 74 KB)

It looks like a bug in how to handle phrases which appear more than once in the document. occurrences of the same phrase.

Change 672707 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Guard against undefined suggestion

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

Change 672707 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Guard against undefined suggestion

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

I suppose that depends on your luck and your browser - works for me on desktop as expected.

Presently, desktop and mobile consistently display - "Link recommendation not found for [this article]"

Screen Shot 2021-04-05 at 4.04.40 PM.png (381×1 px, 74 KB)

Btw, should the warning message be dismissable? The message from the above screenshot has only one clickable option - go back to Homepage.

Presently, desktop and mobile consistently display - "Link recommendation not found for [this article]"

Screen Shot 2021-04-05 at 4.04.40 PM.png (381×1 px, 74 KB)

Btw, should the warning message be dismissable? The message from the above screenshot has only one clickable option - go back to Homepage.

Yes, that's per the spec for T277508: Add a link: dialog for no suggestions available

Note that this task is about an error that happened when a suggestion is available but there was an error in the logic that matched it to the article HTML. With the error fixed, the suggestion should show up normally.

Which brings me to the point: how do we QA the text matching algorithm? That would probably involve fake recommendations (via the JSON subpage method) and trying to create "tricky" situations where the text of the link is similar to some preceding text. That probably requires some konwledge of how the algorithm works. Is that worth the effort, or do we just catch those in the wild? If it's worth QA-ing, should that be done by Elena or an engineer?

Note that this task is about an error that happened when a suggestion is available but there was an error in the logic that matched it to the article HTML. With the error fixed, the suggestion should show up normally.

Which brings me to the point: how do we QA the text matching algorithm? That would probably involve fake recommendations (via the JSON subpage method) and trying to create "tricky" situations where the text of the link is similar to some preceding text. That probably requires some konwledge of how the algorithm works. Is that worth the effort, or do we just catch those in the wild? If it's worth QA-ing, should that be done by Elena or an engineer?

The point about how to QA the text matching algorithm is quite interesting. When I was testing ORES models and SE, I did not evaluated/tested the algorithm per se. And, technically speaking, I was not testing the results of algorithm either (the results that were presented to users in UI). The usability and validity of the models was tested by 1) the engineers who were creating the algorithm and 2) the community ambassadors (when it was available) or just "in the wild".

Sure, when testing I noted/collected/sorted the cases when, in my opinion, they uncover some weak spots. But such cases were kind of a byproduct of feature testing, not the goal.

On one hand, I'd be happy to get familiar with the algorithm and test it whatever is needed, but from the practical point of view, our users (and ambassadors) and data analytics engineers together create quite good QA coverage analysis. I'm not even mentioning that for different languages it'd be impossible for me to do anything beyond rudimentary QA.

Note that this task is about an error that happened when a suggestion is available but there was an error in the logic that matched it to the article HTML. With the error fixed, the suggestion should show up normally.

Which brings me to the point: how do we QA the text matching algorithm? That would probably involve fake recommendations (via the JSON subpage method) and trying to create "tricky" situations where the text of the link is similar to some preceding text. That probably requires some konwledge of how the algorithm works. Is that worth the effort, or do we just catch those in the wild? If it's worth QA-ing, should that be done by Elena or an engineer?

The point about how to QA the text matching algorithm is quite interesting. When I was testing ORES models and SE, I did not evaluated/tested the algorithm per se. And, technically speaking, I was not testing the results of algorithm either (the results that were presented to users in UI). The usability and validity of the models was tested by 1) the engineers who were creating the algorithm and 2) the community ambassadors (when it was available) or just "in the wild".

Sure, when testing I noted/collected/sorted the cases when, in my opinion, they uncover some weak spots. But such cases were kind of a byproduct of feature testing, not the goal.

On one hand, I'd be happy to get familiar with the algorithm and test it whatever is needed, but from the practical point of view, our users (and ambassadors) and data analytics engineers together create quite good QA coverage analysis. I'm not even mentioning that for different languages it'd be impossible for me to do anything beyond rudimentary QA.

Thanks for bringing this up, @Etonkovidova.

I do think we need a plan for QA specifically of the phrase matching approach, to ensure that the recommendations the service returns map correctly to text in the VE document. I'll file a task about it.