Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect value shown for ratings [jetbrains] #7139

Closed
ChrisCarini opened this issue Oct 13, 2021 · 13 comments · Fixed by #7140
Closed

Incorrect value shown for ratings [jetbrains] #7139

ChrisCarini opened this issue Oct 13, 2021 · 13 comments · Fixed by #7140
Labels
question Support questions, usage questions, unconfirmed bugs, discussions, ideas

Comments

@ChrisCarini
Copy link
Contributor

ChrisCarini commented Oct 13, 2021

Are you experiencing an issue with...

shields.io

🐞 Description

Hello!

Original author here of the JetBrains Plugin Rating (#4897 & #4898) :)

I noticed that in #5974 (the PR for #5946; thank you for reporting @frimtec and for implementing @chris48s !), the API was changed for identifying the rating of JetBrains Plugins to use a new API endpoint provided by JetBrains. Specifically, the https://plugins.jetbrains.com/api/plugins/<plugin_id>/rating endpoint.

The Issue: From this endpoint, the meanRating value is used, which I believe is not the intended value to be using - educated guess** tells me that the meanRating value is the mean rating for the particular plugin developer, not the specific plugin being queried. Instead, I believe the values from the votes field need to be used to compute the actual rating of the plugin; see the below screenshot, which shows two of the plugins I have published to plugins.jetbrains.com:

Screen Shot 2021-10-13 at 01 21 18

There is a bit going on in this screenshot, so let me explain :)

  1. The 4 left most windows are the current shields.io rendering of my two plugins, ordered
    a. [ link ] 'new style' <plugin_id> (ie, 10998-environment-variable-settings-summary) INCORRECT RATING VALUE
    b. [ link ] 'old style' <plugin_id> (ie, com.chriscarini.jetbrains.environment-variable-settings-summary) CORRECT RATING VALUE
    c. [ link ] 'new style' <plugin_id> (ie, 11941-automatic-power-saver) INCORRECT RATING VALUE
    d. [ link ] 'old style' <plugin_id> (ie, com.chriscarini.jetbrains.jetbrains-auto-power-saver) CORRECT RATING VALUE
  2. The middle two windows are each of the API calls using the new endpoint
    a. [ link ] 10998-environment-variable-settings-summary - you will notice there are no votes, but the meanRating is 4.15669
    b. [ link ] 11941-automatic-power-saver - you will notice there are 5 votes total - one 4-star, and four 5-star votes; the meanRating remains 4.15669
  3. The right two windows are the actual ratings on plugins.jetbrains.com for each of the two plugins
    a. [ link ] 10998-environment-variable-settings-summary - No votes have been received.
    b. [ link ] 11941-automatic-power-saver - Five votes have been received, for a rating of 4.6 stars.

Now, if you are following closely, you will notice that for the plugin with votes, the API does not seem to return numbers that yield a 4.6 star rating... that is, (4 + 5 + 5 + 5 + 5) / 5 = 4.8, and 4.6 != 4.8 (This puzzled me too, but...) I noticed that JetBrains is using a Bayesian Rating to calculate plugin ratings, documented here: https://plugins.jetbrains.com/docs/marketplace/plugins-rating.html

So, for my 11941-automatic-power-saver plugin, the math would work out to be: ((4 + 5 + 5 + 5 + 5) + (2 * 4.15669)) / (5 + 2) (from the above linked page: bayesianRating = (sum(userRatings) + 2 * meanVote) / (count(userRatings) + 2), where meanVote is the value returned by meanRating).

** = As I am typing this, I am realizing that my guess was in fact incorrect, and that the meanRating value is the mean rating for ALL plugins hosted by JetBrains. TIL!

The Ask: Could we possibly change the logic such that the plugin rating is calculated correctly from the new API results? Right now, the few JetBrains plugins that I have are all showing the incorrect rating (it's actually the average rating for all JetBrains plugins, kind of a neat side-fact!).

🔗 Link to the badge

No response

💡 Possible Solution

No response

@ChrisCarini ChrisCarini added the question Support questions, usage questions, unconfirmed bugs, discussions, ideas label Oct 13, 2021
@calebcartwright
Copy link
Member

Thanks for reaching out and providing the wealth of info. As an aside, I feel like you might have swapped the links in 1c and 1d based on the associated badge url mismatch with the textual description.

Personally, I feel like there's too much ambiguity on the provider side for which I wouldn't want to try to work around or have our own derivation algorithm except as an absolute last result. From my perspective, a driver of the challenges we are facing stem from a lack of clarity on what the "real" or "non-kinda-deprecated" API contract is for JetBrains' Marketplace, and whether both TC and IDEA plugins are supported.

I really don't feel like it should have to be Shields responsibility for calculating the metric, but that we'd instead be able to get the data point from the system of record as we typically do. At a minimum, I'd really like to get some definitive, authoritative documentation from their end that we can point to in the future (e.g. https://plugins.jetbrains.com/docs/marketplace/plugins-list.html still makes no mention of the plugin list API being deprecated even though that seems to be the case from casual comments elsewhere).

However, I agree with the overall thrust of the issue you've raised here that meanRating seems to be a specious data point for deriving the rating value of a specific plugin, and something needs to change

@chris48s
Copy link
Member

At a minimum, I'd really like to get some definitive, authoritative documentation from their end that we can point to in the future

I think I'm happy to take https://plugins.jetbrains.com/docs/marketplace/plugins-rating.html as that, or at least the closest we're going to get.

Accepting the solution proposed in #7140 seems like the right call to me.


Just picking another plugin (but one with more ratings) to work through as an example:

https://img.shields.io/jetbrains/plugin/r/rating/1347 - Rating = 4.2

https://plugins.jetbrains.com/plugin/1347-scala - Rating = 4.4
https://plugins.jetbrains.com/plugins/list?pluginId=1347 - Rating = 4.4
https://shields-staging-pr-7140.herokuapp.com:/jetbrains/plugin/r/rating/1347 - Rating = 4.4

@ChrisCarini
Copy link
Contributor Author

As an aside, I feel like you might have swapped the links in 1c and 1d based on the associated badge url mismatch with the textual description.

Sure did! Just fixed it, thank you for catching!

Personally, I feel like there's too much ambiguity on the provider side for which I wouldn't want to try to work around or have our own derivation algorithm except as an absolute last result. .... I really don't feel like it should have to be Shields responsibility for calculating the metric, but that we'd instead be able to get the data point from the system of record as we typically do.

I agree - it seems really weird to me that their API isn't providing the calculated value; while, as an engineer I find their formula neat and interesting, I ultimately don't care that much.

From my perspective, a driver of the challenges we are facing stem from a lack of clarity on what the "real" or "non-kinda-deprecated" API contract is for JetBrains' Marketplace, and whether both TC and IDEA plugins are supported.

Makes sense - I agree and would love to see something more explicit and clear. I'm not sure how we might get that information, though.

At a minimum, I'd really like to get some definitive, authoritative documentation from their end that we can point to in the future (e.g. https://plugins.jetbrains.com/docs/marketplace/plugins-list.html still makes no mention of the plugin list API being deprecated even though that seems to be the case from casual comments elsewhere).

The rating is precomputed and part of their 'plugin details api' -> https://plugins.jetbrains.com/docs/marketplace/plugin-details.html - I believe this is what was used before, but does not work w/ the TC plugins if I am understanding the original issue #5946 correctly.

However, I agree with the overall thrust of the issue you've raised here that meanRating seems to be a specious data point for deriving the rating value of a specific plugin, and something needs to change

Yeah :( My perspective, right now this badge is just broken (factually incorrect, really); it's (1) displaying the same number for everyone, and (2) it's not the rating for the plugin provided in the url.

I really did not want to have to write their formula to compute the rating in the code, but I figured this is a good intermediate compromise to get the badges to at least show the correct value. Beyond that, I'm not sure the best way to proceed, but I'm open to everyone's ideas!

@ChrisCarini
Copy link
Contributor Author

Thanks @chris48s for the additional example! Good idea to check another w/ more reviews (my plugins are clearly not that popular ;) )

(Unrelated note to the core issue here; I think its so cool that each PR spins up a heroku instance; is that linked somewhere in the PR itself?)

@ChrisCarini
Copy link
Contributor Author

(nvm, answered my own question at the end; it's found at @shields-cd shields-cd deployed to shields-staging-pr-7140 1 hour ago which was not there (and I did not just see) when I originally posted the PR; so cool :) )

@calebcartwright
Copy link
Member

I think I'm happy to take https://plugins.jetbrains.com/docs/marketplace/plugins-rating.html as that, or at least the closest we're going to get.

Let me try to explain my concern better. I'm concerned that the current state of affairs with JetBrains Plugin Marketplace API documentation fails to meet our core criteria around data sources which we do apply as an entry gate on new badges, namely, that we shouldn't be using undocumented nor reverse engineered endpoints.

The only API/data source documentation I can find that has any semblance of being authoritative is https://plugins.jetbrains.com/docs/marketplace/plugin-details.html. However, in a passing comment it was noted by someone on the JetBrains team that API is actually deprecated (https://youtrack.jetbrains.com/issue/MP-3341). So from my perspective, we're bouncing around between a set of APIs that aren't actually documented, and I don't have much faith in any of them. Perhaps this is just a pebkac issue on my side though, and there's some official, current documentation somewhere?

Just picking another plugin (but one with more ratings) to work through as an example:

Yup and as I noted towards the end of my prior comment I completely agree that our current badges are useless for providing info about the rating of a plugin and I concur something needs to change. What I'm driving at though, is that JetBrains do already provide the calculated value in their official-looking-documented-but-apparently-deprecated API, so my preference would be that we drop JetBrains a note to ask for (a) documentation for the non-deprecated API and (b) they include their already-existing calculated value in that response object.

I guess I don't feel too strongly about this if others are opposed to us poking at the vendor, but that seems like a pretty sensible step to me and one that doesn't necessarily have to be done serially/in a blocking fashion to #7140. My worry is that while we do have some anecdotal examples that confirm our custom derivation of their score matches something in their UI, we're basing our implementation off material from the same doc hub which is confirmed to be outdated, and from a page which was lasted updated around the same time as the deprecated APIs. Additionally, we'll have no way of knowing if/when JetBrains decide to make any changes internally that could introduce a divergence, and essentially be back in a similar situation.

@frimtec
Copy link

frimtec commented Oct 14, 2021

Hi all

I'm very sorry that I did not realize the issues with the rating when #5974 was implemented for my original raced issue #5946.
The reason for that - I don't use the rating badges for my plugins at all.

I fully agree that reverse engineering is not a good practice here.
Nevertheless I like the PR #7140 as a quick fix.

I will open a Jetbrains ticket and ask the following questions today:

  • Is there a API documentation for all of the new endpoints?
  • Could the calculated plugin rating be added to the result of the rating endpoint?
  • Is there an official decommission date for the old deprecated endpoint?

Regards
frimtec

@frimtec
Copy link

frimtec commented Oct 14, 2021

Reference to ticket at Jetbrains: https://youtrack.jetbrains.com/issue/MP-3743

@chris48s
Copy link
Member

I'm concerned that the current state of affairs with JetBrains Plugin Marketplace API documentation fails to meet our core criteria around data sources which we do apply as an entry gate on new badges, namely, that we shouldn't be using undocumented nor reverse engineered endpoints

Yes if someone suggests we implement a new badge which involves using an API which is undocumented, has a really restrictive rate limit, unacceptably poor performance, etc we just don't implement it.
There's also this other case where we implement an integration which is based on an API which is fine at the time but then things go downhill in some way at a later date or we've got some integration which was added in the early days of shields.io which we wouldn't meet the criteria if it was proposed today but it already exists and people are using it.
I can think of a few integrations that are good examples of this e.g:

  • David (unreliable API)
  • Chrome Web Store (undocumented API/scraper)
  • Libraries.io (started applying restrictive rate limit)
  • etc

and I think you're right this JetBrains badge/API falls into this bucket too, for the reasons you've described.
To the extent we have any convention or procedure on this, it is broadly: once the badge exists we try to keep it working. There's possibly a wider discussion to be had around deciding/documenting what we do when this happens as it does occasionally leave us in a predicament, but I think we probably don't need to block #7140 on it.

I guess I don't feel too strongly about this if others are opposed to us poking at the vendor, but that seems like a pretty sensible step to me and one that doesn't necessarily have to be done serially/in a blocking fashion to #7140.

Agreed. Merge #7140 for now and monitor https://youtrack.jetbrains.com/issue/MP-3743 for an update seems like a sensible way forward to me 👍 Thanks for raising that @frimtec . Hopefully that will result in a bit of clarification from someone in the know at JetBrains.

@ChrisCarini
Copy link
Contributor Author

Hi @chris48s, @calebcartwright and @frimtec !

Thanks for all the discussion - these are all great perspectives and thoughts.

While I am certainly not a maintainer of this project, I do support @calebcartwright 's perspective, and can see how this project can be put into tricky situations w/ APIs that don't remain stable.

I've up-voted the JB issue that @frimtec has raised (would recommend others do as well, if willing/able - JetBrains tends to look at issues with more upvotes), and am also going to raise attention to it in JetBrains plugin developers Slack channel - if we don't get any responses from them in a reasonable amount of time, I'm happy to ping some of the folks I know at JB that are more active on GitHub. Just let me know if/when that's desired!

Appreciate all the discussion and thoughts here, it's very good to hear everyones perspectives! :)

Best,
Chris

@calebcartwright
Copy link
Member

I'm very sorry that I did not realize the issues with the rating when #5974 was implemented for my original raced issue #5946.
The reason for that - I don't use the rating badges for my plugins at all.

No worries at all @frimtec! The API contract situation is a little confusing to say the least 😄

Thanks so much for opening the issue upstream with JetBrains!

@calebcartwright
Copy link
Member

To the extent we have any convention or procedure on this, it is broadly: once the badge exists we try to keep it working. There's possibly a wider discussion to be had around deciding/documenting what we do when this happens as it does occasionally leave us in a predicament, but I think we probably don't need to block #7140 on it.

Agreed, and to clarify, there wasn't any subtext in my message/I wasn't implying we should deprecate the badge or anything; I just wanted to make sure we got the ball rolling to address the upstream gap with JetBrains.

Think we're on a good path forward 👍

@calebcartwright
Copy link
Member

(Unrelated note to the core issue here; I think its so cool that each PR spins up a heroku instance; is that linked somewhere in the PR itself?)

Btw @ChrisCarini this is entirely driven by Heroku's Review Apps feature which, as it sounds like you subsequently observed, shows up as a deployment/environment on the PR

repo-ranger bot added a commit that referenced this issue Oct 15, 2021
…#7140)

* Fixing incorrect JetBrains Plugin rating values for [JetBrainsRating]

Fixes #7139

* Improving the `votes` `Joi` schema

Co-authored-by: repo-ranger[bot] <39074581+repo-ranger[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Support questions, usage questions, unconfirmed bugs, discussions, ideas
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants