ShawnGarello opened a new pull request, #1317:
URL: https://github.com/apache/flink-agents/pull/1317
Linked issue: #1255
### Purpose of change
A chat model setup whose `model` is `""` or whitespace only now fails when
it is created, in Java and Python, with `Chat model setup model must not be
blank.` An omitted `model` behaves as before.
The setups disagreed on a blank model: four Java setups replaced it with
their default, Java watsonx replaced only `""`, vLLM rejected it with its own
message, and the rest passed it to their connection, where Java Bedrock fell
back to the connection's `model`. A blank value is usually a configuration
slip, and substituting a model hides it. Not every provider has a default, so
failing is the rule that fits every setup.
#### Runtime flow
Java: the `BaseChatModelSetup` constructor throws `IllegalArgumentException`
when `model` is non-null and `isBlank()`. It runs before subclass code, so the
setups that default an omitted model, and vLLM, now test only for `null`.
Python: `BaseChatModelSetup` has a `field_validator("model")` raising
`ValueError` when `not value.strip()`, reported by pydantic as a
`ValidationError`. Setups with a default fill it in `__init__` before
validation. `VLLMChatModelSetup.__init__` checks first and keeps its own
message.
Across languages: `JavaChatModelSetup`, the Python wrapper for a Java setup,
passes `model=""` when the declaration omits it, leaving the default to the
Java setup. It overrides the validator by name with one that returns the value
unchanged; the wrapped Java setup runs the Java check when it is built. A
Python setup used from Java is built on the Python side first, so the Python
check fires first.
#### Key decisions
- One check per base class, not per provider, so user-defined setups get it
too.
- Omitted is unchanged: Java `null`, or a Python argument not passed.
- No trimming: a non-blank value is passed on as given.
- Connections are unchanged; a `model` passed per call in model parameters
is not checked.
- Python vLLM keeps its own blank-`model` message because its check runs
before the base validator; Java vLLM's runs after the base constructor, so Java
reports the base message.
### Behavioral Semantics
#### Interaction decisions
| Setup | `model` omitted (unchanged) | `model` blank |
|---|---|---|
| Java with a default (OpenAI Completions, OpenAI Responses, Anthropic,
Gemini, watsonx) | default | base exception |
| Java without a default (Ollama, Azure OpenAI, Bedrock) | `null` | base
exception |
| Java vLLM | vLLM's error | base exception |
| Python with a default (OpenAI, Anthropic, watsonx, DashScope) | default |
`ValueError` |
| Python without a default (Ollama, Azure OpenAI) | missing-argument error |
`ValueError` |
| Python vLLM | vLLM's error | vLLM's error, unchanged |
| Java setup used from Python | builds; Java applies its default | Java
check raises when the Java setup is built |
| Python setup used from Java | as the Python setup | Python check raises
first |
#### Behavioral contracts
1. Java: building any `BaseChatModelSetup` subclass with a blank `model`
throws `IllegalArgumentException` with message `Chat model setup model must not
be blank.`
2. Python: building a `BaseChatModelSetup` subclass with a blank `model`
raises a `ValidationError` (a `ValueError`) containing that message, except
vLLM (6) and `JavaChatModelSetup` (7).
3. Java: an omitted `model` still gives the setup's default for OpenAI
Completions, OpenAI Responses, Anthropic, Gemini and watsonx, and stays `null`
without a default.
4. Python: an omitted `model` still gives the default for OpenAI, Anthropic,
watsonx and DashScope.
5. Java Bedrock: a blank `model` fails at setup instead of falling back to
the connection's `model`.
6. vLLM: Java throws the base message for blank and `model is required for
vLLM…` for omitted; Python raises `model is required for vLLM…` for both.
7. Python `JavaChatModelSetup` builds when `model` is omitted.
8. A non-blank `model` is used exactly as given.
#### Failure behavior
A blank `model` raises from the setup constructor wherever the setup is
created; nothing catches it, falls back or retries. A non-string `model` fails
as before.
### Tests
| Contract | Tests |
|---|---|
| 1 | `BaseChatModelTest.testBlankModelRejected`; `testBlankModelRejected`
in each Java setup test (`constructor_rejectsBlankModel` for Ollama); `""` and
`" "`, exact message |
| 2 | `test_blank_model_raises_value_error`; `test_blank_model_rejected`
over six setups |
| 3 | `testOmittedModelLeftUnset`; `testDefaultModel` (OpenAI Completions,
OpenAI Responses, watsonx); `testGetParametersDefaults` (Anthropic, Gemini) |
| 4 | `test_omitted_model_uses_the_default` (Anthropic, watsonx, DashScope);
`test_default_model_when_omitted` (OpenAI) |
| 5 | `BedrockChatModelSetupTest.testBlankModelRejected` |
| 6 | `VLLMChatModelSetupTest.testConstructorBlankModel`,
`testConstructorMissingModel`; existing Python vLLM tests |
| 7 | `test_java_chat_model_setup_builds_without_a_model` |
| 8 | existing parameter tests, e.g. Gemini `testGetParametersCustom` |
Coverage by risk: every built-in setup is built with both blank forms in
both languages, and every setup with a default has an omitted-model test.
Not verified:
- No test pins that a non-blank value is not trimmed.
- The cross-language rows: no test builds a Java setup from Python, or a
Python setup from Java, with a blank model.
- End-to-end jobs and live provider calls.
<details>
<summary>Test runs and implementation invariants</summary>
- `mvn -B -fae test -pl
api,plan,integrations/chat-models/{ollama,openai,anthropic,bedrock,gemini,watsonx}`:
all pass. api 831 (14 skipped), plan 651 (3 skipped), Ollama 37, OpenAI 161,
Anthropic 112, Bedrock 92, Gemini 73, watsonx 54 (2 skipped).
- `pytest -k "not e2e_tests" -m "not integration"` over
`api/chat_models/tests/`, `integrations/chat_models/`, and `runtime/tests/`
`test_java_chat_model.py`, `test_cross_language_chat_model_skills.py`,
`test_cross_language_metric_group.py`: 518 passed. Whole unit suite: 2805
passed, 16 skipped.
- `ruff check` (0.11.13), `mvn spotless:check` and
`./tools/check-license.sh` pass.
- With the main-code change reverted, all 20 new Java and 14 new Python
blank cases fail and every omitted-model test passes. Without the
`JavaChatModelSetup` override, contract 7's test fails.
- The Java check runs in the base constructor before any subclass field is
set, so the subclass blank branches were unreachable and are removed. Their
`null` branches stay.
- The `JavaChatModelSetup` override works because pydantic replaces an
inherited field validator with a subclass method of the same name.
- `PythonResourceProvider` builds the Python object before the Java wrapper,
so the Python check fires first for a Python setup used from Java.
- No other setup in either language redeclares or validates `model`.
</details>
### API
No signature change and no new dependency. For a caller who changes nothing,
a setup declared with a blank `model` now fails when it is created instead of
using a default, the Bedrock connection's `model`, or the blank value. An
omitted or non-blank `model` behaves as before.
### Documentation
- [ ] `doc-needed`
- [ ] `doc-not-needed`
- [x] `doc-included`
### Was this patch authored or co-authored using generative AI tooling?
- [x] Yes
- [ ] No
Generated-by: Claude Code 2.1.305 (Claude Opus 5.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]
