Filter stale Bencher alerts before reporting

benchmarks/lib/bencher_report.rb

@@ -84,7 +84,7 @@ def initialize(raw)
     raise FormatError, "report is not a JSON object (got #{raw.class})" unless raw.is_a?(Hash)
 
     @boundaries = index_boundaries(raw)
-    @alerts = parse_alerts(raw)
+    @alerts, @filtered_alerts = parse_alerts(raw).partition { |alert| current_regression_alert?(alert) }
     # Per-benchmark perf links are informational (they only decide whether a name links
     # out), so they live in a separate, fully-lenient builder — a missing field yields an
     # unlinked name, never a FormatError that would fail the job over a cosmetic link.
@@ -93,9 +93,9 @@ def initialize(raw)
 
   attr_reader :alerts
 
-  def regression?
-    !@alerts.empty?
-  end
+  def regression? = !@alerts.empty?
+
+  def filtered_alert? = !@filtered_alerts.empty?
 
   # Boundary for a benchmark+measure, or nil if absent from the report.
   def boundary(benchmark_name, measure_key)
@@ -118,9 +118,7 @@ def perf_url(benchmark_name)
   # plain name. A single benchmark missing its own uuid stays silent (a per-row cosmetic
   # miss); losing the shared context instead is a likely report-contract drift, so the
   # caller surfaces it as a ::warning:: — never a FormatError (the links are cosmetic).
-  def perf_links_unavailable?
-    @perf_urls.any_benchmarks? && !@perf_urls.context_ready?
-  end
+  def perf_links_unavailable? = @perf_urls.any_benchmarks? && !@perf_urls.context_ready?
 
   private
 
@@ -189,6 +187,31 @@ def parse_alerts(raw)
     end
   end
 
+  # rubocop:disable Metrics/CyclomaticComplexity
+  def current_regression_alert?(alert)
+    return true unless alert.benchmark
+
+    direction = { "lower" => :lower, "upper" => :upper }[normalize(alert.limit)]
+    return true unless direction
+
+    unless alert.measure
+      matching_boundaries = @boundaries.fetch(alert.benchmark, {}).values.select do |boundary|
+        threshold_side?(boundary, direction)
+      end
+      return true if matching_boundaries.empty?
+
+      return matching_boundaries.any? { |boundary| boundary.significance(direction) == :regression }
+    end
+
+    boundary = boundary(alert.benchmark, alert.measure)
+    return true unless boundary && threshold_side?(boundary, direction)
+
+    boundary.significance(direction) == :regression
+  end
+  # rubocop:enable Metrics/CyclomaticComplexity
+
+  def threshold_side?(boundary, direction) = direction == :lower ? boundary.lower_limit : boundary.upper_limit
+
   def normalize(key)
     key.to_s.downcase.gsub(/[-\s]+/, "_")
   end

benchmarks/spec/bencher_report_spec.rb

@@ -39,16 +39,27 @@ def report_json(results: [], alerts: [])
     JSON.generate("results" => results, "alerts" => alerts)
   end
 
+  def rps_regression_result(name: "/foo: Core", value: 80.0)
+    benchmark_result(
+      name:,
+      measures: [measure_entry(slug: "rps", name: "rps", value:, baseline: 100.0,
+                               lower_limit: 90.0, upper_limit: nil)]
+    )
+  end
+
   describe ".parse / #regression?" do
     it "reports no regression when alerts is empty" do
       report = described_class.parse(report_json)
       expect(report.regression?).to be(false)
       expect(report.alerts).to be_empty
     end
 
-    it "reports a regression and exposes the active alert's benchmark/measure/side" do
+    it "reports a current regression and exposes the active alert's benchmark/measure/side" do
       report = described_class.parse(
-        report_json(alerts: [alert(benchmark: "/foo: Core", measure_slug: "rps", limit: "lower")])
+        report_json(
+          results: [[rps_regression_result]],
+          alerts: [alert(benchmark: "/foo: Core", measure_slug: "rps", limit: "lower")]
+        )
       )
       expect(report.regression?).to be(true)
       expect(report.alerts.size).to eq(1)
@@ -57,6 +68,134 @@ def report_json(results: [], alerts: [])
       expect(report.alerts.first.limit).to eq("lower")
     end
 
+    it "ignores active alerts when the current metric is not a regression" do
+      report = described_class.parse(
+        report_json(
+          results: [[rps_regression_result(value: 95.0)]],
+          alerts: [alert(benchmark: "/foo: Core", measure_slug: "rps", limit: "lower")]
+        )
+      )
+
+      expect(report.regression?).to be(false)
+      expect(report.alerts).to be_empty
+      expect(report.filtered_alert?).to be(true)
+    end
+
+    it "keeps active alerts with no benchmark name fail-safe" do
+      malformed_alert = alert(benchmark: "/foo: Core", measure_slug: "rps", limit: "lower")
+      malformed_alert.delete("benchmark")
+
+      report = described_class.parse(report_json(alerts: [malformed_alert]))
+
+      expect(report.regression?).to be(true)
+      expect(report.alerts.first.benchmark).to be_nil
+      expect(report.filtered_alert?).to be(false)
+    end
+
+    it "keeps active alerts with no matching boundary fail-safe" do
+      report = described_class.parse(
+        report_json(
+          results: [[benchmark_result(
+            name: "/foo: Core",
+            measures: [measure_entry(slug: "rps", name: "rps", value: 95.0, boundary: nil)]
+          )]],
+          alerts: [alert(benchmark: "/foo: Core", measure_slug: "rps", limit: "lower")]
+        )
+      )
+
+      expect(report.regression?).to be(true)
+      expect(report.filtered_alert?).to be(false)
+    end
+
+    it "keeps measure-less active alerts fail-safe when no boundary exists on the alert side" do
+      measureless_alert = alert(benchmark: "/foo: Core", measure_slug: "rps", limit: "lower")
+      measureless_alert["threshold"] = {}
+
+      report = described_class.parse(
+        report_json(
+          results: [[benchmark_result(
+            name: "/foo: Core",
+            measures: [measure_entry(slug: "p50-latency", name: "p50_latency", value: 5.5,
+                                     baseline: 5.0, lower_limit: nil, upper_limit: 6.0)]
+          )]],
+          alerts: [measureless_alert]
+        )
+      )
+
+      expect(report.regression?).to be(true)
+      expect(report.filtered_alert?).to be(false)
+    end
+
+    it "keeps measure-less active alerts when the benchmark has a current regression on the alert side" do
+      measureless_alert = alert(benchmark: "/foo: Core", measure_slug: "rps", limit: "lower")
+      measureless_alert["threshold"] = {}
+
+      report = described_class.parse(
+        report_json(
+          results: [[rps_regression_result]],
+          alerts: [measureless_alert]
+        )
+      )
+
+      expect(report.regression?).to be(true)
+      expect(report.alerts.first.benchmark).to eq("/foo: Core")
+      expect(report.alerts.first.measure).to be_nil
+    end
+
+    it "ignores measure-less active alerts when the benchmark has no current regression on the alert side" do
+      measureless_alert = alert(benchmark: "/foo: Core", measure_slug: "rps", limit: "lower")
+      measureless_alert["threshold"] = {}
+
+      report = described_class.parse(
+        report_json(
+          results: [[rps_regression_result(value: 95.0)]],
+          alerts: [measureless_alert]
+        )
+      )
+
+      expect(report.regression?).to be(false)
+      expect(report.alerts).to be_empty
+    end
+
+    it "ignores opposite-side improvements when matching measure-less active alerts" do
+      measureless_alert = alert(benchmark: "/foo: Core", measure_slug: "rps", limit: "lower")
+      measureless_alert["threshold"] = {}
+
+      report = described_class.parse(
+        report_json(
+          results: [[benchmark_result(
+            name: "/foo: Core",
+            measures: [
+              measure_entry(slug: "rps", name: "rps", value: 100.0,
+                            baseline: 100.0, lower_limit: 90.0, upper_limit: nil),
+              measure_entry(slug: "p50-latency", name: "p50_latency", value: 3.0,
+                            baseline: 5.0, lower_limit: nil, upper_limit: 6.0)
+            ]
+          )]],
+          alerts: [measureless_alert]
+        )
+      )
+
+      expect(report.regression?).to be(false)
+      expect(report.alerts).to be_empty
+    end
+
+    it "still reports hidden failed_pct regressions when the current value crosses its upper boundary" do
+      report = described_class.parse(
+        report_json(
+          results: [[benchmark_result(
+            name: "/foo: Core",
+            measures: [measure_entry(slug: "failed-pct", name: "failed_pct", value: 2.0,
+                                     baseline: 0.0, lower_limit: nil, upper_limit: 1.0)]
+          )]],
+          alerts: [alert(benchmark: "/foo: Core", measure_slug: "failed-pct", limit: "upper")]
+        )
+      )
+
+      expect(report.regression?).to be(true)
+      expect(report.alerts.first.measure).to eq("failed-pct")
+    end
+
     it "ignores dismissed/silenced alerts (only active counts as a regression)" do
       report = described_class.parse(
         report_json(alerts: [