Skip to content

Commit 2224a3d

Browse files
pk-zipstackclaudepre-commit-ci[bot]
authored
[FIX] Strip deprecated sampling params for Claude Opus 4.7 (#1934)
* Removed temperature parameter for claude opus 4.7 models * [FIX] Strip sampling params on Vertex AI path and anchor model regex Address review feedback on the Opus 4.7 sampling-param strip: - VertexAILLMParameters.validate() now returns the stripped dict, so Claude Opus 4.7 routed through Vertex AI (vertex_ai/claude-opus-4-7@...) no longer sends temperature/top_p/top_k. - Model pattern is now an anchored regex (`claude-opus-4-7(?=$|[-:@/v])`) so prefix collisions like claude-opus-4-70, claude-opus-4-75 do not silently strip sampling params from unrelated future models. The allowed delimiters cover every real id format: date suffix, Vertex @<date>, ARN paths, v1:0 version tag, and end-of-string. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * [FIX] Tighten Opus 4.7 sampling-strip and add test coverage Address PR #1934 review: - HIGH: anchor regex no longer admits `v`-prefixed alpha continuations. `(?=$|[-:@/v])` accepted `claude-opus-4-7verbose`/`vnext`/`variant`. Switched to `(?=$|[-:@/]|v\d)` — keeps `v1` / `v9` version tags valid, rejects bare-`v` continuations. - HIGH: `_strip_deprecated_sampling_params` now logs a debug breadcrumb when sampling params are present but no model id field matches. This is the silent-skip path for opaque Bedrock AIP ARNs / Azure Foundry deployments that omit the model id — operators hitting the upstream 400 now have a way to confirm detection was attempted-and-skipped vs never attempted. - MED: helper no longer mutates its input. Returns a shallow copy, matching the file's `result_metadata = adapter_metadata.copy()` style. - MED: docstring documents the contract change — returned dict may omit the `temperature` key. Consumers must use `.get("temperature")`. Grepped platform-service / SDK / workers / backend for direct `["temperature"]` reads; only one consumer accesses it and uses `.get()` already, so no caller-side change needed today. - LOW: comment drift — `substring-matched` → `regex-searched`; misleading route-prefix bullet rewritten as "leading text passes through because the regex is anchored only at the trailing edge"; docs URL switched from `platform.claude.com` (console host) to `docs.claude.com` (canonical docs host); docstring extends top_p/top_k strip rationale; redundant Foundry inline comment removed (helper docstring covers it). - MED: new `tests/test_sampling_strip.py` (57 cases) pins - 23 positive id formats (native, Bedrock foundation/cross-region/ARNs, Vertex `@<date>`, Azure deployments, dot/underscore separators, `v\d` version tags) - 18 negatives including prefix collisions (`4-70`, `4-75`, `4-79`, `4-71-...`) and bare-`v` continuations (`4-7verbose`/`vnext`/`variant`) - copy-not-mutate, all-three-params strip, AIP fallback via both `model` and `model_id`, double-opaque limitation + debug log, quiet-no-op for non-deprecated models - parametrized end-to-end strip across all four adapters (Anthropic, Bedrock, Azure AI Foundry, Vertex AI) — locks the Vertex AI gap that surfaced in this PR's review. All 278 sdk1 tests pass (221 pre-existing + 57 new). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [FIX] Resolve D205 by splitting docstring summaries The pre-commit.ci auto-fix moved the closing `"""` to its own line but left the multi-line summary intact, so D205 still flagged three docstrings on test_sampling_strip.py:158/177/234. Rewrite the three as single-line summary + blank + body so D205 actually passes. Also let ruff normalize the import block. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [FIX] Use pytest.approx for float equality (SonarCloud reliability) SonarCloud flagged four `== 0.5` checks against temperature reads in test_sampling_strip.py as "Do not perform equality checks with floating point values". Switch to `pytest.approx(0.5)` to match the project convention (test_gemini_adapter.py:42 uses the same form). Reword one trailing comment as a leading comment to keep the assertion under the 90-char line limit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * [FIX] Narrow strip-skipped breadcrumb to opaque AIP ARNs only Greptile flagged that `BaseChatCompletionParameters.temperature` declares `default=0.1`, so Pydantic's `model_dump()` always emits `temperature` — the previous `elif any(p in result for p in _DEPRECATED_SAMPLING_PARAMS)` fired for every routine call through these adapters (claude-3-5-sonnet, claude-opus-4-6, gpt-4o, …), producing log noise that mimicked a failed strip attempt. Replace the guard with `_looks_like_opaque_aip_arn(...)` keyed off the `application-inference-profile` substring — the one concrete case where the model id is genuinely unrecoverable from the string and the upstream will 400 if the underlying foundation model is Opus 4.7+. Add a regression test (`test_strip_does_not_log_when_sampling_params_present_ but_model_not_opaque`) that locks the no-noise invariant for the routine case. The existing test that exercises the breadcrumb path (`test_strip_skipped_when_both_fields_opaque_and_logs_debug`) still passes because both fields hold AIP ARNs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent aae622b commit 2224a3d

2 files changed

Lines changed: 427 additions & 4 deletions

File tree

unstract/sdk1/src/unstract/sdk1/adapters/base1.py

Lines changed: 127 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import inspect
33
import logging
44
import os
5+
import re
56
from abc import ABC, abstractmethod
67
from importlib import import_module
78
from typing import TYPE_CHECKING
@@ -15,6 +16,126 @@
1516

1617
logger = logging.getLogger(__name__)
1718

19+
# Anthropic models that have deprecated sampling parameters (`temperature`,
20+
# `top_p`, `top_k`). The patterns are regex-searched against the model id
21+
# after lowercasing and normalizing `.` / `_` to `-`. The match is anchored at
22+
# the trailing edge so that unrelated future ids (`claude-opus-4-70`,
23+
# `claude-opus-4-75`, `claude-opus-4-7verbose`) do not match. A single entry
24+
# covers every encoding of the id we have observed:
25+
# - Native Anthropic `claude-opus-4-7`, `anthropic/claude-opus-4-7`
26+
# - Bedrock foundation model `anthropic.claude-opus-4-7-<date>-v1:0`
27+
# - Bedrock cross-region profile `us.anthropic.claude-opus-4-7-...`,
28+
# `eu.`, `apac.`, `global.` variants
29+
# - Bedrock foundation-model ARN `arn:aws:bedrock:<region>::foundation-model/
30+
# anthropic.claude-opus-4-7-...`
31+
# - Bedrock inference-profile ARN `arn:aws:bedrock:<region>:<account>:
32+
# inference-profile/us.anthropic.claude-opus-4-7-...`
33+
# - Vertex AI `vertex_ai/claude-opus-4-7@<date>`
34+
# - Azure AI Foundry deployments whose name embeds `claude-opus-4-7`
35+
# Leading text (route prefixes like `converse/`, `invoke/`, `bedrock/`) passes
36+
# through because the regex is anchored only at the trailing edge.
37+
# Add new entries here when Anthropic deprecates sampling on more models.
38+
# Trailing anchor allows: end-of-string, or one of `-`/`:`/`@`/`/` (the
39+
# delimiters used in date suffixes, ARN paths, Vertex `@<date>`, and the
40+
# `v1:0` tag), or `v` followed by a digit (the version-tag start). A bare
41+
# `v` is intentionally rejected so alpha continuations like `4-7verbose` do
42+
# not silently match.
43+
# See https://docs.claude.com/en/about-claude/models/whats-new-claude-4-7
44+
_SAMPLING_DEPRECATED_MODEL_PATTERNS: tuple[re.Pattern[str], ...] = (
45+
re.compile(r"claude-opus-4-7(?=$|[-:@/]|v\d)"),
46+
)
47+
_DEPRECATED_SAMPLING_PARAMS: tuple[str, ...] = ("temperature", "top_p", "top_k")
48+
# Fields whose value can carry a model id. `model` is universal; `model_id` is
49+
# Bedrock's separate ARN field used for Application Inference Profile cost
50+
# tracking — when callers route through an AIP, the standard model id often
51+
# only appears here, not in `model`.
52+
_MODEL_ID_FIELDS: tuple[str, ...] = ("model", "model_id")
53+
# Substring of a Bedrock Application Inference Profile ARN; the rest of the
54+
# ARN is an opaque profile id so the underlying foundation model id is not
55+
# recoverable from the string. Used only to narrow the debug-breadcrumb path
56+
# on the strip's no-match branch.
57+
_OPAQUE_AIP_ARN_MARKER: str = "application-inference-profile"
58+
59+
60+
def _looks_like_opaque_aip_arn(value: str | None) -> bool:
61+
"""Return True when the value looks like a Bedrock AIP ARN.
62+
63+
Bedrock AIP ARNs do not carry the underlying foundation-model id in the
64+
string, so the sampling-strip detector cannot decide whether the call is
65+
bound for Claude Opus 4.7.
66+
"""
67+
return bool(value) and _OPAQUE_AIP_ARN_MARKER in value
68+
69+
70+
def _has_deprecated_sampling_params(model: str | None) -> bool:
71+
"""Return True when the model rejects sampling parameters.
72+
73+
Anthropic deprecated `temperature`, `top_p`, and `top_k` starting with
74+
Claude Opus 4.7; sending any of them yields a 400 from Anthropic and from
75+
the providers that proxy it (Bedrock, Azure AI Foundry, Vertex AI).
76+
77+
The check normalizes case and `.`/`_` separators to `-`, then regex-
78+
searches against the patterns with a trailing-edge boundary, so
79+
`claude-opus-4-70` and `claude-opus-4-7verbose` do not match. This
80+
catches every format that embeds the model id (foundation model ids,
81+
cross-region profiles, foundation-model ARNs, inference-profile ARNs,
82+
Vertex `@`-suffixed ids).
83+
84+
It does NOT catch:
85+
- Bedrock Application Inference Profile ARNs (e.g.
86+
`arn:aws:bedrock:...:application-inference-profile/abcd1234`), whose
87+
tail is an opaque profile id — the underlying model is not recoverable
88+
from the string. Pass the AIP ARN in `model_id` and keep the standard
89+
model id in `model`, or the strip won't fire.
90+
- Azure AI Foundry deployment names that omit the model id; rename the
91+
deployment to include `claude-opus-4-7` so detection works.
92+
"""
93+
if not model:
94+
return False
95+
normalized = model.lower().replace(".", "-").replace("_", "-")
96+
return any(rx.search(normalized) for rx in _SAMPLING_DEPRECATED_MODEL_PATTERNS)
97+
98+
99+
def _strip_deprecated_sampling_params(validated: dict[str, "Any"]) -> dict[str, "Any"]:
100+
"""Return a copy of `validated` with deprecated sampling params removed.
101+
102+
The input dict is not mutated. Returns a shallow copy so callers can rely
103+
on `before is not after` and follow the file's copy-then-mutate style.
104+
105+
`temperature` is the load-bearing case: `BaseChatCompletionParameters`
106+
declares `temperature: float | None = Field(default=0.1)`, so Pydantic's
107+
`model_dump()` re-emits the default even when the caller never set one.
108+
`top_p` and `top_k` are not declared fields and are normally dropped by
109+
Pydantic, but the strip defends against caller-supplied values flowing
110+
through `**adapter_metadata`.
111+
112+
Checks both `model` and `model_id` so Bedrock callers routing through an
113+
Application Inference Profile are covered when the standard model id only
114+
appears in `model_id`.
115+
116+
Contract change: the returned dict may not contain a `temperature` key.
117+
Consumers must read via `.get("temperature")`, not `dict["temperature"]`.
118+
"""
119+
result = dict(validated)
120+
if any(_has_deprecated_sampling_params(result.get(f)) for f in _MODEL_ID_FIELDS):
121+
for param in _DEPRECATED_SAMPLING_PARAMS:
122+
result.pop(param, None)
123+
elif any(_looks_like_opaque_aip_arn(result.get(f)) for f in _MODEL_ID_FIELDS):
124+
# An opaque Bedrock AIP ARN reached us with no Anthropic model id in
125+
# any field. If the underlying foundation model is Opus 4.7+, the
126+
# upstream call will 400 on `temperature is deprecated`; this debug
127+
# log makes the strip-skipped state distinguishable from a
128+
# never-attempted strip when debugging the resulting 400. The guard
129+
# is intentionally narrow — the broader "any sampling param present"
130+
# form fires for every routine call because Pydantic's `model_dump`
131+
# always re-emits the default `temperature=0.1`.
132+
logger.debug(
133+
"Sampling-param strip skipped for opaque Bedrock AIP ARN; no "
134+
"model id field matched a deprecation pattern. Model ids: %s",
135+
{f: result.get(f) for f in _MODEL_ID_FIELDS if result.get(f)},
136+
)
137+
return result
138+
18139

19140
def register_adapters(adapters: dict[str, dict[str, "Any"]], adapter_type: str) -> None:
20141
"""Register all SDK v1 adapters of given type.
@@ -462,7 +583,7 @@ def validate(adapter_metadata: dict[str, "Any"]) -> dict[str, "Any"]:
462583
if "thinking" in result_metadata:
463584
validated_data["thinking"] = result_metadata["thinking"]
464585

465-
return validated_data
586+
return _strip_deprecated_sampling_params(validated_data)
466587

467588
@staticmethod
468589
def validate_model(adapter_metadata: dict[str, "Any"]) -> str:
@@ -646,7 +767,8 @@ def validate(adapter_metadata: dict[str, "Any"]) -> dict[str, "Any"]:
646767
# Keys mode requires non-blank values, legacy (no auth_type) is
647768
# lenient. Reads auth_type from result_metadata since validation_
648769
# metadata strips it before Pydantic.
649-
return _resolve_bedrock_aws_credentials(result_metadata, validated)
770+
validated = _resolve_bedrock_aws_credentials(result_metadata, validated)
771+
return _strip_deprecated_sampling_params(validated)
650772

651773
@staticmethod
652774
def validate_model(adapter_metadata: dict[str, "Any"]) -> str:
@@ -735,7 +857,7 @@ def validate(adapter_metadata: dict[str, "Any"]) -> dict[str, "Any"]:
735857
if enable_extended_context:
736858
validated["extra_headers"] = {"anthropic-beta": "context-1m-2025-08-07"}
737859

738-
return validated
860+
return _strip_deprecated_sampling_params(validated)
739861

740862
@staticmethod
741863
def validate_model(adapter_metadata: dict[str, "Any"]) -> str:
@@ -945,7 +1067,8 @@ def validate(adapter_metadata: dict[str, "Any"]) -> dict[str, "Any"]:
9451067
adapter_metadata
9461068
)
9471069

948-
return AzureAIFoundryLLMParameters(**adapter_metadata).model_dump()
1070+
validated = AzureAIFoundryLLMParameters(**adapter_metadata).model_dump()
1071+
return _strip_deprecated_sampling_params(validated)
9491072

9501073
@staticmethod
9511074
def validate_model(adapter_metadata: dict[str, "Any"]) -> str:

0 commit comments

Comments
 (0)