camaraproject / KnowYourCustomer

Repository to describe, develop, document and test the KnowYourCustomer API family
Apache License 2.0
9 stars 9 forks source link

Include score functionality proposal #104

Closed fernandopradocabrillo closed 2 months ago

fernandopradocabrillo commented 3 months ago

What type of PR is this?

Add one of the following kinds:

What this PR does / why we need it:

Include scoring functionality. When a property match results in false, the api will also return a value representing how similar is the input to the information stored by the Operator.

Update the yaml structure to improve code reusability.

Which issue(s) this PR fixes:

Fixes #85 Fixes #96

Special notes for reviewers:

Changelog input

 ### Added
- Include score functionalily
GillesInnov35 commented 3 months ago

hi @fernandopradocabrillo, thanks for this PR

Gilles

GillesInnov35 commented 2 months ago

@fernandopradocabrillo, all, we discussed internally at Orange on the design proposal concerning new optional score match attributes for some of attributes of KYC match v0.1.0 and the option to have 2 end points.

ToshiWakayama-KDDI commented 2 months ago

Hi @fernandopradocabrillo ,

Thank you for the proposal. Due to my sickness and so on, we are stil looking into the yaml, but we have a clear comment for clarification now.

Based on our previous discussion, it was commonly agreed to keep backward Compatiblity to V0.1 and to have Match Scoring function optional. From KDDI side, it is understood that if Match Scoring function is not available (e.g. has not been implemented), it is ok not to return MatchScoreResult even if a MatchResult is false.

This may be different from one of Gilles comments, so, I am quickly commenting this.

Other parts are still investigated.

many thanks, Toshi

GillesInnov35 commented 2 months ago

hi @ToshiWakayama-KDDI, thanks for your comment.

Based on our previous discussion, it was commonly agreed to keep backward Compatiblity to V0.1 and to have Match Scoring function optional. From KDDI side, it is understood that if Match Scoring function is not available (e.g. has not been implemented), it is ok not to return MatchScoreResult even if a MatchResult is false.

You're right my proposition doesn't work in that case. Unfortunately as we'll have a unique endpoint for the 2 versions (which are quite different I think) we can't associate a mandatory score attribute when False result has been returned. More rules should be developed to process the response on consumer side.

BR Gilles

ToshiWakayama-KDDI commented 2 months ago

Hi @GillesInnov35 ,

Thanks, Gilles. Could you elaborate "we'll have a unique endpoint for the 2 versions (which are quite different I think)", please? I may have misunderstood your comments.

Many thanks, Toshi

GillesInnov35 commented 2 months ago

Toshi,

Could you elaborate "we'll have a unique endpoint for the 2 versions (which are quite different I think)",

I just mean that it'd have been easier to have 2 endpoints as we see that

But never mind, sure it'll work Thanks Gilles

fernandopradocabrillo commented 2 months ago
  • KYC Match v0.1.0 and v0.2.0 will be integrated in the Meta Release of September (to be confirmed)

@GillesInnov35 I don't get this one, only v0.2.0 will go to the Meta Release I think. We cannot propose 2 versions.

ToshiWakayama-KDDI commented 2 months ago

Hi @GillesInnov35 , @fernandopradocabrillo , all,

Thank you, Gilles. A bit difficult for me to fully understand. I need to discuss with my colleagues, I guess.

In the meantime, I would like to share our view. In one of my previous comments, I proposed an idea that Match Result Score will not be returned if the match scoring function is not available. From KDDI side, we would like to keep this.

One reason is that KDDI does not plan to implement the match scoring function because our customers in Japan do not need it. No customer demands. So, we will keep the v0.1 base minimum function, i.e. returning True/False MatchResults only.

Apart from KDDI's own context, we think that not all customers want to receive Match Result Score automatically when Match Result is false, and some API customers may be confused about how to deal with it. Also, for operators, some operators do not want to return Match Result Scores, like KDDI. Operator's implementation is one important point, we think, and we should take a direction to make this API easy to implement to operators and to accelerate this API's implementation by global operators. For this, we could define simple v0.1 True/False MatchResult function as the basic function or MVP?, which is kind of mandatory function, and define the Match Scoring function as an enhanced, optional function, which can be implemented if needed by each operator.

Sorry for the long sentence, but the above is some addtional comments to KDDI' original view that Match Scoring should be optional.

What do you think?

Best regards, Toshi KDDI

KevScarr commented 2 months ago

@ToshiWakayama-KDDI My understanding is that the score is an optional attribute. So it is down to the MNO to either implement it or not implement it. I would suggest it's consistent within an MNOs response, ie if implemented it's always available for 'false' responses so there is consistency. At a market/country level, the MNOs can work together to agree to all provide it or not based on their customers needs (as they would all have common customers, so the ask should be the same) ??

GillesInnov35 commented 2 months ago

hi @fernandopradocabrillo,

@GillesInnov35 I don't get this one, only v0.2.0 will go to the Meta Release I think. We cannot propose 2 versions.

yes you're right, only version v0.2.0 will be published in Meta Release. My sentence was not clear. I wanted to say that functionalities of v0.1.0 and evolutions with v0.2.0 will be published in the Meta Release.

I think that we all agree that scoring could be part of Meta Release but must be optional as it is not available in all countries.

Thanks a lot BR Gilles

claraserranosolsona commented 2 months ago

Hi @GillesInnov35 ,

Can you review and approve the PR #104?

Once you do it, we will include the following comment and we will create the RC v0.2.0 as agreed in last meeting "Unless otherwise captured in the specification, score will use the JaroWinkler distance algorithm for all countries"

Regards, Clara

ToshiWakayama-KDDI commented 2 months ago

Hi @claraserranosolsona , @fernandopradocabrillo , all

Thanks for Kana parameters being included for Scoring.

Please wait for sometime for my approval, because we are still checking the YAML proposal. Sorry for the delay, but we should be ready by tomorro's KYC SP meeting.

Thank you for your understanding. Toshi

ToshiWakayama-KDDI commented 2 months ago

Hi @claraserranosolsona , @fernandopradocabrillo , all,

The proposed v0.2 YAML looks good to us, but just one thing to confirm. V0.2 is backward compatible with v0.1. Correct?

In addition, please let us confirm the following understanding is correct.

1) v0.1 Match basic function; returning value True/False/not-available -Attributes are all optional, so, if an operator does not have specific attributes, it can send responses 'not-available' for match request of those unavailable attributes.

2) v0.2 Match score added: Optional feature returning match score calculated by Jaro-Winkler algo (or by other algo, if applicable), for string attributes. -Score is an optional feature, so, it is up to MNO to implement the feature. ->If implemented, when False is returned for a string attribute, (attribute)MatchScore:XX(%) should be returned. ->If not implemented, (attribute)MatchScore is not returned.

3) For KYC Match, v0.2 (including Scoring) will be included in the Fall24 Meta-release.

4) For string match score algorithm, default algorithm will be Jaro-Winkler distance algorithm, however, if any region/country/operator decide to use a different algorithm, it would be possible. For this, "Unless otherwise captured in the specification, score will use the Jaro-Winkler distance algorithm for all countries." is put in the API description, and in future the chosen algorithm should be written in the API description.

Many thanks, Toshi KDDI

ToshiWakayama-KDDI commented 2 months ago

This PR is to be merged as agreed in the meeting 2024/07/09 and supported by three codeowners.

ToshiWakayama-KDDI commented 2 months ago

Hi @GillesInnov35 ,

Can you review and approve the PR #104?

Once you do it, we will include the following comment and we will create the RC v0.2.0 as agreed in last meeting "Unless otherwise captured in the specification, score will use the JaroWinkler distance algorithm for all countries"

Regards, Clara

To note, the above will be taken care of by another PR.

Thanks. Toshi

ToshiWakayama-KDDI commented 2 months ago

hi Toshi, it has been already done Gilles

Hi @GillesInnov35 ,

Sorry, what has been already done, do you mean?

For the sentence "Unless otherwise captured in the specification, score will use the JaroWinkler distance algorithm for all countries", I cannot find it in the current kyc-match.yaml...

Thanks, Toshi

jgarciahospital commented 2 months ago

hi Toshi, it has been already done Gilles

Hi @GillesInnov35 ,

Sorry, what has been already done, do you mean?

For the sentence "Unless otherwise captured in the specification, score will use the JaroWinkler distance algorithm for all countries", I cannot find it in the current kyc-match.yaml...

Thanks, Toshi

Hi toshi, it is included in #111