UnifiedHighlighter.Builder#withFlags is silently ignored: the highlighter always takes its flags from the four boolean builder options

UnifiedHighlighter.Builder has two ways to choose the highlight flags: four boolean options (withHandleMultiTermQuery, withHighlightPhrasesStrictly, withPassageRelevancyOverSpeed, withWeightMatches), or withFlags(Set<HighlightFlag>), whose javadoc says the set "will override" the four. The second way does nothing. withFlags stores the set in Builder.flags, but the constructor's evaluateFlags(Builder uhBuilder) checks flags, which there means the highlighter's own field, still null while the constructor runs, and then derives the flags from the four booleans; Builder.flags is never read. With withFlags(EnumSet.of(MULTI_TERM_QUERY)), PhraseQuery("body", "alpha", "beta") on "alpha beta alpha gamma" gives the default strict markup <b>alpha beta</b> alpha gamma instead of <b>alpha</b> <b>beta</b> <b>alpha</b> gamma. Nothing throws, and hits, scores and the index are unaffected; only the highlighting configuration is wrong. It shipped with the Builder in 9.1.0 (#412), affects every release up to 10.6.0 and main, and the four boolean options are a working workaround. The fix makes evaluateFlags(Builder) use the builder's set.

private Set<HighlightFlag> flags;                                   // :131  the highlighter's own field
  ...
public static class Builder {
  private Set<HighlightFlag> flags;                                 // :270  Builder.flags
  public Builder withFlags(Set<HighlightFlag> values) {             // :309
    this.flags = values;                                            // :310
    return this;
  }
}
public UnifiedHighlighter(Builder builder) {
  ...
  this.flags = evaluateFlags(builder);                              // :468
protected Set<HighlightFlag> evaluateFlags(Builder uhBuilder) {     // :539
  if (flags != null) {                                              // :540  this.flags, still null here
    return flags;
  }
  return flags = evaluateFlags(uhBuilder.handleMultiTermQuery, uhBuilder.highlightPhrasesStrictly,
                               uhBuilder.passageRelevancyOverSpeed, uhBuilder.weightMatches);   // :543-548
}

1. The set is stored. withFlags writes Builder.flags (:309-312). Only Builder#getFlags() (:430-432) ever reads it.
2. The check reads the wrong object. Inside evaluateFlags(Builder uhBuilder) the unqualified flags is the highlighter's private field (:131), not uhBuilder.flags. The field has no initializer and is written only at :468 and, inside this method, at :543, so when the constructor calls the method the check is always false.
3. The booleans always win. evaluateFlags(boolean, boolean, boolean, boolean) (:502-530) builds a fresh EnumSet from the four options and adds WEIGHT_MATCHES only when the other three are set (:519-528). The defaults (:108-111, all true) give all four flags.
4. The flags drive the highlighting. getFlags(String) returns the stored set (:586-594). WEIGHT_MATCHES selects the Weight.matches path, which marks the whole phrase (:1221-1224); PHRASES without it builds a strict PhraseHelper (:1225-1235); MULTI_TERM_QUERY decides whether multi-term queries are highlighted at all (:1248-1250); two flags choose the offset strategy for re-analysis (:1294-1299).

PhraseQuery("body", "alpha", "beta") on "alpha beta alpha gamma", highlightWithoutSearcher, StandardAnalyzer: withFlags({MULTI_TERM_QUERY}) gives <b>alpha beta</b> alpha gamma, the same as a default builder (all four flags). The same {MULTI_TERM_QUERY} chosen through withHighlightPhrasesStrictly(false) and withPassageRelevancyOverSpeed(false) gives <b>alpha</b> <b>beta</b> <b>alpha</b> gamma. withFlags with the empty set still highlights a PrefixQuery (<b>Lucene</b>), while all four booleans false does not.

• What goes wrong. The highlighter runs with the flags of the four booleans (by default all four) instead of the requested set: strict instead of loose phrases, whole-phrase instead of per-word markup, multi-term queries highlighted although left out, and possibly a different passage ranking and offset strategy. Nothing throws.
• What it does not touch. Search hits, scores and the index; which documents and fields are highlighted; withFieldMatcher, masked fields and withMaxLength, which the constructor copies separately (:469-472). On stock main, a sweep of 6,528 highlights over every withFlags argument, boolean combination, three offset sources and eight query types saw only flag sets the booleans can produce, and no text from another field.
• A cost you cannot switch off this way. "MTQ highlighting can be expensive, particularly when using offsets in postings" (withHandleMultiTermQuery javadoc, :325-327). A caller who leaves MULTI_TERM_QUERY out of its withFlags set still pays for it, as with the default configuration.
• Who is exposed. Applications that call Builder#withFlags (opt-in; no production code in Lucene does). Read-only code search found no caller outside copies of Lucene's own files: Solr overrides getFlags(String), and Elasticsearch and OpenSearch subclass the highlighter and edit or override the getFlags result. Code search does not cover private code.
• Scale. None needed: every stock UnifiedHighlighter built with withFlags is affected, deterministically, on the first call. A subclass that overrides evaluateFlags(Builder) sets its own flags, as Lucene's test helper overriddenBuilderForTests does (TestUnifiedHighlighter.java:139-157).
• Workaround. The four boolean options can express 9 of the 16 flag sets, every set that meets the documented WEIGHT_MATCHES requirement (:344-346); a getFlags(String) override can express any set.
• Lucene's own tests. TestUnifiedHighlighter#overrideFieldMatcherForTests copies a highlighter with withFlags(original.getFlags(fieldName)) (TestUnifiedHighlighter.java:133), so the three testFieldMatcher* tests run their copies with the default flags instead of the randomized ones. No test checks what withFlags does.

The class sits in org.apache.lucene.search.uhighlight, next to TestUnifiedHighlighter, and uses only builderWithoutSearcher and highlightWithoutSearcher with a StandardAnalyzer: no Directory, no IndexWriter, and no random input in the test body (LuceneTestCase still seeds its usual randomization). The expected string is what the same flag set gives when chosen through the boolean options on stock main (a separate control run), and what the fixed highlighter gives. It compiles on 9.1.0 and later.

Regression test: this same test, as written. With the recommended fix it passes, together with the highlighter module's format, ECJ and forbidden-apis checks.

cp $REPRO/TestUnifiedHighlighterBuilderWithFlags.java $LUCENE/lucene/highlighter/src/test/org/apache/lucene/search/uhighlight/
cd $LUCENE && ./gradlew :lucene:highlighter:test --tests org.apache.lucene.search.uhighlight.TestUnifiedHighlighterBuilderWithFlags -Ptests.seed=DEADBEEF
# assertions off fails the same way: add -Ptests.asserts=false (the recorded runs added --offline --no-daemon)

# gradle, scratch worktree of 645d4fe14a20, seed DEADBEEF, assertions on
TestUnifiedHighlighterBuilderWithFlags > testWithFlagsIsHonoured FAILED
    org.junit.ComparisonFailure: expected:<<b>alpha[</b> <b>beta</b> <b>alpha</b>] gamma> but was:<<b>alpha[ beta</b> alpha] gamma>
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:BCCD93156E411B7E]:0)
        at app/junit@4.13.2/org.junit.Assert.assertEquals(Assert.java:117)
        at app/junit@4.13.2/org.junit.Assert.assertEquals(Assert.java:146)
        at app//org.apache.lucene.search.uhighlight.TestUnifiedHighlighterBuilderWithFlags.testWithFlagsIsHonoured(TestUnifiedHighlighterBuilderWithFlags.java:34)

# standalone runner, seed DEADBEEF: the same message on main (assertions on and off) and, with assertions on, on 10.6.0, 10.5.2, 10.0.0, 9.12.3 and 9.1.0

# control on stock main (not the repro): the same flags through the boolean options
withHighlightPhrasesStrictly(false).withPassageRelevancyOverSpeed(false): getFlags = [MULTI_TERM_QUERY], '<b>alpha</b> <b>beta</b> <b>alpha</b> gamma'
default builder:                                                         getFlags = [PHRASES, MULTI_TERM_QUERY, PASSAGE_RELEVANCY_OVER_SPEED, WEIGHT_MATCHES], '<b>alpha beta</b> alpha gamma'

• The contract. "User-defined set of HighlightFlag values which will override the flags set by withHandleMultiTermQuery(boolean), withHighlightPhrasesStrictly(boolean), withPassageRelevancyOverSpeed(boolean) and withWeightMatches(boolean). Here the user can either specify the set of HighlightFlags to be applied or use the boolean flags to populate final list of HighlightFlags." (:299-308). No javadoc, CHANGES or MIGRATE text qualifies it.
• The code shows the intent. The guard if (flags != null) return flags; (:540) is always false when the constructor calls the method; it is consistent with an intended check of the builder's set. Builder#getFlags() exposes the stored set (:430-432).
• History. In #412, at revision d65ee8ab42 (2021-12-05), evaluateFlags() was a Builder method, so its check read Builder.flags: by its source, withFlags would have taken effect (revisions read, not built). Revision a6f70412c9 (2021-12-18, "Attempt at making UH with builder back-portable to v9.x") moved it into UnifiedHighlighter as evaluateFlags(Builder uhBuilder) with the check unchanged, so flags now meant the highlighter's own field. No version ever read uhBuilder.flags there (git log -G).
• The review. The PR author summarised a revision on 2021-10-30 as "if flags is initialized then boolean flags are ignored". On 2021-12-03 a reviewer (dsmiley) asked "Isn't empty valid?" about a check that ignored an empty set, so even an empty set was meant to count.
• Lucene's own test helper relies on it (TestUnifiedHighlighter.java:133).

The strongest case for "intended" (the cross-model reviews, the intended-behaviour check and the adjudication built it): the class is @lucene.experimental (:94); on #412 a reviewer wrote "I'm dubious we even want this on the builder since it's more of an advanced case to customize these flags. Someone can always subclass the getter to customize" (2021-10-30, about a revision that had added a setFlags setter); and the back-port plan on #11234 said "the flags computation needs to be as it is already on 9x instead of moved off to the builder". So the booleans would be the real API and withFlags vestigial.

Why it fails: withFlags and its "will override" javadoc were kept and shipped in every release since 9.1.0, and an experimental tag lets an API change, not a public method do nothing; the back-port note is about where the flags are computed, and, by its source, the revision before the move read the set; the same reviewer later asked "Isn't empty valid?"; no won't-fix or by-design statement was found in the recorded searches. The cross-model reviews (OpenAI GPT-5.6) rated the case weak. Removing withFlags is a legitimate design choice for maintainers who want one way to set flags, but it is still a change, because the method has never worked as its javadoc says.

Two fixes were validated through gradle in a scratch worktree of 645d4fe14a20; the recommended one also keeps the highlighter's set private. Two further options were not built.

When the builder has a set, use a fresh EnumSet copy of it; otherwise derive the flags from the four booleans as today.

protected Set<HighlightFlag> evaluateFlags(Builder uhBuilder) {
  if (flags != null) {
    return flags;
  }
  if (uhBuilder.flags != null) {
    // Builder#withFlags overrides the boolean options; keep a private, mutable copy
    Set<HighlightFlag> copy = EnumSet.noneOf(HighlightFlag.class);
    copy.addAll(uhBuilder.flags);
    return flags = copy;
  }
  return flags = evaluateFlags(uhBuilder.handleMultiTermQuery, uhBuilder.highlightPhrasesStrictly,
                               uhBuilder.passageRelevancyOverSpeed, uhBuilder.weightMatches);
}

Trade-offs

• Why the copy. Today the highlighter's set is always a fresh EnumSet it owns, and downstream subclasses edit it (Elasticsearch calls getFlags(field).remove(HighlightFlag.WEIGHT_MATCHES) in its constructor, by code reading). The copy keeps that true for withFlags callers, and it stops later changes to the caller's set from reaching the highlighter (both executed in the fix-design probe).
• EnumSet.noneOf plus addAll also accepts an empty set, which EnumSet.copyOf(Collection) rejects for a non-EnumSet argument.
• Cost: one small set copy per highlighter construction; nothing per highlight.
• Open point: the fix takes any explicit set as given, including WEIGHT_MATCHES without its documented prerequisites (:344-346); whether such sets should be taken as given, normalized or rejected is a question for the maintainers (option C).

What the checks validated

• Gradle, scratch worktree of 645d4fe14a20, OpenJDK 25.0.3, seed DEADBEEF (2026-10-31). Stock: the repro fails. With the fix: the repro and TestUnifiedHighlighter (156 tests) pass with assertions on and off.
• :lucene:highlighter:test, the whole module with the repro in place: 35 classes, 723 tests, 0 failures, 0 skipped.
• The three testFieldMatcher* tests, whose copied highlighters now keep their randomized flags, with -Ptests.iters=50: 600 tests, 0 failures.
• checkGoogleJavaFormat, ecjLintMain, ecjLintTest (with the repro in place), forbiddenApisMain, forbiddenApisTest: pass.
• A fix-design probe (not the repro) passes 2 of 2: an immutable caller set with a subclass that edits its flags, and a caller that changes its own set after build().

Add if (uhBuilder.flags != null) { return flags = uhBuilder.flags; } after the existing check (+3 lines; the finder's and the independent reproduction's fix).

Trade-offs

The highlighter shares the caller's set. Executed with a fix-design probe: after withFlags(Set.of(MULTI_TERM_QUERY)), a subclass that calls getFlags(f).remove(WEIGHT_MATCHES) in its constructor throws UnsupportedOperationException, and adding PHRASES to the caller's mutable set after build() changes the highlighter's flags to [PHRASES, MULTI_TERM_QUERY].

What the checks validated

The repro and TestUnifiedHighlighter (156 tests) pass through gradle on 645d4fe14a20; earlier, every hunt repro passed with it, and a run of 476 highlighter tests changed only that repro's result.

Drop WEIGHT_MATCHES, or throw IllegalArgumentException, when the set lacks one of the other three flags, as the boolean path does (:519-528).

Trade-offs

Not needed for this bug, and inconsistent with the other way to set flags: a getFlags(String) override can return any set, and Lucene's own randomized tests already run sets such as {WEIGHT_MATCHES, MULTI_TERM_QUERY, PHRASES} without PASSAGE_RELEVANCY_OVER_SPEED (TestUnifiedHighlighter.java:82-95).

Document a getFlags(String) override as the way to pass an arbitrary set, rewrite the test helper to copy the booleans, and remove withFlags, for example with the 11.0 deprecation clean-up tracked in #16918.

Trade-offs

A legitimate design choice if maintainers want one way to set flags. Removes a documented option; until removal the method still does nothing.

Method: non-formal (code reading, then JUnit tests).

• A hunt unit on highlighter offset sources was writing a bounded-exhaustive model of getOptimizedOffsetSource and getOffsetStrategy, where the flag set is an input. Reading UnifiedHighlighter, the finder saw that evaluateFlags(Builder) tests the highlighter's own field. A JUnit on the stock main jars confirmed it (fails with assertions on and off, and on 10.5.2 and 9.12.3). No model or tool output produced the first signal; the model then sets its flag sets through the boolean options because withFlags has no effect.
• An independent reproducer, a devil's-advocate verifier (who traced the PR history), cross-model reviews (OpenAI GPT-5.6) and an adjudication checked it; a follow-up ran it from 9.0.0 to main, searched for prior reports and screened it for security; the repro was then minimized to one public-API test.