fix(mem-wal): reject mismatched shard specs before epoch claim

[u70b3]

Summary

• reject an incoming shard_spec_id that differs from the immutable identity already stored in a shard manifest, before advancing manifest version or writer epoch
• revalidate sealed status and shard identity after write conflicts, preserving the distinguishable sealed-shard error
• keep rejection side-effect free and include the shard id plus incoming and stored spec ids in the error
• cover matching automatic identity, both current 0/1 mismatch directions, sealed manifests, and concurrent first claims

Scope

This is an independent defensive invariant over the identities Lance currently emits: 0 identifies manually managed shards and 1 is the sole automatic sharding spec. The check does not require monotonic spec allocation and does not define revisions or activation.

#8112 is complementary, not a prerequisite. It resolves a dataset writer's default 0 to the current automatic spec before a fresh manifest is created; this PR prevents any caller from changing an identity once the manifest exists. Either change is independently valid and reviewable.

shard_spec_id=0 is an identity, not a wildcard.

Fixes #7945.

Testing

• cargo fmt --all -- --check
• cargo test -p lance dataset::mem_wal::manifest::tests --lib -- --nocapture (14 passed)
• cargo clippy --all --tests --benches -- -D warnings

Behavior change / rollout

Previously, a writer opening an existing shard with a mismatched shard_spec_id (e.g. the default 0 opening an auto-sharded shard stored as 1) was silently tolerated past claim_epoch. It now fails fast with InvalidInput naming the shard id and both spec ids — this is the fix intent of #7945. Mixed manual(0)/auto(1) deployments will see the new hard error; resolving the creation-side default-0 identity is tracked in #8112.

[coderabbitai]

📝 Walkthrough

Walkthrough

claim_epoch now centralizes sealed-manifest and shard-spec mismatch validation, applies it before and after write conflicts, and adds single-threaded and concurrent tests for matching and mismatched shard specifications.

Changes

Shard claim validation

Layer / File(s)	Summary
Centralized claim validation and conflict handling rust/lance/src/dataset/mem_wal/manifest.rs	validate_claimable rejects sealed manifests and shard-spec mismatches. claim_epoch applies it before epoch calculation and after failed writes.
Claim behavior tests rust/lance/src/dataset/mem_wal/manifest.rs	Tests verify matching claims, mismatched specifications, concurrent claim consistency, and sealed-manifest refusal.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• lance-format/lance#7896: Updates the same claim_epoch fencing and epoch-claim validation logic.

Suggested reviewers: hamersaw

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: rejecting mismatched shard specs before epoch claim.
Description check	✅ Passed	The description matches the code changes and issue scope.
Linked Issues check	✅ Passed	The changes satisfy #7945 by rejecting spec mismatches, revalidating after conflicts, and preserving successful claims.
Out of Scope Changes check	✅ Passed	No clear out-of-scope changes are indicated beyond the manifest validation fix and related tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (1)

rust/lance/src/dataset/mem_wal/manifest.rs-445-447 (1)

445-447: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Document sealed-manifest refusal in # Errors.

claim_epoch now returns an error for sealed manifests, but this public API’s error contract only documents epoch conflicts, shard-spec conflicts, and contention. Document the sealed refusal and its no-claim effect. As per coding guidelines, “Ensure doc comments match actual semantics; distinguish mutates-in-place (&mut self) from returns-new-value behavior.”

🤖 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/mem_wal/manifest.rs` around lines 445 - 447, Update
the public `claim_epoch` documentation comment’s `# Errors` section to include
refusal for sealed manifests and state that this path makes no claim or
mutation. Keep the existing documentation for epoch conflicts, shard-spec
conflicts, and contention, and accurately describe the method’s in-place `&mut
self` behavior.

Source: Coding guidelines

🤖 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.

Other comments:
In `@rust/lance/src/dataset/mem_wal/manifest.rs`:
- Around line 445-447: Update the public `claim_epoch` documentation comment’s
`# Errors` section to include refusal for sealed manifests and state that this
path makes no claim or mutation. Keep the existing documentation for epoch
conflicts, shard-spec conflicts, and contention, and accurately describe the
method’s in-place `&mut self` behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: QUIET

Plan: Pro Plus

Run ID: eddae87c-eea7-4d35-949b-1203ad196d84

📥 Commits

Reviewing files that changed from the base of the PR and between 3a72f8a and 5cd2b6a.

📒 Files selected for processing (1)

• rust/lance/src/dataset/mem_wal/manifest.rs

[codecov]

Codecov Report

❌ Patch coverage is 94.54545% with 3 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
rust/lance/src/dataset/mem_wal/manifest.rs	94.54%	1 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

[hamersaw]

I think this change has the right intent, but will not fix underlying issues correctly. It's meant to disallow claiming a table with a different shard_spec_id than the base table. However, Lance currently only uses 0 and 1 as valid values to not manual and automatic sharding configuration. IMO this is part of a larger discussion, we should update the shard spec configuration so that IDs are monotonically increasing. That would make this change valid.

[u70b3]

Thanks, I think I understand the concern. The manifest-side equality check is only meaningful if every distinct sharding-spec revision has a unique ID. Today the public initialization path assigns 1 to every automatic sharding configuration, so two different automatic specs remain indistinguishable (0 remains the manual-shard sentinel).

It sounds like the prerequisites are:

1. Allocate sharding spec IDs monotonically and never reuse them.
2. Ensure a writer claims with the active spec ID from the base-table metadata.
3. Keep the claim-time validation so a stale or mismatched spec cannot advance the epoch and fence the incumbent writer.

Would you prefer that I expand this PR to implement the spec-ID lifecycle, or split the monotonic allocation/base-table validation into a prerequisite PR and rebase this change on it?

[u70b3]

Following up on my previous comment: I split the prerequisite work into #8112.

#8112 introduces monotonic spec-ID allocation (max(existing) + 1), reserves spec ID 0 for manual sharding, validates reserved and duplicate IDs at MemWAL metadata boundaries, and resolves/validates the writer's spec against the active table spec before any shard claim. Manual sharding is restricted to spec ID 0.

The current initializer only creates the first spec, so an empty history produces spec ID 1. A future spec-revision API will provide the complete append-only history.

Once #8112 lands, I'll rebase #7949 on top of it. #7949 will remain focused on claim-time manifest validation.

[u70b3]

Scope and dependency reset completed and force-pushed on latest main.

I narrowed the claim to the identities current Lance code actually emits: manual 0 and sole automatic 1. The manifest guard is independently useful under that invariant: a manual caller cannot claim/fence an automatic shard, an automatic caller cannot claim/fence a manual shard, and a concurrent first claim cannot be followed by the opposite identity. Rejection remains side-effect free and is revalidated after write conflicts.

This PR no longer claims that monotonic spec allocation is a prerequisite or attempts to solve future sharding revisions/activation. #8112 was separately reduced to resolving the writer's default 0 before a fresh automatic manifest is created. Either PR is independently valid; together they cover creation-time resolution and stored-manifest immutability.

The issue reproducer and tests now use only 0/1; the public error docs also include sealed/no-mutation behavior. Fourteen manifest tests and full workspace clippy pass after the latest-main rebase.

The existing CHANGES_REQUESTED review predates this scope decision. Please re-review the independent defensive invariant.

[u70b3]

@hamersaw PTAL when you have a chance — the open CHANGES_REQUESTED predates the Aug 3 scope reset; the branch is current with main and all checks pass.

[u70b3]

Rebased onto latest main and resolved the conflict with #8640 (the read_latest split): the claim path now re-validates via refresh_latest() per its uncached-read invariant, and the two observing tests use latest(). All manifest tests and clippy pass locally.

@hamersaw your CHANGES_REQUESTED (2026-07-27) was against the pre-2026-08-03 version. Scope has since been narrowed to an independent defensive invariant over the 0/1 identities Lance actually emits today — it no longer depends on monotonic spec-id allocation, which lives in #8112 for independent review. Could you take another look?

[u70b3]

@Xuanwo could you help review this when you have a moment? The CHANGES_REQUESTED (2026-07-27) predates the 2026-08-03 scope reset — the PR is now an independent defensive invariant over the 0/1 shard identities Lance actually emits, no longer gated on monotonic spec-id allocation (that lives in #8112). It's rebased onto current main with the #8640 conflict resolved (the claim path re-validates via refresh_latest per its uncached-read invariant), and CI is fully green. I've re-requested review from hamersaw twice over the past weeks without a response, so a fresh pair of eyes would be much appreciated.

[lance-gatekeeper]

✅ Gate recommendation: approve.

shard_spec_id is already an immutable manifest identity, and this change enforces it at the authoritative claim boundary before any epoch mutation and after write conflicts. It preserves sealed-shard error precedence and existing contention behavior. #8112 remains the complementary creation-side identity resolution rather than a prerequisite for this guard.