Extracting a common calculator for feed weights (#15199)

Prior to this commit, there existed duplicate logic between two of the
primary user feeds.  This refactor introduces a new object which is
solely concerned with calculating scores to add to an article base score.

With this commit, we can introduce A/B testing by changing passing
different config values to `ArticleScoreCalculatorForUser.new`.

My suspicion is that for some of this, we might be able to better
leverage the database via select statements and SQL sums.  However, that
is presently outside of what I'm prepared to tackle.
This commit is contained in:
Jeremy Friesen 2021-10-27 04:45:53 -04:00 committed by GitHub
parent 4ec93a4b5f
commit ed78f8f479
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 311 additions and 264 deletions

View file

@ -0,0 +1,89 @@
module Articles
module Feeds
# @api private
#
# The purpose of this class is to encapsulate how we apply scores
# to an article.
#
# @see Articles::Feeds::Basic
# @see Articles::Feeds::LargeForemExperimental
class ArticleScoreCalculatorForUser
# This constant defines some of the levers that we use to help
# calculate an article's score.
DEFAULT_CONFIGURATION = {
comment_weight: 0.2,
xp_level_weight: 1,
default_user_xp_level: 5,
followed_user_score: 1,
not_followed_user_score: 0,
nil_user_tag_score: 0,
followed_tag_weight: 1,
not_followed_tag_score: 0,
followed_org_score: 1,
not_followed_org_score: 0
}.freeze
# @param user [User] the user for whom we're calculating the article score.
# @param config [Hash<Symbol,(Integer|Float)>] exposes the means
# for overriding the default configuration values
#
# @see DEFAULT_CONFIGURATION
def initialize(user:, config: {})
@user = user
DEFAULT_CONFIGURATION.each_pair do |key, value|
instance_variable_set("@#{key}", config.fetch(key, value))
end
end
# @api private
def score_followed_user(article)
user_following_users_ids.include?(article.user_id) ? @followed_user_score : @not_followed_user_score
end
# @api private
def score_followed_tags(article)
return @nil_user_tag_score unless @user
article_tags = article.decorate.cached_tag_list_array
user_followed_tags.sum do |tag|
article_tags.include?(tag.name) ? tag.points * @followed_tag_weight : @not_followed_tag_score
end
end
# @api private
def score_followed_organization(article)
return @not_followed_org_score unless article.organization_id?
user_following_org_ids.include?(article.organization_id) ? @followed_org_score : @not_followed_org_score
end
# @api private
def score_experience_level(article)
user_experience_level = @user&.setting&.experience_level || @default_user_xp_level
# Calculate the distance between the user's experience level
# and that of the article's experience level.
- (((article.experience_level_rating - user_experience_level).abs / 2) * @xp_level_weight)
end
# @api private
def score_comments(article)
article.comments_count * @comment_weight
end
private
def user_followed_tags
@user_followed_tags ||= (@user&.decorate&.cached_followed_tags || [])
end
def user_following_org_ids
@user_following_org_ids ||= (@user&.cached_following_organizations_ids || [])
end
def user_following_users_ids
@user_following_users_ids ||= (@user&.cached_following_users_ids || [])
end
end
end
end

View file

@ -6,6 +6,7 @@ module Articles
@number_of_articles = number_of_articles
@page = page
@tag = tag
@article_score_applicator = Articles::Feeds::ArticleScoreCalculatorForUser.new(user: @user)
end
def feed
@ -18,29 +19,21 @@ module Articles
articles = articles.where.not(user_id: UserBlock.cached_blocked_ids_for_blocker(@user.id))
articles.sort_by.with_index do |article, index|
article_tags = article.decorate.cached_tag_list_array
tag_score = user_followed_tags.sum do |tag|
article_tags.include?(tag.name) ? tag.points : 0
end
user_score = user_following_users_ids.include?(article.user_id) ? 1 : 0
org_score = user_following_org_ids.include?(article.organization_id) ? 1 : 0
tag_score = score_followed_tags(article)
user_score = score_followed_user(article)
org_score = score_followed_organization(article)
# NOTE: Not quite understanding the purpose of the `-
# index`. My guess is that it helps reduce the impact of the
# hotness score on the sort order.
tag_score + org_score + user_score - index
end.reverse!
end
private
def user_followed_tags
@user_followed_tags ||= (@user&.decorate&.cached_followed_tags || [])
end
def user_following_org_ids
@user_following_org_ids ||= (@user&.cached_following_organizations_ids || [])
end
def user_following_users_ids
@user_following_users_ids ||= (@user&.cached_following_users_ids || [])
end
delegate(:score_followed_tags,
:score_followed_user,
:score_followed_organization,
to: :@article_score_applicator)
end
end
end

View file

@ -1,15 +1,12 @@
module Articles
module Feeds
class LargeForemExperimental
DEFAULT_USER_XP_LEVEL = 5
def initialize(user: nil, number_of_articles: 50, page: 1, tag: nil)
@user = user
@number_of_articles = number_of_articles
@page = page
@tag = tag
@tag_weight = 1 # default weight tags play in rankings
@comment_weight = 0.2 # default weight comments play in rankings
@xp_level_weight = 1 # default weight for user experience level
@article_score_applicator = Articles::Feeds::ArticleScoreCalculatorForUser.new(user: user)
end
def default_home_feed(user_signed_in: false)
@ -58,37 +55,12 @@ module Articles
article_points
end
# @api private
def score_followed_user(article, follow_user_score: 1, not_followed_user_score: 0)
user_following_users_ids.include?(article.user_id) ? follow_user_score : not_followed_user_score
end
# @api private
def score_followed_tags(article, nil_user_tag_score: 0, followed_tag_weight: @tag_weight, unfollowed_tag_score: 0)
return nil_user_tag_score unless @user
article_tags = article.decorate.cached_tag_list_array
user_followed_tags.sum do |tag|
article_tags.include?(tag.name) ? tag.points * followed_tag_weight : unfollowed_tag_score
end
end
# @api private
def score_followed_organization(article, followed_org_score: 1, not_followed_org_score: 0)
return not_followed_org_score unless article.organization_id?
user_following_org_ids.include?(article.organization_id) ? followed_org_score : not_followed_org_score
end
# @api private
def score_experience_level(article, xp_level_weight: @xp_level_weight)
- (((article.experience_level_rating - user_experience_level).abs / 2) * xp_level_weight)
end
# @api private
def score_comments(article, comment_weight: @comment_weight)
article.comments_count * comment_weight
end
delegate(:score_followed_user,
:score_followed_tags,
:score_followed_organization,
:score_experience_level,
:score_comments,
to: :@article_score_applicator)
# @api private
def globally_hot_articles(user_signed_in, article_score_threshold: -15, min_rand_limit: 15, max_rand_limit: 80)
@ -113,10 +85,6 @@ module Articles
private
def user_experience_level
@user_experience_level ||= @user&.setting&.experience_level || DEFAULT_USER_XP_LEVEL
end
def experimental_hot_story_grab
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)
@ -125,18 +93,6 @@ module Articles
.order(score: :desc)
end
def user_followed_tags
@user_followed_tags ||= (@user&.decorate&.cached_followed_tags || [])
end
def user_following_org_ids
@user_following_org_ids ||= (@user&.cached_following_organizations_ids || [])
end
def user_following_users_ids
@user_following_users_ids ||= (@user&.cached_following_users_ids || [])
end
def first_quarter(array)
array[0...(array.length / 4)]
end

View file

@ -0,0 +1,201 @@
require "rails_helper"
RSpec.describe Articles::Feeds::ArticleScoreCalculatorForUser, type: :service do
let(:user) { create(:user) }
let!(:article) { create(:article) }
let(:applicator) { described_class.new(user: user) }
describe "#score_followed_user" do
context "when article is written by a followed user" do
before { user.follow(article.user) }
it "returns a score of 1" do
expect(applicator.score_followed_user(article)).to eq 1
end
end
context "when article is not written by a followed user" do
it "returns a score of 0" do
expect(applicator.score_followed_user(article)).to eq 0
end
end
end
describe "#score_followed_organization" do
let(:organization) { create(:organization) }
let(:article) { create(:article, organization: organization) }
context "when article is from a followed organization" do
before { user.follow(organization) }
it "returns a score of 1" do
expect(applicator.score_followed_organization(article)).to eq 1
end
end
context "when article is not from a followed organization" do
it "returns a score of 0" do
expect(applicator.score_followed_organization(article)).to eq 0
end
end
context "when article has no organization" do
let(:article) { create(:article) }
it "returns a score of 0" do
expect(applicator.score_followed_organization(article)).to eq 0
end
end
end
describe "#score_followed_tags" do
let(:tag) { create(:tag) }
let(:unfollowed_tag) { create(:tag) }
context "when article includes a followed tag" do
let(:article) { create(:article, tags: tag.name) }
before do
user.follow(tag)
user.save
user.follows.last.update(explicit_points: 2)
end
it "returns the followed tag point value" do
expect(applicator.score_followed_tags(article)).to eq 2
end
end
context "when article includes multiple followed tags" do
let(:tag2) { create(:tag) }
let(:article) { create(:article, tags: "#{tag.name}, #{tag2.name}") }
before do
user.follow(tag)
user.follow(tag2)
user.save
user.follows.each { |follow| follow.update(explicit_points: 2) }
end
it "returns the sum of followed tag point values" do
expect(applicator.score_followed_tags(article)).to eq 4
end
end
context "when article includes an unfollowed tag" do
let(:article) { create(:article, tags: "#{tag.name}, #{unfollowed_tag.name}") }
before do
user.follow(tag)
user.save
end
it "doesn't score the unfollowed tag" do
expect(applicator.score_followed_tags(article)).to eq 1
end
end
context "when article doesn't include any followed tags" do
let(:article) { create(:article, tags: unfollowed_tag.name) }
it "returns 0" do
expect(applicator.score_followed_tags(article)).to eq 0
end
end
context "when user doesn't follow any tags" do
it "returns 0" do
expect(user.cached_followed_tag_names).to be_empty
expect(applicator.score_followed_tags(article)).to eq 0
end
end
end
describe "#score_experience_level" do
let(:article) { create(:article, experience_level_rating: 7) }
context "when user has a further experience level" do
let(:user) { create(:user) }
before do
user.setting.update(experience_level: 1)
end
it "returns negative of (absolute value of the difference between article and user experience) divided by 2" do
expect(applicator.score_experience_level(article)).to eq(-3)
end
it "returns proper negative when fractional" do
article.experience_level_rating = 8
expect(applicator.score_experience_level(article)).to eq(-3.5)
end
end
context "when user has a closer experience level" do
let(:user) { create(:user) }
before do
user.setting.update(experience_level: 9)
end
it "returns negative of (absolute value of the difference between article and user experience) divided by 2" do
expect(applicator.score_experience_level(article)).to eq(-1)
end
end
context "when the user does not have an experience level set" do
let(:user) { create(:user) }
before do
user.setting.update(experience_level: nil)
end
it "uses a value of 5 for user experience level" do
expect(applicator.score_experience_level(article)).to eq(-1)
end
end
end
describe "#score_comments" do
let(:applicator) { described_class.new(user: user, config: { comment_weight: 1 }) }
let(:article_with_one_comment) { create(:article) }
let(:article_with_five_comments) { create(:article) }
before do
create(:comment, user: user, commentable: article_with_one_comment)
create_list(:comment, 5, user: user, commentable: article_with_five_comments)
article_with_one_comment.update_score
article_with_five_comments.update_score
article_with_one_comment.reload
article_with_five_comments.reload
end
context "when comment_weight is default of 0" do
it "returns 0 for uncommented articles" do
expect(applicator.score_comments(article)).to eq(0)
end
it "returns a multiple of the parameterized weight for articles with comments" do
expect(article_with_five_comments.comments_count).to eq(5)
expect(applicator.score_comments(article_with_five_comments)).to eq(5)
end
end
context "when comment_weight is higher than 0" do
before { applicator.instance_variable_set(:@comment_weight, 2) }
it "returns 0 for uncommented articles" do
expect(applicator.score_comments(article)).to eq(0)
end
it "returns a non-zero score for commented upon articles" do
expect(applicator.score_comments(article_with_one_comment)).to be > 0
end
it "scores article with more comments high than others" do
expect(applicator.score_comments(article_with_five_comments))
.to be > applicator.score_comments(article_with_one_comment)
end
end
end
end

View file

@ -28,10 +28,10 @@ RSpec.describe Articles::Feeds::Basic, type: :service do
let(:feed) { described_class.new(user: user, number_of_articles: 100, page: 1) }
it "returns articles with score above 0 sorted by user preference scores" do
allow(feed).to receive(:user_following_users_ids).and_return([old_story.user_id])
user.follow(old_story.user)
old_story_tag = Tag.find_by(name: unique_tag_name)
old_story_tag.update(points: 10)
allow(feed).to receive(:user_followed_tags).and_return([old_story_tag])
user.follow(old_story_tag)
result = feed.feed
expect(result.first).to eq old_story

View file

@ -132,198 +132,6 @@ RSpec.describe Articles::Feeds::LargeForemExperimental, type: :service do
end
end
describe "#score_followed_user" do
context "when article is written by a followed user" do
before { user.follow(article.user) }
it "returns a score of 1" do
expect(feed.score_followed_user(article)).to eq 1
end
end
context "when article is not written by a followed user" do
it "returns a score of 0" do
expect(feed.score_followed_user(article)).to eq 0
end
end
end
describe "#score_followed_organization" do
let(:organization) { create(:organization) }
let(:article) { create(:article, organization: organization) }
context "when article is from a followed organization" do
before { user.follow(organization) }
it "returns a score of 1" do
expect(feed.score_followed_organization(article)).to eq 1
end
end
context "when article is not from a followed organization" do
it "returns a score of 0" do
expect(feed.score_followed_organization(article)).to eq 0
end
end
context "when article has no organization" do
let(:article) { create(:article) }
it "returns a score of 0" do
expect(feed.score_followed_organization(article)).to eq 0
end
end
end
describe "#score_followed_tags" do
let(:tag) { create(:tag) }
let(:unfollowed_tag) { create(:tag) }
context "when article includes a followed tag" do
let(:article) { create(:article, tags: tag.name) }
before do
user.follow(tag)
user.save
user.follows.last.update(explicit_points: 2)
end
it "returns the followed tag point value" do
expect(feed.score_followed_tags(article)).to eq 2
end
end
context "when article includes multiple followed tags" do
let(:tag2) { create(:tag) }
let(:article) { create(:article, tags: "#{tag.name}, #{tag2.name}") }
before do
user.follow(tag)
user.follow(tag2)
user.save
user.follows.each { |follow| follow.update(explicit_points: 2) }
end
it "returns the sum of followed tag point values" do
expect(feed.score_followed_tags(article)).to eq 4
end
end
context "when article includes an unfollowed tag" do
let(:article) { create(:article, tags: "#{tag.name}, #{unfollowed_tag.name}") }
before do
user.follow(tag)
user.save
end
it "doesn't score the unfollowed tag" do
expect(feed.score_followed_tags(article)).to eq 1
end
end
context "when article doesn't include any followed tags" do
let(:article) { create(:article, tags: unfollowed_tag.name) }
it "returns 0" do
expect(feed.score_followed_tags(article)).to eq 0
end
end
context "when user doesn't follow any tags" do
it "returns 0" do
expect(user.cached_followed_tag_names).to be_empty
expect(feed.score_followed_tags(article)).to eq 0
end
end
end
describe "#score_experience_level" do
let(:article) { create(:article, experience_level_rating: 7) }
context "when user has a further experience level" do
let(:user) { create(:user) }
before do
user.setting.update(experience_level: 1)
end
it "returns negative of (absolute value of the difference between article and user experience) divided by 2" do
expect(feed.score_experience_level(article)).to eq(-3)
end
it "returns proper negative when fractional" do
article.experience_level_rating = 8
expect(feed.score_experience_level(article)).to eq(-3.5)
end
end
context "when user has a closer experience level" do
let(:user) { create(:user) }
before do
user.setting.update(experience_level: 9)
end
it "returns negative of (absolute value of the difference between article and user experience) divided by 2" do
expect(feed.score_experience_level(article)).to eq(-1)
end
end
context "when the user does not have an experience level set" do
let(:user) { create(:user) }
before do
user.setting.update(experience_level: nil)
end
it "uses a value of 5 for user experience level" do
expect(feed.score_experience_level(article)).to eq(-1)
end
end
end
describe "#score_comments" do
let(:article_with_one_comment) { create(:article) }
let(:article_with_five_comments) { create(:article) }
before do
create(:comment, user: user, commentable: article_with_one_comment)
create_list(:comment, 5, user: user, commentable: article_with_five_comments)
article_with_one_comment.update_score
article_with_five_comments.update_score
article_with_one_comment.reload
article_with_five_comments.reload
end
context "when comment_weight is default of 0" do
it "returns 0 for uncommented articles" do
expect(feed.score_comments(article, comment_weight: 1)).to eq(0)
end
it "returns a multiple of the parameterized weight for articles with comments" do
expect(article_with_five_comments.comments_count).to eq(5)
expect(feed.score_comments(article_with_five_comments, comment_weight: 1)).to eq(5)
end
end
context "when comment_weight is higher than 0" do
before { feed.instance_variable_set(:@comment_weight, 2) }
it "returns 0 for uncommented articles" do
expect(feed.score_comments(article)).to eq(0)
end
it "returns a non-zero score for commented upon articles" do
expect(feed.score_comments(article_with_one_comment)).to be > 0
end
it "scores article with more comments high than others" do
expect(feed.score_comments(article_with_five_comments)).to be > feed.score_comments(article_with_one_comment)
end
end
end
describe "#rank_and_sort_articles" do
let(:article1) { create(:article) }
let(:article2) { create(:article) }