-
Notifications
You must be signed in to change notification settings - Fork 536
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
Change growth threshold algorithm do use percentage points increase #22875
Change growth threshold algorithm do use percentage points increase #22875
Conversation
Also contains a refactor to make the code easier to test, as well as admin & reviewer tools tweaks to make the consequences of changing that threshold more obvious. If the average hotness in a tier is 0.1, and the growth threshold is set to 50, we used to flag add-ons in that tier with hotness over 0.15 (percentage increase) now we'll flag over 0.51 (percentage points increase).
a07420e
to
75689b9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good in principle.. I have a slight doubt/question about the algo change.
The value is computed from the average growth (hotness) of add-ons in | ||
that tier plus the growth_threshold_before_flagging converted to | ||
decimal.""" | ||
return self.average_growth + self.growth_threshold_before_flagging / 100 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the old algorithm this would have been:
return self.average_growth * (
1 + self.growth_threshold_before_flagging / 100
)
@diox is it possble to get more specific criteria for a given addon, which values should be set to what in order to trigger or not trigger the flag? |
It's difficult since it depends on the average On a local env, assuming a completely empty database (or at least no add-ons with a
With that setup, the average |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 🚢
…22875) * Change growth threshold algorithm do use percentage points increase Also contains a refactor to make the code easier to test, as well as admin & reviewer tools tweaks to make the consequences of changing that threshold more obvious. If the average hotness in a tier is 0.1, and the growth threshold is set to 50, we used to flag add-ons in that tier with hotness over 0.15 (percentage increase) now we'll flag over 0.51 (percentage points increase). * Add test with negative average hotness * Add assert num queries
Also contains a refactor to make the code easier to test, as well as admin & reviewer tools tweaks to make the consequences of changing that threshold more obvious.
If the average hotness in a tier is 0.1, and the growth threshold is set to 50, we used to flag add-ons in that tier with hotness over 0.15 (percentage increase) now we'll flag over 0.51 (percentage points increase).
Fixes mozilla/addons#15178
Screenshots
New admin features:
Testing
You can add addons with various
hotness
andaverage_daily_users
values to your local environment to test and triggerflag_high_hotness_according_to_review_tier
task (if the add-on is flagged, it should get aNeedsHumanReview
instance withHOTNESS_THRESHOLD
reason attached to the latest signed version in each channel that hasn't already been explicitly reviewed by a human), or trust the unit tests that do that.