SimpleSpanFragmenter cuts a phrase match when phrases overlap, share a start, or sit on stacked synonyms

The classic Highlighter cuts a text into fragments, and SimpleSpanFragmenter promises to cut about every N characters without splitting a phrase match. It keeps that promise for one phrase at a time: it remembers one position to wait for, sets it only at a span's start from the first span listed there, skips the lookup while it is waiting, and sees only the first token of each token group. So two phrases that share a start ("new york" OR "new york city hall"), a phrase that starts inside another ("new york" OR "york city hall") and a phrase whose first words carry stacked synonyms get cut in two, and getBestFragment returns part of the phrase. In the same-start case the outcome depends on clause order. Hits, scores and marked words are correct; only snippet boundaries are wrong. Present since the class was added (2.4.0, 2008); a 13-line bookkeeping fix passes the whole highlighter test suite.

• Fragmenter: decides, token by token, whether a new fragment starts before the current token (Fragmenter.isNewFragment, "Test to see if this token from the stream should be held in a new TextFragment", Fragmenter.java:37-41). The Highlighter's default is SimpleFragmenter (Highlighter.java:44); SimpleSpanFragmenter is opt-in: "breaks text up into same-size fragments but does not split up {@link Spans}. This is a simple sample class." (SimpleSpanFragmenter.java:26-29).
• Token group: tokens whose offsets overlap the group so far (TokenGroup.java:73-74). When a token starts a new group, the Highlighter encodes and appends the previous group and then calls isNewFragment() with the attributes of the new group's first token (Highlighter.java:209-224); true starts a new fragment before that token (:224-230).
• Spans per term: QueryScorer extracts a BooleanQuery's clauses in order (WeightedSpanTermExtractor.java:133-138), and each phrase's span positions are appended to every one of its terms (:314-353; WeightedSpanTerm.java:62-64). For "new york" OR "new york city hall" the term york carries [3,4][3,6], in that order.

isNewFragment (SimpleSpanFragmenter.java:62-93) keeps one value, waitForPos, the position up to which it refuses to cut. While it is set, the method returns false before any span lookup (:65-69). Otherwise it looks up the current term's spans and takes the first one with start == position: waitForPos = positionSpan.end + 1; break; (:76-80). Then the size test decides (:84-86).

1. Same start, first listed wins. york carries [3,4] before [3,6], so the loop takes [3,4] and breaks: waitForPos = 5. Because the counter lags one token (LFV-859, below), that wait still covers City and ends at Hall, where the size test cuts: We flew to New York City | Hall last week | for a conference (size 20). With exact positions the cut would come one word earlier, before City (executed with the counter fix alone); the phrase is split either way. With the longer clause listed first, [3,6] is taken and the phrase stays whole.
2. Overlap, no lookup while waiting. For "new york" OR "york city hall" on The museum is near New York City Hall and the park the spans are new=[4,5], york=[4,5][5,7], city=[5,7], hall=[5,7]. The call at which [5,7] would be registered comes while the wait from [4,5] is pending, so it returns before the lookup; [5,7] is never registered and the cut lands before Hall (size 17).
3. Stacked synonyms, first token of a group only. SynonymGraphFilter with the originals kept emits the synonym first and the original at increment 0 with the same offsets (large/inc1/10-13, big/inc0/10-13, notebook/inc1/14-20, laptop/inc0/14-20 for this text). The fragmenter is shown large and notebook, never big and laptop (an independent trace: left(+1) my(+1) large(+1) notebook(+1) bag(+1)|). Neither carries a span; at bag the span [3,5] does not start at the current position, so it is never registered and the cut lands before bag (size 21).

On stock code the position counter (:63) lags one token behind, because the Highlighter never shows the fragmenter the text's first token (LFV-859). The lookups above therefore happen one token after the span start (for case 1 at York), and that lag alone can cut a phrase right after its first word. Every failing case here cuts after the second or a later word, each has a passing control that changes one input (the clause order, the outer phrase or the synonyms), and a fix that keeps the stock counter removes them all. On stock, case 3 needs the phrase's first two words stacked; with only the first word stacked the lag hides it.

• Who is hit. The classic Highlighter with QueryScorer and SimpleSpanFragmenter. It is opt-in (the default fragmenter is SimpleFragmenter, Highlighter.java:44), but it is the classic phrase-aware fragmenter, and Elasticsearch's plain highlighter uses it as its default span fragmenter (code read; not tested).
• Which queries. Two phrases in one query that share a start or overlap, for example phrase-variant expansion ("new york" OR "new york city hall"), a phrase plus a longer one typed by the user, or a query builder that adds sub-phrases; or a phrase highlighted with an analyzer that stacks synonyms on its first two words (SynonymGraphFilter with the originals kept; appending FlattenGraphFilter keeps the same token order and split pattern).
• When it shows. Only when a size threshold falls inside the phrase, after its protected part. At the default size 100, the overlap case split in 8 of 51 text paddings of a longer sentence on stock, in 12 with the counter fix alone and in 0 with the hunt's cumulative fix (counter plus bookkeeping; fix A alone was not run on that probe). In the hunt's model of the fragment loop (it matched the real classes on every bounded case), with the counter of LFV-859 fixed, 121,152 of 7,873,440 short spans in the 2-3-span scope still split (100,800 of them overlap or share a start with another span); the bookkeeping fix leaves 0.
• Effect. Silent and deterministic: the best snippet shows part of a phrase, the rest goes to a neighbouring fragment. getBestFragments(..., n >= 2) merges contiguous fragments (Highlighter.java:140, :299), so the split is hidden when both parts are selected. Not affected: hits, scores, which words are marked, the default SimpleFragmenter, the UnifiedHighlighter.
• Why nobody noticed. The recorded searches found no report of overlapping, same-start or stacked-synonym splits; the three old single-phrase reports (2010, 2012, 2015) belong to LFV-859.

# Lucene main 645d4fe14a20, assertions on (-ea), seed 5EED0858: 1 test(s) run, 1 failed
    => java.lang.AssertionError: expected a fragment holding <B>New</B> <B>York</B> <B>City</B> <B>Hall</B>, got [We flew to <B>New</B> <B>York</B> <B>City</B>]
       org.junit.Assert.fail(Assert.java:89)
       org.junit.Assert.assertTrue(Assert.java:42)
       org.apache.lucene.search.highlight.TestSimpleSpanFragmenterSameStartPhrases.testSameStartPhrasesNotSplit(TestSimpleSpanFragmenterSameStartPhrases.java:41)

• The contract is unqualified. "breaks text up into same-size fragments but does not split up {@link Spans}" (SimpleSpanFragmenter.java:26-28). When the class was contributed (LUCENE-794, #1869), its author described it as one that "fragments based on size, but ensures that Spans are not broken up".
• Clause order decides. "new york" OR "new york city hall" cuts the long phrase, "new york city hall" OR "new york" keeps it whole (executed at size 20 and at the default 100).
• Fixed as a bug before. LUCENE-1389 (#2463, 2008) and LUCENE-2229 (#3305, 2015; CHANGES.txt:9891-9893 under 5.4.1: "Fix Highlighter's SimpleSpanFragmenter when multiple adjacent stop words following a span can unduly make the fragment way too long.").
• No test asserts the current behaviour. TestHighlighter.testSimpleSpanFragmenter (:739-785) only prints when VERBOSE; the LUCENE-2229 test (:414-437) checks only that the result is under 60 characters. No test uses overlapping, same-start or stacked spans with this fragmenter.
• Ordinary inputs. StandardTokenizer, a BooleanQuery of PhraseQuerys, SynonymGraphFilter and the public highlighter API; the class is public and not deprecated, and MIGRATE.md says nothing about it.

The strongest case for "intended": the javadoc ends "This is a simple sample class" and the contributor wrote "This class might not be perfect yet" (#1869); the one-span-per-start rule looks deliberate, since the 2008 original also used only the first matching span (it returned true there, which also started a new fragment at the span's start), and Andrew Duffy, who supplied the LUCENE-1389 patch, called the break "a slight optimisation - the span loop is broken early when a span is found at the current position" (#2463); Fragmenter.isNewFragment does not promise a call per token (Fragmenter.java:37-41), so the stacked-synonym case can be called a Highlighter limitation; the effect is cosmetic; and @dsmiley wrote on PR #1069 (2022): "If only we renamed "Highlighter" to "OriginalHighlighter", maybe folks wouldn't continue to using this thing."

Why it fails: the guarantee is stated without exception, and "sample" has not meant "won't fix": this method's bookkeeping was fixed twice, once with a CHANGES entry. The "optimisation" has the effect of treating the first matching span at a position as enough, which fails as soon as two spans share a start, and the outcome then depends on clause order. The same-start and overlap cases need no Highlighter change. The remark on #1069 is about priority, not the contract: expect "low priority" or "use the UnifiedHighlighter", not "by design". Verdict: not intended (an independent reproducer, a devil's-advocate verifier, a cross-model review that read the code, an adjudication and a credibility audit).

Two fixes were built in a scratch worktree at 645d4fe14a20 and validated through gradle; both diffs apply with git apply there. A fixes this bug alone; B adds the position-counter fix of the sibling LFV-859 and suits an issue that covers both. C and D are for the discussion with maintainers.

On every call, for each span of the current term with start <= position <= end, set waitForPos = max(waitForPos, end + 1), with no early return while waiting and no break; return false while inside a span (strictly after its start), else run the unchanged size test. Written independently by the finder (inside its cumulative fix) and by the independent reproducer.

// isNewFragment(), after the position update (condensed; fix-a-span-bookkeeping.diff is formatted)
if (waitForPos <= position) {
  waitForPos = -1;
}
boolean insideSpan = waitForPos != -1; // no early return while waiting
WeightedSpanTerm wSpanTerm = queryScorer.getWeightedSpanTerm(termAtt.toString());
if (wSpanTerm != null) {
  for (PositionSpan positionSpan : wSpanTerm.getPositionSpans()) { // no break
    if (positionSpan.start <= position && position <= positionSpan.end) {
      waitForPos = Math.max(waitForPos, positionSpan.end + 1);
      if (positionSpan.start < position) insideSpan = true;
    }
  }
}
if (insideSpan) return false; // then the unchanged size test

Trade-offs

• Leaves LFV-859: with the stock counter, cuts right after a phrase's first word remain (the reproducer's strict test: 46 cuts, all right after word 1; on stock the same 46 plus 5 after word 2; the finder's test 2 of 3).
• Calls made inside a span now also scan the term's span list (the stock code skipped that scan while waiting; elsewhere both scan it). No benchmark was run.
• Residual, the same as B's (code reading; not measured for A alone): it sees spans only through the tokens the fragmenter is shown, so a span none of whose shown tokens is a query term stays unprotected; that needs a Highlighter change.

What the checks validated

• Scratch worktree at 645d4fe14a20, gradle: the paste-ready test fails 4 of 8 on stock (assertions on and off; 20 of 40 with 5 random seeds per method); with the patch the whole :lucene:highlighter:test task passes, 35 classes, 730 tests (722 existing plus the 8 new), 0 failures, assertions on and off.
• ecjLintMain, ecjLintTest and checkGoogleJavaFormat for the module pass.
• Shadowed on main through the standalone runner: the reproducer's cut-after-second-word sweep 7 of 7.

Fix A, plus position = queryScorer.getLastTokenPosition() + posIncAtt.getPositionIncrement(); with a new package-private accessor on QueryScorer that returns the position of the last scored token (the scorer sees every token; the fragmenter does not). +22/-6 lines in two files.

Trade-offs

• Couples the fragmenter to the scorer's token position (same package, not public API); it changes LFV-859's behaviour too, so it belongs in an issue that covers both.
• getLastTokenPosition() is right only if the fragmenter is given the same QueryScorer the Highlighter scores with: the constructor javadoc asks for the "QueryScorer that was used to score hits" (SimpleSpanFragmenter.java:43, :50), but nothing enforces it, and no test covers a fragmenter given another scorer.
• Residual (finder): a span none of whose shown tokens is a query term (for example a stacked group whose first token is a synonym, with nothing later in the span shown) is still not protected; that needs a Highlighter change (1,152 of 20,382,180 short spans in the model's single-span scope).

What the checks validated

• Gradle, same worktree: the paste-ready test 8 of 8; the whole highlighter test task 730 tests (722 existing plus the 8 new), 0 failures, assertions on and off; lint and format checks pass.
• Shadowed: the reproducer's two classes 13 of 13 (including its strict test), the finder's 3 of 3, LFV-859's 3 of 3. The hunt's end-to-end sweep of the same logic (1,874,880 runs with real analysis and span extraction) found 0 splits except for a phrase with a hole, whose spans are already wrong (LFV-827).

Compute in start() the positions strictly inside any span and refuse a cut there, or have the Highlighter consult the fragmenter for every token.

Trade-offs

Removes the per-call bookkeeping and the stacked-token residual, but needs a new way to enumerate the scorer's spans: QueryScorer exposes only a per-term lookup (QueryScorer.java:190-192). Untested.

Say in the javadoc that overlapping, same-start or stacked spans may be split.

Trade-offs

Cheapest, but leaves the clause-order dependence in place.

• Signal. A planning probe swept 20,000 random texts with SHOULD clauses of 1-3 phrases and attributed the splits it found to causes: 455 to an overlapping occurrence and 46 to a shared start (415 and 15 still split with the position counter shadowed).
• Model. The hunt unit built a transducer model of Highlighter.getBestTextFragments composed with SimpleSpanFragmenter and checked it against the real classes on every case of two bounded-exhaustive scopes: 56,100,240 single-span cases and 10,060,800 cases with two or three spans, overlapping and same-start ones included; 0 mismatches for stock and every fix variant.
• Attribution by staged fixes. With the sibling's position fix, 121,152 of 7,873,440 short spans in the multi-span scope still split (100,800 of them spans that overlap or share a start with another span) and 20,288 in the single-span scope (spans that start on a token the fragmenter is not shown); adding the span-bookkeeping fix leaves 0 in the multi-span scope. The stacked-synonym repro came from the unit's realistic-analyzer scenarios (SynonymGraphFilter with the originals kept).
• Witness. Public-API JUnits on the stock jars; an independent reproducer then wrote a test that the sibling's lag cannot trigger.