clean up littDB config#3549
Conversation
PR SummaryMedium Risk Overview
Reviewed by Cursor Bugbot for commit 2d3c4ac. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3549 +/- ##
==========================================
- Coverage 59.12% 58.26% -0.87%
==========================================
Files 2218 2145 -73
Lines 183132 174565 -8567
==========================================
- Hits 108285 101716 -6569
+ Misses 65082 63783 -1299
+ Partials 9765 9066 -699
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
LGTM — straightforward refactor splitting non-serializable runtime fields (CTX/Logger/Clock/FatalErrorCallback) into a new RuntimeConfig type, with all call sites mechanically updated.
Extended reasoning...
Overview
This PR cleanly separates non-serializable runtime dependencies (CTX, Logger, Clock, FatalErrorCallback) from the serializable litt.Config by introducing a new RuntimeConfig type with DefaultRuntimeConfig() and Validate(). The SanityCheck() rename to Validate() is consistent with the new type. Constructor signatures (NewDB, NewDBUnsafe, NewDiskTable, NewMemTable, TableBuilderFunc) are updated to pass through the new struct, and all 14 modified files (including tests, benchmarks, and the builder) are updated consistently.
Security risks
None. This is an internal API refactor with no changes to auth, crypto, permissions, or data-on-disk layout. The PR is labeled non-app-hash-breaking.
Level of scrutiny
Low. This is a mechanical type-split refactor preserving existing defaults — the variadic NewDB(config, runtimeConfig...) signature keeps the no-runtime-config call path working, and DefaultRuntimeConfig() produces values identical to the prior defaults (context.Background(), slog.Default(), time.Now). I verified that no callers exist outside sei-db/db_engine/litt (grep confirms LittDB is self-contained), and no leftover references to the removed config.CTX/Logger/Clock/FatalErrorCallback fields or to SanityCheck remain.
Other factors
The bug hunting system found no issues. Unit tests are updated alongside the code, including the renamed TestValidateShardingFactorBounds. The Cursor bot flagged it as medium-risk due to wide API surface change, but since LittDB has no external callers in this repo, the blast radius is contained.
Describe your changes and provide context
Cleans up LittDB config so that non-serializable fields are not in the config struct
Testing performed to validate your change
unit tests