fix: avoid Greek stemmer UTF-8 panic

[lance-gatekeeper]

Summary

• use the corrected Snowball 3 backend only for newly built Greek indexes with stemming enabled
• preserve versionless Greek metadata as the legacy rust-stemmers vocabulary for load, query, incremental update, segmented append, and merge
• normalize field-absent and explicit legacy physical index parameters to one internal representation while retaining historical absent protobuf details
• advertise inverted-index capability version 4 for Snowball Greek segments so released readers reject them instead of using a mismatched analyzer
• omit the Greek discriminator for non-Greek analyzers and disabled stemming, including MemWAL-maintained segments

Root cause

The rust-stemmers 1.2.0 Greek algorithm can retain stale UTF-8 byte offsets after shortening a word, then panic while slicing the shortened string. Replacing it changes some persisted terms, so versionless indexes must keep legacy analyzer semantics. The first metadata follow-up also wrote the discriminator for semantically unchanged English indexes and left Snowball-written Greek segments on capabilities that released readers accept. Later normalization covered manifest details but not field-absent physical metadata.lance parameters, causing appended legacy segments to compare unequal during merge.

Validation

• cargo test -p lance-tokenizer
• cargo test -p lance-index greek_stemmer -- --nocapture
• cargo test -p lance-index capability_gate -- --nocapture
• cargo test -p lance canonicalize_inverted_details_accepts_legacy_empty_details -- --nocapture
• cargo test -p lance test_append_legacy_greek_fts_preserves_canonical_details -- --nocapture
• cargo test -p lance fts_from_metadata_preserves_format_version -- --nocapture
• cargo test -p lance test_mem_wal_maintained_fts_flush_preserves_version -- --nocapture
• cargo fmt --all -- --check
• cargo clippy --all --tests --benches -- -D warnings
• cargo check --locked --manifest-path python/Cargo.toml
• cargo check --locked --manifest-path java/lance-jni/Cargo.toml
• git diff --check

Fixes #5235

[lance-gatekeeper]

❌ Gate recommendation: request changes.

The panic and root cause are verified, and a Greek-only corrected backend is the right scope. The replacement still needs to preserve analyzer semantics for already-persisted FTS terms. Use a compatibility-preserving runtime fix or version the stemmer semantics in index metadata, then cover opening, querying, and incrementally updating a legacy Greek index.

[lance-gatekeeper]

Switching all Greek stemming to this backend also changes the analyzer used when existing FTS indexes are opened. InvertedIndexDetails persists language and stem, not a backend/version, and InvertedIndex::load rebuilds the tokenizer from those params while retaining the old token dictionary.

I ran this comparison against the two locked dependencies on this head:

Reproducer

fn main() {
    let legacy = rust_stemmers::Stemmer::create(rust_stemmers::Algorithm::Greek);
    let replacement = pagefind_stem::Stemmer::create(pagefind_stem::Algorithm::Greek);
    assert_eq!(legacy.stem("ίσα"), replacement.stem("ίσα"));
}

cargo run --quiet --offline --bin compat fails with left: "" and right: "ισ".

Thus a legacy index containing ίσα stores the empty stem, but the upgraded query uses ισ; existing rows silently stop matching, and incremental updates can mix vocabularies. Preserve legacy output with a minimal offset/runtime correction, or persist a backend version with a safe legacy path, and add upgrade/query/update coverage.

[lance-gatekeeper]

Addressed in 889db02. Persisted stemmer semantics keep legacy Greek indexes on the legacy analyzer for load, query, and incremental update, while new Greek indexes use the corrected backend; regressions cover both paths.

[github-actions]

Important

This PR touches the Lance format specification.

Substantive changes to the format specification — the .proto definitions
and the spec docs under docs/src/format/ — require a PMC vote before merge.
Minor edits such as typo fixes, wording, or formatting are excluded; use your
judgment.

If this is a meaningful format change:

• Start a vote following the Lance community voting process.
  Format specification modifications need 3 binding +1 votes (excluding the
  proposer), held on GitHub Discussions, with a minimum voting period of 1 week.
• Once the vote passes, link the completed vote in this PR. It should not be
  merged until the vote is linked.

[lance-gatekeeper]

❌ Gate recommendation: request changes.

The metadata addition preserves legacy-index behavior for current readers, but it does not make Snowball-written indexes safe for released readers and it also marks semantically unchanged non-Greek analyzers as different. Version the changed Greek vocabulary semantics at an index capability boundary that old readers reject (or preserve legacy terms with a corrected implementation), and persist the stemmer discriminator only when it affects Greek stemming.

[lance-gatekeeper]

The new field does not protect new indexes from released readers. Those readers ignore unknown protobuf tag 14 (and persisted JSON params deliberately ignore unknown fields), while the writer still emits FTS index versions 1–3. They therefore accept a Snowball-written segment, rebuild the legacy analyzer, and silently miss terms; an old updater can also append a second vocabulary. Preserve legacy term output with a panic-free implementation, or put Snowball semantics behind a new index version/capability so old engines skip the segment and scan.

Reproducer

I ran the tokenizer at base 336bee5cbb8e0265f992d0ff38845545f5944a32 as the released-reader dependency and this head as the writer dependency:

CARGO_TARGET_DIR=/home/agent/tmp/gate8183-forward-target cargo run --manifest-path /home/agent/tmp/gate8183-forward-compare/Cargo.toml --locked

The comparison builds each RawTokenizer with Stemmer::new(Language::Greek), tokenizes ίσα, and asserts that the stored writer term equals the reader query term. Expected equal terms; observed left: "ισ", right: "".

[lance-gatekeeper]

Addressed in b21bcae. Snowball Greek segments now require inverted-index capability version 4, so released readers reject rather than mis-analyze them.

[lance-gatekeeper]

Snowball3 is assigned for every new index, including English indexes and configurations with stemming disabled, although it changes behavior only for active Greek stemming. This makes semantically identical legacy/current details compare unequal, so load_segment_details rejects mixed segments as inconsistent. Normalize or omit this discriminator unless Greek stemming is active, while retaining the compatibility boundary for that case.

Reproducer

CARGO_TARGET_DIR=/home/agent/tmp/gate8183-head-target cargo test -p lance --locked canonicalize_inverted_details_accepts_legacy_empty_details -- --nocapture

Expected the existing canonicalization invariant to pass. On this head it fails with legacy greek_stemmer: None versus current greek_stemmer: Some(Snowball3).

[lance-gatekeeper]

Addressed in b21bcae. The Greek stemmer discriminator is now emitted only for active Greek stemming; non-Greek and stem-disabled details retain the historical representation.

[lance-gatekeeper]

❌ Gate recommendation: request changes.

The capability gate now protects Snowball-written segments and the discriminator is limited to active Greek stemming. Legacy Greek retraining still changes canonical metadata from an absent discriminator to explicit Legacy, so append or rebuild can create semantically identical segments that readers reject as inconsistent.

Keep legacy as an internal training hint while canonical/protobuf details normalize it to the historical absent representation, and cover a legacy Greek append or rebuild followed by an FTS query.

[lance-gatekeeper]

This training round trip turns historical Greek params with no discriminator into Some(Legacy) when the derived params are serialized to protobuf. Existing segments canonicalize to None, so an append or rebuild driven by derive_index_params() produces details that load_segment_details() treats as inconsistent; subsequent FTS queries fail before execution. Keep "legacy" as the internal training hint needed to preserve analyzer selection, but serialize/canonicalize Legacy back to the historical absent protobuf representation (or normalize both before segment comparison), then add append/rebuild/query coverage.

Reproducer

I ran this assertion against the current head in a small binary:

let current = InvertedIndexParams::new("raw".to_string(), Language::Greek);

let mut absent_json = serde_json::to_value(&current).unwrap();
absent_json.as_object_mut().unwrap().remove("greek_stemmer");
let absent: InvertedIndexParams =
    serde_json::from_value(absent_json.clone()).unwrap();
let absent_details = InvertedIndexDetails::try_from(&absent).unwrap();

absent_json.as_object_mut().unwrap().insert(
    "greek_stemmer".to_string(),
    serde_json::Value::String("legacy".to_string()),
);
let explicit: InvertedIndexParams =
    serde_json::from_value(absent_json).unwrap();
let explicit_details = InvertedIndexDetails::try_from(&explicit).unwrap();

assert_eq!(absent_details, explicit_details);

CARGO_TARGET_DIR=/home/agent/tmp/gate8183-head-target cargo run --manifest-path /home/agent/tmp/gate8183-legacy-canonical/Cargo.toml

Expected equality because both representations select the legacy analyzer. Observed failure: left: greek_stemmer: None; right: greek_stemmer: Some(Legacy).

[lance-gatekeeper]

Addressed in fadb9d7. Explicit Legacy remains an internal training hint but canonical protobuf details omit it, and append/query coverage verifies compatibility with absent historical metadata.

[lance-gatekeeper]

❌ Gate recommendation: request changes.

The manifest normalization fixes legacy append/query compatibility, but historical and appended segments still retain different physical parameter representations, so a later segment merge rejects them.

Normalize absent and explicit Legacy in the persisted index-file parameter load/comparison path as well, and make the regression cover append, query, and merge from genuinely field-absent physical metadata.

[lance-gatekeeper]

This changes only manifest InvertedIndexDetails, so the test does not model historical physical params. CreateIndexBuilder has already passed legacy_params through to_training_json, which makes the initial metadata.lance store explicit "greek_stemmer":"legacy". A released segment stores no key; after append, that yields physical params None and Some(Legacy). Queries now work because manifest details are canonicalized, but OptimizeOptions::merge(2) rejects the strict mismatch. Normalize the physical parameter representation when loading or comparing segments, and make this regression merge a truly field-absent base segment.

Reproducer

I created a one-row raw Greek FTS index with base commit 336bee5cbb8e0265f992d0ff38845545f5944a32 and verified its files contain no greek_stemmer key. Then I opened it with this head, appended a row, ran append optimization, asserted that two segments existed, and required their merge to succeed:

CARGO_TARGET_DIR=/home/agent/tmp/gate8183-head-target cargo run --manifest-path /home/agent/tmp/gate8183-base-writer/Cargo.toml
CARGO_TARGET_DIR=/home/agent/tmp/gate8183-head-target cargo run --manifest-path /home/agent/tmp/gate8183-head-merge/Cargo.toml

The current-head step ends with:

dataset
    .optimize_indices(&OptimizeOptions::append())
    .await
    .unwrap();
assert_eq!(dataset.load_indices_by_name("body_idx").await.unwrap().len(), 2);
dataset
    .optimize_indices(&OptimizeOptions::merge(2))
    .await
    .unwrap();

Expected the semantically identical legacy segments to merge. Observed cannot merge inverted index segments with different parameters from InvertedIndex::merge_segments.

[lance-gatekeeper]

Addressed in 7e93666. Loaded field-absent physical params normalize to legacy semantics, and the regression now rewrites genuinely absent metadata then verifies append, query, merge, and post-merge query.

[lance-gatekeeper]

✅ Gate recommendation: approve.

The versioned analyzer contract now preserves legacy Greek query and update behavior, gates Snowball 3 terms behind capability version 4, and normalizes field-absent physical parameters through append and merge. The lifecycle regression covers the previously failing merge path.

[Xuanwo]

It seems this should be fixed in rust-stemmers or snowball instead. Is there already an issue report filed upstream?

[lance-gatekeeper]

No code change was needed for this feedback. snowballstem/snowball#204 reports this same Greek faulty-slice failure and was fixed upstream in January 2025; it also documents the corrected ισα stem. CurrySoftware/rust-stemmers#27 tracks regenerating rust-stemmers from current Snowball. The Lance-side version boundary remains necessary because the corrected output changes persisted FTS terms and released readers must not mix analyzer vocabularies.

[lance-gatekeeper]

🟡 Gate recommendation: maintainer decision required.

Both directions can be correct, but they choose different durable contracts:

• adopting Snowball 3 now corrects known Greek stemming semantics and therefore requires the v4 reader boundary and legacy backend;
• porting Snowball's targeted crash fix through rust-stemmers keeps this bugfix vocabulary-preserving and avoids a format change.

Choose whether this PR is intended to adopt new Greek vocabulary semantics or only eliminate the panic.

[lance-gatekeeper]

This dependency is the point where a panic-only fix becomes adoption of a new persisted vocabulary. Snowball #204 contains both a targeted crash fix and a later correction for ισα → ισ; the latter explains why Snowball 3 requires this PR's v4 and legacy machinery. If the intended scope is only the panic, port and pin the targeted upstream fix through the existing rust-stemmers dependency and differential-test successful 1.2.0 Greek outputs. If adopting Snowball 3 is intended, the current version boundary is the right compatibility mechanism, but that product and format choice needs an explicit maintainer decision.