Ashfaqbs opened a new pull request, #1051:
URL: https://github.com/apache/flink-agents/pull/1051

   ### What is the purpose of the change
   
   `BaseChatModelConnection` and `BaseChatModelSetup` inherit pydantic's 
default `extra="ignore"` behavior, and every concrete chat-model 
connection/setup forwards `**kwargs` straight into `super().__init__()`. An 
unknown or misspelled constructor argument -- whether passed directly or via 
YAML config through `PythonResourceProvider` -- is silently dropped instead of 
raising, so a typo'd or unsupported config key appears to apply but has no 
effect.
   
   This PR sets `extra="forbid"` on both base classes so an unrecognized key 
raises a clear `ValidationError` naming it, per `code_review.md`'s guidance 
that invalid input should be rejected with a clear error.
   
   Fixes #1050.
   
   ### Brief change log
   
   - `python/flink_agents/api/chat_models/chat_model.py`: set `model_config = 
ConfigDict(arbitrary_types_allowed=True, extra="forbid")` on 
`BaseChatModelConnection` and `BaseChatModelSetup`; declare `name: str | None` 
(excluded from serialization) on both, since callers and the resource provider 
commonly pass a resource `name` through that isn't itself a connection/setup 
field.
   - `python/flink_agents/integrations/chat_models/ollama_chat_model.py`: 
remove `OllamaChatModelSetup`'s `request_timeout` constructor parameter. It was 
forwarded to `super().__init__()` but never declared as a field on that class, 
so with `extra="ignore"` it silently did nothing even before this change 
(`request_timeout` is a real, used field only on `OllamaChatModelConnection`). 
Enabling `extra="forbid"` turned this pre-existing dead parameter into a hard 
failure in tests, which is what surfaced it.
   - `python/flink_agents/api/chat_models/tests/test_chat_model_base.py`: 
regression tests asserting an unknown constructor argument raises 
`ValidationError` on both base classes, and that `name` is still accepted 
without being stored.
   
   ### Tests
   
   - Added unit tests (above) verifying the new rejection behavior.
   - Ran the full existing chat-model test suite (`api/chat_models`, 
`integrations/chat_models/**`, 227 tests) -- all pass with no regressions.
   - Ran the broader `api/`, `plan/`, and `integrations/chat_models/` suites 
(722 tests) to check for any other silent-kwarg dependency this change might 
expose -- no new failures. The 4 pre-existing failures in that run 
(`test_bash_tool.py`, one YAML schema test) reproduce identically on `main` 
without this change and are Windows-environment-specific, unrelated to this PR.
   - `./tools/lint.sh`-equivalent (`ruff check` / `ruff format --check`) passes 
on all changed files.
   
   ### Are these changes tested?
   
   Yes, see above.
   
   ### Are there any user-facing changes?
   
   Yes: constructing a chat-model connection or setup (via Python API or YAML) 
with an unrecognized keyword argument now raises `pydantic.ValidationError` 
instead of silently ignoring it. Any caller currently passing a 
stray/misspelled key will now see a clear error at construction time instead of 
the setting silently having no effect.
   
   ### Was this PR generated with the help of a generative AI tool?
   
   Yes.
   
   Generated-by: Claude Code 2.0.76 (Claude Sonnet 5)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to