Skip to content

fix(anthropic): keep the caller's cache_control when an image block is rewritten - #5867

Closed
vadymhimself wants to merge 1 commit into
lidge-jun:devfrom
vadymhimself:fix/preserve-cache-control-on-image-rewrite
Closed

vadymhimself wants to merge 1 commit into
lidge-jun:devfrom
vadymhimself:fix/preserve-cache-control-on-image-rewrite

Conversation

@vadymhimself

@vadymhimself vadymhimself commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The Anthropic image pipeline rebuilds an image block as a fresh object literal, so every sibling property on that block is silently dropped — including the caller's cache_control breakpoint:

function textify(ref: ImageBlockRef, text: string): void {
  ref.container[ref.index] = { type: "text", text };
}

function replaceImage(ref: ImageBlockRef, data: string, mediaType: string): void {
  ref.container[ref.index] = { type: "image", source: { type: "base64", media_type: mediaType, data } };
}

src/adapters/anthropic-image-normalize.ts (both) and src/adapters/anthropic-image-guard.ts (textify).

On the native Anthropic lane this pipeline runs over the caller's raw body — src/server/messages-native.ts prepareNativeBody, and src/server/claude-messages.ts before the forward. So when a client's cache breakpoint sits on an image block and that image is re-encoded or textified, the breakpoint is destroyed before the send and the prefix is re-written instead of read.

Why this matters

This is upstream's own stated invariant. tests/claude-integration/claude-native-passthrough.test.ts asserts

// Body untouched: thinking signature, cache_control, max_tokens all intact.
expect(hit.body).toEqual(claudeBody());

That test passes today only because its fixture contains no image that the pipeline rewrites. The lane promises passthrough; the image pipeline quietly breaks it for exactly the requests that carry the largest payloads.

Claude Code places a rolling cache breakpoint on the newest user turn. When that turn's content is an image — a pasted screenshot, a Read of a PNG, a screenshot tool_result — the breakpoint lands on the image block and is dropped. An isolated image turn costs one turn's delta. A screenshot-driven loop, where consecutive turns each carry an image, is the pathological case: every rolling breakpoint lands on an image, only the tools/system prefix survives, and the whole conversation tail is re-written each turn. For a 60k-token prefix with a 20k stable head that is ~40k tokens charged at the cache-write multiplier instead of the cache-read one, every turn.

The change

Preserve cache_control across the rebuild, and only cache_control:

export function cacheControlOf(ref: ImageBlockRef): { cache_control?: unknown } {
  const existing = (ref.container[ref.index] as { cache_control?: unknown } | undefined)?.cache_control;
  return existing === undefined ? {} : { cache_control: existing };
}

spread into the three rebuild sites. Nothing else is carried over — an image block's source must never survive onto a text block, and both new tests assert source is undefined after textification.

One shared helper rather than three inline copies: the two textify implementations live in different modules, anthropic-image-normalize.ts already imports from anthropic-image-guard.ts, and the rule ("only cache_control, never the rest of the block") is subtle enough that duplicated copies would drift. If you would rather not widen that module's exported surface, inlining the two lines at each site is a trivial follow-up.

Not changed: the tier logic, the guard policy, the call sites, and the decision of which images get rewritten. This only stops the rewrite from discarding a property it was never asked to touch.

src/adapters/anthropic.ts has a third call into these helpers, on adapter-built wire messages rather than the caller's body. The passthrough argument does not apply there, but the fix covers it too since it lives in the shared functions.

Verification

Proven red before green. Reverting only the two source files, with the tests untouched:

(fail) enforceAnthropicImageLimits > a cache_control breakpoint survives textification
  expect(content[0].cache_control).toEqual({ type: "ephemeral", ttl: "1h" })
  -  { "ttl": "1h", "type": "ephemeral" }
  +  undefined

(fail) normalizeAnthropicImages — real Bun.Image path > a cache_control breakpoint survives both re-encoding and textification
  expect(content[0].cache_control).toEqual({ type: "ephemeral" })
  -  { "type": "ephemeral" }
  +  undefined

 54 pass, 2 fail

Restored: 56 pass, 0 fail, 231 expect() calls.

The normalize case covers both of that module's sites in one test: a real 4000x3000 PNG that actually hits replaceImage (it asserts the encoded data changed, so the re-encode genuinely ran) and undecodable bytes that hit textify, each carrying a distinct cache_control.

bun run typecheck                                    # clean
bun test <the two image files>                       # 56 pass / 0 fail / 231 expect()
bun test claude-native-passthrough.test.ts           # 20 pass / 0 fail — still green
bun run structure:check                              # structure/ SSOT checks passed
bun run privacy:scan                                 # Privacy scan passed
git diff --check                                     # clean

Wider sweep, to show the sibling image lanes are untouched: tests/adapters/anthropic/, kiro-images, openai-chat-image-normalization, chat-native-image-normalization together give 581 pass / 0 fail across 35 files. The Kiro and OpenAI lanes route through normalizeImageTargets with their own replace/drop and are unaffected.

No existing test pins the lossy shape: toEqual({ type: "text" across tests/ and src/ returns only unrelated pipelines, and the two Anthropic image test files contain no toEqual on a rebuilt block.

Not run: the full bun run test.

🤖 Generated with Claude Code

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • Required local validation passed; commands, results, and any full-suite exception are documented. — typecheck clean; the two image suites 56 pass / 0 fail / 231 expect(); claude-native-passthrough 20 pass / 0 fail; wider image sweep 581 pass / 0 fail across 35 files; structure:check, privacy:scan, git diff --check clean. Full bun run test not run.

  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge). — 0 behind f353aac85.

  • I resolved all correct Codex and CodeRabbit findings. — CodeRabbit review completed with no findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • Bug Fixes
    • Image-to-text conversions and image normalization now preserve cache-control settings, including expiration details.
    • Converted text blocks continue to omit image source data.

The Anthropic image pipeline rebuilds a block as a fresh object literal, so
every sibling property of `type` is silently discarded — including the
caller's `cache_control` breakpoint:

- anthropic-image-normalize.ts `textify()` (undecodable/bomb/overflow notes)
- anthropic-image-normalize.ts `replaceImage()` (tier resize/re-encode)
- anthropic-image-guard.ts `textify()` (Rules 1, 1b, 2, 3, 4)

Both run over the caller's raw body on the native Anthropic lane
(messages-native.ts `prepareNativeBody`, claude-messages.ts), so when Claude
Code's rolling cache breakpoint sits on an image block — a pasted screenshot,
a `Read` of a PNG, a screenshot `tool_result` — and that image is textified or
re-encoded, the breakpoint is destroyed. The prefix is then re-written instead
of read; in a screenshot-driven loop the whole conversation tail gets
re-written every turn.

That lane already promises passthrough in its own test:
tests/claude-integration/claude-native-passthrough.test.ts asserts
`expect(hit.body).toEqual(claudeBody())` under "Body untouched: thinking
signature, cache_control, max_tokens all intact." The image pipeline quietly
broke that promise; the test passes today only because its fixture has no
image that gets normalized.

Carry `cache_control` across the rebuild at all three sites via one shared
`cacheControlOf()` helper. Only `cache_control` is carried: blanket-spreading
the old block would leave an image block's `source` on a text block.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 45e38636-919e-479c-8960-c82a5a68d30c

📥 Commits

Reviewing files that changed from the base of the PR and between f353aac and ea9a662.

📒 Files selected for processing (4)
  • src/adapters/anthropic-image-guard.ts
  • src/adapters/anthropic-image-normalize.ts
  • tests/adapters/anthropic/anthropic-image-guard.test.ts
  • tests/adapters/anthropic/anthropic-image-normalize.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The Anthropic image guard and normalization paths now preserve cache_control metadata when they replace image blocks with text or normalized images. Tests cover metadata preservation and confirm that textified blocks do not retain the original image source.

Changes

Anthropic image cache metadata

Layer / File(s) Summary
Preserve cache metadata during image replacement
src/adapters/anthropic-image-guard.ts, src/adapters/anthropic-image-normalize.ts, tests/adapters/anthropic/anthropic-image-guard.test.ts, tests/adapters/anthropic/anthropic-image-normalize.test.ts
cacheControlOf returns the image block’s cache_control property, or an empty object when it is undefined. Textification and image normalization carry that metadata to replacement blocks. Tests check metadata preservation and confirm textified blocks do not retain source.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Security Architecture Review

Security architecture risk: 🔵 Low · up to ea9a6

The change preserves a caller-selected cache breakpoint on rewritten images. It does not add an endpoint or relax image limits, but provider-side caching behavior and full security coverage were not established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The affected scope is caller-supplied cache metadata on rewritten blocks in existing Anthropic requests. The reviewed changes do not add a route or grant new authority; downstream provider caching behavior was not independently verified.

Trust Boundaries and Controls

  • observed — Caller-controlled messages still pass through image normalization and the existing image-limit guard before native forwarding. The changed code preserves metadata rather than changing those controls.

Resilience and Maintainability Implications

  • inferred — Completed per-block rewrites retain their metadata, but normalization does not roll back earlier mutations after cancellation or fatal failure. No evidence establishes an atomic-cancellation contract, and the metadata-preservation edit does not introduce the partial-mutation behavior.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preserving the caller's cache_control when Anthropic image blocks are rewritten. It matches the re-encoding and textification changes.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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

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

@github-actions github-actions Bot added the bug Something isn't working label Sep 25, 2026
@github-actions

github-actions Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ Required local validation passed; commands, results, and any full-suite exception are documented.
  • ✅ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

✅ 4/4 boxes ticked.

This pull request has been marked Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers notified: @lidge-jun @Ingwannu

@github-actions
github-actions Bot marked this pull request as draft September 25, 2026 18:01
@github-actions
github-actions Bot marked this pull request as ready for review September 25, 2026 18:12
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 63 / 80

이 글은 Anthropic으로 그림을 보낼 때, 그림을 글로 바꾸거나 다시 압축하면 캐시 표식이 사라지는 일을 막아요.

표식 이름은 cache_control이에요. 그림 블록에서 type 옆에 붙어요. 예전 코드는 그 칸을 새 객체로 통째로 바꿔 넣었어요. type과 새 내용만 남고, 옆에 있던 표식은 없어졌어요. 바뀌는 곳은 세 군데예요. 한도를 넘는 그림을 글로 바꾸는 anthropic-image-guard.ts의 textify, 압축하는 anthropic-image-normalize.ts의 replaceImage, 읽지 못하는 그림을 글로 바꾸는 그 파일의 textify예요.

이 일은 어댑터가 메시지를 만든 뒤에도 돌아요. 이미 Anthropic 모양으로 들어온 본문에도 돌아요. src/server/messages-native.ts의 prepareNativeBody, src/server/claude-messages.ts의 네이티브 전달이 보내기 전에 둘 다 호출해요. 작성자 말로는 Claude Code가 가장 최근 사용자 말에 표식을 붙이고, 그 말이 스크린샷이면 표식이 그림 위에 있어요. 표식이 없으면 도구와 시스템만 캐시에서 읽고, 대화의 뒷부분은 턴마다 다시 써요. 다시 쓰기는 읽기보다 비싸요. 토큰 숫자는 이 글의 설명이고, 여기서 다시 재지는 않았어요.

고침은 표식만 새 블록에 복사해요. 그림의 source는 글 블록에 남지 않아요. 복사 함수는 cacheControlOf 하나고, 가드 파일에서 꺼내 노멀라이즈가 같이 써요. 테스트는 가드의 글 변환, 노멀라이즈의 재압축, 노멀라이즈의 글 변환을 봐요. 글이 된 블록에 source가 없는지도 봐요.

어댑터는 그림을 고친 뒤에 applyPromptCaching으로 표식을 새로 붙여요. 손님이 미리 붙여 둔 표식을 지키는 이유는 네이티브 전달 쪽에 있어요. 공유 함수라 어댑터 경로도 같이 고쳐져요.

라인 - src/adapters/anthropic-image-guard.ts cacheControlOf. undefined가 아니면 값을 그대로 옮겨요. null, 글자, 배열도 남아요. 그림을 갈아끼우던 예전에는 그 값이 사라져서 요청이 통과할 수 있었어요. 이제는 이상한 표식이 남고, Anthropic이 그 본문을 400으로 거절할 수 있어요. 그림을 안 바꾼 요청은 원래도 그 표식을 받았어요.

라인 - tests/adapters/anthropic/anthropic-image-guard.test.ts, tests/adapters/anthropic/anthropic-image-normalize.test.ts. 새 테스트는 사용자 메시지 맨 바깥 블록만 봐요. 글이 예로 든 스크린샷 tool_result 안 그림은 테스트에 없어요. collectImageRefs는 tool_result의 content 배열까지 들어가고, 그 그림도 같은 textify와 replaceImage를 타요. 코드는 그 그림도 고치고, 테스트는 그 모양을 잠그지 않아요.

메인테이너의 판단이 필요한 지점

이번 턴에 그림 바이트가 바뀌면, 표식을 남겨도 그 턴은 예전 그림을 캐시에서 읽지 못해요. 내용이 달라서 그 자리는 새 캐시 쓰기예요. 다음 턴에 같은 바이트가 다시 나가면 그때 읽어요. 글로 바뀐 문장은 매번 같아서, 그 경우는 바로 안정돼요. 설명의 "읽히던 것이 다시 쓰인다"는 바이트가 그대로인 경우에 맞아요. 이번 범위로 둘지 정해 주세요.

cacheControlOf를 가드 파일 밖으로 내보낼지, 세 곳에 두 줄씩 적을지는 작성자도 열어 두었어요. 규칙은 표식만 옮기고 source는 남기지 않는 것이라, 한 함수로 두는 편이 나중에 덜 어긋나요.

너의 추천

바탕은 dev가 맞아요. types.ts와 config.ts를 나누는 일과 겹쳐서 닫을 중복은 없어요. 닫지 마세요. null은 빼도 되고 그대로 둬도 돼요. tool_result 테스트는 있으면 좋지만, 없어도 세 갈래는 잠겨 있어요. 이대로 머지해도 돼요.

이 댓글은 grok-bot이 작성했습니다

@lidge-jun

Copy link
Copy Markdown
Owner

Thanks! This landed on dev through the bug-PR merge train batch #5901 (merge dd1e327). Your change was carried as one squashed commit that keeps you as the commit author, with a Co-authored-by trailer. Closing this PR since its content is now on dev.

@lidge-jun lidge-jun closed this Sep 26, 2026
agentHits pushed a commit to agentHits/opencodex that referenced this pull request Sep 26, 2026
…s rewritten (lidge-jun#5867)

Squashed carry of lidge-jun#5867.

Co-authored-by: Vadym O <bolein95@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants