From ee2931812627ccde742ef376a62f4c8baf667afa Mon Sep 17 00:00:00 2001 From: Ben Halpern Date: Mon, 29 Jan 2024 12:18:50 -0500 Subject: [PATCH] Add a/b test for which comments to show in feed (#20332) * Add a/b test for which comments to show in feed * Adjust figure * Started at * Move scope placement around * Add tests for different comments_variants passed * Fix typo --- app/controllers/stories/feeds_controller.rb | 5 ++- app/models/article.rb | 36 +++++++++++++++++++ app/services/articles/feeds/variant_query.rb | 22 ++++++++++-- app/views/stories/feeds/show.json.jbuilder | 2 +- config/field_test.yml | 27 ++++++++++++++ .../articles/feeds/variant_query_spec.rb | 13 +++++++ 6 files changed, 101 insertions(+), 4 deletions(-) diff --git a/app/controllers/stories/feeds_controller.rb b/app/controllers/stories/feeds_controller.rb index 709f667fa..cdbee1aac 100644 --- a/app/controllers/stories/feeds_controller.rb +++ b/app/controllers/stories/feeds_controller.rb @@ -4,6 +4,8 @@ module Stories def show @page = (params[:page] || 1).to_i + @comments_variant = field_test(:comments_to_display_20240129, participant: @user) + @stories = assign_feed_stories add_pinned_article @@ -55,7 +57,8 @@ module Stories # weighted strategy has not. I also don't want to alter the # weighted query implementation as it returns a lovely # ActiveRecord::Relation. So this is a compromise. - feed.more_comments_minimal_weight_randomized.to_a + + feed.more_comments_minimal_weight_randomized(comments_variant: @comments_variant) end end diff --git a/app/models/article.rb b/app/models/article.rb index 63b70580a..99ea60796 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -146,6 +146,42 @@ class Article < ApplicationRecord inverse_of: :commentable, class_name: "Comment" + has_many :more_inclusive_top_comments, + lambda { + where(comments: { score: 5.. }, ancestry: nil, hidden_by_commentable_user: false, deleted: false) + .order("comments.score" => :desc) + }, + as: :commentable, + inverse_of: :commentable, + class_name: "Comment" + + has_many :recent_good_comments, + lambda { + where(comments: { score: 8.. }, ancestry: nil, hidden_by_commentable_user: false, deleted: false) + .order("comments.created_at" => :desc) + }, + as: :commentable, + inverse_of: :commentable, + class_name: "Comment" + + has_many :more_inclusive_recent_good_comments, + lambda { + where(comments: { score: 5.. }, ancestry: nil, hidden_by_commentable_user: false, deleted: false) + .order("comments.created_at" => :desc) + }, + as: :commentable, + inverse_of: :commentable, + class_name: "Comment" + + has_many :most_inclusive_recent_good_comments, + lambda { + where(comments: { score: 3.. }, ancestry: nil, hidden_by_commentable_user: false, deleted: false) + .order("comments.created_at" => :desc) + }, + as: :commentable, + inverse_of: :commentable, + class_name: "Comment" + validates :body_markdown, bytesize: { maximum: 800.kilobytes, too_long: proc { I18n.t("models.article.is_too_long") } diff --git a/app/services/articles/feeds/variant_query.rb b/app/services/articles/feeds/variant_query.rb index c4def78a9..d6ffbc550 100644 --- a/app/services/articles/feeds/variant_query.rb +++ b/app/services/articles/feeds/variant_query.rb @@ -90,7 +90,7 @@ module Articles # puts strategy.call.to_sql # # rubocop:disable Layout/LineLength - def call(only_featured: false, must_have_main_image: false, limit: default_limit, offset: default_offset, omit_article_ids: []) + def call(only_featured: false, must_have_main_image: false, limit: default_limit, offset: default_offset, omit_article_ids: [], comments_variant: default_comments_variant) # rubocop:enable Layout/LineLength # These are the variables we'll pass to the SQL statement. @@ -136,10 +136,24 @@ module Articles # goodness of scopes (e.g., limited_column_select) and eager includes. scope = Article.joins(join_fragment) .limited_column_select - .includes(top_comments: :user) .includes(:distinct_reaction_categories) .order(config.order_by.to_sql) + scope = case comments_variant + when "top_comments" + scope.includes(top_comments: :user) + when "more_inclusive_top_comments" + scope.includes(more_inclusive_top_comments: :user) + when "recent_good_comments" + scope.includes(recent_good_comments: :user) + when "more_inclusive_recent_good_comments" + scope.includes(more_inclusive_recent_good_comments: :user) + when "most_inclusive_recent_good_comments" + scope.includes(most_inclusive_recent_good_comments: :user) + else + scope.includes(top_comments: :user) # fallback default + end + if @user.present? && (hidden_tags = @user.cached_antifollowed_tag_names).any? scope = scope.not_cached_tagged_with_any(hidden_tags) end @@ -306,6 +320,10 @@ module Articles (@page.to_i - 1) * default_limit end + def default_comments_variant + "top_comments" + end + # We want to ensure that we're not randomizing someone's feed all the time; and instead aiming # for somewhat repeatable experiences (e.g. I refresh the page it is likely I will have the # same order of pages even though we've randomized things a bit). diff --git a/app/views/stories/feeds/show.json.jbuilder b/app/views/stories/feeds/show.json.jbuilder index d476d66cb..f86214fa3 100644 --- a/app/views/stories/feeds/show.json.jbuilder +++ b/app/views/stories/feeds/show.json.jbuilder @@ -29,7 +29,7 @@ json.array!(@stories) do |article| json.tag_list article.cached_tag_list_array json.extract! article, *article_methods_to_include - json.top_comments article.top_comments do |comment| + json.top_comments article.public_send(@comments_variant.to_sym) do |comment| comment = comment.decorate json.comment_id comment.id json.extract! comment, :user_id, :published_timestamp, :published_at_int, :safe_processed_html, :path diff --git a/config/field_test.yml b/config/field_test.yml index 042f496a8..cbb941e8e 100644 --- a/config/field_test.yml +++ b/config/field_test.yml @@ -27,6 +27,33 @@ ################################################################################ ################################################################################ experiments: + comments_to_display_20240129: + started_at: 2024-01-29 + variants: + - top_comments + - more_inclusive_top_comments + - recent_good_comments + - more_inclusive_recent_good_comments + - most_inclusive_recent_good_comments + weights: + - 60 + - 10 + - 10 + - 10 + - 10 + goals: + - user_creates_pageview + - user_creates_article_reaction + - user_creates_comment + - user_views_pages_on_at_least_two_different_days_within_a_week + - user_views_pages_on_at_least_four_different_days_within_a_week + - user_creates_article_reaction_on_four_different_days_within_a_week + - user_creates_comment_on_at_least_four_different_days_within_a_week + - user_views_pages_on_at_least_three_different_hours_within_a_day + - user_views_pages_on_at_least_four_different_hours_within_a_day + - user_views_pages_on_at_least_twelve_different_hours_within_five_days + - user_views_pages_on_at_least_nine_different_days_within_two_weeks + # NOTE: Our feed strategy testing experiment must begin with "feed_strategy" feed_strategy_starting_20240126: started_at: 2024-01-26 diff --git a/spec/services/articles/feeds/variant_query_spec.rb b/spec/services/articles/feeds/variant_query_spec.rb index 191e505bc..925737c48 100644 --- a/spec/services/articles/feeds/variant_query_spec.rb +++ b/spec/services/articles/feeds/variant_query_spec.rb @@ -58,6 +58,19 @@ RSpec.describe Articles::Feeds::VariantQuery, type: :service do expect(query_call).to be_a(ActiveRecord::Relation) expect(query_call.to_a).not_to include(article) end + + it "returns proper scope when passed the different test values" do + # rubocop:disable Performance/CollectionLiteralInLoop + %w[top_comments + more_inclusive_top_comments + recent_good_comments + more_inclusive_recent_good_comments + most_inclusive_recent_good_comments].each do |test_value| + expect(variant_query.call(comments_variant: test_value).includes_values) + .to include(:distinct_reaction_categories, { test_value.to_sym => :user }) + end + # rubocop:enable Performance/CollectionLiteralInLoop + end end describe "#featured_story_and_default_home_feed" do