From e1b1fe3ed7364b543bdd3c26267b4e1dcde5cd10 Mon Sep 17 00:00:00 2001 From: Ben Halpern Date: Thu, 4 Feb 2021 13:02:00 -0500 Subject: [PATCH] Declare winner in feed_top_articles_query test (#12308) * Declare winner in feed_top_articles_query test * Some WIP modifications to lfe * Fix up tests and generalize * Fix style * Adjust tests to handle different field test scenarios * Remove socre_randomness tests * Fix test to fit stub * Update app/workers/users/record_field_test_event_worker.rb --- app/models/comment.rb | 2 + app/models/page_view.rb | 2 + .../feeds/large_forem_experimental.rb | 72 +++---------------- .../users/record_field_test_event_worker.rb | 30 ++++---- config/field_test.yml | 58 +++++++-------- spec/rails_helper.rb | 18 ++++- spec/requests/comments_spec.rb | 16 +++++ spec/requests/page_views_spec.rb | 16 +++++ .../feeds/large_forem_experimental_spec.rb | 23 ------ .../record_field_test_event_worker_spec.rb | 2 +- 10 files changed, 106 insertions(+), 133 deletions(-) diff --git a/app/models/comment.rb b/app/models/comment.rb index 8570011e6..8ccd0867b 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -304,6 +304,8 @@ class Comment < ApplicationRecord end def record_field_test_event + return if FieldTest.config["experiments"].nil? + Users::RecordFieldTestEventWorker .perform_async(user_id, "user_creates_comment") end diff --git a/app/models/page_view.rb b/app/models/page_view.rb index 2eb1c976c..96297f495 100644 --- a/app/models/page_view.rb +++ b/app/models/page_view.rb @@ -28,6 +28,8 @@ class PageView < ApplicationRecord end def record_field_test_event + return if FieldTest.config["experiments"].nil? + return unless user_id Users::RecordFieldTestEventWorker diff --git a/app/services/articles/feeds/large_forem_experimental.rb b/app/services/articles/feeds/large_forem_experimental.rb index 591b12276..a4baee456 100644 --- a/app/services/articles/feeds/large_forem_experimental.rb +++ b/app/services/articles/feeds/large_forem_experimental.rb @@ -9,7 +9,6 @@ module Articles @number_of_articles = number_of_articles @page = page @tag = tag - @randomness = 3 # default number for randomly adjusting feed @tag_weight = 1 # default weight tags play in rankings @comment_weight = 0 # default weight comments play in rankings @experience_level_weight = 1 # default weight for user experience level @@ -66,9 +65,8 @@ module Articles end def more_comments_minimal_weight_randomized_at_end - @randomness = 0 results = more_comments_minimal_weight - first_half(results).shuffle + last_half(results) + first_quarter(results).shuffle + last_three_quarters(results) end def rank_and_sort_articles(articles) @@ -85,7 +83,6 @@ module Articles article_points += score_followed_user(article) article_points += score_followed_organization(article) article_points += score_followed_tags(article) - article_points += score_randomness article_points += score_experience_level(article) article_points += score_comments(article) article_points @@ -108,10 +105,6 @@ module Articles user_following_org_ids.include?(article.organization_id) ? 1 : 0 end - def score_randomness - rand(3) * @randomness - end - def score_experience_level(article) - (((article.experience_level_rating - (@user&.experience_level || 5)).abs / 2) * @experience_level_weight) end @@ -141,58 +134,13 @@ module Articles private - # Disable complexity cop to allow for variant-driven method - # rubocop:disable Metrics/CyclomaticComplexity def experimental_hot_story_grab - test_variant = @user ? field_test(:feed_top_articles_query, participant: @user) : "base" - case test_variant - when "base_with_more_articles" # equivalent to current base but with higher "number of articles" - articles = Article.published.limited_column_select.includes(top_comments: :user) - .page(@page).per(75) - .where("score >= ? OR featured = ?", SiteConfig.home_feed_minimum_score, true) - .order(hotness_score: :desc) - when "only_followed_tags" # equivalent to base but only on tags user follows (if user follows enough) - followed_tags = @user.cached_followed_tag_names - articles = Article.published.includes(top_comments: :user) - .page(@page).per(@number_of_articles) - .where("score >= ? OR featured = ?", SiteConfig.home_feed_minimum_score, true) - .order(hotness_score: :desc) - # We only want to limit the posts to tagged_with if the participant follows enough tags. - articles = articles.tagged_with(followed_tags, any: true) if followed_tags.size > 4 - - articles = articles.unscope(:select).limited_column_select - when "top_articles_since_last_pageview_3_days_max" # Top articles since last page view (max 3 days) - start_time = [(@user.page_views.last&.created_at || 3.days.ago) - 12.hours, 3.days.ago].max - articles = Article.published.limited_column_select.includes(top_comments: :user) - .where("published_at > ?", start_time) - .page(@page).per(@number_of_articles) - .order(score: :desc) - when "top_articles_since_last_pageview_7_days_max" # Top articles since last page view (max 7 days) - start_time = [(@user.page_views.last&.created_at || 7.days.ago) - 12.hours, 7.days.ago].max - articles = Article.published.limited_column_select.includes(top_comments: :user) - .where("published_at > ?", start_time) - .page(@page).per(@number_of_articles) - .order(score: :desc) - when "combination_only_tags_followed_and_top_max_7_days" # Top articles since last page view (max 7 days) - start_time = [(@user.page_views.last&.created_at || 7.days.ago) - 12.hours, 7.days.ago].max - followed_tags = @user.cached_followed_tag_names - articles = Article.published.includes(top_comments: :user) - .where("published_at > ?", start_time) - .page(@page).per(@number_of_articles) - .order(score: :desc) - # We only want to limit the posts to tagged_with if the participant follows enough tags. - articles = articles.tagged_with(followed_tags, any: true) if followed_tags.size > 4 - - articles = articles.unscope(:select).limited_column_select - else # "base" - articles = Article.published.limited_column_select.includes(top_comments: :user) - .page(@page).per(@number_of_articles) - .where("score >= ? OR featured = ?", SiteConfig.home_feed_minimum_score, true) - .order(hotness_score: :desc) - end - articles + start_time = [(@user.page_views.second_to_last&.created_at || 7.days.ago) - 18.hours, 7.days.ago].max + Article.published.limited_column_select.includes(top_comments: :user) + .where("published_at > ?", start_time) + .page(@page).per(@number_of_articles) + .order(score: :desc) end - # rubocop:enable Metrics/CyclomaticComplexity def user_followed_tags @user_followed_tags ||= (@user&.decorate&.cached_followed_tags || []) @@ -206,12 +154,12 @@ module Articles @user_following_users_ids ||= (@user&.cached_following_users_ids || []) end - def first_half(array) - array[0...(array.length / 2)] + def first_quarter(array) + array[0...(array.length / 4)] end - def last_half(array) - array[(array.length / 2)..array.length] + def last_three_quarters(array) + array[(array.length / 4)..array.length] end end end diff --git a/app/workers/users/record_field_test_event_worker.rb b/app/workers/users/record_field_test_event_worker.rb index c893b7210..a4cb17058 100644 --- a/app/workers/users/record_field_test_event_worker.rb +++ b/app/workers/users/record_field_test_event_worker.rb @@ -9,20 +9,22 @@ module Users @user = User.find_by(id: user_id) return unless @user - @experiment = :feed_top_articles_query # Current experiment running - case goal - # We have special conditional goals for some where we look for past events for commulative wins - # Otherwise we convert the goal as given. - when "user_creates_pageview" - pageview_goal(7.days.ago, "DATE(created_at)", 4, "user_views_article_four_days_in_week") - pageview_goal(24.hours.ago, "DATE_PART('hour', created_at)", 4, "user_views_article_four_hours_in_day") - pageview_goal(14.days.ago, "DATE(created_at)", 9, "user_views_article_nine_days_in_two_week") - pageview_goal(5.days.ago, "DATE_PART('hour', created_at)", 12, "user_views_article_twelve_hours_in_five_days") - when "user_creates_comment" # comments goal. Only page views and comments are currently active. - field_test_converted(@experiment, participant: @user, goal: goal) # base single comment goal. - comment_goal(7.days.ago, "DATE(created_at)", 4, "user_creates_comment_four_days_in_week") - else - field_test_converted(@experiment, participant: @user, goal: goal) # base single comment goal. + FieldTest.config["experiments"].each_key do |key| + @experiment = key.to_sym + case goal + # We have special conditional goals for some where we look for past events for commulative wins + # Otherwise we convert the goal as given. + when "user_creates_pageview" + pageview_goal(7.days.ago, "DATE(created_at)", 4, "user_views_article_four_days_in_week") + pageview_goal(24.hours.ago, "DATE_PART('hour', created_at)", 4, "user_views_article_four_hours_in_day") + pageview_goal(14.days.ago, "DATE(created_at)", 9, "user_views_article_nine_days_in_two_week") + pageview_goal(5.days.ago, "DATE_PART('hour', created_at)", 12, "user_views_article_twelve_hours_in_five_days") + when "user_creates_comment" # comments goal. Only page views and comments are currently active. + field_test_converted(@experiment, participant: @user, goal: goal) # base single comment goal. + comment_goal(7.days.ago, "DATE(created_at)", 4, "user_creates_comment_four_days_in_week") + else + field_test_converted(@experiment, participant: @user, goal: goal) # base single comment goal. + end end end diff --git a/config/field_test.yml b/config/field_test.yml index 877928c0c..8a761c85a 100644 --- a/config/field_test.yml +++ b/config/field_test.yml @@ -1,36 +1,30 @@ experiments: - feed_top_articles_query: - # This tests how we fetch "globally hot articles" — Testing some variants of - # more focus on the particpant's tags, some other alternatives which take more into account - # when the particpant was last on the site and viewed a post. - # -> Recommended experiment duration: 2 weeks. - # This experiment has new goals measuring consistently of coming back and clocking a page view - # and two weeks should be enough to see results. - # Hypotheses. This will be more impactful than past experiments which were too modest. - # We'll get more info on bigger levers on experiments, and should be able to pit the winner - # against some more variation. - # combination_only_tags_followed_and_top_max_7_days will be interesting to observe I think. - variants: - - base - - base_with_more_articles - - only_followed_tags - - top_articles_since_last_pageview_3_days_max - - top_articles_since_last_pageview_7_days_max - - combination_only_tags_followed_and_top_max_7_days - weights: # "base" gets a few more rolls of the dice - - 20 - - 16 - - 16 - - 16 - - 16 - - 16 - goals: - - user_creates_comment - - user_creates_comment_four_days_in_week - - user_views_article_four_days_in_week - - user_views_article_four_hours_in_day - - user_views_article_nine_days_in_two_week - - user_views_article_twelve_hours_in_five_days + # No field test running. + # To run a test, change the five variants here. You can create more than + # five, but five is our typical standard. + # Do not change the goals unless you intend to also change the corresponding code in + # app/workers/users/record_field_test_event_worker.rb + + # variant_name: + # variants: + # - base + # - variant_1 + # - variant_2 + # - variant_3 + # - variant_4 + # weights: # "base" gets a few more rolls of the dice + # - 20 + # - 20 + # - 20 + # - 20 + # - 20 + # goals: + # - user_creates_comment + # - user_creates_comment_four_days_in_week + # - user_views_article_four_days_in_week + # - user_views_article_four_hours_in_day + # - user_views_article_nine_days_in_two_week + # - user_views_article_twelve_hours_in_five_days exclude: bots: true diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index cd54775b9..f83dc0826 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -180,7 +180,23 @@ RSpec.configure do |config| allow(SiteConfig).to receive(:community_description).and_return("Some description") allow(SiteConfig).to receive(:public).and_return(true) allow(SiteConfig).to receive(:waiting_on_first_user).and_return(false) - end + + # Default to have field a field test available. + config = { "experiments" => + { "wut" => + { "variants" => %w[base var_1], + "weights" => [50, 50], + "goals" => %w[user_creates_comment + user_creates_comment_four_days_in_week + user_views_article_four_days_in_week + user_views_article_four_hours_in_day + user_views_article_nine_days_in_two_week + user_views_article_twelve_hours_in_five_days] } }, + "exclude" => { "bots" => true }, + "cache" => true, + "cookies" => false } + allow(FieldTest).to receive(:config).and_return(config) +end config.after do Timecop.return diff --git a/spec/requests/comments_spec.rb b/spec/requests/comments_spec.rb index 16c35f005..852f78170 100644 --- a/spec/requests/comments_spec.rb +++ b/spec/requests/comments_spec.rb @@ -339,6 +339,22 @@ RSpec.describe "Comments", type: :request do expect(Users::RecordFieldTestEventWorker).to have_received(:perform_async).with(*expected_args) end end + + context "when not part of field test" do + before do + sign_in user + allow(FieldTest).to receive(:config).and_return({ "experiments" => nil }) + allow(Users::RecordFieldTestEventWorker).to receive(:perform_async) + end + + it "converts field test" do + post "/comments", params: base_comment_params + + expected_args = [user.id, "user_creates_comment"] + expect(Users::RecordFieldTestEventWorker).not_to have_received(:perform_async) + end + end + end describe "PATCH /comments/:comment_id/hide" do diff --git a/spec/requests/page_views_spec.rb b/spec/requests/page_views_spec.rb index 5bb914221..0ea2d1c1d 100644 --- a/spec/requests/page_views_spec.rb +++ b/spec/requests/page_views_spec.rb @@ -60,6 +60,22 @@ RSpec.describe "PageViews", type: :request do end end + context "when not part of field test" do + before do + sign_in user + allow(FieldTest).to receive(:config).and_return({ "experiments" => nil }) + allow(Users::RecordFieldTestEventWorker).to receive(:perform_async) + end + + it "converts field test" do + post "/page_views", params: { + article_id: article.id, + referrer: "test" + } + expect(Users::RecordFieldTestEventWorker).not_to have_received(:perform_async) + end + end + context "when user not signed in" do it "creates a new page view" do post "/page_views", params: { diff --git a/spec/services/articles/feeds/large_forem_experimental_spec.rb b/spec/services/articles/feeds/large_forem_experimental_spec.rb index 2282b98bd..314cf9360 100644 --- a/spec/services/articles/feeds/large_forem_experimental_spec.rb +++ b/spec/services/articles/feeds/large_forem_experimental_spec.rb @@ -205,29 +205,6 @@ RSpec.describe Articles::Feeds::LargeForemExperimental, type: :service do end end - describe "#score_randomness" do - context "when random number is less than 0.6 but greater than 0.3" do - it "returns 6" do - allow(feed).to receive(:rand).and_return(2) - expect(feed.score_randomness).to eq 6 - end - end - - context "when random number is less than 0.3" do - it "returns 3" do - allow(feed).to receive(:rand).and_return(1) - expect(feed.score_randomness).to eq 3 - end - end - - context "when random number is greater than 0.6" do - it "returns 0" do - allow(feed).to receive(:rand).and_return(0) - expect(feed.score_randomness).to eq 0 - end - end - end - describe "#score_followed_tags" do let(:tag) { create(:tag) } let(:unfollowed_tag) { create(:tag) } diff --git a/spec/workers/users/record_field_test_event_worker_spec.rb b/spec/workers/users/record_field_test_event_worker_spec.rb index 227c6ccf2..8994691bf 100644 --- a/spec/workers/users/record_field_test_event_worker_spec.rb +++ b/spec/workers/users/record_field_test_event_worker_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Users::RecordFieldTestEventWorker, type: :worker do context "with user who is part of field test" do before do - field_test(:feed_top_articles_query, participant: user) + field_test(:wut, participant: user) end it "records user_creates_reaction field test conversion" do