mirror of
https://github.com/Jeuners/ECC.git
synced 2026-09-09 15:02:30 +02:00
fix(llm/providers/claude): attach cache_control to system block, not top-level (#2515)
* fix(llm/providers/claude): attach cache_control to system block, not top-level
The Anthropic Messages API does not accept `cache_control` as a top-level
request parameter — it is a per-content-block field. Passing it at the top
level raises `TypeError` in the Python SDK (which validates kwargs against
`messages.create()`'s signature) or a `400 unknown_parameter` from the API,
so every ClaudeProvider.generate() call fails.
Move `cache_control: {"type": "ephemeral"}` onto the last system-prompt
block so ephemeral prompt caching still works when a system prompt is
present, and drop it when there isn't one (nothing to cache).
Existing tests didn't catch this because `FakeMessages.create(**_params)`
accepted anything and ignored the kwargs. FakeMessages now records
`last_params`, and two regression tests assert that:
- `cache_control` never appears as a top-level param, and
- when a system prompt is set, `cache_control` rides on the last block.
Fixes #2512
* test(claude_provider): split composite isinstance+truthiness assertion (PT018)
Ruff PT018 flagged the combined `isinstance(system, list) and system` check
in `test_generate_does_not_pass_cache_control_as_top_level_param`. Split
it into two focused asserts (`isinstance(system, list)` then `assert system`)
so a failure points at the exact violation instead of a compound condition.
Behavior unchanged; 6/6 tests still pass.
Addresses CodeRabbit review on #2515.
This commit is contained in:
parent
ed38744605
commit
fb98726d0a
2 changed files with 48 additions and 3 deletions
|
|
@ -10,8 +10,10 @@ from llm.providers.claude import ClaudeProvider
|
|||
class FakeMessages:
|
||||
def __init__(self, response: SimpleNamespace) -> None:
|
||||
self.response = response
|
||||
self.last_params: dict[str, Any] = {}
|
||||
|
||||
def create(self, **_params: object) -> SimpleNamespace:
|
||||
def create(self, **params: object) -> SimpleNamespace:
|
||||
self.last_params = dict(params)
|
||||
return self.response
|
||||
|
||||
|
||||
|
|
@ -109,3 +111,41 @@ def test_generate_text_only_has_no_tool_calls() -> None:
|
|||
|
||||
assert output.content == "Hello."
|
||||
assert output.tool_calls is None
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_generate_does_not_pass_cache_control_as_top_level_param() -> None:
|
||||
# cache_control is a per-content-block field on the Anthropic Messages API,
|
||||
# not a top-level parameter. Passing it at the top level raises TypeError
|
||||
# in the Anthropic Python SDK (or a 400 from the API).
|
||||
provider = make_provider(make_response([SimpleNamespace(type="text", text="ok")]))
|
||||
|
||||
provider.generate(
|
||||
LLMInput(
|
||||
messages=[
|
||||
Message(role=Role.SYSTEM, content="system prompt"),
|
||||
Message(role=Role.USER, content="hi"),
|
||||
]
|
||||
)
|
||||
)
|
||||
|
||||
params = provider.client.messages.last_params
|
||||
assert "cache_control" not in params
|
||||
|
||||
# When a system prompt is present, cache_control should ride on the last
|
||||
# system content block so ephemeral prompt caching still works.
|
||||
system = params.get("system")
|
||||
assert isinstance(system, list), "system should be sent as a list of content blocks"
|
||||
assert system, "system content-block list should not be empty"
|
||||
assert system[-1].get("cache_control") == {"type": "ephemeral"}
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_generate_without_system_does_not_set_system_or_cache_control() -> None:
|
||||
provider = make_provider(make_response([SimpleNamespace(type="text", text="ok")]))
|
||||
|
||||
provider.generate(LLMInput(messages=[Message(role=Role.USER, content="hi")]))
|
||||
|
||||
params = provider.client.messages.last_params
|
||||
assert "cache_control" not in params
|
||||
assert "system" not in params
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue