report(flow): category tooltip highest impact #13230
Conversation
flow-report/src/summary/category.tsx
Outdated
| .filter(isRelevantAudit) | ||
| .sort((a, b) => { | ||
| if (a.weight === b.weight) return b.result.score - a.result.score; | ||
| return b.weight * (1 - b.result.score) - a.weight * (1 - a.result.score); |
There was a problem hiding this comment.
in #13053, we are considering sorting audits by just weight. here, it's:
- first by some sort of weighted-weight-by-score thing
- then by score
I don't understand 1), could you explain a bit? What's the benefit over just sorting by weight?
Related: perhaps we could sort by overallSavingsMs when the two items are opportunities?
There was a problem hiding this comment.
I don't understand 1), could you explain a bit? What's the benefit over just sorting by weight?
If we just sort by weight, TBT would always be listed at the top of the performance category unless it's >= 0.9 score. If we have a TBT with score 0.89 and a LCP with a score of 0.0, I think the LCP should be listed first.
Related: perhaps we could sort by overallSavingsMs when the two items are opportunities?
Sorting by savings when weight is 0 SGTM
There was a problem hiding this comment.
me and @adamraine discussed:
For perf category, let's add a new section to the tooltip "metrics" showing summary of all metrics. And for the highest impact, for the perf category we only consider opportunities with >0 overallSavingMs
There was a problem hiding this comment.
We can probably land this PR for v9 without metrics in the tooltip. I'll run the metrics section by Jiwoong and we can work on that after CDS.
There was a problem hiding this comment.
OK- can you add that to a new issue / tracking issue?
| const remainingScoreA = getScoreToBeGained(a); | ||
| const remainingScoreB = getScoreToBeGained(b); | ||
| if (remainingScoreA !== remainingScoreB) return remainingScoreB - remainingScoreA; | ||
| return getOverallSavings(b) - getOverallSavings(a); |
There was a problem hiding this comment.
probably rare for the tiebreaker to be used for opp audits, fwiw. this current code will sort opportunities by overalSavingsMs albeit indirectly.
related score calculation:
There was a problem hiding this comment.
remainingScoreX will always be 0 for opportunities because weight is 0.
Are you saying the tiebreaker could be score instead of overallSavingsMs?
There was a problem hiding this comment.
remainingScoreX will always be 0 for opportunities because weight is 0.
Right! I forgot.
So this works, it's just a bit to think about, maybe a comment saying that L80 is always zero for perf category (since we only consider opportunity audits and they are all weight 0)?
Hopefully we can agree on an algo for "highest impact" since it's just ranking audits in a single category.
Thought about excluding metrics, but I think it makes sense to list the highest impact metrics under the performance category.
#11313