fix(anthropic): keep the caller's cache_control when an image block is rewritten - #5867
vadymhimself wants to merge 1 commit into
Conversation
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>
|
✅ Deterministic PR hygiene checks passed. |
|
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 configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe Anthropic image guard and normalization paths now preserve ChangesAnthropic image cache metadata
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to 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 Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request has been marked Ready for Review. |
리뷰 · 우선순위 63 / 80이 글은 Anthropic으로 그림을 보낼 때, 그림을 글로 바꾸거나 다시 압축하면 캐시 표식이 사라지는 일을 막아요. 표식 이름은 이 일은 어댑터가 메시지를 만든 뒤에도 돌아요. 이미 Anthropic 모양으로 들어온 본문에도 돌아요. 고침은 표식만 새 블록에 복사해요. 그림의 어댑터는 그림을 고친 뒤에 라인 - 라인 - 메인테이너의 판단이 필요한 지점 이번 턴에 그림 바이트가 바뀌면, 표식을 남겨도 그 턴은 예전 그림을 캐시에서 읽지 못해요. 내용이 달라서 그 자리는 새 캐시 쓰기예요. 다음 턴에 같은 바이트가 다시 나가면 그때 읽어요. 글로 바뀐 문장은 매번 같아서, 그 경우는 바로 안정돼요. 설명의 "읽히던 것이 다시 쓰인다"는 바이트가 그대로인 경우에 맞아요. 이번 범위로 둘지 정해 주세요.
너의 추천 바탕은 이 댓글은 grok-bot이 작성했습니다 |
…s rewritten (lidge-jun#5867) Squashed carry of lidge-jun#5867. Co-authored-by: Vadym O <bolein95@gmail.com>
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_controlbreakpoint:src/adapters/anthropic-image-normalize.ts(both) andsrc/adapters/anthropic-image-guard.ts(textify).On the native Anthropic lane this pipeline runs over the caller's raw body —
src/server/messages-native.tsprepareNativeBody, andsrc/server/claude-messages.tsbefore 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.tsassertsThat 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
Readof a PNG, a screenshottool_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_controlacross the rebuild, and onlycache_control:spread into the three rebuild sites. Nothing else is carried over — an image block's
sourcemust never survive onto a text block, and both new tests assertsourceisundefinedafter textification.One shared helper rather than three inline copies: the two
textifyimplementations live in different modules,anthropic-image-normalize.tsalready imports fromanthropic-image-guard.ts, and the rule ("onlycache_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.tshas 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:
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 hittextify, each carrying a distinctcache_control.Wider sweep, to show the sibling image lanes are untouched:
tests/adapters/anthropic/,kiro-images,openai-chat-image-normalization,chat-native-image-normalizationtogether give581 pass / 0 fail across 35 files. The Kiro and OpenAI lanes route throughnormalizeImageTargetswith their own replace/drop and are unaffected.No existing test pins the lossy shape:
toEqual({ type: "text"acrosstests/andsrc/returns only unrelated pipelines, and the two Anthropic image test files contain notoEqualon 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. —
typecheckclean; the two image suites 56 pass / 0 fail / 231 expect();claude-native-passthrough20 pass / 0 fail; wider image sweep 581 pass / 0 fail across 35 files;structure:check,privacy:scan,git diff --checkclean. Fullbun run testnot 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