core: align performance audit score with metric savings #15447
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
adamraine
requested review from
connorjclark
and removed request for
a team
adamraine
force-pushed
the
score-update-for-new-sort
branch
from
4401c92
to
844dc46
Compare
adamraine
force-pushed
the
score-update-for-new-sort
branch
from
f5dc2c7
to
b4c5ea5
Compare
adamraine
force-pushed
the
score-update-for-new-sort
branch
from
b4c5ea5
to
32cfb1c
Compare
adamraine
changed the title
adamraine
force-pushed
the
score-update-for-new-sort
branch
from
17d4ab4
to
efe9862
Compare
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
adamraine
force-pushed
the
score-update-for-new-sort
branch
from
1836436
to
d256513
Compare
connorjclark
reviewed
Oct 4, 2023
connorjclark
approved these changes
Oct 4, 2023
adamraine
commented
Oct 4, 2023
| // 1 - audit passed | ||
| // 0.5 - audit failed and had no metric savings | ||
| // 0 - audit failed and had metric savings | ||
| metricSavings = 8; |
There was a problem hiding this comment.
There was a name conflict with metricSavings, so I moved this up
Closed
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a new scoring mode
METRIC_SAVINGSwhich will override the audit product score in the following way:In theory, this should put audits into the same buckets established in #15445. The overall impact calculations in #15445 should only affect the bucket of audits that have 0 score.
We could take this a step further by making the score more continuous using the overall impact calculation. For audits that fail and have nonzero savings, we scale the score with the overall impact so audits with higher impact are closer to 0 and audits with lower impact are closer to 0.5. The reason I haven't done that in this PR (yet?) is because it shouldn't affect the end result of our sorting (assuming we are using the report-side change in #15445), and calculating the overall impact in
runner.jscould get messy.This PR also adds a new audit product option
scoreDisplayModewhich overrides the same option on the audit meta. This is useful for audits such asdom-sizewhich can have nonzero metric savings but we should make informative if there is just 10ms of savings or something.However, this does create situations where audits that have no savings are ranked higher than audits which do have savings. Perhaps it would be better to improve our model for savings rather than proactively limit the visibility of some audits based on arbitrary thresholds?