diff --git a/app/services/articles/feeds/weighted_query_strategy.rb b/app/services/articles/feeds/weighted_query_strategy.rb index b08901608..359c32cd4 100644 --- a/app/services/articles/feeds/weighted_query_strategy.rb +++ b/app/services/articles/feeds/weighted_query_strategy.rb @@ -86,11 +86,11 @@ module Articles daily_decay_factor: { clause: "(current_date - articles.published_at::date)", cases: [ - [0, 1], [1, 0.99], [2, 0.985], - [3, 0.98], [4, 0.975], [5, 0.97], - [6, 0.965], [7, 0.96], [8, 0.955], - [9, 0.95], [10, 0.945], [11, 0.94], - [12, 0.935], [13, 0.93], [14, 0.925] + [0, 1], [1, 0.975], [2, 0.965], + [3, 0.955], [4, 0.945], [5, 0.935], + [6, 0.925], [7, 0.922], [8, 0.0919], + [9, 0.916], [10, 0.913], [11, 0.91], + [12, 0.907], [13, 0.904], [14, 0.901] ], fallback: 0.9, requires_user: false, @@ -349,19 +349,20 @@ module Articles ] end + # Following this blog post: https://pganalyze.com/blog/active-record-subqueries-rails#the-from-subquery + join_fragment = Arel.sql( + "INNER JOIN (#{Article.sanitize_sql(unsanitized_sub_sql)}) AS article_relevancies ON articles.id = article_relevancies.id", + ) + # This sub-query allows us to take the hard work of the # hand-coded unsanitized sql and create a sub-query that we # can use to help ensure that we can use all of the # ActiveRecord goodness of scopes (e.g., # limited_column_select) and eager includes. - finalized_results = Article.where( - Article.arel_table[:id].in( - Arel.sql( - Article.sanitize_sql(unsanitized_sub_sql), - ), - ), - ).limited_column_select.includes(top_comments: :user) - final_order_logic(finalized_results) + Article.joins(join_fragment) + .limited_column_select + .includes(top_comments: :user) + .order("article_relevancies.relevancy_score DESC, articles.published_at DESC") end # rubocop:enable Layout/LineLength @@ -437,10 +438,6 @@ module Articles private - def final_order_logic(articles) - articles.order(Arel.sql("RANDOM() ^ (1.0 / greatest(articles.score, 0.1)) DESC")) - end - # Concatenate the required group by clauses. # # @return [String] @@ -459,12 +456,12 @@ module Articles omit_article_ids: omit_article_ids, ) <<~THE_SQL_STATEMENT - SELECT articles.id + SELECT articles.id, (#{relevance_score_components_as_sql}) as relevancy_score FROM articles #{joins_clauses_as_sql} WHERE #{where_clause} - GROUP BY articles.id - ORDER BY (#{relevance_score_components_as_sql}) DESC, + GROUP BY #{group_by_fields_as_sql} + ORDER BY relevancy_score DESC, articles.published_at DESC #{offset_and_limit_clause(offset: offset, limit: limit)} THE_SQL_STATEMENT @@ -479,12 +476,12 @@ module Articles omit_article_ids: omit_article_ids, ) <<~THE_SQL_STATEMENT - SELECT articles.id + SELECT articles.id, (#{relevance_score_components_as_sql}) as relevancy_score FROM articles #{joins_clauses_as_sql} WHERE #{where_clause} GROUP BY #{group_by_fields_as_sql} - ORDER BY (#{relevance_score_components_as_sql}) DESC, + ORDER BY relevancy_score DESC, articles.published_at DESC #{offset_and_limit_clause(offset: offset, limit: limit)} THE_SQL_STATEMENT @@ -584,9 +581,6 @@ module Articles # then we'll use the default configuration. scoring_config = default_config unless scoring_config.is_a?(Hash) - # Change an alement of config via a/b test strategy - scoring_config = inject_config_ab_test(valid_method_name, scoring_config) - # This scoring method requires a group by clause. @group_by_fields << default_config[:group_by] if default_config.key?(:group_by) @@ -611,29 +605,6 @@ module Articles end end - def inject_config_ab_test(valid_method_name, scoring_config) - return scoring_config unless valid_method_name == :daily_decay_factor # Only proceed on this one factor - return scoring_config if @strategy == AbExperiment::ORIGINAL_VARIANT # Don't proceed if not testing new strategy - - # Rewards comment count with slightly more weight up to 10 comments. - # Testing two case weights beyond what we currently have - scoring_config[:cases] = case @strategy - when "slightly_more_recent_articles" - [[0, 1], [1, 0.98], [2, 0.975], - [3, 0.97], [4, 0.965], [5, 0.96], - [6, 0.955], [7, 0.95], [8, 0.945], - [9, 0.94], [10, 0.935], [11, 0.93], - [12, 0.925], [13, 0.92], [14, 0.915]] - else # much_more_recent_articles - [[0, 1], [1, 0.975], [2, 0.965], - [3, 0.955], [4, 0.945], [5, 0.935], - [6, 0.925], [7, 0.915], [8, 0.905], - [9, 0.895], [10, 0.885], [11, 0.875], - [12, 0.865], [13, 0.855], [14, 0.845]] - end - scoring_config - end - # Responsible for transforming the :clause, :cases, and # :fallback into a SQL fragment that we can use to multiply with # the other SQL fragments. diff --git a/config/field_test.yml b/config/field_test.yml index 3ae032f63..47db80295 100644 --- a/config/field_test.yml +++ b/config/field_test.yml @@ -5,15 +5,11 @@ experiments: # Do not change the goals unless you intend to also change the corresponding code in # app/workers/users/record_field_test_event_worker.rb - feed_strategy_round_6: + feed_strategy_round_7: variants: - original - - slightly_more_recent_articles - - much_more_recent_articles weights: - - 80 - - 10 - - 10 + - 100 goals: - user_creates_comment - user_creates_comment_on_at_least_four_different_days_within_a_week