Skip to content

Commit 18a60c0

Browse files
MB-71114 : Fix for KNN scoring details (#2367)
Fix for KNN scoring details when a doc matches both text and vector branches in a root level hybrid query
1 parent 38511dc commit 18a60c0

3 files changed

Lines changed: 90 additions & 8 deletions

File tree

search/searcher/search_custom_fields.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,6 @@ import (
2222
index "github.com/blevesearch/bleve_index_api"
2323
)
2424

25-
// loadDocValuesOnHit resolves hit.ID and returns the hit's doc-value fields in a
26-
// new map the caller owns. See loadDocValuesOnHitWithTypes.
27-
func loadDocValuesOnHit(hit *search.DocumentMatch, dvReader index.DocValueReader,
28-
r index.IndexReader) (map[string]interface{}, error) {
29-
return loadDocValuesOnHitWithTypes(hit, dvReader, r, nil)
30-
}
31-
3225
// loadDocValuesOnHitWithTypes resolves hit.ID and returns the hit's doc-value
3326
// fields in a new map. It does not touch hit.Fields, so these UDF-input fields
3427
// can't leak into SearchRequest.Fields; the caller owns the map. Returns nil

search_knn.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,7 @@ func setKnnHitsInCollector(knnHits []*search.DocumentMatch, coll *collector.TopN
480480
// Boost the FTS score using the KNN score
481481
ftsMatch.Score += knnMatch.Score
482482
// Combine the FTS explanation with the KNN explanation, if present
483-
ftsMatch.Expl.MergeWith(knnMatch.Expl)
483+
ftsMatch.Expl = ftsMatch.Expl.MergeWith(knnMatch.Expl)
484484
}
485485
coll.SetKNNHits(knnHits, search.HybridMergeCallbackFn(mergeFn))
486486
}

search_knn_test.go

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3051,3 +3051,92 @@ func TestVectorIndexExhaustion(t *testing.T) {
30513051
})
30523052
}
30533053
}
3054+
3055+
// TestHybridExplainNotDropped verifies that a doc matching BOTH the text query
3056+
// and the KNN branch keeps a merged explanation (text + vector) whose value
3057+
// equals the summed score, i.e. the KNN part isn't dropped from the explain tree.
3058+
func TestHybridExplainNotDropped(t *testing.T) {
3059+
tmpIndexPath := createTmpIndexPath(t)
3060+
defer cleanupTmpIndexPath(t, tmpIndexPath)
3061+
3062+
// vdocBoth matches the text query (category=health) AND is the nearest
3063+
// vector to the query vector, so it exercises the hybrid merge path.
3064+
docs := map[string]map[string]interface{}{
3065+
"vdocBoth": {"category": "health", "vector": []float32{0.1, 0.9, 0.2, 0.5}},
3066+
"vdocVecOnly": {"vector": []float32{0.1, 0.9, 0.2, 0.5}},
3067+
"vdocTextOnly": {"category": "health", "vector": []float32{0.1, 0.2, 0.9, 0.1}},
3068+
"vdocNeither": {"category": "finance", "vector": []float32{0.8, 0.1, 0.1, 0.9}},
3069+
}
3070+
3071+
indexMapping := NewIndexMapping()
3072+
catFM := NewTextFieldMapping()
3073+
catFM.Analyzer = "keyword"
3074+
indexMapping.DefaultMapping.AddFieldMappingsAt("category", catFM)
3075+
3076+
vecFM := mapping.NewVectorFieldMapping()
3077+
vecFM.Index = true
3078+
vecFM.Dims = 4
3079+
vecFM.Similarity = "l2_norm"
3080+
indexMapping.DefaultMapping.AddFieldMappingsAt("vector", vecFM)
3081+
3082+
idx, err := New(tmpIndexPath, indexMapping)
3083+
if err != nil {
3084+
t.Fatal(err)
3085+
}
3086+
defer func() {
3087+
if err := idx.Close(); err != nil {
3088+
t.Fatal(err)
3089+
}
3090+
}()
3091+
3092+
batch := idx.NewBatch()
3093+
for id, doc := range docs {
3094+
if err := batch.Index(id, doc); err != nil {
3095+
t.Fatal(err)
3096+
}
3097+
}
3098+
if err := idx.Batch(batch); err != nil {
3099+
t.Fatal(err)
3100+
}
3101+
3102+
// Root-level text query (matches vdocBoth and vdocTextOnly) + KNN branch
3103+
// whose nearest neighbours are vdocBoth and vdocVecOnly.
3104+
tq := query.NewTermQuery("health")
3105+
tq.SetField("category")
3106+
req := NewSearchRequest(tq)
3107+
req.AddKNN("vector", []float32{0.1, 0.9, 0.2, 0.4}, 2, 1.0)
3108+
req.Explain = true
3109+
req.Fields = []string{"category"}
3110+
3111+
res, err := idx.Search(req)
3112+
if err != nil {
3113+
t.Fatal(err)
3114+
}
3115+
3116+
var checkedBoth bool
3117+
for _, hit := range res.Hits {
3118+
if hit.Expl == nil {
3119+
t.Fatalf("hit %s has no explanation", hit.ID)
3120+
}
3121+
// The invariant the bug violated: the explanation value must equal the
3122+
// score it is attached to.
3123+
if diff := hit.Expl.Value - hit.Score; diff > 1e-6 || diff < -1e-6 {
3124+
t.Errorf("hit %s: explanation.value=%v != score=%v (vector explanation dropped)",
3125+
hit.ID, hit.Expl.Value, hit.Score)
3126+
}
3127+
3128+
if hit.ID == "vdocBoth" {
3129+
checkedBoth = true
3130+
// A both-branches hit must be a merged explanation carrying both
3131+
// the text and the vector sub-explanations.
3132+
if len(hit.Expl.Children) < 2 {
3133+
t.Errorf("hit vdocBoth: expected merged explanation with >=2 children "+
3134+
"(text + vector), got message=%q value=%v children=%d",
3135+
hit.Expl.Message, hit.Expl.Value, len(hit.Expl.Children))
3136+
}
3137+
}
3138+
}
3139+
if !checkedBoth {
3140+
t.Fatal("vdocBoth was not returned by the hybrid query")
3141+
}
3142+
}

0 commit comments

Comments
 (0)