fix(ci): stop the benchmark suite starving CI and publishing partial numbers

Three open issues on 2.4.0 turned out to be one lane: nothing rationed the
most expensive workflow in the repository.

Runs accumulated. benchmarks.yml declared no concurrency group, so five
pushes over one review loop created five uncancelled eight-shard runs and
left CI and Coverage queued for ~50 minutes behind numbers nobody would
read. The workflow now supersedes its own in-flight run, keyed on the PR
number; the main path is keyed on the commit SHA instead, so every commit
is its own group, cancel-in-progress can never discard one, and the
published series keeps every point. Closes #319.

Runs overlapped. With three branches in flight every shard measured
1.75-2.0x its baseline and one hit the 120-minute cap - on two pull
requests whose diffs were XML doc comments only. The quiet failure is the
worse one: a shard that finishes under uneven contention still publishes
its skewed delta. scripts/benchmark_relevant_changes.js now gates the PR
path and does not run the suite when the diff cannot move a number. It is
one-directional by construction (only documentation, the three projects
Celerity.Benchmarks.csproj does not reference, and .cs files whose text is
unchanged once comments are stripped can be skipped) and never applies to
main, so a gate mistake costs a missing PR comment rather than an unseen
regression. It correctly skips both pull requests the issue names and runs
on every code change it was tested against.

The issue's own first choice - a global serialize-everything concurrency
group - was deliberately not taken: GitHub queues at most one pending run
per group and cancels the older pending one, so serializing would silently
drop runs. What ships is its option 2, which it rated cheapest and most
obviously correct. Closes #335.

The two sides of the A/B packed from different class lists. Greedy
bin-packing is a function of the whole list and the PR head has a class
main does not, so shard i was not the same slice on both sides and could
pair a light head slice with a heavy base one. The base now replays the
class list the head resolved, which makes it a subset of the head by
construction: the pair is bounded by twice the head slice, the quantity
the packer already balances. Option 4 of that issue ships alongside - a
report missing a shard says so above the fold, since a partial comparison
previously read exactly like a complete one. Closes #300.

The comment-stripping rests on a real C# scanner rather than a //-prefix
test, because // occurs inside literals and the verbatim / interpolated /
raw forms desynchronise a guess; a --self-test pins it in a new
benchmark-gate job. --shard-dry-run resolves a shard's class list without
measuring, so the packing is inspectable without a multi-hour run.

Author: marius-bughiu

.github/workflows/benchmarks.yml

@@ -12,6 +12,20 @@ on:
       - 'src/**'
       - '.github/workflows/benchmarks.yml'
 
+# Only the newest run for a given pull request is meaningful — the dashboard and the
+# comparison comment both read the latest — so a push supersedes its predecessor rather
+# than stacking another eight-runner matrix behind it. Five pushes over one review loop
+# created five uncancelled runs and left `CI` and `Coverage`, the checks that actually
+# gate correctness, queued for ~50 minutes behind perf numbers nobody would read.
+#
+# The key is the PR number on the pull-request path and the commit SHA on the main path.
+# That makes every main push its own group, so `cancel-in-progress` can never discard one:
+# each commit's numbers are independently meaningful there, and a cancelled run would
+# leave a hole in the gh-pages time series.
+concurrency:
+  group: benchmarks-${{ github.event.pull_request.number || github.sha }}
+  cancel-in-progress: true
+
 # The core suite is ~300 cases and, at full BenchmarkDotNet accuracy (Job.Default x2
 # launches, see CiConfig.cs), one pass takes ~3h. The PR path runs it TWICE on one
 # runner (PR head + main base, same-runner A/B, so hardware variance cancels), which
@@ -35,8 +49,49 @@ env:
   SHARD_TOTAL: '8'
 
 jobs:
+  # The `paths: src/**` trigger above is a path filter, not a semantic one: an XML
+  # doc-comment edit is a `src/**` change and buys a full sharded A/B run for a diff with
+  # zero IL in it. Two such PRs overlapping is what pushed every shard to 1.75-2.0x its
+  # baseline and one of them into the timeout, so the cheapest correct fix is not
+  # to run the suite when the diff provably cannot change what it measures.
+  #
+  # Pull requests only. A push to main always measures, so the gh-pages series never gains
+  # a hole and a wrongly-skipped PR is still measured on merge — which caps the worst case
+  # of a gate mistake at "the PR comment was missing", never "the regression was unseen".
+  changes:
+    name: relevance gate
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+
+    permissions:
+      contents: read
+
+    outputs:
+      run_benchmarks: ${{ steps.gate.outputs.run }}
+
+    steps:
+      # fetch-depth: 0 so the script can resolve the merge base of the two event SHAs.
+      - uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+
+      - name: Decide whether this diff can move a measured number
+        id: gate
+        run: |
+          set -euo pipefail
+          if [ "${{ github.event_name }}" != "pull_request" ]; then
+            echo "Push to ${{ github.ref }} — always measured, so the published series has no gaps."
+            echo "run=true" >> "$GITHUB_OUTPUT"
+            exit 0
+          fi
+          node scripts/benchmark_relevant_changes.js \
+            "${{ github.event.pull_request.base.sha }}" \
+            "${{ github.event.pull_request.head.sha }}"
+
   benchmark-shard:
     name: benchmark (shard ${{ matrix.shard }})
+    needs: changes
+    if: needs.changes.outputs.run_benchmarks == 'true'
     runs-on: ubuntu-latest
     # Measured at SHARD_TOTAL=6: slices ran 43-65 min, so head + base came to 86-130 min
     # and the heaviest shard overran this timeout. At 8 the same work divides further,
@@ -75,8 +130,14 @@ jobs:
       - name: Run head benchmarks (shard ${{ matrix.shard }})
         # working-directory must be the project folder, not src/. BenchmarkDotNet writes
         # its artifacts to ./BenchmarkDotNet.Artifacts relative to the process CWD.
+        #
+        # --shard-classes-out records the slice the packer resolved here, so the base run
+        # below can replay it instead of packing its own. See that step for why.
         working-directory: src/Celerity.Benchmarks
-        run: dotnet run --configuration Release -- --ci --shard "${SHARD_TOTAL}" "${{ matrix.shard }}"
+        run: >-
+          dotnet run --configuration Release --
+          --ci --shard "${SHARD_TOTAL}" "${{ matrix.shard }}"
+          --shard-classes-out /tmp/head-shard-classes.txt
 
       - name: Stage head report
         run: |
@@ -91,11 +152,19 @@ jobs:
         # on THIS runner so hardware variance cancels (hosted runners vary 20-50%
         # run-to-run, so a stored cross-runner baseline would be noise-dominated).
         #
-        # `--shard` is on main now, so the base tip honours it and measures only this
-        # shard's slice — the transitional full-suite base run that this comment used to
-        # warn about no longer happens. A cancellation here is therefore a real signal (the
-        # slice genuinely exceeded the job timeout) rather than expected behaviour that will
-        # self-heal, and should be investigated instead of dismissed.
+        # The base REPLAYS the head's slice (--shard-classes) rather than packing its own.
+        # Shard membership comes from greedy bin-packing over the benchmark class list, and
+        # a PR that adds a benchmark class gives the two sides *different* lists — so shard
+        # `i` was not the same slice on head and base, a job could draw a light head slice
+        # and a heavy base slice, and the pair overran the timeout even though every
+        # individual slice was well inside it. Replaying makes the base a subset of
+        # the head by construction: the pair is bounded by twice the head slice, which is
+        # the quantity the packer balances, and shard `i` compares like with like.
+        #
+        # A class the PR adds is simply absent from the base's own suite; the process says
+        # so and skips it, and the comparison reports it as new rather than as a delta.
+        # A cancellation here is a real signal (the slice genuinely exceeded the job
+        # timeout) and should be investigated rather than dismissed.
         if: github.event_name == 'pull_request'
         working-directory: ${{ github.workspace }}
         run: |
@@ -105,8 +174,13 @@ jobs:
           echo "Benchmarking base (main) at $BASE_SHA, shard ${{ matrix.shard }}"
           git worktree add /tmp/base-tree "$BASE_SHA"
           cd /tmp/base-tree/src/Celerity.Benchmarks
-          dotnet run --configuration Release -- --ci --shard "${SHARD_TOTAL}" "${{ matrix.shard }}"
-          report=$(ls BenchmarkDotNet.Artifacts/results/*-report-full.json | head -n 1)
+          dotnet run --configuration Release -- --ci --shard-classes /tmp/head-shard-classes.txt
+          report=$(ls BenchmarkDotNet.Artifacts/results/*-report-full.json 2>/dev/null | head -n 1 || true)
+          if [ -z "$report" ]; then
+            echo "Base produced no report for shard ${{ matrix.shard }}: it shares no benchmark"
+            echo "class with the head slice, so every case in it is new on this PR."
+            exit 0
+          fi
           cp "$report" "/tmp/reports/base-shard-${{ matrix.shard }}.json"
           echo "Staged base shard ${{ matrix.shard }}: $report"
 
@@ -120,10 +194,11 @@ jobs:
 
   aggregate:
     name: aggregate & report
-    needs: benchmark-shard
-    # Run even if some shards were cancelled/failed (e.g. the transitional base run
-    # above) so partial results are still reported where possible.
-    if: always()
+    needs: [changes, benchmark-shard]
+    # Run even if some shards were cancelled or failed so partial results are still
+    # reported where possible — but say loudly which shards are missing (see the merge
+    # step), because a partial report otherwise reads exactly like a complete one.
+    if: always() && needs.changes.outputs.run_benchmarks == 'true'
     runs-on: ubuntu-latest
     timeout-minutes: 20
 
@@ -148,6 +223,22 @@ jobs:
         run: |
           set -euo pipefail
           shopt -s nullglob
+
+          # Name the shards that did not report before anything else. `if: always()` means
+          # this job runs on a partial matrix, and the merged report is then legitimately
+          # missing whole benchmark classes — including, on the PR that motivated this,
+          # part of the new collection's own numbers. Nothing downstream could tell.
+          missing=""
+          for s in $(seq 0 $((SHARD_TOTAL - 1))); do
+            if [ ! -f "/tmp/reports/head-shard-${s}.json" ]; then
+              missing="${missing}${missing:+, }${s}"
+            fi
+          done
+          if [ -n "$missing" ]; then
+            echo "::warning title=Incomplete benchmark report::Shard(s) ${missing} produced no report; the comparison below is missing every benchmark class packed onto them."
+          fi
+          echo "missing_shards=${missing}" >> "$GITHUB_OUTPUT"
+
           head_files=(/tmp/reports/head-shard-*.json)
           if [ ${#head_files[@]} -eq 0 ]; then
             echo "No head shard reports found (all shards cancelled/failed) — nothing to report."
@@ -194,6 +285,7 @@ jobs:
         uses: actions/github-script@v7
         env:
           ALERT_THRESHOLD_RATIO: '1.10'
+          MISSING_SHARDS: ${{ steps.merge.outputs.missing_shards }}
         with:
           github-token: ${{ secrets.GITHUB_TOKEN }}
           script: |
@@ -334,11 +426,27 @@ jobs:
             const collections = entries.filter(e => !e.isHasher);
             const hashers = entries.filter(e => e.isHasher);
 
+            // A partial merge otherwise reads exactly like a complete one: the tables are
+            // well-formed and simply have fewer rows, so a silently-dropped shard looks
+            // like a clean report. Say it above the fold, before any numbers.
+            const missingShards = (process.env.MISSING_SHARDS || '').trim();
+            const incomplete = missingShards.length === 0 ? [] : [
+              `> [!WARNING]`,
+              `> **Incomplete report.** Shard(s) \`${missingShards}\` produced no measurements, so every`,
+              `> benchmark class packed onto them is missing from the tables below — including, possibly,`,
+              `> the ones this PR changed. Treat the comparison as partial rather than as a clean run.`,
+              '',
+            ];
+            if (missingShards.length > 0) {
+              core.warning(`Benchmark comparison is missing shard(s) ${missingShards}.`);
+            }
+
             const marker = '<!-- celerity-benchmarks-comment -->';
             const body = [
               marker,
               '## Benchmarks',
               '',
+              ...incomplete,
               subtitle,
               '',
               ...highlights,

.github/workflows/ci.yml

@@ -140,6 +140,22 @@ jobs:
       - name: Check markdown anchors and relative links
         run: node scripts/check_doc_anchors.js
 
+  # The benchmark relevance gate decides whether an expensive sharded A/B run happens at
+  # all, so the C# lexer it rests on is pinned here rather than only exercised in the
+  # workflow it gates — where a wrong answer costs either runner hours or an unmeasured
+  # regression, and neither failure announces itself.
+  benchmark-gate:
+    name: benchmark-gate
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          filter: tree:0
+
+      - name: Pin the comment-stripping lexer
+        run: node scripts/benchmark_relevant_changes.js --self-test
+
   aot-publish:
     name: aot-publish (linux-x64, ${{ matrix.tfm }})
     runs-on: ubuntu-latest

CHANGELOG.md

@@ -24,9 +24,15 @@ All notable changes to Celerity are documented here. This project follows [Keep
 - `scripts/check_doc_anchors.js` — a CI guard that resolves every anchor link and relative file link across all tracked markdown, so a link that scrolls nowhere fails the build instead of shipping. Closes [#339](https://github.com/marius-bughiu/Celerity/issues/339).
 - A `--self-test` mode on that script, pinning the heading-slug rule against ids GitHub actually rendered, and a `doc-anchors` job in `ci.yml` that runs both modes on every PR. Closes [#339](https://github.com/marius-bughiu/Celerity/issues/339).
 - A "Documentation links" section in `CONTRIBUTING.md` covering the slug rule and how to look an anchor up rather than guess it. Closes [#339](https://github.com/marius-bughiu/Celerity/issues/339).
+- `scripts/benchmark_relevant_changes.js` — a gate that skips the sharded benchmark run on a pull request whose diff cannot move a measured number: documentation, the test / fuzz / AOT-smoke projects, or comments inside `.cs` files. It is one-directional (anything it cannot prove inert runs the suite) and never applies to `main`, so a wrong answer costs a PR comment rather than an unmeasured regression. A `--self-test` mode pins its C# comment-stripping lexer, run by a new `benchmark-gate` job in `ci.yml`. Closes [#335](https://github.com/marius-bughiu/Celerity/issues/335).
+- A `--shard-dry-run` switch on the benchmarks runner that resolves a shard's class list and stops, so the packing can be inspected without a multi-hour measuring run. Closes [#300](https://github.com/marius-bughiu/Celerity/issues/300).
 
 ### Fixed
 
+- Pushing to a pull request no longer stacks another eight-runner benchmark matrix behind the last — the workflow supersedes its own in-flight run. Five pushes over one review loop had left `CI` and `Coverage`, the checks that actually gate correctness, queued for ~50 minutes behind perf numbers nobody would read. Pushes to `main` are keyed per commit instead, so none is ever cancelled and the published history keeps every point. Closes [#319](https://github.com/marius-bughiu/Celerity/issues/319).
+- A benchmark shard no longer times out on a pull request that adds a benchmark class. The `main` base now replays the class list the PR head resolved rather than bin-packing its own, so shard *i* is the same slice on both sides and a job can no longer pair a light head slice with a heavy base one. Closes [#300](https://github.com/marius-bughiu/Celerity/issues/300).
+- A benchmark comparison that is missing a shard now says so above the fold in the PR comment and as a workflow warning. Previously a partial report was indistinguishable from a complete one — the tables were simply shorter. Closes [#300](https://github.com/marius-bughiu/Celerity/issues/300).
+
 - `PartialSort.TopK` now throws `ArgumentException` when its `destination` overlaps its `source`, instead of silently returning a wrong answer and writing to the source it documents as untouched. Disjoint slices of one array are still accepted, matching `RadixSort` and `CountingSort`.
 - Eight documentation links pointed at anchors that do not exist: seven `CeleritySet` / `SwissSet` references in `docs/api/collections.md` and one in `CHANGELOG.md`. GitHub deletes `<`, `>` and `,` from a heading without substituting a separator, so `CeleritySet&lt;T, THasher&gt;` anchors as `#celeritysett-thasher`, not the `#celerityset-t-thasher` everyone writes. Closes [#339](https://github.com/marius-bughiu/Celerity/issues/339).
 

CONTRIBUTING.md

@@ -82,9 +82,15 @@ dotnet run -c Release -- --filter '*' # run everything with the default (slow, h
 
 Results are parsed by [`benchmark-action/github-action-benchmark`](https://github.com/benchmark-action/github-action-benchmark) and:
 
-- **On a PR**: a comment is posted with the same-runner A/B comparison vs `main`. Rows that move by more than ±10% *and* beyond the combined standard deviation of both measurements are flagged; the flags are advisory, so a noisy row does not fail the job.
+- **On a PR**: a comment is posted with the same-runner A/B comparison vs `main`. Rows that move by more than ±10% *and* beyond the combined standard deviation of both measurements are flagged; the flags are advisory, so a noisy row does not fail the job. If any shard failed to report, the comment says so above the fold — a partial comparison is otherwise indistinguishable from a clean one.
 - **On a push to `main`**: the new measurement is appended to the `gh-pages`-stored history powering the dashboard at <https://marius-bughiu.github.io/Celerity/dev/bench/>.
 
+Three things about the run are worth knowing before you wonder why it did or did not happen:
+
+- **It supersedes itself.** Pushing to a PR cancels that PR's in-flight benchmark run rather than stacking another eight-runner matrix behind it; only the newest numbers are ever read. Pushes to `main` are keyed per commit instead, so none is ever cancelled and the published history has no gaps.
+- **It is skipped when the diff cannot move a number.** [`scripts/benchmark_relevant_changes.js`](scripts/benchmark_relevant_changes.js) gates the PR path: a diff that touches only documentation, only the test / fuzz / AOT-smoke projects, or only comments inside `.cs` files does not buy a three-hour A/B run. The gate is one-directional — anything it cannot prove inert (an added or deleted file, a `.csproj`, a git command that fails) runs the suite — and it never applies to `main`, so a wrongly-skipped PR is still measured on merge. Run it yourself with `node scripts/benchmark_relevant_changes.js <base> <head>`.
+- **Shard *i* means the same slice on both sides.** The base run replays the class list the head resolved instead of packing its own. Shard membership comes from bin-packing over the benchmark class list, so a PR that *adds* a benchmark class would otherwise pack the two sides differently and could pair a light head slice with a heavy base one.
+
 If a change is motivated by performance, include before/after numbers from a local Release run in the PR description — the CI job is a guardrail, not a precision instrument. Numbers without `-c Release` are not useful — BenchmarkDotNet refuses to run in Debug.
 
 ### The dashboard