Simplified change

This commit is contained in:
Eric Traut
2026-01-12 13:22:12 -08:00
parent 30a26ce7b6
commit d6b38d80dd

View File

@@ -386,11 +386,6 @@ pub struct ConfigBuilder {
loader_overrides: Option<LoaderOverrides>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
struct ConfigWarning {
pub message: String,
}
impl ConfigBuilder {
pub fn codex_home(mut self, codex_home: PathBuf) -> Self {
self.codex_home = Some(codex_home);
@@ -413,11 +408,6 @@ impl ConfigBuilder {
}
pub async fn build(self) -> std::io::Result<Config> {
let (config, _) = self.build_with_warnings().await?;
Ok(config)
}
async fn build_with_warnings(self) -> std::io::Result<(Config, Vec<ConfigWarning>)> {
let Self {
codex_home,
cli_overrides,
@@ -436,7 +426,7 @@ impl ConfigBuilder {
load_config_layers_state(&codex_home, Some(cwd), &cli_overrides, loader_overrides)
.await?;
let merged_toml = config_layer_stack.effective_config();
let (merged_toml, warnings) = sanitize_config_toml(merged_toml);
let merged_toml = sanitize_config_toml(merged_toml);
// Note that each layer in ConfigLayerStack should have resolved
// relative paths to absolute paths based on the parent folder of the
@@ -445,80 +435,44 @@ impl ConfigBuilder {
let config_toml: ConfigToml = merged_toml
.try_into()
.map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?;
let config = Config::load_config_with_layer_stack(
Config::load_config_with_layer_stack(
config_toml,
harness_overrides,
codex_home,
config_layer_stack,
)?;
Ok((config, warnings))
)
}
}
fn sanitize_config_toml(mut merged_toml: TomlValue) -> (TomlValue, Vec<ConfigWarning>) {
let mut warnings = Vec::new();
let allowed = [
ReasoningEffort::None,
ReasoningEffort::Minimal,
ReasoningEffort::Low,
ReasoningEffort::Medium,
ReasoningEffort::High,
ReasoningEffort::XHigh,
]
.iter()
.map(ReasoningEffort::to_string)
.collect::<Vec<_>>();
fn sanitize_config_toml(mut merged_toml: TomlValue) -> TomlValue {
if let TomlValue::Table(table) = &mut merged_toml {
sanitize_reasoning_effort_table(table, "model_reasoning_effort", &allowed, &mut warnings);
sanitize_reasoning_effort_table(table);
if let Some(TomlValue::Table(profiles)) = table.get_mut("profiles") {
for (profile_name, profile_value) in profiles.iter_mut() {
for (_profile_name, profile_value) in profiles.iter_mut() {
if let TomlValue::Table(profile_table) = profile_value {
let path = format!("profiles.{profile_name}.model_reasoning_effort");
sanitize_reasoning_effort_table(profile_table, &path, &allowed, &mut warnings);
sanitize_reasoning_effort_table(profile_table);
}
}
}
}
(merged_toml, warnings)
merged_toml
}
fn sanitize_reasoning_effort_table(
table: &mut toml::map::Map<String, TomlValue>,
path: &str,
allowed: &[String],
warnings: &mut Vec<ConfigWarning>,
) {
fn sanitize_reasoning_effort_table(table: &mut toml::map::Map<String, TomlValue>) {
let Some(value) = table.get("model_reasoning_effort") else {
return;
};
let raw = match value.as_str() {
Some(raw) => raw.to_string(),
None => value.to_string(),
};
let normalized = raw.trim().to_ascii_lowercase();
if allowed.iter().any(|effort| effort == &normalized) {
if normalized != raw {
table.insert(
"model_reasoning_effort".to_string(),
TomlValue::String(normalized),
);
}
let Some(raw) = value.as_str() else {
table.remove("model_reasoning_effort");
return;
};
if serde_json::from_str::<ReasoningEffort>(&format!("\"{raw}\"")).is_err() {
table.remove("model_reasoning_effort");
}
table.remove("model_reasoning_effort");
let allowed_text = allowed.join(", ");
warnings.push(ConfigWarning {
message: format!(
"Invalid config value for {path}: \"{raw}\". Allowed values: {allowed_text}. Falling back to default."
),
});
}
impl Config {
@@ -568,7 +522,7 @@ pub async fn load_config_as_toml_with_cli_overrides(
.await?;
let merged_toml = config_layer_stack.effective_config();
let (merged_toml, _) = sanitize_config_toml(merged_toml);
let merged_toml = sanitize_config_toml(merged_toml);
let cfg = deserialize_config_toml_with_base(merged_toml, codex_home).map_err(|e| {
tracing::error!("Failed to deserialize overridden config: {e}");
e
@@ -1720,7 +1674,7 @@ persistence = "none"
}
#[tokio::test]
async fn invalid_reasoning_effort_emits_warning_and_uses_default() -> anyhow::Result<()> {
async fn invalid_reasoning_effort_is_ignored() -> anyhow::Result<()> {
let codex_home = TempDir::new()?;
let config_path = codex_home.path().join(CONFIG_TOML_FILE);
@@ -1732,15 +1686,12 @@ model_reasoning_effort = "super-high"
)
.await?;
let (config, warnings) = ConfigBuilder::default()
let config = ConfigBuilder::default()
.codex_home(codex_home.path().to_path_buf())
.build_with_warnings()
.build()
.await?;
assert_eq!(config.model_reasoning_effort, None);
assert_eq!(warnings.len(), 1);
assert!(warnings[0].message.contains("model_reasoning_effort"));
assert!(warnings[0].message.contains("Allowed values"));
Ok(())
}