Test framework: CheckHits.checkTopScores can falsely fail on correct minimum-should-match queries

CheckHits.checkTopScores is the public test-framework helper that runs a query in COMPLETE and in TOP_SCORES mode and checks that the top hits agree and that the score bounds are valid. Its second part, doCheckMaxScores, walks both scorers' two-phase approximations (their candidate lists) side by side and asserts after each step that they are on the same doc. No documented contract promises that: an approximation only has to be a superset of the matching documents. With minimumNumberShouldMatch above 1, both scorers the checker builds are WANDScorers, which choose candidates differently per mode, so the TOP_SCORES list can contain a doc that the COMPLETE list skips and that matches() then rejects.

On two docs (doc 0 = c, doc 1 = a b), (f:a f:b f:c)~2 matches only doc 1, with score 0.5545178 in both modes; the COMPLETE candidates are [1], the TOP_SCORES candidates [0, 1], and the checker fails with expected:<1> but was:<0>. The assertion dates from 2018 (8.0.0) and has been reachable since 8.8.0. Only tests are affected, and none of Lucene's own calls of the checker uses minimum-should-match. We think the fix belongs in CheckHits; a maintainer may instead read the lockstep as an intended efficiency check (see the counter-case below).

// first loop of doCheckMaxScores: "Check boundaries and max scores when iterating all matches" (:717)
TwoPhaseIterator twoPhase1 = s1.twoPhaseIterator();                      // :737  s1: COMPLETE scorer
TwoPhaseIterator twoPhase2 = s2.twoPhaseIterator();                      // :738  s2: TOP_SCORES scorer
DocIdSetIterator approx1 = twoPhase1 == null ? s1.iterator() : twoPhase1.approximation();
DocIdSetIterator approx2 = twoPhase2 == null ? s2.iterator() : twoPhase2.approximation();
...
for (int doc2 = approx2.nextDoc(); ; doc2 = approx2.nextDoc()) {         // :744
  int doc1;
  for (doc1 = approx1.nextDoc(); doc1 < doc2; doc1 = approx1.nextDoc()) {  // COMPLETE candidates that
    if (twoPhase1 == null || twoPhase1.matches()) {                        // TOP_SCORES skipped must not
      assertTrue(s1.score() < minScore);                                   // be competitive
    }
  }
  assertEquals(doc1, doc2);                                               // :751

• What it requires. TOP_SCORES may skip COMPLETE candidates, but may never stop on a doc that COMPLETE skipped. The advance loop (:777+) repeats the pattern: approximations at :797-800, catch-up at :833, assertEquals(doc1, doc2) at :838.
• What Lucene promises. TwoPhaseIterator#approximation (TwoPhaseIterator.java:88-90): "The returned DocIdSetIterator is a superset of the matching documents, and each match needs to be confirmed with matches()". Nothing ties the approximations of two score modes together, and checkTopScores (:690) has no javadoc.

• BooleanScorerSupplier.java:589-590 returns a WANDScorer for any minShouldMatch > 1, in both modes. That is the scorer path doCheckMaxScores uses (CheckHits.java:719-727); nothing here is claimed about the bulk scorers IndexSearcher.search may use.
• WANDScorer parks clauses in its "tail" (WANDScorer.java:601): up to minShouldMatch - 1 of them and, in TOP_SCORES, also clauses whose summed score bound is below the competitive threshold (0 when this repro fails). A doc that only parked clauses contain is never a candidate. The candidate is the top of the "head" (:300-319).
• COMPLETE has no score bounds (scaling factor 0, :195), so it parks clauses by cost only (:674-687). In the 2-doc index it parks c; a and b sit on doc 1, so its candidates are [1].
• TOP_SCORES, on entering a new block (:307-312), recomputes the score bounds and moves parked clauses back while their summed bound is at least the competitive threshold (:479-485). The threshold starts at 0 (:163) and this loop has no minimum-should-match term, so the parked clause, c, comes back and doc 0 becomes a candidate. matches() rejects it (freq + tailSize < minShouldMatch, :342-344).
• A probe on the 2-doc index (exploration only; it calls setTopLevelScoringClause() in both modes and never setMinCompetitiveScore) shows exactly this: COMPLETE head docs [1, 1] with one parked clause, TOP_SCORES head docs [0, 1, 1] with none; all three clauses have the same max score, 0.6931472. The diagram's probe, which builds both scorers exactly as doCheckMaxScores does, prints the same candidate lists.

Making the tail loop at WANDScorer.java:481 respect minimum-should-match makes the 2-doc test pass. But the same change still fails two larger inputs of an independent check on 645d4fe14a20: a 300-doc pattern (expected:<111> but was:<110>) and a boosted (a b c^10)~2 (expected:<7> but was:<6>). That verifier's reading: TOP_SCORES parks clauses by score bound and COMPLETE by cost, so the two lists differ for other reasons too. So the tested variant does not restore the lockstep for the larger inputs; no other scorer-side change was tried, so one is not ruled out.

• Who. Test code, in Lucene or downstream: the test framework "can also be used for testing third-party code that uses the Lucene API" (lucene/test-framework/src/java/overview.html:24-25). The trigger is an ordinary BooleanQuery of TermQuery SHOULD clauses with setMinimumNumberShouldMatch(2).
• Ordinary settings. Default IndexWriterConfig and similarity, a plain IndexSearcher, and Lucene's standard codec pinned with TestUtil.getDefaultCodec(); assertions on or off. With the pin removed, the test failed 10 of 10 seeds under LuceneTestCase's randomized codecs (fact check), and an independent run with newDirectory / newSearcher failed 20 of 20.
• Not every index. It needs a doc that one mode proposes and the other does not: small or sparse indexes such as the 2-doc one trigger it; a dense layout in the study's security probe passed.
• Effect. A false AssertionError, which also hides what the rest of checkTopScores would check for that query (score equality, max-score bounds). No effect on search results in any recorded run: 20 probe searches on a 200,000-doc segment, with deleted docs and FILTER, MUST_NOT and nested restrictions, returned exactly the COMPLETE hits in TOP_SCORES mode, and the repro's and verifiers' inputs agree too. That is evidence on these inputs, not a proof for every query.
• Why nobody has hit it. None of the 24 upstream checkTopScores call sites (8 classes in lucene/core tests) sets minimumNumberShouldMatch; TestWANDScorer has minimum-should-match tests but does not pass them to the checker. The study's GitHub code searches of the Elasticsearch, OpenSearch and Solr default branches found no caller.

• Inputs. ByteBuffersDirectory, default IndexWriterConfig (standard analyzer, BM25) with TestUtil.getDefaultCodec(), a plain IndexSearcher: nothing else is randomized. The random() passed to checkTopScores is not reached before the failing assertion (seeds DEADBEEF and 5EED1234 fail the same way).
• Assertions. Pass -Ptests.asserts=true or false explicitly; it fails the same way in both modes.
• Releases. The same file runs unchanged on the releases tried: it fails on 10.1.0, 10.5.2 and 10.6.0 and passes on 9.11.1, 9.12.3 and 10.0.0; on those three the false failure needs a larger input, such as the study's 300-doc pattern or a boosted (a b c^10)~2 (see the history below). It was not run before 9.11.1.
• As the regression test: this same method, for example added to TestWANDScorer (same package).

cp $REPRO/TestCheckTopScoresMinShouldMatch.java $LUCENE/lucene/core/src/test/org/apache/lucene/search/
cd $LUCENE
./gradlew :lucene:core:test --tests org.apache.lucene.search.TestCheckTopScoresMinShouldMatch -Ptests.seed=DEADBEEF -Ptests.asserts=true
# assertions off: -Ptests.asserts=false (same failure)

# gradle, main 645d4fe14a20, seed DEADBEEF, -Ptests.asserts=true
TestCheckTopScoresMinShouldMatch > testMinShouldMatch FAILED
    java.lang.AssertionError: expected:<1> but was:<0>
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:240808E5B9A8FCC1]:0)
        at org.junit.Assert.fail(Assert.java:89)
        at org.junit.Assert.failNotEquals(Assert.java:835)
        at org.junit.Assert.assertEquals(Assert.java:647)
        at org.junit.Assert.assertEquals(Assert.java:633)
        at org.apache.lucene.tests.search.CheckHits.doCheckMaxScores(CheckHits.java:751)
        at org.apache.lucene.tests.search.CheckHits.checkTopScores(CheckHits.java:697)
        at org.apache.lucene.search.TestCheckTopScoresMinShouldMatch.testMinShouldMatch(TestCheckTopScoresMinShouldMatch.java:53)
1 test completed, 1 failed
# -Ptests.asserts=false: the same AssertionError at CheckHits.java:751
# with fix A or fix B: BUILD SUCCESSFUL, 1 test, assertions on and off

# exploration probe on the same index (not part of the repro)
COMPLETE   WANDScorer approximation: [1 (match, score 0.5545178)]
TOP_SCORES WANDScorer approximation: [0 (no match), 1 (match, score 0.5545178)]
IndexSearcher.search: 1 hit, doc 1, score 0.5545178

• The contract. TwoPhaseIterator.java:88-90 allows any superset of the matches. checkTopScores (CheckHits.java:690) states no precondition on the query; the class javadoc says only "Utility class for asserting expected hits in tests." (:57), and the first loop calls itself a check "when iterating all matches" (:717).
• The query is right. In the same test the checker's own first stage (doCheckTopScores, top 1 and top 10) passes, and IndexSearcher.search returns the same single hit, doc 1, score 0.5545178.
• Precedent. In #13069 / #13088 (2024) another COMPLETE vs TOP_SCORES structural difference (a null TOP_SCORES scorer) broke doCheckMaxScores, and the change went into CheckHits; jpountz (on #13069) asked for a follow-up assertion "to make sure that both scorers produce the same hits", which became #13088. #13120 / #13122 did the same for the advance loop; on #13120 Chris Hegarty wrote "I believe that this is a test bug."
• Latent, then reachable. The lockstep came with checkTopScores in LUCENE-8135 (2018, 8.0.0) and held while both modes used MinShouldMatchSumScorer for msm > 1. LUCENE-9346 (8.8.0) moved TOP_SCORES to WANDScorer, and suitable inputs have failed ever since. The assumption was never written down; it was simply never exercised.

The strongest case for "intended": the lockstep is a deliberate white-box check that TOP_SCORES never visits a candidate COMPLETE skips. In the lucene-solr#2141 review (comment r550807975, 2021-01-01) jpountz kept a filter in WANDScorer's approximation "so that the approximation doesn't return documents knowing that no match is possible", so a reviewer may ask whether WANDScorer should change instead. The lockstep is also one-sided: TOP_SCORES may skip COMPLETE candidates (if they score below the threshold) but may not add its own, which reads as a coherent pruning invariant; a maintainer could call the extra TOP_SCORES candidate a minor WANDScorer inefficiency and decline to relax CheckHits (this doc's cross-model fact check).

Why we still think it is not intended: no javadoc, comment, CHANGES or MIGRATE entry states such an invariant, and TwoPhaseIterator promises only a superset. #13996 (10.1.0, by the same author) then moved that approximation-side filter into matches() on purpose ("moves work from advance(int target) to TwoPhaseIterator#matches()"). And the one scorer-side change we tried, an msm-aware tail loop, fixes the 2-doc input, but two larger inputs still fail. Ratings of the counter-case: weak (cross-model); moderate (the cross-model refute review and this doc's cross-model fact check); the independent reproduction, the intended-behaviour check and the adjudication rejected it without a strength label. Our verdict: not intended, a test-framework defect; the open question is whether checkTopScores should require equal candidate lists, or only agreeing hits, scores and score bounds.

Noise call (contested): the cross-model fact check also argues that this is noise rather than a bug, because its whole effect is a false test failure. It stays confirmed here: the defect is the behaviour of a public test-framework API that downstream test suites call directly, and it fails with JVM assertions on and off; it is not a Java assert tripping inside Lucene.

Both loops need the change: with only the first loop relaxed, the advance loop fails on its own at its assertEquals(doc1, doc2) (CheckHits.java:838) for 2 of 3 seeds (DEADBEEF and 1A2B3C4D fail, 5EED1234 passes).

Not recommended:

• Changing only WANDScorer (for example making the tail loop at WANDScorer.java:481 respect minimum-should-match): the variant we tried makes the 2-doc test pass, but larger inputs still fail. It may be a small efficiency gain on its own (the study's security probe saw 0-1 extra candidates per query); that is a separate question.
• Documenting a precondition ("no minimumNumberShouldMatch > 1") on checkTopScores: it would exclude an ordinary query shape for no stated reason.

In both loops of doCheckMaxScores, hold the COMPLETE side's next candidate across iterations; when it is already past doc2, doc2 is a candidate only TOP_SCORES proposed, so assert twoPhase2.matches() == false and move on. The advance loop advances the COMPLETE side only when it is behind target, because DocIdSetIterator#advance must not go backwards.

diff --git a/lucene/test-framework/src/java/org/apache/lucene/tests/search/CheckHits.java b/lucene/test-framework/src/java/org/apache/lucene/tests/search/CheckHits.java
index 5fb954b2d85..a07cc0796b4 100644
--- a/lucene/test-framework/src/java/org/apache/lucene/tests/search/CheckHits.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/tests/search/CheckHits.java
@@ -741,14 +741,19 @@ public class CheckHits {
       int upTo = -1;
       float maxScore = 0;
       float minScore = 0;
+      int doc1 = approx1.nextDoc();
       for (int doc2 = approx2.nextDoc(); ; doc2 = approx2.nextDoc()) {
-        int doc1;
-        for (doc1 = approx1.nextDoc(); doc1 < doc2; doc1 = approx1.nextDoc()) {
+        for (; doc1 < doc2; doc1 = approx1.nextDoc()) {
           if (twoPhase1 == null || twoPhase1.matches()) {
             assertTrue(s1.score() < minScore);
           }
         }
-        assertEquals(doc1, doc2);
+        if (doc1 > doc2) {
+          // Approximations only need to be supersets of the matches: the TOP_SCORES one may stop
+          // on a candidate that the COMPLETE one skipped, as long as it does not match.
+          assertTrue(twoPhase2 != null && twoPhase2.matches() == false);
+          continue;
+        }
         if (doc2 == DocIdSetIterator.NO_MORE_DOCS) {
           break;
         }
@@ -771,6 +776,7 @@ public class CheckHits {
             s2.setMinCompetitiveScore(minScore);
           }
         }
+        doc1 = approx1.nextDoc();
       }
     }
 
@@ -802,6 +808,7 @@ public class CheckHits {
       int upTo = -1;
       float minScore = 0;
       float maxScore = 0;
+      int doc1 = -1;
       while (true) {
         int doc2 = s2.docID();
         boolean advance;
@@ -829,13 +836,19 @@ public class CheckHits {
           doc2 = approx2.nextDoc();
         }
 
-        int doc1;
-        for (doc1 = approx1.advance(target); doc1 < doc2; doc1 = approx1.nextDoc()) {
+        if (doc1 < target) {
+          doc1 = approx1.advance(target);
+        }
+        for (; doc1 < doc2; doc1 = approx1.nextDoc()) {
           if (twoPhase1 == null || twoPhase1.matches()) {
             assertTrue(s1.score() < minScore);
           }
         }
-        assertEquals(doc1, doc2);
+        if (doc1 > doc2) {
+          // See the first loop: an extra TOP_SCORES candidate must not match.
+          assertTrue(twoPhase2 != null && twoPhase2.matches() == false);
+          continue;
+        }
 
         if (doc2 == DocIdSetIterator.NO_MORE_DOCS) {
           break;

Trade-offs

• Relaxes only the unsupported assertion. The checker still walks the raw approximations, still calls advanceShallow / getMaxScore and setMinCompetitiveScore between candidates, still requires COMPLETE matches that TOP_SCORES skipped to score below the threshold, and still fails if TOP_SCORES matches a doc that COMPLETE did not propose. One check is lost: in the first loop the continue for a candidate only TOP_SCORES proposed comes before advanceShallow / getMaxScore, so upTo >= doc2 is not checked at such a doc (the score checks apply only to matches, and the advance loop calls advanceShallow at doc2 only for matches anyway).
• About 25 changed lines, in CheckHits.java only.

What the checks validated

• Gradle, scratch worktree of 645d4fe14a20, seed DEADBEEF: the minimal test fails before and passes after, with -Ptests.asserts=true and false.
• The 8 classes that call checkTopScores (TestWANDScorer, TestReqOptSumScorer, TestBlockMaxConjunction, TestDisjunctionMaxQuery, TestBayesianScoreQuery, TestLogOddsFusionQuery, TestLongDistanceFeatureQuery, TestLatLonPointDistanceFeatureQuery) plus the minimal test: 129 tests, 0 failures, assertions on and off (3 @Nightly tests skipped, so 126 ran), and 129 of 129 with -Ptests.nightly=true.
• The study's two larger independent tests (7 docs with 4 tests; 300 docs, plain and boosted): 6 of 6 pass.
• :lucene:test-framework:checkGoogleJavaFormat and forbiddenApisMain pass.
• Re-run by the fact check (2026-11-01): the 1-test pass with fix B standalone, assertions on and off, and the 9-class gradle run (129 tests, 3 skipped, 0 failures).
• Not run: the whole gradlew check. That B still catches a real extra TOP_SCORES match is by reading, not mutation-tested.

In both loops use s1.iterator() / s2.iterator() (for WANDScorer, TwoPhaseIterator.asDocIdSetIterator, so matches() is applied inside) and leave the two-phase objects null. The same change appears again at :797-800 in the advance loop.

--- a/lucene/test-framework/src/java/org/apache/lucene/tests/search/CheckHits.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/tests/search/CheckHits.java
@@ -734,10 +734,12 @@
         assertTrue(s1.iterator().nextDoc() == DocIdSetIterator.NO_MORE_DOCS);
         continue;
       }
-      TwoPhaseIterator twoPhase1 = s1.twoPhaseIterator();
-      TwoPhaseIterator twoPhase2 = s2.twoPhaseIterator();
-      DocIdSetIterator approx1 = twoPhase1 == null ? s1.iterator() : twoPhase1.approximation();
-      DocIdSetIterator approx2 = twoPhase2 == null ? s2.iterator() : twoPhase2.approximation();
+      // Compare matching docs only: the two-phase approximations of the COMPLETE and TOP_SCORES
+      // scorers are both valid supersets of the matches but need not be equal.
+      TwoPhaseIterator twoPhase1 = null;
+      TwoPhaseIterator twoPhase2 = null;
+      DocIdSetIterator approx1 = s1.iterator();
+      DocIdSetIterator approx2 = s2.iterator();
       int upTo = -1;
       float maxScore = 0;
       float minScore = 0;
@@ -794,10 +796,12 @@
         assertTrue(s1.iterator().nextDoc() == DocIdSetIterator.NO_MORE_DOCS);
         continue;
       }
-      TwoPhaseIterator twoPhase1 = s1.twoPhaseIterator();
-      TwoPhaseIterator twoPhase2 = s2.twoPhaseIterator();
-      DocIdSetIterator approx1 = twoPhase1 == null ? s1.iterator() : twoPhase1.approximation();
-      DocIdSetIterator approx2 = twoPhase2 == null ? s2.iterator() : twoPhase2.approximation();
+      // Compare matching docs only: the two-phase approximations of the COMPLETE and TOP_SCORES
+      // scorers are both valid supersets of the matches but need not be equal.
+      TwoPhaseIterator twoPhase1 = null;
+      TwoPhaseIterator twoPhase2 = null;
+      DocIdSetIterator approx1 = s1.iterator();
+      DocIdSetIterator approx2 = s2.iterator();
 
       int upTo = -1;
       float minScore = 0;

Trade-offs

• Four lines per loop, and it matches the first loop's own comment ("when iterating all matches").
• The checker no longer sees non-matching candidates: advanceShallow / getMaxScore run only on matching docs, and the interleaving of candidate steps with setMinCompetitiveScore is exercised only inside the iterator. A committer may prefer to drop the always-null twoPhase variables (cosmetic, larger diff).

What the checks validated

• The same gradle runs as B: the minimal test fails before and passes after, assertions on and off; 129 tests with assertions on and off (3 @Nightly skipped) and 129 of 129 with -Ptests.nightly=true; checkGoogleJavaFormat passes.
• In the study: the finder's, the independent reproduction's, the intended-behaviour check's and the cross-model reviewers' tests pass with it, and a port to 8.8.0's CheckHits passes 80 of 80 over 20 seeds.
• Study caveat: with A, the finder's own scripted-scorer fuzz harness still reported failures on a top-level DisjunctionMaxQuery over scripted leaves, at the score-equality assertion; the finder judged that a separate checker-strictness question and did not report it. Upstream TestDisjunctionMaxQuery passes.
• Not run: the whole gradlew check; not mutation-tested.

• Harness. A hunt unit (WAND and block-max score bounds) built a scripted-scorer fuzzer with an exact dyadic brute-force oracle and a score-bound recorder. With CheckHits.checkTopScores added as a second oracle, it failed on every query with a nested or top-level minimumNumberShouldMatch > 1 WANDScorer while the exact oracle passed.
• Reduction. The finder minimised it by hand to (f:a f:b f:c)~2 over a 300-doc pattern on the stock jars, and the hunt later reduced that to the 2-doc, one-method test (2026-10-29).
• Verification. An independent reproducer (own tests, from the claim only), cross-model lenses (OpenAI GPT-5.6, refute and intended), an intended-behaviour devil's advocate and an adjudication all called it real; the version-history runs placed the first affected release; a credibility audit kept it.