fix: panic when lance.auto_cleanup.interval is set to 0

[majin1102]

For now, when lance.auto_cleanup.interval is set to 0, dataset commits panic with a "division by zero" error:

thread 'dataset::cleanup::tests::test_auto_cleanup_interval_zero' panicked at rust/lance/src/dataset/cleanup.rs:672:12:
attempt to calculate the remainder with a divisor of zero
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

To fix the panic, we can interpret lance.auto_cleanup.interval = 0 as triggering cleanup after each commit, which is equivalent to lance.auto_cleanup.interval = 1

[github-actions]

PR Review

P1 - Semantic Inconsistency: The fix treats interval = 0 as "run every commit" (same as interval = 1), but this is inconsistent with typical interval semantics where 0 usually means "disabled".

Consider either:

1. Treating interval = 0 as "disabled" (return early with Ok(None)) - more intuitive
2. Documenting the current behavior clearly if "0 means every commit" is intentional

The current silent conversion of 0 → 1 behavior could confuse users who expect 0 to disable the feature.

Suggestion: A cleaner fix would be:

if interval == 0 || manifest.version % interval \!= 0 {
    return Ok(None);
}

This interprets interval = 0 as "auto cleanup disabled" which is more standard.

---

Test coverage is good. The helper function refactoring is a nice improvement.

[github-actions]

PR Review

Summary: Fix for division-by-zero panic when lance.auto_cleanup.interval is set to 0.

Assessment

The fix is correct and minimal. The change interprets interval = 0 as "cleanup on every commit" by short-circuiting the modulo check. When interval == 0, the condition interval != 0 && manifest.version % interval != 0 evaluates to false, so cleanup proceeds.

No blocking issues found.

The test coverage is adequate - it verifies that cleanup runs on consecutive commits with interval=0 and that file counts stabilize correctly.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[majin1102]

Hi, @westonpace
Please let me know what do you think about the semantics of lance.auto_cleanup.interval = 0

[wjones127]

0 seems good to me, given you can always unset auto cleanup.