Skip to content

Commit 1c7c0f8

Browse files
committed
fix(models): apply warn_at_caller to remaining deprecation sites
greptile-apps (PR #594, r3189904028): `ProviderRepository.load`'s YAML-default `DeprecationWarning` was using `warnings.warn(stacklevel=2)`, which attributes to whichever data_designer frame called `load()` — controllers, services, list/reset commands, agent introspection. Every real call path lands on `data_designer.cli.*`, which falls under Python's default `ignore::DeprecationWarning` filter and is silenced. Audit found two more sites with the same problem: - `DatasetBuilder._resolve_async_compatibility` (`allow_resize` / issue #552) — was using `stacklevel=4` to walk past `_resolve_async_compatibility -> build/build_preview -> interface -> user`. Brittle: any added frame (decorator, async wrapping, the `try/except DeprecationWarning: raise` boundary) shifts attribution silently. The existing test passed only because it used `simplefilter("always") + record=True`, which records warnings regardless of attribution. - `ProviderController._handle_change_default` — was using `stacklevel=2`, which lands on the menu dispatcher in the same controller module. `print_warning` already shows the message visually, but programmatic observers (`pytest.warns`, `filterwarnings("error", ...)`) saw a library-attributed entry that default filters silenced. All three migrated to `warn_at_caller` (the helper from 247fa30) so attribution lands on the user's call site regardless of internal chain shape. `data_designer` is already in `DEFAULT_INTERNAL_PREFIXES`, so the walk escapes the entire library in one pass. Added attribution regression tests at each site asserting `warning.filename == __file__`. A future regression to `warnings.warn(stacklevel=N)` now fails CI instead of silently silencing the user-facing nudge: - `test_load_with_yaml_default_attributes_warning_to_caller` (test_provider_repository.py) - `test_resolve_async_compatibility` extended with the same assertion - `test_handle_change_default_emits_deprecation_warning` rewritten from `pytest.warns(...)` to a `catch_warnings(record=True)` block that filters for the message and asserts `filename == __file__` (`pytest.warns` does not check attribution, so the rewrite is required to actually catch the regression). 3,125 tests pass (548 config + 1,923 engine + 654 interface). Refs #589
1 parent 247fa30 commit 1c7c0f8

6 files changed

Lines changed: 84 additions & 9 deletions

File tree

packages/data-designer-engine/src/data_designer/engine/dataset_builders/dataset_builder.py

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import os
1010
import time
1111
import uuid
12-
import warnings
1312
from pathlib import Path
1413
from typing import TYPE_CHECKING, Any, Callable
1514

@@ -23,6 +22,7 @@
2322
ProcessorConfig,
2423
ProcessorType,
2524
)
25+
from data_designer.config.utils.warning_helpers import warn_at_caller
2626
from data_designer.config.version import get_library_version
2727
from data_designer.engine.column_generators.generators.base import (
2828
ColumnGenerator,
@@ -319,7 +319,17 @@ def _resolve_async_compatibility(self) -> bool:
319319
"use workflow chaining instead (see issue #552)."
320320
)
321321
logger.warning(f"⚠️ {msg}")
322-
warnings.warn(msg, DeprecationWarning, stacklevel=4)
322+
# ``warn_at_caller`` rather than ``warnings.warn(stacklevel=N)`` so
323+
# attribution lands on the user's call site instead of an internal
324+
# ``DatasetBuilder.build`` / ``data_designer.interface`` frame.
325+
# The exact internal-frame depth from this method up to user code
326+
# depends on which entry point invoked the builder (build vs.
327+
# build_preview, sync vs. async wrapping), so a hard-coded
328+
# ``stacklevel`` is brittle; ``warn_at_caller`` walks past every
329+
# ``data_designer.*`` frame regardless of chain shape. Library
330+
# attribution would also be silenced under Python's default
331+
# ``ignore::DeprecationWarning`` filter. See PR #594 review.
332+
warn_at_caller(msg, DeprecationWarning)
323333
return False
324334
return True
325335

packages/data-designer-engine/tests/engine/dataset_builders/test_async_builder_integration.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,14 @@ def test_resolve_async_compatibility(configs: list[Mock], expected: bool) -> Non
101101
assert len(w) == 1
102102
assert issubclass(w[0].category, DeprecationWarning)
103103
assert "allow_resize" in str(w[0].message)
104+
# Regression for PR #594 review: the warning must attribute to the
105+
# caller's frame (this test file), not to a ``data_designer.*`` library
106+
# frame. Library-attributed ``DeprecationWarning`` entries fall under
107+
# Python's default ``ignore::DeprecationWarning`` filter and are
108+
# silenced. A regression to ``warnings.warn(..., stacklevel=N)`` would
109+
# land somewhere inside the engine package and silently break the
110+
# user-facing nudge.
111+
assert w[0].filename == __file__
104112
else:
105113
assert len(w) == 0
106114

packages/data-designer/src/data_designer/cli/controllers/provider_controller.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
import copy
77
import re
8-
import warnings
98
from pathlib import Path
109
from typing import TYPE_CHECKING
1110

@@ -28,6 +27,7 @@
2827
print_warning,
2928
select_with_arrows,
3029
)
30+
from data_designer.config.utils.warning_helpers import warn_at_caller
3131

3232
if TYPE_CHECKING:
3333
from data_designer.engine.model_provider import ModelProvider
@@ -295,7 +295,15 @@ def _handle_change_default(self) -> None:
295295
"instead of relying on a registry-level default. See issue #589."
296296
)
297297
print_warning(deprecation_msg)
298-
warnings.warn(deprecation_msg, DeprecationWarning, stacklevel=2)
298+
# ``print_warning`` always shows the user the message in the console,
299+
# but ``warnings.warn`` is what's observable to programmatic callers
300+
# (``pytest.warns``, ``filterwarnings("error", ...)``). With
301+
# ``stacklevel=2`` attribution lands on the menu dispatcher in this
302+
# same module — a ``data_designer.cli.*`` frame — and Python's default
303+
# ``ignore::DeprecationWarning`` filter silences it. ``warn_at_caller``
304+
# walks past every ``data_designer.*`` frame so the warning attributes
305+
# to the user's call site and stays visible. See PR #594 review.
306+
warn_at_caller(deprecation_msg, DeprecationWarning)
299307
console.print()
300308

301309
providers = self.service.list_all()

packages/data-designer/src/data_designer/cli/repositories/provider_repository.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
from __future__ import annotations
55

6-
import warnings
76
from pathlib import Path
87

98
from pydantic import BaseModel
@@ -12,6 +11,7 @@
1211
from data_designer.config.models import ModelProvider
1312
from data_designer.config.utils.constants import MODEL_PROVIDERS_FILE_NAME
1413
from data_designer.config.utils.io_helpers import load_config_file, save_config_file
14+
from data_designer.config.utils.warning_helpers import warn_at_caller
1515

1616

1717
class ModelProviderRegistry(BaseModel):
@@ -43,14 +43,17 @@ def load(self) -> ModelProviderRegistry | None:
4343
# ``DeprecationWarning`` is an ``Exception`` subclass, so under
4444
# ``filterwarnings("error", DeprecationWarning)`` a warn raised inside
4545
# the catch-all would be silently swallowed and ``load`` would drop the
46-
# registry. See PR #594 review.
46+
# registry. ``warn_at_caller`` (rather than ``warnings.warn(stacklevel=2)``)
47+
# so the warning attributes to the user's call site rather than a
48+
# ``data_designer.cli.*`` frame; under default Python filters,
49+
# library-attributed ``DeprecationWarning`` entries are silenced
50+
# (``ignore::DeprecationWarning``). See PR #594 review.
4751
if config_dict.get("default") is not None:
48-
warnings.warn(
52+
warn_at_caller(
4953
f"The 'default:' key in {self.config_file} is deprecated and will "
5054
"be removed in a future release. Remove it and specify provider= "
5155
"explicitly on each ModelConfig instead. See issue #589.",
5256
DeprecationWarning,
53-
stacklevel=2,
5457
)
5558

5659
try:

packages/data-designer/tests/cli/controllers/test_provider_controller.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
# SPDX-License-Identifier: Apache-2.0
33

4+
import warnings
45
from pathlib import Path
56
from unittest.mock import MagicMock, patch
67

@@ -216,12 +217,29 @@ def test_handle_change_default_emits_deprecation_warning(
216217
"""Regression for #589: entering the 'Change default provider' workflow
217218
must emit a ``DeprecationWarning`` so users see the migration nudge before
218219
setting another value that's also slated for removal.
220+
221+
Also pins the attribution contract from PR #594 review: the warning must
222+
land on the caller's frame (this test file), not on a
223+
``data_designer.cli.*`` library frame. Library attribution falls under
224+
Python's default ``ignore::DeprecationWarning`` filter and would silently
225+
suppress the user-facing nudge for any caller that isn't using
226+
``simplefilter("always")``.
219227
"""
220228
mock_select.side_effect = ["change_default", "test-provider-2"]
221229

222-
with pytest.warns(DeprecationWarning, match="'Change default provider' workflow is deprecated"):
230+
with warnings.catch_warnings(record=True) as caught:
231+
warnings.simplefilter("always")
223232
controller_with_providers.run()
224233

234+
deprecations = [
235+
w
236+
for w in caught
237+
if issubclass(w.category, DeprecationWarning)
238+
and "'Change default provider' workflow is deprecated" in str(w.message)
239+
]
240+
assert len(deprecations) == 1
241+
assert deprecations[0].filename == __file__
242+
225243

226244
@patch("data_designer.cli.controllers.provider_controller.confirm_action", return_value=False)
227245
@patch("data_designer.cli.controllers.provider_controller.select_with_arrows")

packages/data-designer/tests/cli/repositories/test_provider_repository.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,34 @@ def test_load_with_yaml_default_emits_deprecation_warning(
6767
assert registry.default == stub_model_providers[0].name
6868

6969

70+
def test_load_with_yaml_default_attributes_warning_to_caller(
71+
tmp_path: Path, stub_model_providers: list[ModelProvider]
72+
) -> None:
73+
"""Regression for PR #594 review: the YAML-default ``DeprecationWarning``
74+
must attribute to the *caller's* frame (this test file), not to a
75+
``data_designer.cli.*`` library frame. Library-attributed
76+
``DeprecationWarning`` entries fall under Python's default
77+
``ignore::DeprecationWarning`` filter and are silenced, so attribution at
78+
a library frame == invisible warning. ``warn_at_caller`` keeps this
79+
visible; a regression to ``warnings.warn(stacklevel=2)`` would land on
80+
``provider_repository.py`` and silently break the user nudge.
81+
"""
82+
providers_file_path = tmp_path / MODEL_PROVIDERS_FILE_NAME
83+
save_config_file(
84+
providers_file_path,
85+
ModelProviderRegistry(providers=stub_model_providers, default=stub_model_providers[0].name).model_dump(),
86+
)
87+
repository = ProviderRepository(tmp_path)
88+
89+
with warnings.catch_warnings(record=True) as caught:
90+
warnings.simplefilter("always")
91+
repository.load()
92+
93+
deprecations = [w for w in caught if issubclass(w.category, DeprecationWarning)]
94+
assert len(deprecations) == 1
95+
assert deprecations[0].filename == __file__
96+
97+
7098
def test_load_without_yaml_default_does_not_warn(tmp_path: Path, stub_model_providers: list[ModelProvider]) -> None:
7199
"""Pin the post-deprecation happy path: a YAML without a ``default:`` key
72100
must load cleanly with no ``DeprecationWarning``.

0 commit comments

Comments
 (0)