Basic API to twiddle with search weights (#49866)

enterprise/backend/src/metabase_enterprise/search/scoring.clj

@@ -56,7 +56,7 @@
   {:name   (legacy-name k)
    :score  (let [f (get legacy-scorers k)]
              (f result))
-   :weight (search.config/weights k)})
+   :weight (search.config/weight k)})
 
 (defenterprise score-result
   "Scoring implementation that adds score for items in official collections."

src/metabase/api/search.clj

 (ns metabase.api.search
+  ;; Allowing search.config to be accessed for developer API to set weights
+  #_{:clj-kondo/ignore [:metabase/ns-module-checker]}
   (:require
+   [clojure.string :as str]
    [compojure.core :refer [GET]]
    [java-time.api :as t]
    [metabase.api.common :as api]
    [metabase.public-settings :as public-settings]
    [metabase.search :as search]
+   [metabase.search.config :as search.config]
    [metabase.server.middleware.offset-paging :as mw.offset-paging]
    [metabase.task :as task]
    [metabase.task.search-index :as task.search-index]
@@ -46,17 +50,40 @@
   (api/check-superuser)
   (cond
     (not (public-settings/experimental-fulltext-search-enabled))
-    {:status-code 501, :message "Search index is not enabled."}
+    (throw (ex-info "Search index is not enabled." {:status-code 501}))
 
     (search/supports-index?)
-    (do
-      (if (task/job-exists? task.search-index/job-key)
-        (task/trigger-now! task.search-index/job-key)
-        (search/reindex!))
-      {:status-code 200})
+    (if (task/job-exists? task.search-index/job-key)
+      (do (task/trigger-now! task.search-index/job-key) {:message "task triggered"})
+      (do (search/reindex!) {:message "done"}))
 
     :else
-    {:status-code 501, :message "Search index is not supported for this installation."}))
+    (throw (ex-info "Search index is not supported for this installation." {:status-code 501}))))
+
+(defn- set-weights! [overrides]
+  (api/check-superuser)
+  (let [allowed-key? (set (keys @#'search.config/default-weights))
+        unknown-weights (seq (remove allowed-key? (keys overrides)))]
+    (when unknown-weights
+      (throw (ex-info (str "Unknown weights: " (str/join ", " (map name (sort unknown-weights))))
+                      {:status-code 400})))
+    (public-settings/experimental-search-weight-overrides!
+     (merge (public-settings/experimental-search-weight-overrides) overrides))
+    (search.config/weights)))
+
+(api/defendpoint GET "/weights"
+  "Return the current weights being used to rank the search results"
+  [:as {overrides :params}]
+  ;; remove cookie
+  (let [overrides (-> overrides (dissoc :search_engine) (update-vals parse-double))]
+    (if (seq overrides)
+      (set-weights! overrides)
+      (search.config/weights))))
+
+(api/defendpoint PUT "/weights"
+  "Return the current weights being used to rank the search results"
+  [:as {overrides :body}]
+  (set-weights! overrides))
 
 (api/defendpoint GET "/"
   "Search for items in Metabase.

src/metabase/models/model_index.clj

@@ -36,8 +36,8 @@
 (derive :model/ModelIndexValue :metabase/model)
 
 (derive :model/ModelIndex :hook/created-at-timestamped?)
-(derive :model/ModelIndex :hook/search-index)
-(derive :model/ModelIndexValue :hook/search-index)
+;; TODO disabled due to issues having an update hook causes, seemingly due to a toucan2 bug
+#_(derive :model/ModelIndex :hook/search-index)
 
 (t2/deftransforms ModelIndex
   {:pk_ref    mi/transform-field-ref
@@ -190,7 +190,7 @@
    :attrs        {:id            :model_pk
                   :collection-id :collection.id
                   :creator-id    false
-                  ;; this seems wrong, I'd expect it to track whether the model is archived.
+                ;; this seems wrong, I'd expect it to track whether the model is archived.
                   :archived      false
                   :database-id   :model.database_id
                   :created-at    false
@@ -205,3 +205,6 @@
    :joins        {:model_index [:model/ModelIndex [:= :model_index.id :this.model_index_id]]
                   :model       [:model/Card [:= :model.id :model_index.model_id]]
                   :collection  [:model/Collection [:= :collection.id :model.collection_id]]}})
+
+;; TODO resolve the toucan2 issue preventing us from using this hook
+(underive :model/ModelIndexValue :hook/search-index)

src/metabase/public_settings.clj

@@ -1049,6 +1049,15 @@ See [fonts](../configuring-metabase/fonts.md).")
   :default    false
   :type       :boolean)
 
+(defsetting experimental-search-weight-overrides
+  (deferred-tru "Used to override weights used for search ranking")
+  :visibility :internal
+  :cache?     false
+  :encryption :no
+  :export?    false
+  :default    nil
+  :type       :json)
+
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;; Deprecated uploads settings begin
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

src/metabase/search/config.clj

@@ -79,8 +79,7 @@
 
 (assert (= all-models (set models-search-order)) "The models search order has to include all models")
 
-(def weights
-  "Strength of the various scorers. Copied from metabase.search.in-place.scoring, but allowing divergence."
+(def ^:private default-weights
   {:pinned              2
    :bookmarked          2
    :recency             1.5
@@ -91,6 +90,16 @@
    :view-count          2
    :text                10})
 
+(defn weights
+  "Strength of the various scorers. Copied from metabase.search.in-place.scoring, but allowing divergence."
+  []
+  (merge default-weights (public-settings/experimental-search-weight-overrides)))
+
+(defn weight
+  "The relative strength the corresponding score has in influencing the total score."
+  [scorer-key]
+  (get (weights) scorer-key 0))
+
 (defn model->alias
   "Given a model string returns the model alias"
   [model]

src/metabase/search/postgres/scoring.clj

@@ -53,7 +53,7 @@
     [:inline 1]))
 
 (defn- weighted-score [[column-alias expr]]
-  [:* [:inline (search.config/weights column-alias 0)] expr])
+  [:* [:inline (search.config/weight column-alias)] expr])
 
 (defn- select-items [scorers]
   (concat

test/metabase/api/search_test.clj

@@ -19,6 +19,7 @@
    [metabase.models.permissions :as perms]
    [metabase.models.permissions-group :as perms-group]
    [metabase.models.revision :as revision]
+   [metabase.public-settings :as public-settings]
    [metabase.public-settings.premium-features :as premium-features]
    [metabase.search :as search]
    [metabase.search.config :as search.config]
@@ -1589,3 +1590,20 @@
                 (when (pos? attempts-left)
                   (Thread/sleep 200)
                   (recur (dec attempts-left))))))))))
+
+(deftest weights-test
+  (let [original-weights   (search.config/weights)
+        original-overrides (public-settings/experimental-search-weight-overrides)]
+    (try
+      (is (= original-weights (mt/user-http-request :crowberto :get 200 "search/weights")))
+      (is (mt/user-http-request :rasta :put 403 "search/weights"))
+      (is (= original-weights (mt/user-http-request :crowberto :put 200 "search/weights")))
+      (is (= (assoc original-weights :recency 4 :text 20)
+             (mt/user-http-request :crowberto :put 200 "search/weights" {:recency 4, :text 20})))
+      (is (= (assoc original-weights :recency 4 :text 30.0)
+             (mt/user-http-request :crowberto :get 200 "search/weights?text=30")))
+      (is (mt/user-http-request :crowberto :put 400 "search/weights" {:bad-spelling 2}))
+      (is (= (assoc original-weights :recency 4 :text 30.0)
+             (mt/user-http-request :crowberto :get 200 "search/weights")))
+      (finally
+        (public-settings/experimental-search-weight-overrides! original-overrides)))))

test/metabase/search/postgres/index_test.clj

@@ -56,35 +56,33 @@
 
 (deftest incremental-update-test
   (with-index
-    (mt/with-temporary-setting-values [experimental-search-index-realtime-updates true]
-      (testing "The index is updated when models change"
-        ;; Has a second entry is "Revenue Project(ions)", when using English dictionary
-        (is (= 1 #_2 (count (search.index/search "Projected Revenue"))))
-        (is (= 0 (count (search.index/search "Protected Avenue"))))
-        (t2/update! :model/Card {:name "Projected Revenue"} {:name "Protected Avenue"})
-        (is (= 0 #_1 (count (search.index/search "Projected Revenue"))))
-        (is (= 1 (count (search.index/search "Protected Avenue"))))
-        ;; Delete hooks are disabled, for now, over performance concerns.
-        ;(t2/delete! :model/Card :name "Protected Avenue")
-        (search.ingestion/delete-model! (t2/select-one :model/Card :name "Protected Avenue"))
-        (is (= 0 #_1 (count (search.index/search "Projected Revenue"))))
-        (is (= 0 (count (search.index/search "Protected Avenue"))))))))
+    (testing "The index is updated when models change"
+     ;; Has a second entry is "Revenue Project(ions)", when using English dictionary
+      (is (= 1 #_2 (count (search.index/search "Projected Revenue"))))
+      (is (= 0 (count (search.index/search "Protected Avenue"))))
+      (t2/update! :model/Card {:name "Projected Revenue"} {:name "Protected Avenue"})
+      (is (= 0 #_1 (count (search.index/search "Projected Revenue"))))
+      (is (= 1 (count (search.index/search "Protected Avenue"))))
+     ;; Delete hooks are disabled, for now, over performance concerns.
+     ;(t2/delete! :model/Card :name "Protected Avenue")
+      (search.ingestion/delete-model! (t2/select-one :model/Card :name "Protected Avenue"))
+      (is (= 0 #_1 (count (search.index/search "Projected Revenue"))))
+      (is (= 0 (count (search.index/search "Protected Avenue")))))))
 
 (deftest related-update-test
   (with-index
-    (mt/with-temporary-setting-values [experimental-search-index-realtime-updates true]
-      (testing "The index is updated when model dependencies change"
-        (let [index-table    @#'search.index/active-table
-              table-id       (t2/select-one-pk :model/Table :name "Indexed Table")
-              legacy-input   #(-> (t2/select-one [index-table :legacy_input] :model "table" :model_id table-id)
-                                  :legacy_input
-                                  (json/parse-string true))
-              db-id          (t2/select-one-fn :db_id :model/Table table-id)
-              db-name-fn     (comp :database_name legacy-input)
-              alternate-name (str (random-uuid))]
-          (is (= "Indexed Database" (db-name-fn)))
-          (t2/update! :model/Database db-id {:name alternate-name})
-          (is (= alternate-name (db-name-fn))))))))
+    (testing "The index is updated when model dependencies change"
+      (let [index-table    @#'search.index/active-table
+            table-id       (t2/select-one-pk :model/Table :name "Indexed Table")
+            legacy-input   #(-> (t2/select-one [index-table :legacy_input] :model "table" :model_id table-id)
+                                :legacy_input
+                                (json/parse-string true))
+            db-id          (t2/select-one-fn :db_id :model/Table table-id)
+            db-name-fn     (comp :database_name legacy-input)
+            alternate-name (str (random-uuid))]
+        (is (= "Indexed Database" (db-name-fn)))
+        (t2/update! :model/Database db-id {:name alternate-name})
+        (is (= alternate-name (db-name-fn)))))))
 
 (deftest consistent-subset-test
   (with-index

test/metabase/search/spec_test.clj

@@ -128,7 +128,8 @@
 
 (deftest search-index-model-test
   (testing "All the required models descend from :hook/search-index\n"
-    (let [expected-models (keys (search.spec/model-hooks))
+    ;; TODO restore hooks to ModelIndex when toucan issue is resolved
+    (let [expected-models (keys (dissoc (search.spec/model-hooks) :model/ModelIndex :model/ModelIndexValue))
           ;; Some models have submodels, so absorb those too
           expected-models (into (set expected-models) (mapcat descendants) expected-models)
           actual-models   (set (descendants :hook/search-index))]