[code-infra] Split PR comment table into sections#1490
Conversation
Group the performance benchmark comment into Regressions, Improvements, and Added & removed sections. Within-noise entries no longer get a row — they collapse into the footer count and the details link only. Add an inline-snapshot test pinning the exact comment markdown across every mutation (duration regression/improvement, render-count change, added, removed, within-noise). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
oliviertassinari
temporarily deployed
to
feat/benchmark-comment-sections - code-infra-dashboard PR #1490
Render
Destroyed
oliviertassinari
temporarily deployed
to
feat/benchmark-comment-sections - mui-tools-public PR #1490
Render
Destroyed
Deploy previewhttps://deploy-preview-1490--mui-internal.netlify.app/ Bundle sizeTotal Size Change: 0B(0.00%) - Total Gzip Change: 0B(0.00%) Show details for 29 more bundles@mui/internal-docs-infra/abstractCreateDemo parsed: 0B(0.00%) gzip: 0B(0.00%) PerformanceTotal duration: 15.97 ms +0.17 ms(+1.0%) | Renders: 4 (+0) | Paint: 70.45 ms +0.76 ms(+1.1%) No significant changes — details Check out the code infra dashboard for more information about this PR. |
JCQuintas
added
type: enhancement
JCQuintas
changed the title
| const renders = `${renderCount}${report.hasBase && entry.renderCount ? formatDiff(entry.renderCount, 'count') : ''}`; | ||
| lines.push(`| ${entry.name} | ${duration} | ${renders} |`); | ||
| } | ||
| renderSection('🔺 Regressions', visibleRegressions); |
There was a problem hiding this comment.
In #673 I was arguing about he potential to optimize the visual feedback, based on the importance of the change.
For having sections, I'm not sure to get the value of sections. The root value seems to be more about how rows are sorted and what information is easy to see first in each row, which seems to be an equivalent problem as with bundle size (#673).
In any case, I do see the ease of quickly scanning the results as equally important as having the data, since we need to see what's stand out to not introduce regressions. So +1 for exploring improvements.
I guess it could use the constant value instead of the emoji for consistency.
| renderSection('🔺 Regressions', visibleRegressions); | |
| renderSection(`${SEVERITY_PREFIX.error} Regressions`, visibleRegressions); |
|
Should we align the way we render benchmarks and bundle size regressions? |
3 participants
Summary
Reworks the performance benchmark PR comment table into clearly separated sections instead of one flat list.
Sections (rendered in this order, each omitted when empty):
#### 🔺 Regressions— duration or render-count got worse#### ▼ Improvements— duration or render-count got better#### ➕ Added & removed— new tests (no baseline) and removed tests (~~name~~ (removed)), no diff columns[details]linkRegressions take priority in the shared
maxRowsbudget so a wall of new/improved tests can't bury a regression. The no-baseline path (single plain table, no diffs) is unchanged.Test plan
pnpm test --run buildMarkdownReport compareBenchmarkReports— 32 passingpnpm typescript+pnpm eslintclean🤖 Generated with Claude Code