ci: clarify LLGo dashboard benchmark groups - #15
Conversation
There was a problem hiding this comment.
Review summary
This is a small, focused UI change that groups the matrix rows by benchmark (name shown once per group, full-width separator before each subsequent benchmark). The implementation is clean and correct:
- HTML/attribute escaping is handled correctly — the new
aria-labelatapp.js:312wrapsbenchmarkNameinescapeHtml, which encodes"/</>, preventing attribute breakout in the double-quoted value. - The higher-specificity
.benchmark-group-start ... > .selected-a/.selected-brule (style.css:85-86) correctly layers the group separator on top of the existing accent side-borders rather than clobbering them. - The README description (
README.md:44-46) accurately matches the implemented behavior.
No blocking issues. One optional observation and one inline nit below.
Optional (pre-existing, not introduced here): rankFor (app.js:247, reached per cell via cellHtml) re-derives the full per-(benchmark, measure) ranking — filter + new Set + sort — once for every config row, even though the result is identical across all configs of the same benchmark. This PR's new outer-benchmarkName/inner-config loop shape now makes it natural to compute the ranking once per (column, benchmark) and reuse it across config rows. Not user-perceptible at current data scale; flagging only because the changed loop is the surrounding hot path.
| configs.forEach(function (config) { | ||
| const label = '<th class="matrix-label-cell"><span class="benchmark-name">' + escapeHtml(benchmarkName) + '</span><span class="config-name">' + escapeHtml(compactConfigLabels[config]) + "</span></th>"; | ||
| configs.forEach(function (config, configIndex) { | ||
| const benchmarkLabel = configIndex === 0 ? '<span class="benchmark-name">' + escapeHtml(benchmarkName) + "</span>" : ""; |
There was a problem hiding this comment.
Minor (DRY): configIndex === 0 is tested three times in this loop (lines 310, 311, 316). Hoisting const isGroupStart = configIndex === 0; at the top of the callback and reusing it would remove the repetition and make the grouping intent self-documenting.
Summary
Validation
go test ./cmd/bentbash -n ci/llgo-size/report.shgit diff --check