From 7350663364981d524a7c09514627df4e8f543820 Mon Sep 17 00:00:00 2001
From: Andrew Duthie <andrew.duthie@gsa.gov>
Date: Mon, 3 Feb 2025 12:16:42 -0500
Subject: [PATCH 01/16] LG-15559: Add support for A/B tests daily reporting

changelog: Internal, A/B Tests, Add support for A/B tests daily reporting
---
 app/jobs/reports/ab_tests_report.rb           | 54 +++++++++++
 config/initializers/job_configurations.rb     |  6 ++
 lib/ab_test.rb                                | 15 +--
 lib/reporting/ab_tests_report.rb              | 95 +++++++++++++++++++
 spec/jobs/reports/ab_tests_report_spec.rb     | 94 ++++++++++++++++++
 spec/lib/ab_test_spec.rb                      | 41 +++++++-
 spec/lib/reporting/ab_tests_report_spec.rb    | 93 ++++++++++++++++++
 .../mailers/previews/report_mailer_preview.rb | 47 ++++++++-
 8 files changed, 433 insertions(+), 12 deletions(-)
 create mode 100644 app/jobs/reports/ab_tests_report.rb
 create mode 100644 lib/reporting/ab_tests_report.rb
 create mode 100644 spec/jobs/reports/ab_tests_report_spec.rb
 create mode 100644 spec/lib/reporting/ab_tests_report_spec.rb

diff --git a/app/jobs/reports/ab_tests_report.rb b/app/jobs/reports/ab_tests_report.rb
new file mode 100644
index 00000000000..7661a7f21ea
--- /dev/null
+++ b/app/jobs/reports/ab_tests_report.rb
@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+require 'reporting/ab_tests_report'
+
+module Reports
+  class AbTestsReport < BaseReport
+    attr_reader :report_date
+
+    def initialize(report_date = nil, *args, **kwargs)
+      @report_date = report_date
+      super(*args, **kwargs)
+    end
+
+    # @param [DateTime]
+    def perform(report_date)
+      @report_date = report_date
+
+      report_configs.each do |config|
+        tables_report(config).deliver_now
+      end
+    end
+
+    def tables_report(config)
+      experiment_name = config[:experiment_name]
+      subject = "A/B Tests Report - #{experiment_name} - #{report_date}"
+      reports = ab_tests_report(config).as_emailable_reports
+
+      ReportMailer.tables_report(
+        email: config[:email],
+        subject:,
+        message: subject,
+        reports:,
+        attachment_format: :csv,
+      )
+    end
+
+    def ab_tests_report(config)
+      Reporting::AbTestsReport.new(
+        queries: config[:queries],
+        time_range: report_date.yesterday..report_date,
+      )
+    end
+
+    private
+
+    def report_configs
+      AbTests
+        .all
+        .values
+        .select { |ab_test| ab_test.report&.[](:email)&.present? && ab_test.active? }
+        .map { |ab_test| ab_test.report.merge(experiment_name: ab_test.experiment_name) }
+    end
+  end
+end
diff --git a/config/initializers/job_configurations.rb b/config/initializers/job_configurations.rb
index 0f3bb7adf49..e8cb3ce8563 100644
--- a/config/initializers/job_configurations.rb
+++ b/config/initializers/job_configurations.rb
@@ -236,6 +236,12 @@
         cron: cron_every_monday,
         args: -> { [Time.zone.yesterday.end_of_day] },
       },
+      # Send A/B test reports
+      ab_tests_report: {
+        class: 'Reports::AbTestsReport',
+        cron: cron_24h,
+        args: -> { [Time.zone.yesterday.end_of_day] },
+      },
       # Send fraud metrics to Team Judy
       fraud_metrics_report: {
         class: 'Reports::FraudMetricsReport',
diff --git a/lib/ab_test.rb b/lib/ab_test.rb
index 1d7be784b49..4ed1a24a510 100644
--- a/lib/ab_test.rb
+++ b/lib/ab_test.rb
@@ -3,7 +3,7 @@
 class AbTest
   include ::NewRelic::Agent::MethodTracer
 
-  attr_reader :buckets, :experiment_name, :default_bucket, :should_log
+  attr_reader :buckets, :experiment_name, :default_bucket, :should_log, :report
 
   MAX_SHA = (16 ** 64) - 1
 
@@ -20,6 +20,7 @@ def initialize(
     buckets: {},
     should_log: nil,
     default_bucket: :default,
+    report: nil,
     &discriminator
   )
     @buckets = buckets
@@ -27,6 +28,7 @@ def initialize(
     @experiment_name = experiment_name
     @default_bucket = default_bucket
     @should_log = should_log
+    @report = report
     raise 'invalid bucket data structure' unless valid_bucket_data_structure?
     ensure_numeric_percentages
     raise 'bucket percentages exceed 100' unless within_100_percent?
@@ -39,7 +41,7 @@ def initialize(
   # @param [User] user
   # @param [Hash] user_session
   def bucket(request:, service_provider:, session:, user:, user_session:)
-    return nil if no_percentages?
+    return nil if !active?
 
     discriminator = resolve_discriminator(
       request:, service_provider:, session:, user:,
@@ -71,6 +73,11 @@ def include_in_analytics_event?(event_name)
     end
   end
 
+  def active?
+    return @active if defined?(@active)
+    @active = buckets.present? && buckets.values.any?(&:positive?)
+  end
+
   private
 
   def resolve_discriminator(user:, **)
@@ -81,10 +88,6 @@ def resolve_discriminator(user:, **)
     end
   end
 
-  def no_percentages?
-    buckets.empty? || buckets.values.all? { |pct| pct == 0 }
-  end
-
   def percent(discriminator)
     Digest::SHA256.hexdigest("#{discriminator}:#{experiment_name}").to_i(16).to_f / MAX_SHA * 100
   end
diff --git a/lib/reporting/ab_tests_report.rb b/lib/reporting/ab_tests_report.rb
new file mode 100644
index 00000000000..36146c635a3
--- /dev/null
+++ b/lib/reporting/ab_tests_report.rb
@@ -0,0 +1,95 @@
+# frozen_string_literal: true
+
+begin
+  require 'reporting/cloudwatch_client'
+  require 'reporting/cloudwatch_query_quoting'
+  require 'reporting/command_line_options'
+rescue LoadError => e
+  warn 'could not load paths, try running with "bundle exec rails runner"'
+  raise e
+end
+
+module Reporting
+  class AbTestsReport
+    attr_reader :queries, :time_range
+
+    # @param [Array<String>] queries
+    # @param [Range<Time>] time_range
+    def initialize(
+      queries:,
+      time_range:,
+      verbose: false
+    )
+      @queries = queries
+      @time_range = time_range
+      @verbose = verbose
+    end
+
+    def verbose?
+      @verbose
+    end
+
+    def as_tables
+      queries.map(&method(:table_for_query))
+    end
+
+    def as_emailable_reports
+      queries.map do |query|
+        Reporting::EmailableReport.new(
+          title: query[:title],
+          table: table_for_query(query),
+        )
+      end
+    end
+
+    def table_for_query(query)
+      query_data = fetch_results(query: query[:query])
+      headers = column_labels(query_data.first)
+      rows = query_data.map(&:values)
+
+      [
+        headers,
+        *format_rows(rows, headers, query[:row_labels]),
+      ]
+    end
+
+    def format_rows(rows, headers, row_labels)
+      if row_labels
+        rows = rows.each_with_index.map do |value, index|
+          [row_labels[index], *value.slice(1)]
+        end
+      end
+
+      headers.each_index.select { |i| headers[i] =~ /percent/i }.each do |percent_index|
+        rows.each { |row| row[percent_index] = format_as_percent(row[percent_index]) }
+      end
+
+      rows
+    end
+
+    def fetch_results(query:)
+      cloudwatch_client.fetch(query:, from: time_range.begin, to: time_range.end)
+    end
+
+    def column_labels(row)
+      row.keys
+    end
+
+    def cloudwatch_client
+      @cloudwatch_client ||= Reporting::CloudwatchClient.new(
+        progress: false,
+        ensure_complete_logs: false,
+        logger:,
+      )
+    end
+
+    def logger
+      Logger.new(STDERR) if verbose?
+    end
+
+    # @return [String]
+    def format_as_percent(number)
+      '%.2f%%' % number
+    end
+  end
+end
diff --git a/spec/jobs/reports/ab_tests_report_spec.rb b/spec/jobs/reports/ab_tests_report_spec.rb
new file mode 100644
index 00000000000..caf3938cf34
--- /dev/null
+++ b/spec/jobs/reports/ab_tests_report_spec.rb
@@ -0,0 +1,94 @@
+require 'rails_helper'
+
+RSpec.describe Reports::AbTestsReport do
+  let(:report_date) { Date.new(2023, 12, 25) }
+  let(:email) { 'email@example.com' }
+  let(:tested_percent) { 1 }
+  let(:queries) do
+    [
+      {
+        title: 'Sign in success rate by CAPTCHA validation performed',
+        query: <<~QUERY,
+          fields properties.event_properties.captcha_validation_performed as `Captcha Validation Performed`
+          | filter name = 'Email and Password Authentication'
+          | stats avg(properties.event_properties.success)*100 as `Success Percent` by `Captcha Validation Performed`
+          | sort `Captcha Validation Performed` asc
+        QUERY
+        row_labels: ['Validation Not Performed', 'Validation Performed'],
+      },
+    ]
+  end
+  let(:ab_tests) do
+    {
+      RECAPTCHA_SIGN_IN: AbTest.new(
+        experiment_name: 'reCAPTCHA at Sign-In',
+        buckets: { sign_in_recaptcha: tested_percent },
+        report: { email:, queries: },
+      ),
+    }
+  end
+
+  before do
+    allow(AbTests).to receive(:all).and_return(ab_tests)
+  end
+
+  describe '#perform' do
+    let(:emailable_reports) do
+      [
+        Reporting::EmailableReport.new(
+          title: 'Sign in success rate by CAPTCHA validation performed',
+          table: [
+            ['Captcha Validation Performed', 'Success Percent'],
+            ['Validation Not Performed', '90.19%'],
+            ['Validation Performed', '85.68%'],
+          ],
+        ),
+      ]
+    end
+
+    let(:report) do
+      double(Reporting::AbTestsReport, as_emailable_reports: emailable_reports)
+    end
+
+    before do
+      allow(Reporting::AbTestsReport).to receive(:new).with(
+        queries:,
+        time_range: report_date.yesterday..report_date,
+      ).and_return(report)
+
+      allow(ReportMailer).to receive(:tables_report).and_call_original
+    end
+
+    it 'emails the table report with csv' do
+      expect(ReportMailer).to receive(:tables_report).with(
+        email:,
+        subject: "A/B Tests Report - reCAPTCHA at Sign-In - #{report_date}",
+        message: "A/B Tests Report - reCAPTCHA at Sign-In - #{report_date}",
+        reports: emailable_reports,
+        attachment_format: :csv,
+      )
+
+      subject.perform(report_date)
+    end
+
+    context 'when associated report email is nil' do
+      let(:email) { nil }
+
+      it 'does not email the table report' do
+        expect(ReportMailer).not_to receive(:tables_report)
+
+        subject.perform(report_date)
+      end
+    end
+
+    context 'when a/b test buckets are zero (test is inactive)' do
+      let(:tested_percent) { 0 }
+
+      it 'does not email the table report' do
+        expect(ReportMailer).not_to receive(:tables_report)
+
+        subject.perform(report_date)
+      end
+    end
+  end
+end
diff --git a/spec/lib/ab_test_spec.rb b/spec/lib/ab_test_spec.rb
index 9e147fe574c..5cb3355caf6 100644
--- a/spec/lib/ab_test_spec.rb
+++ b/spec/lib/ab_test_spec.rb
@@ -2,12 +2,15 @@
 
 RSpec.describe AbTest do
   subject(:ab_test) do
-    AbTest.new(
+    AbTest.new(**options, &discriminator)
+  end
+
+  let(:options) do
+    {
       experiment_name: 'test',
       buckets:,
       should_log:,
-      &discriminator
-    )
+    }
   end
 
   let(:discriminator) do
@@ -228,4 +231,36 @@ def include?(event_name)
       end
     end
   end
+
+  describe '#report' do
+    subject(:report) { ab_test.report }
+    let(:options) { super().merge(report: report_option) }
+    let(:report_option) { {} }
+
+    it 'reflects value provided from initializer' do
+      expect(report).to eq(report_option)
+    end
+  end
+
+  describe '#active?' do
+    subject(:active) { ab_test.active? }
+
+    context 'with non-zero buckets' do
+      let(:buckets) { { foo: 0, bar: 30, baz: 0 } }
+
+      it { is_expected.to be true }
+    end
+
+    context 'with all zero buckets' do
+      let(:buckets) { { foo: 0, bar: 0, baz: 0 } }
+
+      it { is_expected.to be false }
+    end
+
+    context 'with empty buckets' do
+      let(:buckets) { {} }
+
+      it { is_expected.to be false }
+    end
+  end
 end
diff --git a/spec/lib/reporting/ab_tests_report_spec.rb b/spec/lib/reporting/ab_tests_report_spec.rb
new file mode 100644
index 00000000000..0079c159724
--- /dev/null
+++ b/spec/lib/reporting/ab_tests_report_spec.rb
@@ -0,0 +1,93 @@
+require 'rails_helper'
+require 'reporting/ab_tests_report'
+
+RSpec.describe Reporting::AbTestsReport do
+  let(:time_range) { Date.new(2025, 1, 1).all_day }
+  let(:options) { {} }
+
+  subject(:report) do
+    Reporting::AbTestsReport.new(
+      queries: [
+        {
+          title: 'Sign in success rate by CAPTCHA validation performed',
+          query: <<~QUERY,
+            fields properties.event_properties.captcha_validation_performed as `Captcha Validation Performed`
+            | filter name = 'Email and Password Authentication'
+            | stats avg(properties.event_properties.success)*100 as `Success Percent` by `Captcha Validation Performed`
+            | sort `Captcha Validation Performed` asc
+          QUERY
+          row_labels: ['Validation Not Performed', 'Validation Performed'],
+        },
+      ],
+      time_range:,
+      **options,
+    )
+  end
+
+  before do
+    stub_cloudwatch_logs(
+      [
+        { 'Captcha Validation Performed' => '0', 'Success Percent' => '90.18501' },
+        { 'Captcha Validation Performed' => '1', 'Success Percent' => '85.68103' },
+      ],
+    )
+  end
+
+  describe '#as_tables' do
+    subject(:tables) { report.as_tables }
+
+    it 'generates the tabular data' do
+      expect(tables).to eq(
+        [
+          [
+            ['Captcha Validation Performed', 'Success Percent'],
+            ['Validation Not Performed', '90.19%'],
+            ['Validation Performed', '85.68%'],
+          ],
+        ],
+      )
+    end
+  end
+
+  describe '#as_emailable_reports' do
+    subject(:emailable_reports) { report.as_emailable_reports }
+
+    it 'adds a "first row" hash with a title for tables_report mailer' do
+      aggregate_failures do
+        emailable_reports.each do |emailable_report|
+          expect(emailable_report.title).to be_present
+        end
+      end
+    end
+  end
+
+  describe '#cloudwatch_client' do
+    subject(:cloudwatch_client) { report.cloudwatch_client }
+    let(:default_args) do
+      {
+        ensure_complete_logs: false,
+        logger: nil,
+      }
+    end
+
+    describe 'when all args are default' do
+      it 'creates a client with the default options' do
+        expect(Reporting::CloudwatchClient).to receive(:new).with(default_args)
+
+        cloudwatch_client
+      end
+    end
+
+    describe 'when verbose is passed in' do
+      let(:options) { { verbose: true } }
+      let(:logger) { double(Logger) }
+
+      it 'creates a client with the expected logger' do
+        expect(Logger).to receive(:new).with(STDERR).and_return(logger)
+        expect(Reporting::CloudwatchClient).to receive(:new).with({ **default_args, logger: })
+
+        cloudwatch_client
+      end
+    end
+  end
+end
diff --git a/spec/mailers/previews/report_mailer_preview.rb b/spec/mailers/previews/report_mailer_preview.rb
index 7c86f0cc165..665219d11cf 100644
--- a/spec/mailers/previews/report_mailer_preview.rb
+++ b/spec/mailers/previews/report_mailer_preview.rb
@@ -21,6 +21,39 @@ def warn_error
     )
   end
 
+  def ab_tests_report
+    report = Reports::AbTestsReport.new(Time.zone.now).ab_tests_report(
+      queries: [
+        {
+          title: 'Sign in success rate by CAPTCHA validation performed',
+          query: <<~QUERY,
+            fields properties.event_properties.captcha_validation_performed as `Captcha Validation Performed`
+            | filter name = 'Email and Password Authentication'
+            | stats avg(properties.event_properties.success)*100 as `Success Percent` by `Captcha Validation Performed`
+            | sort `Captcha Validation Performed` asc
+          QUERY
+          row_labels: ['Validation Not Performed', 'Validation Performed'],
+        },
+      ],
+    )
+
+    stub_cloudwatch_client(
+      report,
+      data: [
+        { 'Captcha Validation Performed' => '0', 'Success Percent' => '90.18501' },
+        { 'Captcha Validation Performed' => '1', 'Success Percent' => '85.68103' },
+      ],
+    )
+
+    ReportMailer.tables_report(
+      email: 'email@example.com',
+      subject: "A/B Tests Report - reCAPTCHA at Sign-In - #{Time.zone.now.to_date}",
+      message: "A/B Tests Report - reCAPTCHA at Sign-In - #{Time.zone.now.to_date}",
+      reports: report.as_emailable_reports,
+      attachment_format: :csv,
+    )
+  end
+
   def monthly_key_metrics_report
     monthly_key_metrics_report = Reports::MonthlyKeyMetricsReport.new(Time.zone.yesterday)
 
@@ -102,16 +135,24 @@ def tables_report
   private
 
   class FakeCloudwatchClient
+    def initialize(data:)
+      @data = data
+    end
+
     def fetch(**)
-      []
+      @data
     end
   end
 
-  def stub_cloudwatch_client(report)
+  def stub_cloudwatch_client(report, data: [])
     class << report
+      attr_accessor :data
+
       def cloudwatch_client
-        FakeCloudwatchClient.new
+        FakeCloudwatchClient.new(data: @data)
       end
     end
+    report.data = data
+    report
   end
 end

From 3f87e453dbfc8d5a73387fb993c4051878c54596 Mon Sep 17 00:00:00 2001
From: Andrew Duthie <andrew.duthie@gsa.gov>
Date: Tue, 4 Feb 2025 08:37:24 -0500
Subject: [PATCH 02/16] Add expected progress argument value

---
 spec/lib/reporting/ab_tests_report_spec.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/spec/lib/reporting/ab_tests_report_spec.rb b/spec/lib/reporting/ab_tests_report_spec.rb
index 0079c159724..4f827dad186 100644
--- a/spec/lib/reporting/ab_tests_report_spec.rb
+++ b/spec/lib/reporting/ab_tests_report_spec.rb
@@ -67,6 +67,7 @@
       {
         ensure_complete_logs: false,
         logger: nil,
+        progress: false,
       }
     end
 

From 4c10d2db73bbebce52dbcfa59780a1fbaa975a48 Mon Sep 17 00:00:00 2001
From: Andrew Duthie <andrew.duthie@gsa.gov>
Date: Tue, 4 Feb 2025 08:39:12 -0500
Subject: [PATCH 03/16] Revise A/B test report options to struct

See: https://github.com/18F/identity-idp/pull/11835#discussion_r1939785484
---
 app/jobs/reports/ab_tests_report.rb        | 10 ++++-----
 lib/ab_test.rb                             | 11 +++++++++-
 lib/reporting/ab_tests_report.rb           |  6 +++---
 spec/lib/ab_test_spec.rb                   | 24 +++++++++++++++++++---
 spec/lib/reporting/ab_tests_report_spec.rb |  4 ++--
 5 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/app/jobs/reports/ab_tests_report.rb b/app/jobs/reports/ab_tests_report.rb
index 7661a7f21ea..e96618e72cf 100644
--- a/app/jobs/reports/ab_tests_report.rb
+++ b/app/jobs/reports/ab_tests_report.rb
@@ -21,12 +21,12 @@ def perform(report_date)
     end
 
     def tables_report(config)
-      experiment_name = config[:experiment_name]
+      experiment_name = config.experiment_name
       subject = "A/B Tests Report - #{experiment_name} - #{report_date}"
       reports = ab_tests_report(config).as_emailable_reports
 
       ReportMailer.tables_report(
-        email: config[:email],
+        email: config.email,
         subject:,
         message: subject,
         reports:,
@@ -36,7 +36,7 @@ def tables_report(config)
 
     def ab_tests_report(config)
       Reporting::AbTestsReport.new(
-        queries: config[:queries],
+        queries: config.queries,
         time_range: report_date.yesterday..report_date,
       )
     end
@@ -47,8 +47,8 @@ def report_configs
       AbTests
         .all
         .values
-        .select { |ab_test| ab_test.report&.[](:email)&.present? && ab_test.active? }
-        .map { |ab_test| ab_test.report.merge(experiment_name: ab_test.experiment_name) }
+        .select { |ab_test| ab_test.report&.email&.present? && ab_test.active? }
+        .map(&:report)
     end
   end
 end
diff --git a/lib/ab_test.rb b/lib/ab_test.rb
index 4ed1a24a510..6c64cd106a6 100644
--- a/lib/ab_test.rb
+++ b/lib/ab_test.rb
@@ -7,6 +7,15 @@ class AbTest
 
   MAX_SHA = (16 ** 64) - 1
 
+  ReportQueryConfig = Struct.new(:title, :query, :row_labels, keyword_init: true).freeze
+
+  ReportConfig = Struct.new(:experiment_name, :email, :queries, keyword_init: true) do
+    def initialize(queries: [], **)
+      super
+      self.queries.map!(&ReportQueryConfig.method(:new))
+    end
+  end.freeze
+
   # @param [Proc<String>,Regexp,string,Boolean,nil] should_log Controls whether bucket data for this
   #                                                            A/B test is logged with specific
   #                                                            events.
@@ -28,7 +37,7 @@ def initialize(
     @experiment_name = experiment_name
     @default_bucket = default_bucket
     @should_log = should_log
-    @report = report
+    @report = ReportConfig.new(experiment_name:, **report.to_h)
     raise 'invalid bucket data structure' unless valid_bucket_data_structure?
     ensure_numeric_percentages
     raise 'bucket percentages exceed 100' unless within_100_percent?
diff --git a/lib/reporting/ab_tests_report.rb b/lib/reporting/ab_tests_report.rb
index 36146c635a3..c2e9b4abf55 100644
--- a/lib/reporting/ab_tests_report.rb
+++ b/lib/reporting/ab_tests_report.rb
@@ -36,20 +36,20 @@ def as_tables
     def as_emailable_reports
       queries.map do |query|
         Reporting::EmailableReport.new(
-          title: query[:title],
+          title: query.title,
           table: table_for_query(query),
         )
       end
     end
 
     def table_for_query(query)
-      query_data = fetch_results(query: query[:query])
+      query_data = fetch_results(query: query.query)
       headers = column_labels(query_data.first)
       rows = query_data.map(&:values)
 
       [
         headers,
-        *format_rows(rows, headers, query[:row_labels]),
+        *format_rows(rows, headers, query.row_labels),
       ]
     end
 
diff --git a/spec/lib/ab_test_spec.rb b/spec/lib/ab_test_spec.rb
index 5cb3355caf6..c12e240c851 100644
--- a/spec/lib/ab_test_spec.rb
+++ b/spec/lib/ab_test_spec.rb
@@ -235,10 +235,28 @@ def include?(event_name)
   describe '#report' do
     subject(:report) { ab_test.report }
     let(:options) { super().merge(report: report_option) }
-    let(:report_option) { {} }
+    let(:report_option) do
+      { email: 'email@example.com', queries: [{ title: 'Example Query', query: 'limit 1' }] }
+    end
+
+    it 'builds struct value from given hash option' do
+      expect(report).to be_a(AbTest::ReportConfig)
+      expect(report.experiment_name).to eq('test')
+      expect(report.email).to eq('email@example.com')
+      expect(report.queries).to all be_a(AbTest::ReportQueryConfig)
+      expect(report.queries.first.title).to eq('Example Query')
+      expect(report.queries.first.query).to eq('limit 1')
+    end
 
-    it 'reflects value provided from initializer' do
-      expect(report).to eq(report_option)
+    context 'with blank options' do
+      let(:report_option) { {} }
+
+      it 'gracefully builds an empty struct value' do
+        expect(report).to be_a(AbTest::ReportConfig)
+        expect(report.experiment_name).to eq('test')
+        expect(report.email).to be_nil
+        expect(report.queries).to eq([])
+      end
     end
   end
 
diff --git a/spec/lib/reporting/ab_tests_report_spec.rb b/spec/lib/reporting/ab_tests_report_spec.rb
index 4f827dad186..810af2afff4 100644
--- a/spec/lib/reporting/ab_tests_report_spec.rb
+++ b/spec/lib/reporting/ab_tests_report_spec.rb
@@ -8,7 +8,7 @@
   subject(:report) do
     Reporting::AbTestsReport.new(
       queries: [
-        {
+        AbTest::ReportQueryConfig.new(
           title: 'Sign in success rate by CAPTCHA validation performed',
           query: <<~QUERY,
             fields properties.event_properties.captcha_validation_performed as `Captcha Validation Performed`
@@ -17,7 +17,7 @@
             | sort `Captcha Validation Performed` asc
           QUERY
           row_labels: ['Validation Not Performed', 'Validation Performed'],
-        },
+        ),
       ],
       time_range:,
       **options,

From da8f1db467ccdf806a131d032de753cbda4ef08d Mon Sep 17 00:00:00 2001
From: Andrew Duthie <andrew.duthie@gsa.gov>
Date: Tue, 4 Feb 2025 08:52:45 -0500
Subject: [PATCH 04/16] Return `nil` for report if option not passed

---
 lib/ab_test.rb           | 2 +-
 spec/lib/ab_test_spec.rb | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/ab_test.rb b/lib/ab_test.rb
index 6c64cd106a6..ac393f9acfd 100644
--- a/lib/ab_test.rb
+++ b/lib/ab_test.rb
@@ -37,7 +37,7 @@ def initialize(
     @experiment_name = experiment_name
     @default_bucket = default_bucket
     @should_log = should_log
-    @report = ReportConfig.new(experiment_name:, **report.to_h)
+    @report = ReportConfig.new(experiment_name:, **report.to_h) if report
     raise 'invalid bucket data structure' unless valid_bucket_data_structure?
     ensure_numeric_percentages
     raise 'bucket percentages exceed 100' unless within_100_percent?
diff --git a/spec/lib/ab_test_spec.rb b/spec/lib/ab_test_spec.rb
index c12e240c851..06b08a77ab6 100644
--- a/spec/lib/ab_test_spec.rb
+++ b/spec/lib/ab_test_spec.rb
@@ -248,6 +248,12 @@ def include?(event_name)
       expect(report.queries.first.query).to eq('limit 1')
     end
 
+    context 'with nil report option' do
+      let(:report_option) { nil }
+
+      it { is_expected.to be_nil }
+    end
+
     context 'with blank options' do
       let(:report_option) { {} }
 

From 1333069cff9762aaf1c84a37201bb5de74b94aa2 Mon Sep 17 00:00:00 2001
From: Andrew Duthie <andrew.duthie@gsa.gov>
Date: Tue, 4 Feb 2025 09:26:14 -0500
Subject: [PATCH 05/16] Memoize active value without modifying frozen instance

---
 lib/ab_test.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/ab_test.rb b/lib/ab_test.rb
index ac393f9acfd..3e7a7a84326 100644
--- a/lib/ab_test.rb
+++ b/lib/ab_test.rb
@@ -41,6 +41,7 @@ def initialize(
     raise 'invalid bucket data structure' unless valid_bucket_data_structure?
     ensure_numeric_percentages
     raise 'bucket percentages exceed 100' unless within_100_percent?
+    @active = buckets.present? && buckets.values.any?(&:positive?)
   end
 
   # @param [ActionDispatch::Request] request
@@ -83,8 +84,7 @@ def include_in_analytics_event?(event_name)
   end
 
   def active?
-    return @active if defined?(@active)
-    @active = buckets.present? && buckets.values.any?(&:positive?)
+    @active
   end
 
   private

From 4653c8831f0784491053f9d710a110599250a013 Mon Sep 17 00:00:00 2001
From: Andrew Duthie <andrew.duthie@gsa.gov>
Date: Tue, 4 Feb 2025 10:16:57 -0500
Subject: [PATCH 06/16] Update hash option to struct value

---
 .../mailers/previews/report_mailer_preview.rb | 26 ++++++++++---------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/spec/mailers/previews/report_mailer_preview.rb b/spec/mailers/previews/report_mailer_preview.rb
index 665219d11cf..85780837ab5 100644
--- a/spec/mailers/previews/report_mailer_preview.rb
+++ b/spec/mailers/previews/report_mailer_preview.rb
@@ -23,18 +23,20 @@ def warn_error
 
   def ab_tests_report
     report = Reports::AbTestsReport.new(Time.zone.now).ab_tests_report(
-      queries: [
-        {
-          title: 'Sign in success rate by CAPTCHA validation performed',
-          query: <<~QUERY,
-            fields properties.event_properties.captcha_validation_performed as `Captcha Validation Performed`
-            | filter name = 'Email and Password Authentication'
-            | stats avg(properties.event_properties.success)*100 as `Success Percent` by `Captcha Validation Performed`
-            | sort `Captcha Validation Performed` asc
-          QUERY
-          row_labels: ['Validation Not Performed', 'Validation Performed'],
-        },
-      ],
+      AbTest::ReportConfig.new(
+        queries: [
+          {
+            title: 'Sign in success rate by CAPTCHA validation performed',
+            query: <<~QUERY,
+              fields properties.event_properties.captcha_validation_performed as `Captcha Validation Performed`
+              | filter name = 'Email and Password Authentication'
+              | stats avg(properties.event_properties.success)*100 as `Success Percent` by `Captcha Validation Performed`
+              | sort `Captcha Validation Performed` asc
+            QUERY
+            row_labels: ['Validation Not Performed', 'Validation Performed'],
+          },
+        ],
+      ),
     )
 
     stub_cloudwatch_client(

From 93f1a7dae087018f95c566fdfaf587783b8c690f Mon Sep 17 00:00:00 2001
From: Andrew Duthie <andrew.duthie@gsa.gov>
Date: Tue, 4 Feb 2025 11:54:25 -0500
Subject: [PATCH 07/16] Avoid naming conflicts with existing report class for
 stubbed data

---
 spec/mailers/previews/report_mailer_preview.rb | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/spec/mailers/previews/report_mailer_preview.rb b/spec/mailers/previews/report_mailer_preview.rb
index 85780837ab5..e99fea4edda 100644
--- a/spec/mailers/previews/report_mailer_preview.rb
+++ b/spec/mailers/previews/report_mailer_preview.rb
@@ -148,13 +148,13 @@ def fetch(**)
 
   def stub_cloudwatch_client(report, data: [])
     class << report
-      attr_accessor :data
+      attr_accessor :_stubbed_cloudwatch_data
 
       def cloudwatch_client
-        FakeCloudwatchClient.new(data: @data)
+        FakeCloudwatchClient.new(data: @_stubbed_cloudwatch_data)
       end
     end
-    report.data = data
+    report._stubbed_cloudwatch_data = data
     report
   end
 end

From fcd8b744905b2a60775dfbbb0f24eb17c08ffe03 Mon Sep 17 00:00:00 2001
From: Andrew Duthie <andrew.duthie@gsa.gov>
Date: Wed, 5 Feb 2025 09:46:18 -0500
Subject: [PATCH 08/16] Format float_as_percent if between 0 and 1

---
 app/views/report_mailer/tables_report.html.erb | 2 +-
 spec/mailers/report_mailer_spec.rb             | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/app/views/report_mailer/tables_report.html.erb b/app/views/report_mailer/tables_report.html.erb
index 403db2ce5fa..902e46430be 100644
--- a/app/views/report_mailer/tables_report.html.erb
+++ b/app/views/report_mailer/tables_report.html.erb
@@ -35,7 +35,7 @@
         <tr>
           <% row.each do |cell| %>
             <td <%= cell.is_a?(Numeric) ? 'class=table-number' : nil %> >
-              <% if cell.is_a?(Float) && report.float_as_percent? %>
+              <% if cell.is_a?(Float) && report.float_as_percent? && cell.between?(0.0, 1.0) %>
                 <%= number_to_percentage(cell * 100, precision: report.precision || 2) %>
               <% else %>
                 <%= number_with_delimiter(cell) %>
diff --git a/spec/mailers/report_mailer_spec.rb b/spec/mailers/report_mailer_spec.rb
index 414f62a9ec7..c0f1362c5cd 100644
--- a/spec/mailers/report_mailer_spec.rb
+++ b/spec/mailers/report_mailer_spec.rb
@@ -121,6 +121,10 @@
       expect(percent_cell.text.strip).to eq('50.00%')
       expect(percent_cell['class']).to eq('table-number')
 
+      percent_float_outside_range_cell = percent_table.at_css('tbody tr:nth-child(2) td:last-child')
+      expect(percent_float_outside_range_cell.text.strip).to eq('1.5')
+      expect(percent_float_outside_range_cell['class']).to eq('table-number')
+
       float_cell = float_table.at_css('tbody tr:nth-child(1) td:last-child')
       expect(float_cell.text.strip).to eq('1.0')
       expect(percent_cell['class']).to eq('table-number')

From cf7825dc71d31feea227061384abfe964869fc8f Mon Sep 17 00:00:00 2001
From: Andrew Duthie <andrew.duthie@gsa.gov>
Date: Wed, 5 Feb 2025 09:56:46 -0500
Subject: [PATCH 09/16] Handle Float::NAN when formatting float as percent

---
 app/views/report_mailer/tables_report.html.erb | 2 +-
 spec/mailers/report_mailer_spec.rb             | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/app/views/report_mailer/tables_report.html.erb b/app/views/report_mailer/tables_report.html.erb
index 902e46430be..d92d051c27b 100644
--- a/app/views/report_mailer/tables_report.html.erb
+++ b/app/views/report_mailer/tables_report.html.erb
@@ -35,7 +35,7 @@
         <tr>
           <% row.each do |cell| %>
             <td <%= cell.is_a?(Numeric) ? 'class=table-number' : nil %> >
-              <% if cell.is_a?(Float) && report.float_as_percent? && cell.between?(0.0, 1.0) %>
+              <% if cell.is_a?(Float) && report.float_as_percent? && cell.finite? && cell.between?(0.0, 1.0) %>
                 <%= number_to_percentage(cell * 100, precision: report.precision || 2) %>
               <% else %>
                 <%= number_with_delimiter(cell) %>
diff --git a/spec/mailers/report_mailer_spec.rb b/spec/mailers/report_mailer_spec.rb
index c0f1362c5cd..0f36fac9fa5 100644
--- a/spec/mailers/report_mailer_spec.rb
+++ b/spec/mailers/report_mailer_spec.rb
@@ -71,6 +71,7 @@
         ['Float', 'Int', 'Float'],
         ['Row 1', 1, 0.5],
         ['Row 2', 1, 1.5],
+        ['Row 2', 1, Float::NAN],
       ]
     end
 
@@ -125,6 +126,10 @@
       expect(percent_float_outside_range_cell.text.strip).to eq('1.5')
       expect(percent_float_outside_range_cell['class']).to eq('table-number')
 
+      percent_float_nan_cell = percent_table.at_css('tbody tr:nth-child(3) td:last-child')
+      expect(percent_float_nan_cell.text.strip).to eq('NaN')
+      expect(percent_float_nan_cell['class']).to eq('table-number')
+
       float_cell = float_table.at_css('tbody tr:nth-child(1) td:last-child')
       expect(float_cell.text.strip).to eq('1.0')
       expect(percent_cell['class']).to eq('table-number')

From 28160fbef1898d8b583a791fdb5b70267540940b Mon Sep 17 00:00:00 2001
From: Andrew Duthie <1779930+aduth@users.noreply.github.com>
Date: Wed, 5 Feb 2025 10:14:53 -0500
Subject: [PATCH 10/16] Use format method to format as percent

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
---
 lib/reporting/ab_tests_report.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/reporting/ab_tests_report.rb b/lib/reporting/ab_tests_report.rb
index c2e9b4abf55..aad020a4350 100644
--- a/lib/reporting/ab_tests_report.rb
+++ b/lib/reporting/ab_tests_report.rb
@@ -89,7 +89,7 @@ def logger
 
     # @return [String]
     def format_as_percent(number)
-      '%.2f%%' % number
+      format('%.2f%%', number)
     end
   end
 end

From 1da10186292b381508a0fc7d24603de6abecbc0d Mon Sep 17 00:00:00 2001
From: Andrew Duthie <1779930+aduth@users.noreply.github.com>
Date: Wed, 5 Feb 2025 10:21:14 -0500
Subject: [PATCH 11/16] Remove unused CLI supports

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
---
 lib/reporting/ab_tests_report.rb | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/lib/reporting/ab_tests_report.rb b/lib/reporting/ab_tests_report.rb
index aad020a4350..65ac366aaf8 100644
--- a/lib/reporting/ab_tests_report.rb
+++ b/lib/reporting/ab_tests_report.rb
@@ -1,13 +1,7 @@
 # frozen_string_literal: true
 
-begin
-  require 'reporting/cloudwatch_client'
-  require 'reporting/cloudwatch_query_quoting'
-  require 'reporting/command_line_options'
-rescue LoadError => e
-  warn 'could not load paths, try running with "bundle exec rails runner"'
-  raise e
-end
+require 'reporting/cloudwatch_client'
+require 'reporting/cloudwatch_query_quoting'
 
 module Reporting
   class AbTestsReport

From e625a4e953777b6089acf8130e05ae7b429351d8 Mon Sep 17 00:00:00 2001
From: Andrew Duthie <andrew.duthie@gsa.gov>
Date: Wed, 5 Feb 2025 10:23:42 -0500
Subject: [PATCH 12/16] Remove unused CloudwatchClient options

---
 lib/reporting/ab_tests_report.rb           | 13 +--------
 spec/lib/reporting/ab_tests_report_spec.rb | 31 ----------------------
 2 files changed, 1 insertion(+), 43 deletions(-)

diff --git a/lib/reporting/ab_tests_report.rb b/lib/reporting/ab_tests_report.rb
index 65ac366aaf8..39cfdabc35b 100644
--- a/lib/reporting/ab_tests_report.rb
+++ b/lib/reporting/ab_tests_report.rb
@@ -11,16 +11,10 @@ class AbTestsReport
     # @param [Range<Time>] time_range
     def initialize(
       queries:,
-      time_range:,
-      verbose: false
+      time_range:
     )
       @queries = queries
       @time_range = time_range
-      @verbose = verbose
-    end
-
-    def verbose?
-      @verbose
     end
 
     def as_tables
@@ -73,14 +67,9 @@ def cloudwatch_client
       @cloudwatch_client ||= Reporting::CloudwatchClient.new(
         progress: false,
         ensure_complete_logs: false,
-        logger:,
       )
     end
 
-    def logger
-      Logger.new(STDERR) if verbose?
-    end
-
     # @return [String]
     def format_as_percent(number)
       format('%.2f%%', number)
diff --git a/spec/lib/reporting/ab_tests_report_spec.rb b/spec/lib/reporting/ab_tests_report_spec.rb
index 810af2afff4..4b2b903982a 100644
--- a/spec/lib/reporting/ab_tests_report_spec.rb
+++ b/spec/lib/reporting/ab_tests_report_spec.rb
@@ -60,35 +60,4 @@
       end
     end
   end
-
-  describe '#cloudwatch_client' do
-    subject(:cloudwatch_client) { report.cloudwatch_client }
-    let(:default_args) do
-      {
-        ensure_complete_logs: false,
-        logger: nil,
-        progress: false,
-      }
-    end
-
-    describe 'when all args are default' do
-      it 'creates a client with the default options' do
-        expect(Reporting::CloudwatchClient).to receive(:new).with(default_args)
-
-        cloudwatch_client
-      end
-    end
-
-    describe 'when verbose is passed in' do
-      let(:options) { { verbose: true } }
-      let(:logger) { double(Logger) }
-
-      it 'creates a client with the expected logger' do
-        expect(Logger).to receive(:new).with(STDERR).and_return(logger)
-        expect(Reporting::CloudwatchClient).to receive(:new).with({ **default_args, logger: })
-
-        cloudwatch_client
-      end
-    end
-  end
 end

From e91bc1324d0a63d85ea2f3d643092f9b4ba0b586 Mon Sep 17 00:00:00 2001
From: Andrew Duthie <andrew.duthie@gsa.gov>
Date: Wed, 5 Feb 2025 10:24:00 -0500
Subject: [PATCH 13/16] Add method visibility for public API

---
 lib/reporting/ab_tests_report.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/reporting/ab_tests_report.rb b/lib/reporting/ab_tests_report.rb
index 39cfdabc35b..2487d4e6d12 100644
--- a/lib/reporting/ab_tests_report.rb
+++ b/lib/reporting/ab_tests_report.rb
@@ -30,6 +30,8 @@ def as_emailable_reports
       end
     end
 
+    private
+
     def table_for_query(query)
       query_data = fetch_results(query: query.query)
       headers = column_labels(query_data.first)

From ebb07f58f83285fb4a5678f2674c2643dffbe14b Mon Sep 17 00:00:00 2001
From: Andrew Duthie <andrew.duthie@gsa.gov>
Date: Wed, 5 Feb 2025 10:25:14 -0500
Subject: [PATCH 14/16] Fix YARDoc param type for queries

---
 lib/reporting/ab_tests_report.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/reporting/ab_tests_report.rb b/lib/reporting/ab_tests_report.rb
index 2487d4e6d12..8dd45408c69 100644
--- a/lib/reporting/ab_tests_report.rb
+++ b/lib/reporting/ab_tests_report.rb
@@ -7,7 +7,7 @@ module Reporting
   class AbTestsReport
     attr_reader :queries, :time_range
 
-    # @param [Array<String>] queries
+    # @param [Array<AbTest::ReportQueryConfig>] queries
     # @param [Range<Time>] time_range
     def initialize(
       queries:,

From d2a453d027ed224ff2345c13f706327e69121b7a Mon Sep 17 00:00:00 2001
From: Andrew Duthie <andrew.duthie@gsa.gov>
Date: Wed, 5 Feb 2025 11:49:03 -0500
Subject: [PATCH 15/16] Format numbers outside 0-1 as percent

See: https://github.com/18F/identity-idp/pull/11835#discussion_r1943261698
---
 app/views/report_mailer/tables_report.html.erb | 2 +-
 spec/mailers/report_mailer_spec.rb             | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/views/report_mailer/tables_report.html.erb b/app/views/report_mailer/tables_report.html.erb
index d92d051c27b..8bb9fc0df7f 100644
--- a/app/views/report_mailer/tables_report.html.erb
+++ b/app/views/report_mailer/tables_report.html.erb
@@ -35,7 +35,7 @@
         <tr>
           <% row.each do |cell| %>
             <td <%= cell.is_a?(Numeric) ? 'class=table-number' : nil %> >
-              <% if cell.is_a?(Float) && report.float_as_percent? && cell.finite? && cell.between?(0.0, 1.0) %>
+              <% if cell.is_a?(Float) && report.float_as_percent? && cell.finite? %>
                 <%= number_to_percentage(cell * 100, precision: report.precision || 2) %>
               <% else %>
                 <%= number_with_delimiter(cell) %>
diff --git a/spec/mailers/report_mailer_spec.rb b/spec/mailers/report_mailer_spec.rb
index 0f36fac9fa5..cfd5223df8a 100644
--- a/spec/mailers/report_mailer_spec.rb
+++ b/spec/mailers/report_mailer_spec.rb
@@ -122,9 +122,9 @@
       expect(percent_cell.text.strip).to eq('50.00%')
       expect(percent_cell['class']).to eq('table-number')
 
-      percent_float_outside_range_cell = percent_table.at_css('tbody tr:nth-child(2) td:last-child')
-      expect(percent_float_outside_range_cell.text.strip).to eq('1.5')
-      expect(percent_float_outside_range_cell['class']).to eq('table-number')
+      percent_exceeding_100_cell = percent_table.at_css('tbody tr:nth-child(2) td:last-child')
+      expect(percent_exceeding_100_cell.text.strip).to eq('150.00%')
+      expect(percent_exceeding_100_cell['class']).to eq('table-number')
 
       percent_float_nan_cell = percent_table.at_css('tbody tr:nth-child(3) td:last-child')
       expect(percent_float_nan_cell.text.strip).to eq('NaN')

From 5f96a9daaa3d93fe3bc573c6511b7f26e06d7505 Mon Sep 17 00:00:00 2001
From: Andrew Duthie <andrew.duthie@gsa.gov>
Date: Thu, 6 Feb 2025 11:40:56 -0500
Subject: [PATCH 16/16] Fix spec row label to increment

---
 spec/mailers/report_mailer_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/mailers/report_mailer_spec.rb b/spec/mailers/report_mailer_spec.rb
index cfd5223df8a..a582f93e535 100644
--- a/spec/mailers/report_mailer_spec.rb
+++ b/spec/mailers/report_mailer_spec.rb
@@ -71,7 +71,7 @@
         ['Float', 'Int', 'Float'],
         ['Row 1', 1, 0.5],
         ['Row 2', 1, 1.5],
-        ['Row 2', 1, Float::NAN],
+        ['Row 3', 1, Float::NAN],
       ]
     end