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