From 3068c935c629977c80f0e93dc45f86a54470151b Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Mon, 8 Jun 2026 16:54:52 -1000 Subject: [PATCH] Name the regressed benchmark+measure pairs in the issue body MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Confirmed regression issues only showed the per-suite summary tables, whose 🔴 flag covers rps/p50 but never the column-less failed_pct — so a reader had to open the tables, and a failed_pct-only regression looked blank. Add a "What regressed" section to the issue body built from the confirmed ALERTS payload (benchmark + measure pairs), aggregated across all suites and computed once. Defaults to empty (body unchanged) when no structured pairs were handed off, so it composes with the existing flow and older payloads. Refs #3755 Co-Authored-By: Claude Opus 4.8 (1M context) --- benchmarks/report_regressions.rb | 52 ++++++++++++++++++++-- benchmarks/spec/report_regressions_spec.rb | 37 +++++++++++++++ 2 files changed, 85 insertions(+), 4 deletions(-) diff --git a/benchmarks/report_regressions.rb b/benchmarks/report_regressions.rb index 8165d12d25..d0123ce376 100755 --- a/benchmarks/report_regressions.rb +++ b/benchmarks/report_regressions.rb @@ -40,10 +40,12 @@ def self.report(summary:, **attributes) new(**attributes).report(summary) end - def initialize(suite_name:, github_run_url:, bencher_url:, issue_number_cache: nil, report_comment_id_cache: nil) + def initialize(suite_name:, github_run_url:, bencher_url:, regressed_overview: "", + issue_number_cache: nil, report_comment_id_cache: nil) @suite_name = suite_name @github_run_url = github_run_url @bencher_url = bencher_url + @regressed_overview = regressed_overview @issue_number_cache = issue_number_cache @report_comment_id_cache = report_comment_id_cache @commit_short = ENV.fetch("GITHUB_SHA")[0, 7] @@ -63,7 +65,8 @@ def report(summary) private - attr_reader :suite_name, :github_run_url, :bencher_url, :issue_number_cache, :report_comment_id_cache, :commit_short + attr_reader :suite_name, :github_run_url, :bencher_url, :regressed_overview, :issue_number_cache, + :report_comment_id_cache, :commit_short def ensure_regression_label GithubCli.run( @@ -321,7 +324,7 @@ def issue_body | **Pushed by** | @#{github_actor} | | **Workflow run** | [Run ##{github_run_number}](#{github_run_url}) | | **Bencher dashboard** | [View history](#{bencher_url}) | - + #{regressed_section} ### What to do 1. Check the workflow run for the full Bencher HTML report @@ -335,6 +338,24 @@ def issue_body MARKDOWN end + # A "What regressed" section naming the confirmed benchmark+measure pairs, so the issue + # body itself says what regressed (the per-suite tables flag only rps/p50, never the + # column-less failed_pct). Empty when no structured pairs were handed off, leaving the + # body unchanged. + def regressed_section + return "" if regressed_overview.to_s.strip.empty? + + <<~MARKDOWN + + ### What regressed + + The fresh-runner confirmation re-alerted on these benchmark + measure pairs: + + #{regressed_overview} + + MARKDOWN + end + def github_actor @github_actor ||= ENV.fetch("GITHUB_ACTOR") end @@ -400,6 +421,23 @@ def shard_summary(payload) MARKDOWN end +# A flat, deduped list of the confirmed benchmark+measure pairs across all suites, for the +# issue body — so a reader sees what regressed without opening the per-suite tables. Built +# from each payload's structured ALERTS ({benchmark, measure}); "" when no payload carried +# them (older writer, or alerts with no benchmark name), which leaves the body unchanged. +def regressed_overview_markdown(payloads) + payloads + .flat_map { |payload| payload[RegressionReport::ALERTS] || [] } + .select { |pair| pair.is_a?(Hash) && pair[RegressionReport::ALERT_BENCHMARK] } + .uniq + .map do |pair| + benchmark = pair[RegressionReport::ALERT_BENCHMARK] + measure = pair[RegressionReport::ALERT_MEASURE] + measure ? "- `#{benchmark}` — **#{measure}**" : "- `#{benchmark}`" + end + .join("\n") +end + # Reports the confirmed regressions. Returns: # :clean — no confirmed payloads (every first-run alert cleared as noise / ignored) # :filed — at least one confirmed regression filed/updated successfully @@ -418,12 +456,17 @@ def report_regressions(artifacts_dir) # One section per suite: a sharded suite emits one payload per shard, so combine # them rather than filing a section per shard. Suites sorted for stable output. by_suite = readable.group_by { |payload| payload.fetch(RegressionReport::SUITE_NAME) } + # Name every confirmed benchmark+measure pair across all suites in the issue body so it is + # actionable on its own. Computed once and passed to each suite's reporter; only the + # reporter that creates the issue renders the body. + regressed_overview = regressed_overview_markdown(readable) issue_number_cache = {} report_comment_id_cache = {} reported_ok = by_suite.keys.sort.map do |suite_name| report_suite( suite_name, by_suite.fetch(suite_name), + regressed_overview:, issue_number_cache:, report_comment_id_cache: ) @@ -435,7 +478,7 @@ def report_regressions(artifacts_dir) :filed end -def report_suite(suite_name, payloads, issue_number_cache: nil, report_comment_id_cache: nil) +def report_suite(suite_name, payloads, regressed_overview: "", issue_number_cache: nil, report_comment_id_cache: nil) # Order shard summaries by shard number ("2/5" before "10/5"); each already # self-labels with its shard in its headers, so concatenation reads cleanly. summary = payloads @@ -449,6 +492,7 @@ def report_suite(suite_name, payloads, issue_number_cache: nil, report_comment_i github_run_url: Github.run_url, bencher_url: BENCHER_URL, summary:, + regressed_overview:, issue_number_cache:, report_comment_id_cache: ) diff --git a/benchmarks/spec/report_regressions_spec.rb b/benchmarks/spec/report_regressions_spec.rb index 8bed225ebf..123aef66e2 100644 --- a/benchmarks/spec/report_regressions_spec.rb +++ b/benchmarks/spec/report_regressions_spec.rb @@ -117,6 +117,24 @@ def report_with_cache(suite_name, issue_number_cache, report_comment_id_cache = expect(calls).to eq(["label create", "issue list", "issue create", "comment list", "comment create"]) end + it "names the confirmed benchmark+measure pairs in the created issue body" do + described_class.report( + summary: "core regressed", + suite_name: "core", + github_run_url: "https://github.com/run/1", + bencher_url: "https://bencher.dev/dash", + regressed_overview: "- `/x: Core` — **rps**" + ) + body = posted_bodies.fetch("issue create") + expect(body).to include("### What regressed") + expect(body).to include("- `/x: Core` — **rps**") + end + + it "omits the what-regressed section when no pairs were handed off" do + report + expect(posted_bodies.fetch("issue create")).not_to include("What regressed") + end + it "reuses a just-created issue number for later suites in the same reporter run" do issue_number_cache = {} @@ -274,6 +292,25 @@ def section_for(suite, content = "") # first-run/confirmation evidence rendering, and the exit status. A confirmed # regression fails the workflow (exit 1) even though the issue was filed successfully — # this job owns the final pass/fail. + describe "regressed_overview_markdown" do + it "renders deduped benchmark+measure bullets from each payload's ALERTS" do + payloads = [ + { RegressionReport::ALERTS => [ + { RegressionReport::ALERT_BENCHMARK => "/a: Core", RegressionReport::ALERT_MEASURE => "rps" }, + { RegressionReport::ALERT_BENCHMARK => "/a: Core", RegressionReport::ALERT_MEASURE => "rps" } + ] }, + { RegressionReport::ALERTS => [ + { RegressionReport::ALERT_BENCHMARK => "/b: Pro", RegressionReport::ALERT_MEASURE => nil } + ] } + ] + expect(regressed_overview_markdown(payloads)).to eq("- `/a: Core` — **rps**\n- `/b: Pro`") + end + + it "is empty when no payload carries structured alert pairs" do + expect(regressed_overview_markdown([{ RegressionReport::SUITE_NAME => "Core" }])).to eq("") + end + end + describe "report_regressions.rb (script)" do script = File.expand_path("../report_regressions.rb", __dir__)