You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "weiqingy (via GitHub)" <gi...@apache.org> on 2026/05/31 20:28:16 UTC

[PR] [integrations][api] Rename connection-layer chat() argument to modelParams [flink-agents]

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

   Linked issue: #715
   
   ### Purpose of change
   
   After #698 renamed the setup-layer `BaseChatModelSetup.chat()` model-parameters argument to `modelParams`, the connection layer still named the same value `arguments` — the setup's `modelParams` was passed straight into `BaseChatModelConnection.chat(... Map<String, Object> arguments)`, i.e. one concept under two names across a single call hop, and `arguments` also collided with the (now-renamed) prompt-template and the tool-call `arguments`.
   
   This renames the connection-layer model-parameters identifier `arguments` → `modelParams` so the concept has one consistent name end-to-end. It covers the `BaseChatModelConnection.chat(...)` abstract method (signature + Javadoc), the Pemja bridge override, the seven provider connections (Anthropic, Ollama, Bedrock, OpenAI Responses/Completions/AzureOpenAI, AzureAI), and their in-file model-params locals/helper params (including `rawArguments` → `rawModelParams` in the `buildRequest` helpers).
   
   Unrelated `arguments` are deliberately left untouched: tool-call `"arguments"` JSON keys, the `parseArguments`/`serializeArguments` tool-call helpers, MCP `arguments`, and `JavaResourceAdapter`'s tool-call `arguments`.
   
   **Compatibility:** `BaseChatModelConnection.chat(...)` is a public abstract method, so custom connection implementations update their override's parameter name. The change is erasure-compatible (positional), so call sites are unaffected, and it is invisible across the Python boundary — the Pemja bridge forwards the map contents, never a key named `"arguments"`, so the Python connection layer (which already uses `**kwargs`) needs no change.
   
   ### Tests
   
   No new tests — this is a behavior-preserving, name-only positional rename; existing tests recompiling and passing against the new signature is the correctness guarantee. Verified locally:
   - `mvn -pl api -am test-compile` and `test-compile` of the five chat-model modules: pass
   - `mvn -pl api test -Dtest='BaseChatModelSetupSkillsTest,BaseChatModelTest,PythonChatModelConnectionTest'`: 20/20 pass
   - `spotless:check` on all touched modules: clean
   
   ### API
   
   Yes — renames the parameter of the public abstract method `BaseChatModelConnection.chat(...)` (and all provider overrides) from `arguments` to `modelParams`. Erasure-compatible/positional: call sites unaffected; only custom subclass overrides update the parameter name.
   
   ### 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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [integrations][api] Rename connection-layer chat() argument to modelParams [flink-agents]

Posted by "xintongsong (via GitHub)" <gi...@apache.org>.
xintongsong merged PR #720:
URL: https://github.com/apache/flink-agents/pull/720


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org