[Cros SR] Tweak search result scores with category ranking model.

This uses the Dolphin model added in previous CLs to tweak the search result
scores. For context, see:

  https://docs.google.com/document/d/1BUHZyQ1a07laOgbxSRNNbQnTHaUPLRGjENJEJHYTtn0/edit?usp=sharing

Details:

 - Currently rankings are very conservative, with a multiplier set via a Finch
   flag.
 - ranking_item_util has been slightly refactored. This isn't strictly necessary
   for this CL, but will prevent a merge conflict with CLs from other team
   members.
 - Tests have been expanded to support testing the effect of the Dolphin model.

Bug: 931149
Change-Id: Ida824072d7553033000eeba771f97816bd71548a
Reviewed-on: https://chromium-review.googlesource.com/c/1481172
Commit-Queue: Tony Yeoman <tby@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Jia Meng <jiameng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634484}

chrome/browser/ui/app_list/app_list_client_impl.cc

@@ -102,8 +102,7 @@ void AppListClientImpl::OpenSearchResult(const std::string& result_id,
 
     // Send training signal to search controller.
     search_controller_->Train(
-        result_id,
-        app_list::RankingItemTypeFromSearchResultType(result->result_type()));
+        result_id, app_list::RankingItemTypeFromSearchResult(*result));
   }
 }
 

chrome/browser/ui/app_list/search/mixer.cc

@@ -11,7 +11,9 @@
 #include <utility>
 #include <vector>
 
+#include "ash/public/cpp/app_list/app_list_features.h"
 #include "base/macros.h"
+#include "base/metrics/field_trial_params.h"
 #include "chrome/browser/ui/app_list/app_list_model_updater.h"
 #include "chrome/browser/ui/app_list/search/chrome_search_result.h"
 #include "chrome/browser/ui/app_list/search/search_provider.h"
@@ -77,8 +79,11 @@ class Mixer::Group {
 };
 
 Mixer::Mixer(AppListModelUpdater* model_updater)
-    : model_updater_(model_updater) {}
-
+    : model_updater_(model_updater),
+      boost_coefficient_(base::GetFieldTrialParamByFeatureAsDouble(
+          app_list_features::kEnableAdaptiveResultRanker,
+          "boost_coefficient",
+          0.1)) {}
 Mixer::~Mixer() = default;
 
 size_t Mixer::AddGroup(size_t max_results, double multiplier, double boost) {
@@ -110,8 +115,23 @@ void Mixer::MixAndPublish(size_t num_max_results) {
   // result with the same ID).
   RemoveDuplicates(&results);
 
-  // TODO(https://crbug.com/931149): tweak the scores of |results| using
-  // |ranker_|.
+  // Tweak the rankings using the ranker if it exists.
+  if (app_list_features::IsAdaptiveResultRankerEnabled() && ranker_) {
+    base::flat_map<std::string, float> ranks = ranker_->Rank();
+
+    for (auto& result : results) {
+      RankingItemType type = RankingItemTypeFromSearchResult(*result.result);
+      const auto& rank_it = ranks.find(std::to_string(static_cast<int>(type)));
+      // The ranker only contains entries trained with types
+      // |RankingItemType::kFile| and |RankingItemType::kOmnibox|. This means
+      // scores for apps and answer cards will be unchanged.
+      if (rank_it != ranks.end())
+        // Ranker scores are guaranteed to be in [0,1]. But, enforce that the
+        // result of tweaking does not put the score above 3.0, as that may
+        // interfere with apps or answer cards.
+        result.score += std::min(rank_it->second * boost_coefficient_, 3.0f);
+    }
+  }
 
   std::sort(results.begin(), results.end());
 

chrome/browser/ui/app_list/search/mixer.h

@@ -88,6 +88,10 @@ class Mixer {
   // Adaptive category ranking model, which tweaks the score of search results.
   std::unique_ptr<RecurrenceRanker> ranker_;
 
+  // How much the scores produced by |ranker_| affect the final scores.
+  // Controlled by Finch.
+  float boost_coefficient_;
+
   DISALLOW_COPY_AND_ASSIGN(Mixer);
 };
 

chrome/browser/ui/app_list/search/search_controller_factory.cc

@@ -172,6 +172,7 @@ std::unique_ptr<SearchController> CreateSearchController(
 
   if (app_list_features::IsAdaptiveResultRankerEnabled()) {
     RecurrenceRankerConfigProto group_ranker_config;
+    group_ranker_config.set_min_seconds_between_saves(240u);
     auto* predictor =
         group_ranker_config.mutable_zero_state_frecency_predictor();
     predictor->set_target_limit(base::GetFieldTrialParamByFeatureAsInt(

chrome/browser/ui/app_list/search/search_result_ranker/ranking_item_util.cc

@@ -7,14 +7,15 @@
 #include "ash/public/cpp/app_list/app_list_types.h"
 #include "base/macros.h"
 #include "chrome/browser/ui/app_list/chrome_app_list_item.h"
+#include "chrome/browser/ui/app_list/search/chrome_search_result.h"
 
 namespace app_list {
 
 using SearchResultType = ash::SearchResultType;
 
-RankingItemType RankingItemTypeFromSearchResultType(
-    const ash::SearchResultType& type) {
-  switch (type) {
+RankingItemType RankingItemTypeFromSearchResult(
+    const ChromeSearchResult& result) {
+  switch (result.result_type()) {
     case SearchResultType::kInstalledApp:
     case SearchResultType::kInternalApp:
       return RankingItemType::kApp;

chrome/browser/ui/app_list/search/search_result_ranker/ranking_item_util.h

@@ -5,11 +5,8 @@
 #ifndef CHROME_BROWSER_UI_APP_LIST_SEARCH_SEARCH_RESULT_RANKER_RANKING_ITEM_UTIL_H_
 #define CHROME_BROWSER_UI_APP_LIST_SEARCH_SEARCH_RESULT_RANKER_RANKING_ITEM_UTIL_H_
 
-namespace ash {
-enum class SearchResultType;
-}
-
 class ChromeAppListItem;
+class ChromeSearchResult;
 
 namespace app_list {
 
@@ -24,9 +21,9 @@ enum class RankingItemType {
   kArcAppShortcut
 };
 
-// Convert the enum used by |ChromeSearchResult|s into a |RankingItemType|.
-RankingItemType RankingItemTypeFromSearchResultType(
-    const ash::SearchResultType& type);
+// Convert a |ChromeSearchResult| into its |RankingItemType|.
+RankingItemType RankingItemTypeFromSearchResult(
+    const ChromeSearchResult& result);
 
 // Return the type of an |ChromeAppListItem|. We currently do not distinguish
 // between different kinds of apps, and all |AppServiceAppItem|s are apps, so we

chrome/browser/ui/app_list/search/tests/mixer_unittest.cc

@@ -12,13 +12,17 @@
 
 #include "ash/public/cpp/app_list/app_list_config.h"
 #include "ash/public/cpp/app_list/app_list_features.h"
+#include "ash/public/cpp/app_list/app_list_types.h"
+#include "base/files/scoped_temp_dir.h"
 #include "base/stl_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/scoped_feature_list.h"
+#include "base/test/scoped_task_environment.h"
 #include "chrome/browser/ui/app_list/search/search_provider.h"
 #include "chrome/browser/ui/app_list/search/search_result_ranker/ranking_item_util.h"
+#include "chrome/browser/ui/app_list/search/search_result_ranker/recurrence_ranker.h"
 #include "chrome/browser/ui/app_list/test/fake_app_list_model_updater.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -27,11 +31,15 @@ class FakeAppListModelUpdater;
 namespace app_list {
 namespace test {
 
+using ResultType = ash::SearchResultType;
+
 // Maximum number of results to show in each mixer group.
 const size_t kMaxAppsGroupResults = 4;
 const size_t kMaxOmniboxResults = 4;
 const size_t kMaxWebstoreResults = 2;
 
+const bool kEphemeralUser = false;
+
 class TestSearchResult : public ChromeSearchResult {
  public:
   TestSearchResult(const std::string& id, double relevance)
@@ -64,12 +72,13 @@ int TestSearchResult::instantiation_count = 0;
 
 class TestSearchProvider : public SearchProvider {
  public:
-  explicit TestSearchProvider(const std::string& prefix)
+  TestSearchProvider(const std::string& prefix, ResultType result_type)
       : prefix_(prefix),
         count_(0),
         bad_relevance_range_(false),
         small_relevance_range_(false),
-        display_type_(ash::SearchResultDisplayType::kList) {}
+        display_type_(ash::SearchResultDisplayType::kList),
+        result_type_(result_type) {}
   ~TestSearchProvider() override {}
 
   // SearchProvider overrides:
@@ -89,6 +98,7 @@ class TestSearchProvider : public SearchProvider {
         relevance = 0.5 - i / 100.0;
       TestSearchResult* result = new TestSearchResult(id, relevance);
       result->SetDisplayType(display_type_);
+      result->SetResultType(result_type_);
       Add(std::unique_ptr<ChromeSearchResult>(result));
     }
   }
@@ -107,6 +117,7 @@ class TestSearchProvider : public SearchProvider {
   bool bad_relevance_range_;
   bool small_relevance_range_;
   ChromeSearchResult::DisplayType display_type_;
+  ResultType result_type_;
 
   DISALLOW_COPY_AND_ASSIGN(TestSearchProvider);
 };
@@ -120,15 +131,19 @@ class MixerTest : public testing::Test {
   void SetUp() override {
     model_updater_ = std::make_unique<FakeAppListModelUpdater>();
 
-    providers_.push_back(std::make_unique<TestSearchProvider>("app"));
-    providers_.push_back(std::make_unique<TestSearchProvider>("omnibox"));
-    providers_.push_back(std::make_unique<TestSearchProvider>("webstore"));
+    providers_.push_back(
+        std::make_unique<TestSearchProvider>("app", ResultType::kInternalApp));
+    providers_.push_back(
+        std::make_unique<TestSearchProvider>("omnibox", ResultType::kOmnibox));
+    providers_.push_back(std::make_unique<TestSearchProvider>(
+        "webstore", ResultType::kWebStoreApp));
   }
 
-  void CreateMixer(bool use_adaptive_ranker) {
+  void CreateMixer(bool use_adaptive_ranker,
+                   const std::map<std::string, std::string>& params = {}) {
     if (use_adaptive_ranker) {
-      scoped_feature_list_.InitWithFeatures(
-          {app_list_features::kEnableAdaptiveResultRanker}, {});
+      scoped_feature_list_.InitAndEnableFeatureWithParameters(
+          app_list_features::kEnableAdaptiveResultRanker, params);
     } else {
       scoped_feature_list_.InitWithFeatures(
           {}, {app_list_features::kEnableAdaptiveResultRanker});
@@ -136,6 +151,26 @@ class MixerTest : public testing::Test {
 
     mixer_ = std::make_unique<Mixer>(model_updater_.get());
 
+    if (use_adaptive_ranker) {
+      ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+
+      RecurrenceRankerConfigProto ranker_config;
+      ranker_config.set_min_seconds_between_saves(240u);
+      auto* predictor = ranker_config.mutable_zero_state_frecency_predictor();
+      predictor->set_target_limit(200u);
+      predictor->set_decay_coeff(0.8f);
+      auto* fallback = ranker_config.mutable_fallback_predictor();
+      fallback->set_target_limit(200u);
+      fallback->set_decay_coeff(0.8f);
+
+      std::unique_ptr<RecurrenceRanker> ranker =
+          std::make_unique<RecurrenceRanker>(
+              temp_dir_.GetPath().AppendASCII("ranker_model.proto"),
+              ranker_config, kEphemeralUser);
+      Wait();
+      mixer_->SetRecurrenceRanker(std::move(ranker));
+    }
+
     // TODO(warx): when fullscreen app list is default enabled, modify this test
     // to test answer card/apps group having relevance boost.
     size_t apps_group_id = mixer_->AddGroup(kMaxAppsGroupResults, 1.0, 0.0);
@@ -173,13 +208,17 @@ class MixerTest : public testing::Test {
     mixer_->Train(id, type);
   }
 
+  void Wait() { scoped_task_environment_.RunUntilIdle(); }
+
   Mixer* mixer() { return mixer_.get(); }
   TestSearchProvider* app_provider() { return providers_[0].get(); }
   TestSearchProvider* omnibox_provider() { return providers_[1].get(); }
   TestSearchProvider* webstore_provider() { return providers_[2].get(); }
 
  private:
+  base::test::ScopedTaskEnvironment scoped_task_environment_;
   base::test::ScopedFeatureList scoped_feature_list_;
+  base::ScopedTempDir temp_dir_;
 
   std::unique_ptr<Mixer> mixer_;
   std::unique_ptr<FakeAppListModelUpdater> model_updater_;
@@ -284,5 +323,23 @@ TEST_F(MixerTest, RankerIsDisabledWithFlag) {
             "app0,omnibox0,app1,omnibox1,app2,omnibox2,app3,omnibox3");
 }
 
+TEST_F(MixerTest, RankerImprovesScores) {
+  CreateMixer(true, {{"boost_coefficient", "10.0"}});
+
+  for (int i = 0; i < 20; ++i)
+    Train("omnibox2", RankingItemType::kOmnibox);
+
+  app_provider()->set_count(4);
+  app_provider()->set_small_relevance_range();
+  omnibox_provider()->set_count(4);
+  omnibox_provider()->set_small_relevance_range();
+  RunQuery();
+
+  // Omnibox results exist in the ranker and should be up-weighted to the top of
+  // the list.
+  EXPECT_EQ(GetResults(),
+            "omnibox0,omnibox1,omnibox2,omnibox3,app0,app1,app2,app3");
+}
+
 }  // namespace test
 }  // namespace app_list