Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.

Expand Down
159 changes: 125 additions & 34 deletions packages/data-designer/tests/interface/test_data_designer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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):
Expand All @@ -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(
Expand All @@ -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:
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -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 (
Expand Down
Loading