Skip to content

Commit 209cd61

Browse files
fix(forge_config): fix f32/f64 TOML serialization noise with Decimal newtype (#2733)
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent 46c936c commit 209cd61

7 files changed

Lines changed: 376 additions & 40 deletions

File tree

crates/forge_config/src/compact.rs

Lines changed: 65 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ use fake::Dummy;
55
use schemars::JsonSchema;
66
use serde::{Deserialize, Serialize};
77

8+
use crate::Percentage;
9+
810
/// Frequency at which forge checks for updates
911
#[derive(Default, Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, fake::Dummy)]
1012
#[serde(rename_all = "snake_case")]
@@ -37,21 +39,6 @@ pub struct Update {
3739
pub auto_update: Option<bool>,
3840
}
3941

40-
fn deserialize_percentage<'de, D>(deserializer: D) -> Result<f64, D::Error>
41-
where
42-
D: serde::Deserializer<'de>,
43-
{
44-
use serde::de::Error;
45-
46-
let value = f64::deserialize(deserializer)?;
47-
if !(0.0..=1.0).contains(&value) {
48-
return Err(Error::custom(format!(
49-
"percentage must be between 0.0 and 1.0, got {value}"
50-
)));
51-
}
52-
Ok(value)
53-
}
54-
5542
/// Configuration for automatic context compaction for all agents
5643
#[derive(Debug, Clone, Serialize, Deserialize, Setters, JsonSchema, PartialEq)]
5744
#[setters(strip_option, into)]
@@ -68,8 +55,8 @@ pub struct Compact {
6855
/// compaction and 1.0 allows summarizing all messages. Works alongside
6956
/// retention_window - the more conservative limit (fewer messages to
7057
/// compact) takes precedence.
71-
#[serde(default, deserialize_with = "deserialize_percentage")]
72-
pub eviction_window: f64,
58+
#[serde(default)]
59+
pub eviction_window: Percentage,
7360

7461
/// Maximum number of tokens to keep after compaction
7562
#[serde(skip_serializing_if = "Option::is_none")]
@@ -113,7 +100,7 @@ impl Compact {
113100
turn_threshold: None,
114101
message_threshold: None,
115102
model: None,
116-
eviction_window: 0.2,
103+
eviction_window: Percentage::new(0.2).unwrap(),
117104
retention_window: 0,
118105
on_turn_end: None,
119106
}
@@ -125,7 +112,7 @@ impl Dummy<fake::Faker> for Compact {
125112
use fake::Fake;
126113
Self {
127114
retention_window: fake::Faker.fake_with_rng(rng),
128-
eviction_window: (0.0f64..=1.0f64).fake_with_rng(rng),
115+
eviction_window: Percentage::from((0.0f64..=1.0f64).fake_with_rng::<f64, R>(rng)),
129116
max_tokens: fake::Faker.fake_with_rng(rng),
130117
token_threshold: fake::Faker.fake_with_rng(rng),
131118
turn_threshold: fake::Faker.fake_with_rng(rng),
@@ -135,3 +122,62 @@ impl Dummy<fake::Faker> for Compact {
135122
}
136123
}
137124
}
125+
126+
#[cfg(test)]
127+
mod tests {
128+
use pretty_assertions::assert_eq;
129+
130+
use super::*;
131+
use crate::reader::ConfigReader;
132+
133+
#[test]
134+
fn test_f64_eviction_window_round_trip() {
135+
let fixture = Compact {
136+
eviction_window: Percentage::new(0.2).unwrap(),
137+
..Compact::new()
138+
};
139+
140+
let toml = toml_edit::ser::to_string_pretty(&fixture).unwrap();
141+
142+
assert!(
143+
toml.contains("eviction_window = 0.2\n"),
144+
"expected `eviction_window = 0.2` in TOML output, got:\n{toml}"
145+
);
146+
}
147+
148+
#[test]
149+
fn test_f64_eviction_window_deserialize_round_trip() {
150+
let fixture = Compact {
151+
eviction_window: Percentage::new(0.2).unwrap(),
152+
..Compact::new()
153+
};
154+
155+
let toml = toml_edit::ser::to_string_pretty(&fixture).unwrap();
156+
157+
let actual = ConfigReader::default()
158+
.read_defaults()
159+
.read_toml(&toml)
160+
.build()
161+
.unwrap()
162+
.compact
163+
.unwrap_or_default();
164+
165+
assert_eq!(actual.eviction_window, fixture.eviction_window);
166+
}
167+
168+
#[test]
169+
fn test_eviction_window_rejects_out_of_range() {
170+
let toml = "[compact]\neviction_window = 1.5\n";
171+
172+
let result = ConfigReader::default()
173+
.read_defaults()
174+
.read_toml(toml)
175+
.build();
176+
177+
assert!(
178+
result.is_err(),
179+
"expected error for eviction_window = 1.5, got: {:?}",
180+
result.ok()
181+
);
182+
}
183+
}

crates/forge_config/src/config.rs

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize};
77

88
use crate::reader::ConfigReader;
99
use crate::writer::ConfigWriter;
10-
use crate::{AutoDumpFormat, Compact, HttpConfig, ModelConfig, RetryConfig, Update};
10+
use crate::{AutoDumpFormat, Compact, Decimal, HttpConfig, ModelConfig, RetryConfig, Update};
1111

1212
/// Top-level Forge configuration merged from all sources (defaults, file,
1313
/// environment).
@@ -85,12 +85,12 @@ pub struct ForgeConfig {
8585
/// Output randomness for all agents; lower values are deterministic, higher
8686
/// values are creative (0.0–2.0).
8787
#[serde(default, skip_serializing_if = "Option::is_none")]
88-
pub temperature: Option<f32>,
88+
pub temperature: Option<Decimal>,
8989

9090
/// Nucleus sampling threshold for all agents; limits token selection to the
9191
/// top cumulative probability mass (0.0–1.0).
9292
#[serde(default, skip_serializing_if = "Option::is_none")]
93-
pub top_p: Option<f32>,
93+
pub top_p: Option<Decimal>,
9494

9595
/// Top-k vocabulary cutoff for all agents; restricts sampling to the k
9696
/// highest-probability tokens (1–1000).
@@ -125,6 +125,49 @@ pub struct ForgeConfig {
125125
pub tool_supported: bool,
126126
}
127127

128+
#[cfg(test)]
129+
mod tests {
130+
use pretty_assertions::assert_eq;
131+
132+
use super::*;
133+
use crate::reader::ConfigReader;
134+
135+
#[test]
136+
fn test_f32_temperature_round_trip() {
137+
let fixture = ForgeConfig { temperature: Some(Decimal(0.1)), ..Default::default() };
138+
139+
let toml = toml_edit::ser::to_string_pretty(&fixture).unwrap();
140+
141+
assert!(
142+
toml.contains("temperature = 0.1\n"),
143+
"expected `temperature = 0.1` in TOML output, got:\n{toml}"
144+
);
145+
}
146+
147+
#[test]
148+
fn test_f32_top_p_round_trip() {
149+
let fixture = ForgeConfig { top_p: Some(Decimal(0.9)), ..Default::default() };
150+
151+
let toml = toml_edit::ser::to_string_pretty(&fixture).unwrap();
152+
153+
assert!(
154+
toml.contains("top_p = 0.9\n"),
155+
"expected `top_p = 0.9` in TOML output, got:\n{toml}"
156+
);
157+
}
158+
159+
#[test]
160+
fn test_f32_temperature_deserialize_round_trip() {
161+
let fixture = ForgeConfig { temperature: Some(Decimal(0.1)), ..Default::default() };
162+
163+
let toml = toml_edit::ser::to_string_pretty(&fixture).unwrap();
164+
165+
let actual = ConfigReader::default().read_toml(&toml).build().unwrap();
166+
167+
assert_eq!(actual.temperature, fixture.temperature);
168+
}
169+
}
170+
128171
impl ForgeConfig {
129172
/// Reads and merges configuration from all sources, returning the resolved
130173
/// [`ForgeConfig`].

crates/forge_config/src/decimal.rs

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
/// A floating-point newtype that serializes to two decimal places, preventing
2+
/// `toml_edit` from emitting noisy bit-pattern approximations such as
3+
/// `0.10000000149011612` or `0.20000000000000001`.
4+
///
5+
/// The inner value is stored as `f64`. When used for fields that ultimately
6+
/// require `f32`, callers should cast via `value() as f32`.
7+
pub struct Decimal(pub f64);
8+
9+
impl Decimal {
10+
/// Returns the inner `f64` value.
11+
pub fn value(&self) -> f64 {
12+
self.0
13+
}
14+
}
15+
16+
impl std::fmt::Debug for Decimal {
17+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
18+
self.0.fmt(f)
19+
}
20+
}
21+
22+
impl Clone for Decimal {
23+
fn clone(&self) -> Self {
24+
Self(self.0)
25+
}
26+
}
27+
28+
impl Copy for Decimal {}
29+
30+
impl PartialEq for Decimal {
31+
fn eq(&self, other: &Self) -> bool {
32+
self.0 == other.0
33+
}
34+
}
35+
36+
impl PartialOrd for Decimal {
37+
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
38+
self.0.partial_cmp(&other.0)
39+
}
40+
}
41+
42+
impl Default for Decimal {
43+
fn default() -> Self {
44+
Self(0.0)
45+
}
46+
}
47+
48+
impl From<f64> for Decimal {
49+
fn from(v: f64) -> Self {
50+
Self(v)
51+
}
52+
}
53+
54+
impl From<f32> for Decimal {
55+
fn from(v: f32) -> Self {
56+
Self(v as f64)
57+
}
58+
}
59+
60+
impl From<Decimal> for f64 {
61+
fn from(d: Decimal) -> Self {
62+
d.0
63+
}
64+
}
65+
66+
impl schemars::JsonSchema for Decimal {
67+
fn schema_name() -> std::borrow::Cow<'static, str> {
68+
f64::schema_name()
69+
}
70+
71+
fn json_schema(r#gen: &mut schemars::generate::SchemaGenerator) -> schemars::Schema {
72+
f64::json_schema(r#gen)
73+
}
74+
}
75+
76+
impl fake::Dummy<fake::Faker> for Decimal {
77+
fn dummy_with_rng<R: fake::RngExt + ?Sized>(_: &fake::Faker, rng: &mut R) -> Self {
78+
use fake::Fake;
79+
Self((0.0f64..2.0f64).fake_with_rng(rng))
80+
}
81+
}
82+
83+
impl serde::Serialize for Decimal {
84+
fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
85+
let formatted: f64 = format!("{:.2}", self.0).parse().unwrap();
86+
serializer.serialize_f64(formatted)
87+
}
88+
}
89+
90+
impl<'de> serde::Deserialize<'de> for Decimal {
91+
fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
92+
Ok(Self(f64::deserialize(deserializer)?))
93+
}
94+
}

crates/forge_config/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,24 @@
11
mod auto_dump;
22
mod compact;
33
mod config;
4+
mod decimal;
45
mod error;
56
mod http;
67
mod legacy;
78
mod model;
9+
mod percentage;
810
mod reader;
911
mod retry;
1012
mod writer;
1113

1214
pub use auto_dump::*;
1315
pub use compact::*;
1416
pub use config::*;
17+
pub use decimal::*;
1518
pub use error::Error;
1619
pub use http::*;
1720
pub use model::*;
21+
pub use percentage::*;
1822
pub use reader::*;
1923
pub use retry::*;
2024
pub use writer::*;

0 commit comments

Comments
 (0)