-
Notifications
You must be signed in to change notification settings - Fork 2.9k
fix(bedrock): enable prompt caching for custom ARN and default to ON #11373
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Set supportsPromptCache to true for custom-arn model info in useSelectedModel.ts - Change awsUsePromptCache default from false to true using nullish coalescing - Add tests for custom-arn prompt caching support Closes #10846
Re-reviewed the backend fix (a051b98). The previously flagged UI/backend mismatch is now resolved. No new issues found.
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
| <> | ||
| <Checkbox | ||
| checked={apiConfiguration?.awsUsePromptCache || false} | ||
| checked={apiConfiguration?.awsUsePromptCache ?? true} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UI/backend mismatch: this line renders the checkbox as checked when awsUsePromptCache is undefined (the case for any user who never explicitly toggled this setting). However, the backend at src/api/providers/bedrock.ts:274 evaluates Boolean(this.options.awsUsePromptCache && ...), which treats undefined as falsy -- so caching is actually OFF. The result is a checkbox that shows "enabled" while the backend is not caching. To make this consistent, the backend line should also default to true, e.g. Boolean((this.options.awsUsePromptCache ?? true) && this.supportsAwsPromptCache(modelConfig)).
Fix it with Roo Code or mention @roomote and request a fix.
|
@roomote please fix all unresolved issues and if successful, remove draft status |
Fixed the UI/backend default mismatch for |
The backend treated undefined awsUsePromptCache as falsy (OFF) while the UI checkbox defaulted to true via nullish coalescing. This caused the UI to show prompt caching as ON but the backend to keep it OFF for new users. Apply the same ?? true default in the backend so both sides agree.
Reviewed a051b98. No issues found. The UI/backend mismatch previously flagged has been resolved. Both the UI and backend now consistently default
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
Related GitHub Issue
Closes: #10846
Description
This PR attempts to address Issue #10846. Feedback and guidance are welcome.
The prompt caching toggle was not shown for Bedrock custom ARN users because
supportsPromptCachewas set tofalsein the custom ARN model info. This PR makes two targeted changes:Enable prompt caching toggle for custom ARN users (
useSelectedModel.ts): ChangedsupportsPromptCachefromfalsetotruefor the custom-arn case. Custom ARN is an advanced option where users know what they are doing, and their ARN often points to newer Claude models that support prompt caching.Default prompt caching to ON (
Bedrock.tsx): Changed the default value ofawsUsePromptCachefrom|| falseto?? true. The nullish coalescing operator (??) properly respects explicitly setfalsevalues while defaulting new users to ON for cost savings and latency improvements.Related issues: #8669, #10576 (previously closed prematurely)
Related PR: #10697 (stale, only linked to #10576)
Test Procedure
useSelectedModel.spec.tsto verify custom-arn model returnssupportsPromptCache: trueandsupportsImages: trueuseSelectedModel.spec.ts: 37 tests pass (35 existing + 2 new)Bedrock.spec.tsx: all tests passtsc --noEmitpasses across all packagesPre-Submission Checklist
Documentation Updates
Important
Enable prompt caching for Bedrock custom ARN users and set default to ON, with tests updated accordingly.
supportsPromptCachetotrueinuseSelectedModel.ts.awsUsePromptCachetoONusing?? trueinBedrock.tsx.useSelectedModel.spec.tsto verifysupportsPromptCacheandsupportsImagesfor custom-arn models.bedrock.spec.tsto check prompt caching default behavior whenawsUsePromptCacheis undefined or explicitly false.This description was created by
for a051b98. You can customize this summary. It will automatically update as commits are pushed.