From d48cdc67a07b0e6dcdfdff2097e26865453db473 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Lecrenier?= <loic.lecrenier@me.com>
Date: Thu, 30 Mar 2023 14:01:52 +0200
Subject: [PATCH] Fix term matching strategy bugs

---
 milli/src/search/new/graph_based_ranking_rule.rs | 5 +----
 milli/src/search/new/query_graph.rs              | 6 +++---
 milli/src/search/new/words.rs                    | 4 +++-
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/milli/src/search/new/graph_based_ranking_rule.rs b/milli/src/search/new/graph_based_ranking_rule.rs
index 4bb31fc43..88df56448 100644
--- a/milli/src/search/new/graph_based_ranking_rule.rs
+++ b/milli/src/search/new/graph_based_ranking_rule.rs
@@ -107,9 +107,6 @@ impl<'ctx, G: RankingRuleGraphTrait> RankingRule<'ctx, QueryGraph> for GraphBase
         query_graph: &QueryGraph,
     ) -> Result<()> {
         let removal_cost = if let Some(terms_matching_strategy) = self.terms_matching_strategy {
-            // oh no this is wrong!
-            // because
-            // skipping the second node should require that the first one be skipped too
             match terms_matching_strategy {
                 TermsMatchingStrategy::Last => {
                     let removal_order =
@@ -123,7 +120,7 @@ impl<'ctx, G: RankingRuleGraphTrait> RankingRule<'ctx, QueryGraph> for GraphBase
                             *costs.get_mut(n) = Some((cost, forbidden_nodes.clone()));
                         }
                         forbidden_nodes.union(&ns);
-                        cost = 1000;
+                        cost += 100;
                     }
                     costs
                 }
diff --git a/milli/src/search/new/query_graph.rs b/milli/src/search/new/query_graph.rs
index ce3732927..368e22fda 100644
--- a/milli/src/search/new/query_graph.rs
+++ b/milli/src/search/new/query_graph.rs
@@ -300,14 +300,14 @@ impl QueryGraph {
             }
         };
         let mut nodes_to_remove = BTreeMap::<u16, SmallBitmap<QueryNode>>::new();
-        for (node_id, node) in self.nodes.iter() {
+        'outer: for (node_id, node) in self.nodes.iter() {
             let QueryNodeData::Term(t) = &node.data else { continue };
             let mut cost = 0;
             for id in t.term_ids.clone() {
                 if let Some(t_cost) = cost_of_term_idx(id) {
-                    cost += t_cost;
+                    cost = std::cmp::max(cost, t_cost);
                 } else {
-                    continue;
+                    continue 'outer;
                 }
             }
             nodes_to_remove
diff --git a/milli/src/search/new/words.rs b/milli/src/search/new/words.rs
index 263e9220c..b5ee5e085 100644
--- a/milli/src/search/new/words.rs
+++ b/milli/src/search/new/words.rs
@@ -40,7 +40,9 @@ impl<'ctx> RankingRule<'ctx, QueryGraph> for Words {
         self.query_graph = Some(parent_query_graph.clone());
         self.nodes_to_remove = match self.terms_matching_strategy {
             TermsMatchingStrategy::Last => {
-                parent_query_graph.removal_order_for_terms_matching_strategy_last()
+                let mut ns = parent_query_graph.removal_order_for_terms_matching_strategy_last();
+                ns.reverse();
+                ns
             }
             TermsMatchingStrategy::All => {
                 vec![]