maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Add preferred_article_ids to billboards (#20437)

Browse source 
* Add preferred_article_ids to billboards

* Adjust query

* Refactor method

* Refactor method

* Fix input type

* Add feature flag
This commit is contained in:
Ben Halpern 2023-12-18 14:20:56 -05:00 • committed by GitHub
parent 78b42c5efd
commit f0673648c2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 139 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/controllers/api/v1/billboards_controller.rb
View file

@ -51,7 +51,7 @@ module Api
:name, :organization_id, :placement_area, :published,
:tag_list, :type_of, :exclude_article_ids, :weight,
:audience_segment_type, :audience_segment_id, :priority,
:custom_display_label, :template, :render_mode,
:custom_display_label, :template, :render_mode, :preferred_article_ids,
# Permitting twice allows both comma-separated string and array values
:target_geolocations, target_geolocations: []
end

67
app/models/billboard.rb
View file

@ -92,7 +92,7 @@ class Billboard < ApplicationRecord
# Here we sample from only billboards with fewer than 1000 impressions (with a fallback
# if there are none of those, causing an extra query, but that shouldn't happen very often).
relation = billboards_for_display.seldom_seen(area)
weighted_random_selection(relation) || billboards_for_display.sample
weighted_random_selection(relation, article&.id) || billboards_for_display.sample
else # large range, 65%
# Ads that get engagement have a higher "success rate", and among this category, we sample from the top 15 that
@ -105,24 +105,49 @@ class Billboard < ApplicationRecord
end
end
def self.weighted_random_selection(relation)
def self.weighted_random_selection(relation, target_article_id = nil)
base_query = relation.to_sql
random_val = rand.to_f
query = <<-SQL
WITH base AS (#{base_query}),
weighted AS (
SELECT *, weight,
SUM(weight) OVER () AS total_weight,
SUM(weight) OVER (ORDER BY id ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS running_weight
FROM base
)
SELECT *, running_weight, ? * total_weight AS random_value FROM weighted
WHERE running_weight >= ? * total_weight
ORDER BY running_weight ASC
LIMIT 1
SQL
if FeatureFlag.enabled?(:article_id_adjusted_weight)
condition = target_article_id.blank? ? "FALSE" : "#{target_article_id} = ANY(preferred_article_ids)"
query = <<-SQL
WITH base AS (#{base_query}),
weighted AS (
SELECT *,
CASE
WHEN #{condition} THEN weight * 10
ELSE weight
END AS adjusted_weight,
SUM(CASE
WHEN #{condition} THEN weight * 10
ELSE weight
END) OVER () AS total_weight,
SUM(CASE
WHEN #{condition} THEN weight * 10
ELSE weight
END) OVER (ORDER BY id ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS running_weight
FROM base
)
SELECT *, running_weight, ? * total_weight AS random_value FROM weighted
WHERE running_weight >= ? * total_weight
ORDER BY running_weight ASC
LIMIT 1
SQL
else
query = <<-SQL
WITH base AS (#{base_query}),
weighted AS (
SELECT *, weight,
SUM(weight) OVER () AS total_weight,
SUM(weight) OVER (ORDER BY id ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS running_weight
FROM base
)
SELECT *, running_weight, ? * total_weight AS random_value FROM weighted
WHERE running_weight >= ? * total_weight
ORDER BY running_weight ASC
LIMIT 1
SQL
end
relation.find_by_sql([query, random_val, random_val]).first
end
@ -196,7 +221,7 @@ class Billboard < ApplicationRecord
end
# rubocop:enable Style/OptionHash
# exclude_article_ids is an integer array, web inputs are comma-separated strings
# exclude_article_ids and preferred_article_ids are integer arrays, web inputs are comma-separated strings
# ActiveRecord normalizes these in a bad way, so we are intervening
def exclude_article_ids=(input)
adjusted_input = input.is_a?(String) ? input.split(",") : input
@ -204,6 +229,12 @@ class Billboard < ApplicationRecord
write_attribute :exclude_article_ids, (adjusted_input || [])
end
def preferred_article_ids=(input)
adjusted_input = input.is_a?(String) ? input.split(",") : input
adjusted_input = adjusted_input&.filter_map { |value| value.presence&.to_i }
write_attribute :preferred_article_ids, (adjusted_input || [])
end
private
def generate_billboard_name

9
db/migrate/20231208165353_add_article_ids_to_billboards.rb Normal file
View file

@ -0,0 +1,9 @@
class AddArticleIdsToBillboards < ActiveRecord::Migration[7.0]
def up
add_column :display_ads, :preferred_article_ids, :integer, array: true, default: []
end
def down
safety_assured { remove_column :display_ads, :preferred_article_ids, :integer, array: true }
end
end

7
db/migrate/20231208165454_add_article_ids_index_to_billboards.rb Normal file
View file

@ -0,0 +1,7 @@
class AddArticleIdsIndexToBillboards < ActiveRecord::Migration[7.0]
disable_ddl_transaction!
def change
add_index :display_ads, :preferred_article_ids, using: 'gin', algorithm: :concurrently
end
end

4
db/schema.rb
View file

@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema[7.0].define(version: 2023_11_29_135338) do
ActiveRecord::Schema[7.0].define(version: 2023_12_08_165454) do
# These are extensions that must be enabled in order to support this database
enable_extension "citext"
enable_extension "ltree"
@ -483,6 +483,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_11_29_135338) do
t.string "name"
t.bigint "organization_id"
t.string "placement_area"
t.integer "preferred_article_ids", default: [], array: true
t.boolean "priority", default: false
t.text "processed_html"
t.boolean "published", default: false
@ -496,6 +497,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_11_29_135338) do
t.index ["cached_tag_list"], name: "index_display_ads_on_cached_tag_list", opclass: :gin_trgm_ops, using: :gin
t.index ["exclude_article_ids"], name: "index_display_ads_on_exclude_article_ids", using: :gin
t.index ["placement_area"], name: "index_display_ads_on_placement_area"
t.index ["preferred_article_ids"], name: "index_display_ads_on_preferred_article_ids", using: :gin
t.index ["target_geolocations"], name: "gist_index_display_ads_on_target_geolocations", using: :gist
end

91
spec/models/billboard_spec.rb
View file

@ -553,36 +553,79 @@ RSpec.describe Billboard do
end
describe ".weighted_random_selection" do
it "samples with weights correctly" do
described_class.delete_all
bb1 = create(:billboard, weight: 5)
bb2 = create(:billboard, weight: 1)
bb3 = create(:billboard, weight: 1)
bb4 = create(:billboard, weight: 2)
bb5 = create(:billboard, weight: 1)
context "when no target_article_id is provided" do
it "samples with weights correctly" do
described_class.delete_all
bb1 = create(:billboard, weight: 5)
bb2 = create(:billboard, weight: 1)
bb3 = create(:billboard, weight: 1)
bb4 = create(:billboard, weight: 2)
bb5 = create(:billboard, weight: 1)
total_weight = 5 + 1 + 1 + 2 + 1 # 10
expected_probabilities = {
bb1.id => 5.0 / total_weight,
bb2.id => 1.0 / total_weight,
bb3.id => 1.0 / total_weight,
bb4.id => 2.0 / total_weight,
bb5.id => 1.0 / total_weight
}
total_weight = 5 + 1 + 1 + 2 + 1 # 10
expected_probabilities = {
bb1.id => 5.0 / total_weight,
bb2.id => 1.0 / total_weight,
bb3.id => 1.0 / total_weight,
bb4.id => 2.0 / total_weight,
bb5.id => 1.0 / total_weight
}
counts = Hash.new(0)
num_trials = 5_000
counts = Hash.new(0)
num_trials = 5_000
num_trials.times do
id = described_class.weighted_random_selection(described_class.all).id
counts[id] += 1
num_trials.times do
id = described_class.weighted_random_selection(described_class.all).id
counts[id] += 1
end
counts.each do |id, count|
observed_probability = count.to_f / num_trials
expected_probability = expected_probabilities[id]
expect(observed_probability).to be_within(0.025).of(expected_probability)
end
end
end
counts.each do |id, count|
observed_probability = count.to_f / num_trials
expected_probability = expected_probabilities[id]
context "when a target_article_id is provided" do
it "favors records containing the target_article_id" do
allow(FeatureFlag).to receive(:enabled?).with(:article_id_adjusted_weight).and_return(true)
described_class.delete_all
target_article_id = 123
favored_weight_multiplier = 10
expect(observed_probability).to be_within(0.025).of(expected_probability)
# Create billboards with different weights and article_ids
bb1 = create(:billboard, weight: 5, preferred_article_ids: [target_article_id])
bb2 = create(:billboard, weight: 1, preferred_article_ids: [])
bb3 = create(:billboard, weight: 1, preferred_article_ids: [target_article_id])
bb4 = create(:billboard, weight: 2, preferred_article_ids: [])
bb5 = create(:billboard, weight: 1, preferred_article_ids: [])
# Adjusted total weight accounting for the favored records
total_weight = (5 * favored_weight_multiplier) + 1 + (1 * favored_weight_multiplier) + 2 + 1
expected_probabilities = {
bb1.id => (5.0 * favored_weight_multiplier) / total_weight,
bb2.id => 1.0 / total_weight,
bb3.id => (1.0 * favored_weight_multiplier) / total_weight,
bb4.id => 2.0 / total_weight,
bb5.id => 1.0 / total_weight
}
counts = Hash.new(0)
num_trials = 5_000
num_trials.times do
id = described_class.weighted_random_selection(described_class.all, target_article_id).id
counts[id] += 1
end
counts.each do |id, count|
observed_probability = count.to_f / num_trials
expected_probability = expected_probabilities[id]
expect(observed_probability).to be_within(0.025).of(expected_probability)
end
end
end
end

6
spec/requests/api/v1/billboards_spec.rb
View file

@ -52,7 +52,7 @@ RSpec.describe "Api::V1::Billboards" do
"success_rate", "tag_list", "type_of", "updated_at",
"creator_id", "exclude_article_ids",
"audience_segment_type", "audience_segment_id",
"custom_display_label", "template", "render_mode",
"custom_display_label", "template", "render_mode", "preferred_article_ids",
"priority", "weight", "target_geolocations")
expect(response.parsed_body["target_geolocations"]).to contain_exactly("US-WA", "CA-BC")
end
@ -72,7 +72,7 @@ RSpec.describe "Api::V1::Billboards" do
"success_rate", "tag_list", "type_of", "updated_at",
"creator_id", "exclude_article_ids",
"audience_segment_type", "audience_segment_id",
"custom_display_label", "template", "render_mode",
"custom_display_label", "template", "render_mode", "preferred_article_ids",
"priority", "weight", "target_geolocations")
expect(response.parsed_body["target_geolocations"]).to contain_exactly("US-WA", "CA-BC")
end
@ -138,7 +138,7 @@ RSpec.describe "Api::V1::Billboards" do
"success_rate", "tag_list", "type_of", "updated_at",
"creator_id", "exclude_article_ids",
"audience_segment_type", "audience_segment_id",
"custom_display_label", "template", "render_mode",
"custom_display_label", "template", "render_mode", "preferred_article_ids",
"priority", "weight", "target_geolocations")
end