ConditionalTokenFilter loses a trailing stop-word hole when the last token went through the wrapped filters

A StopFilter that removes a value's last word records the gap in end(): abc the gives the token abc and a final position increment of 1, so the next value of the same field starts one position later. Put a ConditionalTokenFilter after it (ProtectedTermFilter, or a CustomAnalyzer when()/whenTerm() branch) and that 1 becomes 0 whenever the last token went through the wrapped filters, even when they never touch positions (LowerCaseFilter, or in -> in). The cause: end() keeps the wrapped chain's end state, and that chain ends on the internal OneTimeWrapper, whose end() imitates a tokenizer, resets the increment to 0 and restores only the final offset. In the middle of the stream the same class adds the wrapped filters' end increment instead. With the default gap of 0, the multi-valued field ['abc the', 'def'] indexes def at position 1 instead of 2 and the phrase "abc def" matches (1 hit instead of 0); TokenStreamToAutomaton and a downstream ShingleFilter lose the trailing hole too. It fails on all ten versions run, from 7.4.0 (2018, the class's first release) to 10.5.2 and main, and the file's history shows no release in between with a different end-of-stream path. Handing the input's final increment to the wrapped filters (a few lines in the class) fixes it; the recommended fix also covers a related end-of-stream case (a removed stacked last token adds a position), and the analysis module's tests and TestRandomChains pass with it.

• Trailing hole: StopFilter on abc the emits abc and, in end(), a final position increment of 1 for the removed the (FilteringTokenFilter.java:72-75; TokenStream.java:152-154).
• Conditional filter: ConditionalTokenFilter sends each token through some wrapped filters only if shouldFilter() is true. ProtectedTermFilter does that for every token that is not in its protected set (ProtectedTermFilter.java:51-54); CustomAnalyzer's when() and whenTerm() build one.
• Who reads the final increment: IndexingChain adds it to the field position before the next value of the same field (IndexingChain.java:2105-2109, then the gap at :2122; the default gap is 0, Analyzer.java:296-311). TokenStreamToAutomaton turns it into trailing HOLE arcs (TokenStreamToAutomaton.java:212-239), ShingleFilter into trailing filler shingles (:379-382).

1. The wrapped filters are built on an internal stream, OneTimeWrapper, over the same attributes as the input (ConditionalTokenFilter.java:119).
2. OneTimeWrapper.end() "imitate[s] Tokenizer.end()": it calls endAttributes(), which sets the position increment to 0 (PackedTokenAttributeImpl.java:183-186, the default implementation), and puts back only the final offset (:84-95).
3. ConditionalTokenFilter.end() first captures the input's end state (increment 1), then, if the last token was filtered, calls delegate.end() and keeps that state (:139-151). The 1 is overwritten with 0, not added.
4. In the middle of the stream, endDelegating() adds the wrapped filters' end increment to the next token's own increment (:213-230, the addition at :223). So holes in the middle survive, and so does a trailing hole the wrapped filters make themselves; the trailing hole of the input is lost, and only when the last token was filtered.
5. Related, the other way round. A stacked token (increment 0) that goes to the wrapped filters is handed to them with an increment of 1 (:56-60); mid-stream the class takes that 1 back (:188-192, :224-227), at the end of the stream it does not. So when the wrapped filters remove a stacked last token, a StopFilter among them reports one position too many: tokens x/1, a/0, a condition only for a around StopFilter({"a"}) give a final increment of 1 instead of 0; also with public API only (SynonymGraphFilter x -> a keeping the original, then ProtectedTermFilter({a}, StopFilter({"x"}))). Found by the cross-model fact check of this doc and executed on main; the recommended fix covers it.

"Plain chain" is the same chain without the condition. The hunt's verifiers also executed TokenStreamToAutomaton on abc the (abc instead of abc + POS_SEP + HOLE) and 200,000 trailing stop words (0 instead of 200,000).

Only the position increment and the final offset were examined: OneTimeWrapper.end() resets every attribute and puts back only the offset, so a custom attribute with its own end-of-stream state would not reach the wrapped filters either (not tested; no fix here changes that).

• Who is exposed. Applications that put a ConditionalTokenFilter behind a filter that leaves a trailing hole (typically StopFilter): ProtectedTermFilter / ProtectedTermFilterFactory (SPI name protectedTerm), CustomAnalyzer when() / whenTerm() branches, and ConditionalTokenFilterFactory subclasses. No stock Lucene analyzer uses the class, so it is opt-in.
• What it takes. A value that ends in a removed token right after a token that went through the wrapped filters; for ProtectedTermFilter, any value ending in a stop word whose previous word is not protected.
• What changes. The next value of a multi-valued field starts one position early per lost hole. With the default gap 0, a cross-value phrase query over-matches ("abc def": 1 hit instead of 0) or misses (the phrase built for abc the def, f:"abc ? def", no longer matches the values abc the | def); both directions executed with PhraseQuery. Span and interval queries read the same positions, so they can do the same (by code reading; not run).
• Token automata and shingles. TokenStreamToAutomaton with its default settings and a ShingleFilter inside or after the condition lose the trailing hole, also on a single value (so a shingle field's terms change). What a suggester sees depends on its settings: AnalyzingSuggester turns on finalOffsetGapAsHole (AnalyzingSuggester.java:317-321), which can hide a single lost hole such as abc the's. No suggester was run end to end.
• What does not change. For a chain where nothing inside or after the condition reads the final increment (LowerCaseFilter, in -> in): terms, offsets and positions inside a value (executed), and, by code reading, a single-valued field's index content, since the final increment only moves where a next value starts. Also: the queries QueryBuilder and Intervals.analyzedText build, MemoryIndex, and CheckIndex (clean). With a gap of 100, as common search servers configure, a cross-value phrase result changes only at slop 100 for one lost hole and 100-101 for two (executed over slops 0-250).
• Not assessed end to end. Other readers of the final increment, such as FreeTextSuggester.lookup() and highlighters that re-analyse multi-valued fields.
• Scale and versions. Deterministic, any index size, no error. Fails on every version run (ten, from 7.4.0 to 10.5.2, and main); by the file's history every release in between is affected too.
• How likely. Rare: the wrong number is common, but it shows only with a small gap and a cross-value positional query, or a shingle or suggester consumer; nobody has reported it since 2018.

# Lucene main 645d4fe14a20, assertions on (-ea), seed DEADBEEF: 1 test(s) run, 1 failed
    => java.lang.AssertionError: finalPosInc 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.tests.analysis.BaseTokenStreamTestCase.assertTokenStreamContents(BaseTokenStreamTestCase.java:436)
       org.apache.lucene.tests.analysis.BaseTokenStreamTestCase.assertTokenStreamContents(BaseTokenStreamTestCase.java:544)
       org.apache.lucene.analysis.miscellaneous.TestConditionalTokenFilterEndPositionIncrement.testTrailingHoleKeptThroughIdentityDelegate(TestConditionalTokenFilterEndPositionIncrement.java:43)

• The contract. TokenStream#end: "any skipped positions (such as those removed by a stopfilter) can be applied to the position increment" (TokenStream.java:152-154). The core analysis package docs, "End of Input Cleanup": components "must put accurate values into the token attributes to reflect the situation at the end of the field" (package-info.java:276-281). LUCENE-3849 made end() carry the increment "so we can handle trailing holes" (lucene/CHANGES.txt:13337-13340).
• The class's own promise. "If it returns false, then the wrapped filter(s) will be skipped" (ConditionalTokenFilter.java:31-33); CustomAnalyzer.when(): the conditional filters "will only be used if the current token matches the condition" (CustomAnalyzer.java:463-465). So a condition around in -> in should change nothing, and ProtectedTermFilter with nothing protected should equal its wrapped filter. Only the final increment differs (1 vs 0).
• It flips with the last token. The same chain gives 1 when the last token is protected and 0 when it is not, although LowerCaseFilter never touches positions.
• The class adds mid-stream (endDelegating(), :223); the end of the stream has no such addition.
• Stated intent, kept mid-stream only. The LUCENE-8360 report (#9407, 2018) says the bug was found through this class, "which calls end() on its delegate when it reaches a token that shouldn't have the delegate applied to it, so that things like position increments and position length are preserved". That is the mid-stream path (endDelegating()), which keeps the promise by adding; the end of the stream, where this bug is, does not. LUCENE-8376/8371 (7.5.0 Bug Fixes) already treated wrong end() propagation in this class as a bug, and fixed FixedShingleFilter, which "did not set position increment in end()" (lucene/CHANGES.txt:7198-7200).
• Nothing pins the current value. No upstream test asserts a final increment on any conditional path; testEndPropagation asserts final offsets only (18 and 20, TestConditionalTokenFilter.java:174-197). The generic check that would have caught it (LUCENE-8361, #9408) was never committed. With fix B the 44 tests of the four closest suites pass.

The checkers built the strongest case for "intended": (a) TokenStream#end says skipped positions "can" be applied, not "must"; (b) OneTimeWrapper imitates Tokenizer.end() on purpose, and the package docs say "The base method TokenStream#end() sets PositionIncrement to 0, which is required" (package-info.java:283-284); (c) the wrapped chain's end state wins on purpose when the last token was filtered, and testEndPropagation pins that (final offset 18 instead of 20); (d) nobody has reported it since 7.4.0, although TestRandomChains wraps random filters in conditions.

Why it fails: (a) the same package docs require accurate values at the end of the field, and the class javadoc promises that the wrapped filters are skipped or applied, not that the stream's end changes. (b) The "0, which is required" sentence is the base method's starting value; each filter then adds its own, as FilteringTokenFilter.end() does. A tokenizer has nothing upstream; OneTimeWrapper does, and it passes the input's final offset on but not its increment. (c) In testEndPropagation the wrapped filter saw the input's real final offset and changed it; it never sees the input's increment, so "the wrapped chain wins" becomes "the input's hole is erased". (d) Nothing checks the final increment on these paths. The adjudication names (a) as the main risk. Every checker rejected the works-as-designed case.

Fixes A, B and B+ were applied in scratch worktrees of main a9779ef3c193 and run through gradle with assertions on and off. A probe ran 21 chains, 8 plain and 13 conditional (two of them controls in which the wrapped filters never see the last token: x the with x protected, and the alone): stock main differs from the plain chain in 8 of the 13 conditional runs, fix A in 1, fixes B and B+ in none, the hunt's sketch in 2. A second probe covers the related stacked last token: of its 9 conditional runs, stock main differs in 4, fixes A and B in 5, fix B+ in none.

Fix B (next option) for the reported bug. For the related case: a stacked token (increment 0) that goes to the wrapped filters is handed to them with an increment of 1 (ConditionalTokenFilter.java:56-60); mid-stream the class takes it back (:188-192, :224-227), at the end of the stream it did not. When adjustPosition is still set at the end, end() now takes the extra position back after delegate.end(); Math.max(0, ...) keeps the increment from going negative. +13/-2 lines, 7 of the added lines are comments.

New output. The 21-chain probe prints the same lines as with fix B: all 13 conditional runs equal their plain chain. The stacked-token probe: all 9 conditional runs equal their plain chain (x/1 a/0 ends with 0; with an input trailing hole 1; the public-API SynonymGraphFilter chain 0; a repeated end() 0 and 0), assertions on and off.

     public void end() throws IOException {
-      // imitate Tokenizer.end() call - endAttributes, set final offset
+      // imitate the end() of our input: ... once the input is exhausted, its final position increment
+      int finalPosInc = 0;
       if (exhausted) {
         ...
         endOffset = offsetAtt.endOffset();
+        finalPosInc = posIncAtt.getPositionIncrement();
       }
       endAttributes();
       offsetAtt.setOffset(endOffset, endOffset);
+      posIncAtt.setPositionIncrement(finalPosInc);
     }
   ...
     if (lastTokenFiltered) {
       this.delegate.end();
-      endState = captureState();
+      if (adjustPosition) {
+        // the last token was stacked (increment 0), went to the wrapped filters with an increment
+        // of 1 and they emitted nothing for it: take that position back, as endDelegating() does
+        posIncAtt.setPositionIncrement(Math.max(0, posIncAtt.getPositionIncrement() - 1));
+      }
     }

Trade-offs

• Those of fix B, plus: like the mid-stream code, it assumes the wrapped filters counted the increment of 1 they were handed for the stacked token, as FilteringTokenFilter does; a wrapped filter that drops a token without counting its increment would now report one position fewer at the end (never below 0).
• It changes the final increment only when a stacked last token went to the wrapped filters and they emitted nothing for it. The stacked-token test uses the test framework's CannedTokenStream; the probe's SynonymGraphFilter chain shows the same with public API only.

What the checks validated

• The 5 new tests, the 2 stacked-token tests and the closest upstream suites pass with assertions on and off: 51 of 51 (TestConditionalTokenFilter 12, TestProtectedTermFilter 1, TestProtectedTermFilterFactory 5, TestCustomAnalyzer 26); with fix B the 2 stacked-token tests fail, on stock main 1 of them.
• All of :lucene:analysis:common:test with assertions on and -Ptests.iters=4: 341 classes, 7,661 test runs, 69 skipped, 0 failures.
• TestRandomChains (which wraps random filters in conditional filters), 2 methods x 4 iterations: 8 runs, 0 failures.
• :lucene:analysis:common:check -x test passes (google-java-format, ecjLint, forbiddenApis).
• Through the repro harness: the new test 5 of 5, the finder's JUnit 5 of 5, the independent reproducer's 8 of 8 and the stacked-token test 2 of 2 (assertions on and off) pass.

When the input is exhausted, OneTimeWrapper.end() (ConditionalTokenFilter.java:84-95) keeps the input's final position increment instead of resetting it to 0; mid-stream (endDelegating(), input not exhausted) it still reports 0, so the additive path is unchanged. end() keeps the wrapped filters' state as before, which now includes the input's hole plus anything they add, and no longer re-captures endState after delegate.end() (:149), so endState stays the input's end state and a repeated end() returns the same value. +7/-2 lines, 4 of the added lines are comments.

New output. All 13 conditional chain runs of the probe equal their plain chain: abc the 1, abc the the 2, abc a the 2, the hole in the middle unchanged, a ShingleFilter after or inside the condition gives abc, abc _, and a repeated end() gives 2 and 2.

     public void end() throws IOException {
-      // imitate Tokenizer.end() call - endAttributes, set final offset
+      // imitate the end() of our input: ... once the input is exhausted, its final position increment
+      int finalPosInc = 0;
       if (exhausted) {
         ...
         endOffset = offsetAtt.endOffset();
+        finalPosInc = posIncAtt.getPositionIncrement();
       }
       endAttributes();
       offsetAtt.setOffset(endOffset, endOffset);
+      posIncAtt.setPositionIncrement(finalPosInc);
     }
   ...
     if (lastTokenFiltered) {
       this.delegate.end();
-      endState = captureState();
     }

Trade-offs

• Not covered: the related stacked last token stays wrong (x/1 a/0: 1 instead of 0), and with an input trailing hole after it fix B gives 2 instead of 1, a value stock main gets right by accident; the stacked-token test fails 2 of 2 with fix B (gradle and the harness).
• The wrapped filters now see at the end of the stream what they see without the condition; one that reads the final increment (ShingleFilter, FlattenGraphFilter) acts on the input's trailing hole as in a plain chain.
• Indexed positions change only after a value whose last token was filtered and that ends in a hole; an existing index keeps the old positions until reindexed. No benchmark was run (one int read and write per end()).

What the checks validated

• The 5 new tests and the closest upstream suites pass with assertions on and off: 49 of 49 (TestConditionalTokenFilter 12, TestProtectedTermFilter 1, TestProtectedTermFilterFactory 5, TestCustomAnalyzer 26); stock: the 5 new tests fail.
• All of :lucene:analysis:common:test with assertions on and -Ptests.iters=4: 340 classes, 7,653 test runs, 69 skipped, 0 failures.
• TestRandomChains (which wraps random filters in conditional filters), 2 methods x 4 iterations: 8 runs, 0 failures.
• :lucene:analysis:common:check -x test passes (google-java-format, ecjLint, forbiddenApis).
• Through the repro harness: the finder's JUnit 5 of 5 and the independent reproducer's 8 of 8 pass.
• The module-wide and TestRandomChains runs above used an earlier revision of the new test (same assertions; it differed only in the phrase assertion's message and the whenTerm predicate); the fact check re-ran both with the final revision and a new seed (FC542B01): 340 classes, 7,653 runs and 8 runs, 0 failures.
• Not covered: the related stacked-token case; its 2 tests fail with fix B (gradle and the harness).

Read the input's final increment before delegate.end(), add it to what the wrapped filters report, and drop the re-capture of endState (+5/-1, 3 comment lines). This is the hunt's sketch plus the re-capture fix.

Trade-offs

Smaller, and keeps the "imitate Tokenizer.end()" design, but the wrapped filters still see 0 at the end: a ShingleFilter inside the condition gives abc (end 1) instead of abc, abc _. The other 12 conditional runs of the probe equal the plain chain. Like fix B, it does not cover the stacked last token (1 instead of 0; 2 instead of 1 with an input trailing hole).

What the checks validated

• The 5 new tests and the 4 closest suites pass with assertions on and off (49 of 49; with the final test revision, this doc's run with assertions on and the fact check's also with them off); all of :lucene:analysis:common:test (-Ptests.iters=4: 340 classes, 7,653 runs, 0 failures) and TestRandomChains (8 runs, 0 failures) pass; the finder's and the reproducer's JUnit pass shadowed.
• check -x test: this doc's run failed only on an ecjLint error in the earlier revision of the new test (.whenTerm(t -> true), an unused lambda parameter), not on fix A; the fact check re-ran it with fix A and the final test revision, and it passes.

Option A without dropping endState = captureState().

Trade-offs

Passes the new test (5 of 5) and, in the hunt, the 44 tests of the four closest suites, but a second end() call adds the input's increment again: a condition around StopFilter({"a"}) on abc a the gives 2, then 3. A second end() is outside the TokenStream contract, so this is robustness, not a reachable bug; the checkers flagged it as the sketch's open point.

• Method (non-formal): code reading in the hunt's unit on analysis filters against semantic references, while modelling the end() contract of the n-gram filters; the LUCENE-8360 report says that bug was found through ConditionalTokenFilter, which pointed the finder at this class. It was outside the unit's planned file list (same module and contract), and the unit's exhaustive harness did not find it.
• Confirmation. An executed probe, then a public-API JUnit (WhitespaceTokenizer, StopFilter, ProtectedTermFilter, CustomAnalyzer.whenTerm, IndexWriter) that fails on main, 10.5.2 and 9.12.3, and a release-portable main() that fails from 7.4.0 on. Independent checkers followed (see "Independent checks").