Bug 1931373 - Add FTS matching data

Added extra data to `Suggestion::Fakespot` to capture how the FTS match
was made.  The plan is to use this as a facet for our metrics to help us
consider how to tune the matching logic (i.e. maybe we should not use
stemming, maybe we should reqiure that terms are close together).

Added Suggest CLI flag to print out the FTS match info.

components/suggest/src/db.rs

@@ -22,13 +22,14 @@ use crate::{
     geoname::GeonameCache,
     pocket::{split_keyword, KeywordConfidence},
     provider::SuggestionProvider,
+    query::FtsQuery,
     rs::{
         DownloadedAmoSuggestion, DownloadedAmpSuggestion, DownloadedAmpWikipediaSuggestion,
         DownloadedExposureSuggestion, DownloadedFakespotSuggestion, DownloadedMdnSuggestion,
         DownloadedPocketSuggestion, DownloadedWikipediaSuggestion, Record, SuggestRecordId,
     },
     schema::{clear_database, SuggestConnectionInitializer},
-    suggestion::{cook_raw_suggestion_url, AmpSuggestionType, Suggestion},
+    suggestion::{cook_raw_suggestion_url, AmpSuggestionType, FtsMatchInfo, Suggestion},
     util::full_keyword,
     weather::WeatherCache,
     Result, SuggestionQuery,
@@ -722,11 +723,12 @@ impl<'a> SuggestDao<'a> {
         Ok(suggestions)
     }
 
-    /// Fetches fakespot suggestions
+    /// Fetches Fakespot suggestions
     pub fn fetch_fakespot_suggestions(&self, query: &SuggestionQuery) -> Result<Vec<Suggestion>> {
-        self.conn.query_rows_and_then_cached(
-            r#"
+        let fts_query = query.fts_query();
+        let sql = r#"
             SELECT
+                s.id,
                 s.title,
                 s.url,
                 s.score,
@@ -753,29 +755,79 @@ impl<'a> SuggestDao<'a> {
                 fakespot_fts MATCH ?
             ORDER BY
                 s.score DESC
-            "#,
-            (&query.fts_query(),),
-            |row| {
-                let score = fakespot::FakespotScore::new(
-                    &query.keyword,
-                    row.get(9)?,
-                    row.get(10)?,
-                    row.get(2)?,
-                )
-                .as_suggest_score();
-                Ok(Suggestion::Fakespot {
-                    title: row.get(0)?,
-                    url: row.get(1)?,
-                    score,
-                    fakespot_grade: row.get(3)?,
-                    product_id: row.get(4)?,
-                    rating: row.get(5)?,
-                    total_reviews: row.get(6)?,
-                    icon: row.get(7)?,
-                    icon_mimetype: row.get(8)?,
-                })
-            },
-        )
+            "#
+        .to_string();
+
+        // Store the list of results plus the suggestion id for calculating the FTS match info
+        let mut results =
+            self.conn
+                .query_rows_and_then_cached(&sql, (&fts_query.match_arg,), |row| {
+                    let id: usize = row.get(0)?;
+                    let score = fakespot::FakespotScore::new(
+                        &query.keyword,
+                        row.get(10)?,
+                        row.get(11)?,
+                        row.get(3)?,
+                    )
+                    .as_suggest_score();
+                    Result::Ok((
+                        Suggestion::Fakespot {
+                            title: row.get(1)?,
+                            url: row.get(2)?,
+                            score,
+                            fakespot_grade: row.get(4)?,
+                            product_id: row.get(5)?,
+                            rating: row.get(6)?,
+                            total_reviews: row.get(7)?,
+                            icon: row.get(8)?,
+                            icon_mimetype: row.get(9)?,
+                            match_info: None,
+                        },
+                        id,
+                    ))
+                })?;
+        // Sort the results, then add the FTS match info to the first one
+        // For performance reasons, this is only calculated for the result with the highest score.
+        // We assume that only one that will be shown to the user and therefore the only one we'll
+        // collect metrics for.
+        results.sort();
+        if let Some((suggestion, id)) = results.first_mut() {
+            match suggestion {
+                Suggestion::Fakespot {
+                    match_info, title, ..
+                } => {
+                    *match_info = Some(self.fetch_fakespot_fts_match_info(&fts_query, *id, title)?);
+                }
+                _ => unreachable!(),
+            }
+        }
+        Ok(results
+            .into_iter()
+            .map(|(suggestion, _)| suggestion)
+            .collect())
+    }
+
+    fn fetch_fakespot_fts_match_info(
+        &self,
+        fts_query: &FtsQuery<'_>,
+        suggestion_id: usize,
+        title: &str,
+    ) -> Result<FtsMatchInfo> {
+        let prefix = if fts_query.is_prefix_query {
+            // If the query was a prefix match query then test if the query without the prefix
+            // match would have also matched.  If not, then this counts as a prefix match.
+            let sql = "SELECT 1 FROM fakespot_fts WHERE rowid = ? AND fakespot_fts MATCH ?";
+            let params = (&suggestion_id, &fts_query.match_arg_without_prefix_match);
+            !self.conn.exists(sql, params)?
+        } else {
+            // If not, then it definitely wasn't a prefix match
+            false
+        };
+
+        Ok(FtsMatchInfo {
+            prefix,
+            stemming: fts_query.match_required_stemming(title),
+        })
     }
 
     /// Fetches exposure suggestions

components/suggest/src/query.rs

@@ -143,43 +143,85 @@ impl SuggestionQuery {
         }
     }
 
-    // Other Functionality
-
-    /// Parse the `keyword` field into a set of keywords.
-    ///
-    /// This is used when passing the keywords into an FTS search.  It:
-    ///   - Strips out any `():^*"` chars.  These are typically used for advanced searches, which
-    ///     we don't support and it would be weird to only support for FTS searches, which
-    ///     currently means Fakespot searches.
-    ///   - Splits on whitespace to get a list of individual keywords
-    ///
-    pub(crate) fn parse_keywords(&self) -> Vec<&str> {
-        self.keyword
-            .split([' ', '(', ')', ':', '^', '*', '"'])
-            .filter(|s| !s.is_empty())
-            .collect()
+    /// Create an FTS query term for our keyword(s)
+    pub(crate) fn fts_query(&self) -> FtsQuery<'_> {
+        FtsQuery::new(&self.keyword)
     }
+}
 
-    /// Create an FTS query term for our keyword(s)
-    pub(crate) fn fts_query(&self) -> String {
-        let keywords = self.parse_keywords();
+pub struct FtsQuery<'a> {
+    pub match_arg: String,
+    pub match_arg_without_prefix_match: String,
+    pub is_prefix_query: bool,
+    keyword_terms: Vec<&'a str>,
+}
+
+impl<'a> FtsQuery<'a> {
+    fn new(keyword: &'a str) -> Self {
+        // Parse the `keyword` field into a set of keywords.
+        //
+        // This is used when passing the keywords into an FTS search.  It:
+        //   - Strips out any `():^*"` chars.  These are typically used for advanced searches, which
+        //     we don't support and it would be weird to only support for FTS searches.
+        //   - splits on whitespace to get a list of individual keywords
+        let keywords = Self::split_terms(keyword);
         if keywords.is_empty() {
-            return String::from(r#""""#);
+            return Self {
+                keyword_terms: keywords,
+                match_arg: String::from(r#""""#),
+                match_arg_without_prefix_match: String::from(r#""""#),
+                is_prefix_query: false,
+            };
         }
         // Quote each term from `query` and join them together
-        let mut fts_query = keywords
+        let mut sqlite_match = keywords
             .iter()
             .map(|keyword| format!(r#""{keyword}""#))
             .collect::<Vec<_>>()
             .join(" ");
         // If the input is > 3 characters, and there's no whitespace at the end.
         // We want to append a `*` char to the end to do a prefix match on it.
         let total_chars = keywords.iter().fold(0, |count, s| count + s.len());
-        let query_ends_in_whitespace = self.keyword.ends_with(' ');
-        if (total_chars > 3) && !query_ends_in_whitespace {
-            fts_query.push('*');
+        let query_ends_in_whitespace = keyword.ends_with(' ');
+        let prefix_match = (total_chars > 3) && !query_ends_in_whitespace;
+        let sqlite_match_without_prefix_match = sqlite_match.clone();
+        if prefix_match {
+            sqlite_match.push('*');
+        }
+        Self {
+            keyword_terms: keywords,
+            is_prefix_query: prefix_match,
+            match_arg: sqlite_match,
+            match_arg_without_prefix_match: sqlite_match_without_prefix_match,
         }
-        fts_query
+    }
+
+    /// Try to figure out if a FTS match required stemming
+    ///
+    /// To test this, we have to try to mimic the SQLite FTS logic. This code doesn't do it
+    /// perfectly, but it should return the correct result most of the time.
+    pub fn match_required_stemming(&self, title: &str) -> bool {
+        let title = title.to_lowercase();
+        let split_title = Self::split_terms(&title);
+
+        !self.keyword_terms.iter().enumerate().all(|(i, keyword)| {
+            split_title.iter().any(|title_word| {
+                let last_keyword = i == self.keyword_terms.len() - 1;
+
+                if last_keyword && self.is_prefix_query {
+                    title_word.starts_with(keyword)
+                } else {
+                    title_word == keyword
+                }
+            })
+        })
+    }
+
+    fn split_terms(phrase: &str) -> Vec<&str> {
+        phrase
+            .split([' ', '(', ')', ':', '^', '*', '"', ','])
+            .filter(|s| !s.is_empty())
+            .collect()
     }
 }
 
@@ -189,7 +231,7 @@ mod test {
 
     fn check_parse_keywords(input: &str, expected: Vec<&str>) {
         let query = SuggestionQuery::all_providers(input);
-        assert_eq!(query.parse_keywords(), expected);
+        assert_eq!(query.fts_query().keyword_terms, expected);
     }
 
     #[test]
@@ -207,7 +249,7 @@ mod test {
 
     fn check_fts_query(input: &str, expected: &str) {
         let query = SuggestionQuery::all_providers(input);
-        assert_eq!(query.fts_query(), expected);
+        assert_eq!(query.fts_query().match_arg, expected);
     }
 
     #[test]
@@ -234,4 +276,24 @@ mod test {
         check_fts_query(" ", r#""""#);
         check_fts_query("()", r#""""#);
     }
+
+    #[test]
+    fn test_fts_query_match_required_stemming() {
+        // These don't require stemming, since each keyword matches a term in the title
+        assert!(!FtsQuery::new("running shoes").match_required_stemming("running shoes"));
+        assert!(
+            !FtsQuery::new("running shoes").match_required_stemming("new balance running shoes")
+        );
+        // Case changes shouldn't matter
+        assert!(!FtsQuery::new("running shoes").match_required_stemming("Running Shoes"));
+        // This doesn't require stemming, since `:` is not part of the word
+        assert!(!FtsQuery::new("running shoes").match_required_stemming("Running: Shoes"));
+        // This requires the keywords to be stemmed in order to match
+        assert!(FtsQuery::new("run shoes").match_required_stemming("running shoes"));
+        // This didn't require stemming, since the last keyword was a prefix match
+        assert!(!FtsQuery::new("running sh").match_required_stemming("running shoes"));
+        // This does require stemming (we know it wasn't a prefix match since there's not enough
+        // characters).
+        assert!(FtsQuery::new("run").match_required_stemming("running shoes"));
+    }
 }