From 286d310287837a188098124a26da9cdafc5c6f3a Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Thu, 27 Feb 2025 14:58:22 +0100 Subject: [PATCH] Fix inconsistency in attribute ranking rule computation **Changes:** The building of the Attributes ranking rule graph was comparing fieldids with weights which doesn't make sense and may be bug prone, we are now comparing fieldids with fieldids. **Impact:** - search: Attribute ranking rule --- crates/milli/src/fieldids_weights_map.rs | 5 +++++ crates/milli/src/search/new/ranking_rule_graph/fid/mod.rs | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/milli/src/fieldids_weights_map.rs b/crates/milli/src/fieldids_weights_map.rs index 57c99f77f..f23bc1512 100644 --- a/crates/milli/src/fieldids_weights_map.rs +++ b/crates/milli/src/fieldids_weights_map.rs @@ -48,6 +48,11 @@ impl FieldidsWeightsMap { self.map.values().copied().max() } + /// Returns the field id with the highest weight. + pub fn max_weight_fid(&self) -> Option<(FieldId, Weight)> { + self.map.iter().max_by_key(|(_, weight)| *weight).map(|(fid, weight)| (*fid, *weight)) + } + /// Return an iterator visiting all field ids in arbitrary order. pub fn ids(&self) -> impl Iterator + '_ { self.map.keys().copied() diff --git a/crates/milli/src/search/new/ranking_rule_graph/fid/mod.rs b/crates/milli/src/search/new/ranking_rule_graph/fid/mod.rs index 67775ddea..62d75d2ac 100644 --- a/crates/milli/src/search/new/ranking_rule_graph/fid/mod.rs +++ b/crates/milli/src/search/new/ranking_rule_graph/fid/mod.rs @@ -88,10 +88,10 @@ impl RankingRuleGraphTrait for FidGraph { } // always lookup the max_fid if we don't already and add an artificial condition for max scoring - let max_weight: Option = weights_map.max_weight(); + let max_weight_fid = weights_map.max_weight_fid(); - if let Some(max_weight) = max_weight { - if !all_fields.contains(&max_weight) { + if let Some((max_fid, max_weight)) = max_weight_fid { + if !all_fields.contains(&max_fid) { edges.push(( max_weight as u32 * term.term_ids.len() as u32, // TODO improve the fid score i.e. fid^10. conditions_interner.insert(FidCondition {