Skip to content

ci(bench): remove fail-on-alert — regression is informational, not a gate #158

Description

@polaz

Problem

.github/workflows/ci.yml benchmark-aggregate job runs benchmark-action/github-action-benchmark@v1 with fail-on-alert: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }}. When a benchmark exceeds fail-threshold: 160% on a main push, the action sets the step to fail → the whole benchmark-aggregate job fails → downstream benchmark-pages (which has `needs: benchmark-aggregate`) is skipped, and the same step's `save-data-file` / `auto-push` never run because they're inside the failed step.

This creates a stuck-baseline cascade:

  1. Main push A introduces a regression > 60% on some bench → `Store benchmark results` step fails → `save-data-file` doesn't run → baseline in `gh-pages` stays on the previous SHA
  2. Main push B inherits the same regression (it's now in `main` HEAD) → comparison against the same old baseline → same alert → same fail
  3. ... until either the regression is reverted in code, or someone manually unbreaks the baseline

Broken since SHA `1e0f6954` (16 May 02:28 KYIV), where `compress/level_9_lazy/decodecorpus-z000033/matrix/pure_rust` hit ratio 1.67 vs baseline `fa2e9ff6`. Every main push since then either failed at the same alert or got cancelled by concurrency before reaching the aggregate step.

Design intent

Regression alerts are an informational gate for developer-submitted PRs, where a reviewer can decide whether the change is acceptable. They are not a gate for:

  • Main push — `main` is the immutable historical record. The dashboard at `dev/bench` surfaces regressions visually over time. Gating publish on alerts means the dashboard goes stale at the worst possible moment — exactly when there's a regression to look at.
  • release-plz PRs — those are version-bump only (no source changes) so perf comparison is noise; they shouldn't be perf-gated.
  • Cancelled concurrency — main pushes that get cancelled mid-flight must not poison subsequent runs.

Fix — option C: separate the gate

Two changes to .github/workflows/ci.yml:

  1. In benchmark-aggregate's Store benchmark results step: drop `fail-on-alert` (always false), move `comment-on-alert` to the new gate job. Keep `save-data-file` + `auto-push` for main push so the baseline keeps advancing.
  2. Add a new `benchmark-regression-check` job with `needs: benchmark-aggregate`. `if:` filter excludes `release-plz` PRs and direct main pushes — runs ONLY on regular developer PRs from the same repo. This job does the alert + comment + fail-on-alert handling. Failing this job does NOT block publish or baseline updates.

Acceptance

  • `fail-on-alert` removed from the save step → `benchmark-pages` always runs after `benchmark-aggregate` completes
  • Baseline in `gh-pages` advances on every main push (assuming bench shards themselves passed)
  • Regular PRs: regression > 60% → comment on commit + `@polaz` ping + new `benchmark-regression-check` job fails red (does NOT block merge by itself unless it's a required check)
  • release-plz PRs: no regression-check job runs at all (filtered out)
  • Main pushes: `save-data-file` + `auto-push` run unconditionally (subject only to bench shards passing); no perf-gate failure path on main

Time estimate

30 min.

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2-mediumMedium priority — important improvementenhancementNew feature or requestperformancePerformance optimization

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions