From 87f58dfd16dd8ab3cd21fc6428392799ab1d329a Mon Sep 17 00:00:00 2001 From: hyunwoo-kurly Date: Mon, 3 Aug 2026 09:29:07 +0900 Subject: [PATCH] Return a non-matching explanation for documents that are not nearest neighbors KNNWeight.explain() reported every document as a match, including documents the query does not match. getKnnScore() already knew whether the scorer landed on the document but collapsed that answer to a score of 0, and every return path of explain() was Explanation.match(...), so a document that is not among the segment's nearest neighbors received Explanation.match(0.0f, ...) whose isMatch() is true. That breaks the agreement Lucene expects between Weight.explain() and the scorer produced by the same Weight, and callers such as bool, script_score and function_score branch on isMatch() before using the sub-query scorer. explain() now uses the advance result directly and returns Explanation.noMatch(...) when the scorer does not land on the document. The score value cannot serve as the match signal on its own, because KNNScorer.score() multiplies by boost and a genuine nearest neighbor queried with a boost of 0 also scores 0. Documents that do match keep their existing behavior, and callers that pass a score of their own (disk-based search and DocAndScoreQuery) are unaffected. getKnnScore() had no other caller and is removed. Signed-off-by: hyunwoo-kurly --- CHANGELOG.md | 1 + .../opensearch/knn/index/query/KNNWeight.java | 12 +++-- .../knn/index/query/ExplainTests.java | 49 +++++++++++++++++++ 3 files changed, 57 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 279c912c05..d1b9ea68b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * Re-enable flaky BWC test testKNNWarmupCustomLegacyFieldMapping and restore legacy index settings in BWC test fixtures [#2415](https://github.com/opensearch-project/k-NN/issues/2415) ### Bug Fixes +* Return a non-matching explanation from `KNNWeight.explain()` for documents that are not nearest neighbors [#3480](https://github.com/opensearch-project/k-NN/pull/3480) * Preserve raw non-XContent `_source` fields when derived source is enabled [#3402](https://github.com/opensearch-project/k-NN/pull/3402) * Fix dimension-based oversampling not applying for 32x compression [#3455](https://github.com/opensearch-project/k-NN/pull/3455) * Fix NPE in nested kNN search when index contains documents without nested object [#3368](https://github.com/opensearch-project/k-NN/pull/3368) diff --git a/src/main/java/org/opensearch/knn/index/query/KNNWeight.java b/src/main/java/org/opensearch/knn/index/query/KNNWeight.java index 675f19211f..0207a4fae6 100644 --- a/src/main/java/org/opensearch/knn/index/query/KNNWeight.java +++ b/src/main/java/org/opensearch/knn/index/query/KNNWeight.java @@ -133,7 +133,13 @@ public Explanation explain(LeafReaderContext context, int doc, float score) { // calculate score only when its 0 as for disk-based search, // score will be passed from the caller and there is no need to re-compute the score if (score == 0) { - score = getKnnScore(knnScorer, doc); + // explain() can be called for a document this query does not match, for instance when the knn clause is + // one of several should clauses. Reporting a match for such a document breaks the contract that + // Weight.explain() and the scorer of the same Weight have to agree on which documents match. + if (knnScorer.iterator().advance(doc) != doc) { + return Explanation.noMatch("the document is not a nearest neighbor result for the field [" + knnQuery.getField() + "]"); + } + score = knnScorer.score(); } } catch (IOException e) { throw new RuntimeException(String.format("Error while explaining KNN score for doc [%d], score [%f]", doc, score), e); @@ -273,10 +279,6 @@ private Scorer getOrCreateKnnScorer(LeafReaderContext context) throws IOExceptio return scorer; } - private float getKnnScore(Scorer knnScorer, int doc) throws IOException { - return (knnScorer.iterator().advance(doc) == doc) ? knnScorer.score() : 0; - } - @Override public ScorerSupplier scorerSupplier(LeafReaderContext context) { return new ScorerSupplier() { diff --git a/src/test/java/org/opensearch/knn/index/query/ExplainTests.java b/src/test/java/org/opensearch/knn/index/query/ExplainTests.java index 3eac94cbd5..d05a7ad324 100644 --- a/src/test/java/org/opensearch/knn/index/query/ExplainTests.java +++ b/src/test/java/org/opensearch/knn/index/query/ExplainTests.java @@ -37,6 +37,7 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Locale; @@ -376,6 +377,54 @@ public void testDefaultANNSearch() { assertTrue(Comparators.isInOrder(actualDocIds, Comparator.naturalOrder())); } + @SneakyThrows + public void testExplain_whenDocIsNotANearestNeighbor_thenNoMatch() { + // Given + int k = 3; + jniServiceMockedStatic.when( + () -> JNIService.queryIndex(anyLong(), eq(QUERY_VECTOR), eq(k), eq(HNSW_METHOD_PARAMETERS), any(), eq(null), anyInt(), any()) + ).thenReturn(getFilteredKNNQueryResults()); + + final int[] filterDocIds = new int[] { 0, 1, 2, 3, 4, 5 }; + final Map attributesMap = ImmutableMap.of( + KNN_ENGINE, + KNNEngine.FAISS.getName(), + SPACE_TYPE, + SpaceType.L2.getValue() + ); + + setupTest(filterDocIds, attributesMap); + + final KNNQuery query = KNNQuery.builder() + .field(FIELD_NAME) + .queryVector(QUERY_VECTOR) + .k(k) + .indexName(INDEX_NAME) + .filterQuery(FILTER_QUERY) + .methodParameters(HNSW_METHOD_PARAMETERS) + .vectorDataType(VectorDataType.FLOAT) + .explain(true) + .build(); + query.setExplain(true); + + final KNNWeight knnWeight = new DefaultKNNWeight(query, 1f, filterQueryWeight); + final KNNScorer knnScorer = (KNNScorer) knnWeight.scorer(leafReaderContext); + assertNotNull(knnScorer); + knnWeight.getKnnExplanation().addKnnScorer(leafReaderContext, knnScorer); + + // The query returns FILTERED_DOC_ID_TO_SCORES, so this document is not one of the nearest neighbors. Advancing + // the scorer to it lands on the next matching document instead. + final int docIdWithoutMatch = Collections.min(FILTERED_DOC_ID_TO_SCORES.keySet()) - 1; + + // When + final Explanation explanation = knnWeight.explain(leafReaderContext, docIdWithoutMatch); + + // Then + assertNotNull(explanation); + assertFalse(explanation.isMatch()); + assertTrue(explanation.getDescription().contains(FIELD_NAME)); + } + @SneakyThrows public void testANN_FilteredExactSearchAfterANN() { ExactSearcher mockedExactSearcher = mock(ExactSearcher.class);