fix(interface): don't leak YAML default provider into user-supplied list (#591)

* fix(interface): don't leak YAML default provider into user-supplied list

When a caller passes ``model_providers`` to ``DataDesigner.__init__``, the
YAML's ``default:`` key from ``~/.data-designer/model_providers.yaml`` was
still being applied to the resulting ``ModelProviderRegistry``. This caused
two related problems:

1. Hard failure: if the YAML default named a provider absent from the
   user-supplied list, construction raised
   ``ValidationError: Specified default 'X' not found in providers list``.
2. Silent override: if the YAML default matched a non-first user-supplied
   provider, the documented "first wins" behavior was silently overridden.

Gate the YAML lookup on ``model_providers is None`` so that user-supplied
lists own their own default. Also expose ``model_provider_registry`` and
``run_config`` as public read-only properties on ``DataDesigner``, paired
with the existing ``secret_resolver`` property and ``set_run_config``
setter; tests now use these instead of the underscore-prefixed attributes.

Closes #588.

Made-with: Cursor

* fix(interface): tighten model_provider_registry docstring; pin YAML-fallback path

Address review feedback on PR #591:
- Clarify ``model_provider_registry`` docstring so it reflects the full
  fallback chain: user-supplied first → YAML default (when set) → first
  provider in the YAML list.
- Add ``test_init_no_user_providers_uses_yaml_default`` to lock the
  YAML-fallback contract that the #588 fix preserved but didn't pin.

Made-with: Cursor

Author: nabinchha

packages/data-designer/src/data_designer/interface/data_designer.py

@@ -37,7 +37,7 @@
 from data_designer.engine.compiler import compile_data_designer_config
 from data_designer.engine.dataset_builders.dataset_builder import DatasetBuilder
 from data_designer.engine.mcp.io import list_tool_names
-from data_designer.engine.model_provider import resolve_model_provider_registry
+from data_designer.engine.model_provider import ModelProviderRegistry, resolve_model_provider_registry
 from data_designer.engine.resources.person_reader import (
     PersonReader,
     create_person_reader,
@@ -150,11 +150,20 @@ def __init__(
         self._run_config = RunConfig()
         self._managed_assets_path = Path(managed_assets_path or MANAGED_ASSETS_PATH)
         self._person_reader = person_reader
-        self._model_providers = self._resolve_model_providers(model_providers)
+        # Only consult the YAML's `default:` key when we are also falling back to
+        # the YAML's `providers:` list. A user-supplied `model_providers` list
+        # owns its own default (first wins), so the YAML default must not leak
+        # in and either (a) hard-fail validation when the YAML names a provider
+        # absent from the supplied list or (b) silently override the
+        # documented first-wins ordering. See issue #588.
+        if model_providers is None:
+            self._model_providers = self._resolve_model_providers(None)
+            default_provider_name = get_default_provider_name()
+        else:
+            self._model_providers = self._resolve_model_providers(model_providers)
+            default_provider_name = None
         self._mcp_providers = mcp_providers or []
-        self._model_provider_registry = resolve_model_provider_registry(
-            self._model_providers, get_default_provider_name()
-        )
+        self._model_provider_registry = resolve_model_provider_registry(self._model_providers, default_provider_name)
         self._seed_reader_registry = SeedReaderRegistry(readers=seed_readers or DEFAULT_SEED_READERS)
 
     @property
@@ -423,6 +432,32 @@ def secret_resolver(self) -> SecretResolver:
         """
         return self._secret_resolver
 
+    @property
+    def model_provider_registry(self) -> ModelProviderRegistry:
+        """Get the resolved model provider registry.
+
+        Returns:
+            The ModelProviderRegistry containing the providers and default
+            resolved at construction time. The default is taken from the
+            first user-supplied provider when ``model_providers`` was passed
+            to the constructor; otherwise from the YAML's ``default:`` key
+            when set, falling back to the first provider in the YAML list.
+        """
+        return self._model_provider_registry
+
+    @property
+    def run_config(self) -> RunConfig:
+        """Get the runtime configuration applied to dataset generation.
+
+        Returns:
+            The active RunConfig instance. Note that ``RunConfig`` normalizes
+            some fields on construction (e.g., ``shutdown_error_rate`` becomes
+            ``1.0`` when ``disable_early_shutdown=True``), so the returned
+            object may not exactly equal the one originally passed to
+            ``set_run_config``.
+        """
+        return self._run_config
+
     def set_run_config(self, run_config: RunConfig) -> None:
         """Set the runtime configuration for dataset generation.
 

packages/data-designer/tests/interface/test_data_designer.py

@@ -420,17 +420,110 @@ def test_init_with_path_object(stub_artifact_path, stub_model_providers):
     assert designer is not None
 
 
+def test_init_user_supplied_providers_ignore_unrelated_yaml_default(
+    stub_artifact_path: Path,
+    stub_model_providers: list[ModelProvider],
+    stub_managed_assets_path: Path,
+) -> None:
+    """Regression for #588: a YAML ``default:`` that names a provider absent
+    from a user-supplied ``model_providers`` list must not leak into
+    construction.
+
+    Pre-fix this raised ``ValidationError: Specified default 'unrelated' not
+    found in providers list``.
+    """
+    with patch.object(dd_mod, "get_default_provider_name", return_value="unrelated"):
+        data_designer = DataDesigner(
+            artifact_path=stub_artifact_path,
+            model_providers=stub_model_providers,
+            secret_resolver=PlaintextResolver(),
+            managed_assets_path=stub_managed_assets_path,
+        )
+
+    assert data_designer.model_provider_registry.get_default_provider_name() == "stub-model-provider"
+
+
+def test_init_user_supplied_providers_preserve_first_wins_over_yaml_default(
+    stub_artifact_path: Path,
+    stub_managed_assets_path: Path,
+) -> None:
+    """Regression for #588: when the YAML ``default:`` matches a user-supplied
+    provider that isn't first in the list, the documented ``model_providers[0]``
+    "first wins" behavior must not be silently overridden.
+    """
+    user_providers = [
+        ModelProvider(
+            name="first-provider",
+            endpoint="https://first.example.com/v1",
+            api_key="FIRST_API_KEY",
+        ),
+        ModelProvider(
+            name="second-provider",
+            endpoint="https://second.example.com/v1",
+            api_key="SECOND_API_KEY",
+        ),
+    ]
+
+    with patch.object(dd_mod, "get_default_provider_name", return_value="second-provider"):
+        data_designer = DataDesigner(
+            artifact_path=stub_artifact_path,
+            model_providers=user_providers,
+            secret_resolver=PlaintextResolver(),
+            managed_assets_path=stub_managed_assets_path,
+        )
+
+    assert data_designer.model_provider_registry.get_default_provider_name() == "first-provider"
+
+
+def test_init_no_user_providers_uses_yaml_default(
+    stub_artifact_path: Path,
+    stub_managed_assets_path: Path,
+) -> None:
+    """Pin the unchanged YAML-fallback path: when the caller omits
+    ``model_providers``, DataDesigner consults both ``providers:`` and
+    ``default:`` from the YAML.
+
+    The fix in #588 only changes the user-supplied branch; this test locks the
+    YAML-fallback branch's contract so a future refactor can't silently regress
+    it.
+    """
+    yaml_providers = [
+        ModelProvider(
+            name="yaml-first",
+            endpoint="https://yaml-first.example.com/v1",
+            api_key="yaml-first-key",
+        ),
+        ModelProvider(
+            name="yaml-second",
+            endpoint="https://yaml-second.example.com/v1",
+            api_key="yaml-second-key",
+        ),
+    ]
+
+    with (
+        patch.object(dd_mod, "get_default_providers", return_value=yaml_providers),
+        patch.object(dd_mod, "get_default_provider_name", return_value="yaml-second"),
+    ):
+        data_designer = DataDesigner(
+            artifact_path=stub_artifact_path,
+            secret_resolver=PlaintextResolver(),
+            managed_assets_path=stub_managed_assets_path,
+        )
+
+    assert data_designer.model_provider_registry.get_default_provider_name() == "yaml-second"
+
+
 def test_run_config_setting_persists(stub_artifact_path, stub_model_providers):
     """Test that run config setting persists across multiple calls."""
     data_designer = DataDesigner(artifact_path=stub_artifact_path, model_providers=stub_model_providers)
 
     # Test default values
-    assert data_designer._run_config.disable_early_shutdown is False
-    assert data_designer._run_config.shutdown_error_rate == 0.5
-    assert data_designer._run_config.shutdown_error_window == 10
-    assert data_designer._run_config.buffer_size == 1000
-    assert data_designer._run_config.max_conversation_restarts == 5
-    assert data_designer._run_config.max_conversation_correction_steps == 0
+    assert data_designer.run_config.disable_early_shutdown is False
+    assert data_designer.run_config.shutdown_error_rate == 0.5
+    assert data_designer.run_config.shutdown_error_window == 10
+    assert data_designer.run_config.buffer_size == 1000
+    assert data_designer.run_config.max_conversation_restarts == 5
+    assert data_designer.run_config.max_conversation_correction_steps == 0
 
     # Test setting custom values
     data_designer.set_run_config(
@@ -443,12 +536,12 @@ def test_run_config_setting_persists(stub_artifact_path, stub_model_providers):
             max_conversation_correction_steps=2,
         )
     )
-    assert data_designer._run_config.disable_early_shutdown is True
-    assert data_designer._run_config.shutdown_error_rate == 1.0  # normalized when disabled
-    assert data_designer._run_config.shutdown_error_window == 25
-    assert data_designer._run_config.buffer_size == 500
-    assert data_designer._run_config.max_conversation_restarts == 7
-    assert data_designer._run_config.max_conversation_correction_steps == 2
+    assert data_designer.run_config.disable_early_shutdown is True
+    assert data_designer.run_config.shutdown_error_rate == 1.0  # normalized when disabled
+    assert data_designer.run_config.shutdown_error_window == 25
+    assert data_designer.run_config.buffer_size == 500
+    assert data_designer.run_config.max_conversation_restarts == 7
+    assert data_designer.run_config.max_conversation_correction_steps == 2
 
     # Test updating values
     data_designer.set_run_config(
@@ -461,12 +554,12 @@ def test_run_config_setting_persists(stub_artifact_path, stub_model_providers):
             max_conversation_correction_steps=1,
         )
     )
-    assert data_designer._run_config.disable_early_shutdown is False
-    assert data_designer._run_config.shutdown_error_rate == 0.3
-    assert data_designer._run_config.shutdown_error_window == 5
-    assert data_designer._run_config.buffer_size == 750
-    assert data_designer._run_config.max_conversation_restarts == 9
-    assert data_designer._run_config.max_conversation_correction_steps == 1
+    assert data_designer.run_config.disable_early_shutdown is False
+    assert data_designer.run_config.shutdown_error_rate == 0.3
+    assert data_designer.run_config.shutdown_error_window == 5
+    assert data_designer.run_config.buffer_size == 750
+    assert data_designer.run_config.max_conversation_restarts == 9
+    assert data_designer.run_config.max_conversation_correction_steps == 1
 
 
 def test_run_config_normalizes_error_rate_when_disabled(stub_artifact_path, stub_model_providers):
@@ -480,7 +573,7 @@ def test_run_config_normalizes_error_rate_when_disabled(stub_artifact_path, stub
             shutdown_error_rate=0.7,
         )
     )
-    assert data_designer._run_config.shutdown_error_rate == 0.7
+    assert data_designer.run_config.shutdown_error_rate == 0.7
 
     # When disabled, shutdown_error_rate should be normalized to 1.0
     data_designer.set_run_config(
@@ -489,7 +582,7 @@ def test_run_config_normalizes_error_rate_when_disabled(stub_artifact_path, stub
             shutdown_error_rate=0.7,
         )
     )
-    assert data_designer._run_config.shutdown_error_rate == 1.0
+    assert data_designer.run_config.shutdown_error_rate == 1.0
 
 
 def test_run_config_rejects_invalid_buffer_size() -> None:
@@ -858,13 +951,12 @@ def test_create_logs_secure_jinja_rendering_mode(
     stub_sampler_only_config_builder: DataDesignerConfigBuilder,
     stub_managed_assets_path: Path,
 ) -> None:
-    with patch.object(dd_mod, "get_default_provider_name", return_value="stub-model-provider"):
-        data_designer = DataDesigner(
-            artifact_path=stub_artifact_path,
-            model_providers=stub_model_providers,
-            secret_resolver=PlaintextResolver(),
-            managed_assets_path=stub_managed_assets_path,
-        )
+    data_designer = DataDesigner(
+        artifact_path=stub_artifact_path,
+        model_providers=stub_model_providers,
+        secret_resolver=PlaintextResolver(),
+        managed_assets_path=stub_managed_assets_path,
+    )
     data_designer.set_run_config(RunConfig(jinja_rendering_engine=JinjaRenderingEngine.SECURE))
 
     with (
@@ -898,13 +990,12 @@ def test_preview_logs_native_jinja_rendering_mode(
     stub_sampler_only_config_builder: DataDesignerConfigBuilder,
     stub_managed_assets_path: Path,
 ) -> None:
-    with patch.object(dd_mod, "get_default_provider_name", return_value="stub-model-provider"):
-        data_designer = DataDesigner(
-            artifact_path=stub_artifact_path,
-            model_providers=stub_model_providers,
-            secret_resolver=PlaintextResolver(),
-            managed_assets_path=stub_managed_assets_path,
-        )
+    data_designer = DataDesigner(
+        artifact_path=stub_artifact_path,
+        model_providers=stub_model_providers,
+        secret_resolver=PlaintextResolver(),
+        managed_assets_path=stub_managed_assets_path,
+    )
     data_designer.set_run_config(RunConfig(jinja_rendering_engine=JinjaRenderingEngine.NATIVE))
 
     with (