A rejected DENSE vector batch leaves the HNSW and scalar-quantized writers out of step with a custom flat field writer

Since #16734, IndexWriter#addBatch hands a DENSE vector column to the field's vectors writer in one addDenseValues call, through a cursor that validates each vector as it is consumed. The core HNSW and scalar-quantized writers forward that call to the flat field writer they wrap and update their own state only if it returns normally, assuming the flat writer stores all of the batch or none of it. Lucene's own flat writer does; the default addDenseValues, which every other flat field writer inherits, stores one vector at a time. So when Lucene rejects vector k >= 1 of a batch (NaN or infinite, a zero vector with COSINE, a wrong length, or a cursor that throws), such a flat writer keeps vectors 0..k-1 while the wrapper counts none, and stays k vectors behind. With assertions on, the next commit fails and closes the IndexWriter; with them off, the HNSW writer commits a graph with fewer nodes than vectors, on which kNN search returns wrong documents or throws and fail-fast CheckIndex fails, and the COSINE scalar-quantized writer fails at commit or commits a segment in which kNN finds nothing. It needs a non-default vectors format, such as upstream's own TestKnnVectorsFormatCustomWriter composition, and the experimental addBatch; the stock formats are not affected. Main only, unreleased.

• DENSE vector column. IndexWriter#addBatch (experimental) indexes a batch column by column. Since #16734 a column declared DENSE (one vector per document) goes to the field's vectors writer in a single addDenseValues(firstDocID, cursor) call (IndexingChain.java:1318-1369). The cursor checks each vector as it hands it out (ValidatingVectorValuesCursor.java:141-147); a bad one throws, the batch's documents are deleted, and the writer stays open.
• Flat field writer. Buffers the raw vectors of one field until flush. The HNSW writer and the scalar-quantized (SQ) writer wrap one: the HNSW writer adds a graph node per vector, the SQ writer adds each vector to the centroid sums and, for COSINE, caches its length (magnitude).
• The default addDenseValues (KnnFieldVectorsWriter.java:58-63) calls addValue once per vector. Lucene's own flat writer overrides it all-or-nothing (Lucene99FlatVectorsWriter.java:507-531); any other flat field writer, for example one plugged in through the public strategyFactory constructor (:87-105, since 10.5.0), inherits the default.

1. The HNSW writer forwards the batch first, flatFieldVectorsWriter.addDenseValues(firstDocID, values) (Lucene99HnswVectorsWriter.java:857), and only after it returns advances its node counter and adds graph nodes (:858-868). The SQ writer accumulates the new vectors only after the delegate returns (Lucene104ScalarQuantizedVectorsWriter.java:750-757); its javadoc says "The flat writer records nothing if the cursor throws, so neither does this writer" (:745-748).
2. With the default, a batch rejected at vector k leaves vectors 0..k-1 in the flat writer, which the contract allows: "every doc ID it has recorded must have its vector" (KnnFieldVectorsWriter.java:46-48). The wrapper counted none of them: it is now k vectors behind.
3. HNSW. The next addValue stores the vector in the flat writer and calls addGraphNode(node) with the wrapper's counter (:839). The graph scores ordinals against the flat writer's vectors (:797-801), so it inserts the vector of a deleted batch document, and the new vector, k ordinals further, gets no node. The graph ends with fewer nodes than vectors.
4. Flush, assertions on. writeMeta asserts nodesOnLevel.size() == count : "Level 0 expects to have all nodes" (:647, count from the flat writer, :272). The exception is tragic: the writer closes and drops everything not committed.
5. Flush, assertions off. The short graph is written. The reader sizes level 0 by the vector count (Lucene99HnswVectorsReader.java:589-591) and seeks without bounds checks (:603-604), so reading a missing node, or an upper-level node whose offsets the missing ones shift, throws EOFException; a query that meets only existing nodes returns a wrong document.
6. SQ, COSINE. The prefix has no cached magnitude, so at flush scaleToUnitLength divides each later ordinal by another vector's magnitude (:828-833) and the last ordinals read past the list: an AssertionError in the quantizer (assertions on); with assertions off FloatArrayList.get checks the index only by assertion (FloatArrayList.java:130-135), so it throws ArrayIndexOutOfBoundsException past its backing array or returns a never-written 0 (a division by zero, by reading), after which kNN finds no document of the segment. For EUCLIDEAN and DOT_PRODUCT only the centroid is biased (by reading; not tested).

Before #16734 every vector of a column went through the wrapper's own addValue, so wrapper and flat writer moved together; the same scenario passes on the parent commit (NaN / zero and wrong-length triggers) and, with a wrong-length rejected vector, on 10.5.0 and 10.5.2 (10.5.x does not reject NaN or zero vectors).

• Who is hit. Applications on main that (1) use a vectors format in which the core HNSW writer or the core scalar-quantized writer wraps a flat field writer that does not override addDenseValues: their own per-field storage through the public Lucene99FlatVectorsWriter(state, scorer, strategyFactory) constructor (#16053, released in 10.5.0; upstream's TestKnnVectorsFormatCustomWriter builds exactly this, :84-96), or their own FlatVectorsWriter under the public writer constructors; every field writer written for 10.5 inherits the default without any change; (2) index vectors through the experimental addBatch (or updateDocuments(Term|Query, ColumnBatch)) with a DENSE VectorColumn; (3) send a batch in which a vector after the first is rejected (NaN or infinite, a zero vector with COSINE, a wrong length) or whose cursor throws. A NaN embedding or a zero vector is an ordinary data accident.
• Not affected. The stock core formats: their flat writer is all-or-nothing (controls pass; the hunt's bounded sweep found 0 mismatches for the stock HNSW stack in 5,562 scenarios per mode; its few stock HNSW + SQ and stock SQ mismatches are all the separate LFV-047). The shipped sandbox Dedup formats have the composition and reproduce it (outside the hunt's scope).
• Scale. None beyond a segment that ends up with an HNSW graph: with the default tiny-segment threshold, more than 100 * ln(n) vectors, about 650 (Lucene99HnswVectorsWriter.java:716-722); a probe with upstream's composition failed after 1,000 documents, and after 3 documents followed by enough to build the graph later, and passed a 600-document segment that never builds a graph. With tinySegmentsThreshold <= 0 any size fails. The COSINE SQ case has no size threshold.
• Assertions on (tests, many development setups): the next flush or commit throws, the IndexWriter closes with a tragic exception, and every document added since the last commit is lost.
• Assertions off (production): HNSW commits a graph that lacks the field's last vectors; kNN returns wrong documents or throws EOFException (all 1,010 queries in the 1,000-document probe). COSINE SQ throws at commit and closes the writer, or commits a segment in which kNN finds no document. The damage stays until the segment is replaced (for example by reindexing); whether a later merge repairs or spreads it was not tested.
• Hard to see. CheckIndex checks vectors only at level 2 or higher (CheckIndex.java:1114-1144), which is not its default (level 0 in code unless setLevel is called, level 1 on the command line). At level 3 without fail-fast it reports the HNSW graph error of the small threshold-0 case but calls the index clean (LFV-561); in the 1,000-document case its vector search also fails and it reports the index broken. The COSINE SQ segment with no kNN hits passed CheckIndex at level 3 either way.
• Why the suites miss it. BaseKnnVectorsFormatTestCase never calls addBatch, so TestKnnVectorsFormatCustomWriter never reaches addDenseValues; the addBatch tests use the stock formats only.
• Downstream. A codec that builds the core HNSW writer over its own flat field writer, written before addDenseValues existed, inherits the default; one such downstream format was found by read-only code reading. No downstream product is known to send DENSE vector columns through addBatch today.

# Lucene main 645d4fe14a20, assertions on (-ea), seed DEADBEEF: 1 test(s) run, 1 failed
    => java.lang.AssertionError: Level 0 expects to have all nodes
       org.apache.lucene.codecs.lucene99.Lucene99HnswVectorsWriter.writeMeta(Lucene99HnswVectorsWriter.java:647)
       org.apache.lucene.codecs.lucene99.Lucene99HnswVectorsWriter.writeField(Lucene99HnswVectorsWriter.java:268)
       org.apache.lucene.codecs.lucene99.Lucene99HnswVectorsWriter.flush(Lucene99HnswVectorsWriter.java:224)
       org.apache.lucene.codecs.perfield.PerFieldKnnVectorsFormat$FieldsWriter.flush(PerFieldKnnVectorsFormat.java:126)
       org.apache.lucene.index.VectorValuesConsumer.flush(VectorValuesConsumer.java:76)
       org.apache.lucene.index.IndexingChain.flush(IndexingChain.java:336)
       org.apache.lucene.index.DocumentsWriterPerThread.flush(DocumentsWriterPerThread.java:492)
       org.apache.lucene.index.DocumentsWriter.doFlush(DocumentsWriter.java:541)
       org.apache.lucene.index.DocumentsWriter.maybeFlush(DocumentsWriter.java:496)
       [...]

• The contract allows a partial batch. "The cursor may throw while it is being consumed ... In that case the documents of the whole batch are marked as deleted, but the writer must remain in a consistent state: every doc ID it has recorded must have its vector" (KnnFieldVectorsWriter.java:46-48). The default records a prefix by construction (:58-63), and overriding is an optimisation: "Override for a more efficient bulk path" (:52-53). No javadoc, CHANGES.txt or MIGRATE.md entry says a wrapped delegate must be all-or-nothing.
• The duty binds the wrappers too. Both wrappers override addDenseValues, so they must stay consistent; their own flush code needs one graph node per vector (:647) and one cached magnitude per ordinal (:828-829).
• Recovery is tested upstream. TestColumnBatchIndexing.testDenseVectorValidationFailureRecovers (:886-922) rejects a DENSE batch with NaN at position 2 and continues in the same segment: "the failed batch stays in the same segment as the recovery batch, so flush checks that the vectors writer was left consistent". It uses the stock format. #16708 added these rejections; they do not abort the writer.
• The composition is a released extension point. 10.5.0 CHANGES (GITHUB#16053): a field-writer creation strategy "to allow different formats to change how vectors are stored in memory"; its test exists to "ensure that the use of a different, custom strategy does not break any existing invariants". That test's writer keeps the default addDenseValues under the stock HNSW writer.
• The review asked for this to be safe. In #16734 jimczi asked that "The HNSW and scalar quantized field writers would also need to forward to their flat delegate" and that "the writer should only record doc IDs after fill returns"; mayya-sharipova added both. Nobody discussed a delegate that keeps the default; there is no by-design statement.
• The other paths are consistent. The same rows as a SPARSE column, and the pre-#16734 form on the parent commit (both triggers) and on 10.5.0 and 10.5.2 (a wrong-length trigger; 10.5.x accepts NaN and zero vectors), pass.

The strongest case for "intended" (an intended-behaviour checker and a cross-model refute review): the SQ writer's javadoc states the all-or-nothing assumption on purpose; the base javadoc tells wrappers to forward, which reads as expecting every delegate to override; every API involved is @lucene.experimental; a batch that fails part-way is caller error; and a committer could declare that delegates must be all-or-nothing.

Why it fails: the SQ sentence sits on a package-private nested class and records an assumption that holds for one delegate only, against the base-class default that every other flat field writer inherits, while the public constructors accept any flat writer. The "should forward" sentence is about reaching the delegate's fast path; the same javadoc calls overriding optional. "Experimental" allows the API to change, not a core writer to close the IndexWriter or commit a malformed graph, and upstream's own test builds the composition. The rejection is Lucene's own non-aborting validation, and a caller cursor that throws does the same. If upstream decides delegates must be all-or-nothing, the default and the "every doc ID it has recorded" wording would need to change, or a stronger precondition be documented for delegates wrapped by these writers: either way the wrappers and the default disagree today. The refute reviewer agreed with the root cause and rated the intended case weak; its "not real" was procedural (it could not run code). Verdict: not intended.

One fix was built and validated (option A, from the hunt); the others are alternatives upstream may prefer, not tested.

In Lucene99HnswVectorsWriter.FieldWriter.addDenseValues, catch the delegate's exception, add graph nodes (or create the graph) for [firstNode, flat writer size), then rethrow, keeping the cursor's exception primary and suppressing any failure of the catch-up into it. In Lucene104ScalarQuantizedVectorsWriter.FieldWriter.addDenseValues, accumulate [firstOrd, vectors.size()) in a finally. +37/-13 lines in two files; the stock flat writer's bulk path is unchanged.

diff --git a/lucene/core/src/java/org/apache/lucene/codecs/lucene104/Lucene104ScalarQuantizedVectorsWriter.java b/lucene/core/src/java/org/apache/lucene/codecs/lucene104/Lucene104ScalarQuantizedVectorsWriter.java
index 322585e7fc4..62b93c4a77a 100644
--- a/lucene/core/src/java/org/apache/lucene/codecs/lucene104/Lucene104ScalarQuantizedVectorsWriter.java
+++ b/lucene/core/src/java/org/apache/lucene/codecs/lucene104/Lucene104ScalarQuantizedVectorsWriter.java
@@ -749,10 +749,14 @@ public class Lucene104ScalarQuantizedVectorsWriter extends FlatVectorsWriter {
     @Override
     public void addDenseValues(int firstDocID, VectorValuesCursor<T> values) throws IOException {
       final int firstOrd = getVectors().size();
-      flatFieldVectorsWriter.addDenseValues(firstDocID, values);
-      final List<T> vectors = getVectors();
-      for (int ord = firstOrd; ord < vectors.size(); ord++) {
-        accumulate(vectors.get(ord));
+      try {
+        flatFieldVectorsWriter.addDenseValues(firstDocID, values);
+      } finally {
+        // the delegate may have recorded a prefix of the batch before the cursor threw
+        final List<T> vectors = getVectors();
+        for (int ord = firstOrd; ord < vectors.size(); ord++) {
+          accumulate(vectors.get(ord));
+        }
       }
     }
 
diff --git a/lucene/core/src/java/org/apache/lucene/codecs/lucene99/Lucene99HnswVectorsWriter.java b/lucene/core/src/java/org/apache/lucene/codecs/lucene99/Lucene99HnswVectorsWriter.java
index 3287674b25a..898c5dabbc3 100644
--- a/lucene/core/src/java/org/apache/lucene/codecs/lucene99/Lucene99HnswVectorsWriter.java
+++ b/lucene/core/src/java/org/apache/lucene/codecs/lucene99/Lucene99HnswVectorsWriter.java
@@ -854,17 +854,37 @@ public final class Lucene99HnswVectorsWriter extends KnnVectorsWriter {
       if (count == 0) {
         return;
       }
-      flatFieldVectorsWriter.addDenseValues(firstDocID, values);
       final int firstNode = node;
-      node += count;
-      lastDocID = firstDocID + count - 1;
-      if (hnswGraphBuilder != null) {
-        for (int ord = firstNode; ord < node; ord++) {
-          hnswGraphBuilder.addGraphNode(ord);
+      Throwable failure = null;
+      try {
+        flatFieldVectorsWriter.addDenseValues(firstDocID, values);
+      } catch (Throwable t) {
+        failure = t;
+      }
+      // The delegate may have recorded a prefix of the batch before the cursor threw (the default
+      // addDenseValues adds one vector at a time): index exactly the vectors it holds.
+      final int added = flatFieldVectorsWriter.getVectors().size() - firstNode;
+      if (added > 0) {
+        node += added;
+        lastDocID = firstDocID + added - 1;
+        try {
+          if (hnswGraphBuilder != null) {
+            for (int ord = firstNode; ord < node; ord++) {
+              hnswGraphBuilder.addGraphNode(ord);
+            }
+          } else if (shouldCreateGraph(graphThreshold, node)) {
+            initializeGraphBuilder();
+            replayBufferedVectors();
+          }
+        } catch (Throwable t) {
+          if (failure == null) {
+            throw t;
+          }
+          failure.addSuppressed(t); // keep the cursor's exception as the primary one
         }
-      } else if (shouldCreateGraph(graphThreshold, node)) {
-        initializeGraphBuilder();
-        replayBufferedVectors();
+      }
+      if (failure != null) {
+        throw IOUtils.rethrowAlways(failure);
       }
     }
 

Trade-offs

• For: fixes every delegate that keeps a consecutive prefix of the batch (the default does), including flat field writers written for 10.5 and downstream ones, with no change to them, provided the catch-up itself succeeds; the rejected batch's recorded vectors stay in the segment as deleted documents, exactly as on the SPARSE path.
• Against: the wrappers rely on getVectors().size() being the flat writer's vector count (true for every FlatFieldVectorsWriter; it is also what replayBufferedVectors uses); catching Throwable means the catch-up also runs after an Error, then rethrows it; in the SQ part an exception from accumulate in the finally would replace the cursor's exception (unlikely: only accepted vectors are accumulated); lastDocID = firstDocID + added - 1 assumes the recorded vectors belong to the batch's first added documents, which the public contract does not state.
• Not done: a regression test for BYTE and FLOAT16 fields (the hunt's sweep shows HNSW mismatches there too; the JUnit uses float32), benchmarks (by reading, the success path gains no per-vector work: a try block and one size query per batch), x86.

What the checks validated

Through gradle in a scratch worktree of a9779ef3c193, JDK 25.0.3, ./gradlew --no-daemon, seed DEADBEEF unless stated:

Also: the kNN probe with the fix compiled onto the main jars (all cases clean, assertions on and off, 3 seeds); in the hunt, the finder's JUnit 4 of 4 and its sandbox variant 3 of 3, the independent reproducer's 5 of 5, and a separate randomized harness run with the fix (3,586 builds, assertions on) has 0 custom-strategy mismatches; the cross-model reviewer ran upstream TestKnnVectorsFormatCustomWriter with the fix (57 tests pass).

Consume and copy the whole batch before the first addValue, so a cursor that throws leaves any writer that keeps the default unchanged.

Trade-offs

Fixes every wrapper at once, but the default cannot allocate a T itself: copying through copyValue breaks writers whose copyValue is unsupported, including upstream's own PagedFieldVectorsWriter (TestKnnVectorsFormatCustomWriter.java:179-185) and both wrappers; next() may reuse its array, so a copy is required; the batch is held twice in memory. Untested.

Change the addDenseValues and wrapper javadocs only.

Trade-offs

No code change, but every existing strategy and downstream flat writer stays broken until it overrides addDenseValues, and the "every doc ID it has recorded" sentence and the per-vector default would contradict the rule. Untested.

Drop the two overrides, so the default calls the wrapper's own addValue per vector, as before #16734.

Trade-offs

Keeps wrapper and delegate in step per vector, as before #16734, but the stock flat writer's bulk path is never reached, which the #16734 review explicitly asked for. Untested.

Method: Z3 bounded model checking of the vectors-writer stack, then a bounded-exhaustive differential harness (the study's "semi-formal" tier: the first signal came from reading the code while modelling).

• The hunt unit targeted the code new in #16734. Its Z3 model (dense_vector_commit.py, extending an earlier unit's dense-commit model) covers addValue / addDenseValues with a cursor that throws at position k. With the stock all-or-nothing flat writer, its four invariants hold to depth 4 and an inductive invariant is proved for unbounded counts; with a delegate that keeps the default addDenseValues, the HNSW node counter, the graph ordinals and the SQ magnitudes invariants are violated by 4-operation witnesses (the bounded model checking unrolls exactly 4 operations), and a 1-step counterexample to induction exists.
• A bounded-exhaustive harness on the real jars compared DENSE against SPARSE against addDocument over batch sizes 1 to 6, every failure position, 4 failure kinds, 3 encodings, 3 similarities, 3 thresholds and index sort on and off: 944 to 1,415 mismatches per mode for the custom-strategy HNSW stacks, 146 to 242 for SQ, 0 for the stock HNSW stack. In a separate randomized run with the fix (3,586 builds, assertions on) the custom-strategy mismatches drop to 0. The stock HNSW + SQ and stock SQ stacks show a few mismatches per mode, all the separate LFV-047.
• The finder then wrote a public-API JUnit (2 of 4 fail, 25 of 25 iterations, with assertions on and off). This doc's JUnit is a compact rewrite of it with the default-threshold case added.