Added support for the lexical query failure

[kirlat]

This build fixes the #562 and adds some related improvements to the GraphQL API.

Here is the short summary of changes:

1. The word GraphQL query response has been updated according to the results of the discussion in #562. If the query failed completely, then the homonyms field in the response will be null and there will be errors in the error field. If the query failed only partially, then the homonyms field will contain homonyms data and the errors field will contain warnings.
2. Warnings were added to both client adapters and the GraphQL backend.
3. I've started to add error codes to more error and warning objects. I think error codes are important if we want to identify what type of error it is (i.e. if (errCode ===A) {}) and act accordingly. The good thing about error codes is that they can be transferred from object to object and even across the network.

Please let me know what do you think.

[irina060981]

@kirlat and @balmas, I have a question here about wordList workflow. We could have such a case, when the morph adapter doesn't return the result, but we need to have an empty homonym (or may be some other format) to save this word to the wordList with the current context.

For example (from embed-lib index.html),

No, I am not sure, but I believe we didn't get any homonym and it is not saved to the wordlist now. Do we decide not to track such words in wordlist with only the context?

[irina060981]

The second case with Persian words. There was done a workflow with a warning before. When we didn't get lexical results , we show a notification for the user that he/she should wait for definitions.

For now we lost this behaviour, I couldn't found my PR with this change ad couldn't even find the notice with such a text. Do we remove this feature?

[irina060981]

Also I have a question now we have two progress bars at the same time in popup

I believe the second one is for short definitions, but what does mean the first progress? According to my case in the screenshot all morph data is retrieved from the source

[kirlat]

We could have such a case, when the morph adapter doesn't return the result, but we need to have an empty homonym (or may be some other format) to save this word to the wordList with the current context.

The second case with Persian words. There was done a workflow with a warning before. When we didn't get lexical results , we show a notification for the user that he/she should wait for definitions.

Thanks for catching this! I've missed those two cases during my testing. I will review them and see what's going on. I think we would need to create integration tests for them, if possible.

For now we lost this behaviour, I couldn't found my PR with this change ad couldn't even find the notice with such a text. Do we remove this feature?

I don't remember removing it specifically. @irina060981, can you recall the number of your PR? I can try to track changes from it to see what happened with the code since then.

[kirlat]

now we have two progress bars at the same time in popup

I believe the second one is for short definitions, but what does mean the first progress? According to my case in the screenshot all morph data is retrieved from the source

The upper progress bar is for the overall lexical query request. It disappears when the LEXICAL_QUERY_COMPLETE event is published at the finalize() method of the LexicalQuery, or in some other places if request fails.

The lower progress bar is for short definitions. It disappears when either short definitions are retrieved or when we figure out that there are no short definitions for this word.

@balmas, what would be the desired UI here? Both bars could make sense (especially if to change the wording a little bit), or we can show one or the other.

We can, for example, use the upper bar for the lexeme retrieval and it can say something like "Lexemes are loading" and then the lower bar for short definitions ("Short definitions are loading"). First the upper bar will disappear and then the lower bar will follow suit, when short definitions' retrieval will be complete. This might give users a more detailed insight about what's going on behind the scenes.

[balmas]

@kirlat and @balmas, I have a question here about wordList workflow. We could have such a case, when the morph adapter doesn't return the result, but we need to have an empty homonym (or may be some other format) to save this word to the wordList with the current context.

For example (from embed-lib index.html),

No, I am not sure, but I believe we didn't get any homonym and it is not saved to the wordlist now. Do we decide not to track such words in wordlist with only the context?

this is a good point. What should happen when you lookup a word without results is that the word should be added to the wordlist with itself as a lemma -- e.g. in these lines of the production lexical query, if we have no result form the morph adapter, we create a homonym with the form itself as the lemma: https://github.com/alpheios-project/alpheios-core/blob/production/packages/components/src/lib/queries/lexical-query.js#L181-L182

[balmas]

now we have two progress bars at the same time in popup I believe the second one is for short definitions, but what does mean the first progress? According to my case in the screenshot all morph data is retrieved from the source

The upper progress bar is for the overall lexical query request. It disappears when the LEXICAL_QUERY_COMPLETE event is published at the finalize() method of the LexicalQuery, or in some other places if request fails.

The lower progress bar is for short definitions. It disappears when either short definitions are retrieved or when we figure out that there are no short definitions for this word.

@balmas, what would be the desired UI here? Both bars could make sense (especially if to change the wording a little bit), or we can show one or the other.

We can, for example, use the upper bar for the lexeme retrieval and it can say something like "Lexemes are loading" and then the lower bar for short definitions ("Short definitions are loading"). First the upper bar will disappear and then the lower bar will follow suit, when short definitions' retrieval will be complete. This might give users a more detailed insight about what's going on behind the scenes.

I would defer to @monzug on what should happen in the UI here.

[monzug]

"lexical data is loading" message is generated while loading the morphology pop-up. when morphology pop-up is generated the message does not show up anymore. In the case of λεπτοψαμάθων it does briefly co-exist with the loading of the short definition progress bar in some cases (e.g. first look up of a Greek word) then it does quickly fade away. I am ok with it.

[balmas]

"lexical data is loading" message is generated while loading the morphology pop-up. when morphology pop-up is generated the message does not show up anymore. In the case of λεπτοψαμάθων it does briefly co-exist with the loading of the short definition progress bar in some cases (e.g. first look up of a Greek word) then it does quickly fade away. I am ok with it.

Thanks @monzug . So @kirlat let's leave this for now.

[kirlat]

this is a good point. What should happen when you lookup a word without results is that the word should be added to the wordlist with itself as a lemma -- e.g. in these lines of the production lexical query, if we have no result form the morph adapter, we create a homonym with the form itself as the lemma: https://github.com/alpheios-project/alpheios-core/blob/production/packages/components/src/lib/queries/lexical-query.js#L181-L182

I've examined the code carefully and it seems that the WordList controller does not listen to the MORPH_DATA_READY event at all. It subscribes to HOMONYM_READY instead: https://github.com/alpheios-project/alpheios-core/blob/production/packages/wordlist/src/controllers/wordlist-controller.js#L13-L21

In the LexicalQuery a Homonym is constructed even if the morph data request fails (at the point that Bridget referenced in the quote above), and then, a few lines below, this Homonym object used in the HOMONYM_READY event: https://github.com/alpheios-project/alpheios-core/blob/production/packages/components/src/lib/queries/lexical-query.js#L217-L247.

So it seems that in the current workflow, even if the morphological query failes, we still create a homonym from a target word nonethless nonetheless and, depending on the configuration, try to retrieve short definitions, full definitions, lemma translations, and word usage examples. Should we keep it this way? If so, for failed request the new workflow should create a Homonym, and pass it to the Lexical Query. Not sure how to be with short definitions in this case since those are retrieved behind the GraphQL facade. Should the GraphQL backend create a Homonym even if no morph data is available and try to retrieve short definitions for it?

Analyzing the code, I've discovered that MORPH_DATA_READY event is kind of a duplicate of HOMONYM_READY in our current workflow. There is only one subscriber for this event: https://github.com/alpheios-project/alpheios-core/blob/production/packages/components/src/lib/controllers/app-controller.js#L1150-L1152. All it does is updates the text message. I think we can move the message update to the onHomonymReady() method and get rid of MORPH_DATA_READY completely.

@balmas, @irina060981 please let me know what do you think about the desired workflow.

[kirlat]

"lexical data is loading" message is generated while loading the morphology pop-up. when morphology pop-up is generated the message does not show up anymore. In the case of λεπτοψαμάθων it does briefly co-exist with the loading of the short definition progress bar in some cases (e.g. first look up of a Greek word) then it does quickly fade away. I am ok with it.

Thanks @monzug . So @kirlat let's leave this for now.

Maybe we should change the text for the lower progress bar to Short definitions are loading? It might be more understandable what this bar is for this way. What do you think?

[balmas]

"lexical data is loading" message is generated while loading the morphology pop-up. when morphology pop-up is generated the message does not show up anymore. In the case of λεπτοψαμάθων it does briefly co-exist with the loading of the short definition progress bar in some cases (e.g. first look up of a Greek word) then it does quickly fade away. I am ok with it.

Thanks @monzug . So @kirlat let's leave this for now.

Maybe we should change the text for the lower progress bar to Short definitions are loading? It might be more understandable what this bar is for this way. What do you think?

Yes ! I think that's a very good idea. Thank you!

[monzug]

Absolutely, YES

On Thu, Dec 3, 2020 at 11:46 AM Bridget Almas notifications@github.com wrote:

"lexical data is loading" message is generated while loading the morphology pop-up. when morphology pop-up is generated the message does not show up anymore. In the case of λεπτοψαμάθων it does briefly co-exist with the loading of the short definition progress bar in some cases (e.g. first look up of a Greek word) then it does quickly fade away. I am ok with it.

Thanks @monzug https://github.com/monzug . So @kirlat https://github.com/kirlat let's leave this for now.

Maybe we should change the text for the lower progress bar to Short definitions are loading? It might be more understandable what this bar is for this way. What do you think?

Yes ! I think that's a very good idea. Thank you!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/alpheios-project/alpheios-core/pull/565#issuecomment-738130767, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJ32UOP6J4GZMODJFXY323LSS66HFANCNFSM4UK264MA .

[kirlat]

Yes ! I think that's a very good idea. Thank you! Absolutely, YES

Will update the text accordingly.

[balmas]

I've examined the code carefully and it seems that the WordList controller does not listen to the MORPH_DATA_READY event at all. It subscribes to HOMONYM_READY instead: https://github.com/alpheios-project/alpheios-core/blob/production/packages/wordlist/src/controllers/wordlist-controller.js#L13-L21

In the LexicalQuery a Homonym is constructed even if the morph data request fails (at the point that Bridget referenced in the quote above), and then, a few lines below, this Homonym object used in the HOMONYM_READY event: https://github.com/alpheios-project/alpheios-core/blob/production/packages/components/src/lib/queries/lexical-query.js#L217-L247.

So it seems that in the current workflow, even if the morphological query failes, we still create a homonym from a target word nonethless nonetheless and, depending on the configuration, try to retrieve short definitions, full definitions, lemma translations, and word usage examples. Should we keep it this way? If so, for failed request the new workflow should create a Homonym, and pass it to the Lexical Query. Not sure how to be with short definitions in this case since those are retrieved behind the GraphQL facade. Should the GraphQL backend create a Homonym even if no morph data is available and try to retrieve short definitions for it?

Analyzing the code, I've discovered that MORPH_DATA_READY event is kind of a duplicate of HOMONYM_READY in our current workflow. There is only one subscriber for this event: https://github.com/alpheios-project/alpheios-core/blob/production/packages/components/src/lib/controllers/app-controller.js#L1150-L1152. All it does is updates the text message. I think we can move the message update to the onHomonymReady() method and get rid of MORPH_DATA_READY completely.

@balmas, @irina060981 please let me know what do you think about the desired workflow.

Per discussion in Slack, the current production code is a bit of a hack, that allows us to continue to lookup a word in the additional resources (short definitions, full definitions, usage examples, etc) even if we fail to parse a lemma in the morphology service.

I think we still want to do that, but I want to expand the data model so that we don't pretend the form is a "lemma" . This is described further in https://github.com/alpheios-project/documentation/issues/33

Aside from making the code easier to understand and less error-prone, and supporting the additional use cases for the queries, our annotation workflows will require us to be explicit about what is being annotated, and we don't want to misrepresent an unparsed word form as a lemma as an annotation target.

[kirlat]

Analyzing the code, I've discovered that MORPH_DATA_READY event is kind of a duplicate of HOMONYM_READY in our current workflow. There is only one subscriber for this event: https://github.com/alpheios-project/alpheios-core/blob/production/packages/components/src/lib/controllers/app-controller.js#L1150-L1152. All it does is updates the text message. I think we can move the message update to the onHomonymReady() method and get rid of MORPH_DATA_READY completely.

@balmas, @irina060981: do you think we can remove the MORPH_DATA_READY event?

[irina060981]

For now we lost this behaviour, I couldn't found my PR with this change ad couldn't even find the notice with such a text. Do we remove this feature? I don't remember removing it specifically. @irina060981, can you recall the number of your PR? I can try to track changes from it to see what happened with the code since then.

I couldn't find the PR it was rather long ago. But I found that it was produced inside

  onMorphDataNotFound () {
    this._store.commit('ui/setNotification', { text: this.api.l10n.getMsg('TEXT_NOTICE_MORPHDATA_NOTFOUND'), important: true })
    this._store.commit('app/setQueryStillActive', true)
  }

@balmas, @irina060981: do you think we can remove the MORPH_DATA_READY event?

We have a workflow for MORPH_DATA_NOTAVAILABLE. If we don't need this message:

  onMorphDataReady () {
    this._store.commit('ui/addMessage', this.api.l10n.getMsg('TEXT_NOTICE_MORPHDATA_READY'))
  }

I believe we could remove it.

[kirlat]

In our current workflow the HOMONYM_READY is published right after the MORPH_DATA_READY event. So if we'll move this._store.commit('ui/addMessage', this.api.l10n.getMsg('TEXT_NOTICE_MORPHDATA_READY')) to the beginning of the onHomonymReady() handler the result should be almost the same, and we'll be able to simplify our workflow a little bit with that.

[kirlat]

As discussed with @balmas, I will merge the current PR with the current changes and two small additional fixes, and will implement other changes discussed as separate PR(s). This is to prevent discussion in the current PR from becoming too big.