Add implicit points to follows to improve feed relevancy (in all algorithms) (#11513)

* Fix menu dropdown disable issue

* Initial base work

* Add real algorithm

* Add working object and test

* Fix schema

* Fix schema

* Fix schema

* Finalize

* Add update script and explict_points

* Proper implicit/explicit points

* Fix tests

* Add docs

* Fix tests

* Fix data update script test

* Fix data update script test

* Fix typo

* Improve efficiency and add more logic

* Changed to find_each

* Change to bulk insert

* Update tests and refactor logic

* Various improvements

* Fix typo

* Fix tests

* Remove extra line

* Fix tests

* Fix test typo

* Cache site-wide tag names

app/controllers/follows_controller.rb

@@ -82,7 +82,8 @@ class FollowsController < ApplicationController
   def update
     @follow = Follow.find(params[:id])
     authorize @follow
-    redirect_to "/dashboard/following" if @follow.update(follow_params)
+    @follow.explicit_points = follow_params[:explicit_points]
+    redirect_to dashboard_following_path if @follow.save
   end
 
   private

app/models/follow.rb

@@ -31,6 +31,7 @@ class Follow < ApplicationRecord
   after_create :send_email_notification
   before_destroy :modify_chat_channel_status
   after_save :touch_follower
+  before_save :calculate_points
   after_create_commit :create_chat_channel
 
   validates :blocked, inclusion: { in: [true, false] }
@@ -46,6 +47,10 @@ class Follow < ApplicationRecord
 
   private
 
+  def calculate_points
+    self.points = explicit_points + implicit_points
+  end
+
   def touch_follower
     follower.touch(:updated_at, :last_followed_at)
   end

app/models/reaction.rb

@@ -91,7 +91,7 @@ class Reaction < ApplicationRecord
   private
 
   def update_reactable
-    Reactions::UpdateReactableWorker.perform_async(id)
+    Reactions::UpdateRelevantScoresWorker.perform_async(id)
   end
 
   def bust_reactable_cache
@@ -107,7 +107,7 @@ class Reaction < ApplicationRecord
   end
 
   def update_reactable_without_delay
-    Reactions::UpdateReactableWorker.new.perform(id)
+    Reactions::UpdateRelevantScoresWorker.new.perform(id)
   end
 
   def reading_time

app/policies/follow_policy.rb

@@ -1,4 +1,6 @@
 class FollowPolicy < ApplicationPolicy
+  PERMITTED_ATTRIBUTES = %i[explicit_points].freeze
+
   def create?
     !user_is_banned?
   end
@@ -8,7 +10,7 @@ class FollowPolicy < ApplicationPolicy
   end
 
   def permitted_attributes
-    %i[points]
+    PERMITTED_ATTRIBUTES
   end
 
   private

app/views/dashboards/following_tags.html.erb

@@ -25,12 +25,12 @@
             <% tag = follow.followable %>
             <% if tag %>
               <% color = HexComparer.new([tag.bg_color_hex || "#0000000", tag.text_color_hex || "#ffffff"]).brightness(0.8) %>
-              <div class="crayons-card branded-2 p-4 m:p-6 m:pt-4 flex flex-col single-article break-word content-center <% if follow.points < 0 %>opacity-75<% end %>" style="border-top-color: <%= color %>;" id="follows-<%= follow.id %>">
+              <div class="crayons-card branded-2 p-4 m:p-6 m:pt-4 flex flex-col single-article break-word content-center <% if follow.explicit_points < 0 %>opacity-75<% end %>" style="border-top-color: <%= color %>;" id="follows-<%= follow.id %>">
                 <h3 class="s:mb-1 -ml-1 p-0 fw-medium">
                   <a href="/t/<%= tag.name %>" class="crayons-tag crayons-tag--l">
                     <span class="crayons-tag__prefix">#</span><%= tag.name %>
                   </a>
-                  <% if follow.points < 0 %>
+                  <% if follow.explicit_points < 0 %>
                     <span class="crayons-indicator crayons-indicator--critical crayons-indicator--outlined" title="This tag has negative follow weight">Anti-follow</span>
                   <% end %>
                 </h3>
@@ -40,7 +40,7 @@
                 </p>
 
                 <%= form_for(follow, html: { class: "flex items-right w-100" }) do |f| %>
-                  <%= f.number_field(:points, step: :any, required: true, class: "crayons-textfield grow-1 fs-s inline-block w-75") %>
+                  <%= f.number_field(:explicit_points, step: :any, required: true, class: "crayons-textfield grow-1 fs-s inline-block w-75") %>
                   <button type="submit" class="crayons-btn crayons-btn--ghost crayons-btn--s inline-block ml-2" name="commit" style="min-width:60px;">Save</button>
                 <% end %>
               </div>

app/workers/follows/update_points_worker.rb

@@ -0,0 +1,72 @@
+module Follows
+  class UpdatePointsWorker
+    include Sidekiq::Worker
+    sidekiq_options queue: :low_priority, retry: 10
+
+    def perform(article_id, user_id)
+      article = Article.find_by(id: article_id)
+      user = User.find_by(id: user_id)
+      return unless article && user
+
+      adjust_other_tag_follows_of_user(user.id)
+      followed_tag_names = user.cached_followed_tag_names
+      article.decorate.cached_tag_list_array.each do |tag_name|
+        if followed_tag_names.include?(tag_name)
+          recalculate_tag_follow_points(tag_name, user)
+        end
+      end
+    end
+
+    def recalculate_tag_follow_points(tag_name, user)
+      tag = Tag.find_by(name: tag_name)
+      follow = Follow.follower_tag(user.id).where(followable_id: tag.id).last
+
+      follow.implicit_points = calculate_implicit_points(tag, user)
+      follow.save
+    end
+
+    def calculate_implicit_points(tag, user)
+      last_100_reactable_ids = user.reactions.where(reactable_type: "Article", points: 0..)
+        .pluck(:reactable_id).last(100)
+      last_100_long_page_view_article_ids = user.page_views.where(time_tracked_in_seconds: 45..)
+        .pluck(:article_id).last(100)
+      articles = Article.where(id: last_100_reactable_ids + last_100_long_page_view_article_ids)
+      tags = articles.pluck(:cached_tag_list).flat_map { |list| list.split(", ") }
+      occurrences = tags.count(tag.name)
+      bonus = inverse_popularity_bonus(tag)
+      Math.log(occurrences + bonus + 1) # +1 is purelt to avoid log(0) => -infinity
+    end
+
+    def adjust_other_tag_follows_of_user(user_id)
+      # As we bump one follow up, we should also give a slight penalty
+      # to other follows to ensure re-balancing of overall points
+      # This will help stale tags fade after a temporary interest bump
+
+      # 0.98 is used to ensure this is "a very small amount"
+      # And percentage seems to makes sense, as it will be relative to
+      # size of current points.
+
+      # That number could be adjusted at any point if we have reason to
+      # believe it is too much or too little.
+      Follow.follower_tag(user_id).order(Arel.sql("RANDOM()")).limit(5).each do |follow|
+        follow.update_column(:points, (follow.points * 0.98))
+      end
+    end
+
+    def inverse_popularity_bonus(tag)
+      # Let's give a bonus to "less popular" tags on the platform
+      # To help balance the weight of popular topic.
+      # On DEV, javascript has way more taggings than rust, for example, so we can
+      # help rust outweigh JS in this calculation slightly.
+      # The bonus will be applied to the logarithmic scale, as to blunt any outsized impact.
+      top_100_tag_names = cached_app_wide_top_tag_names
+      top_100_tag_names.index(tag.name) || (top_100_tag_names.size * 1.5)
+    end
+
+    def cached_app_wide_top_tag_names
+      Rails.cache.fetch("top-100-tags") do
+        Tag.order(hotness_score: :desc).limit(100).pluck(:name)
+      end
+    end
+  end
+end

app/workers/reactions/update_relevant_scores_worker.rb

@@ -1,5 +1,5 @@
 module Reactions
-  class UpdateReactableWorker
+  class UpdateRelevantScoresWorker
     include Sidekiq::Worker
 
     sidekiq_options queue: :high_priority, retry: 10
@@ -10,6 +10,9 @@ module Reactions
 
       reaction.reactable.touch_by_reaction if reaction.reactable.respond_to?(:touch_by_reaction)
       reaction.reactable.sync_reactions_count if rand(6) == 1 && reaction.reactable.respond_to?(:sync_reactions_count)
+      return unless reaction.reactable_type == "Article" && Reaction::PUBLIC_CATEGORIES.include?(reaction.category)
+
+      Follows::UpdatePointsWorker.perform_async(reaction.reactable_id, reaction.user_id)
     end
   end
 end

db/migrate/20201119153512_add_explicit_and_implicit_follow_points.rb

@@ -0,0 +1,6 @@
+class AddExplicitAndImplicitFollowPoints < ActiveRecord::Migration[6.0]
+  def change
+    add_column :follows, :explicit_points, :float, default: 1.0 # 1 is equivalent to the current default set in "score"
+    add_column :follows, :implicit_points, :float, default: 0.0
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2020_11_14_151157) do
+ActiveRecord::Schema.define(version: 2020_11_19_153512) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "citext"
@@ -560,10 +560,12 @@ ActiveRecord::Schema.define(version: 2020_11_14_151157) do
   create_table "follows", force: :cascade do |t|
     t.boolean "blocked", default: false, null: false
     t.datetime "created_at"
+    t.float "explicit_points", default: 1.0
     t.bigint "followable_id", null: false
     t.string "followable_type", null: false
     t.bigint "follower_id", null: false
     t.string "follower_type", null: false
+    t.float "implicit_points", default: 0.0
     t.float "points", default: 1.0
     t.string "subscription_status", default: "all_articles", null: false
     t.datetime "updated_at"

docs/technical-overview/architecture.md

@@ -188,6 +188,11 @@ article in the user's reading list.
 How a user keeps track of the tags, users, or articles they care about. Follows
 impact a user's home feed and notifications.
 
+Follows can have a "score" which indicates how much a user wants to see the element
+in their feed. Currently we only calculate these for tag follows, but it could be
+expanded to users. The user can set an "explicit" score, and the system also calculates
+an "implicit" score based on their activity.
+
 ## Roles
 
 Through the "rolify" gem, users can have roles like "admin", etc. A role can

lib/data_update_scripts/20201120001727_populate_explicit_follow_points.rb

@@ -0,0 +1,7 @@
+module DataUpdateScripts
+  class PopulateExplicitFollowPoints
+    def run
+      Follow.connection.execute('UPDATE "follows" SET "explicit_points" = "points" WHERE points != 1')
+    end
+  end
+end

spec/decorators/user_decorator_spec.rb

@@ -47,7 +47,7 @@ RSpec.describe UserDecorator, type: :decorator do
 
     it "returns adjusted points for tag" do
       follow = saved_user.follow(tag1)
-      follow.update(points: 0.1)
+      follow.update(explicit_points: 0.1)
       expect(saved_user.decorate.cached_followed_tags.first.points).to eq(0.1)
     end
   end

spec/lib/data_update_scripts/populate_explicit_follow_points_spec.rb

@@ -0,0 +1,25 @@
+require "rails_helper"
+require Rails.root.join(
+  "lib/data_update_scripts/20201120001727_populate_explicit_follow_points.rb",
+)
+
+describe DataUpdateScripts::PopulateExplicitFollowPoints do
+  it "updates follows that had points to having explicit points", :aggregate_failures do
+    follow = create(:follow)
+    second_follow = create(:follow)
+    third_follow = create(:follow)
+    fourth_follow = create(:follow)
+
+    follow.update_column(:points, 3)
+    second_follow.update_column(:points, 7)
+    third_follow.update_column(:points, 1)
+    fourth_follow.update_column(:points, 0.5)
+    expect(follow.explicit_points).to eq(1.0)
+    expect(second_follow.explicit_points).to eq(1.0)
+    described_class.new.run
+    expect(follow.reload.explicit_points).to eq(3)
+    expect(second_follow.reload.explicit_points).to eq(7)
+    expect(third_follow.reload.explicit_points).to eq(1)
+    expect(fourth_follow.reload.explicit_points).to eq(0.5)
+  end
+end

spec/models/follow_spec.rb

@@ -2,6 +2,7 @@ require "rails_helper"
 
 RSpec.describe Follow, type: :model do
   let(:user) { create(:user) }
+  let(:tag) { create(:tag) }
   let(:user_2) { create(:user) }
 
   describe "validations" do
@@ -20,6 +21,15 @@ RSpec.describe Follow, type: :model do
     expect(user.following?(user_2)).to eq(true)
   end
 
+  it "calculates points with explicit and implicit combined" do
+    user.follow(tag)
+    follow = described_class.last
+    follow.explicit_points = 2.0
+    follow.implicit_points = 3.0
+    follow.save
+    expect(follow.points).to eq(5.0)
+  end
+
   context "when enqueuing jobs" do
     it "enqueues create channel job" do
       expect do

spec/requests/follows_update_spec.rb

@@ -12,7 +12,8 @@ RSpec.describe "Following/Unfollowing", type: :request do
   describe "PUT follows/:id" do
     it "updates follow points" do
       user.follow(tag)
-      put "/follows/#{Follow.last.id}", params: { follow: { points: 3.0 } }
+      put "/follows/#{Follow.last.id}", params: { follow: { explicit_points: 3.0 } }
+      expect(Follow.last.explicit_points).to eq(3.0)
       expect(Follow.last.points).to eq(3.0)
     end
 

spec/services/articles/feeds/large_forem_experimental_spec.rb

@@ -237,7 +237,7 @@ RSpec.describe Articles::Feeds::LargeForemExperimental, type: :service do
       before do
         user.follow(tag)
         user.save
-        user.follows.last.update(points: 2)
+        user.follows.last.update(explicit_points: 2)
       end
 
       it "returns the followed tag point value" do
@@ -253,7 +253,7 @@ RSpec.describe Articles::Feeds::LargeForemExperimental, type: :service do
         user.follow(tag)
         user.follow(tag2)
         user.save
-        user.follows.each { |follow| follow.update(points: 2) }
+        user.follows.each { |follow| follow.update(explicit_points: 2) }
       end
 
       it "returns the sum of followed tag point values" do

spec/support/sidekiq_test_helpers.rb

@@ -47,6 +47,20 @@ module SidekiqTestHelpers
     expect(matching_job).to be_present, "No enqueued job found with #{expected}"
   end
 
+  def sidekiq_assert_not_enqueued_with(job: nil, args: nil, at: nil, queue: nil)
+    expected = { job: job, args: args, at: at, queue: queue }.compact
+    expected_args = Utils.prepare_args(expected)
+
+    yield
+
+    # check there's at least one job with the given args
+    matching_job = job.jobs.detect do |queued_job|
+      expected_args.all? { |key, value| value == queued_job[key] }
+    end
+
+    expect(matching_job).not_to be_present, "Job unexpectedly found with #{expected}"
+  end
+
   # Asserts that no jobs have been enqueued.
   # see <https://api.rubyonrails.org/v5.2/classes/ActiveJob/TestHelper.html#method-i-assert_no_enqueued_jobs>
   def sidekiq_assert_no_enqueued_jobs(only: nil, except: nil, &block)

spec/system/dashboards/user_scrolls_down_dashboard_follows_spec.rb

@@ -45,12 +45,12 @@ RSpec.describe "Infinite scroll on dashboard", type: :system, js: true do
     it "updates a tag point value" do
       last_div = page.all('div[id^="follows"]').last
       within last_div do
-        fill_in "follow_points", with: 10.0
+        fill_in "follow_explicit_points", with: 10.0
         click_button "commit"
       end
       first_div = page.find('div[id^="follows"]', match: :first)
       within first_div do
-        expect(page).to have_field("follow_points", with: 10.0)
+        expect(page).to have_field("follow_explicit_points", with: 10.0)
       end
     end
   end

spec/system/homepage/user_visits_homepage_spec.rb

@@ -108,7 +108,7 @@ RSpec.describe "User visits a homepage", type: :system do
         find("body")["data-user"]
 
         within("#sidebar-nav-followed-tags") do
-          expect(all(".crayons-link--block").map(&:text)).to eq(%w[#javascript #go #ruby])
+          expect(all(".crayons-link--block").map(&:text).sort).to eq(%w[#javascript #go #ruby].sort)
         end
       end
     end

spec/workers/follows/update_points_worker_spec.rb

@@ -0,0 +1,78 @@
+require "rails_helper"
+
+RSpec.describe Follows::UpdatePointsWorker, type: :worker do
+  include_examples "#enqueues_on_correct_queue", "low_priority", 1
+
+  describe "#perform" do
+    let(:worker) { subject }
+
+    let(:user) { create(:user) }
+    let(:tag) { create(:tag, name: "tag") }
+    let(:second_tag) { create(:tag, name: "secondtag") }
+    let(:third_tag) { create(:tag, name: "thirdtag") }
+    let(:article) { create(:article, tags: [tag.name]) }
+    let(:second_article) { create(:article, tags: [tag.name]) }
+    let(:reaction) { create(:reaction, reactable: article, user: user) }
+    let(:page_view) { create(:page_view, user: user, article: article, time_tracked_in_seconds: 100) }
+
+    before do
+      user.follow(second_tag)
+      user.follow(tag)
+    end
+
+    it "calculates scores" do
+      follow = Follow.last
+      follow.update_column(:explicit_points, 2.2)
+      worker.perform(reaction.reactable_id, reaction.user_id)
+      follow.reload
+      expect(follow.implicit_points).to be > 0
+      expect(follow.reload.points.round(2)).to eq (follow.implicit_points + follow.explicit_points).round(2)
+    end
+
+    it "has higher score with more long page views" do
+      follow = Follow.last
+      worker.perform(reaction.reactable_id, reaction.user_id)
+      follow.reload
+      first_implicit_score = follow.implicit_points
+
+      create(:page_view, user: user, article: second_article, time_tracked_in_seconds: 100)
+
+      worker.perform(reaction.reactable_id, reaction.user_id)
+      follow.reload
+      expect(follow.implicit_points).to be > first_implicit_score
+    end
+
+    it "has higher score with more reactions" do
+      follow = Follow.last
+      worker.perform(reaction.reactable_id, reaction.user_id)
+      follow.reload
+      first_implicit_score = follow.implicit_points
+
+      create(:reaction, reactable: second_article, user: user)
+
+      worker.perform(reaction.reactable_id, reaction.user_id)
+      follow.reload
+      expect(follow.implicit_points).to be > first_implicit_score
+    end
+
+    it "bumps down tag follow points not included in this calc" do
+      follow = Follow.first
+      worker.perform(reaction.reactable_id, reaction.user_id)
+      expect(follow.reload.points.round(2)).to eq(0.98)
+    end
+
+    it "applies inverse bonus to slightly penalize more popular tags" do
+      follow = Follow.last
+      tag.update_column(:hotness_score, 1000)
+      second_tag.update_column(:hotness_score, 100)
+      worker.perform(reaction.reactable_id, reaction.user_id)
+      follow.reload
+      original_points = follow.points
+      tag.update_column(:hotness_score, 50)
+      tag.reload
+      worker.perform(reaction.reactable_id, reaction.user_id)
+
+      expect(follow.reload.points).to be > original_points # should be higher because tag is now less popular
+    end
+  end
+end

spec/workers/reactions/update_reactable_worker_spec.rb

@@ -1,30 +0,0 @@
-require "rails_helper"
-
-RSpec.describe Reactions::UpdateReactableWorker, type: :worker do
-  describe "#perform" do
-    let(:article) { create(:article) }
-    let(:reaction) { create(:reaction, reactable: article) }
-    let(:comment) { create(:comment, commentable: article) }
-    let(:comment_reaction) { create(:reaction, reactable: comment) }
-    let(:worker) { subject }
-
-    it " updates the reactable Article" do
-      sidekiq_assert_enqueued_with(job: Articles::ScoreCalcWorker) do
-        worker.perform(reaction.id)
-      end
-    end
-
-    it " updates the reactable Comment" do
-      updated_at = 1.day.ago
-      comment.update_columns(updated_at: updated_at)
-      worker.perform(comment_reaction.id)
-      expect(comment.reload.updated_at).to be > updated_at
-    end
-
-    it " doesn't fail if a reaction doesn't exist" do
-      expect do
-        worker.perform(Reaction.maximum(:id).to_i + 1)
-      end.not_to raise_error
-    end
-  end
-end

spec/workers/reactions/update_relevant_scores_worker_spec.rb

@@ -0,0 +1,49 @@
+require "rails_helper"
+
+RSpec.describe Reactions::UpdateRelevantScoresWorker, type: :worker do
+  describe "#perform" do
+    let(:article) { create(:article) }
+    let(:reaction) { create(:reaction, reactable: article) }
+    let(:comment) { create(:comment, commentable: article) }
+    let(:comment_reaction) { create(:reaction, reactable: comment) }
+    let(:worker) { subject }
+
+    it "kicks off point update if article" do
+      sidekiq_assert_enqueued_with(job: Follows::UpdatePointsWorker) do
+        worker.perform(reaction.id)
+      end
+    end
+
+    it "does not kick off points updater if not comment reaction" do
+      sidekiq_assert_not_enqueued_with(job: Follows::UpdatePointsWorker) do
+        worker.perform(comment_reaction.id)
+      end
+    end
+
+    it "does not kick off points updater if reaction is non-public" do
+      reaction.update_column(:category, "vomit")
+      sidekiq_assert_not_enqueued_with(job: Follows::UpdatePointsWorker) do
+        worker.perform(reaction.id)
+      end
+    end
+
+    it "updates the reactable Article" do
+      sidekiq_assert_enqueued_with(job: Articles::ScoreCalcWorker) do
+        worker.perform(reaction.id)
+      end
+    end
+
+    it "updates the reactable Comment" do
+      updated_at = 1.day.ago
+      comment.update_columns(updated_at: updated_at)
+      worker.perform(comment_reaction.id)
+      expect(comment.reload.updated_at).to be > updated_at
+    end
+
+    it "doesn't fail if a reaction doesn't exist" do
+      expect do
+        worker.perform(Reaction.maximum(:id).to_i + 1)
+      end.not_to raise_error
+    end
+  end
+end