Skip to content

[issue-7526][BE] fix: make Anthropic adaptive-thinking opt-out sync-proof#7590

Merged
Nimrod007 merged 5 commits into
mainfrom
Nimrod007/issue-7526-sync-proof-sampling-flag
Jul 23, 2026
Merged

[issue-7526][BE] fix: make Anthropic adaptive-thinking opt-out sync-proof#7590
Nimrod007 merged 5 commits into
mainfrom
Nimrod007/issue-7526-sync-proof-sampling-flag

Conversation

@Nimrod007

@Nimrod007 Nimrod007 commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Details

Follow-up to #7531. That PR opted the Anthropic adaptive-thinking models (claude-opus-4-7, claude-opus-4-8, claude-sonnet-5) out of sampling params using a per-constant flag in the AnthropicModelName enum:

CLAUDE_OPUS_4_8("claude-opus-4-8", false),

The enum constant list is regenerated by the recurring chore: sync provider model definitions job (#7088, #7315, #7582). That generator emits every model as a single-arg NAME("value") constant and has no notion of the extra capability argument. #7582 ran the sync ~18h after #7531 merged and silently dropped all three , false flags (reverting them to the single-arg constructor, which defaults supportsSamplingParams to true) — reopening #7526:

-    CLAUDE_OPUS_4_7("claude-opus-4-7", false),
-    CLAUDE_OPUS_4_8("claude-opus-4-8", false),
     ...
-    CLAUDE_SONNET_5("claude-sonnet-5", false);
+    CLAUDE_OPUS_4_7("claude-opus-4-7"),
+    CLAUDE_OPUS_4_8("claude-opus-4-8"),
+    CLAUDE_SONNET_5("claude-sonnet-5");

Current main still has the flags dropped, so online LLM-as-judge rules on these models 400 again with `temperature` is deprecated for this model whenever a temperature is present (API-created rules, which bypass the OPIK_6244 FE sanitizer — the exact case #7526/#7531 targeted).

Change

Move the opt-out out of the generated constant list into a hand-maintained ADAPTIVE_THINKING_MODELS set that supportsSamplingParams() consults. The constant list stays in the exact single-arg shape the generator emits, so future syncs can add/remove/reorder constants freely without ever clobbering the capability. Public API (supportsSamplingParams(String)) and behavior are unchanged; the field-backed Map + helper constructor are removed.

  • AnthropicModelName — drop supportsSamplingParams enum field, the this(value, true) helper ctor, and the field-derived map; add ADAPTIVE_THINKING_MODELS set + doc explaining why it must live outside the generated list.
  • AnthropicClientGenerator unchanged — the max_tokens / thinking / temperature-gating logic from [issue-7526][BE] fix: support Anthropic adaptive-thinking models as online-rule judges #7531 was never regressed (it lives outside the generated file).

Change checklist

  • User facing
  • Documentation update

Issues

Testing

  • Existing AnthropicClientGeneratorTest.SamplingParamsCapability cases (assert false for the three adaptive models) pass again — they were effectively dead on main since test-compile is currently broken there, which is why the [NA] [BE][FE] chore: sync provider model definitions #7582 regression went undetected.
  • New adaptiveThinkingModelsRemainEnumeratedAndRejectSamplingParams regression guard: asserts each adaptive model id still maps to a real enum constant (so the set can't drift when a sync renames/removes a constant) and reports no sampling-param support.
  • Verified live on staging (opik-backend @ the regressed build): an API-created online-judge rule on claude-opus-4-8 with temperature 400s on temperature is deprecated; the same rule without temperature scores cleanly (confirming [issue-7526][BE] fix: support Anthropic adaptive-thinking models as online-rule judges #7531's max_tokens defaulting is intact and the temperature opt-out is the only regressed piece).
  • Note: full mvn test-compile requires JDK 25 and is broken on pristine main (unrelated JDK-25/Lombok mismatch), so the suite was not run locally; the change is mechanical.

Documentation

No user-facing docs change required. Two follow-ups (out of scope): (1) the broken test-compile on main means these guards don't run in CI yet — worth fixing so this class of regression is caught; (2) the sync provider model definitions generator could be taught to preserve/emit capability flags, removing the need for a parallel set.

AI-WATERMARK

AI-WATERMARK: yes

  • Tools: Claude Code
  • Model(s): Claude Opus 4.8
  • Scope: full change (implementation + test), authored under human direction and review
  • Human verification: root cause reproduced end-to-end on staging; diff reviewed

🤖 Generated with Claude Code

…roof

PR #7531 opted claude-opus-4-7/4-8 and claude-sonnet-5 out of sampling
params via a per-constant flag (NAME("value", false)) in the
AnthropicModelName enum. The enum constant list is regenerated by the
recurring "sync provider model definitions" chore, which emits single-arg
NAME("value") constants and has no notion of the extra argument. #7582
ran that sync ~18h later and silently dropped all three flags, reverting
supportsSamplingParams() to true for these models and reopening #7526:
online LLM-as-judge rules on adaptive-thinking models 400 again with
"temperature is deprecated for this model".

Move the opt-out out of the generated constant list into a hand-maintained
ADAPTIVE_THINKING_MODELS set that supportsSamplingParams() consults, so
syncs can add/remove/reorder constants freely without clobbering the
capability. Behavior and the public API are unchanged.

Add a regression test asserting each adaptive model id still maps to a
real enum constant (so the set can't drift when a sync renames/removes a
constant) and reports no sampling-param support.

Verified on staging: with the flag lost, an API-created judge rule on
claude-opus-4-8 with a temperature 400s; with no temperature it scores
cleanly (max_tokens defaulting from #7531 is intact).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Nimrod007
Nimrod007 requested a review from a team as a code owner July 23, 2026 10:37
@github-actions github-actions Bot added java Pull requests that update Java code Backend tests Including test files, or tests related like configuration. 🟢 size/S labels Jul 23, 2026
@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

⏱️ pre-commit per-hook timing

Hook Description Result Duration
☕ spotless — java backend Format Java code 3.98s
Total (1 ran) 3.98s
⏭️ 40 skipped (no matching files changed)
Hook Description Result
🐍 trim trailing whitespace — python sdk Strip trailing whitespace ⏭️
🐍 fix end of files — python sdk Ensure files end in a newline ⏭️
🐍 ruff — python sdk Lint + autofix Python (ruff) ⏭️
🐍 ruff-format — python sdk Format Python code (ruff) ⏭️
🐍 mypy — python sdk Static type check ⏭️
🤖 trim trailing whitespace — optimizer Strip trailing whitespace ⏭️
🤖 fix end of files — optimizer Ensure files end in a newline ⏭️
🤖 check yaml — optimizer Validate YAML syntax ⏭️
🤖 check json — optimizer Validate JSON syntax ⏭️
🤖 check toml — optimizer Validate TOML syntax ⏭️
🤖 check for added large files — optimizer Block large files (>1MB) ⏭️
🔐 detect private key — optimizer Block committed private keys ⏭️
🤖 check for merge conflicts — optimizer Block merge-conflict markers ⏭️
🤖 check for case conflicts — optimizer Block case-only name clashes ⏭️
🤖 pyupgrade — optimizer Modernize Python syntax ⏭️
🤖 ruff — optimizer Lint + autofix Python (ruff) ⏭️
🤖 ruff-format — optimizer Format Python code (ruff) ⏭️
🤖 mypy — optimizer Static type check ⏭️
📓 nbstripout — optimizer notebooks Strip notebook output ⏭️
📝 markdownlint — optimizer Lint Markdown ⏭️
🔤 codespell — optimizer Fix common misspellings ⏭️
📊 radon cc — optimizer Cyclomatic-complexity gate ⏭️
📊 radon raw — optimizer Raw size metrics gate ⏭️
📊 xenon — optimizer Fail on complexity thresholds ⏭️
📊 lizard — optimizer Cyclomatic-complexity gate ⏭️
🧹 vulture — optimizer Find dead code ⏭️
🛡️ trim trailing whitespace — guardrails Strip trailing whitespace ⏭️
🛡️ fix end of files — guardrails Ensure files end in a newline ⏭️
🛡️ ruff — guardrails Lint + autofix Python (ruff) ⏭️
🛡️ ruff-format — guardrails Format Python code (ruff) ⏭️
🛡️ mypy — guardrails Static type check ⏭️
⚓ helm-docs Regenerate Helm chart README ⏭️
block non-public FE plugins Block non-public FE plugins ⏭️
🧪 pre-commit wrapper smoke tests Self-test the wrapper scripts ⏭️
🌐 eslint — frontend Lint + autofix JS/TS ⏭️
🌐 typecheck — frontend Whole-project tsc type check ⏭️
📘 eslint — typescript sdk Lint + autofix JS/TS ⏭️
📘 typecheck — typescript sdk Whole-project tsc type check ⏭️
⚙️ actionlint — github workflows Lint GitHub Actions workflows ⏭️
🐳 hadolint — dockerfiles Lint Dockerfiles ⏭️

@andrescrz andrescrz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's polish this a bit, to prevent this issue from happening again.

@JetoPistola

Copy link
Copy Markdown
Contributor

👋 Review summary

What looks good

Overall
Small, well-motivated, mechanical fix that re-resolves #7526 the durable way. It also underpins the sibling playground/inference-path fix in #7575, which consumes the same supportsSamplingParams contract — so the two land as a coherent pair. andrescrz has already flagged the substantive polish items (DRY the set against the enum values, use enum-backed ValueSource, value getter instead of toString(), single-membership check) — all worth doing, no need to repeat them here.

Inline comments: 1 question — on a residual coverage gap in the new regression guard (it protects against rename/removal of known adaptive models, but not against a sync adding a new adaptive model). Non-blocking.

🤖 Review posted via /review-github-pr

… guard

- Build ADAPTIVE_THINKING_MODELS from enum constant values instead of
  duplicated string literals, giving compile-time drift protection.
- Add Lombok @Getter and use getValue() instead of toString().
- Drive the regression guard from @EnumSource so renaming/removing a
  constant fails at discovery time.
- Soften docs to keep the "add new adaptive models by hand" step visible.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…KING_MODEL_IDS

The set holds serialized Anthropic API id strings used as lookup keys in
supportsSamplingParams(String); the "_IDS" suffix makes that explicit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Set.of(...).contains(null) throws NPE. Guard null/blank via StringUtils.isBlank
so the method honors its documented "unknown model names default to true"
contract. Covered by a new test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

TS SDK E2E Tests - Node 22

317 tests   315 ✅  13m 42s ⏱️
 38 suites    2 💤
  1 files      0 ❌

Results for commit bab46fc.

♻️ This comment has been updated with latest results.

Parameterize the null default-true test with @NullAndEmptySource + a
whitespace case so the StringUtils.isBlank branch is exercised for null,
empty, and whitespace inputs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Backend Tests - Integration Group 13

 49 files   49 suites   5m 13s ⏱️
348 tests 346 ✅ 2 💤 0 ❌
343 runs  341 ✅ 2 💤 0 ❌

Results for commit bab46fc.

♻️ This comment has been updated with latest results.

@JetoPistola

Copy link
Copy Markdown
Contributor

👋 Re-review summary (revision bab46fc)

The revision cleanly addresses every prior thread:

  • DRYADAPTIVE_THINKING_MODEL_IDS now references CLAUDE_OPUS_4_7.value / CLAUDE_OPUS_4_8.value / CLAUDE_SONNET_5.value instead of duplicated literals, so a sync that renames/removes a referenced constant now fails compilation rather than drifting.
  • Test drift — driven by @EnumSource(mode = INCLUDE, names = {…}), so renaming/removing an adaptive constant fails at test-discovery time; the set-scan was dropped.
  • value getter@Getter added; the test asserts via model.getValue(), not toString().
  • Residual-gap doc — both the production and test doc comments now spell out the one remaining manual step: a newly synced adaptive model still has to be added to the set by hand (the durable fix being generator-emitted capability). That was my only open question — resolved.

Bonus: null/blank handling is now explicit (StringUtils.isBlank(...)) and covered by a dedicated @NullAndEmptySource + blank case, with identical semantics to the old getOrDefault(…, true).

No net-new findings; CI is fully green. Nicely done. ⚠️ This is not a formal approval — a human reviewer should still approve.

🤖 Review posted via /review-github-pr

@JetoPistola JetoPistola left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addresses all review feedback (DRY via enum .value, @EnumSource drift guard, getValue(), residual-gap docs) plus explicit null/blank handling. Sound, sync-proof fix. CI green.

🤖 Approved via /review-github-pr follow-up

@Nimrod007
Nimrod007 merged commit 95833e1 into main Jul 23, 2026
88 of 89 checks passed
@Nimrod007
Nimrod007 deleted the Nimrod007/issue-7526-sync-proof-sampling-flag branch July 23, 2026 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Backend java Pull requests that update Java code 🟢 size/S tests Including test files, or tests related like configuration.

Projects

None yet

3 participants