From 734756d86d25092186ba7b7c7380a611a7d979d2 Mon Sep 17 00:00:00 2001 From: fabro-bot Date: Mon, 4 May 2026 18:07:19 +0000 Subject: [PATCH] feat(config): add [llm] settings layer for provider/model catalog Adds the trusted, mergeable [llm.providers.] and [llm.models.] TOML schema described by the settings-driven LLM catalog plan: - LlmLayer wires a new top-level [llm] section into SettingsLayer. - ProviderSettings carries display_name, adapter, base_url, credentials, priority, enabled, aliases. - ModelSettings carries provider, api_id, display_name, family, knowledge_cutoff (NaiveDate), default, enabled, aliases, estimated_output_tps plus nested limits/features/controls/costs. - ModelControls exposes per-model reasoning_effort and speed allow- lists as string arrays; concrete enum validation happens later when the resolved Catalog is built. - ModelCostTable holds base CostRates plus optional per-speed CostRates overrides keyed by speed string. - CredentialRef is the typed credential reference enum (credential: or env:); literal secret strings fail deserialization, and Display never echoes resolved secret values. Per-provider and per-model entries field-merge across layers via MergeMap; inner arrays (credentials, aliases, controls.*) replace wholesale per the plan's whole-array merge rule. The legacy [llm] migration error is preserved: [llm.providers] / [llm.models] are accepted, but the old top-level [llm] keys (provider, model, temperature, max_tokens, fallbacks) still surface the [run.model] migration hint, fulfilling the targeted-error requirement instead of a generic serde unknown-field error. Adds 23 unit tests covering parsing, deny_unknown_fields, whole-array replacement, field-merge inheritance, MergeMap behavior, and the new parse-level acceptance/rejection rules. --- Cargo.toml | 4 +- lib/crates/fabro-config/src/layers/llm.rs | 699 ++++++++++++++++++ lib/crates/fabro-config/src/layers/mod.rs | 8 +- .../fabro-config/src/layers/settings.rs | 5 +- lib/crates/fabro-config/src/lib.rs | 16 +- lib/crates/fabro-config/src/parse.rs | 67 +- 6 files changed, 785 insertions(+), 14 deletions(-) create mode 100644 lib/crates/fabro-config/src/layers/llm.rs diff --git a/Cargo.toml b/Cargo.toml index f7897daf1..48598329d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,7 @@ tokio-util = "0.7" clap = { version = "4", features = ["derive", "env"] } clap_complete = "4" jsonschema = { version = "0.42", default-features = false } -chrono = { version = "0.4", features = ["clock"] } +chrono = { version = "0.4", features = ["clock", "serde"] } dashmap = "6" bollard = "0.18" tar = "0.4" @@ -148,4 +148,4 @@ opt-level = 2 [profile.dev.package.regex-automata] opt-level = 2 [profile.dev.package.regex-syntax] -opt-level = 2 +opt-level = 2 \ No newline at end of file diff --git a/lib/crates/fabro-config/src/layers/llm.rs b/lib/crates/fabro-config/src/layers/llm.rs new file mode 100644 index 000000000..f19a528d4 --- /dev/null +++ b/lib/crates/fabro-config/src/layers/llm.rs @@ -0,0 +1,699 @@ +//! `[llm]` settings layer. +//! +//! Holds the trusted, mergeable LLM provider/model catalog data: +//! +//! ```toml +//! [llm.providers.kimi] +//! display_name = "Kimi" +//! adapter = "openai_compatible" +//! base_url = "https://api.moonshot.ai/v1" +//! credentials = ["credential:kimi", "env:KIMI_API_KEY"] +//! priority = 60 +//! enabled = true +//! aliases = ["moonshot"] +//! +//! [llm.models."kimi-k2.5"] +//! provider = "kimi" +//! ... +//! ``` +//! +//! Per-provider and per-model entries field-merge across layers (default → +//! user → server → project → workflow/run). Inner arrays such as +//! `credentials`, `aliases`, `controls.reasoning_effort`, and +//! `controls.speed` replace as whole arrays. +//! +//! Adapter keys (`adapter = "..."`) are parsed as plain strings here. +//! Resolution against the static adapter registry happens in `fabro-model` +//! when the resolved [`Catalog`](fabro_model::Catalog) is built. + +use std::collections::BTreeMap; + +use chrono::NaiveDate; +use serde::{Deserialize, Deserializer, Serialize}; + +use super::maps::MergeMap; + +/// Top-level `[llm]` settings layer. +#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, fabro_macros::Combine)] +#[serde(deny_unknown_fields)] +pub struct LlmLayer { + /// Provider definitions keyed by provider ID. + #[serde(default, skip_serializing_if = "MergeMap::is_empty")] + pub providers: MergeMap, + /// Model definitions keyed by canonical model ID. + #[serde(default, skip_serializing_if = "MergeMap::is_empty")] + pub models: MergeMap, +} + +/// One entry in `[llm.providers.]`. +#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, fabro_macros::Combine)] +#[serde(deny_unknown_fields)] +pub struct ProviderSettings { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub display_name: Option, + /// Adapter registry key (e.g. `"openai_compatible"`). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub adapter: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub base_url: Option, + /// Ordered list of credential references — first successful wins. Each + /// entry must be a typed `CredentialRef` (`credential:` or + /// `env:`); literal secret strings fail deserialization. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub credentials: Option>, + /// Higher wins; missing → `0`; ties broken by canonical provider ID. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub priority: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub enabled: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub aliases: Option>, +} + +/// One entry in `[llm.models.]`. +#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, fabro_macros::Combine)] +#[serde(deny_unknown_fields)] +pub struct ModelSettings { + /// Provider ID this model belongs to. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub provider: Option, + /// Identifier sent to the provider API. Defaults to the catalog model ID + /// when omitted. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub api_id: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub display_name: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub family: Option, + /// Knowledge cutoff as an exact `YYYY-MM-DD` date. Lower-precision labels + /// (e.g. `May 2025`) migrate to the first of the month (`2025-05-01`); + /// presentation can render lower precision. + #[serde( + default, + deserialize_with = "deserialize_knowledge_cutoff", + skip_serializing_if = "Option::is_none" + )] + pub knowledge_cutoff: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub default: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub enabled: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub aliases: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub estimated_output_tps: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub limits: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub features: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub controls: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub costs: Option, +} + +#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, fabro_macros::Combine)] +#[serde(deny_unknown_fields)] +pub struct ModelLimits { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub context_window: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub max_output: Option, +} + +#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, fabro_macros::Combine)] +#[serde(deny_unknown_fields)] +pub struct ModelFeatures { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub tools: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub vision: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub reasoning: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub effort: Option, +} + +/// User-facing allow-list for native control values Fabro accepts on this +/// model. Whole-array replacement on merge. +#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, fabro_macros::Combine)] +#[serde(deny_unknown_fields)] +pub struct ModelControls { + /// Allowed reasoning-effort values. Strings (e.g. `"low"`, `"high"`, + /// `"xhigh"`) — validated as `ReasoningEffort` at catalog build. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub reasoning_effort: Option>, + /// Additional speeds beyond `Speed::Standard`. Strings — validated as + /// `Speed` at catalog build. `Speed::Standard` is implicit and must not + /// appear here. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub speed: Option>, +} + +/// Pricing table. Base [`CostRates`] always apply; per-speed overrides +/// substitute when the request specifies a non-standard speed. +#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, fabro_macros::Combine)] +#[serde(deny_unknown_fields)] +pub struct ModelCostTable { + #[serde(flatten)] + pub base: CostRates, + /// Per-speed cost overrides (e.g. `costs.speed.fast = { ... }`). Keys + /// must reference a speed declared in `controls.speed`. `standard` is + /// not a valid override key — base rates serve standard speed. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub speed: Option>, +} + +#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, fabro_macros::Combine)] +#[serde(deny_unknown_fields)] +pub struct CostRates { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub input_cost_per_mtok: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub output_cost_per_mtok: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub cache_input_cost_per_mtok: Option, +} + +/// Accept either a TOML local-date (`2025-01-01` → `Datetime`) or a +/// `YYYY-MM-DD` string for `knowledge_cutoff`. JSON has no native date +/// literal; settings authors use the bare TOML date form, but JSON loaders +/// (e.g. defaults bundled as JSON) supply a string. +fn deserialize_knowledge_cutoff<'de, D>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + use serde::de::Error; + + #[derive(Deserialize)] + #[serde(untagged)] + enum Either { + Toml(toml::value::Datetime), + Str(String), + } + + let value = Option::::deserialize(deserializer)?; + match value { + None => Ok(None), + Some(Either::Str(s)) => NaiveDate::parse_from_str(&s, "%Y-%m-%d") + .map(Some) + .map_err(D::Error::custom), + Some(Either::Toml(dt)) => { + let date = dt + .date + .ok_or_else(|| D::Error::custom("knowledge_cutoff requires a date component"))?; + NaiveDate::from_ymd_opt(date.year.into(), date.month.into(), date.day.into()) + .ok_or_else(|| D::Error::custom("knowledge_cutoff is not a valid calendar date")) + .map(Some) + } + } +} + +// --------------------------------------------------------------------------- +// CredentialRef — typed credential reference +// --------------------------------------------------------------------------- + +/// A typed credential reference. Literal secret strings are rejected at +/// deserialization so settings never carry a successful "secret string" +/// representation. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(into = "String", try_from = "String")] +pub enum CredentialRef { + /// Structured credential stored in `fabro-vault` keyed by ``. + Credential(String), + /// Process environment variable ``. Falls back to a raw vault + /// secret with the same name when the env var is unset. + Env(String), +} + +impl CredentialRef { + /// The string form, e.g. `"credential:openai_codex"` or + /// `"env:KIMI_API_KEY"`. Stable for serialization round-trips. + #[must_use] + pub fn as_serialized(&self) -> String { + match self { + Self::Credential(id) => format!("credential:{id}"), + Self::Env(name) => format!("env:{name}"), + } + } +} + +impl std::fmt::Display for CredentialRef { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // Display deliberately writes only the typed reference form, never + // any resolved secret value. Env names and credential IDs are not + // themselves secret. + f.write_str(&self.as_serialized()) + } +} + +impl From for String { + fn from(value: CredentialRef) -> Self { + value.as_serialized() + } +} + +/// Error returned when a credential string is neither `credential:` nor +/// `env:`. Literal secret strings always fall into this branch and +/// fail deserialization — by design. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct CredentialRefParseError { + /// Reason the input was rejected. Never echoes the input itself, since + /// it could be a literal secret. + message: String, +} + +impl std::fmt::Display for CredentialRefParseError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.message) + } +} + +impl std::error::Error for CredentialRefParseError {} + +impl std::str::FromStr for CredentialRef { + type Err = CredentialRefParseError; + + fn from_str(s: &str) -> Result { + if let Some(id) = s.strip_prefix("credential:") { + if id.is_empty() { + return Err(CredentialRefParseError { + message: "credential reference is missing an ID after `credential:`" + .to_string(), + }); + } + return Ok(Self::Credential(id.to_string())); + } + if let Some(name) = s.strip_prefix("env:") { + if name.is_empty() { + return Err(CredentialRefParseError { + message: "credential reference is missing a name after `env:`".to_string(), + }); + } + return Ok(Self::Env(name.to_string())); + } + Err(CredentialRefParseError { + message: "credential reference must be `credential:` or `env:`; literal \ + secret strings are rejected" + .to_string(), + }) + } +} + +impl TryFrom for CredentialRef { + type Error = CredentialRefParseError; + + fn try_from(value: String) -> Result { + value.parse() + } +} + +impl<'de> Deserialize<'de> for CredentialRef { + fn deserialize>(deserializer: D) -> Result { + let s = String::deserialize(deserializer)?; + s.parse().map_err(serde::de::Error::custom) + } +} + +// --------------------------------------------------------------------------- +// Combine glue for primitives that don't already have it. +// --------------------------------------------------------------------------- + +use super::combine::Combine; + +// `Option`, `Option`, etc. already have `Combine` impls in +// `combine.rs`. Only add the impls that are unique to the LLM layer types. + +impl Combine for Option { + fn combine(self, other: Self) -> Self { + self.or(other) + } +} + +impl Combine for Option { + fn combine(self, other: Self) -> Self { + self.or(other) + } +} + +impl Combine for Option> { + fn combine(self, other: Self) -> Self { + self.or(other) + } +} + +impl Combine for Option> { + fn combine(self, other: Self) -> Self { + self.or(other) + } +} + +#[cfg(test)] +mod tests { + use std::str::FromStr; + + use super::*; + + // ---- CredentialRef ---------------------------------------------------- + + #[test] + fn credential_ref_parses_credential_form() { + let r = CredentialRef::from_str("credential:openai_codex").unwrap(); + assert_eq!(r, CredentialRef::Credential("openai_codex".to_string())); + } + + #[test] + fn credential_ref_parses_env_form() { + let r = CredentialRef::from_str("env:KIMI_API_KEY").unwrap(); + assert_eq!(r, CredentialRef::Env("KIMI_API_KEY".to_string())); + } + + #[test] + fn credential_ref_rejects_literal_secret() { + // A literal API key contains no `credential:` or `env:` prefix. + let err = CredentialRef::from_str("sk-ant-1234").unwrap_err(); + assert!(err.to_string().contains("must be")); + assert!( + !err.to_string().contains("sk-ant-1234"), + "error must not echo the literal secret string back to the user", + ); + } + + #[test] + fn credential_ref_rejects_empty_credential_id() { + let err = CredentialRef::from_str("credential:").unwrap_err(); + assert!(err.to_string().contains("missing")); + } + + #[test] + fn credential_ref_rejects_empty_env_name() { + let err = CredentialRef::from_str("env:").unwrap_err(); + assert!(err.to_string().contains("missing")); + } + + #[test] + fn credential_ref_round_trips_through_string() { + let r = CredentialRef::Credential("kimi".to_string()); + assert_eq!(r.to_string(), "credential:kimi"); + let back: CredentialRef = r.to_string().parse().unwrap(); + assert_eq!(back, r); + } + + #[test] + fn credential_ref_serializes_as_string_in_toml() { + let r = CredentialRef::Env("KIMI_API_KEY".to_string()); + let s = toml::Value::try_from(&r).unwrap(); + assert_eq!(s.as_str(), Some("env:KIMI_API_KEY")); + } + + #[test] + fn credential_ref_deserializes_from_toml_string() { + let parsed: CredentialRef = toml::from_str(r#"v = "credential:foo""#) + .map(|v: toml::Value| { + v.as_table() + .unwrap() + .get("v") + .unwrap() + .clone() + .try_into() + .unwrap() + }) + .unwrap(); + assert_eq!(parsed, CredentialRef::Credential("foo".to_string())); + } + + #[test] + fn credential_ref_in_array_rejects_literal_secret() { + // serde rejects literal secrets when parsed inside an array of + // CredentialRef. The error bubbles up as a TOML deserialization + // failure. + #[derive(Deserialize)] + struct Wrap { + v: Vec, + } + let err: Result = toml::from_str(r#"v = ["sk-literal-secret"]"#); + assert!(err.is_err(), "literal secret strings must fail to parse"); + } + + // ---- LlmLayer parsing ------------------------------------------------- + + #[test] + fn parses_minimal_provider_entry() { + let toml = r#" +[providers.kimi] +display_name = "Kimi" +adapter = "openai_compatible" +base_url = "https://api.moonshot.ai/v1" +credentials = ["credential:kimi", "env:KIMI_API_KEY"] +priority = 60 +enabled = true +aliases = ["moonshot"] +"#; + let layer: LlmLayer = toml::from_str(toml).unwrap(); + let kimi = layer.providers.get("kimi").unwrap(); + assert_eq!(kimi.display_name.as_deref(), Some("Kimi")); + assert_eq!(kimi.adapter.as_deref(), Some("openai_compatible")); + assert_eq!( + kimi.base_url.as_deref(), + Some("https://api.moonshot.ai/v1") + ); + assert_eq!(kimi.priority, Some(60)); + assert_eq!(kimi.enabled, Some(true)); + assert_eq!(kimi.aliases.as_deref(), Some(&["moonshot".to_string()][..])); + assert_eq!( + kimi.credentials.as_ref().unwrap(), + &vec![ + CredentialRef::Credential("kimi".to_string()), + CredentialRef::Env("KIMI_API_KEY".to_string()), + ] + ); + } + + #[test] + fn parses_full_model_entry() { + let toml = r#" +[models."kimi-k2.5"] +provider = "kimi" +api_id = "kimi-k2.5" +display_name = "Kimi K2.5" +family = "kimi" +knowledge_cutoff = 2025-01-01 +default = true +enabled = true +aliases = ["kimi"] +estimated_output_tps = 50 + +[models."kimi-k2.5".limits] +context_window = 262144 +max_output = 32768 + +[models."kimi-k2.5".features] +tools = true +vision = false +reasoning = true +effort = false + +[models."kimi-k2.5".costs] +input_cost_per_mtok = 0.60 +output_cost_per_mtok = 2.50 +cache_input_cost_per_mtok = 0.15 +"#; + let layer: LlmLayer = toml::from_str(toml).unwrap(); + let m = layer.models.get("kimi-k2.5").unwrap(); + assert_eq!(m.provider.as_deref(), Some("kimi")); + assert_eq!(m.api_id.as_deref(), Some("kimi-k2.5")); + assert_eq!(m.display_name.as_deref(), Some("Kimi K2.5")); + assert_eq!(m.family.as_deref(), Some("kimi")); + assert_eq!( + m.knowledge_cutoff, + Some(NaiveDate::from_ymd_opt(2025, 1, 1).unwrap()) + ); + assert_eq!(m.default, Some(true)); + assert_eq!(m.enabled, Some(true)); + assert_eq!(m.aliases.as_deref(), Some(&["kimi".to_string()][..])); + assert_eq!(m.estimated_output_tps, Some(50.0)); + + let limits = m.limits.as_ref().unwrap(); + assert_eq!(limits.context_window, Some(262_144)); + assert_eq!(limits.max_output, Some(32_768)); + + let features = m.features.as_ref().unwrap(); + assert_eq!(features.tools, Some(true)); + assert_eq!(features.vision, Some(false)); + assert_eq!(features.reasoning, Some(true)); + assert_eq!(features.effort, Some(false)); + + let costs = m.costs.as_ref().unwrap(); + assert_eq!(costs.base.input_cost_per_mtok, Some(0.60)); + assert_eq!(costs.base.output_cost_per_mtok, Some(2.50)); + assert_eq!(costs.base.cache_input_cost_per_mtok, Some(0.15)); + assert!(costs.speed.is_none()); + } + + #[test] + fn parses_controls_and_per_speed_costs() { + let toml = r#" +[models."claude-opus-4-6".controls] +reasoning_effort = ["low", "medium", "high"] +speed = ["fast"] + +[models."claude-opus-4-6".costs.speed.fast] +input_cost_per_mtok = 90.0 +output_cost_per_mtok = 450.0 +cache_input_cost_per_mtok = 9.0 +"#; + let layer: LlmLayer = toml::from_str(toml).unwrap(); + let m = layer.models.get("claude-opus-4-6").unwrap(); + + let controls = m.controls.as_ref().unwrap(); + assert_eq!( + controls.reasoning_effort.as_deref(), + Some( + &[ + "low".to_string(), + "medium".to_string(), + "high".to_string() + ][..] + ) + ); + assert_eq!( + controls.speed.as_deref(), + Some(&["fast".to_string()][..]) + ); + + let costs = m.costs.as_ref().unwrap(); + let fast = costs.speed.as_ref().unwrap().get("fast").unwrap(); + assert_eq!(fast.input_cost_per_mtok, Some(90.0)); + assert_eq!(fast.output_cost_per_mtok, Some(450.0)); + assert_eq!(fast.cache_input_cost_per_mtok, Some(9.0)); + } + + #[test] + fn rejects_unknown_provider_field() { + let toml = r#" +[providers.kimi] +adapter = "openai_compatible" +unknown_field = true +"#; + let err = toml::from_str::(toml).unwrap_err(); + assert!(err.to_string().contains("unknown_field")); + } + + #[test] + fn rejects_unknown_model_field() { + let toml = r#" +[models.foo] +provider = "x" +mystery = 1 +"#; + let err = toml::from_str::(toml).unwrap_err(); + assert!(err.to_string().contains("mystery")); + } + + // ---- Combine / merge -------------------------------------------------- + + #[test] + fn provider_field_merge_keeps_self_values_and_fills_holes() { + let high = ProviderSettings { + adapter: Some("openai_compatible".to_string()), + base_url: Some("https://override.example".to_string()), + ..ProviderSettings::default() + }; + let low = ProviderSettings { + adapter: Some("anthropic".to_string()), + base_url: Some("https://defaults.example".to_string()), + display_name: Some("Default".to_string()), + priority: Some(10), + ..ProviderSettings::default() + }; + let merged = high.combine(low); + assert_eq!(merged.adapter.as_deref(), Some("openai_compatible")); + assert_eq!(merged.base_url.as_deref(), Some("https://override.example")); + assert_eq!(merged.display_name.as_deref(), Some("Default")); + assert_eq!(merged.priority, Some(10)); + } + + #[test] + fn provider_credentials_array_replaces_wholesale() { + // Higher layer redeclares credentials → low layer's list is dropped + // entirely (whole-array replacement). + let high = ProviderSettings { + credentials: Some(vec![CredentialRef::Env("FOO".to_string())]), + ..ProviderSettings::default() + }; + let low = ProviderSettings { + credentials: Some(vec![ + CredentialRef::Credential("bar".to_string()), + CredentialRef::Env("BAZ".to_string()), + ]), + ..ProviderSettings::default() + }; + let merged = high.combine(low); + assert_eq!( + merged.credentials.unwrap(), + vec![CredentialRef::Env("FOO".to_string())] + ); + } + + #[test] + fn provider_credentials_inherits_when_unset_in_higher_layer() { + let high = ProviderSettings::default(); + let low = ProviderSettings { + credentials: Some(vec![CredentialRef::Env("FOO".to_string())]), + ..ProviderSettings::default() + }; + let merged = high.combine(low); + assert_eq!( + merged.credentials.unwrap(), + vec![CredentialRef::Env("FOO".to_string())] + ); + } + + #[test] + fn merge_map_field_merges_per_provider_id() { + let mut high_map: std::collections::HashMap = + std::collections::HashMap::new(); + high_map.insert( + "kimi".to_string(), + ProviderSettings { + base_url: Some("https://override".to_string()), + ..ProviderSettings::default() + }, + ); + let high: MergeMap = MergeMap::from(high_map); + + let mut low_map: std::collections::HashMap = + std::collections::HashMap::new(); + low_map.insert( + "kimi".to_string(), + ProviderSettings { + adapter: Some("openai_compatible".to_string()), + base_url: Some("https://defaults".to_string()), + ..ProviderSettings::default() + }, + ); + let low: MergeMap = MergeMap::from(low_map); + + let merged = high.combine(low); + let kimi = merged.get("kimi").unwrap(); + assert_eq!(kimi.adapter.as_deref(), Some("openai_compatible")); + assert_eq!(kimi.base_url.as_deref(), Some("https://override")); + } + + #[test] + fn model_controls_replace_wholesale() { + // Whole-array replacement: high layer's `reasoning_effort` shadows + // the low layer's list completely. + let high = ModelControls { + reasoning_effort: Some(vec!["high".to_string()]), + ..ModelControls::default() + }; + let low = ModelControls { + reasoning_effort: Some(vec!["low".to_string(), "high".to_string()]), + speed: Some(vec!["fast".to_string()]), + }; + let merged = high.combine(low); + assert_eq!(merged.reasoning_effort.as_deref(), Some(&["high".to_string()][..])); + assert_eq!(merged.speed.as_deref(), Some(&["fast".to_string()][..])); + } +} \ No newline at end of file diff --git a/lib/crates/fabro-config/src/layers/mod.rs b/lib/crates/fabro-config/src/layers/mod.rs index 60af5e79e..fd08346cd 100644 --- a/lib/crates/fabro-config/src/layers/mod.rs +++ b/lib/crates/fabro-config/src/layers/mod.rs @@ -1,6 +1,7 @@ mod cli; mod combine; mod features; +mod llm; mod log_filter; mod maps; mod project; @@ -16,6 +17,11 @@ pub use cli::{ }; pub(crate) use combine::Combine; pub use features::FeaturesLayer; +pub use llm::{ + CostRates, CredentialRef, CredentialRefParseError, LlmLayer, ModelControls, ModelCostTable, + ModelFeatures as LlmModelFeatures, ModelLimits as LlmModelLimits, ModelSettings, + ProviderSettings, +}; pub use log_filter::LogFilter; pub use maps::{MergeMap, ReplaceMap, StickyMap}; pub use project::ProjectLayer; @@ -36,4 +42,4 @@ pub use server::{ SlackIntegrationLayer, TeamsIntegrationLayer, }; pub(crate) use settings::SettingsLayer; -pub use workflow::WorkflowLayer; +pub use workflow::WorkflowLayer; \ No newline at end of file diff --git a/lib/crates/fabro-config/src/layers/settings.rs b/lib/crates/fabro-config/src/layers/settings.rs index 4d3401c79..17785b53b 100644 --- a/lib/crates/fabro-config/src/layers/settings.rs +++ b/lib/crates/fabro-config/src/layers/settings.rs @@ -11,6 +11,7 @@ use serde::{Deserialize, Serialize}; use super::cli::CliLayer; use super::features::FeaturesLayer; +use super::llm::LlmLayer; use super::project::ProjectLayer; use super::run::RunLayer; use super::server::ServerLayer; @@ -34,6 +35,8 @@ pub(crate) struct SettingsLayer { pub server: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub features: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub llm: Option, } impl FromStr for SettingsLayer { @@ -131,4 +134,4 @@ impl SettingsLayer { let auth = server.auth.get_or_insert_with(ServerAuthLayer::default); auth.methods = Some(vec![ServerAuthMethod::DevToken]); } -} +} \ No newline at end of file diff --git a/lib/crates/fabro-config/src/lib.rs b/lib/crates/fabro-config/src/lib.rs index 1394f08b9..d1942d517 100644 --- a/lib/crates/fabro-config/src/lib.rs +++ b/lib/crates/fabro-config/src/lib.rs @@ -37,12 +37,14 @@ pub use fabro_util::path::expand_tilde; pub use home::Home; pub use layers::{ CliAuthLayer, CliExecAgentLayer, CliExecLayer, CliExecModelLayer, CliLayer, CliLoggingLayer, - CliOutputLayer, CliTargetLayer, CliUpdatesLayer, DaytonaDockerfileLayer, DaytonaSandboxLayer, - DaytonaSnapshotLayer, DiscordIntegrationLayer, DockerSandboxLayer, FeaturesLayer, - GitAuthorLayer, GithubIntegrationLayer, HookAgentMarker, HookEntry, HookTlsMode, - IntegrationWebhooksLayer, InterviewProviderLayer, InterviewsLayer, LocalSandboxLayer, - LogFilter, McpEntryLayer, MergeMap, ModelRefOrSplice, NotificationProviderLayer, - NotificationRouteLayer, ObjectStoreLocalLayer, ObjectStoreS3Layer, PrepareStep, ProjectLayer, + CliOutputLayer, CliTargetLayer, CliUpdatesLayer, CostRates, CredentialRef, + CredentialRefParseError, DaytonaDockerfileLayer, DaytonaSandboxLayer, DaytonaSnapshotLayer, + DiscordIntegrationLayer, DockerSandboxLayer, FeaturesLayer, GitAuthorLayer, + GithubIntegrationLayer, HookAgentMarker, HookEntry, HookTlsMode, IntegrationWebhooksLayer, + InterviewProviderLayer, InterviewsLayer, LlmLayer, LlmModelFeatures, LlmModelLimits, + LocalSandboxLayer, LogFilter, McpEntryLayer, MergeMap, ModelControls, ModelCostTable, + ModelRefOrSplice, ModelSettings, NotificationProviderLayer, NotificationRouteLayer, + ObjectStoreLocalLayer, ObjectStoreS3Layer, PrepareStep, ProjectLayer, ProviderSettings, ReplaceMap, RunAgentLayer, RunArtifactsLayer, RunCheckpointLayer, RunExecutionLayer, RunGitLayer, RunGoalLayer, RunLayer, RunModelLayer, RunPrepareLayer, RunPullRequestLayer, RunSandboxLayer, RunScmLayer, ScmGitHubLayer, ServerApiLayer, ServerArtifactsLayer, @@ -85,4 +87,4 @@ where Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(T::default()), Err(e) => Err(Error::read_file(&default_path, e)), } -} +} \ No newline at end of file diff --git a/lib/crates/fabro-config/src/parse.rs b/lib/crates/fabro-config/src/parse.rs index 50d842b31..e109b64bb 100644 --- a/lib/crates/fabro-config/src/parse.rs +++ b/lib/crates/fabro-config/src/parse.rs @@ -5,7 +5,19 @@ use crate::SettingsLayer; const CURRENT_VERSION: u32 = 1; const ALLOWED_TOP_LEVEL_KEYS: &[&str] = &[ - "_version", "project", "workflow", "run", "cli", "server", "features", + "_version", "project", "workflow", "run", "cli", "server", "features", "llm", +]; + +/// Legacy `[llm]` keys that pre-date the settings-driven catalog plan and +/// should still produce the migration hint for `[run.model]` rather than be +/// silently accepted by the new `[llm]` schema. +const LEGACY_LLM_KEYS: &[&str] = &[ + "provider", + "model", + "temperature", + "max_tokens", + "fallbacks", + "fallback", ]; #[derive(Debug, Clone, PartialEq, Eq)] @@ -26,7 +38,7 @@ impl fmt::Display for ParseError { } else { write!( f, - "unknown top-level settings key `{key}`: expected one of `_version`, `project`, `workflow`, `run`, `cli`, `server`, `features`" + "unknown top-level settings key `{key}`: expected one of `_version`, `project`, `workflow`, `run`, `cli`, `server`, `features`, `llm`" ) } } @@ -71,6 +83,21 @@ pub(crate) fn parse_settings(input: &str) -> Result { }); } } + + // The settings-driven catalog plan re-introduced `[llm]` for provider + // and model rows. Old top-level `[llm]` keys (`provider`, `model`, + // ...) are forbidden and must continue to surface the migration hint + // for `[run.model]` rather than be silently dropped or generic-error. + if let Some(llm_table) = table.get("llm").and_then(toml::Value::as_table) { + for legacy_key in LEGACY_LLM_KEYS { + if llm_table.contains_key(*legacy_key) { + return Err(ParseError::UnknownTopLevelKey { + key: format!("llm.{legacy_key}"), + hint: rename_hint("llm"), + }); + } + } + } } raw.try_into::() @@ -166,4 +193,38 @@ mod tests { let err = "_version = 99".parse::().unwrap_err(); assert!(err.to_string().contains("Upgrade")); } -} + + #[test] + fn accepts_new_llm_providers_subtree() { + let parsed = "[llm.providers.kimi]\nadapter = \"openai_compatible\"\n" + .parse::() + .unwrap(); + assert!(parsed.llm.unwrap().providers.contains_key("kimi")); + } + + #[test] + fn accepts_new_llm_models_subtree() { + let parsed = "[llm.models.\"foo\"]\nprovider = \"kimi\"\n" + .parse::() + .unwrap(); + assert!(parsed.llm.unwrap().models.contains_key("foo")); + } + + #[test] + fn rejects_legacy_llm_provider_key_with_run_model_hint() { + let err = "[llm]\nprovider = \"openai\"\n" + .parse::() + .unwrap_err(); + let text = err.to_string(); + assert!(text.contains("run.model") || text.contains("llm"), "got: {text}"); + } + + #[test] + fn rejects_legacy_llm_model_key_with_run_model_hint() { + let err = "[llm]\nmodel = \"opus\"\n" + .parse::() + .unwrap_err(); + let text = err.to_string(); + assert!(text.contains("run.model") || text.contains("llm"), "got: {text}"); + } +} \ No newline at end of file