fabro(01KQT1TNZ0QXK0QHP10G0V5X84): simplify_opus (succeeded)

Fabro-Run: 01KQT1TNZ0QXK0QHP10G0V5X84
Fabro-Completed: 6

⚒️ Generated with [Fabro](https://fabro.sh)

lib/crates/fabro-config/src/layers/llm.rs

@@ -217,7 +217,7 @@ where
 /// 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)]
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(into = "String", try_from = "String")]
 pub enum CredentialRef {
     /// Structured credential stored in `fabro-vault` keyed by `<id>`.
@@ -227,30 +227,21 @@ pub enum CredentialRef {
     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())
+        match self {
+            Self::Credential(id) => write!(f, "credential:{id}"),
+            Self::Env(name) => write!(f, "env:{name}"),
+        }
     }
 }
 
 impl From<CredentialRef> for String {
     fn from(value: CredentialRef) -> Self {
-        value.as_serialized()
+        value.to_string()
     }
 }
 
@@ -309,14 +300,6 @@ impl TryFrom<String> for CredentialRef {
     }
 }
 
-impl<'de> Deserialize<'de> for CredentialRef {
-    fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
-        use serde::de::Error;
-        let s = String::deserialize(deserializer)?;
-        s.parse().map_err(D::Error::custom)
-    }
-}
-
 // ---------------------------------------------------------------------------
 // Combine glue for primitives that don't already have it.
 // ---------------------------------------------------------------------------

lib/crates/fabro-dev/tests/it/policy.rs

@@ -4,7 +4,7 @@
 //! product-level invariants. They run as part of `cargo nextest` and are
 //! cheap (text scans only).
 
-use std::path::{Path, PathBuf};
+use walkdir::WalkDir;
 
 use crate::workspace_root;
 
@@ -16,6 +16,9 @@ use crate::workspace_root;
 ///
 /// The allowed-callers list below is the policy boundary. Adding a new
 /// caller is intentional and requires updating this list.
+///
+/// The walker only descends into `lib/`, so non-`lib/` paths (docs, top-level
+/// markdown) are not part of the allowlist.
 const BOOTSTRAP_CATALOG_ALLOWED_PATH_FRAGMENTS: &[&str] = &[
     // The bootstrap module itself.
     "lib/crates/fabro-model/src/bootstrap_catalog",
@@ -30,17 +33,48 @@ const BOOTSTRAP_CATALOG_ALLOWED_PATH_FRAGMENTS: &[&str] = &[
     "test_support",
     "/tests/it/",
     "/tests/policy.rs",
-    // Documentation files referencing the policy.
-    "docs/",
-    "CLAUDE.md",
-    "AGENTS.md",
 ];
 
 #[test]
+#[expect(
+    clippy::disallowed_methods,
+    reason = "policy test reads source files synchronously with std::fs"
+)]
 fn bootstrap_catalog_references_stay_in_allowlist() {
     let root = workspace_root();
-    let mut violations: Vec<(PathBuf, usize, String)> = Vec::new();
-    walk_rust_sources(&root, &mut |path, contents| {
+    let lib_root = root.join("lib");
+    let mut violations: Vec<(String, usize, String)> = Vec::new();
+
+    let walker = WalkDir::new(&lib_root).into_iter().filter_entry(|entry| {
+        // Skip generated/output directories at any depth.
+        let name = entry.file_name().to_string_lossy();
+        !matches!(
+            name.as_ref(),
+            "target" | ".git" | "node_modules" | "dist" | "build"
+        )
+    });
+
+    for entry in walker.flatten() {
+        let path = entry.path();
+        if !path.is_file() || path.extension().is_none_or(|ext| ext != "rs") {
+            continue;
+        }
+        let Ok(contents) = std::fs::read_to_string(path) else {
+            continue;
+        };
+        // Cheap early-out: avoids per-line work for the ~99% of files with no
+        // reference to the symbol.
+        if !contents.contains("bootstrap_catalog") {
+            continue;
+        }
+        let rel = path.strip_prefix(&root).unwrap_or(path);
+        let rel_str = rel.to_string_lossy().replace('\\', "/");
+        let path_allowed = BOOTSTRAP_CATALOG_ALLOWED_PATH_FRAGMENTS
+            .iter()
+            .any(|frag| rel_str.contains(frag));
+        if path_allowed {
+            continue;
+        }
         for (idx, line) in contents.lines().enumerate() {
             if !line.contains("bootstrap_catalog") {
                 continue;
@@ -50,57 +84,17 @@ fn bootstrap_catalog_references_stay_in_allowlist() {
             if trimmed.starts_with("//") || trimmed.starts_with("/*") || trimmed.starts_with('*') {
                 continue;
             }
-            let rel = path.strip_prefix(&root).unwrap_or(path);
-            let rel_str = rel.to_string_lossy().replace('\\', "/");
-            if BOOTSTRAP_CATALOG_ALLOWED_PATH_FRAGMENTS
-                .iter()
-                .any(|frag| rel_str.contains(frag))
-            {
-                continue;
-            }
-            violations.push((rel.to_path_buf(), idx + 1, line.to_string()));
+            violations.push((rel_str.clone(), idx + 1, line.to_string()));
         }
-    });
+    }
 
     assert!(
         violations.is_empty(),
         "bootstrap_catalog (install-only) referenced from non-allowlisted source files:\n{}\n\nIf this is intentional, add the path fragment to BOOTSTRAP_CATALOG_ALLOWED_PATH_FRAGMENTS in lib/crates/fabro-dev/tests/it/policy.rs.",
         violations
             .into_iter()
-            .map(|(p, l, s)| format!("  {}:{}: {}", p.display(), l, s.trim()))
+            .map(|(p, l, s)| format!("  {p}:{l}: {}", s.trim()))
             .collect::<Vec<_>>()
             .join("\n"),
     );
 }
-
-#[expect(
-    clippy::disallowed_methods,
-    reason = "policy test reads source files synchronously with std::fs"
-)]
-fn walk_rust_sources(root: &Path, on_file: &mut dyn FnMut(&Path, &str)) {
-    let mut stack: Vec<PathBuf> = vec![root.join("lib")];
-    while let Some(dir) = stack.pop() {
-        let Ok(entries) = std::fs::read_dir(&dir) else {
-            continue;
-        };
-        for entry in entries.flatten() {
-            let path = entry.path();
-            let name = entry.file_name();
-            let name_str = name.to_string_lossy();
-            // Skip generated/output directories.
-            if matches!(
-                name_str.as_ref(),
-                "target" | ".git" | "node_modules" | "dist" | "build"
-            ) {
-                continue;
-            }
-            if path.is_dir() {
-                stack.push(path);
-            } else if path.extension().is_some_and(|ext| ext == "rs") {
-                if let Ok(contents) = std::fs::read_to_string(&path) {
-                    on_file(&path, &contents);
-                }
-            }
-        }
-    }
-}

lib/crates/fabro-llm/src/adapter_registry.rs

@@ -18,6 +18,7 @@ use std::sync::Arc;
 use fabro_auth::ApiKeyHeader;
 use fabro_model::adapter::{self as model_adapter, AdapterMetadata};
 
+use crate::client::auth_value;
 use crate::provider::ProviderAdapter;
 use crate::providers;
 
@@ -60,96 +61,95 @@ impl AdapterConfig {
     }
 }
 
-fn auth_value(header: &ApiKeyHeader) -> String {
-    match header {
-        ApiKeyHeader::Bearer(value) | ApiKeyHeader::Custom { value, .. } => value.clone(),
-    }
-}
-
 /// Factory function signature. Takes a fully-resolved [`AdapterConfig`] and
 /// returns a registered-ready [`ProviderAdapter`].
 ///
 /// Adapter constructors are infallible today; if a future adapter needs to
 /// fail at construction time, add a separate fallible factory variant
 /// rather than re-shaping every existing factory.
-pub type AdapterFactory = fn(&AdapterConfig) -> Arc<dyn ProviderAdapter>;
+pub type AdapterFactory = fn(AdapterConfig) -> Arc<dyn ProviderAdapter>;
 
-const KIMI_BASE_URL: &str = "https://api.moonshot.ai/v1";
-
-fn build_anthropic(config: &AdapterConfig) -> Arc<dyn ProviderAdapter> {
+fn build_anthropic(config: AdapterConfig) -> Arc<dyn ProviderAdapter> {
     let mut adapter = providers::AnthropicAdapter::new(auth_value(&config.auth_header));
-    if let Some(base_url) = config.base_url.clone() {
+    if let Some(base_url) = config.base_url {
         adapter = adapter.with_base_url(base_url);
     }
     if !config.extra_headers.is_empty() {
-        adapter = adapter.with_default_headers(config.extra_headers.clone());
+        adapter = adapter.with_default_headers(config.extra_headers);
     }
     Arc::new(adapter)
 }
 
-fn build_openai(config: &AdapterConfig) -> Arc<dyn ProviderAdapter> {
+fn build_openai(config: AdapterConfig) -> Arc<dyn ProviderAdapter> {
     let mut adapter = providers::OpenAiAdapter::new(auth_value(&config.auth_header));
-    if let Some(base_url) = config.base_url.clone() {
+    if let Some(base_url) = config.base_url {
         adapter = adapter.with_base_url(base_url);
     }
     if !config.extra_headers.is_empty() {
-        adapter = adapter.with_default_headers(config.extra_headers.clone());
+        adapter = adapter.with_default_headers(config.extra_headers);
     }
     if config.codex_mode {
         adapter = adapter.with_codex_mode();
     }
-    if let Some(org_id) = config.org_id.clone() {
+    if let Some(org_id) = config.org_id {
         adapter = adapter.with_org_id(org_id);
     }
-    if let Some(project_id) = config.project_id.clone() {
+    if let Some(project_id) = config.project_id {
         adapter = adapter.with_project_id(project_id);
     }
     Arc::new(adapter)
 }
 
-fn build_gemini(config: &AdapterConfig) -> Arc<dyn ProviderAdapter> {
+fn build_gemini(config: AdapterConfig) -> Arc<dyn ProviderAdapter> {
     let mut adapter = providers::GeminiAdapter::new(auth_value(&config.auth_header));
-    if let Some(base_url) = config.base_url.clone() {
+    if let Some(base_url) = config.base_url {
         adapter = adapter.with_base_url(base_url);
     }
     if !config.extra_headers.is_empty() {
-        adapter = adapter.with_default_headers(config.extra_headers.clone());
+        adapter = adapter.with_default_headers(config.extra_headers);
     }
     Arc::new(adapter)
 }
 
-fn build_openai_compatible(config: &AdapterConfig) -> Arc<dyn ProviderAdapter> {
-    let base_url = config
-        .base_url
-        .clone()
-        .unwrap_or_else(|| KIMI_BASE_URL.to_string());
+fn build_openai_compatible(config: AdapterConfig) -> Arc<dyn ProviderAdapter> {
+    // `openai_compatible` providers vary widely in base URL; the catalog must
+    // pre-resolve `[llm.providers.<id>].base_url` before constructing
+    // `AdapterConfig`. There is no sensible default — silently routing to one
+    // provider's host would produce wrong-host requests for every other.
+    let base_url = config.base_url.expect(
+        "openai_compatible adapter requires a base_url; resolve it from provider settings before \
+         building AdapterConfig",
+    );
     let mut adapter =
         providers::OpenAiCompatibleAdapter::new(auth_value(&config.auth_header), base_url)
-            .with_name(config.provider_id.clone());
+            .with_name(config.provider_id);
     if !config.extra_headers.is_empty() {
-        adapter = adapter.with_default_headers(config.extra_headers.clone());
+        adapter = adapter.with_default_headers(config.extra_headers);
     }
     Arc::new(adapter)
 }
 
+/// Single source of truth pairing every adapter key with its factory. Both
+/// `factory_for` and `registered_keys` derive from this table.
+const FACTORIES: &[(&str, AdapterFactory)] = &[
+    ("anthropic", build_anthropic),
+    ("openai", build_openai),
+    ("gemini", build_gemini),
+    ("openai_compatible", build_openai_compatible),
+];
+
 /// Look up a factory by adapter key. Returns `None` if the key has no factory
 /// registered.
 #[must_use]
 pub fn factory_for(adapter_key: &str) -> Option<AdapterFactory> {
-    match adapter_key {
-        "anthropic" => Some(build_anthropic),
-        "openai" => Some(build_openai),
-        "gemini" => Some(build_gemini),
-        "openai_compatible" => Some(build_openai_compatible),
-        _ => None,
-    }
+    FACTORIES
+        .iter()
+        .find_map(|(key, factory)| (*key == adapter_key).then_some(*factory))
 }
 
 /// Iterate every adapter key with a factory registered.
 pub fn registered_keys() -> impl Iterator<Item = &'static str> {
-    ["anthropic", "openai", "gemini", "openai_compatible"]
-        .iter()
-        .copied()
+    FACTORIES.iter().map(|(key, _)| *key)
 }
 
 /// Look up adapter metadata by key, ensuring the metadata + factory pair
@@ -201,7 +201,7 @@ mod tests {
             name:  "x-api-key".to_string(),
             value: "test-key".to_string(),
         });
-        let adapter = factory_for("anthropic").unwrap()(&config);
+        let adapter = factory_for("anthropic").unwrap()(config);
         assert_eq!(adapter.name(), "anthropic");
     }
 
@@ -216,7 +216,14 @@ mod tests {
             org_id:        None,
             project_id:    None,
         };
-        let adapter = factory_for("openai_compatible").unwrap()(&config);
+        let adapter = factory_for("openai_compatible").unwrap()(config);
         assert_eq!(adapter.name(), "kimi");
     }
+
+    #[test]
+    #[should_panic(expected = "openai_compatible adapter requires a base_url")]
+    fn openai_compatible_factory_panics_without_base_url() {
+        let config = AdapterConfig::new("kimi", ApiKeyHeader::Bearer("k".to_string()));
+        let _ = factory_for("openai_compatible").unwrap()(config);
+    }
 }

lib/crates/fabro-llm/src/client.rs

@@ -321,7 +321,7 @@ impl Client {
     }
 }
 
-fn auth_value(auth_header: &ApiKeyHeader) -> String {
+pub(crate) fn auth_value(auth_header: &ApiKeyHeader) -> String {
     match auth_header {
         ApiKeyHeader::Bearer(value) | ApiKeyHeader::Custom { value, .. } => value.clone(),
     }

lib/crates/fabro-llm/src/types.rs

@@ -410,29 +410,10 @@ pub struct RateLimitInfo {
 }
 
 // --- 3.8 ReasoningEffort ---
-
-#[derive(
-    Debug,
-    Clone,
-    Copy,
-    PartialEq,
-    Eq,
-    Hash,
-    Serialize,
-    Deserialize,
-    strum::Display,
-    strum::EnumString,
-    strum::IntoStaticStr,
-)]
-#[serde(rename_all = "lowercase")]
-#[strum(serialize_all = "lowercase")]
-pub enum ReasoningEffort {
-    Low,
-    Medium,
-    High,
-    XHigh,
-    Max,
-}
+//
+// Re-exported from `fabro-model` so catalog data, request validation, OpenAPI
+// replacement types, and the LLM client share one enum.
+pub use fabro_model::ReasoningEffort;
 
 // --- 3.6 Request ---
 
@@ -1277,30 +1258,4 @@ mod tests {
     fn tool_choice_mode_str_named() {
         assert_eq!(ToolChoice::named("get_weather").mode_str(), "named");
     }
-
-    #[test]
-    fn reasoning_effort_from_str_round_trip() {
-        use std::str::FromStr;
-        assert_eq!(ReasoningEffort::from_str("low"), Ok(ReasoningEffort::Low));
-        assert_eq!(
-            ReasoningEffort::from_str("medium"),
-            Ok(ReasoningEffort::Medium)
-        );
-        assert_eq!(ReasoningEffort::from_str("high"), Ok(ReasoningEffort::High));
-        assert_eq!(
-            ReasoningEffort::from_str("xhigh"),
-            Ok(ReasoningEffort::XHigh)
-        );
-        assert_eq!(ReasoningEffort::from_str("max"), Ok(ReasoningEffort::Max));
-        assert_eq!(ReasoningEffort::XHigh.to_string(), "xhigh");
-        assert_eq!(<&'static str>::from(ReasoningEffort::XHigh), "xhigh");
-        assert_eq!(ReasoningEffort::Max.to_string(), "max");
-        assert_eq!(<&'static str>::from(ReasoningEffort::Max), "max");
-    }
-
-    #[test]
-    fn reasoning_effort_from_str_rejects_unknown() {
-        use std::str::FromStr;
-        assert!(ReasoningEffort::from_str("bogus").is_err());
-    }
 }

lib/crates/fabro-model/src/adapter.rs

@@ -8,6 +8,8 @@
 //! (in `fabro-model`) and the LLM factory registry (in `fabro-llm`) must agree
 //! on the same set of adapter keys; the parity is enforced by tests.
 
+use strum::VariantArray;
+
 use crate::Speed;
 use crate::reasoning::ReasoningEffort;
 
@@ -64,13 +66,9 @@ pub struct AdapterMetadata {
     pub controls:        AdapterControlCapabilities,
 }
 
-const FULL_REASONING_EFFORTS: &[ReasoningEffort] = &[
-    ReasoningEffort::Low,
-    ReasoningEffort::Medium,
-    ReasoningEffort::High,
-    ReasoningEffort::XHigh,
-    ReasoningEffort::Max,
-];
+/// Every reasoning-effort variant. Re-exposed as a const slice so static
+/// adapter metadata can reference it without re-listing variants.
+const FULL_REASONING_EFFORTS: &[ReasoningEffort] = ReasoningEffort::VARIANTS;
 
 const FAST_SPEEDS: &[Speed] = &[Speed::Fast];