weiqingy commented on code in PR #1037:
URL: https://github.com/apache/flink-agents/pull/1037#discussion_r3888659742
##########
integrations/chat-models/anthropic/src/main/java/org/apache/flink/agents/integrations/chatmodels/anthropic/AnthropicChatModelConnection.java:
##########
@@ -366,15 +471,16 @@ BuiltRequest buildRequest(
}
Object temperature = modelParams.remove("temperature");
- if (temperature instanceof Number) {
+ if (temperature instanceof Number
+ && sendSamplingParam(modelName, "temperature", temperature)) {
Review Comment:
Yes, good catch. The warning is owed once per model and parameter, so the
top-level value claimed it first and named a temperature that was about to be
overridden anyway.
Fixed in `daa13b34`. `additional_kwargs` is read before the gate now, and
`effectiveTemperature` picks the winner, so the warning names the value the
request would otherwise have carried. The `temperature` case in the switch
stays, just empty: dropping the label would send the value through the default
branch and onto the body raw.
The test was the awkward part. On a rejecting model neither value is sent,
so the only place the choice shows up is the warning text, and the module has
no logging binding. What I settled on was passing both temperatures as a
`Number` that records whether `toString()` ran, since the warning is built with
`%s`. The value that got rendered is the value that got named. Putting the old
order back fails that test and nothing else.
There's also a parameterized test on `effectiveTemperature` for the cases
where the top-level value still wins: no `temperature` key, or one holding a
non-number.
--
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]
