refactor(config): apply simplify cleanup to interpolation demotions

This commit is contained in:
Scott Werner 2026-06-16 12:37:04 -04:00
parent c35b6a90ed
commit 971a7945ff
5 changed files with 56 additions and 59 deletions

View file

@ -11,7 +11,7 @@ use fabro_config::{
use fabro_static::EnvVars;
use fabro_types::settings::cli::CliTargetSettings;
use fabro_types::settings::server::LogDestination;
use fabro_types::settings::{CliNamespace, InterpString, RunNamespace};
use fabro_types::settings::{InterpString, RunNamespace};
use fabro_types::{ServerSettings, UserSettings};
use fabro_util::error::SharedError;
use fabro_util::version::FABRO_VERSION;
@ -220,21 +220,12 @@ fn value_at_path<'a>(document: &'a toml::Value, path: &[&str]) -> Option<&'a tom
Some(current)
}
/// Pull the resolved CLI target configuration out of `[cli.target]`.
/// Returns either an http(s) URL or a unix socket path.
fn cli_target_from_settings(settings: &CliNamespace) -> Option<String> {
let target = settings.target.as_ref()?;
match target {
CliTargetSettings::Http { url } => Some(url.clone()),
CliTargetSettings::Unix { path } => Some(path.clone()),
}
}
fn configured_server_target(settings: &UserSettings) -> Result<Option<ServerTarget>> {
let Some(value) = cli_target_from_settings(&settings.cli) else {
return Ok(None);
};
parse_server_target(&value).map(Some)
match settings.cli.target.as_ref() {
Some(CliTargetSettings::Http { url }) => ServerTarget::http_url(url).map(Some),
Some(CliTargetSettings::Unix { path }) => ServerTarget::unix_socket_path(path).map(Some),
None => Ok(None),
}
}
pub(crate) fn default_server_target() -> ServerTarget {

View file

@ -21,12 +21,7 @@ pub(crate) fn require_interp(
path: &str,
errors: &mut Vec<ResolveError>,
) -> InterpString {
value.cloned().unwrap_or_else(|| {
errors.push(ResolveError::Missing {
path: path.to_string(),
});
InterpString::parse("")
})
require_value(value, path, errors, || InterpString::parse(""))
}
pub(crate) fn require_string(
@ -34,11 +29,20 @@ pub(crate) fn require_string(
path: &str,
errors: &mut Vec<ResolveError>,
) -> String {
require_value(value, path, errors, String::new)
}
fn require_value<T: Clone>(
value: Option<&T>,
path: &str,
errors: &mut Vec<ResolveError>,
missing: impl FnOnce() -> T,
) -> T {
value.cloned().unwrap_or_else(|| {
errors.push(ResolveError::Missing {
path: path.to_string(),
});
String::new()
missing()
})
}
@ -77,13 +81,20 @@ pub(crate) fn default_interp(path: impl AsRef<std::path::Path>) -> InterpString
/// String pass itself is retired in a later slice. Unclaimed `{{ ... }}` text
/// (jq programs, Go templates) never interpolated and does not warn.
pub(crate) fn warn_if_demoted_template(field: &str, value: Option<&str>) {
if value.is_some_and(|value| !InterpString::parse(value).is_literal()) {
tracing::warn!(
field = %field,
"this field no longer interpolates template tokens and uses the value literally; it \
was demoted to a plain string in the interpolation unification"
);
let Some(value) = value else {
return;
};
if !tracing::enabled!(tracing::Level::WARN) || !value.contains("{{") {
return;
}
if InterpString::parse(value).is_literal() {
return;
}
tracing::warn!(
field = %field,
"this field no longer interpolates template tokens and uses the value literally; it \
was demoted to a plain string in the interpolation unification"
);
}
#[cfg(test)]

View file

@ -527,7 +527,6 @@ async fn build_preflight_report(
&mut checks,
graph,
&resolved_run,
&configured_providers,
catalog.as_ref(),
llm_result,
)
@ -987,12 +986,16 @@ async fn run_llm_check(
checks: &mut Vec<CheckResult>,
graph: &Graph,
settings: &RunNamespace,
configured_providers: &[ProviderId],
catalog: &Catalog,
llm_result: Result<LlmClientResult>,
) -> bool {
let (model, provider) = resolve_model_provider(settings, graph, configured_providers, catalog);
let default_provider = provider.as_deref().unwrap_or("anthropic");
let model = settings
.model
.name
.as_deref()
.unwrap_or_else(|| catalog.default_for_configured_ids(&[]).id.as_str());
let provider = settings.model.provider.as_deref();
let default_provider = provider.unwrap_or("anthropic");
let mut model_providers = std::collections::BTreeSet::new();
let mut has_llm_nodes = false;
@ -1001,7 +1004,7 @@ async fn run_llm_check(
continue;
}
has_llm_nodes = true;
let node_model = node.model().unwrap_or(&model);
let node_model = node.model().unwrap_or(model);
let node_provider = node.provider().unwrap_or(default_provider);
let (resolved_model, resolved_provider) = if let Some(info) = catalog.get(node_model) {
(info.id.clone(), info.provider.to_string())
@ -1142,29 +1145,6 @@ fn canonical_provider_id(catalog: &Catalog, provider_name: &str) -> ProviderId {
.map_or(provider_id, |provider| provider.id.clone())
}
fn resolve_model_provider(
settings: &RunNamespace,
_graph: &Graph,
configured_providers: &[ProviderId],
catalog: &Catalog,
) -> (String, Option<String>) {
let provider = settings.model.provider.clone();
let model = settings.model.name.clone().unwrap_or_else(|| {
catalog
.default_for_configured_ids(configured_providers)
.id
.clone()
});
match catalog.get(&model) {
Some(info) => (
info.id.clone(),
provider.or(Some(info.provider.to_string())),
),
None => (model, provider),
}
}
async fn run_github_token_check(
checks: &mut Vec<CheckResult>,
prepared: &PreparedManifest,

View file

@ -15,6 +15,7 @@
//! the value, via [`InterpString::resolve_with`]. Provenance tracking lets
//! outward-facing renderers redact env- and secret-sourced values uniformly.
use std::borrow::Cow;
use std::fmt;
use serde::de::{self, Visitor};
@ -388,19 +389,29 @@ impl InterpString {
/// [`InterpString::substitute_variables`] for settings fields stored as
/// `String`; it keeps the raw-source round-trip in one audited place.
pub fn substitute_variables_in_str<F>(value: &str, lookup: F) -> Result<String, ResolveError>
where
F: FnMut(&str) -> Option<String>,
{
Self::substitute_variables_in_str_cow(value, lookup).map(Cow::into_owned)
}
pub(crate) fn substitute_variables_in_str_cow<F>(
value: &str,
lookup: F,
) -> Result<Cow<'_, str>, ResolveError>
where
F: FnMut(&str) -> Option<String>,
{
let parsed = Self::parse(value);
if !parsed.references(Namespace::Vars) {
return Ok(value.to_owned());
return Ok(Cow::Borrowed(value));
}
#[expect(
clippy::disallowed_methods,
reason = "canonical raw-source round-trip for String-typed settings fields whose \
remaining tokens resolve downstream"
)]
Ok(parsed.substitute_variables(lookup)?.as_source())
Ok(Cow::Owned(parsed.substitute_variables(lookup)?.as_source()))
}
}

View file

@ -178,7 +178,11 @@ where
if !may_reference_variable(value) {
return Ok(());
}
*value = InterpString::substitute_variables_in_str(value, lookup)?;
if let std::borrow::Cow::Owned(substituted) =
InterpString::substitute_variables_in_str_cow(value, lookup)?
{
*value = substituted;
}
Ok(())
}