weiqingy opened a new pull request, #685:
URL: https://github.com/apache/flink-agents/pull/685
<!--
* Thank you very much for contributing to Flink Agents.
* Please add the relevant components in the PR title. E.g., [api],
[runtime], [java], [python], [hotfix], etc.
-->
Linked issue: #159
### Purpose of change
Different `ChatModelSetup` implementations diverged in how they exposed the
`model` parameter. On the Java side, every chat-model subclass except
`OllamaChatModelSetup` already used the inherited `BaseChatModelSetup.model`
field — Ollama re-declared its own `private final String model` shadow. On the
Python side, `BaseChatModelSetup` had no `model` field at all, so every
chat-model subclass redeclared `model: str = Field(...)` locally. This diverged
from `BaseEmbeddingModelSetup`, which has owned the field at the base on both
languages from the start.
This PR aligns `ChatModelSetup` with `EmbeddingModelSetup`:
- **Commit 1 (Java)** — removes the redundant `model` field shadow in
`OllamaChatModelSetup`. `getParameters()` now reads from the inherited
`BaseChatModelSetup.model` field that was already populated by the same
descriptor argument. Behavior is bit-for-bit identical; the redundant surface
is gone.
- **Commit 2 (Python)** — lifts `model: str = Field(description="Name of the
chat model to use.")` to `BaseChatModelSetup` (required, no default — matching
`BaseEmbeddingModelSetup`). Removes the now-redundant per-subclass `Field`
redeclarations from Ollama, Anthropic, OpenAI, Tongyi, and AzureOpenAI.
`__init__` signatures, per-integration defaults, and
`super().__init__(model=model, ...)` call sites are all preserved.
The cross-language bridge (`runtime/java/java_chat_model.py`) pops `model`
from kwargs and forwards it to `super().__init__()`, symmetric with the
existing `connection` handling. Test mocks and the cross-language compatibility
fixture (`agent_plan.json`) are updated to satisfy the new required field.
### Tests
- New: `OllamaChatModelSetupTest` (Java) locks the invariant that
`getModel()` and `getParameters().get("model")` both return the
descriptor-supplied value after the shadow removal.
- New: `test_chat_model_base.py` (Python) — a minimal subclass that omits
the local `model` field inherits it from the base; missing `model` raises
`ValidationError`.
- Each Python integration test file gains `test_model_field_roundtrip` (5
files); Anthropic / OpenAI / Tongyi also gain `test_default_model_when_omitted`
to confirm `__init__`-level defaults still apply post-refactor.
- All existing tests stay green: `./tools/ut.sh -j` (Java), `cd python && uv
run pytest flink_agents -k "not e2e_tests"` (Python).
- `./tools/lint.sh -c` and `./tools/check-license.sh` pass.
### API
`BaseChatModelSetup` (Python) now declares `model: str` as a required
base-class field, matching `BaseEmbeddingModelSetup`. Public `__init__`
signatures of every chat-model subclass are unchanged. Descriptor argument key
remains `"model"`.
One behavior tightening, intentional and aligned with the embedding
contract: calling `<Subclass>.model_validate({"connection": "x"})` without a
`"model"` key now raises `pydantic.ValidationError` instead of falling back to
a subclass field-level default. Construction through `__init__` (the normal
path) is unaffected — per-integration defaults still apply.
### Documentation
- [ ] `doc-needed`
- [x] `doc-not-needed`
- [ ] `doc-included`
--
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]
