-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat(bedrock): enable prompt caching toggle for custom ARN and default to ON #10697
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
Conversation
Review completed. No issues found. The changes correctly enable the prompt caching toggle for custom ARN users and default the setting to ON while respecting explicit user choices. Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
|
This is working in many normal circumstances but DOES NOT WORK for custom ARN - this PR should fix that 🥳 Noting here because the last couple attempts at this were closed prematurely FYI: @mrubens @cte @jr @hannesrudolph |
|
Any updates on this one? |
58540ea to
62214b3
Compare
|
@roomote is it possible to update this PR to remove the draft status? It seems ready to go as soon as it's approved. And not having this is costing a ton of aws resources. Not sure why it hasn't gotten a look quite yet from code owners but it may be because it's marked as a draft |
Removed draft status from the PR. It is now marked as "ready for review" and should be visible to code owners for approval. |
Review completed. No issues found. The changes correctly enable the prompt caching toggle for custom ARN users and default the setting to ON while respecting explicit user choices. Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
|
Closing in favor of #11373 as requested by @wisestmumbler. |
Related GitHub Issue
Closes: #10576
Description
This PR attempts to address Issue #10576. Feedback and guidance are welcome.
Changes:
Enable prompt caching toggle for custom ARN users: Changed
supportsPromptCachefromfalsetotruefor custom ARN model info. Custom ARN is treated as an advanced option where users know what they are doing, and their custom ARN often points to newer Claude models that support prompt caching.Default prompt caching to ON: Changed the default value of
awsUsePromptCachefromfalsetotrueby using?? trueinstead of|| false. This ensures users get cost savings and latency improvements by default, while the nullish coalescing operator (??) properly respects explicitly setfalsevalues.Files modified:
webview-ui/src/components/ui/hooks/useSelectedModel.ts- Enable supportsPromptCache for custom-arnwebview-ui/src/components/settings/providers/Bedrock.tsx- Default checkbox to trueTest Procedure
cd src && npx vitest run api/providers/__tests__/bedrock*- all 143 tests passnpx tsc --noEmit- no type errorsPre-Submission Checklist
Documentation Updates
Additional Notes
This change treats custom ARN as an advanced option where users know what they are doing. Since custom ARNs often point to newer Claude models that support prompt caching, it makes sense to show the toggle and let users decide. This is a re-implementation of the closed PR #10577 per request in the issue comments.
Important
Enable prompt caching toggle for custom ARN users and default prompt caching to ON in
useSelectedModel.tsandBedrock.tsx.supportsPromptCachefor custom ARN inuseSelectedModel.ts, allowing users to toggle prompt caching.awsUsePromptCachetotrueinBedrock.tsxusing?? trueto respect explicitly setfalsevalues.useSelectedModel.ts: ChangesupportsPromptCachetotruefor custom ARN.Bedrock.tsx: Change default checkbox value for prompt caching totrue.This description was created by
for 62214b3. You can customize this summary. It will automatically update as commits are pushed.