Stack for rivet-dev/rivet

• docs: update CLAUDE.md for jj #5216 towkupzu · 2026-06-11 05:04:32
• feat(rivetkit): update Rust package #5214 tsxpqnrt · 2026-06-11 04:29:22
• fix(rivetkit): preserve explicit actor startup newness #5215 onpuruqz · 2026-06-11 05:04:29
• feat(depot): support disabling sqlite compaction via config #5205 ltzsplvu · 2026-06-08 23:41:37 👈
• docs: restructure actor state & storage docs into single high-level page #5204 soovppnu · 2026-06-08 23:41:36
• main

Get stack: forklift get 5205
Push local edits: forklift submit
Merge when ready: forklift merge 5205

Code Review: feat(depot): support disabling sqlite compaction via config

Summary

This PR adds a new unstable configuration option unstable_disable_compaction to allow SQLite compaction to be disabled. The feature is implemented across the config layer, Depot conveyer commit logic, and pegboard-envoy database initialization. The changes are well-scoped and follow existing patterns.

Strengths

1. Consistent pattern matching: Mirrors the existing unstable_disable_commit_size_cap pattern in sqlite.rs.
2. Layered properly: Configuration flows correctly from schema to Rust config type to accessor method to usage sites.
3. Test coverage: A new test case commit_without_compaction_signaler_skips_dirty_marker validates core behavior when compaction is disabled.
4. Marked unstable: The config field is correctly marked UNSTABLE, signaling this is not a stable public API.

Issues and Recommendations

1. Logic Flow Clarity in apply.rs (Minor)

File: engine/packages/depot/src/conveyer/commit/apply.rs

When compaction_enabled is false, last_deltas_available_at_ms is set to None even though it is only consumed when compaction is enabled. The logic is correct but could confuse future readers. A short inline comment explaining that the value is only read when compaction is active would clarify intent.

2. Test Coverage Gap (Minor)

File: engine/packages/depot/tests/conveyer_commit.rs

The test validates core behavior when Db is constructed without a compaction signaler, but does not exercise the full config-based path from ws_to_tunnel_task.rs. Not a blocker, but an integration test covering the full path would add confidence.

3. Pre-existing TODO in ws_to_tunnel_task.rs

The compaction_signaler closure contains a pre-existing TODO for incomplete hot compaction work. When compaction is disabled via config the signaler is never created, so the TODO is moot in that code path. Worth tracking in a follow-up whether this relates to the disable feature or a separate hot-compaction enablement effort.

Verification Checklist

• Configuration option flows through the stack correctly
• Conditional logic short-circuits compaction work when disabled
• Test validates basic behavior
• Follows UNSTABLE naming convention
• Mirrors existing unstable_disable_commit_size_cap pattern
• No breaking changes; defaults are safe

Minor Notes

• The .agent/notes/driver-test-progress.md update (78-237 lines) appears to be a test harness status refresh unrelated to this feature. Verify it is intentional.
• For a future docs-sync pass, consider expanding the config description to explain what hot compaction is and when operators would want to disable it.

Recommendation

Approve - The PR is well-structured and ready to merge. The suggestions above are non-blocking improvements for clarity and coverage.