Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ All notable changes to Celerity are documented here. This project follows [Keep
- **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).
- **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).
- 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).
- **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).
- 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).
- **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).

## [2.4.0] - 2026-07-26
Expand Down
4 changes: 3 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,16 @@ Two rules keep a benchmark chartable:

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).

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.

[`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:

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

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.
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.

## Versioning

Expand Down
2 changes: 1 addition & 1 deletion ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ The 2026-Q3 review surveyed the shipped surface against the current .NET 8/9/10
- 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).
- 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).
- 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).
- 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).
- 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).

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).

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

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

// ---- Unescaped-label detection ------------------------------------------------------
// The COLLECTIONS titles and `vs` baselines are trusted in-repo literals, but they are
// markup-shaped: `IntDictionary<int>` concatenated into an innerHTML template is parsed
// as a start tag, so the visible heading loses its generic parameters (and, for
// `EnumSet<TEnum>`, materializes a stray <tenum> element). Every such label has to pass
// through the page's escapeHtml() on its way to an HTML sink.
//
// Both concatenation directions are matched, but only inside a statement that is building
// markup: one feeding an HTML sink, or one splicing the label into a string literal that
// opens a tag. Concatenating a label anywhere else is fine — the attribute API, textContent
// and a query string all take text, not markup — so those are not flagged.
const LABEL_FIELDS = '(?:title|vs|sub)';
const LABEL_OWNERS = '(?:col|collection|meta|c)';
const RAW_LABEL = new RegExp(
`\\+\\s*${LABEL_OWNERS}\\.${LABEL_FIELDS}\\b|\\b${LABEL_OWNERS}\\.${LABEL_FIELDS}\\s*\\+`
);
const MARKUP_CONTEXT = /\.(?:inner|outer)HTML\b|insertAdjacentHTML\(|document\.write\(|['"]</;

// A template is routinely spread over several source lines, and the sink that says whether
// it is markup at all sits on the first of them — so lines are folded into logical
// statements first, joining any line that ends on a continuation token with the next.
const CONTINUES = /[,+(=]$|&&$|\|\|$/;

function logicalLines(source) {
const raw = source.split(/\r?\n/);
const out = [];
let buf = null;
raw.forEach((line, i) => {
const trimmed = line.trim();
if (buf === null) buf = { line: i + 1, text: trimmed };
else buf.text += ' ' + trimmed;
if (!CONTINUES.test(trimmed)) {
out.push(buf);
buf = null;
}
});
if (buf !== null) out.push(buf);
return out;
}

function findRawLabels(file) {
return logicalLines(fs.readFileSync(file, 'utf8'))
.filter((s) => MARKUP_CONTEXT.test(s.text) && RAW_LABEL.test(s.text))
.map((s) => ({
line: s.line,
match: s.text.match(RAW_LABEL)[0].trim(),
text: s.text.length > 160 ? s.text.slice(0, 157) + '...' : s.text,
}));
}

// ---- Checks -------------------------------------------------------------------------

function main() {
Expand Down Expand Up @@ -142,10 +195,21 @@ function main() {
}
}

// (3) A markup-shaped label written raw to innerHTML renders truncated.
for (const file of [INDEX_HTML, DETAIL_HTML]) {
for (const hit of findRawLabels(file)) {
problems.push(
`${file}:${hit.line} concatenates \`${hit.match}\` into a markup string without escapeHtml() — ` +
`a generic type name in that label is parsed as a tag and dropped from the rendered heading. ` +
`Line: ${hit.text}`
);
}
}

if (reportPath) {
const names = loadBenchmarkNames(reportPath);

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

// (4) Every card the dashboard draws must have both series behind it.
// (5) Every card the dashboard draws must have both series behind it.
const idx = {};
for (const name of names) {
const p = index.parseName(name);
Expand Down
9 changes: 5 additions & 4 deletions web/dev/bench/detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -540,8 +540,8 @@
function renderShell() {
contentEl.innerHTML =
'<div class="title-block">' +
'<h1 class="detail-title">' + collection.title + ' &middot; <span id="op-label"></span></h1>' +
'<div class="detail-sub">vs <code id="vs-label">' + collection.vs + '</code> &middot; <span id="n-label"></span></div>' +
'<h1 class="detail-title">' + escapeHtml(collection.title) + ' &middot; <span id="op-label"></span></h1>' +
'<div class="detail-sub">vs <code id="vs-label">' + escapeHtml(collection.vs) + '</code> &middot; <span id="n-label"></span></div>' +
'<div class="toggle" id="n-toggle"></div>' +
'</div>' +
'<div class="headline">' +
Expand Down Expand Up @@ -614,8 +614,9 @@
// Replace the dynamic region with a notice but keep the header.
var notice = document.createElement('div');
notice.className = 'notice';
// `op` is whatever the query string says, so it is escaped before it reaches innerHTML.
notice.innerHTML = 'No measurements recorded for <code>' + collection.title + '.' + escapeHtml(params.op) + '</code>' +
// The title is markup-shaped and `op` is whatever the query string says, so both are
// escaped before they reach innerHTML.
notice.innerHTML = 'No measurements recorded for <code>' + escapeHtml(collection.title) + '.' + escapeHtml(params.op) + '</code>' +
(params.n == null ? '' : ' at ' + params.n.toLocaleString() + ' items') + ' yet. <a href="./">Back to dashboard &rarr;</a>';
// Remove headline + chart + table; insert notice in their place.
['headline','chart-wrap','section-head','measurements'].forEach(function () { /* placeholder */ });
Expand Down
Loading
Loading