Fix deprecated kwargs validation to prevent silent failures (#2047)

Co-authored-by: J S <jan@swap-commerce.com>
Co-authored-by: Douwe Maan <douwe@pydantic.dev>

Author: 3 people

pydantic_ai_slim/pydantic_ai/_utils.py

@@ -31,6 +31,8 @@
 
 from pydantic_graph._utils import AbstractSpan
 
+from . import exceptions
+
 AbstractSpan = AbstractSpan
 
 if TYPE_CHECKING:
@@ -415,6 +417,20 @@ def merge_json_schema_defs(schemas: list[dict[str, Any]]) -> tuple[list[dict[str
     return rewritten_schemas, all_defs
 
 
+def validate_empty_kwargs(_kwargs: dict[str, Any]) -> None:
+    """Validate that no unknown kwargs remain after processing.
+
+    Args:
+        _kwargs: Dictionary of remaining kwargs after specific ones have been processed.
+
+    Raises:
+        UserError: If any unknown kwargs remain.
+    """
+    if _kwargs:
+        unknown_kwargs = ', '.join(f'`{k}`' for k in _kwargs.keys())
+        raise exceptions.UserError(f'Unknown keyword arguments: {unknown_kwargs}')
+
+
 def strip_markdown_fences(text: str) -> str:
     if text.startswith('{'):
         return text

pydantic_ai_slim/pydantic_ai/agent.py

@@ -294,41 +294,43 @@ def __init__(
         self.name = name
         self.model_settings = model_settings
 
-        if 'result_type' in _deprecated_kwargs:  # pragma: no cover
-            if output_type is not str:
+        if 'result_type' in _deprecated_kwargs:
+            if output_type is not str:  # pragma: no cover
                 raise TypeError('`result_type` and `output_type` cannot be set at the same time.')
             warnings.warn('`result_type` is deprecated, use `output_type` instead', DeprecationWarning)
-            output_type = _deprecated_kwargs['result_type']
+            output_type = _deprecated_kwargs.pop('result_type')
 
         self.output_type = output_type
 
         self.instrument = instrument
 
         self._deps_type = deps_type
 
-        self._deprecated_result_tool_name = _deprecated_kwargs.get('result_tool_name')
-        if self._deprecated_result_tool_name is not None:  # pragma: no cover
+        self._deprecated_result_tool_name = _deprecated_kwargs.pop('result_tool_name', None)
+        if self._deprecated_result_tool_name is not None:
             warnings.warn(
                 '`result_tool_name` is deprecated, use `output_type` with `ToolOutput` instead',
                 DeprecationWarning,
             )
 
-        self._deprecated_result_tool_description = _deprecated_kwargs.get('result_tool_description')
-        if self._deprecated_result_tool_description is not None:  # pragma: no cover
+        self._deprecated_result_tool_description = _deprecated_kwargs.pop('result_tool_description', None)
+        if self._deprecated_result_tool_description is not None:
             warnings.warn(
                 '`result_tool_description` is deprecated, use `output_type` with `ToolOutput` instead',
                 DeprecationWarning,
             )
-        result_retries = _deprecated_kwargs.get('result_retries')
-        if result_retries is not None:  # pragma: no cover
-            if output_retries is not None:
+        result_retries = _deprecated_kwargs.pop('result_retries', None)
+        if result_retries is not None:
+            if output_retries is not None:  # pragma: no cover
                 raise TypeError('`output_retries` and `result_retries` cannot be set at the same time.')
             warnings.warn('`result_retries` is deprecated, use `max_result_retries` instead', DeprecationWarning)
             output_retries = result_retries
 
         default_output_mode = (
             self.model.profile.default_structured_output_mode if isinstance(self.model, models.Model) else None
         )
+        _utils.validate_empty_kwargs(_deprecated_kwargs)
+
         self._output_schema = _output.OutputSchema[OutputDataT].build(
             output_type,
             default_mode=default_output_mode,
@@ -469,7 +471,9 @@ async def main():
             if output_type is not str:
                 raise TypeError('`result_type` and `output_type` cannot be set at the same time.')
             warnings.warn('`result_type` is deprecated, use `output_type` instead.', DeprecationWarning)
-            output_type = _deprecated_kwargs['result_type']
+            output_type = _deprecated_kwargs.pop('result_type')
+
+        _utils.validate_empty_kwargs(_deprecated_kwargs)
 
         async with self.iter(
             user_prompt=user_prompt,
@@ -635,7 +639,9 @@ async def main():
             if output_type is not str:
                 raise TypeError('`result_type` and `output_type` cannot be set at the same time.')
             warnings.warn('`result_type` is deprecated, use `output_type` instead.', DeprecationWarning)
-            output_type = _deprecated_kwargs['result_type']
+            output_type = _deprecated_kwargs.pop('result_type')
+
+        _utils.validate_empty_kwargs(_deprecated_kwargs)
 
         deps = self._get_deps(deps)
         new_message_index = len(message_history) if message_history else 0
@@ -872,7 +878,9 @@ def run_sync(
             if output_type is not str:
                 raise TypeError('`result_type` and `output_type` cannot be set at the same time.')
             warnings.warn('`result_type` is deprecated, use `output_type` instead.', DeprecationWarning)
-            output_type = _deprecated_kwargs['result_type']
+            output_type = _deprecated_kwargs.pop('result_type')
+
+        _utils.validate_empty_kwargs(_deprecated_kwargs)
 
         return get_event_loop().run_until_complete(
             self.run(
@@ -988,7 +996,9 @@ async def main():
             if output_type is not str:
                 raise TypeError('`result_type` and `output_type` cannot be set at the same time.')
             warnings.warn('`result_type` is deprecated, use `output_type` instead.', DeprecationWarning)
-            output_type = _deprecated_kwargs['result_type']
+            output_type = _deprecated_kwargs.pop('result_type')
+
+        _utils.validate_empty_kwargs(_deprecated_kwargs)
 
         yielded = False
         async with self.iter(

tests/test_agent.py

@@ -3125,3 +3125,79 @@ def hello(_: list[ModelMessage], _info: AgentInfo) -> ModelResponse:
 
     with pytest.raises(UserError, match='Output tools are not supported by the model.'):
         agent.run_sync('Hello')
+
+
+def test_deprecated_kwargs_validation_agent_init():
+    """Test that invalid kwargs raise UserError in Agent constructor."""
+    with pytest.raises(UserError, match='Unknown keyword arguments: `usage_limits`'):
+        Agent('test', usage_limits='invalid')  # type: ignore[call-arg]
+
+    with pytest.raises(UserError, match='Unknown keyword arguments: `invalid_kwarg`'):
+        Agent('test', invalid_kwarg='value')  # type: ignore[call-arg]
+
+    with pytest.raises(UserError, match='Unknown keyword arguments: `foo`, `bar`'):
+        Agent('test', foo='value1', bar='value2')  # type: ignore[call-arg]
+
+
+def test_deprecated_kwargs_validation_agent_run():
+    """Test that invalid kwargs raise UserError in Agent.run method."""
+    agent = Agent('test')
+
+    with pytest.raises(UserError, match='Unknown keyword arguments: `invalid_kwarg`'):
+        agent.run_sync('test', invalid_kwarg='value')  # type: ignore[call-arg]
+
+    with pytest.raises(UserError, match='Unknown keyword arguments: `foo`, `bar`'):
+        agent.run_sync('test', foo='value1', bar='value2')  # type: ignore[call-arg]
+
+
+def test_deprecated_kwargs_still_work():
+    """Test that valid deprecated kwargs still work with warnings."""
+    import warnings
+
+    with warnings.catch_warnings(record=True) as w:
+        warnings.simplefilter('always')
+
+        agent = Agent('test', result_type=str)  # type: ignore[call-arg]
+        assert len(w) == 1
+        assert issubclass(w[0].category, DeprecationWarning)
+        assert '`result_type` is deprecated' in str(w[0].message)
+        assert agent.output_type is str
+
+    with warnings.catch_warnings(record=True) as w:
+        warnings.simplefilter('always')
+
+        agent = Agent('test', result_tool_name='test_tool')  # type: ignore[call-arg]
+        assert len(w) == 1
+        assert issubclass(w[0].category, DeprecationWarning)
+        assert '`result_tool_name` is deprecated' in str(w[0].message)
+
+    with warnings.catch_warnings(record=True) as w:
+        warnings.simplefilter('always')
+
+        agent = Agent('test', result_tool_description='test description')  # type: ignore[call-arg]
+        assert len(w) == 1
+        assert issubclass(w[0].category, DeprecationWarning)
+        assert '`result_tool_description` is deprecated' in str(w[0].message)
+
+    with warnings.catch_warnings(record=True) as w:
+        warnings.simplefilter('always')
+
+        agent = Agent('test', result_retries=3)  # type: ignore[call-arg]
+        assert len(w) == 1
+        assert issubclass(w[0].category, DeprecationWarning)
+        assert '`result_retries` is deprecated' in str(w[0].message)
+
+
+def test_deprecated_kwargs_mixed_valid_invalid():
+    """Test that mix of valid deprecated and invalid kwargs raises error for invalid ones."""
+    import warnings
+
+    with pytest.raises(UserError, match='Unknown keyword arguments: `usage_limits`'):
+        with warnings.catch_warnings():
+            warnings.simplefilter('ignore', DeprecationWarning)  # Ignore the deprecation warning for result_type
+            Agent('test', result_type=str, usage_limits='invalid')  # type: ignore[call-arg]
+
+    with pytest.raises(UserError, match='Unknown keyword arguments: `foo`, `bar`'):
+        with warnings.catch_warnings():
+            warnings.simplefilter('ignore', DeprecationWarning)  # Ignore the deprecation warning for result_tool_name
+            Agent('test', result_tool_name='test', foo='value1', bar='value2')  # type: ignore[call-arg]

tests/test_utils.py

@@ -20,6 +20,7 @@
     merge_json_schema_defs,
     run_in_executor,
     strip_markdown_fences,
+    validate_empty_kwargs,
 )
 
 from .models.mock_async_stream import MockAsyncStream
@@ -503,3 +504,40 @@ def test_strip_markdown_fences():
         == '{"foo": "bar"}'
     )
     assert strip_markdown_fences('No JSON to be found') == 'No JSON to be found'
+
+
+def test_validate_empty_kwargs_empty():
+    """Test that empty dict passes validation."""
+    validate_empty_kwargs({})
+
+
+def test_validate_empty_kwargs_with_unknown():
+    """Test that unknown kwargs raise UserError."""
+    with pytest.raises(UserError, match='Unknown keyword arguments: `unknown_arg`'):
+        validate_empty_kwargs({'unknown_arg': 'value'})
+
+
+def test_validate_empty_kwargs_multiple_unknown():
+    """Test that multiple unknown kwargs are properly formatted."""
+    with pytest.raises(UserError, match='Unknown keyword arguments: `arg1`, `arg2`'):
+        validate_empty_kwargs({'arg1': 'value1', 'arg2': 'value2'})
+
+
+def test_validate_empty_kwargs_message_format():
+    """Test that the error message format matches expected pattern."""
+    with pytest.raises(UserError) as exc_info:
+        validate_empty_kwargs({'test_arg': 'test_value'})
+
+    assert 'Unknown keyword arguments: `test_arg`' in str(exc_info.value)
+
+
+def test_validate_empty_kwargs_preserves_order():
+    """Test that multiple kwargs preserve order in error message."""
+    kwargs = {'first': '1', 'second': '2', 'third': '3'}
+    with pytest.raises(UserError) as exc_info:
+        validate_empty_kwargs(kwargs)
+
+    error_msg = str(exc_info.value)
+    assert '`first`' in error_msg
+    assert '`second`' in error_msg
+    assert '`third`' in error_msg