Skip to content

Commit e40127e

Browse files
Merge pull request #336 from marius-bughiu/fix/issue-328-dashboard-title-escaping
fix(dashboard): render the full generic type name in card titles
2 parents b3dab85 + c5ec4b6 commit e40127e

6 files changed

Lines changed: 99 additions & 23 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ All notable changes to Celerity are documented here. This project follows [Keep
3232
- **A bad `CHANGELOG.md` could half-publish a release.** The release-notes check ran after the irreversible NuGet push, so a missing section — or one over GitHub's ~125k release-body cap, which this repo has overrun before — left six packages published and no release. It now runs before anything is pushed, with a body-size assertion added, and the validated notes are handed to the release step verbatim. Closes [#315](https://github.com/marius-bughiu/Celerity/issues/315).
3333
- **The `EnumMap` and `EnumSet` cards on the benchmark dashboard rendered empty.** The page required an `(ItemCount: N)` suffix on every result name, and both benchmarks deliberately declare no item-count sweep, so their measurements were published and then discarded at render time. Both cards now chart their real numbers, and unparameterized benchmarks are excluded from the headline speedup stats. Closes [#301](https://github.com/marius-bughiu/Celerity/issues/301).
3434
- A blank dashboard card is now a red CI check rather than a silent gap: `scripts/check_dashboard_coverage.js` fails when a published result name is unparseable, when a card has no measurements behind it, or when a charted collection is missing from either `COLLECTIONS` array or from the CI benchmark suite. Closes [#301](https://github.com/marius-bughiu/Celerity/issues/301).
35+
- **Benchmark-dashboard cards showed truncated type names.** Every card and detail page dropped the generic parameters from its title and `vs` baseline — `IntDictionary` instead of `IntDictionary<int>`, and an identical `vs Dictionary` on cards measured against `Dictionary<int, int>`, `Dictionary<long, int>` and `Dictionary<int, List<int>>` alike. Titles now render in full on the grid, the detail pages, and the hasher section. Closes [#328](https://github.com/marius-bughiu/Celerity/issues/328).
36+
- A card label that would render truncated is now caught in CI: `scripts/check_dashboard_coverage.js` fails on any dashboard title or baseline concatenated into markup without escaping, and `CONTRIBUTING.md` records that `COLLECTIONS` labels are written as plain text. Closes [#328](https://github.com/marius-bughiu/Celerity/issues/328).
3537
- **The coverage gate measured only one of the six shipped packages.** Coverlet's assembly filter is exact-match, so `Celerity.Hashing`, `Celerity.Primitives`, and the three showcase packages had been outside the gate since the 2.0.0 package split — any of them could have dropped to 0% with CI green. All six are now measured, the gaps that exposed are backfilled to **100% line and branch** coverage, and the floor is raised from 95%/90% to match. Closes [#314](https://github.com/marius-bughiu/Celerity/issues/314).
3638

3739
## [2.4.0] - 2026-07-26

CONTRIBUTING.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,16 @@ Two rules keep a benchmark chartable:
9797

9898
Adding a collection to the site means updating three lists by hand, since the published data alone does not tell the page what to draw: the ship card in `web/index.html`, and the `COLLECTIONS` array in **both** `web/dev/bench/index.html` and `web/dev/bench/detail.html` (`items: [NO_SWEEP]` for an unparameterized class).
9999

100+
Write the `title` and `vs` in those arrays as plain text — `BTreeSet<int>`, not `BTreeSet&lt;int&gt;`. They are escaped at render time, so a pre-escaped label renders its entities literally. The flip side is that a label must never be concatenated into an `innerHTML` template raw: a generic parameter is then parsed as a start tag and vanishes from the heading. The check below fails CI on that.
101+
100102
[`scripts/check_dashboard_coverage.js`](scripts/check_dashboard_coverage.js) enforces all of this so the failure mode is a red check rather than a blank card. It lifts the `COLLECTIONS` tables and the name parsers out of the dashboard HTML rather than reimplementing them, so it validates the code that actually ships. Run it any time you touch a benchmark or the dashboard:
101103

102104
```bash
103105
node scripts/check_dashboard_coverage.js # structural checks
104106
node scripts/check_dashboard_coverage.js path/to/joined-report-full.json # + verify the data
105107
```
106108

107-
The structural half — the two `COLLECTIONS` arrays agree, and every charted collection has a `{Key}Benchmark` registered in `CoreBenchmarks` — runs on every PR in `ci.yml`. The full check runs in the aggregate job of `benchmarks.yml`, against the merged report, and additionally asserts that every published name parses and that every card resolves to both a BCL and a Celerity measurement.
109+
The structural half — the two `COLLECTIONS` arrays agree, every charted collection has a `{Key}Benchmark` registered in `CoreBenchmarks`, and no label reaches an `innerHTML` template unescaped — runs on every PR in `ci.yml`. The full check runs in the aggregate job of `benchmarks.yml`, against the merged report, and additionally asserts that every published name parses and that every card resolves to both a BCL and a Celerity measurement.
108110

109111
## Versioning
110112

ROADMAP.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ The 2026-Q3 review surveyed the shipped surface against the current .NET 8/9/10
231231
- The coverage gate measures one of the six shipping assemblies. `src/coverage.runsettings` filters to `[Celerity]*` with the comment "Measure only the shipping library assembly" — written when there was one. `Celerity.Hashing`, `Celerity.Primitives` and the three showcase packages are unmeasured, while `CONTRIBUTING.md` and `CLAUDE.md` describe the 95%/90% gate as library-wide. Status: `done` — all six are now measured and the floor is 100% line / 100% branch. Tracked in [#314](https://github.com/marius-bughiu/Celerity/issues/314).
232232
- Nothing can fail after the NuGet push. `release.yml` pushes six packages irreversibly, *then* extracts the release notes and creates the GitHub Release — so an over-long release body (a failure this repo has actually hit) leaves a half-published release. The notes check should be hoisted ahead of the push. Status: `done` — extraction, the empty-section check and a new body-size assertion all run in `build`. Tracked in [#315](https://github.com/marius-bughiu/Celerity/issues/315).
233233
- No API-compatibility gate. Six packages publish on a tag with no `ApiCompat` / `PackageValidation` / public-API-baseline check anywhere in the repo — in a project that already needed a hand-written `TypeForwarders.cs` to survive one assembly split. Status: `done``EnablePackageValidation` against a pinned baseline now fails `pack` on any breaking change. Tracked in [#315](https://github.com/marius-bughiu/Celerity/issues/315).
234-
- No guard on the benchmark dashboard. The site parses BenchmarkDotNet result *names*, so a benchmark it cannot parse is dropped at render time — the data publishes correctly and the card just goes blank, with no CI signal. `EnumMap` and `EnumSet` had rendered empty since they shipped (they declare no `[Params]` sweep, by design), and `DisjointSet` blanked for five runs when its params property was briefly named `ElementCount`. Status: `done` — the parser now treats the `ItemCount` suffix as optional and renders an unparameterized class as a single bucket, excluded from the headline stats; `scripts/check_dashboard_coverage.js` fails CI on an unparseable name, a card with no measurements behind it, a collection missing from either `COLLECTIONS` array, or one not registered in the CI benchmark suite. It lifts those tables and the parsers out of the dashboard HTML rather than reimplementing them, so the check cannot drift from the page it guards. Tracked in [#301](https://github.com/marius-bughiu/Celerity/issues/301).
234+
- No guard on the benchmark dashboard. The site parses BenchmarkDotNet result *names*, so a benchmark it cannot parse is dropped at render time — the data publishes correctly and the card just goes blank, with no CI signal. `EnumMap` and `EnumSet` had rendered empty since they shipped (they declare no `[Params]` sweep, by design), and `DisjointSet` blanked for five runs when its params property was briefly named `ElementCount`. Status: `done` — the parser now treats the `ItemCount` suffix as optional and renders an unparameterized class as a single bucket, excluded from the headline stats; `scripts/check_dashboard_coverage.js` fails CI on an unparseable name, a card with no measurements behind it, a collection missing from either `COLLECTIONS` array, or one not registered in the CI benchmark suite. It lifts those tables and the parsers out of the dashboard HTML rather than reimplementing them, so the check cannot drift from the page it guards. Tracked in [#301](https://github.com/marius-bughiu/Celerity/issues/301). A second silent-drop mode in the same page — a *label* rather than a measurement — was found and closed afterwards: the `COLLECTIONS` titles and `vs` baselines were concatenated into `innerHTML` raw, so every card lost its generic parameters (`IntDictionary` for `IntDictionary<int>`, and one indistinguishable `vs Dictionary` for three different baselines) and `EnumSet<TEnum>` even materialized a stray `<tenum>` element. Both dashboard pages now escape every label, and the coverage check gained a structural rule that fails CI on any label reaching a markup template unescaped. Status: `done`. Tracked in [#328](https://github.com/marius-bughiu/Celerity/issues/328).
235235

236236
Two areas were judged real but deliberately deferred rather than rostered: a `Celerity.Statistics` package (DDSketch / reservoir sampling / running moments — a coherent fourth axis, but two new packages in one cycle is too much at once), and a batch of fuzz-target and AOT-smoke-coverage gaps (real, but low expected defect yield; better folded into whichever collection PR lands next than pursued on their own).
237237

scripts/check_dashboard_coverage.js

Lines changed: 74 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,33 @@
11
#!/usr/bin/env node
22
//
3-
// Fails when the benchmark dashboard would silently render an empty card.
3+
// Fails when the benchmark dashboard would silently drop what it was asked to render.
44
//
55
// The dashboard parses BenchmarkDotNet result names with regexes, and a name that does
66
// not match is dropped without a trace: the data publishes to gh-pages correctly and the
77
// card just renders blank. That has happened twice — EnumMap / EnumSet declare no
88
// [Params] at all, and DisjointSet once named its params property ElementCount — and
9-
// neither produced any CI signal.
9+
// neither produced any CI signal. The same silence applies to the card *labels*: a title
10+
// like `IntDictionary<int>` written straight to innerHTML has its generic parameters
11+
// parsed as a start tag and swallowed, so the card renders with a truncated heading.
1012
//
11-
// This check closes that gap. It lifts the COLLECTIONS tables and the two name parsers
13+
// This check closes those gaps. It lifts the COLLECTIONS tables and the two name parsers
1214
// straight out of the dashboard HTML rather than reimplementing them, so it validates
1315
// the parser that actually ships and cannot drift from it.
1416
//
1517
// Structural checks, run on every PR (no benchmark run needed):
1618
// 1. index.html and detail.html agree on the collection keys and their item counts;
1719
// 2. every charted collection has a matching benchmark class registered in the
18-
// CoreBenchmarks array of src/Celerity.Benchmarks/Program.cs.
20+
// CoreBenchmarks array of src/Celerity.Benchmarks/Program.cs;
21+
// 3. no markup-shaped label is concatenated into an innerHTML template unescaped.
1922
//
2023
// Report checks, run in the benchmark job once the sharded suite has been merged:
21-
// 3. every benchmark name in the report is understood by one of the dashboard parsers;
22-
// 4. every (collection, op) pair the dashboard draws a card for resolves to both a BCL
24+
// 4. every benchmark name in the report is understood by one of the dashboard parsers;
25+
// 5. every (collection, op) pair the dashboard draws a card for resolves to both a BCL
2326
// and a Celerity measurement.
2427
//
2528
// Usage:
26-
// node scripts/check_dashboard_coverage.js # 1-2 only
27-
// node scripts/check_dashboard_coverage.js <joined-report-full.json> # 1-4
29+
// node scripts/check_dashboard_coverage.js # 1-3 only
30+
// node scripts/check_dashboard_coverage.js <joined-report-full.json> # 1-5
2831
// Run from the repository root.
2932

3033
'use strict';
@@ -102,6 +105,56 @@ function loadBenchmarkNames(reportPath) {
102105
return report.Benchmarks.map((b) => b.FullName).filter(Boolean);
103106
}
104107

108+
// ---- Unescaped-label detection ------------------------------------------------------
109+
// The COLLECTIONS titles and `vs` baselines are trusted in-repo literals, but they are
110+
// markup-shaped: `IntDictionary<int>` concatenated into an innerHTML template is parsed
111+
// as a start tag, so the visible heading loses its generic parameters (and, for
112+
// `EnumSet<TEnum>`, materializes a stray <tenum> element). Every such label has to pass
113+
// through the page's escapeHtml() on its way to an HTML sink.
114+
//
115+
// Both concatenation directions are matched, but only inside a statement that is building
116+
// markup: one feeding an HTML sink, or one splicing the label into a string literal that
117+
// opens a tag. Concatenating a label anywhere else is fine — the attribute API, textContent
118+
// and a query string all take text, not markup — so those are not flagged.
119+
const LABEL_FIELDS = '(?:title|vs|sub)';
120+
const LABEL_OWNERS = '(?:col|collection|meta|c)';
121+
const RAW_LABEL = new RegExp(
122+
`\\+\\s*${LABEL_OWNERS}\\.${LABEL_FIELDS}\\b|\\b${LABEL_OWNERS}\\.${LABEL_FIELDS}\\s*\\+`
123+
);
124+
const MARKUP_CONTEXT = /\.(?:inner|outer)HTML\b|insertAdjacentHTML\(|document\.write\(|['"]</;
125+
126+
// A template is routinely spread over several source lines, and the sink that says whether
127+
// it is markup at all sits on the first of them — so lines are folded into logical
128+
// statements first, joining any line that ends on a continuation token with the next.
129+
const CONTINUES = /[,+(=]$|&&$|\|\|$/;
130+
131+
function logicalLines(source) {
132+
const raw = source.split(/\r?\n/);
133+
const out = [];
134+
let buf = null;
135+
raw.forEach((line, i) => {
136+
const trimmed = line.trim();
137+
if (buf === null) buf = { line: i + 1, text: trimmed };
138+
else buf.text += ' ' + trimmed;
139+
if (!CONTINUES.test(trimmed)) {
140+
out.push(buf);
141+
buf = null;
142+
}
143+
});
144+
if (buf !== null) out.push(buf);
145+
return out;
146+
}
147+
148+
function findRawLabels(file) {
149+
return logicalLines(fs.readFileSync(file, 'utf8'))
150+
.filter((s) => MARKUP_CONTEXT.test(s.text) && RAW_LABEL.test(s.text))
151+
.map((s) => ({
152+
line: s.line,
153+
match: s.text.match(RAW_LABEL)[0].trim(),
154+
text: s.text.length > 160 ? s.text.slice(0, 157) + '...' : s.text,
155+
}));
156+
}
157+
105158
// ---- Checks -------------------------------------------------------------------------
106159

107160
function main() {
@@ -142,10 +195,21 @@ function main() {
142195
}
143196
}
144197

198+
// (3) A markup-shaped label written raw to innerHTML renders truncated.
199+
for (const file of [INDEX_HTML, DETAIL_HTML]) {
200+
for (const hit of findRawLabels(file)) {
201+
problems.push(
202+
`${file}:${hit.line} concatenates \`${hit.match}\` into a markup string without escapeHtml() — ` +
203+
`a generic type name in that label is parsed as a tag and dropped from the rendered heading. ` +
204+
`Line: ${hit.text}`
205+
);
206+
}
207+
}
208+
145209
if (reportPath) {
146210
const names = loadBenchmarkNames(reportPath);
147211

148-
// (3) Nothing in the report may be silently unrenderable.
212+
// (4) Nothing in the report may be silently unrenderable.
149213
const unparsed = names.filter((n) => !index.parseName(n) && !index.parseHasher(n, 0));
150214
if (unparsed.length > 0) {
151215
const classes = [...new Set(unparsed.map((n) => n.split('.')[0]))];
@@ -156,7 +220,7 @@ function main() {
156220
);
157221
}
158222

159-
// (4) Every card the dashboard draws must have both series behind it.
223+
// (5) Every card the dashboard draws must have both series behind it.
160224
const idx = {};
161225
for (const name of names) {
162226
const p = index.parseName(name);

web/dev/bench/detail.html

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -540,8 +540,8 @@
540540
function renderShell() {
541541
contentEl.innerHTML =
542542
'<div class="title-block">' +
543-
'<h1 class="detail-title">' + collection.title + ' &middot; <span id="op-label"></span></h1>' +
544-
'<div class="detail-sub">vs <code id="vs-label">' + collection.vs + '</code> &middot; <span id="n-label"></span></div>' +
543+
'<h1 class="detail-title">' + escapeHtml(collection.title) + ' &middot; <span id="op-label"></span></h1>' +
544+
'<div class="detail-sub">vs <code id="vs-label">' + escapeHtml(collection.vs) + '</code> &middot; <span id="n-label"></span></div>' +
545545
'<div class="toggle" id="n-toggle"></div>' +
546546
'</div>' +
547547
'<div class="headline">' +
@@ -614,8 +614,9 @@
614614
// Replace the dynamic region with a notice but keep the header.
615615
var notice = document.createElement('div');
616616
notice.className = 'notice';
617-
// `op` is whatever the query string says, so it is escaped before it reaches innerHTML.
618-
notice.innerHTML = 'No measurements recorded for <code>' + collection.title + '.' + escapeHtml(params.op) + '</code>' +
617+
// The title is markup-shaped and `op` is whatever the query string says, so both are
618+
// escaped before they reach innerHTML.
619+
notice.innerHTML = 'No measurements recorded for <code>' + escapeHtml(collection.title) + '.' + escapeHtml(params.op) + '</code>' +
619620
(params.n == null ? '' : ' at ' + params.n.toLocaleString() + ' items') + ' yet. <a href="./">Back to dashboard &rarr;</a>';
620621
// Remove headline + chart + table; insert notice in their place.
621622
['headline','chart-wrap','section-head','measurements'].forEach(function () { /* placeholder */ });

0 commit comments

Comments
 (0)