fix: handle f64 extraction in kvbm-config under arbitrary_precision#317
Conversation
…peninfer-project#296) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a custom deserializer deserialize_opt_f64 for the cache_size_gb fields in HostCacheConfig and DiskCacheConfig. This custom deserializer correctly handles serde_json's arbitrary-precision numbers (using the internal $serde_json::private::Number map representation) and ensures that non-finite values are rejected, preserving the behavior of the native f64 deserializer. Unit tests have been added to verify these behaviors. No review comments were provided, and the implementation is clean and well-tested.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
xiaguan
left a comment
There was a problem hiding this comment.
LGTM. Verified fmt plus kvbm-config lib tests and the kvbm-config + pegainfer-server feature-unification repro locally.
Summary
kvbm-config'scache_size_gb: Option<f64>fields now deserialize correctly whether figment hands them a native JSON number or theserde_jsonarbitrary_precision"number map" form. This unblocks the documented workspace test commandcargo test --release --workspace --lib, which was failing onmain.Why this matters
Per #296,
test_profile_with_defaults_and_overlaypanics withInvalidType(Map, "f64") at path ["cache", "host", "cache_size_gb"]. The crate is green in isolation but red when co-compiled withpegainfer-server:vllm-chat(a transitive git dep) enablesserde_json/arbitrary_precision, and Cargo feature unification turns that feature on for everyserde_jsonuser in the workspace. Underarbitrary_precision, JSON numbers deserialize as an internal map{"$serde_json::private::Number": "2.0"}, so figment'sJsonsource handskvbm-configaMapwhere a plainf64is expected.Pinning or stripping the feature isn't viable: it's owned by a git dependency and would regress on the next bump. The robust, behavior-preserving fix is to make the float fields tolerant of either representation, so the config-load path stays correct no matter which workspace features get unified on.
Changes
A small, self-contained
deserialize_opt_f64serde helper was added incache.rsand applied tocache_size_gbon bothHostCacheConfigandDiskCacheConfig. It accepts a native JSON number, thearbitrary_precisionsingle-entry map form, or an omitted/null value (yieldingNone). The helper uses onlyserde, so there is no dependency change andserde_jsonstays a dev-dependency.It also rejects non-finite parses (such as
1e10000overflowing toinf), matching whatserde_json's native f64 deserializer already does. Without this guard, a malformed config could smuggle in an infinite cache size that later casts tousize::MAXincompute_num_blocks.Testing
cargo fmt --checkandcargo clippy --all-targetsare both clean for the crate, andcargo test --release -p kvbm-config --libpasses 65 tests (up from 63). To confirm the fix targets the exact feature-unification path, I forcedserde_json/arbitrary_precisionon the crate's devserde_json: onmainthis reproduces the failure (test_profile_with_defaults_and_overlayFAILED), and with this change it passes.Two new tests cover the behavior.
test_cache_size_gb_arbitrary_precision_mapasserts the map form, a plain integer (4->4.0), and a fractional value all extract correctly and that an omitted field yieldsNone.test_cache_size_gb_rejects_non_finiteasserts that an out-of-range value is rejected rather than becomingSome(inf).Fixes #296