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

Adjusting feed query to establish a new incumbent (#17308)

Browse source 
This commit modifies two things:

1.  The sort order of the feed.
2.  Locking in the next incumbent for the feed.

In consultation with Jamie regarding the SQL, I've switched from a WHERE
IN type clause to creating a JOIN.  This allows for the calculated
relevancy score to become a sortable value for the feed.

Second the incumbent feed is a slight modification of the past winner;
based on the results of a long running experiment.  This adjustment is
per conversations with product.

The past experiment ran since early February, so it's time to retire it,
regroup and move forward.

Closes forem/forem#17307
This commit is contained in:
Jeremy Friesen 2022-04-19 09:10:00 -04:00 • committed by GitHub
parent b4b7d4bd39
commit aebde42730
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 21 additions and 54 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

67
app/services/articles/feeds/weighted_query_strategy.rb
View file

@ -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.

8
config/field_test.yml
View file

@ -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