Skip to content

retain tool cache_control for bedrock converse - #5811

Merged
akshaydeo merged 1 commit into
mainfrom
08-03-retain_tool_cache_control_for_bedrock_converse
Aug 4, 2026
Merged

akshaydeo merged 1 commit into
mainfrom
08-03-retain_tool_cache_control_for_bedrock_converse

Conversation

@akshaydeo

@akshaydeo akshaydeo commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Prompt caching markers (cache_control) on Anthropic-native invoke requests were silently dropped when routing through Bedrock's Converse API. This meant cache breakpoints set on system blocks and tools never reached the model, and cache token counts (cache_creation_input_tokens, cache_read_input_tokens) were never surfaced in either streaming or non-streaming /invoke responses.

Closes #5629

Changes

  • System block cache_control translation: parseSystemMessages now detects a per-block cache_control field on Anthropic-dialect system entries and appends a positional cachePoint entry immediately after the marked block, matching the behavior convertSystemMessages already applies on the Bifrost→Bedrock egress path.
  • Tool cache_control translation: convertAnthropicTools now detects cache_control on each Anthropic-native tool map and appends a positional cachePoint entry after the marked tool. Nova-family exclusion is handled downstream by the shared ToBifrostResponsesRequest builder, so no additional gating is needed here.
  • Cache token fields in non-streaming responses: toBedrockInvokeAnthropicResponse now routes usage through buildBedrockTokenUsage and populates cache_creation_input_tokens and cache_read_input_tokens on BedrockInvokeMessagesUsage, which gains those two omitempty fields.
  • Cache token fields in streaming responses: toAnthropicInvokeStreamBytes now includes input_tokens and, when non-zero, cache_read_input_tokens and cache_creation_input_tokens in the message_delta usage object. Because Bedrock Converse only reports usage on the terminal stream event, all token counts are consolidated there rather than split across message_start/message_delta as native Anthropic does.
  • cacheControlTTL helper: Extracts the optional "ttl" string from an Anthropic-dialect cache_control map so it can be forwarded to newBedrockCachePoint, which already enforces Bedrock's supported TTL allow-list.
  • New test file invoke_test.go: Covers no-cache-control regression, cache_control on tools and system blocks, TTL forwarding and fallback, Nova-family exclusion, the full invoke→Converse→Bifrost pipeline, streaming usage fields, and non-streaming usage fields.

Type of change

  • Bug fix

Affected areas

  • Core (Go)
  • Providers/Integrations

How to test

go test ./core/providers/bedrock/... -v -run "TestConvertAnthropicTools|TestParseSystemMessages|TestToBedrockConverseRequest_InvokeCache|TestToAnthropicInvokeStreamBytes|TestToBedrockInvokeAnthropicResponse"

Expected: all new tests pass. Existing Bedrock tests remain unaffected.

To validate end-to-end, send an Anthropic-native /invoke request with cache_control on a system block or tool to a Claude model via Bedrock and confirm:

  • The model receives the cache breakpoint (observable via cache_creation_input_tokens > 0 on the first turn).
  • Subsequent identical requests show cache_read_input_tokens > 0 and reduced input_tokens.
  • Streaming responses include all three token fields in the message_delta event.

Breaking changes

  • No

Related issues

Closes #5629

Security considerations

None. Changes are limited to token-count field mapping and cache marker translation within the Bedrock provider.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a5fab34b-3d86-421f-a11f-92fb7191ac8d

📥 Commits

Reviewing files that changed from the base of the PR and between ffe8e62 and d76cb55.

📒 Files selected for processing (3)
  • core/providers/bedrock/invoke.go
  • core/providers/bedrock/invoke_test.go
  • core/providers/bedrock/types.go
🚧 Files skipped from review as they are similar to previous changes (3)
  • core/providers/bedrock/types.go
  • core/providers/bedrock/invoke.go
  • core/providers/bedrock/invoke_test.go

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added support for prompt caching metadata in Bedrock system messages and tools, including supported cache expiration settings.
    • Added cache creation and cache read token usage to invoke responses for both streaming and non-streaming requests.
    • Preserved model-specific behavior by excluding unsupported tool caching.
  • Bug Fixes

    • Improved cache metadata and usage propagation across Bedrock invocations.

Walkthrough

Changes

Bedrock invoke cache and usage

Layer / File(s) Summary
Cache control conversion
core/providers/bedrock/invoke.go, core/providers/bedrock/invoke_test.go
System messages and tools convert Anthropic cache_control metadata into Bedrock CachePoint entries. Supported TTL values are preserved, and unsupported values use the Bedrock default.
Usage token mapping
core/providers/bedrock/types.go, core/providers/bedrock/invoke.go, core/providers/bedrock/invoke_test.go
Non-streaming and terminal streaming responses expose input, output, cache creation, and cache read token counts through shared usage mapping.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • maximhq/bifrost#5814: This PR overlaps in Bedrock InvokeModel handling and regression coverage, but focuses on content-block normalization.

Suggested reviewers: pratham-mishra04, tejasghatte

Sequence Diagram(s)

sequenceDiagram
  participant AnthropicInvoke
  participant BedrockConverse
  participant BifrostResponse
  AnthropicInvoke->>BedrockConverse: send system and tool CachePoint entries
  BedrockConverse->>BifrostResponse: return input, output, cache creation, and cache read usage
  BifrostResponse->>AnthropicInvoke: expose mapped usage fields
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: preserving tool cache_control for Bedrock Converse.
Description check ✅ Passed The description covers the purpose, implementation, testing, affected areas, issue link, and security impact; unchecked checklist items are non-critical.
Linked Issues check ✅ Passed The changes satisfy issue #5629 by preserving system and tool cache markers, TTL behavior, streaming and unary usage, and regression coverage.
Out of Scope Changes check ✅ Passed All changes are directly related to Bedrock cache marker translation, token usage mapping, and regression tests required by issue #5629.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 08-03-retain_tool_cache_control_for_bedrock_converse

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


Comment @coderabbitai help to get the list of available commands.

@akshaydeo
akshaydeo marked this pull request as ready for review August 3, 2026 19:03
@coderabbitai
coderabbitai Bot requested a review from Pratham-Mishra04 August 3, 2026 19:04
Comment thread core/providers/bedrock/invoke.go
@akshaydeo
akshaydeo force-pushed the 08-03-retain_tool_cache_control_for_bedrock_converse branch from ffe8e62 to d76cb55 Compare August 4, 2026 04:15
Comment thread core/providers/bedrock/invoke.go

akshaydeo commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Merge activity

  • Aug 4, 9:07 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Aug 4, 9:08 AM UTC: Graphite couldn't merge this PR because it failed for an unknown reason.
  • Aug 4, 9:11 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Aug 4, 9:11 AM UTC: Graphite couldn't merge this PR because it failed for an unknown reason.
  • Aug 4, 9:13 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Aug 4, 9:13 AM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo merged commit 4ac18bc into main Aug 4, 2026
19 checks passed
@akshaydeo
akshaydeo deleted the 08-03-retain_tool_cache_control_for_bedrock_converse branch August 4, 2026 09:13
atharvamhaske pushed a commit to atharvamhaske/bifrost that referenced this pull request Aug 13, 2026
## Summary

Prompt caching markers (`cache_control`) on Anthropic-native invoke requests were silently dropped when routing through Bedrock's Converse API. This meant cache breakpoints set on system blocks and tools never reached the model, and cache token counts (`cache_creation_input_tokens`, `cache_read_input_tokens`) were never surfaced in either streaming or non-streaming `/invoke` responses.

Closes maximhq#5629

## Changes

- **System block cache_control translation**: `parseSystemMessages` now detects a per-block `cache_control` field on Anthropic-dialect system entries and appends a positional `cachePoint` entry immediately after the marked block, matching the behavior `convertSystemMessages` already applies on the Bifrost→Bedrock egress path.
- **Tool cache_control translation**: `convertAnthropicTools` now detects `cache_control` on each Anthropic-native tool map and appends a positional `cachePoint` entry after the marked tool. Nova-family exclusion is handled downstream by the shared `ToBifrostResponsesRequest` builder, so no additional gating is needed here.
- **Cache token fields in non-streaming responses**: `toBedrockInvokeAnthropicResponse` now routes usage through `buildBedrockTokenUsage` and populates `cache_creation_input_tokens` and `cache_read_input_tokens` on `BedrockInvokeMessagesUsage`, which gains those two `omitempty` fields.
- **Cache token fields in streaming responses**: `toAnthropicInvokeStreamBytes` now includes `input_tokens` and, when non-zero, `cache_read_input_tokens` and `cache_creation_input_tokens` in the `message_delta` usage object. Because Bedrock Converse only reports usage on the terminal stream event, all token counts are consolidated there rather than split across `message_start`/`message_delta` as native Anthropic does.
- **`cacheControlTTL`** **helper**: Extracts the optional `"ttl"` string from an Anthropic-dialect `cache_control` map so it can be forwarded to `newBedrockCachePoint`, which already enforces Bedrock's supported TTL allow-list.
- **New test file** **`invoke_test.go`**: Covers no-cache-control regression, cache_control on tools and system blocks, TTL forwarding and fallback, Nova-family exclusion, the full invoke→Converse→Bifrost pipeline, streaming usage fields, and non-streaming usage fields.

## Type of change

- [x] Bug fix

## Affected areas

- [x] Core (Go)
- [x] Providers/Integrations

## How to test

```sh
go test ./core/providers/bedrock/... -v -run "TestConvertAnthropicTools|TestParseSystemMessages|TestToBedrockConverseRequest_InvokeCache|TestToAnthropicInvokeStreamBytes|TestToBedrockInvokeAnthropicResponse"
```

Expected: all new tests pass. Existing Bedrock tests remain unaffected.

To validate end-to-end, send an Anthropic-native `/invoke` request with `cache_control` on a system block or tool to a Claude model via Bedrock and confirm:

- The model receives the cache breakpoint (observable via `cache_creation_input_tokens` > 0 on the first turn).
- Subsequent identical requests show `cache_read_input_tokens` > 0 and reduced `input_tokens`.
- Streaming responses include all three token fields in the `message_delta` event.

## Breaking changes

- [x] No

## Related issues

Closes maximhq#5629

## Security considerations

None. Changes are limited to token-count field mapping and cache marker translation within the Bedrock provider.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
akhsaul pushed a commit to akhsaul/bifrost that referenced this pull request Aug 27, 2026
## Summary

Prompt caching markers (`cache_control`) on Anthropic-native invoke requests were silently dropped when routing through Bedrock's Converse API. This meant cache breakpoints set on system blocks and tools never reached the model, and cache token counts (`cache_creation_input_tokens`, `cache_read_input_tokens`) were never surfaced in either streaming or non-streaming `/invoke` responses.

Closes maximhq#5629

## Changes

- **System block cache_control translation**: `parseSystemMessages` now detects a per-block `cache_control` field on Anthropic-dialect system entries and appends a positional `cachePoint` entry immediately after the marked block, matching the behavior `convertSystemMessages` already applies on the Bifrost→Bedrock egress path.
- **Tool cache_control translation**: `convertAnthropicTools` now detects `cache_control` on each Anthropic-native tool map and appends a positional `cachePoint` entry after the marked tool. Nova-family exclusion is handled downstream by the shared `ToBifrostResponsesRequest` builder, so no additional gating is needed here.
- **Cache token fields in non-streaming responses**: `toBedrockInvokeAnthropicResponse` now routes usage through `buildBedrockTokenUsage` and populates `cache_creation_input_tokens` and `cache_read_input_tokens` on `BedrockInvokeMessagesUsage`, which gains those two `omitempty` fields.
- **Cache token fields in streaming responses**: `toAnthropicInvokeStreamBytes` now includes `input_tokens` and, when non-zero, `cache_read_input_tokens` and `cache_creation_input_tokens` in the `message_delta` usage object. Because Bedrock Converse only reports usage on the terminal stream event, all token counts are consolidated there rather than split across `message_start`/`message_delta` as native Anthropic does.
- **`cacheControlTTL`** **helper**: Extracts the optional `"ttl"` string from an Anthropic-dialect `cache_control` map so it can be forwarded to `newBedrockCachePoint`, which already enforces Bedrock's supported TTL allow-list.
- **New test file** **`invoke_test.go`**: Covers no-cache-control regression, cache_control on tools and system blocks, TTL forwarding and fallback, Nova-family exclusion, the full invoke→Converse→Bifrost pipeline, streaming usage fields, and non-streaming usage fields.

## Type of change

- [x] Bug fix

## Affected areas

- [x] Core (Go)
- [x] Providers/Integrations

## How to test

```sh
go test ./core/providers/bedrock/... -v -run "TestConvertAnthropicTools|TestParseSystemMessages|TestToBedrockConverseRequest_InvokeCache|TestToAnthropicInvokeStreamBytes|TestToBedrockInvokeAnthropicResponse"
```

Expected: all new tests pass. Existing Bedrock tests remain unaffected.

To validate end-to-end, send an Anthropic-native `/invoke` request with `cache_control` on a system block or tool to a Claude model via Bedrock and confirm:

- The model receives the cache breakpoint (observable via `cache_creation_input_tokens` > 0 on the first turn).
- Subsequent identical requests show `cache_read_input_tokens` > 0 and reduced `input_tokens`.
- Streaming responses include all three token fields in the `message_delta` event.

## Breaking changes

- [x] No

## Related issues

Closes maximhq#5629

## Security considerations

None. Changes are limited to token-count field mapping and cache marker translation within the Bedrock provider.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
occcat pushed a commit to occcat/bifrost that referenced this pull request Sep 2, 2026
## Summary

Prompt caching markers (`cache_control`) on Anthropic-native invoke requests were silently dropped when routing through Bedrock's Converse API. This meant cache breakpoints set on system blocks and tools never reached the model, and cache token counts (`cache_creation_input_tokens`, `cache_read_input_tokens`) were never surfaced in either streaming or non-streaming `/invoke` responses.

Closes maximhq#5629

## Changes

- **System block cache_control translation**: `parseSystemMessages` now detects a per-block `cache_control` field on Anthropic-dialect system entries and appends a positional `cachePoint` entry immediately after the marked block, matching the behavior `convertSystemMessages` already applies on the Bifrost→Bedrock egress path.
- **Tool cache_control translation**: `convertAnthropicTools` now detects `cache_control` on each Anthropic-native tool map and appends a positional `cachePoint` entry after the marked tool. Nova-family exclusion is handled downstream by the shared `ToBifrostResponsesRequest` builder, so no additional gating is needed here.
- **Cache token fields in non-streaming responses**: `toBedrockInvokeAnthropicResponse` now routes usage through `buildBedrockTokenUsage` and populates `cache_creation_input_tokens` and `cache_read_input_tokens` on `BedrockInvokeMessagesUsage`, which gains those two `omitempty` fields.
- **Cache token fields in streaming responses**: `toAnthropicInvokeStreamBytes` now includes `input_tokens` and, when non-zero, `cache_read_input_tokens` and `cache_creation_input_tokens` in the `message_delta` usage object. Because Bedrock Converse only reports usage on the terminal stream event, all token counts are consolidated there rather than split across `message_start`/`message_delta` as native Anthropic does.
- **`cacheControlTTL`** **helper**: Extracts the optional `"ttl"` string from an Anthropic-dialect `cache_control` map so it can be forwarded to `newBedrockCachePoint`, which already enforces Bedrock's supported TTL allow-list.
- **New test file** **`invoke_test.go`**: Covers no-cache-control regression, cache_control on tools and system blocks, TTL forwarding and fallback, Nova-family exclusion, the full invoke→Converse→Bifrost pipeline, streaming usage fields, and non-streaming usage fields.

## Type of change

- [x] Bug fix

## Affected areas

- [x] Core (Go)
- [x] Providers/Integrations

## How to test

```sh
go test ./core/providers/bedrock/... -v -run "TestConvertAnthropicTools|TestParseSystemMessages|TestToBedrockConverseRequest_InvokeCache|TestToAnthropicInvokeStreamBytes|TestToBedrockInvokeAnthropicResponse"
```

Expected: all new tests pass. Existing Bedrock tests remain unaffected.

To validate end-to-end, send an Anthropic-native `/invoke` request with `cache_control` on a system block or tool to a Claude model via Bedrock and confirm:

- The model receives the cache breakpoint (observable via `cache_creation_input_tokens` > 0 on the first turn).
- Subsequent identical requests show `cache_read_input_tokens` > 0 and reduced `input_tokens`.
- Streaming responses include all three token fields in the `message_delta` event.

## Breaking changes

- [x] No

## Related issues

Closes maximhq#5629

## Security considerations

None. Changes are limited to token-count field mapping and cache marker translation within the Bedrock provider.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants