fix(llm): remove top-level cache_control from Claude provider (fixes #2512)

[nankingjing]

Fixes #2512

The top-level cache_control: {"type": "ephemeral"} param was being passed to Anthropic().messages.create() which the SDK rejects (cache_control belongs on individual content blocks, not the top-level request). This broke every Claude invocation.

Fix: Remove the top-level cache_control param. Users who need cache control can add it to individual content blocks per the Anthropic API docs.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8333cbad-de16-4dc7-a0b2-7ca7581d79e3

📥 Commits

Reviewing files that changed from the base of the PR and between 6685dea and 78a0e21.

📒 Files selected for processing (1)

• src/llm/providers/claude.py

💤 Files with no reviewable changes (1)

• src/llm/providers/claude.py

📜 Recent review details

⏰ Context from checks skipped due to timeout. (1)

• GitHub Check: Greptile Review

---

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes
  • Updated Claude chat and completion requests to avoid explicitly enabling ephemeral caching.
  • All other request handling and response behavior remain unchanged.

Walkthrough

The Claude provider request payload removes the top-level Anthropic cache_control parameter. Model, messages, token limits, request execution, and response parsing remain unchanged.

Changes

Claude provider request construction

Layer / File(s)	Summary
Remove invalid cache parameter src/llm/providers/claude.py	ClaudeProvider.generate no longer sends top-level ephemeral caching configuration to the Anthropic Messages API.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

• affaan-m/ECC#2133: Modifies Claude provider request construction around the Anthropic cache_control parameter.

Suggested reviewers: affaan-m

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The invalid top-level field is removed, but the required regression test for messages.create kwargs is still missing [#2512].	Add a regression test that inspects messages.create kwargs and verifies cache_control is not passed at the top level.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly states the main change: removing top-level cache_control from the Claude provider.
Description check	✅ Passed	The description matches the bug fix and accurately describes the change.
Out of Scope Changes check	✅ Passed	The PR only removes the invalid cache_control param, with no unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This PR fixes Claude request construction for Anthropic messages.

• Removes the invalid top-level cache_control field from Claude requests.
• Keeps the existing model, messages, and token parameters unchanged.
• Leaves system prompts joined into the existing Anthropic system parameter.

Confidence Score: 5/5

This looks safe to merge.

• No blocking issues found in the changed code.

Important Files Changed

Filename	Overview
src/llm/providers/claude.py	Removes the invalid top-level Claude cache_control request field while keeping the rest of the request shape unchanged.

_{Reviews (2): Last reviewed commit: "fix(llm): remove top-level cache_control..." | Re-trigger Greptile}

[greptile-apps]

Prompt Caching Becomes Unreachable

When generate() builds Claude requests, callers only pass plain LLMInput messages and Message.to_dict() does not expose a way to add per-block cache_control. Removing the provider-level cache setting means existing Claude calls lose the only automatic cache breakpoint in this path, so long repeated prompts can stop producing cache token usage and pay full input cost on every request.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[nankingjing]

Verified against the SDK floor this repo pins (anthropic>=0.111.0): top-level cache_control on messages.create() is the documented automatic prompt caching parameter on the first-party Anthropic API - the SDK auto-places the breakpoint on the last cacheable block - so it is accepted, not rejected, on every SDK version the project supports. That makes this finding substantively right: the removal trades away automatic caching to fix an error that does not reproduce on a pin-compliant install. The failure reported in #2512 points to an out-of-range environment (an SDK older than the pin, or a platform without automatic caching such as Bedrock/Vertex). Deliberately not adding a regression test asserting the parameter's absence, since that would enshrine the caching regression; flagging for maintainers to confirm the #2512 reproduction environment before merging this or #2515.

[daltino]

DECISION: APPROVE

This PR removes an unused "cache_control" parameter from the generate method in the Claude provider, resolving issue #2512. The change simplifies the payload without impacting functionality, aligning with clean code practices. No additional context or documentation updates are necessary, as this is a minor fix.

[rjjelley]

Local review from Codex automation: this overlaps with #2515 but lacks the same regression coverage and appears to remove the top-level parameter without preserving the structured prompt-cache behavior. Recommendation: close/supersede this in favor of #2515, or add regression tests and preserve cache-control-on-system-block behavior if this PR is kept alive.

[nankingjing]

@rjjelley Thanks for the pointer to #2515 - agreed the two PRs overlap. One finding worth weighing before either merges: with the SDK floor this repo pins (anthropic>=0.111.0), top-level cache_control on messages.create() is the documented automatic-prompt-caching parameter on the first-party Anthropic API (the SDK auto-places the cache breakpoint on the last cacheable block), so the failure in #2512 does not reproduce on a pin-compliant install - it indicates an older SDK or a platform without automatic caching (e.g. Bedrock/Vertex). Given that, this PR's removal disables caching outright, while #2515's restructuring preserves a cache point on the system block but gives up automatic last-block placement. Rather than adding regression tests that lock in either behavior, it seems better for maintainers to first confirm the environment behind #2512; if it turns out to be an unsupported configuration, the minimal correct change may be neither PR as-is. Happy to adapt this PR (e.g. platform-gate the parameter or align with #2515's structure) once that is settled.