fix(providers): widen ANCHOR_OFFSET_RAW_BLOCKS cache-reach budget from 10 to 20 (PRI-1821)

[ada-sen]

Problem

Anthropic's cache lookback window is ~20 raw content blocks per breakpoint. ANCHOR_OFFSET_RAW_BLOCKS (currently 10) sets how far behind the tail the stable anchor breakpoint sits, which bounds how much a conversation can grow between two successful calls before the prior cache write becomes unreachable. Post-deploy data (PRI-1819) found a real cache bust caused by a 41-block growth between successful calls — 11 blocks past the anchor path's current budget (Δ≤30, since Δ≤N+20 at N=10).

Change

Widen ANCHOR_OFFSET_RAW_BLOCKS from 10 to 20, raising the anchor-path budget to Δ≤40. No steady-state cost change: the tail breakpoint is always stamped on the last cacheable block of the last message regardless of N — moving the anchor doesn't leave anything uncached that wasn't already. (An earlier version of this PR body claimed ~$0.06/call extra cache_creation cost; that was wrong and has been removed — see jc's review below.)

While verifying this I found the anchor-offset math in the original proposal's writeup didn't hold up under its own formula (it claimed Δ≤50 at N=20; the formula it stated works out to Δ≤40). I corrected the in-code derivation comment to match, and noted there that a 41-block outlier would still exceed a Δ≤40 budget by one block — this narrows the gap rather than fully closing the class. The derivation comment now also spells out that the tail path (Δ≤20) and anchor path (N≤Δ≤N+20) only union gap-free for N≤21 — raising N further without adding another breakpoint opens a hole just above the tail path's own reach.

Three existing unit tests hardcoded a raw-block count sized for the old offset (10), so they silently lost their anchor once the offset grew past their fixture length. Rebuilt those fixtures to size off ANCHOR_OFFSET_RAW_BLOCKS instead of a literal, tightened two of them to assert exact anchor placement (raw distance, not cacheable-only distance) instead of a loose >= bound, and generalized the PRI-1821 regression test to loop the full Δ range instead of sampling only Δ=40 — so an over-widened N (e.g. 22) that opens a gap at Δ=21 now fails it.

Review response (#405, jc)

jc confirmed the constant change itself is correct and requested changes on six findings, all addressed in the latest commit:

1. Rebuilt the raw-vs-cacheable-distance test with an exact-placement assertion and a fixture that distinguishes raw counting from cacheable-only counting.
2. Rebuilt the thinking-block test so the thinking block sits exactly at the naive (tail − N) target, forcing the anchor to skip past it.
3. Rewrote the cache-control.ts derivation comment: tail path is Δ≤20 (not 10 as previously stated); anchor path is N≤Δ≤N+20 (not Δ≤N+20); the union is gap-free only for N≤21. Fixed the in-function comment's overbroad "reachable from the next request's tail" claim.
4. Generalized the regression test to loop Δ over the full range instead of sampling only Δ=40, so it now catches an over-widened N (e.g. 22) that opens a gap at Δ=21.
5. Fixed this PR body's cost claim (see the "Change" section above).
6. Updated stale (10) comments and gave the streaming/message-caching long-conversation fixtures headroom, sized off the constant.

Testing

• vitest run src/providers/__tests__/cache-control.test.ts src/providers/__tests__/cache-control-byte-stable.test.ts src/providers/__tests__/cache-control-streaming.test.ts src/providers/__tests__/anthropic-provider-message-caching.test.ts src/providers/__tests__/anthropic-provider-smoke-cache-markers.test.ts: 29/29 passing.
• Full packages/agent unit suite (vitest run --exclude 'src/__tests__/**' ...): 3813 passed, 35 skipped, 0 failed.
• tsc --noEmit: clean.
• eslint: clean.
• Mutation checks against the strengthened tests:
  • Adding 'thinking' to CACHEABLE_BLOCK_TYPES (jc's finding-1 mutation): now caught (3 tests fail, including the rebuilt thinking-block test).
  • Swapping the anchor's raw-distance check for a cacheable-only distance check (jc's finding-2 mutation, cacheablePositions.length - 1 - i >= ANCHOR_OFFSET_RAW_BLOCKS): now caught by the rebuilt exact-placement test (expected 2 to be 20).
  • ANCHOR_OFFSET_RAW_BLOCKS = 22 (over-widened N, jc's finding-4 scenario): now caught by the generalized regression test at Δ=21, with every other cache-control test still green — reproducing exactly what jc found against the old single-sample test.
  • ANCHOR_OFFSET_RAW_BLOCKS = 10 (reverted): the generalized regression test fails at Δ∈(30,40], preserving the original red-on-revert coverage for the PRI-1819 incident magnitude.
  • ANCHOR_OFFSET_RAW_BLOCKS = 20 (current): all of the above pass cleanly.

[obra]

Review from jc: competing adversarial reviewers, then a verifier that tried to refute every finding against the PR head and current origin/main. Verdict: changes-needed.

The constant change itself is correct: [0,20]∪[20,40] is contiguous, and it still applies cleanly on origin/main 4b4533c, which still has N=10. But the fixture rewrites silently removed coverage for two named invariants. I confirmed that the 'thinking cacheable' and 'cacheable-only distance' mutations both pass at the PR head. The comment derivation and the PR-body cost claim are also wrong. All the fixes are small.

Confirmed findings:

1. [important, fix: small] packages/agent/src/providers/__tests__/cache-control.test.ts:126-155 ('never stamps cache_control on a thinking block, even when one would otherwise be at the anchor distance')
   The padded fixture moves the anchor target away from the thinking block. The thinking block sits at raw 1 and the target is raw 18 (tail 38), so the test no longer exercises what its name says. Its comment ('A thinking block sits right where ... the naive anchor target would land') is false. This merges A1 and B2.
   Why it's real: I verified at PR head 08d628c that adding 'thinking' to CACHEABLE_BLOCK_TYPES fails only the unrelated 'last message contains only thinking blocks' test, and this test passes. Both reviewers reported that the same mutation fails this test on the base file.
   Suggested fix: Generate the fixture so the thinking block lands exactly ANCHOR_OFFSET_RAW_BLOCKS raw blocks before the tail. For example, put the thinking group first, then add exactly N-1 single cacheable blocks after it and end with the tail. Assert that the anchor lands on the nearest cacheable block before the thinking block and that no thinking block carries cache_control. Fix the comment.
2. [important, fix: small] packages/agent/src/providers/__tests__/cache-control.test.ts:88-124 ('places anchor at least N RAW blocks behind tail, counting thinking blocks')
   The exact anchor-index assertion was replaced with tailIdx - anchorIdx >= N. An implementation that measures distance in cacheable-only blocks also satisfies that, so the test no longer checks its namesake property, and nothing else in the suite catches that regression. This merges A2 and B1.
   Why it's real: I verified that the mutation cacheablePositions.length - 1 - i >= ANCHOR_OFFSET_RAW_BLOCKS passes 15/15 on the PR's cache-control.test.ts. Reviewer A reports the full provider suite stays green, and the base test file catches it.
   Suggested fix: Assert exact placement. Compute the expected index as the last cacheable raw index with tailIdx - idx >= N from the generated layout (or toBe(tailIdx - N) when the layout guarantees a cacheable block there). A test that is also sensitive to raw vs cacheable counting should assert that the anchor distance is strictly less than what cacheable-only counting would give.
3. [minor, fix: trivial] packages/agent/src/providers/cache-control.ts:16-27 and :165-170
   The rewritten derivation comment is wrong or misleading in three places. (a) '(Δ ≤ 10 via the plain tail-to-tail path)' should say Δ ≤ 20. (b) The anchor path covers Δ∈[N, N+20], not Δ ≤ N+20, so for N > 21 the union has a hole at Δ∈(20, N). The comment's advice to 'revisit N again' reads as if a bigger N is always safer. (c) The in-function comment still claims the offset 'keeps the anchor reachable from the next request's tail breakpoint', which at N=20 holds only when Δ=0. This merges A3, A6-comment and B3/B4. On A3's N=21 point: the code covers the 41-block incident at N=21 with no hole, which is worth a sentence, but the ~20 lookback is approximate and 20 is a defensible margin choice. The PR body also calls the choice gap-narrowing, so this is not a defect.
   Why it's real: Reading the code: the tail is always stamped, and its lookback reaches prev_tail for Δ ≤ 20. The anchor is the first cacheable block at distance ≥ N, so it reaches prev_tail only when N ≤ d ≤ Δ ≤ d+20. The comment text is as quoted.
   Suggested fix: Rewrite the comment. Tail path: Δ ≤ 20. Anchor path: N ≤ Δ ≤ N+20. The union is contiguous only for N ≤ 21, so never raise N past 21 without adding a breakpoint. Also fix or remove the in-function 'reachable from the next request's tail' clause.
4. [minor, fix: small] packages/agent/src/providers/__tests__/cache-control.test.ts:217-252 (PRI-1821 regression test)
   The regression test samples only Δ=40, so it cannot catch an over-widened N that opens a hole just above the tail lookback (Δ=21..N-1).
   Why it's real: I verified that with N=22 all 17 cache-control* tests pass, even though Δ=21 then has no full-prefix hit.
   Suggested fix: Loop Δ over 0..ANCHOR_OFFSET_RAW_BLOCKS+20 and assert (tail2-tail1 ≤ 20) || (0 ≤ anchor2-tail1 ≤ 20). Size turn 1 off the constant too.
5. [minor, fix: trivial] PR #405 body, 'Change' section
   The cost claim ('10 more trailing raw blocks going uncached per turn ($0.06/call extra cache_creation)') is false for the code. The tail breakpoint is always stamped on the last cacheable block of the last message whatever N is, so moving the anchor leaves nothing uncached. For Δ ≤ 20 the cost is unchanged, and for Δ∈(30,40] N=20 is cheaper. This merges A5 and B5.
   Why it's real: cache-control.ts:162-163 picks the tail independently of ANCHOR_OFFSET_RAW_BLOCKS, and the constant is used only in the anchor loop. The PR gives no measurement behind the $0.06 figure.
   Suggested fix: Edit the PR body to drop the cost claim, or replace it with: 'no steady-state cost change; the tail breakpoint always covers the full prefix'.
6. [minor, fix: small] packages/agent/src/providers/__tests__/cache-control-streaming.test.ts:176; anthropic-provider-message-caching.test.ts:250,296; anthropic-provider-smoke-cache-markers.test.ts:176
   Comments in other tests still describe the offset as 10. The streaming and message-caching anchor fixtures are hardcoded at 25 raw blocks, which leaves only 4 blocks of headroom over N=20. That is the same brittleness the PR says it fixed. This merges A6-tests and B6.
   Why it's real: The grep output shows the '(10)' and '10-block offset' comments at those lines. The tests pass today at N=20.
   Suggested fix: Update the comments to refer to ANCHOR_OFFSET_RAW_BLOCKS without a literal, and size those fixtures off the constant (e.g. longConversation(Math.ceil(N/4)+2)).

Refuted by the verifier: Separate finding (A3): N=20 should have been N=21 to cover the 41-block incident; Implied concern that the change interacts with the API's 4-cache_control-block limit

https://claude.ai/code/session_01PvaV8ypCvokp6DCcW6rT7f

[ada-sen]

Addressed all six findings at 448ade0 (rebased onto current main @ 4b4533c).

1. Rebuilt the raw-vs-cacheable-distance test (places anchor at EXACTLY the raw distance ANCHOR_OFFSET_RAW_BLOCKS behind tail...) with an exact-placement assertion derived from a hand-built raw layout, plus an explicit check that it differs from what cacheable-only counting would produce.
2. Rebuilt the thinking-block test (never stamps cache_control on a thinking block, even when one sits exactly at the naive anchor target) so the thinking block sits at exactly tail − N, forcing the anchor to skip past it to the one real earlier candidate.
3. Rewrote the cache-control.ts derivation comment: tail path is Δ≤20 (not 10, that was a typo carried over from the old N); anchor path is N≤Δ≤N+20 (not "Δ≤N+20" as a standalone claim); the union is gap-free only for N≤21, with "revisit N" reworded so it doesn't read as "bigger N is always safer." Also fixed the in-loop comment's overbroad "reachable from the next request's tail" claim.
4. Generalized the regression test to loop Δ over 0..max(N+20, 40) instead of sampling only Δ=40, asserting (tail2−tail1≤20) || (0≤anchor2−tail1≤20) at each point, with turn 1 sized off the constant too.
5. Fixed the PR body's cost claim — replaced with "no steady-state cost change; the tail breakpoint always covers the full prefix."
6. Updated the stale (10) comments in cache-control-streaming.test.ts, anthropic-provider-message-caching.test.ts, and anthropic-provider-smoke-cache-markers.test.ts, and gave the streaming/message-caching long-conversation fixtures headroom by sizing them off ANCHOR_OFFSET_RAW_BLOCKS instead of a hardcoded 6-round-trip / 25-raw-block count.

Mutation evidence:

• Your finding-1 mutation (adding 'thinking' to CACHEABLE_BLOCK_TYPES) now fails 3 tests, including the rebuilt thinking-block test.
• Your finding-2 mutation (cacheablePositions.length - 1 - i >= ANCHOR_OFFSET_RAW_BLOCKS) now fails the rebuilt exact-placement test (expected 2 to be 20 — 2 is exactly what the cacheable-only-distance formula predicts).
• ANCHOR_OFFSET_RAW_BLOCKS = 22 now fails the generalized regression test at Δ=21 (every other cache-control test still passes, reproducing what you found against the old test).
• ANCHOR_OFFSET_RAW_BLOCKS = 10 (reverted) fails the same generalized test at Δ∈(30,40], preserving red-on-revert coverage for the PRI-1819 incident magnitude.
• ANCHOR_OFFSET_RAW_BLOCKS = 20 (current): all green.

Also ran the full packages/agent unit suite (3813 passed, 35 skipped, 0 failed), tsc --noEmit (clean), and eslint (clean).