Add Anthropic prompt caching breakpoints#1017
Merged
Merged
Conversation
Split static and dynamic system prompt content so Anthropic-compatible providers can cache only the stable prefix, matching the Python agent-sdk behavior. Mark the last user/tool turn for cache extension and cover the native Anthropic and LiteLLM Claude request shapes with focused tests.
Contributor
📝 WalkthroughWalkthroughSplits system prompts into cacheable and dynamic segments, detects Anthropic models that support prompt caching, emits ephemeral ChangesPrompt Caching Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Collaborator
Author
|
/gemini review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/agent-sdk/src/sdk/runtime/Agent.ts (3)
1021-1049: 💤 Low valueOptional: Consider clarifying JSDoc terminology.
The JSDoc mentions "registered tool summaries" but the code actually uses tool definitions (name + description from
getToolDefinitions()). Consider rewording to "tool definitions" for clarity, though this is a minor nitpick.🤖 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 `@packages/agent-sdk/src/sdk/runtime/Agent.ts` around lines 1021 - 1049, Update the JSDoc on buildCacheableSystemPrompt to replace the phrase "registered tool summaries" with clearer wording such as "tool definitions (name + description)"; locate the comment above the private buildCacheableSystemPrompt() method and change the description to reference getToolDefinitions() semantics so the doc matches the implementation that extracts tool.function.name and tool.function.description.
1051-1066: ⚡ Quick winConsider adding JSDoc for consistency.
Since
buildCacheableSystemPrompt()has comprehensive JSDoc explaining the caching strategy, adding similar documentation tobuildDynamicSystemPrompt()would improve consistency and help developers understand the split. For example, explain that this returns runtime-mutated context (editor state, secrets, etc.) that should not be cached.📝 Example JSDoc
+ /** + * Builds the dynamic system-prompt suffix that changes at runtime. + * + * This suffix contains runtime-mutated context such as the current editor state, + * available secrets, and LLM provider details. It should NOT be cached by + * Anthropic prompt caching, as it varies across runs. + * + * `@returns` The dynamic suffix, or null if no agentContext is configured. + */ private buildDynamicSystemPrompt(): string | null {🤖 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 `@packages/agent-sdk/src/sdk/runtime/Agent.ts` around lines 1051 - 1066, Add a JSDoc block above the buildDynamicSystemPrompt method (matching the style used for buildCacheableSystemPrompt) that explains this function returns the runtime-mutated system prompt pieces (editor state, secrets, runtime context) and therefore must not be cached; mention the inputs used (agentContext, secrets.getRegisteredNames(), resolved llmModel/llmProvider/llmBaseUrl via resolveSystemPromptLlmContext) and the intended usage so readers understand why caching is split between buildCacheableSystemPrompt and buildDynamicSystemPrompt.
1068-1073: ⚡ Quick winConsider adding JSDoc for clarity.
This method recombines the cacheable and dynamic prompt parts for use in system prompt events. Adding JSDoc would clarify its role in the caching strategy and improve code documentation consistency.
📝 Example JSDoc
+ /** + * Recombines cacheable and dynamic system prompt parts into a single string. + * + * Used by ensureSystemPrompt() to emit the full system prompt event. + * The split parts are sent separately to LLM clients that support prompt caching. + */ private buildSystemPrompt(): string {🤖 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 `@packages/agent-sdk/src/sdk/runtime/Agent.ts` around lines 1068 - 1073, The buildSystemPrompt method lacks documentation explaining its purpose and relation to caching; add a concise JSDoc comment above the buildSystemPrompt method describing that it recombines cacheable and dynamic prompt parts (via buildCacheableSystemPrompt and buildDynamicSystemPrompt), explains the returned string format (joined with double newlines), and notes its role in system prompt events and caching strategy so callers understand when caching applies versus when dynamic content is included.
🤖 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.
Nitpick comments:
In `@packages/agent-sdk/src/sdk/runtime/Agent.ts`:
- Around line 1021-1049: Update the JSDoc on buildCacheableSystemPrompt to
replace the phrase "registered tool summaries" with clearer wording such as
"tool definitions (name + description)"; locate the comment above the private
buildCacheableSystemPrompt() method and change the description to reference
getToolDefinitions() semantics so the doc matches the implementation that
extracts tool.function.name and tool.function.description.
- Around line 1051-1066: Add a JSDoc block above the buildDynamicSystemPrompt
method (matching the style used for buildCacheableSystemPrompt) that explains
this function returns the runtime-mutated system prompt pieces (editor state,
secrets, runtime context) and therefore must not be cached; mention the inputs
used (agentContext, secrets.getRegisteredNames(), resolved
llmModel/llmProvider/llmBaseUrl via resolveSystemPromptLlmContext) and the
intended usage so readers understand why caching is split between
buildCacheableSystemPrompt and buildDynamicSystemPrompt.
- Around line 1068-1073: The buildSystemPrompt method lacks documentation
explaining its purpose and relation to caching; add a concise JSDoc comment
above the buildSystemPrompt method describing that it recombines cacheable and
dynamic prompt parts (via buildCacheableSystemPrompt and
buildDynamicSystemPrompt), explains the returned string format (joined with
double newlines), and notes its role in system prompt events and caching
strategy so callers understand when caching applies versus when dynamic content
is included.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c11df5ac-36e6-4ca5-8762-6ee769f1abd7
📒 Files selected for processing (7)
packages/agent-sdk/src/sdk/llm/__tests__/promptCaching.test.tspackages/agent-sdk/src/sdk/llm/__tests__/thinkingBlocks.test.tspackages/agent-sdk/src/sdk/llm/anthropic.tspackages/agent-sdk/src/sdk/llm/openai-compatible.tspackages/agent-sdk/src/sdk/llm/providerQuirks.tspackages/agent-sdk/src/sdk/runtime/Agent.tspackages/agent-sdk/src/sdk/runtime/__tests__/Agent.system-prompt.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/agent-sdk/src/sdk/runtime/tests/Agent.system-prompt.test.ts
- packages/agent-sdk/src/sdk/llm/providerQuirks.ts
- packages/agent-sdk/src/sdk/llm/openai-compatible.ts
- packages/agent-sdk/src/sdk/llm/anthropic.ts
- packages/agent-sdk/src/sdk/llm/tests/promptCaching.test.ts
🔧 VSCode Extension Built Successfully• File: openhands-tab-0.9.3.vsix (548 KB) To install:
Built with Node 22. Commit 63b0c15. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
Review
PinkStone; no additional blocking findings came back before merge.cache_controlon thetool_resultcontent block, while the LiteLLM/OpenAI-compatible tool-message envelope behavior was intentionally kept for Python parity and the LiteLLM tool-result quirk.opus-46profile and showed cache markers on outgoing requests plus cache-read tokens on all five turns of a single conversation, including turns withterminalandfile_editortool use.Summary by CodeRabbit
New Features
Tests