Grouping treats new Sort() differently from Sort.RELEVANCE: wrong top documents per group across slices or shards, and a NaN maxScore

Sort.RELEVANCE is new Sort(), and new Sort(), new Sort(SortField.FIELD_SCORE) and the constant are all equal. Two-pass grouping does not treat them alike: the second pass, TopGroupsCollector, picks its per-group collector with withinGroupSort == Sort.RELEVANCE (object identity), so an equal copy is collected by the field-sort collector, whose hits carry score NaN, while TopGroups.merge checks equals and merges those hits by score. Every comparison with NaN is false, so the merged page is slice 0's hits, then slice 1's: a group shows docs [1, 2] (scores 5 and 1) instead of [1, 0] (scores 5 and 4). Since 10.5.0 the merge also takes maxScore from the first hit's NaN score. With 10.5.0's collector managers and main's GroupingSearch, a searcher with an executor and 2 or more slices merges the pages slice by slice, wrong whenever a later slice holds a better hit, and maxScore is NaN even with one slice; a hand-sharded TopGroups.merge has merged shard by shard since 8.0.0. Nothing throws, and the Sort.RELEVANCE constant is correct everywhere. A one-line fix (equals instead of ==) passes the grouping module's tests.

• Two-pass grouping: the first pass finds the top groups; the second pass (TopGroupsCollector) collects the top documents inside each of them, ordered by the within-group sort (GroupingSearch.setSortWithinGroup, default Sort.RELEVANCE).
• Slices: an IndexSearcher with an executor runs one collector per slice; the collector manager merges their results in reduce. Default slicing sorts the segments by size, largest first, and closes a slice once it holds 5 segments or more than 250,000 documents; a segment of more than 250,000 documents gets a slice of its own (IndexSearcher.java:106-108, :350-379). The slices do not depend on the executor's pool size.
• Equal sorts: Sort.RELEVANCE is new Sort() (Sort.java:41), Sort() is this(SortField.FIELD_SCORE) (:54-56), and equals compares the fields (:114-119).

1. The TopGroupsCollector.TopDocsReducer constructor (TopGroupsCollector.java:127-148) tests if (withinGroupSort == Sort.RELEVANCE) (:129). Only the constant gets a TopScoreDocCollector (:130-136); an equal copy gets a TopFieldCollector plus a MaxScoreCollector (:137-146).
2. Field-collected hits are created with score NaN (FieldValueHitQueue.java:36-38); the score lives only in FieldDoc.fields[0]. Within one slice the order and the slice's maxScore are still right: 5.0 for slice 0, 4.0 for slice 1.
3. TopGroups.merge tests docSort.equals(Sort.RELEVANCE) (TopGroups.java:183), true here, wraps the hits as plain TopDocs (:218-220) and merges them by ScoreDoc.score (:241).
4. With NaN, score < score and score > score are both false, so TopDocs.merge falls back to its tie-break, shard index then doc id (TopDocs.java:44-45, :110-120): slice 0's hits come first, [doc 1, doc 2], and doc 0 is cut by the page size 2.
5. Since #15879 (10.5.0) the merge sets maxScore = scoreDocs[0].score for a relevance sort (TopGroups.java:244-245), NaN here, and skips the per-slice values it uses for other sorts (:231-233).
6. TopGroupsCollectorManager.reduce always calls TopGroups.merge, even for one collector (TopGroupsCollectorManager.java:112-123), and on main GroupingSearch runs its second pass through it (GroupingSearch.java:197-212). One slice: right page, NaN maxScore. Two or more slices, or a hand-sharded merge: the inputs' pages one after another, so the page is also wrong whenever a later input holds a better hit, as doc 0 does here.

Line 129 is the only == comparison with a Sort constant in Lucene's main code; every other relevance check in the grouping module uses equals (AllGroupHeadsCollector.java:63, AllGroupHeadsCollectorManager.java:146, BlockGroupingCollector.java:275, :285, TopGroups.java:183, :405).

Index: 6 segments, NoMergePolicy, one group; segment 0 = doc 0 (score 4), segment 1 = doc 1 (5) and doc 2 (1), segments 2-5 = docs 3-10 (1 each). An executor (the test uses 2 threads; any pool size gives the same slices) gives slice 0 = segments 1-5 and slice 1 = segment 0.

Inside the merge (executor, new Sort(SortField.FIELD_SCORE)): slice 0 = [doc 1 (FieldDoc, score NaN, sort value 5.0), doc 2 (NaN, 1.0)], maxScore 5.0; slice 1 = [doc 0 (NaN, 4.0)], maxScore 4.0; merged = [doc 1, doc 2], maxScore NaN.

• Who is exposed. Applications that use the grouping module directly and pass an equal copy of Sort.RELEVANCE as the within-group sort, for example a sort builder that turns a requested "score" key into new Sort(SortField.FIELD_SCORE).
• Which paths, from which release (executed on the releases under Version history; the tags in between by code reading). A wrong page needs a better hit in a later slice or shard:
  • hand-sharded TopGroups.merge, the way the package docs describe sharding ("Sharding is not directly supported, though is not too difficult, if you can merge the top groups and top documents per group yourself", package-info.java:76-77): wrong page since 8.0.0 (7.x only with scores and max scores off), NaN maxScore since 10.5.0;
  • the grouping collector managers on a searcher with an executor and 2 or more slices: wrong page since 10.5.0; NaN maxScore since 10.5.0 even with one slice;
  • GroupingSearch itself: main and, by reading, 10.6.0 (#16247), wrong page with an executor and NaN maxScore even sequentially.
• Default or opt-in. Opt-in: the default within-group sort is the constant, which is correct on every path. Nothing else is needed: default codec, config and slicing.
• Scale. None: 11 documents in the test; the bounded-exhaustive check saw wrong pages at 2 documents, one per slice or shard. Deterministic.
• What you see. No error, the same with assertions on and off. Groups, totals and counts are right; only which matching documents a group shows, their order, and maxScore change. Comparisons with NaN are false, so a rule that drops groups with maxScore < threshold keeps every group, and one that keeps groups with maxScore >= threshold drops every group.
• Downstream (read, not run). Solr normalizes a plain score desc to the constant; Elasticsearch and OpenSearch do not use TopGroupsCollector. That fits the absence of reports since 2017, and says nothing about private code.

# Lucene main 645d4fe14a20, assertions on (-ea), seed DEADBEEF: 1 test(s) run, 1 failed
    => java.lang.AssertionError: shard of group g's top hit expected:<1> but was:<0>
       org.junit.Assert.fail(Assert.java:89)
       org.junit.Assert.failNotEquals(Assert.java:835)
       org.junit.Assert.assertEquals(Assert.java:647)
       org.apache.lucene.search.grouping.TestEqualRelevanceSortShardMerge.testMergedGroupKeepsBestHit(TestEqualRelevanceSortShardMerge.java:64)

• Contracts. Sort class javadoc: "A Sort can be created with an empty constructor, yielding an object that will instruct searches to return their hits sorted by relevance" (Sort.java:27-29); Sort() "Sorts by computed relevance" and delegates to Sort(SortField.FIELD_SCORE) (:49-56). setSortWithinGroup: "Specified how documents inside a group are sorted. Defaults to Sort#RELEVANCE." (GroupingSearch.java:296-305). GroupDocs.maxScore: "Max score in this group" (GroupDocs.java:27). TopGroups.merge: "Merges a list of TopGroups, for example obtained from the second-pass collector across multiple shards" (TopGroups.java:112-115).
• History. LUCENE-7701 (caf329717ef, 7.0.0) removed if (withinGroupSort.equals(Sort.RELEVANCE)) { // optimize to use TopScoreDocCollector from the old second-pass collector and wrote withinGroupSort == Sort.RELEVANCE in the new one; the same commit used sort.equals(Sort.RELEVANCE) in the new AllGroupHeadsCollector, and its MIGRATE entry says "To reproduce the old behaviour of SecondPassGroupingCollector, you should instead use TopGroupsCollector." No comment in the issue mentions the change. LUCENE-6900 (2015) and SOLR-9783 (2016) had deliberately made the same check equals.
• Tests. TestAllGroupHeadsCollector passes new Sort(SortField.FIELD_SCORE) as the within-group sort of AllGroupHeadsCollectorManager (which tests equals) and expects the group heads in score order (TestAllGroupHeadsCollector.java:363-371, :511-514, :542-548): upstream already treats an equal copy as relevance there. No test passes an equal copy as the within-group sort of TopGroupsCollector, TopGroupsCollectorManager or GroupingSearch: TestGrouping.getRandomSort always appends an id field (TestGrouping.java:524-542), and the other grouping tests use Sort.RELEVANCE or field sorts.
• One-line swaps flip it both ways (executed): 6.6.6 with == fails like 7.0.0; 7.0.0, 8.0.0, 9.12.3 and 10.5.2 with equals pass.

The strongest case for "intended" (built by OpenAI GPT-5.6 reviewers, a devil's-advocate AI tool and the adjudication): grouping is @lucene.experimental; several javadocs say "if you want to groupSort by relevance use Sort.RELEVANCE" (FirstPassGroupingCollector.java:71, :85, BlockGroupingCollector.java:203, TopGroups.java:363); the package docs say sharding is not directly supported; and LUCENE-6528 (2015, "Not A Problem") says NaN scores from a FIELD_SCORE field sort are expected.

Why it fails (every reviewer rated it weak): those javadoc lines are about the group sort and about passing a non-null sort (LUCENE-6900), not about object identity; TopGroups.merge itself uses equals and treats the sort as relevance, so the module disagrees with itself; the sequential maxScore NaN on main involves no sharding, with setIncludeMaxScore(true) requested; LUCENE-6528 explains why the hits carry NaN, while the defect is that the merge orders by those NaN scores and takes maxScore from them; and @lucene.experimental marks an unstable API, not permission to return wrong results.

What does not fail is compatibility: new Sort(SortField.FIELD_SCORE) is a field sort by construction, GroupDocs says the hits "may be FieldDoc instances if the withinGroupSort sorted by fields" (GroupDocs.java:29-30), and callers may rely on getting FieldDocs. That bears on the shape of the fix (A returns ScoreDocs, as 6.x did; a type-aware merge, option C, keeps FieldDocs), not on whether the wrong page and the NaN maxScore are intended.

One fix was built and validated in a scratch worktree at ea57360912ed, removed afterwards; the other options were not run.

In TopGroupsCollector.TopDocsReducer, replace withinGroupSort == Sort.RELEVANCE with withinGroupSort.equals(Sort.RELEVANCE). An equal copy then takes the same score-collector path as the constant, which is what 6.x did.

-      if (withinGroupSort == Sort.RELEVANCE) {
+      if (withinGroupSort.equals(Sort.RELEVANCE)) {

Trade-offs

• Hit type changes. Hits of an equal copy become plain ScoreDocs with real scores instead of FieldDocs with score NaN (probe: stock FieldDoc doc=1 score=NaN maxScore=NaN, fixed ScoreDoc doc=1 score=5.0 maxScore=5.0, the same as Sort.RELEVANCE). A caller that casts them to FieldDoc or reads fields[0] would break; GroupDocs documents FieldDoc hits only "if the withinGroupSort sorted by fields" (GroupDocs.java:29-30). The hits then come from TopScoreDocCollector instead of TopFieldCollector.
• Cost. Not benchmarked: one Sort.equals call per collector construction, and an equal copy then uses the constant's collector.
• Scope. It fixes what Lucene's own collectors produce. TopGroups.merge still treats any input whose docSort equals Sort.RELEVANCE as score-sorted, so FieldDocs with score NaN from another producer (a custom second-pass collector) would still merge in slice order; option C covers that.
• Related open PR. #16569 edits the same constructor and keeps ==; the repros still fail with it. A 3-way merge of the fixed file with the PR's head on their shared base is clean (0 conflicts).

What the checks validated

• Gradle, scratch worktree at ea57360912ed, OpenJDK 25.0.3: the test fails 3 of 5 on stock (assertions on and off) and passes 5 of 5 with the fix.
• :lucene:grouping:test with the fix and the test: 15 classes, 61 tests (56 upstream + 5 new), 0 failures, with assertions on, off, and seed 5A17C0DE.
• :lucene:grouping:check -x test passes (google-java-format, ecjLint, forbiddenApis) after tidy re-wrapped two javadoc paragraphs of the test.
• Standalone harness with the fix shadowed: the test passes 5 of 5 with assertions on and off (seeds DEADBEEF and 7A11C0DE); the independent reproducer's 17 tests, the refute reviewer's tests and 56 upstream grouping test methods pass; the security probe shows 0 pages out of score order and no NaN maxScore.

GroupingSearch.setSortWithinGroup, the TopGroupsCollector constructor and TopGroupsCollectorManager replace a sort equal to Sort.RELEVANCE with the constant.

Trade-offs

Same effect for collection, but more places to change, and it keeps the identity check that the rest of the module avoids.

Merge with TopFieldDocs whenever the hits are FieldDocs, and keep the per-shard maxScore.

Trade-offs

Keeps the FieldDoc hit type for callers that rely on it and also fixes merged results from any other producer, but touches the merge for every caller and leaves the collector's identity check in place. Switching the merge to == instead would be wrong: it would break callers that collect with the constant and merge with new Sort(), since the hits would then be treated as FieldDocs. A is the minimal change; C is a real alternative, or an addition to A.

Say in the javadoc that only the constant means relevance.

Trade-offs

Makes Sort.equals meaningless for this API and leaves every existing caller wrong.

Method: code reading (the hunt's non-formal tier), a hunt unit (grouping), 2026-10-26.

• The finder, an AI tool, compared the relevance check in TopGroupsCollector (==) with the one in TopGroups.merge (equals) and wrote a scratch probe that showed the wrong page.
• It then added a third within-group sort, new Sort(SortField.FIELD_SCORE), to the unit's bounded-exhaustive harness, which compares sequential, concurrent and hand-sharded grouping with a reference over every small index. Wrong pages (distributed and sequential-versus-concurrent kinds) and wrong maxScore appeared already at 2 documents, one per slice or shard. With the constant and with real field sorts the same harness found no such mismatch in larger runs.
• So the hypothesis came from reading; the harness confirmed it. A public-API JUnit test on the stock jars followed, then the independent checks below.