Return a non-matching explanation for documents that are not nearest neighbors

[hyunwoo-kurly]

Description

KNNWeight.explain() reported every document as a match, including documents the query does not match.

explain(context, doc) (the radial search path) delegates to explain(context, doc, 0), which computed the score through getKnnScore():

private float getKnnScore(Scorer knnScorer, int doc) throws IOException {
    return (knnScorer.iterator().advance(doc) == doc) ? knnScorer.score() : 0;
}

That helper already knew whether the scorer landed on the document but collapsed the answer to a score of 0, and every return path of explain() was Explanation.match(...). A document that is not among the segment's nearest neighbors therefore got Explanation.match(0.0f, ...), whose isMatch() is true, which breaks the agreement Lucene expects between Weight.explain() and the scorer of the same Weight.

This change makes explain() use the advance result directly and return Explanation.noMatch(...) when the scorer does not land on the document. The score value cannot be used as the match signal on its own: KNNScorer.score() multiplies by boost, so a genuine nearest neighbor queried with "boost": 0 also scores 0.

Documents that do match are unaffected: the score is still taken from knnScorer.score() when the caller did not supply one, and callers that pass a non-zero score (disk-based search, DocAndScoreQuery) keep their existing path.

getKnnScore() had no other caller and is removed.

Testing:

• ExplainTests#testExplain_whenDocIsNotANearestNeighbor_thenNoMatch asserts a non-matching explanation for a document outside the segment's result set. Reverting the guard makes it fail, because the old code returns Explanation.match(0.0f, ...).
• ./gradlew test --tests "*ExplainTests*" passes.

Related Issues

Resolves #3479

Related: opensearch-project/OpenSearch#22619 and opensearch-project/OpenSearch#22624 cover the core side of the same disagreement, where a sub-query weight that explains a match while producing no scorer makes ScriptScoreQuery.explain() throw a NullPointerException. This change fixes the k-NN side, which that core guard does not address.

Check List

• New functionality includes testing.
• New functionality has been documented.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Guard against null scorer in explain knnScorer may be null (for example, when there are no results in the segment), which would cause a NullPointerException when calling iterator(). Add a null check and return a non-match explanation before attempting to advance the iterator. src/main/java/org/opensearch/knn/index/query/KNNWeight.java [132-143] final Scorer knnScorer = getOrCreateKnnScorer(context); // 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) { // 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) { + if (knnScorer == null || knnScorer.iterator().advance(doc) != doc) { return Explanation.noMatch("the document is not a nearest neighbor result for the field [" + knnQuery.getField() + "]"); } score = knnScorer.score(); } Suggestion importance[1-10]: 7 __ Why: Valid defensive check: getOrCreateKnnScorer can return null (via scorer(context)), which would cause an NPE in the new knnScorer.iterator().advance(doc) call. The previous getKnnScore helper was called on a potentially-null scorer too, but the new inline code makes this more directly exploitable.	Medium