test(lance): guard an untrained full-text segment and stale delta-segment entries

[ragnorc]

Two Lance surface guards for facts that decide how full-text and scalar indexes may be maintained. Tests and developer docs only; no engine behaviour changes.

fts_untrained_segment_applies_its_analyzer_to_rows_it_does_not_cover. An inverted index with no trained rows, built over an empty table or with train(false) over a populated one, has an empty fragment bitmap. Lance still matches every row it does not cover with that index's tokenizer, and the rows and BM25 scores equal a full build's bit for bit. With no segment at all, Lance's flat path uses a bare SimpleTokenizer, so "deep" misses "Deep Learning". This is the fact a fix for #904 would use: declare a full-text index's analyzer in the publication that creates or rewrites the table, and build postings later. It also bears on #840.

index_delta_segment_serves_stale_entries_of_an_updated_row. Lance masks a logical index's results by the union of its segments' fragment coverage. On a stable-row-ID table, an update keeps the row ID and moves the row to a new fragment. Once a delta segment covers that fragment, the older segment's entry passes the mask:

• full-text search returns the row for a term it no longer holds, and twice for a term both versions hold;
• a BTREE equality returns the row for its old value.

The guard reproduces this with Lance's own optimize_indices(OptimizeOptions::append()). Lance fixed the same defect for vector search only (lance#7371, lance#8351), and it reproduces on pylance 11.0.0, 12.0.0 and 13.0.0; reported upstream as lance-format/lance#9817. This is why OmniGraph never adds a delta segment: stage_index_fold rebuilds a lagging index whole, and the full-text rebuild replaces every segment. The guard goes red at the Lance release that applies segment ownership to these paths. At that point an incremental full-text fold becomes safe after updates. Until then, a delta is exact only when every row it covers was inserted after the newest existing segment was built.

docs/dev/lance.md gains an "Index segments" row in the compatibility fences, and docs/dev/testing.md lists segment ownership among the pinned facts.

Validation: cargo test -p omnigraph-engine --features failpoints --test lance_surface_guards (the full target: 63 passed, 2 ignored as before), cargo clippy -p omnigraph-engine --features failpoints --test lance_surface_guards --locked -- -D warnings -W clippy::dbg_macro, cargo fmt --all --check, python3 scripts/check-docs.py, bash scripts/check-agents-md.sh, typos.

[ragnorc]

Recommendation: approve this test and documentation change, once the required CI gates pass. I found no blocking defect in the PR diff. Reviewed exact head 76d0229d7105b0f395887ed53d5ad590136ecf25 against parent 207e44486334564efb3d3e327809ed2b4ea1285f. This is a COMMENT review because the authenticated account is the PR author.

This PR records two Lance behaviors that affect safe index maintenance. An untrained full-text segment retains its analyzer, the rules that turn text into searchable terms. A delta segment can expose an updated row's stale entries from an older segment. The PR adds two small guards to the existing Lance test owner and documents these facts. It changes no production behavior.

The guards distinguish the mechanisms that matter:

• The analyzer guard compares a full build with both an index created on an empty table and a populated table indexed with train(false). It checks rows and exact score bits for three queries. Its no-segment control distinguishes the bare tokenizer from the stored analyzer.
• The ownership guard changes one row while retaining its stable row ID. Before the delta, it checks correct FTS and BTREE results. After the delta, it checks the stale term, duplicate result, and stale scalar value. It also requires two segments per index. The result helper preserves duplicates.

The relevant invariant is that physical index coverage must not change logical results. A union of segment coverage cannot establish which segment owns a row's current value. These tests expose that general cause. They do not add a special-case filter or claim to fix it.

I traced the whole-index fold and index replacement. They remove the old segments in the staged CreateIndex operation. This preserves one replacement owner and the existing detached publication path. The tradeoff is rebuilding more data instead of adding a cheap delta after updates.

Liability decreases through explicit, executable dependency assumptions. The PR adds private test helpers and 267 test lines, but no runtime API, stored state, or new execution path. Five similar compatibility changes can remain in this owner. Their maintenance cost is reviewing a red guard during a Lance upgrade, rather than preserving an undocumented restriction indefinitely.

I read the repository instructions, invariants, testing, systems, and Lance guides. I also read the complete relevant upstream index, BTREE, FTS, tokenizer, FTS quickstart, distributed-indexing, read/write, and row-lineage pages. Current upstream documentation can describe newer capabilities. I checked the material claims against the pinned Lance 11.0.0 implementation at ab6b5bbe46009ed78746b444df8db59a8bc5d842. The fetched FTS source matches the local pin byte for byte. Its shared FTS prefilter, tokenizer selection, scalar segment fanout, and fragment-union mask support the stated cause. Lance #9817 records the upstream defect.

Local validation:

• Two independent probes using pylance==11.0.0, PyArrow, stable row IDs, and V2.2 local files passed. Command: /tmp/review-905-pylance/bin/python -u /tmp/review-905-probe.py. The three analyzer queries matched the full build's rows and score bits. Bare search for “deep” returned only e2. Before adding the delta, the three ownership results were [a2], [a1], [a2]. Afterward they were [a1,a2], [a1,a1], [a1,a2].
• scripts/check-docs.py passed for 225 files. bash scripts/check-agents-md.sh and git diff --check 207e44486334564efb3d3e327809ed2b4ea1285f HEAD passed.
• The requested Rust command was cargo +1.97.1 test -p omnigraph-engine --locked --features failpoints --test lance_surface_guards segment -- --nocapture. It waited for another active build's lock. I cancelled only my queued attempt. No Rust test ran locally. The Python probes are independent substrate evidence, not execution of these Rust tests.
• The isolated checkout is clean. No temporary source or test edits were made.

Exact-head GQT CI failed in the existing hydrate_return_columns_above_the_limit.gqt case: consumed node:Doc._rowaddr has no declared field. The workspace job failed in the existing hydrating-top-k report test before reaching this integration target. These files and production code are unchanged by this PR. This is not a green qualification run. Exact-head Clippy, format, cloud integration checks, and the separate pinned DST suite passed.

There is no implemented bug fix here, so no before-fail/after-pass fix proof is claimed. GQT cannot create untrained Lance segments or invoke append-mode index maintenance directly. The existing Rust substrate owner is appropriate. A later engine fix must add minimal GQT evidence for its query-visible contract and retain mechanism tests for publication and segment ownership.

A red ownership guard triggers review of the changed substrate behavior. It alone does not qualify every scalar index type, segment merge, analyzer setting, or maintenance path. This review makes no performance, live-cloud, or general score-equivalence claim beyond the tested fixture.

[ragnorc]

Recommendation: approve this test and documentation change. No new blocking finding. This is a COMMENT review because the authenticated account is the author.

This follow-up reviews exact head 068fa7deede564c640242961ba2e8aafdba2b6ac against the previously reviewed 76d0229d7105b0f395887ed53d5ad590136ecf25. The follow-up merges main. The two new Lance guards, their compatibility documentation, and the pinned Lance version are unchanged. The relevant whole-index fold and replacement behavior is also unchanged. This is an integration follow-up, not a separate review of every change already merged into main.

The PR records why index maintenance needs two safeguards. An untrained full-text segment carries its analyzer to uncovered data. Adding a delta after an update can expose stale entries from an older segment, because combined fragment coverage does not establish which segment owns the current value. The tests distinguish that cause from a tokenizer issue and preserve duplicate results in their assertions. They change no production behavior and do not claim to fix the upstream defect.

The contract remains that derived indexes must not change logical query results. OmniGraph's existing whole-index replacement avoids the unsafe delta path by design. The tradeoff is more rebuild work. These guards reduce liability by making the dependency assumptions executable; they add private test helpers and upgrade-review work, with no new runtime state or API. Five similar changes can stay in the same substrate test owner. A failing guard at an upgrade calls for a design review, not an automatic removal of every maintenance restriction.

Supporting code: analyzer guard, segment-ownership guard, whole-index fold. The prior review's full upstream reading and pinned Lance 11.0.0 implementation analysis remain applicable: the relevant source and dependency assumptions did not change in this merge.

Validation:

• Exact-commit workspace CI now passes. I checked the run's head_sha and downloaded its log. Both new tests executed and passed; lance_surface_guards reports 63 passed, 0 failed, 2 ignored. This closes the earlier limitation where workspace tests stopped before reaching this target.
• Exact-head ordinary GQT, DST GQT, pinned DST, format and Clippy checks pass. The earlier GQT hydration failure is no longer a red gate on this head.
• Local checks: git diff --check, bash scripts/check-agents-md.sh, and python3 scripts/check-docs.py passed (228 Markdown files). The isolated exact-head checkout is clean. No temporary source changes were needed.
• No local Rust execution is claimed for this follow-up. The Rust results above are exact-commit CI evidence. The prior independent pinned-substrate probes remain supporting evidence, not a substitute for the Rust tests.

There is no implemented bug fix in this PR, so no before-fail/after-pass fix claim is appropriate. GQT cannot create an untrained native Lance segment or invoke append-mode index optimization. These minimal mechanism guards belong in the existing Rust owner. A later query-visible engine fix still needs a minimal GQT regression that fails behaviorally before the fix and passes afterward.

Limitations: the fixtures do not qualify every analyzer, scalar index type, or segment topology. No performance improvement or live-cloud qualification is claimed. No optional changes requested.