fix(fts): do not count empty documents in num_docs by Ar-maan05 · Pull Request #7745 · lance-format/lance

Ar-maan05 · 2026-07-12T15:08:52Z

Follow-up to #7656. Empty and whitespace-only string documents tokenize to nothing but were still appended to the inverted index, inflating num_docs and skewing BM25 document-count statistics. #7656 introduced skip-empty behavior for list-string documents via a skip_empty_document flag; the plain-string path still passed false.

Since both call sites should now skip zero-token documents, this removes the flag and makes IndexWorker::process_document unconditionally skip when a document produces no tokens, unifying string and list behavior.

Verified locally: test_create_fts_index_with_empty_strings (strengthened to assert num_docs), cargo fmt, and cargo clippy --features protoc --tests -- -D warnings all pass.

Summary by CodeRabbit

Bug Fixes
- Empty and whitespace-only text is no longer counted as searchable content.
- Full-text search results now exclude documents that produce no searchable tokens.
- Index statistics accurately reflect only documents containing searchable text.
Tests
- Added coverage for mixed datasets containing searchable, empty, and whitespace-only values.

coderabbitai · 2026-07-12T15:09:11Z

📝 Walkthrough

Walkthrough

Changes

FTS Empty Document Handling

Layer / File(s)	Summary
Unconditional zero-token document skipping `rust/lance-index/src/scalar/inverted/builder.rs`	`IndexWorker::process_document` no longer accepts a skip flag and excludes all documents that produce zero tokens, for both scalar strings and string lists.
Empty document indexing coverage `rust/lance/src/dataset/tests/dataset_index.rs`	Regression coverage verifies that empty and whitespace-only rows are excluded from `num_docs` and search results.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and accurately summarizes the main FTS indexing fix for empty documents.
Linked Issues check	✅ Passed	The changes match the issue by skipping zero-token string documents, keeping them out of num_docs, and adding regression coverage.
Out of Scope Changes check	✅ Passed	The PR stays focused on FTS empty-document counting and its regression test, with no clear unrelated code changes.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@rust/lance/src/dataset/tests/dataset_index.rs`:
- Around line 408-418: Replace the manual schema, RecordBatch, and StringArray
construction in the modified test fixture with the repository’s record_batch!()
helper, preserving the existing string values and resulting batch data.
- Around line 405-416: Add a dedicated regression case alongside the existing
mixed-token fixture that builds an index exclusively from empty and
whitespace-only strings, then assert the resulting BM25/statistics metadata
reports num_docs == 0. Keep the existing mixed-document coverage unchanged and
target the dataset index test’s index-building and statistics assertions.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 20035755-89a9-431d-8657-c3fd83cd10b7

📥 Commits

Reviewing files that changed from the base of the PR and between f698426 and 12fd887.

📒 Files selected for processing (2)

rust/lance-index/src/scalar/inverted/builder.rs
rust/lance/src/dataset/tests/dataset_index.rs

coderabbitai · 2026-07-12T15:13:03Z

+    // Mix documents that produce tokens with ones that tokenize to nothing
+    // (empty and whitespace-only strings). The latter carry no searchable
+    // content and must not be counted in `num_docs`.
    let batches: Vec<RecordBatch> = vec![
        RecordBatch::try_new(
            schema.clone(),
-            vec![Arc::new(StringArray::from(vec!["", "", ""]))],
+            vec![Arc::new(StringArray::from(vec![
+                "lance",
+                "",
+                "lance database",
+                "   ",
+            ]))],


📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash set -euo pipefail # Map the relevant test file and inspect surrounding tests for index coverage. git ls-files rust/lance/src/dataset/tests/dataset_index.rs wc -l rust/lance/src/dataset/tests/dataset_index.rs sed -n '1,520p' rust/lance/src/dataset/tests/dataset_index.rs | cat -n | sed -n '1,520p' # Search for all token/index-related assertions in this file. rg -n "num_docs|empty|whitespace|token|search|index" rust/lance/src/dataset/tests/dataset_index.rs

Repository: lance-format/lance

Length of output: 43487

🏁 Script executed:

#!/bin/bash set -euo pipefail # Check whether any other test asserts FTS statistics for zero-token / empty-index behavior. rg -n "num_docs|empty index|empty_table|empty strings|tokenize to nothing|no search hits|full_text_search" rust/lance/src/dataset/tests/dataset_index.rs # Inspect the nearby FTS tests around the empty-index cases. sed -n '1210,1295p' rust/lance/src/dataset/tests/dataset_index.rs | cat -n sed -n '244,280p' rust/lance/src/dataset/tests/dataset_index.rs | cat -n

Repository: lance-format/lance

Length of output: 9394

Add an all-empty regression case.
The empty-table search path is already covered, but this fixture still doesn’t assert the num_docs == 0 BM25/statistics path. Keep a dedicated case for an index built only from empty/whitespace strings if that behavior is part of the regression target.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/lance/src/dataset/tests/dataset_index.rs` around lines 405 - 416, Add a dedicated regression case alongside the existing mixed-token fixture that builds an index exclusively from empty and whitespace-only strings, then assert the resulting BM25/statistics metadata reports num_docs == 0. Keep the existing mixed-document coverage unchanged and target the dataset index test’s index-building and statistics assertions.

coderabbitai · 2026-07-12T15:13:03Z

    let batches: Vec<RecordBatch> = vec![
        RecordBatch::try_new(
            schema.clone(),
-            vec![Arc::new(StringArray::from(vec!["", "", ""]))],
+            vec![Arc::new(StringArray::from(vec![
+                "lance",
+                "",
+                "lance database",
+                "   ",
+            ]))],
        )
        .unwrap(),


📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use record_batch!() for the modified fixture.

The changed test still manually constructs the schema, RecordBatch, and StringArray; repository guidance requires record_batch!() instead of this boilerplate.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/lance/src/dataset/tests/dataset_index.rs` around lines 408 - 418, Replace the manual schema, RecordBatch, and StringArray construction in the modified test fixture with the repository’s record_batch!() helper, preserving the existing string values and resulting batch data.

Source: Coding guidelines

codecov · 2026-07-12T15:53:30Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

fix(fts): do not count empty documents in num_docs

12fd887

github-actions Bot added A-index Vector index, linalg, tokenizer bug Something isn't working labels Jul 12, 2026

coderabbitai Bot reviewed Jul 12, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(fts): do not count empty documents in num_docs#7745

fix(fts): do not count empty documents in num_docs#7745
Ar-maan05 wants to merge 1 commit into
lance-format:mainfrom
Ar-maan05:fix/fts-skip-empty-string-docs-7660

Ar-maan05 commented Jul 12, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 12, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Jul 12, 2026

Uh oh!

coderabbitai Bot Jul 12, 2026

Uh oh!

codecov Bot commented Jul 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Ar-maan05 commented Jul 12, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Jul 12, 2026

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Ar-maan05 commented Jul 12, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 12, 2026 •

edited

Loading