ImpactsSource#advanceShallow javadoc omits that the current document must not be read after a shallow advance past it

ImpactsSource#advanceShallow(target) lets a caller look at score upper bounds for documents at or after target while docID() stays where it is. Its javadoc lists what becomes illegal afterwards (nextDoc() while docID() < target - 1, advance() below target) and says nothing about the current document. The default codec, however, moves its internal streams to the target block during the shallow advance and refills only at the next nextDoc()/advance(). So code that follows the written contract and reads the current document after peeking ahead gets another block's positions or run end, and a lazily decoded freq() (whose value is still right) breaks the next legal advance(): in the repro, advance(256) returns 261 although every document contains the term.

The codec side is intended (the javadoc calls the shallow advance "cheaper than calling advance", and no checked in-tree caller reads the current document after a shallow advance past it without moving first; AssertingScorer already rejects the Scorer-level form). What is missing is the precondition in the javadoc and a check in the test framework's AssertingImpactsEnum, which passes freq() and nextPosition() through and whose eager freq() hides the symptom from randomized tests. The gap exists in every release from 8.0.0 to 10.6.0 and on main.

1. Freqs are decoded lazily. A full-block refill decodes the doc ids, remembers where the block's freqs start (freqFP) and skips over them (Lucene104PostingsReader.java:626-631); freq() seeks back and decodes them on first use (:581-589). After nextDoc() on doc 0, the doc stream sits at the start of block 1's skip data (skip data is stored in front of each block).
2. The shallow advance moves the streams, not the document. advanceShallow(256) sees target > level0LastDocID (255) and runs doAdvanceShallow / skipLevel0To, which reads block 1's skip data from the doc stream (and, with positions, seeks the position stream), then sets needsRefilling = true (:879-895, :818-876). doc and the decoded buffers stay on block 0. The field comment: "true if we shallow-advanced to a new block that we have not decoded yet" (:406-407).
3. The lazy freq() moves the doc stream back. It seeks to block 0's freqs and decodes them: the value for doc 0 is right (1), but the stream is left at block 1's skip header again.
4. The next legal advance() decodes the wrong bytes. advance(256) refills because needsRefilling is set (:924-931), from the current stream position, so it reads the skip header as the block's bits-per-value byte and packed doc deltas (:591-596) and lands on 261.
5. Other reads. nextPosition() reads the position stream, which already points at the target block; docIDRunEnd() combines the old block's dense bit set with the moved level bounds (:1099-1129); that path exists only when the current block is a fully dense bit set (UNARY encoding), otherwise the default one-doc run is returned, which is correct. The study's security probe also saw intoBitSet() not return within 20 s (5 of 6 cases) and nextPostings() skip matches with assertions off; nextPostings starts with assert needsRefilling == false (:1036).

For nextDoc()/advance(), AssertingImpactsEnum checks only the two documented limits (AssertingLeafReader.java:701-713) and passes freq() and nextPosition() straight through (:671-679). Its inner AssertingPostingsEnum calls freq() eagerly in every nextDoc()/advance() (:502, :521), which decodes the freqs before any shallow advance: through the wrapper, the repro's advance(256) returns 256 and no assertion fires. SlowImpactsEnum keeps the current document valid.

• Minimal repro (1,200 docs, all with t, freq 1 to 5): advance(256) after freq() returns 261; without freq(), 256.
• Study tests (two 4,000-document indexes, advanceShallow(3000)): with t in every third document, positions of doc 0 [5] instead of [0], of doc 3 [2, ...] instead of [1, 4], and freq() then advance(3000) = 3005, a document without t; with the term in docs 0-255 (one dense block) and then in every third document, docIDRunEnd() from doc 0 = 3328 although doc 256 does not match.
• Other layouts: EOFException, IndexOutOfBoundsException, or a doc id beyond maxDoc (2897 with 1,200 docs on 9.12.x).
• One level up, a TermScorer in TOP_SCORES mode: advanceShallow(256), score(), then advance(256) also returns 261 (AssertingScorer rejects that score() with SHALLOW_ADVANCING).

• Who. Code that uses ImpactsEnum directly (from the public TermsEnum#impacts, TermsEnum.java:172-177) and reads freq(), positions, docIDRunEnd() or bulk postings of the current document after advanceShallow(target) into a later block. The same order through Scorer#advanceShallow and score() on a TermScorer gives the same result; that javadoc says the call "doesn't modify the current docID()" (Scorer.java:66).
• What goes wrong. Wrong per-document data, then a wrong next document (one without the term), an EOFException, or a doc id beyond maxDoc. The same with assertions on and off.
• Who is not hit. TermQuery, PhraseQuery, SynonymQuery, FeatureQuery, WAND, MaxScore, block-max conjunctions, ReqOptSumScorer and CheckIndex move the iterator after a shallow advance past the current document before reading (CheckIndex reads after advanceShallow(docID()), which is harmless); non-scoring clauses get plain postings, at most wrapped in a SlowImpactsEnum whose advanceShallow does nothing; the study's security probe found stock top-scores queries over 20 fixed and 400 random shapes matching exhaustive scoring. No caller was found in Elasticsearch, OpenSearch or Solr main (not exhaustive).
• Default vs opt-in. Default codec, no configuration; it takes direct use of an @lucene.experimental API in an order the javadoc does not forbid. The freq-then-advance symptom needs a second full 256-doc block after the current one (terms with 334 and 500 postings were fine, 512 to 2,500 were not).
• Scale and age. 1,200 documents suffice; the gap is in every release from 8.0.0.

• Deterministic input. ByteBuffersDirectory, IndexWriterConfig() with the default StandardAnalyzer, the default codec pinned, no random() in the test (the test framework still randomizes locale, time zone and the like); seeds DEADBEEF, 5EED1234 and three more give the same failure, with assertions on and off.
• Control. The same test without the impacts.freq() line passes: advance(256) returns 256.
• It transcribes the current javadoc. Once the precondition is documented, this call order is off-contract: the test is the reason for the sentence, not a regression test for a code change, and no correct codec change is expected to make it pass.
• Regression test for the assertion half (optional): this same test with the reader wrapped in AssertingDirectoryReader and the last two lines replaced by expectThrows(AssertionError.class, impacts::freq); assertEquals(256, impacts.advance(256)); (method testFreqAfterAdvanceShallowIsRejected). On stock main it fails ("Expected exception AssertionError but no exception was thrown"); with option B it passes. It requires assertions: run it with -Ptests.asserts=true (without it the build enables assertions for only about 75% of seeds).
• Releases. The same file fails unchanged on the 10.6.0 and 10.5.2 release jars (261) and on 9.12.0 to 10.2.0 (2897 or 258); it passes on 9.11.1, where the freq value itself is wrong instead (Version history).

cp $REPRO/TestAdvanceShallowThenFreqThenAdvance.java $LUCENE/lucene/core/src/test/org/apache/lucene/index/
cd $LUCENE
./gradlew :lucene:core:test --tests org.apache.lucene.index.TestAdvanceShallowThenFreqThenAdvance -Ptests.seed=DEADBEEF -Ptests.asserts=true
# assertions off: -Ptests.asserts=false (same failure)

# gradle, main 645d4fe14a20, seed DEADBEEF, -Ptests.asserts=true
TestAdvanceShallowThenFreqThenAdvance > testAdvanceAfterFreq FAILED
    java.lang.AssertionError: advance(256) after freq() expected:<256> but was:<261>
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:465540CFEC35E1F9]:0)
        at org.junit.Assert.fail(Assert.java:89)
        at org.junit.Assert.failNotEquals(Assert.java:835)
        at org.junit.Assert.assertEquals(Assert.java:647)
        at org.apache.lucene.index.TestAdvanceShallowThenFreqThenAdvance.testAdvanceAfterFreq(TestAdvanceShallowThenFreqThenAdvance.java:48)
:lucene:core:test (FAILURE): 1 test, 1  failure
# -Ptests.asserts=false: the same failure
# control (standalone): the same test without the freq() line passes, advance(256) = 256 (assertions on and off)
# release jars (standalone): 10.6.0 and 10.5.2: 261; 10.1.0, 10.2.0: 258; 9.12.0, 9.12.3, 10.0.0: 2897; 9.11.1: passes
# with fix A + fix B applied: this test still fails the same way (no behaviour change);
#   the same test with the reader wrapped in AssertingDirectoryReader, expecting AssertionError from freq(), passes

• The javadoc enumerates the forbidden calls (ImpactsSource.java:33-37): "After this method has been called, nextDoc() may not be called if the current doc ID is less than target - 1 and advance(int) may not be called on targets that are less than target."
• The inherited contracts point the other way. PostingsEnum#freq (PostingsEnum.java:74-77): "Returns term frequency in the current document ... Do not call this before nextDoc is first called, nor after nextDoc returns NO_MORE_DOCS." DocIdSetIterator#advance for target > current "behaves as if written" as a nextDoc() loop (DocIdSetIterator.java:144-153), so advance(256) must return 256. docIDRunEnd(): "All docs in range [docID(), docIDRunEnd()) match this iterator" (:251). Scorer#advanceShallow: "Calling this method doesn't modify the current docID()" (Scorer.java:66).
• Users are sent here. The index package overview shows impactsEnum.advanceShallow(targetDocID) (index/package-info.java:228-248). On java-user ("How to access block-max metadata?", 2020-10-12) Adrien Grand pointed a user to TermsEnum#impacts and the "documentation for ImpactsSource#advanceShallow and ImpactsSource#getImpacts", and wrote that "advanceShallow only advances the cursor for block-max metadata, this allows reasoning about maximum scores without actually advancing the doc ID".
• Not self-evident to committers. In the #14390 review (2025-03-22) gf2121 asked whether docIDRunEnd should be tested against "advance, intoBitset, advanceShallow", and jpountz answered that "'sane' impls of docIDRun should be free of side effects".
• The rule exists only in code. AssertingScorer (LUCENE-8135, 2018) enters SHALLOW_ADVANCING after advanceShallow(target != doc) and rejects score() (AssertingScorer.java:110, :127); nextPostings asserts needsRefilling == false (#14679); #14185 removed a special-case eager refill from advanceShallow (fixing its interaction with #14164), so the refill is always deferred since. No javadoc, package doc, CHANGES.txt or MIGRATE.md text states the precondition.

Strongest case for "works as designed" (the study's intended-behaviour checker and the cross-model refute review): ImpactsSource is @lucene.internal and ImpactsEnum @lucene.experimental; advanceShallow is the skip step of advance(); every checked in-tree caller follows "peek, then move"; AssertingScorer already encodes the rule; and the behaviour has been stable since 2018 with no complaint.

Why it fails. It shows the codec is right, which this page concedes; it does not make the javadoc complete. The javadoc lists the forbidden follow-ups explicitly, so a reader takes the list as complete; ImpactsEnum comes from the public TermsEnum#impacts, is shown in the package overview and was recommended to users by its author; the reviewer itself called the javadoc "genuinely misleading". The fix is one sentence and an assertion, with no behaviour change: a small hardening that upstream may still decline as "internal".

Javadoc plus test framework; the codec's deferred refill is intended and should not change. Options A and B were applied together in a scratch worktree of main 645d4fe14a20 and checked with gradle.

One sentence after the existing restrictions: per-document reads are not allowed after a shallow advance past docID() until the iterator is moved.

    * - 1} and {@link DocIdSetIterator#advance(int)} may not be called on targets that are less than
-   * {@code target}.
+   * {@code target}. If {@code target} is greater than the current doc ID, methods that read data of
+   * the current document, such as {@link PostingsEnum#freq()}, {@link PostingsEnum#nextPosition()}
+   * or {@link DocIdSetIterator#docIDRunEnd()}, may not be called until the iterator has been moved
+   * with {@link DocIdSetIterator#nextDoc()} or {@link DocIdSetIterator#advance(int)}: the current
+   * doc ID does not change, but implementations may have already moved their internal state past
+   * the current document.

Trade-offs

• Implementation-neutral (no mention of blocks); it mirrors the rule AssertingScorer already enforces (target != doc).
• It is stricter than the default codec needs (a target inside the current block moves no state there, Lucene104PostingsReader.java:879), which keeps the rule simple and future-proof. The methods are examples ("such as"); it does not say whether the bulk intoBitSet() and nextPostings() are covered, and B does not check them either.
• Scorer#advanceShallow has the same gap for score() (option C).

What the checks validated

• Gradle, scratch worktree of 645d4fe14a20: :lucene:core:checkGoogleJavaFormat and :lucene:core:renderJavadoc pass (together with B, in the second pass; the first stopped at the format check on the copied repro's 101-column comment), and the rendered ImpactsSource.html shows the new sentence.
• The minimal test was re-run with the fix and still fails (expected:<256> but was:<261>), as it must: the change is in a comment.
• Not run: gradlew check or the whole core suite.

Remember whether advanceShallow went past the current doc; assert on freq(), nextPosition(), startOffset(), endOffset() and getPayload(); clear the flag on nextDoc()/advance(). +19 lines in AssertingLeafReader.java (one blank), none removed.

    // true after advanceShallow() to a target beyond the current doc, until nextDoc() or advance()
    private boolean shallowAdvancedPastDoc;
    // advanceShallow(target), after the existing asserts:
      shallowAdvancedPastDoc |= target > docID();
    // first line of freq(), nextPosition(), startOffset(), endOffset(), getPayload():
      assert shallowAdvancedPastDoc == false : shallowAdvancedPastDocMessage("freq");
    // nextDoc() and advance(), before delegating:
      shallowAdvancedPastDoc = false;

Trade-offs

• Catches the misuse at the call, before any data is read, so the eager freq() in AssertingPostingsEnum no longer hides anything and can stay.
• It does not cover docIDRunEnd(), intoBitSet() or nextPostings() of the codec enum: AssertingImpactsEnum does not delegate those today (the wrapper runs the safe defaults).

What the checks validated

• Gradle, same worktree, A and B applied: :lucene:test-framework:checkGoogleJavaFormat and :lucene:core:checkGoogleJavaFormat pass.
• The wrapped form of the minimal test (reader in AssertingDirectoryReader, expectThrows(AssertionError.class, impacts::freq), then advance(256) = 256) fails on stock main ("Expected exception AssertionError but no exception was thrown") and passes with B (assertions on).
• None of twelve selected suites tripped the new assertion in the configurations run: TestTermScorer, TestWANDScorer, TestMaxScoreBulkScorer, TestBlockMaxConjunction, TestBlockMaxConjunctionBulkScorer, TestReqOptSumScorer, TestSynonymQuery, TestPhraseQuery, TestDisjunctionScoreBlockBoundaryPropagator, TestBoolean2, TestFeatureField and TestCheckIndex pass with -Ptests.codec=Asserting (the Asserting codec's postings come wrapped in AssertingImpactsEnum), 116 tests with 3 skipped (the log does not name them), seeds DEADBEEF and 5EED1234; TestAssertingLeafReader (test-framework) passes. Not a whole-tree result: TestBoolean2 builds some indexes with TestUtil.getDefaultCodec() (TestBoolean2.java:102,148,174,186), bypassing the forced codec.
• Not run: the whole core and module suites, nightly tests, other codecs forced.

Scorer.java:62-70 has the same gap for score(); AssertingScorer already enforces it, so only the javadoc is missing there.

Decode pending freqs before shallow-advancing and defer the position seek and the docCountLeft update to the refill. It would add work to a call the javadoc describes as "cheaper than calling advance" (ImpactsSource.java:30), so it would need a performance evaluation, and every checked in-tree caller already follows the protocol. Not built.

• Lead. Code reading of Lucene104PostingsReader.BlockPostingsEnum in a hunt unit (postings), while modelling it for the unit's differential test (191M checks, 0 mismatches). That test excluded per-document reads after a pending shallow advance by design, so no tool output gave the first signal.
• Witness. A public-API JUnit test on the stock jars, later reduced to the one-method test shown here (1,200 documents, deterministic).
• Verification. An independent reproducer, cross-model reviews (OpenAI GPT-5.6: an executed intended-lens review, a reading-only refute review in the study and an executed refute review of the reduced test), an intended-behaviour devil's advocate, an adjudication, version-history runs (8.0.0 to main, plus the introducing commit built from source), a security probe, a re-verification and tie-break on main 645d4fe14a20, and this brief's gradle runs.