From fb98726d0a35d5c978f4f9451c57a39d06d6729a Mon Sep 17 00:00:00 2001
From: Thejesh <35212698+thejesh23@users.noreply.github.com>
Date: Fri, 17 Jul 2026 13:04:32 -0700
Subject: [PATCH] fix(llm/providers/claude): attach cache_control to system
 block, not top-level (#2515)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* 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.
---
 src/llm/providers/claude.py   |  9 ++++++--
 tests/test_claude_provider.py | 42 ++++++++++++++++++++++++++++++++++-
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/src/llm/providers/claude.py b/src/llm/providers/claude.py
index 1acc7e67..20b45aed 100644
--- a/src/llm/providers/claude.py
+++ b/src/llm/providers/claude.py
@@ -67,10 +67,15 @@ class ClaudeProvider(LLMProvider):
                 "model": model,
                 "messages": api_messages,
                 "max_tokens": input.max_tokens if input.max_tokens else 16000,
-                "cache_control": {"type": "ephemeral"},
             }
             if system_parts:
-                params["system"] = "\n\n".join(system_parts)
+                params["system"] = [
+                    {
+                        "type": "text",
+                        "text": "\n\n".join(system_parts),
+                        "cache_control": {"type": "ephemeral"},
+                    }
+                ]
             if input.tools:
                 params["tools"] = [tool.to_anthropic_tool() for tool in input.tools]
             if not _uses_adaptive_thinking_only(model):
diff --git a/tests/test_claude_provider.py b/tests/test_claude_provider.py
index 29f0256f..7a4d54f9 100644
--- a/tests/test_claude_provider.py
+++ b/tests/test_claude_provider.py
@@ -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