_apply_per_block_caching_fallback independently re-reads and re-interprets the cache settings (anthropic_cache / anthropic_cache_messages) via a different code path than _build_automatic_cache_control. The latter handles the conflict check, the deprecation warning, and the True → '5m' normalization — none of which are replicated here. That works today because _build_automatic_cache_control is always called first, but it's fragile.

Consider resolving the effective cache setting once — either in _build_automatic_cache_control (returning both the top-level param and the resolved TTL/boolean), or in a small shared helper — and passing the resolved value into _apply_per_block_caching_fallback instead of having it re-derive it from raw settings. This would also let you drop the or chain here and the duplicate '5m' if ... is True else ... on line 1262.

@DouweM — judgment call on a refactor.

The bot's concern: _apply_per_block_caching_fallback re-reads anthropic_cache / anthropic_cache_messages and re-derives the TTL, duplicating the resolution logic from _build_automatic_cache_control. Works today because _build_automatic_cache_control is always called first (so the deprecation warning fires and the conflict raises), but it's fragile to reorder.

Concrete shape of the refactor: have _build_automatic_cache_control return a (top_level_param_or_None, resolved_ttl_or_None) tuple or a small dataclass, pass it into _apply_per_block_caching_fallback instead of having it re-derive from model_settings. Each callsite already calls them back-to-back so plumbing is straightforward.

It's a real improvement, ~15-20 lines of change. Want me to do it?

Several of the new tests (test_automatic_cache_control_none_on_unsupported_clients, test_anthropic_cache_per_block_fallback_on_unsupported_clients, test_deprecated_cache_messages_per_block_fallback_on_unsupported_clients, test_per_block_fallback_preserves_existing_cache_control) invoke private methods (_build_automatic_cache_control, _apply_per_block_caching_fallback) directly.

The test guidelines prefer testing through public APIs. Since the Bedrock/Vertex behavior is already covered end-to-end by the VCR test test_anthropic_cache_bedrock_real_api and the integration tests use mock clients + agent.run, these private-method tests are largely redundant. Consider consolidating them into integration tests that exercise the same behavior through agent.run with mock Bedrock/Vertex clients — similar to how test_anthropic_cache_messages_deprecated already works.

@DouweM — judgment call on test consolidation.

The bot's point is fair: test_automatic_cache_control_none_on_unsupported_clients, test_anthropic_cache_per_block_fallback_on_unsupported_clients, test_deprecated_cache_messages_per_block_fallback_on_unsupported_clients, and test_per_block_fallback_preserves_existing_cache_control all poke at private methods directly. tests/CLAUDE.md says to test through public APIs.

There's some end-to-end coverage already: test_anthropic_cache_bedrock_real_api (VCR, Bedrock multi-turn with cache) and test_anthropic_cache_messages_deprecated (mock client, end-to-end via agent.run).

But the private-method tests also cover edge cases the end-to-end tests don't:

• Vertex (not just Bedrock) — no Vertex cassette exists
• TTL passthrough on fallback for '1h' — currently only True is end-to-end tested
• CachePoint-preserved-by-fallback — no end-to-end test

Options:

1. Leave as-is (private-method tests exist because end-to-end coverage is incomplete).
2. Add mock-client Vertex/TTL/CachePoint tests that exercise agent.run, then delete the private-method tests.
3. Delete the private-method tests now, accept the coverage gap.

My lean is (2), but it's ~100 lines of churn on a PR that's already large. Want me to do it, or leave it for a follow-up?