diff --git a/docs/content/docs/development/chat_models.md b/docs/content/docs/development/chat_models.md index d9a21f96ce..588bff641a 100644 --- a/docs/content/docs/development/chat_models.md +++ b/docs/content/docs/development/chat_models.md @@ -291,9 +291,8 @@ Anthropic provides cloud-based chat models featuring the Claude family, known fo | `prompt` | Prompt \| str | None | Prompt template or reference to prompt resource | | `tools` | List[str] | None | List of tool names available to the model | | `max_tokens` | int | `1024` | Maximum number of tokens to generate | -| `temperature` | float | `0.1` | Sampling temperature (0.0 to 1.0) | +| `temperature` | float | `0.1` | Sampling temperature (0.0 to 1.0) (not sent on Claude 4.7 and later, which reject a non-default sampling parameter) | | `json_prefill` | bool | `False` | Prefill assistant response with "{" to enforce JSON output (applies only on models that accept assistant-message prefilling; disabled when tools are used, or when the request carries an `output_config`) | -| `additional_kwargs` | dict | `{}` | Additional Anthropic API parameters | {{< /tab >}} @@ -306,10 +305,10 @@ Anthropic provides cloud-based chat models featuring the Claude family, known fo | `prompt` | Prompt \| String | None | Prompt template or reference to prompt resource | | `tools` | List | None | List of tool names available to the model | | `max_tokens` | long | `1024` | Maximum number of tokens to generate | -| `temperature` | double | `0.1` | Sampling temperature (0.0 to 1.0) | +| `temperature` | double | `0.1` | Sampling temperature (0.0 to 1.0) (not sent on Claude 4.7 and later, which reject a non-default sampling parameter) | | `json_prefill` | boolean | `false` | Prefill assistant response with "{" to enforce JSON output (applies only on models that accept assistant-message prefilling; disabled when tools are used, or when the request carries an `output_config`) | | `strict_tools` | boolean | `false` | Enable strict mode for tool calling schemas | -| `additional_kwargs` | Map | `{}` | Additional Anthropic API parameters (top_k, top_p, stop_sequences); an `output_config` supplied here takes precedence over one derived from an output schema | +| `additional_kwargs` | Map | `{}` | Additional Anthropic API parameters (`temperature`, `top_k`, `top_p`, `stop_sequences`); none of the three sampling parameters is sent on Claude 4.7 and later. A `temperature` supplied here takes precedence over the top-level one, and an `output_config` supplied here takes precedence over one derived from an output schema | {{< /tab >}} diff --git a/integrations/chat-models/anthropic/pom.xml b/integrations/chat-models/anthropic/pom.xml index 2fd9b28710..07d0b592bc 100644 --- a/integrations/chat-models/anthropic/pom.xml +++ b/integrations/chat-models/anthropic/pom.xml @@ -43,6 +43,12 @@ under the License. anthropic-java ${anthropic.version} + + + org.slf4j + slf4j-api + ${slf4j.version} + diff --git a/integrations/chat-models/anthropic/src/main/java/org/apache/flink/agents/integrations/chatmodels/anthropic/AnthropicChatModelConnection.java b/integrations/chat-models/anthropic/src/main/java/org/apache/flink/agents/integrations/chatmodels/anthropic/AnthropicChatModelConnection.java index bf8e0afe0f..e2dcb98311 100644 --- a/integrations/chat-models/anthropic/src/main/java/org/apache/flink/agents/integrations/chatmodels/anthropic/AnthropicChatModelConnection.java +++ b/integrations/chat-models/anthropic/src/main/java/org/apache/flink/agents/integrations/chatmodels/anthropic/AnthropicChatModelConnection.java @@ -42,6 +42,8 @@ import org.apache.flink.agents.api.resource.ResourceContext; import org.apache.flink.agents.api.resource.ResourceDescriptor; import org.apache.flink.agents.api.tools.ToolMetadata; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.time.Duration; import java.util.ArrayList; @@ -51,6 +53,7 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; /** @@ -81,6 +84,8 @@ */ public class AnthropicChatModelConnection extends BaseChatModelConnection { + private static final Logger LOG = LoggerFactory.getLogger(AnthropicChatModelConnection.class); + private static final TypeReference> MAP_TYPE = new TypeReference<>() {}; private final ObjectMapper mapper = new ObjectMapper(); @@ -223,6 +228,128 @@ static boolean supportsJsonPrefill(String effectiveModel) { return !PREFILL_UNSUPPORTED_MODELS.contains(effectiveModel); } + // Models that reject a non-default sampling parameter. Source of truth: + // https://platform.claude.com/docs/en/about-claude/models/migration-guide + // + // The documented rule is that setting temperature, top_p or top_k to any non-default value on + // Claude Opus 4.7 or later returns a 400, and the Claude Sonnet 5 release notes carry the same + // rule for the Sonnet line while stating it is new for Sonnet-class models. Sending the + // provider default, or omitting the parameter, stays acceptable on every model, so the way to + // honour this is to drop the parameter rather than to substitute a value. + // + // This is the third boundary encoded in this class and it lines up with neither of the others. + // Structured output starts at the 4.5 generation and prefill rejection at 4.6, while sampling + // rejection starts at 4.7. Claude 4.6 therefore rejects a prefill while still accepting a + // temperature, so the prefill list above cannot be reused here even though the two overlap. + // + // Claude Fable 5, Claude Mythos 5 and Claude Mythos Preview are listed without a matching + // sentence in the migration guide. That guide records each release's deltas, and these models + // succeed Claude Opus 4.8, which already rejects sampling parameters, so there was no delta to + // record. They are treated as rejecting because they inherit the constraint, not because a + // release note restates it. + private static final Set SAMPLING_UNSUPPORTED_MODELS = + Set.of( + "claude-opus-4-7", + "claude-opus-4-8", + "claude-opus-5", + "claude-sonnet-5", + "claude-fable-5", + "claude-mythos-5", + "claude-mythos-preview"); + + // The {model name, parameter name} pairs already reported through the warning in + // sendSamplingParam, so a rejected sampling parameter is surfaced once per model and parameter + // instead of on every request. The parameter belongs in the key because a model withdraws three + // of them at once: keying on the name alone would report whichever was dropped first and leave + // every later one silent. + private static final Set> SAMPLING_WARNED_PARAMS = ConcurrentHashMap.newKeySet(); + + /** + * Whether {@code effectiveModel} accepts non-default sampling parameters, meaning {@code + * temperature}, {@code top_p} and {@code top_k}. + * + *

See the list above for the source of truth and for why it is kept apart from the prefill + * and structured-output lists. An unrecognized name reports {@code true}, matching the + * documented rule: sampling parameters are the long-standing behaviour and only the listed + * names withdraw them. That default carries the same cost as {@link #supportsJsonPrefill}: a + * rejecting model this list has not caught up with is sent a sampling parameter and answered + * with a 400. + */ + static boolean supportsSamplingParams(String effectiveModel) { + // Load-bearing: the list is an immutable Set, whose contains(null) throws rather than + // reporting absence. + if (effectiveModel == null) { + return true; + } + return !SAMPLING_UNSUPPORTED_MODELS.contains(effectiveModel); + } + + /** + * Whether the sampling parameter {@code param}, carrying {@code value}, may be put on a request + * for {@code effectiveModel}, warning the first time it may not. + * + *

A rejected parameter is dropped rather than clamped: the provider accepts an omitted + * parameter but answers a non-default one with a 400, and substituting the provider default + * would quietly change sampling behaviour instead of leaving it to the provider. + */ + private static boolean sendSamplingParam(String effectiveModel, String param, Object value) { + if (supportsSamplingParams(effectiveModel)) { + return true; + } + samplingWarning(effectiveModel, param, value).ifPresent(LOG::warn); + return false; + } + + /** + * The warning owed for dropping {@code param}, which carried {@code value}, on {@code + * effectiveModel}: the message on the first call for that model and parameter, and empty on + * every call after. + * + *

The message is owed against the pair rather than against the model name alone. A model + * withdraws three sampling parameters at once, so a debt held against the name would be settled + * by whichever parameter happened to be dropped first, leaving every later one dropped in + * silence. + * + *

Building the message here rather than at the call site keeps it on the same side of the + * once-per-pair decision as the decision itself, so there is no arrangement of the caller that + * reports a parameter on every request. + * + *

{@code effectiveModel} must not be null, which holds because only a name on the list above + * ever reaches this method. + */ + static Optional samplingWarning(String effectiveModel, String param, Object value) { + if (!SAMPLING_WARNED_PARAMS.add(List.of(effectiveModel, param))) { + return Optional.empty(); + } + return Optional.of( + String.format( + "Model %s rejects non-default sampling parameters, so the configured %s %s" + + " was not sent. Steer the model through its prompt instead.", + effectiveModel, param, value)); + } + + /** + * The temperature a request would carry, given the top-level {@code temperature} and an {@code + * additional_kwargs} map that may hold one of its own. + * + *

The map wins when it holds a {@link Number} under that key: an entry naming the parameter + * outright is the more specific setting, so it takes the top-level parameter's place. An absent + * key, or one holding anything other than a {@link Number}, leaves {@code topLevel} in place, + * because such a value reaches the request by neither route. + * + *

Resolving here rather than gating each route separately is what keeps the + * dropped-parameter warning honest. That warning is owed once per model and parameter, so + * gating the top-level value first would spend the one report on a value the map was about to + * override, naming a setting that was never going to be sent. + */ + static Object effectiveTemperature(Object topLevel, Map additionalKwargs) { + if (additionalKwargs == null) { + return topLevel; + } + Object override = additionalKwargs.get("temperature"); + return override instanceof Number ? override : topLevel; + } + /** * Derives the native {@code output_config} for a POJO class through the SDK's typed * structured-output builder. @@ -365,16 +492,19 @@ BuiltRequest buildRequest( builder.maxTokens(((Number) maxTokens).longValue()); } - Object temperature = modelParams.remove("temperature"); - if (temperature instanceof Number) { - builder.temperature(((Number) temperature).doubleValue()); - } - @SuppressWarnings("unchecked") Map additionalKwargs = (Map) modelParams.remove("additional_kwargs"); + + Object temperature = + effectiveTemperature(modelParams.remove("temperature"), additionalKwargs); + if (temperature instanceof Number + && sendSamplingParam(modelName, "temperature", temperature)) { + builder.temperature(((Number) temperature).doubleValue()); + } + if (additionalKwargs != null) { - applyAdditionalKwargs(builder, additionalKwargs); + applyAdditionalKwargs(builder, additionalKwargs, modelName); } // Read here rather than inside the native structured-output branch below because it governs @@ -687,22 +817,29 @@ private String extractJsonFromMarkdown(String content) { } private void applyAdditionalKwargs( - MessageCreateParams.Builder builder, Map kwargs) { + MessageCreateParams.Builder builder, Map kwargs, String modelName) { for (Map.Entry entry : kwargs.entrySet()) { String key = entry.getKey(); Object value = entry.getValue(); switch (key) { case "top_k": - if (value instanceof Number) { + if (value instanceof Number && sendSamplingParam(modelName, key, value)) { builder.topK(((Number) value).longValue()); } break; case "top_p": - if (value instanceof Number) { + if (value instanceof Number && sendSamplingParam(modelName, key, value)) { builder.topP(((Number) value).doubleValue()); } break; + case "temperature": + // Resolved against the top-level "temperature" by effectiveTemperature and + // gated before this map is applied, so there is nothing left to do with it + // here. The case still has to exist: without a label of its own the key falls + // to the default branch and goes onto the body as a raw property, ungated, + // which the models that withdraw sampling parameters answer with a 400. + break; case "stop_sequences": if (value instanceof List) { @SuppressWarnings("unchecked") diff --git a/integrations/chat-models/anthropic/src/main/java/org/apache/flink/agents/integrations/chatmodels/anthropic/AnthropicChatModelSetup.java b/integrations/chat-models/anthropic/src/main/java/org/apache/flink/agents/integrations/chatmodels/anthropic/AnthropicChatModelSetup.java index d58befb610..629da88221 100644 --- a/integrations/chat-models/anthropic/src/main/java/org/apache/flink/agents/integrations/chatmodels/anthropic/AnthropicChatModelSetup.java +++ b/integrations/chat-models/anthropic/src/main/java/org/apache/flink/agents/integrations/chatmodels/anthropic/AnthropicChatModelSetup.java @@ -48,8 +48,9 @@ *

  • strict_tools (optional): When true, tool calls adhere strictly to the JSON schema. * (default: false) *
  • tools (optional): List of tool names available for the model to use - *
  • additional_kwargs (optional): Additional parameters (top_k, top_p, stop_sequences). - * An output_config supplied here takes precedence over one derived from an output schema. + *
  • additional_kwargs (optional): Additional parameters (temperature, top_k, top_p, + * stop_sequences). A temperature supplied here takes precedence over the top-level one, and + * an output_config supplied here takes precedence over one derived from an output schema. * * *

    Example usage: diff --git a/integrations/chat-models/anthropic/src/test/java/org/apache/flink/agents/integrations/chatmodels/anthropic/AnthropicChatModelConnectionTest.java b/integrations/chat-models/anthropic/src/test/java/org/apache/flink/agents/integrations/chatmodels/anthropic/AnthropicChatModelConnectionTest.java index bc63533d0b..e1e2057b58 100644 --- a/integrations/chat-models/anthropic/src/test/java/org/apache/flink/agents/integrations/chatmodels/anthropic/AnthropicChatModelConnectionTest.java +++ b/integrations/chat-models/anthropic/src/test/java/org/apache/flink/agents/integrations/chatmodels/anthropic/AnthropicChatModelConnectionTest.java @@ -19,6 +19,7 @@ package org.apache.flink.agents.integrations.chatmodels.anthropic; import com.anthropic.models.messages.Message; +import com.anthropic.models.messages.MessageCreateParams; import com.anthropic.models.messages.MessageParam; import com.anthropic.models.messages.Model; import com.anthropic.models.messages.OutputConfig; @@ -40,6 +41,7 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.NullSource; import org.junit.jupiter.params.provider.ValueSource; @@ -52,8 +54,10 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Stream; +import static org.assertj.core.api.Assertions.as; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.InstanceOfAssertFactories.STRING; /** * Unit tests for {@link AnthropicChatModelConnection}'s request construction, its native @@ -628,6 +632,253 @@ void testPrefillSuppressedOnUnsupportedModel() { assertPrefillDecisionForModel("claude-opus-4-6", false); } + /** + * The models the provider documents as rejecting a non-default sampling parameter, in the order + * the connection lists them. Mirroring that order keeps the two lists comparable side by side, + * so a name added to one and not the other stands out. + */ + private static Stream samplingUnsupportedModels() { + return Stream.of( + "claude-opus-4-7", + "claude-opus-4-8", + "claude-opus-5", + "claude-sonnet-5", + "claude-fable-5", + "claude-mythos-5", + "claude-mythos-preview"); + } + + /** + * Names that accept a temperature. The two 4.6-generation names are the load-bearing ones: both + * reject a prefill, so deriving the sampling rule from the prefill list would strip a + * temperature the provider still accepts. + */ + private static Stream samplingSupportedModels() { + return Stream.of( + "claude-opus-4-6", + "claude-sonnet-4-6", + "claude-opus-4-5", + "claude-sonnet-4-5", + "claude-sonnet-4-20250514", + "claude-3-5-sonnet-latest", + ""); + } + + /** + * Builds a request for {@code model} carrying each supplied sampling parameter, so the caller + * can read back which of them survived. {@code temperature} is a top-level parameter while + * {@code top_p} and {@code top_k} travel in {@code additional_kwargs}, and a null argument + * leaves that parameter out of the request entirely. + */ + private static MessageCreateParams samplingRequest( + String model, Double temperature, Double topP, Long topK) { + Map params = paramsWithModel(model, null); + if (temperature != null) { + params.put("temperature", temperature); + } + Map additionalKwargs = new HashMap<>(); + if (topP != null) { + additionalKwargs.put("top_p", topP); + } + if (topK != null) { + additionalKwargs.put("top_k", topK); + } + params.put("additional_kwargs", additionalKwargs); + return connection().buildRequest(userMessage(), List.of(), params, null).params; + } + + /** + * Builds a request for {@code model} whose temperature arrives through {@code + * additional_kwargs}, the second route the parameter can reach the request body by. + */ + private static MessageCreateParams kwargsTemperatureRequest(String model) { + Map params = paramsWithModel(model, null); + params.put("additional_kwargs", Map.of("temperature", 0.5d)); + return connection().buildRequest(userMessage(), List.of(), params, null).params; + } + + /** + * Builds a request for {@code model} carrying a top-level temperature and a different one in + * {@code additional_kwargs}, the case where the two routes disagree about the value. + */ + private static MessageCreateParams competingTemperatureRequest(String model) { + Map params = paramsWithModel(model, null); + params.put("temperature", 0.1d); + params.put("additional_kwargs", Map.of("temperature", 0.5d)); + return connection().buildRequest(userMessage(), List.of(), params, null).params; + } + + /** + * A top-level temperature, an {@code additional_kwargs} map and the value the two resolve to. + * The map wins only when it holds a number, since only a number reaches the request by either + * route. + */ + private static Stream temperatureResolutions() { + return Stream.of( + Arguments.of(0.1d, Map.of("temperature", 0.5d), 0.5d), + Arguments.of(0.1d, Map.of("top_p", 0.9d), 0.1d), + Arguments.of(0.1d, Map.of("temperature", "0.5"), 0.1d), + Arguments.of(null, Map.of("temperature", 0.5d), 0.5d), + Arguments.of(0.1d, null, 0.1d)); + } + + @ParameterizedTest + @MethodSource("samplingUnsupportedModels") + @DisplayName("every model documented as rejecting sampling parameters reports unsupported") + void testSamplingUnsupportedModelsReportUnsupported(String model) { + assertThat(AnthropicChatModelConnection.supportsSamplingParams(model)).isFalse(); + } + + @ParameterizedTest + @NullSource + @MethodSource("samplingSupportedModels") + @DisplayName("a model outside that list reports sampling supported") + void testSamplingSupportedModelsReportSupported(String model) { + assertThat(AnthropicChatModelConnection.supportsSamplingParams(model)).isTrue(); + } + + @Test + @DisplayName("temperature is dropped on a model that rejects sampling parameters") + void testTemperatureDroppedOnUnsupportedModel() { + assertThat(samplingRequest("claude-opus-4-7", 0.1d, null, null).temperature()).isEmpty(); + } + + @Test + @DisplayName("top_p is dropped on a model that rejects sampling parameters") + void testTopPDroppedOnUnsupportedModel() { + MessageCreateParams params = samplingRequest("claude-opus-4-7", null, 0.9d, null); + + assertThat(params.topP()).isEmpty(); + // An additional_kwargs key with no case of its own reaches the body as a raw property, so + // an empty typed field is not on its own proof the parameter was left off the request. + assertThat(params._additionalBodyProperties()).doesNotContainKey("top_p"); + } + + @Test + @DisplayName("top_k is dropped on a model that rejects sampling parameters") + void testTopKDroppedOnUnsupportedModel() { + MessageCreateParams params = samplingRequest("claude-opus-4-7", null, null, 5L); + + assertThat(params.topK()).isEmpty(); + assertThat(params._additionalBodyProperties()).doesNotContainKey("top_k"); + } + + @Test + @DisplayName("all three sampling parameters are sent on a model that accepts them") + void testSamplingParamsSentOnSupportedModel() { + MessageCreateParams params = samplingRequest("claude-sonnet-4-20250514", 0.1d, 0.9d, 5L); + + assertThat(params.temperature()).contains(0.1d); + assertThat(params.topP()).contains(0.9d); + assertThat(params.topK()).contains(5L); + } + + @Test + @DisplayName("temperature supplied through additional_kwargs is dropped on a rejecting model") + void testKwargsTemperatureDroppedOnUnsupportedModel() { + MessageCreateParams params = kwargsTemperatureRequest("claude-opus-4-7"); + + assertThat(params.temperature()).isEmpty(); + // Without a case of its own the key falls to the default branch and reaches the body as a + // raw property, which serializes to the same "temperature" field the typed setter writes. + assertThat(params._additionalBodyProperties()).doesNotContainKey("temperature"); + } + + @Test + @DisplayName("temperature supplied through additional_kwargs is sent on an accepting model") + void testKwargsTemperatureSentOnSupportedModel() { + assertThat(kwargsTemperatureRequest("claude-sonnet-4-20250514").temperature()) + .contains(0.5d); + } + + @Test + @DisplayName("stop_sequences supplied through additional_kwargs reaches the typed field") + void testStopSequencesFromAdditionalKwargs() { + Map params = paramsWithModel("claude-sonnet-4-20250514", null); + params.put("additional_kwargs", Map.of("stop_sequences", List.of("STOP", "END"))); + + MessageCreateParams built = + connection().buildRequest(userMessage(), List.of(), params, null).params; + + assertThat(built.stopSequences()).contains(List.of("STOP", "END")); + } + + @ParameterizedTest + @MethodSource("temperatureResolutions") + @DisplayName( + "additional_kwargs overrides the top-level temperature only when it holds a number") + void testEffectiveTemperature(Object topLevel, Map kwargs, Object expected) { + assertThat(AnthropicChatModelConnection.effectiveTemperature(topLevel, kwargs)) + .isEqualTo(expected); + } + + @Test + @DisplayName("the additional_kwargs temperature is the one an accepting model is sent") + void testCompetingTemperaturesResolveToKwargsOnSupportedModel() { + assertThat(competingTemperatureRequest("claude-sonnet-4-20250514").temperature()) + .contains(0.5d); + } + + @Test + @DisplayName("neither competing temperature reaches a model that rejects sampling parameters") + void testCompetingTemperaturesDroppedOnUnsupportedModel() { + // A rejecting name whose temperature no other test drives through buildRequest. The + // warning owed for a dropped parameter is tracked per model and parameter for the life of + // the JVM, so sharing a name would leave this test's meaning dependent on run order. + MessageCreateParams params = competingTemperatureRequest("claude-opus-4-8"); + + assertThat(params.temperature()).isEmpty(); + // An ungated additional_kwargs value reaches the body as a raw property, which serializes + // to the same "temperature" field the typed setter writes, so the typed field being empty + // is not on its own proof the parameter was left off the request. + assertThat(params._additionalBodyProperties()).doesNotContainKey("temperature"); + // One report is owed for the pair and the request spent it, so nothing is left to report. + // That the request above is what spent it holds only while no other test claims this + // model name and parameter first; one that did would satisfy this assertion for its own + // reason and leave nothing here for it to catch. + assertThat( + AnthropicChatModelConnection.samplingWarning( + "claude-opus-4-8", "temperature", 0.5d)) + .isEmpty(); + } + + @Test + @DisplayName( + "a dropped sampling parameter owes one warning naming the model, parameter and value") + void testDroppedSamplingParamOwesOneWarningPerParameter() { + // A name no other test asks a warning for, since the bookkeeping behind it lives for the + // life of the JVM and a pair already reported would answer empty here. + String model = "claude-mythos-preview"; + + Optional first = + AnthropicChatModelConnection.samplingWarning(model, "temperature", 0.1d); + Optional repeat = + AnthropicChatModelConnection.samplingWarning(model, "temperature", 0.1d); + Optional other = AnthropicChatModelConnection.samplingWarning(model, "top_p", 0.9d); + + // The message has to identify which request parameter went missing and what it held, + // because that is the only trace the dropped value leaves. + assertThat(first).get(as(STRING)).contains(model, "temperature", "0.1"); + // The repeat is silent, but a different parameter is a different fact about the request + // and owes a report of its own. + assertThat(repeat).isEmpty(); + assertThat(other).get(as(STRING)).contains(model, "top_p", "0.9"); + } + + @Test + @DisplayName("a 4.6 model rejects the prefill while keeping its sampling parameters") + void testSamplingAndPrefillBoundariesDiffer() { + // The two rules draw different lines, and this model sits between them: the provider + // withdraws prefilling from 4.6 on but sampling parameters only from 4.7 on. Deriving the + // sampling rule from the prefill list would drop the temperature here, where the provider + // still accepts it. + assertThat(AnthropicChatModelConnection.supportsJsonPrefill("claude-sonnet-4-6")).isFalse(); + assertThat(AnthropicChatModelConnection.supportsSamplingParams("claude-sonnet-4-6")) + .isTrue(); + assertThat(samplingRequest("claude-sonnet-4-6", 0.1d, null, null).temperature()) + .contains(0.1d); + } + @Test @DisplayName("json_prefill is applied on a structured-output capable model that accepts it") void testPrefillAppliedOnStructuredOutputCapableModel() { @@ -640,6 +891,79 @@ void testPrefillAppliedOnStructuredOutputCapableModel() { assertPrefillDecisionForModel("claude-sonnet-4-5", true); } + /** + * A temperature value that records whether anything rendered it as text. + * + *

    The report owed for a dropped sampling parameter is assembled with a {@code %s} + * conversion, so the value it names is whichever one had {@code toString()} called on it. A + * value that was never rendered was never reported. That is the only handle on the choice from + * outside the class: a model rejecting sampling parameters is sent neither candidate, so the + * request itself looks the same whichever one the report picked. + * + *

    Rendering is the measurement, so observing one taints it. A breakpoint that displays this + * value, or a debug print of it, calls {@code toString()} and makes {@code wasRendered()} + * answer true regardless of what the code under test did. + */ + private static final class RecordingTemperature extends Number { + private final double value; + private boolean rendered; + + RecordingTemperature(double value) { + this.value = value; + } + + boolean wasRendered() { + return rendered; + } + + @Override + public String toString() { + rendered = true; + return Double.toString(value); + } + + @Override + public int intValue() { + return (int) value; + } + + @Override + public long longValue() { + return (long) value; + } + + @Override + public float floatValue() { + return (float) value; + } + + @Override + public double doubleValue() { + return value; + } + } + + @Test + @DisplayName("a dropped temperature is reported as the additional_kwargs value it resolved to") + void testDroppedTemperatureIsReportedAsTheResolvedValue() { + // A rejecting name no other test builds a request for or asks a temperature report of. + // The report is owed once per model and parameter for the life of the JVM, so a pair + // claimed elsewhere first would leave nothing to render here and the first assertion + // below would fail: a collision surfaces as a failure, not as a test that still passes + // while pinning less. + RecordingTemperature topLevel = new RecordingTemperature(0.1d); + RecordingTemperature override = new RecordingTemperature(0.5d); + Map params = paramsWithModel("claude-mythos-5", null); + params.put("temperature", topLevel); + params.put("additional_kwargs", Map.of("temperature", override)); + + connection().buildRequest(userMessage(), List.of(), params, null); + + assertThat(override.wasRendered()).isTrue(); + // The overridden value would have named a setting the request was never going to carry. + assertThat(topLevel.wasRendered()).isFalse(); + } + /** Minimal tool stub; only its presence in the tools list matters. */ private static class StubTool extends Tool { StubTool() { diff --git a/python/flink_agents/integrations/chat_models/anthropic/anthropic_chat_model.py b/python/flink_agents/integrations/chat_models/anthropic/anthropic_chat_model.py index dde0a89413..f4406cc12a 100644 --- a/python/flink_agents/integrations/chat_models/anthropic/anthropic_chat_model.py +++ b/python/flink_agents/integrations/chat_models/anthropic/anthropic_chat_model.py @@ -15,6 +15,7 @@ # See the License for the specific language governing permissions and # limitations under the License. ################################################################################# +import logging import uuid from typing import Any, Dict, List, Sequence @@ -32,6 +33,8 @@ ) from flink_agents.api.tools.tool import Tool, ToolMetadata +logger = logging.getLogger(__name__) + def to_anthropic_tool( *, metadata: ToolMetadata, skip_length_check: bool = False @@ -197,6 +200,64 @@ def _supports_json_prefill(effective_model: str | None) -> bool: return effective_model not in _PREFILL_UNSUPPORTED_MODELS +# Models that reject a non-default sampling parameter. Source of truth: +# https://platform.claude.com/docs/en/about-claude/models/migration-guide +# +# The documented rule is that setting temperature, top_p or top_k to any non-default +# value on Claude Opus 4.7 or later returns a 400, and the Claude Sonnet 5 release notes +# carry the same rule for the Sonnet line while stating it is new for Sonnet-class +# models. Sending the provider default, or omitting the parameter, stays acceptable on +# every model, so the way to honour this is to drop the parameter rather than to +# substitute a value. +# +# This is the third boundary encoded in this module and it lines up with neither of the +# others. Structured output starts at the 4.5 generation and prefill rejection at 4.6, +# while sampling rejection starts at 4.7. Claude 4.6 therefore rejects a prefill while +# still accepting a temperature, so the prefill list above cannot be reused here even +# though the two overlap. +# +# Claude Fable 5, Claude Mythos 5 and Claude Mythos Preview are listed without a +# matching sentence in the migration guide. That guide records each release's deltas, +# and these models succeed Claude Opus 4.8, which already rejects sampling parameters, +# so there was no delta to record. They are treated as rejecting because they inherit +# the constraint, not because a release note restates it. +_SAMPLING_UNSUPPORTED_MODELS = frozenset( + { + "claude-opus-4-7", + "claude-opus-4-8", + "claude-opus-5", + "claude-sonnet-5", + "claude-fable-5", + "claude-mythos-5", + "claude-mythos-preview", + } +) + +# The request parameters those models withdraw. +_SAMPLING_PARAMS = ("temperature", "top_p", "top_k") + +# The (model name, parameter name) pairs already reported through the warning in +# ``chat``, so a rejected sampling parameter is surfaced once per model and parameter +# instead of on every request. The parameter belongs in the key because a model +# withdraws three of them at once: keying on the name alone would report whichever was +# dropped first and leave every later one silent. +_SAMPLING_WARNED_PARAMS: set[tuple[str, str]] = set() + + +def _supports_sampling_params(effective_model: str | None) -> bool: + """Whether ``effective_model`` accepts non-default sampling parameters. + + The parameters are ``temperature``, ``top_p`` and ``top_k``. See the list above for + the source of truth and for why it is kept apart from the prefill and + structured-output lists. An unrecognized name reports ``True``, matching the + documented rule: sampling parameters are the long-standing behaviour and only the + listed names withdraw them. That default carries the same cost as + ``_supports_json_prefill``: a rejecting model this list has not caught up with is + sent a sampling parameter and answered with a 400. + """ + return effective_model not in _SAMPLING_UNSUPPORTED_MODELS + + def _native_output_config(output_schema: Any) -> Dict[str, Any] | None: """Build the Anthropic ``output_config`` for a native structured-output request. @@ -398,6 +459,27 @@ def chat( {"role": MessageRole.ASSISTANT.value, "content": "{"}, ] + # Dropped rather than clamped on a model that rejects them: the provider + # accepts an omitted parameter but answers a non-default one with a 400, + # and substituting the provider default would quietly change sampling + # behaviour instead of leaving it to the provider. + if not _supports_sampling_params(kwargs.get("model")): + model_name = kwargs.get("model") + for param in _SAMPLING_PARAMS: + if param not in kwargs: + continue + dropped = kwargs.pop(param) + if (model_name, param) not in _SAMPLING_WARNED_PARAMS: + _SAMPLING_WARNED_PARAMS.add((model_name, param)) + logger.warning( + "Model %s rejects non-default sampling parameters, so the " + "configured %s %s was not sent. Steer the model through its " + "prompt instead.", + model_name, + param, + dropped, + ) + message = self.client.messages.create( messages=anthropic_messages, tools=anthropic_tools or NOT_GIVEN, diff --git a/python/flink_agents/integrations/chat_models/anthropic/tests/test_anthropic_response_parsing.py b/python/flink_agents/integrations/chat_models/anthropic/tests/test_anthropic_response_parsing.py index 0577a32fd2..3a492ad355 100644 --- a/python/flink_agents/integrations/chat_models/anthropic/tests/test_anthropic_response_parsing.py +++ b/python/flink_agents/integrations/chat_models/anthropic/tests/test_anthropic_response_parsing.py @@ -15,6 +15,7 @@ # See the License for the specific language governing permissions and # limitations under the License. ################################################################################# +import logging from typing import Any, Callable, Dict from unittest.mock import MagicMock @@ -28,9 +29,11 @@ from flink_agents.api.chat_message import ChatMessage, MessageRole from flink_agents.api.tools.tool import Tool, ToolMetadata, ToolType from flink_agents.integrations.chat_models.anthropic.anthropic_chat_model import ( + _SAMPLING_WARNED_PARAMS, AnthropicChatModelConnection, AnthropicChatModelSetup, _supports_json_prefill, + _supports_sampling_params, ) @@ -595,3 +598,116 @@ def test_setup_honors_explicit_json_prefill() -> None: # unconditionally. setup = AnthropicChatModelSetup(connection="conn", json_prefill=True) assert setup.model_kwargs["json_prefill"] is True + + +# The models the provider documents as rejecting a non-default sampling parameter, in +# the order the module lists them. Mirroring that order keeps the two comparable side by +# side, so a name added to one and not the other stands out. +_SAMPLING_UNSUPPORTED = [ + "claude-opus-4-7", + "claude-opus-4-8", + "claude-opus-5", + "claude-sonnet-5", + "claude-fable-5", + "claude-mythos-5", + "claude-mythos-preview", +] + +# Names that accept sampling parameters. The two 4.6-generation names are the +# load-bearing ones: both reject a prefill, so deriving the sampling rule from the +# prefill list would strip a temperature the provider still accepts. +_SAMPLING_SUPPORTED = [ + "claude-opus-4-6", + "claude-sonnet-4-6", + "claude-opus-4-5", + "claude-sonnet-4-5", + "claude-sonnet-4-20250514", + "claude-3-5-sonnet-latest", + None, +] + + +_ABSENT = object() + +# The three sampling parameters, spelled out rather than read from the module under test. +# Reading the production tuple would make a name dropped from it disappear from the comparison +# below, so the drop would go unnoticed instead of failing an assertion. +_SAMPLING_PARAM_NAMES = ("temperature", "top_p", "top_k") + + +def _sent_sampling(model: str, **sampling: Any) -> Dict[str, Any]: + """Which of the sampling parameters the request reached the client with. + + Each of the three names appears in the result, mapped either to the value the client + saw or to ``_ABSENT`` when it was dropped on the way. + """ + message = Message( + id="m", + model="claude", + role="assistant", + type="message", + stop_reason="end_turn", + content=[TextBlock(type="text", text=_CONTINUATION)], + usage=_usage(), + ) + connection = _connection_returning(message) + connection.chat( + [ChatMessage(role=MessageRole.USER, content="hi")], + model=model, + **sampling, + ) + sent = connection.client.messages.create.call_args.kwargs + return {param: sent.get(param, _ABSENT) for param in _SAMPLING_PARAM_NAMES} + + +@pytest.mark.parametrize("model", _SAMPLING_UNSUPPORTED) +def test_sampling_predicate_rejects_unsupported_models(model) -> None: + assert _supports_sampling_params(model) is False + + +@pytest.mark.parametrize("model", _SAMPLING_SUPPORTED) +def test_sampling_predicate_accepts_other_models(model) -> None: + assert _supports_sampling_params(model) is True + + +@pytest.mark.parametrize( + ("param", "value"), [("temperature", 0.1), ("top_p", 0.9), ("top_k", 5)] +) +def test_sampling_param_dropped_on_unsupported_model(param, value) -> None: + assert _sent_sampling("claude-opus-4-7", **{param: value})[param] is _ABSENT + + +def test_sampling_params_sent_on_supported_model() -> None: + assert _sent_sampling( + "claude-sonnet-4-20250514", temperature=0.1, top_p=0.9, top_k=5 + ) == {"temperature": 0.1, "top_p": 0.9, "top_k": 5} + + +def test_each_dropped_sampling_param_is_reported_once(caplog) -> None: + # The bookkeeping behind the warning lives for the life of the process, so a pair + # left behind by an earlier test would suppress a warning this test has to observe. + model = "claude-mythos-preview" + for param in _SAMPLING_PARAM_NAMES: + _SAMPLING_WARNED_PARAMS.discard((model, param)) + + with caplog.at_level(logging.WARNING): + _sent_sampling(model, temperature=0.1) + _sent_sampling(model, temperature=0.1) + _sent_sampling(model, top_p=0.9) + + # The repeat stays silent, but a different parameter is a different fact about the + # request and has to be reported on its own. + warnings = [record.getMessage() for record in caplog.records] + assert len(warnings) == 2 + assert "temperature" in warnings[0] + assert "top_p" in warnings[1] + + +def test_sampling_and_prefill_boundaries_differ() -> None: + # The two rules draw different lines, and this model sits between them: the provider + # withdraws prefilling from 4.6 on but sampling parameters only from 4.7 on. + # Deriving the sampling rule from the prefill list would drop the temperature here, + # where the provider still accepts it. + assert _supports_json_prefill("claude-sonnet-4-6") is False + assert _supports_sampling_params("claude-sonnet-4-6") is True + assert _sent_sampling("claude-sonnet-4-6", temperature=0.1)["temperature"] == 0.1