From b2942519d51e2f27ebd6b90133df0b3972bd5912 Mon Sep 17 00:00:00 2001
From: Bryan Helmkamp <bryan@brynary.com>
Date: Tue, 28 Jul 2026 15:48:22 -0400
Subject: [PATCH] Merge the duplicate selector catalog tests

catalog_from_settings_rejects_duplicate_provider_api_ids was a copy of
catalog_from_settings_rejects_duplicate_model_aliases with the alias
declaration swapped for an api_id. Fold them into one table-driven test
so the shared invariant is stated once: canonical IDs, aliases, and API
IDs occupy a single identifier namespace per provider.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 lib/foundation/fabro-model/src/catalog.rs | 104 +++++++---------------
 1 file changed, 30 insertions(+), 74 deletions(-)

diff --git a/lib/foundation/fabro-model/src/catalog.rs b/lib/foundation/fabro-model/src/catalog.rs
index 2bdab1e6f..96e0afe0f 100644
--- a/lib/foundation/fabro-model/src/catalog.rs
+++ b/lib/foundation/fabro-model/src/catalog.rs
@@ -4253,9 +4253,15 @@ codec = "anthropic_messages"
     }
 
     #[test]
-    fn catalog_from_settings_rejects_duplicate_model_aliases() {
-        let layer = minimal_settings(
-            r#"
+    /// Canonical IDs, aliases, and API IDs share one identifier namespace per
+    /// provider, so a collision in any of them is rejected the same way.
+    fn catalog_from_settings_rejects_duplicate_provider_model_selectors() {
+        for (declaration, expected) in [
+            (r#"aliases = ["shared"]"#, "shared"),
+            (r#"api_id = "vendor/shared""#, "vendor/shared"),
+        ] {
+            let layer = minimal_settings(&format!(
+                r#"
 [providers.test]
 display_name = "Test"
 adapter = "openai"
@@ -4265,7 +4271,7 @@ enabled = true
 [providers.test.models.one]
 display_name = "One"
 family = "test"
-aliases = ["shared"]
+{declaration}
 
 [providers.test.models.one.limits]
 context_window = 1000
@@ -4278,7 +4284,7 @@ reasoning = false
 [providers.test.models.two]
 display_name = "Two"
 family = "test"
-aliases = ["shared"]
+{declaration}
 
 [providers.test.models.two.limits]
 context_window = 1000
@@ -4287,23 +4293,27 @@ context_window = 1000
 tools = false
 vision = false
 reasoning = false
-"#,
-        );
+"#
+            ));
 
-        let err = Catalog::from_settings(&layer).unwrap_err();
+            let err = Catalog::from_settings(&layer).unwrap_err();
 
-        assert!(matches!(
-            err,
-            CatalogBuildError::DuplicateProviderModelSelector {
-                provider,
-                selector,
-                first,
-                second,
-            } if provider == ProviderId::new("test")
-                && selector == "shared"
-                && first == "one"
-                && second == "two"
-        ));
+            assert!(
+                matches!(
+                    &err,
+                    CatalogBuildError::DuplicateProviderModelSelector {
+                        provider,
+                        selector,
+                        first,
+                        second,
+                    } if provider == &ProviderId::new("test")
+                        && selector == expected
+                        && first == "one"
+                        && second == "two"
+                ),
+                "{declaration}: {err:?}"
+            );
+        }
     }
 
     #[test]
@@ -4355,60 +4365,6 @@ reasoning = false
         ));
     }
 
-    #[test]
-    fn catalog_from_settings_rejects_duplicate_provider_api_ids() {
-        let layer = minimal_settings(
-            r#"
-[providers.test]
-display_name = "Test"
-adapter = "openai"
-agent_profile = "openai"
-
-[providers.test.models.one]
-api_id = "vendor/shared"
-display_name = "One"
-family = "test"
-default = true
-
-[providers.test.models.one.limits]
-context_window = 1000
-
-[providers.test.models.one.features]
-tools = false
-vision = false
-reasoning = false
-
-[providers.test.models.two]
-api_id = "vendor/shared"
-display_name = "Two"
-family = "test"
-
-[providers.test.models.two.limits]
-context_window = 1000
-
-[providers.test.models.two.features]
-tools = false
-vision = false
-reasoning = false
-"#,
-        );
-
-        let err = Catalog::from_settings(&layer).unwrap_err();
-
-        assert!(matches!(
-            err,
-            CatalogBuildError::DuplicateProviderModelSelector {
-                provider,
-                selector,
-                first,
-                second,
-            } if provider == ProviderId::new("test")
-                && selector == "vendor/shared"
-                && first == "one"
-                && second == "two"
-        ));
-    }
-
     #[test]
     fn provider_scoped_model_rejects_redundant_provider_field() {
         let error = Catalog::from_settings(&minimal_settings(