GITHUB#15239: Honor exhaustive ScoreMode in BatchScoreBulkScorer by NextbrickInc · Pull Request #16542 · apache/lucene

NextbrickInc · 2026-08-21T04:11:56Z

Summary

Fixes Change in behavior using SimpleCollector+TopScoreDocCollector between 10.2 and 10.3 when scoreMode==COMPLETE #15239.
Make BatchScoreBulkScorer honor exhaustive ScoreMode before using a minimum competitive score to skip matches.
Preserve all matches for ScoreMode.COMPLETE when collectors with different strategies are combined.
Add regression coverage based on the SimpleCollector + TopScoreDocCollector scenario from the issue.

Rationale

This follows the issue discussion: queries check ScoreMode eagerly before skipping, and BatchScoreBulkScorer should do the same.

Validation

./gradlew :lucene:core:test --tests org.apache.lucene.search.TestMultiCollector (15 tests)
./gradlew :lucene:core:checkGoogleJavaFormat

Nested TopScoreDocCollectors can still call setMinCompetitiveScore, but COMPLETE searches must visit every match. Co-authored-by: Cursor <cursoragent@cursor.com>

NextbrickInc · 2026-08-26T08:40:32Z

@jimczi This implements the eager ScoreMode check you suggested in #15239 so BatchScoreBulkScorer does not skip matches for exhaustive collection. The focused TestMultiCollector suite passes all 15 tests and Google Java format passes. I would appreciate a review when you have time.

NextbrickInc · 2026-08-26T21:38:32Z

@jimczi , a brief follow-up when you have time: this applies the eager ScoreMode check discussed in #15239, and all CI checks are green. The regression test covers exhaustive COMPLETE collection wrapping a TopScoreDocCollector. Could you confirm whether this matches the intended fix, or suggest another search maintainer who could review it? Thank you.

Pls approve, merge sir. when you get chance.

jimczi · 2026-08-27T07:49:07Z

Thanks @NextbrickInc, the approach looks good to me, that's what I had in mind in the issue.

I checked out the branch locally: the new test does fail without the change in BatchScoreBulkScorer, and TestBooleanScorer, TestTermQuery, TestCombinedFieldQuery, TestTopDocsCollector, TestBooleanQuery all still pass.

A few comments:

DenseConjunctionBulkScorer has the same hole. It does if (scorable.minCompetitiveScore > scorable.score) return NO_MORE_DOCS; without checking the score mode, and it is reachable with COMPLETE: BooleanScorerSupplier builds it for a FILTER-only conjunction under scoreMode != TOP_SCORES with a constant score of 0f. With a 5000 docs index and a two clauses FILTER-only BooleanQuery, using the same collector as the new test, I get 4096 collected docs instead of 5000 on this branch. A single setMinCompetitiveScore(nextUp(0f)) kills the second window.

That one predates the 10.3 regression (it came with #14293) so I'm fine if you want to keep it out of scope, but if we state that an exhaustive score mode never prunes then we should be consistent. I'd rather fix it here, it's a two lines change. I checked the other SimpleScorable users and this is the only remaining one: BlockMaxConjunctionBulkScorer and DisjunctionMaxBulkScorer are TOP_SCORES only, the rest don't look at the min score.

The ScoreMode.COMPLETE javadoc doesn't match the code. It says the API is "only honored for TOP_SCORES" but the check is isExhaustive() == false, so TOP_DOCS_WITH_SCORES honors it too. It also mentions Scorer#setMinCompetitiveScore and "callers", while the contract we care about is Scorable#setMinCompetitiveScore, which is called by the collector. That javadoc already says "This method may only be called from collectors that use ScoreMode.TOP_SCORES", which is exactly what @hossman was looking for, so I'd just link to it and drop the (GITHUB#15239) reference, we don't usually put issue numbers in public javadocs.

CHANGES.txt: the entry is only in the 11.0.0 section. The regression is in 10.3 so we'll want it in the 10.6.0 section as well when this gets backported.

Minor nits:

newSearcher(reader, false, false, false) disables the asserting searcher. I ran it with wrapWithAssertions=true and it passes, so we can keep the extra coverage.
The reused Document in the test is load bearing (doc i ends up with bar0..bari, which is what makes bar1 match 999 docs with different norms). Worth a one line comment, it reads like a bug otherwise.
No test for the CombinedFieldQuery call site.

Nothing blocking, the change itself is correct. Mostly want to agree on the DenseConjunctionBulkScorer part before merging.

NextbrickInc · 2026-08-27T09:37:02Z

@jimczi

Thanks for the thorough review — I went through every point locally and they all hold up.
Full write-up with the runs attached; the short version:

DenseConjunctionBulkScorer — confirmed, and it's worse than we thought.

Your repro lands exactly: 5000 docs, two FILTER clauses, the collector from the new test →
4096 collected instead of 5000.

But it isn't only BooleanScorerSupplier. ConstantScoreScorerSupplier#bulkScorer() also
builds a DenseConjunctionBulkScorer, and it doesn't consult the score mode at all. That
supplier backs MatchAllDocsQuery, FieldExistsQuery and point ranges, whose constant score
is the boost rather than 0f — and nextUp(1.0f) > 1.0f, so the same window abort fires.

Those queries look fine today only because scoreWindow() takes the
collectRange(min, minDocIDRunEnd) shortcut when acceptDocs == null and every clause matches
the whole run: the segment is collected in one call, so the check at the top of the loop only
runs once. Delete a single document and acceptDocs becomes non-null, the shortcut is skipped,
and it goes window by window. On a 100k-doc segment with one deletion, COMPLETE:

query	main	this PR	+ Dense fix	expected
`:`	4,095	4,095	99,999	99,999
`FieldExistsQuery[n]`	4,095	4,095	99,999	99,999
`p:[0 TO 100000]`	4,095	4,095	99,999	99,999
`#a:x #b:y`	4,095	4,095	99,999	99,999
`a:x`	2	99,999	99,999	99,999
`a:x` (COMPLETE_NO_SCORES)	4,095	4,095	99,999	99,999

So I'd rather not leave it out of scope — *:* is the most common query there is, and shipping
"an exhaustive score mode never prunes" while half the exhaustive paths still do would be worse
than the status quo. It's the same two-line shape as the BatchScoreBulkScorer change:

private final SimpleScorable scorable;

private final boolean applyMinCompetitiveScore;
// Exhaustive collection must visit every match even if a nested collector calls
// setMinCompetitiveScore (GITHUB#15239).
this.applyMinCompetitiveScore = scoreMode.isExhaustive() == false;

while (min < max) {

if (scorable.minCompetitiveScore > scorable.score) {

if (applyMinCompetitiveScore && scorable.minCompetitiveScore > scorable.score) {

plus threading scoreMode through BooleanScorerSupplier:364, :437 and
ConstantScoreScorerSupplier:92. Keeping the existing 4-arg constructor as a delegate that
passes TOP_SCORES leaves the 45 call sites in TestDenseConjunctionBulkScorer untouched.

I also audited the rest: BlockMaxConjunctionBulkScorer, MaxScoreBulkScorer and
DisjunctionMaxBulkScorer are TOP_SCORES-only by construction, and BooleanScorer /
SortRescorer hold a SimpleScorable but never read minCompetitiveScore. So this is the
last one.

Javadoc — agreed, and it's measurable rather than theoretical: the same TermQuery that
collects all 1000 matches under COMPLETE collects 2 under TOP_DOCS_WITH_SCORES. I'll link
Scorable#setMinCompetitiveScore and drop the (GITHUB#15239) reference.

CHANGES.txt — agreed, 10.6.0 too. Worth noting the DenseConjunctionBulkScorer check
arrived earlier, in 10.2.0 (#14293), so that half is affected from 10.2.0 onward.

Nits — all three taken. newSearcher(reader, true, true, true) passes, so I'll keep the
asserting searcher. Comment added for the reused Document (doc i accumulates bar0..bari,
which is what makes bar1 match 999 docs with distinct norms). And there's now a
CombinedFieldQuery test — it fails on main and passes here, at 1k/20k/65k docs.

Validation: new suite TestExhaustiveScoreModeNoPruning (14 tests, 1k–100k docs) — 14/14
pass with the fix, 5 fail without. :lucene:core:test 8694 tests × 4 seeds, all green.
Downstream modules (queries, sandbox, facet, join, grouping, suggest, misc, queryparser,
highlighter, monitor, memory, classification, expressions, spatial-*, backward-codecs,
analysis:common) — 7227 tests, all green. checkGoogleJavaFormat clean. TOP_SCORES latency
on a 1M-doc segment is within ±1.9% (best-of-20 × 3), so the pruning path is untouched.

Lucene_PR16542_Technical_Review_NextBricks_Shrey_Narayan.pdf
Lucene_PR16542_Technical_Review_NextBricks_Shrey_Narayan.pdf

…r too DenseConjunctionBulkScorer aborts the remaining windows whenever minCompetitiveScore exceeds its constant score, without checking the score mode. It is reachable with an exhaustive ScoreMode from two places: BooleanScorerSupplier, for a FILTER-only conjunction, and ConstantScoreScorerSupplier, which backs MatchAllDocsQuery, FieldExistsQuery and point range queries. Without deletions the ConstantScoreScorerSupplier path is hidden by the collectRange() shortcut in scoreWindow(), which collects the whole run in one call so the check only runs once. With a single deleted document acceptDocs is non-null, the shortcut no longer applies, and a COMPLETE search for *:* over a 100k-doc segment collects 4095 documents. Thread the ScoreMode through and skip the check when it is exhaustive, the same way BatchScoreBulkScorer now does. The existing four-argument constructor is kept as a delegate that passes TOP_SCORES, so the pruning semantics of TestDenseConjunctionBulkScorer are unchanged. Also address review feedback: - ScoreMode.COMPLETE javadoc now describes the exhaustive contract and points at Scorable#setMinCompetitiveScore instead of Scorer, and no longer carries an issue number. - CHANGES.txt entry added to the 10.6.0 section for the backport. - TestMultiCollector uses the asserting searcher, and the reused Document is explained. - New TestExhaustiveScoreModeNoPruning covers both bulk scorers, the CombinedFieldQuery call site, indexes from 1k to 100k documents, and asserts that TOP_SCORES still prunes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

jimczi · 2026-08-27T12:18:16Z

Thanks for the quick turnaround, the DenseConjunctionBulkScorer part looks right to me.

I pulled the branch and reverted just DenseConjunctionBulkScorer + the two suppliers: 5 of the new tests fail, including *:* with one deletion at 4095/99999 and count() vs COMPLETE collection at 8192/30000. Good catch on the ConstantScoreScorerSupplier path, I had only looked at the BooleanScorerSupplier one, and that's the one that makes this worth fixing.

Two things before this can go in:

testMatchAllWithDeletionsStillPrunesUnderTopScores is flaky. It fails with -Ptests.seed=14471C5085979DA7:

java.lang.AssertionError: TOP_SCORES should still prune, collected=99999

RandomIndexWriter.getReader() calls doRandomForceMerge(), which randomly runs forceMerge(1) or forceMergeDeletes(). When that happens the single deleted document is expunged, acceptDocs is null again, the collectRange() shortcut in scoreWindow() collects the whole segment in one call and nothing gets pruned. Same thing silently removes the deletions coverage in testConstantScoreQueriesWithDeletionsUnderComplete, that one keeps passing but stops testing what it says it tests. Use a plain IndexWriter for those two (or setDoRandomForceMerge(false)) and assert reader.hasDeletions().

The four-argument DenseConjunctionBulkScorer constructor defaulting to TOP_SCORES is a trap. ReadAheadMatchAllDocsQuery in the test tree already calls it, and it has a scoreMode in scope two lines above in get(), so it now prunes under COMPLETE. It's test-only today but it's exactly the mistake the next caller will make. I'd rather pass the score mode explicitly everywhere, the 45 call sites in TestDenseConjunctionBulkScorer are a mechanical sed.

Then some trimming on TestExhaustiveScoreModeNoPruning, 500 lines and 14 tests is a lot for what is really three or four cases:

testConstantScoreQueryCompleteVisitsEveryMatchDup duplicates testConstantScoreQueryIsUnaffected, and the Dup suffix looks like a leftover.
The {1000, 5000, 20000, 65536, 100000} loops don't buy much over one size comfortably above WINDOW_SIZE. The suite is 8.5s today, most of it indexing.
Class javadoc lists TOP_DOCS as exhaustive, but isExhaustive() is false for it. Also I'd drop the PR number and the "verification harness" framing, this is a permanent test now.
The comment about the ScoreMode.COMPLETE javadoc claiming "only honored for TOP_SCORES" is stale, you removed that sentence in this same commit.
A few org.apache.lucene.document.NumericDocValuesField / IntPoint fully qualified inline, worth importing.

The javadoc rewording and the 10.6.0 CHANGES entry look good. checkGoogleJavaFormat is clean and TestDenseConjunctionBulkScorer, TestSortOptimization, TestConstantScoreQuery, TestMatchAllDocsQuery, TestBooleanQuery all pass here.

jimczi

Same points as my previous comment, put inline so they are easier to act on.

jimczi · 2026-08-27T12:23:53Z

+  /** Constructor that allows dynamic pruning, for tests and callers that never collect matches. */
  DenseConjunctionBulkScorer(
      List<DocIdSetIterator> iterators,
      List<TwoPhaseIterator> twoPhases,
      int maxDoc,
      float constantScore) {
+    this(iterators, twoPhases, maxDoc, constantScore, ScoreMode.TOP_SCORES);


This default is a trap. ReadAheadMatchAllDocsQuery in the test tree already calls this constructor, and it has a scoreMode in scope two lines above in get(), so it now prunes under COMPLETE without anyone noticing. It's test-only today but it's exactly the mistake the next caller will make.

I'd rather pass the score mode explicitly everywhere and drop the delegate. The 45 call sites in TestDenseConjunctionBulkScorer are a mechanical sed.

Agreed, removed. ScoreMode is passed explicitly everywhere now, and ReadAheadMatchAllDocsQuery passes the scoreMode it already had in scope in get() — which was exactly the bug you predicted the next caller would hit. 44 call sites in TestDenseConjunctionBulkScorer plus that one.

jimczi · 2026-08-27T12:23:53Z

+        w.addDocument(doc);
+      }
+      w.forceMerge(1);
+      w.deleteDocuments(new Term("del", "yes"));


This test is flaky. It fails with -Ptests.seed=14471C5085979DA7:

java.lang.AssertionError: TOP_SCORES should still prune, collected=99999

RandomIndexWriter.getReader() calls doRandomForceMerge(), which randomly runs forceMerge(1) or forceMergeDeletes(). When that happens the deleted document is expunged, acceptDocs is null again, the collectRange() shortcut in scoreWindow() collects the whole segment in one call and nothing gets pruned.

Use a plain IndexWriter here (or setDoRandomForceMerge(false)) and assert reader.hasDeletions().

Confirmed, and thanks for the seed — your diagnosis was exactly right. Both deletion tests now use a plain IndexWriter and assert reader.hasDeletions() before the pruning assertion. Passes on -Ptests.seed=14471C5085979DA7, as does the full :lucene:core:test on that seed.

jimczi · 2026-08-27T12:23:53Z

+        w.addDocument(doc);
+      }
+      w.forceMerge(1);
+      w.deleteDocuments(new Term("del", "yes"));


Same RandomIndexWriter randomness as in testMatchAllWithDeletionsStillPrunesUnderTopScores below. This one keeps passing when the deletion is merged away (the assertions hold either way), but it silently stops testing the acceptDocs != null path, which is the whole point of the test. Worth asserting reader.hasDeletions() so it fails loudly instead.

Same fix here. Worth recording that this one was passing vacuously: with the guard disabled it now fails 30/30 iterations at 4095/19999, whereas before it depended on whether doRandomForceMerge() happened to expunge the deletion. The hasDeletions() assertion makes that loud instead of silent.

jimczi · 2026-08-27T12:23:53Z

+  // 3. DenseConjunctionBulkScorer via ConstantScoreScorerSupplier (single constant-score clause)
+  // ---------------------------------------------------------------------------------------------
+
+  public void testConstantScoreQueryCompleteVisitsEveryMatchDup() throws Exception {


This duplicates testConstantScoreQueryIsUnaffected further down, same query, same index, same assertion. And the Dup suffix looks like a leftover. Can we drop one of the two?

Leftover from an earlier iteration, removed. Kept testConstantScoreQueryIsUnaffected and folded it into the constant-score group as a regression guard on the ConstantBulkScorer wrapping.

jimczi · 2026-08-27T12:23:53Z

+              runCollect(searcher, q, ScoreMode.COMPLETE));
+          // TOP_DOCS_WITH_SCORES is NOT exhaustive, so pruning is honored there. This documents
+          // the mismatch with the ScoreMode.COMPLETE javadoc, which claims setMinCompetitiveScore
+          // is "only honored for TOP_SCORES".


Stale, you removed that sentence from the ScoreMode.COMPLETE javadoc in this same commit. The assertion itself is fine, just reword the comment to say that TOP_DOCS_WITH_SCORES is not exhaustive so pruning is expected there.

Right, I removed that sentence in this same commit. Reworded to say that TOP_DOCS_WITH_SCORES is not exhaustive, so pruning is expected there. The assertion itself is unchanged.

jimczi · 2026-08-27T12:23:53Z

+ * Verification harness for GITHUB#15239 / PR #16542.
+ *
+ * <p>An exhaustive {@link ScoreMode} (COMPLETE / COMPLETE_NO_SCORES / TOP_DOCS) must visit every


TOP_DOCS is not exhaustive, isExhaustive() returns false for it. The exhaustive modes are COMPLETE and COMPLETE_NO_SCORES.

Also I'd drop the PR number and the "verification harness" framing, this is a permanent test now, not a one-off check.

Corrected — isExhaustive() does return false for TOP_DOCS, so the list is now COMPLETE / COMPLETE_NO_SCORES. Also dropped the PR number and the "verification harness" framing; agreed it is a permanent test, not a one-off check.

jimczi · 2026-08-27T12:23:53Z

+        RandomIndexWriter w = new RandomIndexWriter(random(), dir)) {
+      for (int i = 0; i < numDocs; ++i) {
+        Document doc = new Document();
+        doc.add(new org.apache.lucene.document.NumericDocValuesField("n", i));


Fully qualified inline here and in a few other places (IntPoint too). Let's import them.

Imported. No fully qualified names remain outside the import block.

jimczi · 2026-08-27T12:23:53Z

+  // ---------------------------------------------------------------------------------------------
+
+  public void testTermQueryCompleteVisitsEveryMatchAtScale() throws Exception {
+    for (int numDocs : new int[] {1000, 5000, 20000, 65536, 100000}) {


These size loops don't buy much over a single size comfortably above WINDOW_SIZE, the failure mode is the same at 5k and at 100k. Same for the other numDocs loops in the file. The suite is 8.5s today and it's almost all indexing, which is a lot for what is really three or four distinct cases.

Agreed, the failure mode is the same at 5k and at 100k. Collapsed to a single NUM_DOCS = 20_000 throughout and merged the MatchAll / FieldExists / point-range cases onto one index. Suite is 1.6s now, down from 8.5s.

One note: I landed at 10 tests rather than the three or four you suggested — the two bulk scorers, both suppliers, the deletions path and the TOP_SCORES control did not compress further without dropping a distinct case. Say the word if you would still rather see it tighter and I will merge the FILTER-only pair and fold in the CombinedFieldQuery case.

…ollow-up Pass ScoreMode explicitly instead of defaulting it --------------------------------------------------- The four-argument DenseConjunctionBulkScorer constructor that defaulted to TOP_SCORES is removed. ReadAheadMatchAllDocsQuery already called it while holding a scoreMode two lines above in get(), so it pruned under COMPLETE; it now passes that score mode through. The 44 call sites in TestDenseConjunctionBulkScorer pass ScoreMode.TOP_SCORES explicitly, which preserves their existing pruning semantics. Stop the deletions coverage from disappearing at random ------------------------------------------------------- The two tests that need a deleted document used RandomIndexWriter, whose getReader() calls doRandomForceMerge(). When that expunged the deletion, acceptDocs became null again, the collectRange() shortcut in scoreWindow() collected the whole segment in one call, and nothing was pruned: testMatchAllWithDeletionsStillPrunesUnderTopScores failed on -Ptests.seed=14471C5085979DA7, and testConstantScoreQueriesWithDeletions- UnderComplete kept passing while silently no longer testing the acceptDocs != null path. Both now use a plain IndexWriter and assert reader.hasDeletions(). With the guard disabled the latter fails 30/30 iterations instead of intermittently. Do not enable the asserting searcher on contract-violating collectors --------------------------------------------------------------------- TestMultiCollector was changed to newSearcher(reader, true, true, true) in the previous commit. AssertingWeight sets canSetMinCompetitiveScore only for TOP_SCORES with a top level scoring clause, and AssertingScorer asserts on it, so whenever the random draw produced an AssertingIndexSearcher the test's deliberately contract-violating collector tripped the assert: 22 failures in 500 iterations. The original newSearcher(reader, false, false, false) was load bearing. Both this test and the new one now use (true, false, true), keeping reader wrapping and intra-segment concurrency without the assertion wrapping. Trim the new test suite ----------------------- Drop the duplicated testConstantScoreQueryCompleteVisitsEveryMatchDup, collapse the {1000, 5000, 20000, 65536, 100000} loops to a single NUM_DOCS = 20000 comfortably above WINDOW_SIZE, and merge the MatchAll, FieldExists and point range cases onto one index. 14 tests to 10, and the suite drops from 8.5s to 1.6s. With the guard disabled the same five tests still fail, so no coverage is lost. Also correct the class javadoc, which listed TOP_DOCS as exhaustive when isExhaustive() returns false for it, drop the PR number and "verification harness" framing, reword the comment referring to a ScoreMode.COMPLETE javadoc sentence removed in the previous commit, and import NumericDocValuesField and IntPoint instead of qualifying them inline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

# Conflicts: # lucene/CHANGES.txt

NextbrickInc · 2026-08-27T22:55:13Z

Thanks for putting these inline, that made them easy to work through. All eight points taken, and all of them were right. Pushed as 8161b4f, plus a merge of main to clear a CHANGES.txt conflict that had appeared since the last push.

Summary of the changes

The four-argument DenseConjunctionBulkScorer constructor is gone; ScoreMode is explicit at all 45 call sites, including ReadAheadMatchAllDocsQuery, which now passes the scoreMode it already held.
Both deletion tests use a plain IndexWriter and assert reader.hasDeletions().
testConstantScoreQueryCompleteVisitsEveryMatchDup removed.
All size loops collapsed to a single NUM_DOCS = 20_000, and the MatchAll / FieldExists / point-range cases merged onto one index.
Class javadoc corrected (TOP_DOCS is not exhaustive), PR number and "verification harness" framing dropped, stale ScoreMode.COMPLETE comment reworded, IntPoint / NumericDocValuesField imported.

14 tests to 10, suite 8.5s to 1.6s.

To confirm the trimming did not weaken anything: with applyMinCompetitiveScore forced true, the reduced suite still fails in exactly the five places you identified, and testConstantScoreQueriesWithDeletionsUnderComplete now fails 30/30 iterations at 4095/19999 rather than depending on the merge coin flip.

One thing I should have caught before you saw this.

Beasting turned up a flake that this PR introduces into TestMultiCollector. I had changed newSearcher(reader, false, false, false) to (true, true, true) in the previous commit. The third argument is wrapWithAssertions; AssertingWeight sets canSetMinCompetitiveScore = scoreMode == ScoreMode.TOP_SCORES && topLevelScoringClause, and AssertingScorer asserts on it. So whenever the random draw produced an AssertingIndexSearcher, the test's deliberately contract-violating collector tripped the assert:

java.lang.AssertionError
	at org.apache.lucene.tests.search.AssertingScorer.setMinCompetitiveScore(AssertingScorer.java:91)
	at org.apache.lucene.search.TopScoreDocCollector$1.updateGlobalMinCompetitiveScore(TopScoreDocCollector.java:147)
	at org.apache.lucene.search.TestMultiCollector$1.setScorer(TestMultiCollector.java:85)

22 failures in 500 iterations, seed 44073997F88AC6EA. That false, false, false was load bearing and I should not have flipped it. My own new test had the same problem, 2 in 20.

Both now use newSearcher(reader, true, false, true), which keeps the reader wrapping and intra-segment concurrency but drops the assertion wrapping. 0 failures in 500 and 400 iterations respectively, and clean under -Ptests.nightly=true.

That does raise a question I would rather you decide than decide myself: AssertingScorer currently encodes "only TOP_SCORES may call setMinCompetitiveScore", while this PR's premise is that a nested collector may call it under an exhaustive score mode and it must be harmless. I have left the assertion untouched, since relaxing it is a contract change that belongs in its own issue rather than inside this one. Happy to open that issue if you agree it is worth doing.

Verification

Check	Result
`:lucene:core:test` on `-Ptests.seed=14471C5085979DA7`	8696 tests, pass
`:lucene:core:test` full module, random seed	8696 tests, pass
`:lucene:sandbox:test`	339 tests, pass
`TestExhaustiveScoreModeNoPruning -Ptests.iters=40`	400 cases, 0 failures
`TestMultiCollector -Ptests.iters=500`	0 failures (22 before the fix)
Both suites, `-Ptests.iters=15 -Ptests.nightly=true`	0 failures
Negative control, guard disabled	5 tests fail, matching your count
`check -x test` (checkGoogleJavaFormat, ecjLint, forbiddenApis)	clean
`renderJavadoc -Pvalidation.errorprone=true`	clean

Toolchain: Temurin JDK 25.0.4.1, Gradle 9.7.0. The branch is mergeable again after the main merge.

Could you take another look when you have a moment? If it looks good to you now I would appreciate an approval and a merge. And if you would still rather see the test file tighter, or want the AssertingScorer contract settled first, say so and I will turn it around quickly.

NextbrickInc · 2026-08-27T22:57:03Z

Lucene_PR16542_Review_Response_Round2_NextBricks_Shrey_Narayan 2.pdf

NextbrickInc · 2026-08-29T23:50:14Z

@jimczi — both items you flagged as blocking are in 8161b4f.

The four-argument constructor is gone; ScoreMode is explicit at all 45 call sites, including ReadAheadMatchAllDocsQuery, which now passes the scoreMode it already held two lines above — the exact next-caller bug you predicted.

The flaky deletions test. Your diagnosis was right. Both deletion tests use a plain IndexWriter and assert reader.hasDeletions(). -Ptests.seed=14471C5085979DA7 passes, as does the full :lucene:core:test on that seed. To show the coverage is genuinely back rather than merely green: with the guard disabled, testConstantScoreQueriesWithDeletionsUnderComplete now fails 30/30 at 4095/19999, where before it depended on the doRandomForceMerge() coin flip.

Trimming per your note: 14 tests → 10, suite 8.5s → 1.6s. With the guard disabled the same five tests you identified still fail, so nothing was lost.

One I should have caught before you saw it: beasting found that this PR introduced a flake into TestMultiCollector. I'd changed newSearcher(reader, false, false, false) to true, true, true; the third argument is wrapWithAssertions, and that test deliberately violates the setMinCompetitiveScore contract, so AssertingScorer fires whenever the random draw produces an AssertingIndexSearcher. Both call sites now use true, false, true — keeping the reader wrapping and concurrency, dropping only the assertion wrapping. Clean at 500 and 400 iterations and under -Ptests.nightly=true; 22 failures before the fix.

The AssertingScorer invariant itself is left untouched — relaxing "only TOP_SCORES may call setMinCompetitiveScore" is a contract change that belongs in its own issue, not this one.

Nothing outstanding on my side. Please approve and merge when you're happy. Thanks alot.

Honor exhaustive ScoreMode in BatchScoreBulkScorer (GITHUB#15239).

d2ecda1

Nested TopScoreDocCollectors can still call setMinCompetitiveScore, but COMPLETE searches must visit every match. Co-authored-by: Cursor <cursoragent@cursor.com>

github-actions Bot added the module:core/search label Aug 21, 2026

Merge remote-tracking branch 'origin/main' into review-16542

4b55414

jimczi reviewed Aug 27, 2026

View reviewed changes

Shrey Narayan and others added 2 commits August 27, 2026 15:33

Merge remote-tracking branch 'origin/main' into pr16542

cc4e7c8

# Conflicts: # lucene/CHANGES.txt

Uh oh!

Conversation

NextbrickInc commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Rationale

Validation

Uh oh!

NextbrickInc commented Aug 26, 2026

Uh oh!

NextbrickInc commented Aug 26, 2026

Uh oh!

jimczi commented Aug 27, 2026

Uh oh!

NextbrickInc commented Aug 27, 2026

Uh oh!

jimczi commented Aug 27, 2026

Uh oh!

jimczi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NextbrickInc commented Aug 27, 2026

Uh oh!

NextbrickInc commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NextbrickInc commented Aug 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

NextbrickInc commented Aug 21, 2026 •

edited

Loading

NextbrickInc commented Aug 27, 2026 •

edited

Loading