Skip to content

chore: add a forge_config crate#2685

Open
tusharmath wants to merge 38 commits intomainfrom
config-refactor-mar-24
Open

chore: add a forge_config crate#2685
tusharmath wants to merge 38 commits intomainfrom
config-refactor-mar-24

Conversation

@tusharmath
Copy link
Collaborator

  • feat(forge_config): add forge configuration crate and loader
  • refactor(forge_config): switch config serde to snake_case
  • refactor(forge_config): load global config with oncecell panic-on-fail
  • feat(forge_config): load user config from home directory
  • refactor(forge_config): rename retry and parallel read config fields
  • refactor(forge_config): add provider and model selection fields
  • feat(forge_config): add FORGE env var overrides for config
  • refactor(forge_config): switch config loading from json to toml
  • chore(forge_config): add default toml config for tool limits
  • refactor(forge_config): rename forge_config module to config
  • refactor(forge_config): switch to async config reader/writer
  • feat(forge_config): add merge strategies to config structs
  • feat(forge_config): implement multi-source config loading order
  • refactor(forge_config): remove merge derive and use toml_edit serialization
  • refactor(forge_config): load config from resolved path
  • feat(forge_config): add temperature, sampling, update, compact types
  • feat(forge_config): add tool limits and compact/update config
  • refactor(forge_config): make config path optional in reader
  • refactor(forge_repo): decouple disk appconfig from domain type
  • refactor(domain): rename AppConfig to ForgeConfig everywhere

@tusharmath tusharmath changed the title config refactor mar 24 chore: add a forge_config crate Mar 24, 2026
@tusharmath tusharmath force-pushed the config-refactor-mar-24 branch from 41a0729 to 5ce8133 Compare March 24, 2026 13:30
@tusharmath tusharmath force-pushed the config-refactor-mar-24 branch from 5ce8133 to f12fe62 Compare March 24, 2026 13:31
Comment on lines +68 to +71
impl From<f32> for Temperature {
fn from(value: f32) -> Self {
Temperature::new_unchecked(value)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The From<f32> implementation for Temperature bypasses validation by calling new_unchecked(). This allows creating invalid Temperature values (outside the 0.0-2.0 range) in production builds where debug assertions are disabled.

Impact: Code like Temperature::from(5.0) will create an invalid state that violates the type's invariants, potentially causing incorrect behavior downstream.

Fix: Remove this implementation or change to TryFrom:

impl TryFrom<f32> for Temperature {
    type Error = String;
    
    fn try_from(value: f32) -> Result<Self, Self::Error> {
        Temperature::new(value)
    }
}
Suggested change
impl From<f32> for Temperature {
fn from(value: f32) -> Self {
Temperature::new_unchecked(value)
}
impl TryFrom<f32> for Temperature {
type Error = String;
fn try_from(value: f32) -> Result<Self, Self::Error> {
Temperature::new(value)
}
}

Spotted by Graphite

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@tusharmath tusharmath enabled auto-merge (squash) March 25, 2026 06:15
@tusharmath tusharmath disabled auto-merge March 25, 2026 06:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant