Fix deprecated kwargs validation to prevent silent failures #2047
Conversation
- Replace .get() with .pop() for deprecated kwargs in Agent methods - Add validation to raise UserError for unknown kwargs after processing deprecated ones - Prevents silent acceptance of invalid kwargs like usage_limits in Agent() - Applied fix to Agent.__init__, run, run_sync, iter, and run_sync methods - Added comprehensive tests for validation behavior - Maintains backward compatibility for legitimate deprecated kwargs Fixes: pydantic#1987
svilupp
changed the title
Add type: ignore[call-arg] comments to test lines that intentionally pass invalid kwargs to bypass type checker while preserving runtime validation tests.
…d-kwargs-wrong-kwarg-handling
|
The failure is unrelated to the PR (it was there before):
|
|
@svilupp Can you please fix them regardless? They're only showing up now because we only validate coverage when a method is actually edited. |
Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
…tps://github.com/svilupp/pydantic-ai into devin/fix-deprecated-kwargs-wrong-kwarg-handling
…d-kwargs-wrong-kwarg-handling
I tried and it throws an error that coverage is not 100% and two lines are missing (299, 325) -- which marks the block that should raise TypeError (not related to this PR). If I add the no-cover shield back, I get: Run uv run strict-no-cover Not sure what the best way to proceed is -- I could set up the tests for those lines, but it's unrelated to the PR... |
| @@ -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_no_deprecated_kwargs(_deprecated_kwargs: dict[str, Any]) -> None: | |||
There was a problem hiding this comment.
Can we drop deprecated from the names and description? If there are remaining items in the dict here, they wouldn't be deprecated kwargs, just unknown kwargs.
|
@svilupp I fixed the coverage issue by moving the pragma statement to the |
Thank you! I've updated the function name and args as suggested. |
|
@svilupp Thanks you! |
Fixes: #1987