Harden the document length sidecar after review

- check for the sidecar before opening the index rather than after. The
  open populates the whole file set, so the first start after scoring is
  enabled would fault in every segment's postings only to discard them
- treat a sidecar that covers more points than the index as untrustworthy,
  not just one that covers fewer, and warn with both counts the way
  `SortedBlockIndex::open` does. A longer one used to be accepted and then
  silently truncated when materialized
- unlink a stale sidecar when a build records no lengths. It was the only
  file here that could outlive the build that wrote it, and `open` would
  have read it as this build's
- say why an index is being rebuilt from payload instead of leaving a
  silent full re-index announced at debug level
- read `phrase_matching` from the config in the mmap builder, like the
  other two callers of the same helper pair, so the two halves of the
  sentinel rule cannot drift apart
- assert the length invariant the writers actually maintain, and correct
  what `files()` and the field comment claim

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

lib/segment/src/index/field_index/full_text_index/inverted_index/immutable_inverted_index.rs

@@ -50,7 +50,10 @@ pub struct ImmutableInvertedIndex {
     ///
     /// Parallel to `point_to_tokens_count` and zeroed wherever that vector is,
     /// so summing it never counts a deleted document. A zero can still be a
-    /// live document whose tokens were all filtered.
+    /// live document whose tokens were all filtered, but only until the index
+    /// is written out: `create` puts every point with no tokens into the "no
+    /// tokens" mask, so after a round trip through disk that document is
+    /// indistinguishable from a deleted one.
     pub point_to_doc_len: Option<Vec<u32>>,
     pub points_count: usize,
 }
@@ -412,11 +415,13 @@ impl From<MutableInvertedIndex> for ImmutableInvertedIndex {
             })
             .collect();
 
-        // Only pads a trailing point that was never given a length. The two
-        // are written as parallel files and indexed by point offset, so they
-        // have to stay the same length.
+        // The two are written as parallel files and indexed by point offset,
+        // so they have to stay the same length. Every writer already keeps them
+        // equal, since `index_str_tokens` is the only way in and it resizes
+        // both to `point_id + 1`; the resize is the release-build fallback for
+        // a writer that stops doing that.
         if let Some(lens) = point_to_doc_len.as_mut() {
-            debug_assert!(lens.len() <= point_to_tokens_count.len());
+            debug_assert_eq!(lens.len(), point_to_tokens_count.len());
             lens.resize(point_to_tokens_count.len(), 0);
         }
 

lib/segment/src/index/field_index/full_text_index/inverted_index/mod.rs

@@ -765,6 +765,87 @@ mod tests {
         );
     }
 
+    /// Rebuilding the same directory without lengths must not leave the
+    /// previous build's sidecar behind: `open` would read it as this build's,
+    /// at offsets that now belong to different documents.
+    #[rstest]
+    fn rebuilding_without_lengths_removes_the_sidecar(
+        #[values(false, true)] phrase_matching: bool,
+    ) {
+        let hw_counter = HardwareCounterCell::new();
+        let mmap_dir = tempfile::tempdir().unwrap();
+        let sidecar = mmap_dir.path().join(POINT_TO_DOC_LEN_FILE);
+        let empty_deleted = BitVec::new();
+
+        let with_lengths =
+            ImmutableInvertedIndex::from(mutable_inverted_index(64, 0, phrase_matching));
+        OnDiskInvertedIndex::create(mmap_dir.path().into(), &with_lengths).unwrap();
+        assert!(sidecar.exists());
+
+        // The same points, indexed by a build that records nothing.
+        let mut without_lengths = MutableInvertedIndex::new(phrase_matching, false);
+        for idx in 0..64 {
+            let tokens: Vec<String> = (0..=idx % 8).map(|_| generate_word()).collect();
+            without_lengths
+                .index_str_tokens(idx, &tokens, None, &hw_counter)
+                .unwrap();
+        }
+        let without_lengths = ImmutableInvertedIndex::from(without_lengths);
+        OnDiskInvertedIndex::create(mmap_dir.path().into(), &without_lengths).unwrap();
+
+        assert!(
+            !sidecar.exists(),
+            "a stale sidecar outlived the build that wrote it"
+        );
+        let opened = OnDiskInvertedIndex::<MmapFile>::open(
+            &MmapFs,
+            mmap_dir.path().to_path_buf(),
+            Populate::No,
+            phrase_matching,
+            &empty_deleted,
+        )
+        .unwrap()
+        .unwrap();
+        assert!(!opened.records_doc_len());
+        assert!(!opened.files().contains(&sidecar));
+    }
+
+    /// The other end of the same check: a sidecar covering more points than the
+    /// index has is as untrustworthy as one covering fewer, and used to be
+    /// accepted and then silently truncated when materialized.
+    #[rstest]
+    fn oversized_doc_len_sidecar_is_ignored(#[values(false, true)] phrase_matching: bool) {
+        let mutable = mutable_inverted_index(64, 0, phrase_matching);
+        let immutable = ImmutableInvertedIndex::from(mutable);
+
+        let mmap_dir = tempfile::tempdir().unwrap();
+        OnDiskInvertedIndex::create(mmap_dir.path().into(), &immutable).unwrap();
+
+        let sidecar = mmap_dir.path().join(POINT_TO_DOC_LEN_FILE);
+        let full = fs_err::metadata(&sidecar).unwrap().len();
+        fs_err::OpenOptions::new()
+            .write(true)
+            .open(&sidecar)
+            .unwrap()
+            .set_len(full + size_of::<u32>() as u64)
+            .unwrap();
+
+        let empty_deleted = BitVec::new();
+        let opened = OnDiskInvertedIndex::<MmapFile>::open(
+            &MmapFs,
+            mmap_dir.path().to_path_buf(),
+            Populate::No,
+            phrase_matching,
+            &empty_deleted,
+        )
+        .unwrap()
+        .expect("the index still opens");
+        assert!(
+            !opened.records_doc_len(),
+            "a sidecar longer than the index must not be trusted either",
+        );
+    }
+
     /// A truncated sidecar is treated as absent rather than padded, since the
     /// padding would read exactly like real zero-length documents.
     #[rstest]

lib/segment/src/index/field_index/full_text_index/inverted_index/on_disk_inverted_index/mod.rs

@@ -46,6 +46,16 @@ const POINT_TO_TOKENS_COUNT_FILE: &str = "point_to_tokens_count.dat";
 pub(super) const POINT_TO_DOC_LEN_FILE: &str = "point_to_doc_len.dat";
 const DELETED_POINTS_FILE: &str = "deleted_points.dat";
 
+/// Whether a document length sidecar is on disk, without opening the index.
+///
+/// The scoring gate needs the answer *before* `open`, which populates the whole
+/// file set: on the first start after scoring is enabled every existing segment
+/// would otherwise fault in its postings, its vocabulary and its counts only to
+/// be discarded and rebuilt from payload.
+pub(in super::super) fn has_doc_len_sidecar(path: &Path) -> bool {
+    path.join(POINT_TO_DOC_LEN_FILE).exists()
+}
+
 /// Mmap-backed immutable full-text inverted index.
 ///
 /// On-disk state (`postings.dat`, `vocab.dat`, `point_to_tokens_count.dat`,
@@ -145,8 +155,17 @@ impl OnDiskInvertedIndex<MmapFile> {
 
         // No segment total is written: deletions are applied on open, so it can
         // only be summed after masking.
-        if let Some(lens) = point_to_doc_len {
-            MmapSlice::create(&point_to_doc_len_path, lens.iter().copied())?;
+        match point_to_doc_len {
+            Some(lens) => MmapSlice::create(&point_to_doc_len_path, lens.iter().copied())?,
+            // Every other file here is rewritten in place, so this is the only
+            // one that could survive a rebuild. `open` would then read a
+            // previous build's lengths as this build's, at offsets that now
+            // belong to different documents. `save_deleted_mask` unlinks its
+            // own stale file for the same reason.
+            None => match fs_err::remove_file(&point_to_doc_len_path) {
+                Err(err) if err.kind() == std::io::ErrorKind::NotFound => (),
+                result => result?,
+            },
         }
 
         Ok(())
@@ -284,12 +303,27 @@ impl<S: UniversalRead> OnDiskInvertedIndex<S> {
         // offsets).
         let total_count = point_to_tokens_count.len()? as usize;
 
-        // A sidecar shorter than the counts can only come from a partially
-        // copied file set. Treated as absent: padding it would give every point
-        // past the truncation a zero that reads like a real length.
+        // A sidecar that does not cover exactly the index's points can only
+        // come from a partially copied or stale file set, and is not trusted to
+        // locate lengths: a short one would have to be padded with zeroes that
+        // read like real lengths, and a long one would be silently truncated
+        // when it is materialized. Treated as absent, with a warning, the way
+        // `SortedBlockIndex::open` treats a stale block index.
         let point_to_doc_len = match point_to_doc_len {
-            Some(storage) if (storage.len()? as usize) < total_count => None,
-            other => other,
+            Some(storage) => {
+                let sidecar_count = storage.len()? as usize;
+                if sidecar_count == total_count {
+                    Some(storage)
+                } else {
+                    log::warn!(
+                        "Ignoring document length sidecar {path}: it covers {sidecar_count} \
+                         points while the index has {total_count}",
+                        path = point_to_doc_len_path.display(),
+                    );
+                    None
+                }
+            }
+            None => None,
         };
 
         let mut deleted = deleted_points.to_owned();
@@ -679,7 +713,11 @@ impl<S: UniversalRead> OnDiskInvertedIndex<S> {
             self.path.join(POINT_TO_TOKENS_COUNT_FILE),
             deleted_mask_file(&self.path, self.compact_deleted_mask, DELETED_POINTS_FILE),
         ];
-        // Listed only when it exists: this list feeds the snapshot file set.
+        // Listed only when the index loaded it, which is not the same as the
+        // file existing: one rejected at `open` is deliberately left out of the
+        // snapshot file set, since restoring it would only get it rejected
+        // again. `wipe` removes the directory rather than this list, so the
+        // rejected file does not outlive the index.
         if self.storage.point_to_doc_len.is_some() {
             files.push(self.path.join(POINT_TO_DOC_LEN_FILE));
         }

lib/segment/src/index/field_index/full_text_index/lifecycle.rs

@@ -11,6 +11,7 @@ use serde_json::Value;
 
 use super::immutable_text_index::ImmutableFullTextIndex;
 use super::inverted_index::ARRAY_BOUNDARY_SENTINEL;
+use super::inverted_index::on_disk_inverted_index::has_doc_len_sidecar;
 use super::mutable_text_index::MutableFullTextIndex;
 use super::on_disk_text_index::{FullTextMmapIndexBuilder, OnDiskFullTextIndex};
 use super::tokenizers::Tokenizer;
@@ -36,6 +37,18 @@ impl FullTextIndex {
 
         let populate = Populate::from(memory.populate_on_open());
         let scoring = config.scoring();
+
+        // Checked before the open, not after: opening populates the whole file
+        // set, and on the first start after scoring is enabled every existing
+        // segment would fault in its postings only to be discarded here.
+        if scoring && !has_doc_len_sidecar(&path) {
+            log::info!(
+                "Text index at {path} records no document lengths, rebuilding it from payload",
+                path = path.display(),
+            );
+            return Ok(None);
+        }
+
         let Some(on_disk_index) =
             OnDiskFullTextIndex::open(&MmapFs, path, config, populate, deleted_points)?
         else {
@@ -46,7 +59,11 @@ impl FullTextIndex {
         // index absent and let the caller rebuild it from payload. The decision
         // belongs here rather than in `OnDiskInvertedIndex::open`: the read-only
         // stack never builds, and would drop the field instead.
+        //
+        // Reachable past the probe above when the sidecar exists but `open`
+        // rejected it, so the log says which of the two happened.
         if scoring && !on_disk_index.records_doc_len() {
+            log::info!("Text index rejected its document length sidecar, rebuilding from payload");
             return Ok(None);
         }
 

lib/segment/src/index/field_index/full_text_index/on_disk_text_index/lifecycle.rs

@@ -144,8 +144,10 @@ impl ValueIndexer for FullTextMmapIndexBuilder {
         }
 
         // Through the shared helper: `document_length` subtracts the sentinels
-        // it inserts, so the two rules have to agree.
-        let phrase_matching = self.mutable_index.point_to_doc.is_some();
+        // it inserts, so the two rules have to agree. Read from the config like
+        // the other two callers, rather than from whether a container happens
+        // to be allocated, so that the two halves cannot drift apart.
+        let phrase_matching = self.config.phrase_matching.unwrap_or_default();
         let str_tokens =
             FullTextIndex::tokenize_document(&self.tokenizer, phrase_matching, &values);