refactor(ai): address review findings on the provider registry

- Build PROVIDERS via _build_registry(), which raises on a duplicate
  provider name at import time instead of silently dropping the earlier
  spec (dict-comprehension behavior); regression test added
- Pin the exact OpenAI-compatible provider -> discovery URL mapping in
  a test so a registry edit can't silently drop or misassign a URL
- Give TEST_MODELS a real type annotation
  (Dict[str, Tuple[Optional[str], str]]) instead of bare dict

Author: lfnovo

open_notebook/ai/connection_tester.py

@@ -9,7 +9,7 @@
 import json
 import os
 import struct
-from typing import Optional, Tuple
+from typing import Dict, Optional, Tuple
 
 import httpx
 from loguru import logger
@@ -59,7 +59,7 @@ def _is_vertex_credentials_file_error(exc: Exception) -> bool:
 # its own. The provider test also no longer treats a model-level failure as
 # a connection failure (see `_connection_failure_reason`), so even if an
 # alias ever breaks, the test still reports the credentials correctly.
-TEST_MODELS: dict = {
+TEST_MODELS: Dict[str, Tuple[Optional[str], str]] = {
     name: (spec.test_model, spec.test_model_type) for name, spec in PROVIDERS.items()
 }
 

open_notebook/ai/provider_registry.py

@@ -67,9 +67,7 @@ def env_config(self) -> Dict[str, List[str]]:
 _ALL_MODALITIES = ("language", "embedding", "speech_to_text", "text_to_speech")
 
 
-PROVIDERS: Dict[str, ProviderSpec] = {
-    spec.name: spec
-    for spec in (
+_PROVIDER_SPECS: Tuple[ProviderSpec, ...] = (
         ProviderSpec(
             name="openai",
             display_name="OpenAI",
@@ -227,5 +225,23 @@ def env_config(self) -> Dict[str, List[str]]:
             test_model=None,  # Dynamic - uses first available model
             docs_url="https://github.com/lfnovo/open-notebook/blob/main/docs/5-CONFIGURATION/openai-compatible.md",
         ),
-    )
-}
+)
+
+
+def _build_registry(specs: Tuple[ProviderSpec, ...]) -> Dict[str, ProviderSpec]:
+    """Build the name -> spec map, refusing duplicate names at import time.
+
+    A plain dict comprehension would silently drop the earlier spec on a
+    name collision; fail loudly instead.
+    """
+    registry: Dict[str, ProviderSpec] = {}
+    for spec in specs:
+        if spec.name in registry:
+            raise ValueError(
+                f"Duplicate provider name in registry: {spec.name!r}"
+            )
+        registry[spec.name] = spec
+    return registry
+
+
+PROVIDERS: Dict[str, ProviderSpec] = _build_registry(_PROVIDER_SPECS)

tests/test_credential_provider_validation.py

@@ -76,6 +76,46 @@ def test_discovery_functions_cover_registry(self):
 
         assert set(PROVIDER_DISCOVERY_FUNCTIONS.keys()) == set(PROVIDERS.keys())
 
+    def test_registry_rejects_duplicate_provider_names(self):
+        """A plain dict comprehension would silently drop the earlier spec
+        on a name collision; the registry builder must raise instead."""
+        from open_notebook.ai.provider_registry import (
+            ProviderSpec,
+            _build_registry,
+        )
+
+        duplicate = (
+            ProviderSpec(name="dupe", display_name="Dupe A", modalities=("language",)),
+            ProviderSpec(name="dupe", display_name="Dupe B", modalities=("language",)),
+        )
+        with pytest.raises(ValueError, match="Duplicate provider name"):
+            _build_registry(duplicate)
+
+    def test_openai_compat_discovery_urls_are_exactly_as_expected(self):
+        """Pin the derived provider -> discovery URL mapping so a registry
+        edit can't silently drop or misassign a URL (both the model_discovery
+        table and the credentials_service url_map are built from these)."""
+        from open_notebook.ai.model_discovery import OPENAI_COMPAT_PROVIDERS
+
+        expected = {
+            "openai": "https://api.openai.com/v1/models",
+            "groq": "https://api.groq.com/openai/v1/models",
+            "mistral": "https://api.mistral.ai/v1/models",
+            "deepseek": "https://api.deepseek.com/models",
+            "xai": "https://api.x.ai/v1/models",
+            "openrouter": "https://openrouter.ai/api/v1/models",
+            "dashscope": "https://dashscope.aliyuncs.com/compatible-mode/v1/models",
+            "minimax": "https://api.minimax.io/v1/models",
+        }
+        assert {
+            name: spec.openai_compat_discovery_url
+            for name, spec in PROVIDERS.items()
+            if spec.openai_compat_discovery_url
+        } == expected
+        assert {
+            name: spec.url for name, spec in OPENAI_COMPAT_PROVIDERS.items()
+        } == expected
+
 
 class TestSupportedProviderMatchesOtherSourcesOfTruth:
     def test_matches_known_good_provider_list(self):