Skip to content

Commit 3c8394e

Browse files
authored
fix(interface): silence registry-default deprecation when library auto-fills it (#655)
The ``ModelProviderRegistry.default is deprecated`` warning added in #594 fires for every fresh-install ``DataDesigner()`` construction, even when the user wrote ``default=`` nowhere — neither in YAML, nor in Python, nor in any ``ModelConfig``. Root cause: ``resolve_model_provider_registry`` synthesises ``default=providers[0].name`` for the multi-provider case to satisfy ``check_implicit_default``. The auto-seeded ``~/.data-designer/model_providers.yaml`` ships three providers and no ``default:`` key, so this path is hit on every bare ``DataDesigner()`` call. ``_warn_on_explicit_default`` then attributes the warning to the user's ``DataDesigner()`` line, with a remediation message ("Specify provider= explicitly on each ModelConfig") that doesn't even apply when the user hasn't built a ``ModelConfig`` (e.g. a UUID-only sampler config with the GitHub plugin). Fix: broaden the existing warning suppression in ``DataDesigner.__init__`` to also cover the ``model_providers is None`` case. The user is opting into all defaults — the library is the one filling ``default=``, so the deprecation nudge is misdirected. Users who hand-construct a multi-provider list in Python still see the warning (they wrote the multi-provider intent themselves), and direct ``ModelProviderRegistry(default="x")`` always warns — those are the entry points #589 actually targets. New regression test pins the bare-``DataDesigner()`` quiet path so a future tightening of the suppression can't silently re-introduce the spurious warning. Refs #589, follow-up to #594. Signed-off-by: Nabin Mulepati <nmulepati@nvidia.com>
1 parent ef761b8 commit 3c8394e

2 files changed

Lines changed: 78 additions & 6 deletions

File tree

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

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -166,13 +166,31 @@ def __init__(
166166
self._model_providers = self._resolve_model_providers(model_providers)
167167
default_provider_name = None
168168
self._mcp_providers = mcp_providers or []
169-
# When the YAML carries a default, ``get_default_provider_name`` already
170-
# nudged the user with a ``DeprecationWarning``. Building the registry
171-
# below would re-fire ``ModelProviderRegistry._warn_on_explicit_default``
172-
# for the same root cause, so suppress that second warning. See PR #594
173-
# review.
169+
# Suppress ``ModelProviderRegistry._warn_on_explicit_default`` whenever
170+
# *we* are filling ``default=`` on the user's behalf rather than the
171+
# user actively opting into the deprecated registry-level default. Two
172+
# such cases:
173+
# 1. ``model_providers is None`` — the caller passed nothing, so we
174+
# load the YAML's ``providers:`` list and (in the multi-provider
175+
# case) ``resolve_model_provider_registry`` synthesises
176+
# ``default=providers[0].name`` to satisfy ``check_implicit_default``.
177+
# The fresh-install YAML ships three providers and no ``default:``
178+
# key, so this fires for every default ``DataDesigner()``
179+
# construction. The user has no actionable lever here, and the
180+
# warning's "Specify provider= on each ModelConfig" remediation
181+
# doesn't apply when they haven't built a ``ModelConfig`` at all.
182+
# 2. ``default_provider_name is not None`` — the YAML carried a
183+
# ``default:`` key and ``get_default_provider_name`` already
184+
# emitted the YAML-level ``DeprecationWarning``. The registry
185+
# warning would fire for the same root cause, so suppress it to
186+
# avoid double-warning. See PR #594 review.
187+
# Users who hand-construct a multi-provider list in Python still see
188+
# the warning (they wrote the multi-provider intent themselves), and
189+
# users who hand-construct ``ModelProviderRegistry(default=...)``
190+
# directly always see it — those are the entry points #589 targets.
191+
library_synthesised_default = model_providers is None or default_provider_name is not None
174192
with warnings.catch_warnings():
175-
if default_provider_name is not None:
193+
if library_synthesised_default:
176194
warnings.filterwarnings(
177195
"ignore",
178196
message="ModelProviderRegistry.default is deprecated",

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,60 @@ def test_init_yaml_default_emits_single_deprecation_warning(
654654
)
655655

656656

657+
def test_init_no_user_providers_no_yaml_default_stays_quiet(
658+
stub_artifact_path: Path,
659+
stub_managed_assets_path: Path,
660+
) -> None:
661+
"""Pin the bare-``DataDesigner()`` happy path: when the caller passes
662+
nothing and the YAML carries multiple ``providers:`` but no ``default:``
663+
key, ``resolve_model_provider_registry`` synthesises
664+
``default=providers[0].name`` to satisfy ``check_implicit_default``. The
665+
user did not opt into the deprecated registry-level default — the library
666+
filled it in on their behalf — so ``_warn_on_explicit_default`` must stay
667+
quiet. The fresh-install YAML ships exactly this shape (3 providers, no
668+
``default:``), so a regression here is what every user sees on their first
669+
``DataDesigner()`` call.
670+
671+
Counterpart to ``test_init_user_supplied_providers_preserve_first_wins_over_yaml_default``,
672+
which pins that the warning *does* fire when the caller hand-builds a
673+
multi-provider list themselves (they wrote the multi-provider intent, so
674+
the deprecation nudge applies).
675+
"""
676+
yaml_providers = [
677+
ModelProvider(
678+
name="yaml-first",
679+
endpoint="https://yaml-first.example.com/v1",
680+
api_key="yaml-first-key",
681+
),
682+
ModelProvider(
683+
name="yaml-second",
684+
endpoint="https://yaml-second.example.com/v1",
685+
api_key="yaml-second-key",
686+
),
687+
]
688+
689+
with warnings.catch_warnings(record=True) as caught:
690+
warnings.simplefilter("always", DeprecationWarning)
691+
with (
692+
patch.object(dd_mod, "get_default_providers", return_value=yaml_providers),
693+
patch.object(dd_mod, "get_default_provider_name", return_value=None),
694+
):
695+
data_designer = DataDesigner(
696+
artifact_path=stub_artifact_path,
697+
secret_resolver=PlaintextResolver(),
698+
managed_assets_path=stub_managed_assets_path,
699+
)
700+
701+
deprecation_messages = [str(w.message) for w in caught if issubclass(w.category, DeprecationWarning)]
702+
registry_default_warnings = [m for m in deprecation_messages if "ModelProviderRegistry.default is deprecated" in m]
703+
assert registry_default_warnings == [], (
704+
"Library-synthesised default must not emit the registry-level deprecation; "
705+
f"the user did not opt into it. Saw: {deprecation_messages}"
706+
)
707+
# Behavioral pin: first-wins still resolves correctly.
708+
assert data_designer.model_provider_registry.get_default_provider_name() == "yaml-first"
709+
710+
657711
def test_run_config_setting_persists(stub_artifact_path, stub_model_providers):
658712
"""Test that run config setting persists across multiple calls."""
659713
data_designer = DataDesigner(artifact_path=stub_artifact_path, model_providers=stub_model_providers)

0 commit comments

Comments
 (0)