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,77 @@ 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
+        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 mut params: Vec<&dyn ToSql> = vec![];
+        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 NOT EXISTS (SELECT 1 FROM fakespot_fts WHERE rowid = ? AND fakespot_fts MATCH ?)";
+            let params = (&suggestion_id, &fts_query.match_arg_without_prefix_match);
+            self.conn.query_row(sql, params, |row| row.get(0))?
+        } 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"));
+    }
 }

components/suggest/src/schema.rs

@@ -23,7 +23,7 @@ use sql_support::{
 ///     `clear_database()` by adding their names to `conditional_tables`, unless
 ///     they are cleared via a deletion trigger or there's some other good
 ///     reason not to do so.
-pub const VERSION: u32 = 31;
+pub const VERSION: u32 = 32;
 
 /// The current Suggest database schema.
 pub const SQL: &str = "
@@ -131,14 +131,15 @@ CREATE TABLE fakespot_custom_details(
 
 CREATE VIRTUAL TABLE IF NOT EXISTS fakespot_fts USING FTS5(
   title,
+  suggestion_id,
   content='',
   contentless_delete=1,
   tokenize=\"porter unicode61 remove_diacritics 2 tokenchars '''-'\"
 );
 
 CREATE TRIGGER fakespot_ai AFTER INSERT ON fakespot_custom_details BEGIN
-  INSERT INTO fakespot_fts(rowid, title)
-    SELECT id, title
+  INSERT INTO fakespot_fts(rowid, title, suggestion_id)
+    SELECT id, title, id
     FROM suggestions
     WHERE id = new.suggestion_id;
 END;
@@ -597,6 +598,30 @@ CREATE INDEX geonames_alternates_geoname_id ON geonames_alternates(geoname_id);
                 )?;
                 Ok(())
             }
+            31 => {
+                clear_database(tx)?;
+                tx.execute_batch(
+                    "
+DROP TABLE fakespot_fts;
+DROP TRIGGER fakespot_ai;
+CREATE VIRTUAL TABLE fakespot_fts USING FTS5(
+  title,
+  suggestion_id,
+  content='',
+  contentless_delete=1,
+  tokenize=\"porter unicode61 remove_diacritics 2 tokenchars '''-'\"
+);
+
+CREATE TRIGGER fakespot_ai AFTER INSERT ON fakespot_custom_details BEGIN
+  INSERT INTO fakespot_fts(rowid, title, suggestion_id)
+    SELECT id, title, id
+    FROM suggestions
+    WHERE id = new.suggestion_id;
+END;
+                    ",
+                )?;
+                Ok(())
+            }
             _ => Err(open_database::Error::IncompatibleVersion(version)),
         }
     }