[issue-7526][BE] fix: support Anthropic adaptive-thinking models as online-rule judges#7531
Conversation
…online-rule judges Online LLM-as-judge rules failed with Anthropic adaptive-thinking models (claude-sonnet-5, claude-opus-4-7/4-8): the judge-path builder forwarded temperature unconditionally (400 "temperature is deprecated for this model") and never set max_tokens or read the rule's custom_parameters, so thinking consumed the whole budget and the judge returned an empty response (finishReason=LENGTH). In AnthropicClientGenerator.newChatLanguageModel (the judge path): - Gate temperature server-side via AnthropicModelName.supportsSamplingParams, and also skip it whenever thinking is enabled per-rule via custom_parameters (Anthropic rejects sampling params while thinking is on) — covers API-created rules that bypass the FE sanitizer. - Forward custom_parameters (thinking type/budget, max_tokens) onto the native langchain4j builder. - Always send max_tokens, defaulting to the Playground default (4096) and stacking it above any thinking budget so max_tokens > budget_tokens holds. - Ignore non-positive max_tokens/budget_tokens from the bypass path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
🟡 size/M
java
⏱️ pre-commit per-hook timing
⏭️ 40 skipped (no matching files changed)
|
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
Address baz-reviewer comments on the Anthropic adaptive-thinking judge fix: - Fail closed for unknown model names in AnthropicModelName.supportsSamplingParams (default false) so a new adaptive-thinking model can't reintroduce the temperature 400 before it is added to the enum; back the lookup with a cached static map instead of streaming values() on every call. - Treat thinking as enabled unless the type is explicitly "disabled" (covers "adaptive" and any future type), and decode the thinking block once via parseThinking(), reused by both the temperature gate and the builder wiring. - Clamp max_tokens above the thinking budget even when an explicit max_tokens is set, so max_tokens <= budget_tokens can no longer reach Anthropic and 400. - Rename the headroom test to reflect what it asserts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Second round of baz-reviewer feedback on the adaptive-thinking judge fix: - Default unknown model names back to true in supportsSamplingParams so registry-only Anthropic models keep temperature support; adaptive models remain explicitly opted out. - Treat thinking as enabled only for an explicit non-blank type other than "disabled", so an empty/blank thinking block no longer suppresses temperature or shapes max_tokens. - Forward thinking.budget_tokens only when thinking is enabled, so a budget-only (no type) block can't reach Anthropic as a partial config and 400. - Widen to long before adding headroom in resolveMaxTokens so an extreme budget can't overflow max_tokens to a negative int. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hinking case Harden dropsBudgetWhenThinkingDisabled so a regression that changes max_tokens (not just the thinking budget) is caught. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Python SDK E2E Tests Results (Python 3.14)286 tests 283 ✅ 3m 46s ⏱️ Results for commit 07bfed4. ♻️ This comment has been updated with latest results. |
…anthropic-adaptive-thinking-judge
Nimrod007
changed the title
Nimrod007
changed the title
Nimrod007
changed the title
Nimrod007
changed the title
| */ | ||
| private void applyCustomParameters(AnthropicChatModel.AnthropicChatModelBuilder builder, | ||
| JsonNode customParameters, ThinkingParams thinking) { | ||
| Optional.ofNullable(thinking.type()).ifPresent(builder::thinkingType); |
There was a problem hiding this comment.
💡 suggestion | Robustness
thinking.type() is forwarded to builder::thinkingType verbatim, and langchain4j does no client-side validation of the value (it passes straight through to the API). So a rule with an unsupported/typo'd type — "adaptive", "enabld", etc. — will still 400 at Anthropic after temperature has already been silently dropped by the gate above. The gate ("any non-disabled type counts as enabled") is a deliberate forward-compat choice, so this is a fair trade-off rather than a bug — just flagging that the failure surfaces later, at request time, with a less obvious cause. If you wanted to fail fast, you could constrain type to the known set (enabled/disabled) here; otherwise fine to leave as-is.
🤖 Review posted via /review-github-pr
There was a problem hiding this comment.
Leaving as-is — this is the deliberate forward-compat choice: treating any non-disabled type as enabled means new Anthropic thinking types (e.g. adaptive) work without a code change, at the cost of a typo'd type surfacing as an Anthropic 400 at request time rather than a fast local rejection. Given Anthropic keeps evolving these values, we'd rather not hard-code the allowed set here. The tradeoff is on the record — thanks for calling it out.
🤖 Reply posted via /address-github-pr-comments
| budgetTokens = budgetNode.asInt(); | ||
| } | ||
|
|
||
| return new ThinkingParams(type != null && !"disabled".equals(type), type, budgetTokens); |
There was a problem hiding this comment.
❓ question | Scope
The PR description notes the sibling LlmProviderAnthropicMapper.toCreateMessageRequest (playground / online-inference / proxy path) still forwards temperature unconditionally, so adaptive-thinking models 400 there too for API-created requests — deliberately left out of scope here. Just confirming there's a tracked follow-up for that path so it doesn't get lost, since the judge fix and the mapper fix share the same root cause.
🤖 Review posted via /review-github-pr
There was a problem hiding this comment.
Confirmed and tracked: OPIK-7438 — https://comet-ml.atlassian.net/browse/OPIK-7438 (assigned to Daniel Dimenshtein). It covers gating temperature/top_p/top_k in LlmProviderAnthropicMapper.toCreateMessageRequest for the playground/inference/proxy path, reusing AnthropicModelName.supportsSamplingParams + the thinking-enabled check, so it doesn't get lost.
🤖 Reply posted via /address-github-pr-comments
| * guarantees a {@code max_tokens} is always sent. Anthropic requires max_tokens, and without an explicit | ||
| * cap adaptive thinking can consume the whole budget, yielding an empty response (finishReason=LENGTH). | ||
| */ | ||
| private void applyCustomParameters(AnthropicChatModel.AnthropicChatModelBuilder builder, |
There was a problem hiding this comment.
🧹 nit | Consistency
This path does bespoke, field-by-field extraction of custom_parameters (thinking.type, thinking.budget_tokens, max_tokens), whereas the OpenAI path (OpikOpenAiChatModel) does a generic JsonNode → Map pass-through. The divergence is justified here — langchain4j's Anthropic builder exposes typed setters and there's the hard max_tokens > budget_tokens invariant to enforce, which a blind pass-through wouldn't handle — so no change needed. Noting it only so the difference is a conscious choice on the record.
🤖 Review posted via /review-github-pr
There was a problem hiding this comment.
Acknowledged — the divergence from the OpenAI map pass-through is intentional: langchain4j's Anthropic builder exposes typed setters (thinkingType/thinkingBudgetTokens/maxTokens) and we need to enforce the max_tokens > budget_tokens invariant, which a blind JsonNode → Map pass-through wouldn't handle. Recording it as a conscious choice.
🤖 Reply posted via /address-github-pr-comments
|
👋 Review summary What looks good
Overall Inline comments: 1 suggestion, 1 question, 1 nit — all minor, none blocking. 🤖 Review posted via /review-github-pr |
JetoPistola
left a comment
JetoPistola
left a comment
There was a problem hiding this comment.
Approving — all three review notes are resolved: the verbatim thinking.type suggestion is a deliberate forward-compat trade-off (on the record), the mapper follow-up is tracked as OPIK-7438, and the bespoke-vs-pass-through nit is acknowledged as a conscious choice. The playground/inference-path fix lands separately under OPIK-7438.
🤖 Review posted via /review-github-pr
2 participants
Details
Online LLM-as-judge rules failed with Anthropic adaptive-thinking models (
claude-sonnet-5,claude-opus-4-7/4-8): the judge-path builder forwardedtemperatureunconditionally (→400 "temperature is deprecated for this model") and never setmax_tokensor read the rule'scustom_parameters, so thinking consumed the whole budget and the judge returned an empty response (finishReason=LENGTH). API-created rules bypass the FE sanitizer added in OPIK_6244, so the fix must live in the backend.Changes (backend-only,
infrastructure/llm/antropic):AnthropicModelName— newsupportsSamplingParamscapability flag (defaulttrue;falsefor adaptive-thinking models) + static lookup defaulting unknown models totrue.AnthropicClientGenerator.newChatLanguageModel(judge path): gatetemperatureby model capability and whenever thinking is enabled per-rule viacustom_parameters; forwardcustom_parameters(thinkingtype/budget,max_tokens) onto the native langchain4j builder; always sendmax_tokens, defaulting to4096and stacking it above any thinking budget somax_tokens > thinking.budget_tokensholds; ignore non-positive token values.Change checklist
Issues
AI-WATERMARK
AI-WATERMARK: yes
Testing
AnthropicClientGeneratorTest— 21 cases covering the capability lookup, temperature gating (model + thinking-enabled),max_tokensdefaulting/forwarding/budget-clearing, and thinking forwarding. All passing.mvn compileclean;mvn spotless:applyclean.mvn test-compileis currently broken on pristinemain(unrelated JDK-25/Lombok annotation-processing mismatch), so the new test was compiled and run in isolation against the built main classes.Documentation
No user-facing docs change required. Known follow-up (out of scope): the sibling
LlmProviderAnthropicMapper.toCreateMessageRequest(playground / online-inference / proxy path) still forwardstemperatureunconditionally, so adaptive-thinking models 400 there too for API-created requests; this PR is deliberately scoped to the online-rules judge path per the issue.🤖 Generated with Claude Code