fix: stabilize cache inputs - #634
Conversation
📝 WalkthroughWalkthroughClaude prompt-cache inputs are now canonicalized by sorting tool definitions and recognized system-reminder listings. Native passthrough applies the same stabilization, routed requests derive ChangesClaude cache stabilization
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ClaudeClient
participant ClaudeRouting
participant PromptCache
participant NativeUpstream
ClaudeClient->>ClaudeRouting: Send system content, tools, and metadata
ClaudeRouting->>ClaudeRouting: Normalize reminders and sort tools
ClaudeRouting->>PromptCache: Build canonical cache inputs
ClaudeRouting->>NativeUpstream: Forward stabilized request with derived session_id
NativeUpstream-->>ClaudeRouting: Return routed response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be453849c9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| if (Array.isArray(body.tools)) sortToolsByName(body.tools as unknown[]); |
There was a problem hiding this comment.
When a native Anthropic request includes a tool-level cache_control breakpoint on the last tool, this unconditional sort can move that annotated tool away from the end, e.g. [Zeta, Alpha(cache_control)] becomes [Alpha(cache_control), Zeta]. Anthropic cache breakpoints are positional, and the repo’s own applyPromptCaching strategy places the breakpoint on the last tool so the whole tool prefix is cached; after this change the prefix can omit later tools and defeat the cache stabilization this patch is trying to provide. Reapply or move the tool-level breakpoint after sorting, or avoid sorting arrays that already carry explicit tool breakpoints.
Useful? React with 👍 / 👎.
|
|
||
| /** Sort `body.system` skills/deferred-tools listings and `body.tools` by name, in place. */ | ||
| export function stabilizeSystemAndToolOrder(body: Rec): void { | ||
| if (Array.isArray(body.system)) { |
There was a problem hiding this comment.
When native passthrough receives the valid Anthropic string form of system, this guard skips it entirely, so a string system reminder such as User-invocable skills remains in the client’s nondeterministic order while the array form is normalized. That leaves native Anthropic /v1/messages and /count_tokens cache inputs unstable for one of the accepted request shapes; handle typeof body.system === "string" with normalizeSystemReminderText before the array branch.
Useful? React with 👍 / 👎.
| if (isSkillsBlockText(text)) return sortSkillsBlockText(text); | ||
| if (isDeferredToolsBlockText(text)) return sortDeferredToolsBlockText(text); |
There was a problem hiding this comment.
When one system text contains both a skills reminder and a later deferred-tools reminder, this early return normalizes only the skills list and leaves the deferred-tools list in its original discovery order. The code already preserves following reminder blocks, so this combined-block shape is accepted, but routed instructions/prompt_cache_key and native passthrough bodies still vary whenever the later deferred-tools listing arrives in a different order; run both normalizers over the same text or scan each reminder block independently.
Useful? React with 👍 / 👎.
| **Native OpenAI/ChatGPT routing:** when system content exists, derives a content-scoped | ||
| `prompt_cache_key` from the resolved model, normalized system content, and full tool schemas. | ||
| Without system content, it falls back to `metadata.user_id`. When metadata is present, it |
There was a problem hiding this comment.
This section now says system content takes precedence for prompt_cache_key, but anthropicToResponsesTranslation still enters the metadata.user_id branch first whenever Claude Code sends metadata, so normal sessions with both metadata and system content remain session-scoped and do not use the normalized system/tool hash described here. Users following these docs will expect cross-session content cache affinity that the implementation does not provide; either change the precedence in code or keep the docs aligned with metadata-first behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs-site/src/content/docs/guides/claude-code.md`:
- Around line 381-385: Correct the prompt-cache-key precedence description in
docs-site/src/content/docs/guides/claude-code.md:381-385,
docs-site/src/content/docs/ja/guides/claude-code.md:296-300,
docs-site/src/content/docs/ko/guides/claude-code.md:333-336,
docs-site/src/content/docs/ru/guides/claude-code.md:313-318, and
docs-site/src/content/docs/zh-cn/guides/claude-code.md:291-294. In every locale,
state that metadata.user_id takes precedence and produces the session-scoped
prompt_cache_key plus separate session_id; only when metadata.user_id is absent
should system content enable the content-scoped key based on the resolved model,
normalized system content, and full tool schemas, preserving each locale’s
language and existing canonicalization details.
In `@src/adapters/anthropic-sort-stabilize.ts`:
- Around line 70-82: Update stabilizeSystemAndToolOrder to normalize
string-valued body.system using normalizeSystemReminderText, while preserving
the existing per-block handling for array-valued system content. Ensure
native-passthrough requests with a plain string system receive the same
stabilization as array requests, and add coverage for the string shape in the
relevant native passthrough tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7c8f5f1e-2385-4a72-b020-3b0314bae6a4
📒 Files selected for processing (11)
docs-site/src/content/docs/guides/claude-code.mddocs-site/src/content/docs/ja/guides/claude-code.mddocs-site/src/content/docs/ko/guides/claude-code.mddocs-site/src/content/docs/ru/guides/claude-code.mddocs-site/src/content/docs/zh-cn/guides/claude-code.mdsrc/adapters/anthropic-sort-stabilize.tssrc/claude/inbound.tssrc/server/claude-messages.tstests/claude-inbound.test.tstests/claude-messages-endpoint.test.tstests/claude-native-passthrough.test.ts
| **Native OpenAI/ChatGPT routing:** when system content exists, derives a content-scoped | ||
| `prompt_cache_key` from the resolved model, normalized system content, and full tool schemas. | ||
| Without system content, it falls back to `metadata.user_id`. When metadata is present, it | ||
| separately derives a per-session `session_id` header for backend affinity. Tool and | ||
| system-reminder listings are canonicalized before either cache input is built. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Native OpenAI/ChatGPT prompt-cache-key precedence is described backward in all 5 locales. src/claude/inbound.ts (Lines 457-489) checks metadata.user_id FIRST — metadata always wins the session-scoped prompt_cache_key when present, and the content-scoped hash (model + normalized system + tool schemas) is only an else if fallback used when metadata is absent. All five doc pages instead present system-content-presence as the primary discriminator ("when system content exists → content-scoped key; without it → falls back to metadata.user_id"), which inverts the real precedence and misleads readers about when each caching mode actually applies (materially, for Claude Code CLI traffic, which per the code's own comments always sends metadata.user_id, so it never reaches the content-scoped path these docs describe as primary).
docs-site/src/content/docs/guides/claude-code.md#L381-L385: rewrite to state metadata-first precedence, e.g. "whenmetadata.user_idis present, derives a session-scopedprompt_cache_keyfrom it plus a separatesession_idheader; otherwise, when system content exists, falls back to a content-scoped key from the resolved model, normalized system content, and full tool schemas."docs-site/src/content/docs/ja/guides/claude-code.md#L296-L300: apply the equivalent corrected precedence in Japanese.docs-site/src/content/docs/ko/guides/claude-code.md#L333-L336: apply the equivalent corrected precedence in Korean.docs-site/src/content/docs/ru/guides/claude-code.md#L313-L318: apply the equivalent corrected precedence in Russian.docs-site/src/content/docs/zh-cn/guides/claude-code.md#L291-L294: apply the equivalent corrected precedence in Simplified Chinese.
📍 Affects 5 files
docs-site/src/content/docs/guides/claude-code.md#L381-L385(this comment)docs-site/src/content/docs/ja/guides/claude-code.md#L296-L300docs-site/src/content/docs/ko/guides/claude-code.md#L333-L336docs-site/src/content/docs/ru/guides/claude-code.md#L313-L318docs-site/src/content/docs/zh-cn/guides/claude-code.md#L291-L294
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs-site/src/content/docs/guides/claude-code.md` around lines 381 - 385,
Correct the prompt-cache-key precedence description in
docs-site/src/content/docs/guides/claude-code.md:381-385,
docs-site/src/content/docs/ja/guides/claude-code.md:296-300,
docs-site/src/content/docs/ko/guides/claude-code.md:333-336,
docs-site/src/content/docs/ru/guides/claude-code.md:313-318, and
docs-site/src/content/docs/zh-cn/guides/claude-code.md:291-294. In every locale,
state that metadata.user_id takes precedence and produces the session-scoped
prompt_cache_key plus separate session_id; only when metadata.user_id is absent
should system content enable the content-scoped key based on the resolved model,
normalized system content, and full tool schemas, preserving each locale’s
language and existing canonicalization details.
| export function stabilizeSystemAndToolOrder(body: Rec): void { | ||
| if (Array.isArray(body.system)) { | ||
| const system = body.system as unknown[]; | ||
| for (let i = 0; i < system.length; i++) { | ||
| const block = system[i]; | ||
| if (!isRec(block) || block.type !== "text" || typeof block.text !== "string") continue; | ||
| const normalized = normalizeSystemReminderText(block.text); | ||
| if (normalized !== block.text) system[i] = { ...block, text: normalized }; | ||
| } | ||
| } | ||
|
|
||
| if (Array.isArray(body.tools)) sortToolsByName(body.tools as unknown[]); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
stabilizeSystemAndToolOrder skips string-typed body.system.
Only Array.isArray(body.system) is handled (Line 71). Anthropic's /v1/messages API also accepts system as a plain string, and src/claude/inbound.ts's systemToInstructions/systemMessageText explicitly handle both shapes. Here, a string system containing the skills/deferred-tools listing silently skips normalization for native-passthrough requests, leaving that shape's cache prefix unstabilized — the exact bug this PR sets out to fix.
🐛 Proposed fix
export function stabilizeSystemAndToolOrder(body: Rec): void {
- if (Array.isArray(body.system)) {
+ if (typeof body.system === "string") {
+ body.system = normalizeSystemReminderText(body.system);
+ } else if (Array.isArray(body.system)) {
const system = body.system as unknown[];
for (let i = 0; i < system.length; i++) {
const block = system[i];
if (!isRec(block) || block.type !== "text" || typeof block.text !== "string") continue;
const normalized = normalizeSystemReminderText(block.text);
if (normalized !== block.text) system[i] = { ...block, text: normalized };
}
}
if (Array.isArray(body.tools)) sortToolsByName(body.tools as unknown[]);
}tests/claude-native-passthrough.test.ts only exercises the array-system shape (Lines 194-197), so this gap isn't currently covered by tests either.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function stabilizeSystemAndToolOrder(body: Rec): void { | |
| if (Array.isArray(body.system)) { | |
| const system = body.system as unknown[]; | |
| for (let i = 0; i < system.length; i++) { | |
| const block = system[i]; | |
| if (!isRec(block) || block.type !== "text" || typeof block.text !== "string") continue; | |
| const normalized = normalizeSystemReminderText(block.text); | |
| if (normalized !== block.text) system[i] = { ...block, text: normalized }; | |
| } | |
| } | |
| if (Array.isArray(body.tools)) sortToolsByName(body.tools as unknown[]); | |
| } | |
| export function stabilizeSystemAndToolOrder(body: Rec): void { | |
| if (typeof body.system === "string") { | |
| body.system = normalizeSystemReminderText(body.system); | |
| } else if (Array.isArray(body.system)) { | |
| const system = body.system as unknown[]; | |
| for (let i = 0; i < system.length; i++) { | |
| const block = system[i]; | |
| if (!isRec(block) || block.type !== "text" || typeof block.text !== "string") continue; | |
| const normalized = normalizeSystemReminderText(block.text); | |
| if (normalized !== block.text) system[i] = { ...block, text: normalized }; | |
| } | |
| } | |
| if (Array.isArray(body.tools)) sortToolsByName(body.tools as unknown[]); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/adapters/anthropic-sort-stabilize.ts` around lines 70 - 82, Update
stabilizeSystemAndToolOrder to normalize string-valued body.system using
normalizeSystemReminderText, while preserving the existing per-block handling
for array-valued system content. Ensure native-passthrough requests with a plain
string system receive the same stabilization as array requests, and add coverage
for the string shape in the relevant native passthrough tests.
Fixes an issue with cache inputs where I had claude code forward subagent work to GPT 5.6 and ended up with a surprise 7k bill in the morning due to repeated cache writes.
I see both fixes were already in the dev branch (I initially started off
main), so closing this PR as this additional change isn't worth it.Summary by CodeRabbit
New Features
Documentation
Bug Fixes