fix(bedrock): preserve encrypted reasoning replay signatures

[zachgersh]

Summary

Preserve encrypted reasoning replay data when a Responses API reasoning item is translated to Bedrock Converse format.

Responses reasoning items can carry EncryptedContent alongside an empty, non-nil Summary. The Bedrock converter previously treated that empty slice as visible reasoning, emitted no block, and silently dropped the replay payload. Its fallback also represented encrypted reasoning as synthetic text rather than the native Bedrock reasoning signature.

Changes

• Treat only non-empty reasoning summaries as visible reasoning content.
• Map non-empty EncryptedContent to reasoningContent.reasoningText.signature.
• Remove the synthetic [ENCRYPTED_REASONING: ...] text representation.
• Add a focused regression test, committed separately before the implementation, covering the empty-summary encrypted-reasoning shape.
• Add a core changelog entry.

Type of change

• Bug fix
• Feature
• Refactor
• Documentation
• Chore/CI

Affected areas

• Core (Go)
• Transports (HTTP)
• Providers/Integrations
• Plugins
• UI (React)
• Docs

How to test

cd core
GOCACHE=/tmp/bifrost-go-build-cache GOPATH=/tmp/bifrost-go \
  go test ./providers/bedrock \
  -run '^TestConvertBifrostReasoningToBedrockReasoningEncryptedContent$' \
  -count=1

Expected result: the regression test passes and the Bedrock reasoning block contains the encrypted payload in reasoningText.signature, with no synthetic text.

The full Bedrock provider package was also run. It currently fails on pre-existing document-placeholder expectations (TestConvertBifrostResponsesMessageContentBlocksToBedrockContentBlocks_EmptyBlocks and TestDocumentFormatMapping) because current dev emits a placeholder text block alongside document blocks. Those failures are unrelated to this change.

No new configuration or environment variables were added.

Screenshots/Recordings

Not applicable; this change has no UI impact.

Breaking changes

• Yes
• No

Related issues

Related to #5638 and #5094.

Security considerations

No new security surface. The change preserves an existing opaque provider-issued reasoning signature instead of converting it into visible synthetic text.

Checklist

• I read the current contribution and code-convention documentation and followed the guidelines
• I added/updated tests where appropriate
• I updated documentation where needed
• I verified all builds succeed (the focused regression passes; the full Bedrock package has unrelated existing failures noted above)
• I verified the CI pipeline passes locally if applicable

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes

  • Improved preservation of encrypted reasoning during Bedrock response replay.
  • Prevented empty reasoning summaries from producing unintended visible reasoning text.
  • Ensured encrypted reasoning is represented correctly as a replay signature.

• Tests

  • Added regression coverage for encrypted reasoning with empty summaries.

• Documentation

  • Updated the changelog with the reasoning replay fix.

Walkthrough

Bedrock reasoning conversion now preserves non-empty encrypted reasoning content in the Signature field. It omits empty reasoning content and only emits non-empty summaries. A regression test and changelog entry document the behavior.

Changes

Bedrock reasoning replay

Layer / File(s)	Summary
Encrypted reasoning signature conversion core/providers/bedrock/responses.go, core/providers/bedrock/reasoning_replay_test.go, core/changelog.md	The converter maps encrypted reasoning content to the Bedrock Signature field, omits empty summaries and content, and verifies the result with a regression test. The changelog records the behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• maximhq/bifrost#5192: Preserves encrypted reasoning IDs through provider envelopes.
• maximhq/bifrost#5277: Updates reasoning replay conversion and provider-specific reasoning blocks.
• maximhq/bifrost#5668: Updates Bedrock reasoning conversion for encrypted reasoning signatures.

Suggested reviewers: akshaydeo, tejasghatte, pratham-mishra04

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes preserving encrypted Bedrock reasoning replay signatures.
Description check	✅ Passed	The description covers the purpose, implementation, testing, affected areas, risks, and known unrelated test failures.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

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"

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

core/providers/bedrock/responses.go (1)

4553-4564: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Filter summary items by text, not only by slice length.

When Summary contains an item with Text == "" and EncryptedContent is non-empty, Line 4553 enters the summary branch. The else if then skips Lines 4564-4568, so the converter emits an empty reasoning block and drops the replay signature. Emit only non-empty summary text, then use EncryptedContent when no visible summary was emitted. The sibling conversion in core/providers/bedrock/invoke.go already applies this rule. Add a regression case for an empty summary item.

🤖 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 `@core/providers/bedrock/responses.go` around lines 4553 - 4564, The Responses
reasoning conversion must filter Summary entries by non-empty Text rather than
using Summary length. Update the logic around the ResponsesReasoning summary
conversion to append blocks only for visible text, then fall back to
EncryptedContent when no summary block was emitted; mirror the sibling behavior
in invoke.go and add a regression case covering an empty summary item with
non-empty encrypted content.

🧹 Nitpick comments (1)

core/providers/bedrock/reasoning_replay_test.go (1)

30-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert Signature is non-nil before dereferencing it.

require.Equal evaluates the dereference before the assertion runs. If the converter returns a nil signature, the test panics instead of reporting a failed assertion. Add require.NotNil before Line 30.

Suggested test adjustment

+	require.NotNil(t, blocks[0].ReasoningContent.ReasoningText.Signature)
 	require.Equal(t, encryptedContent, *blocks[0].ReasoningContent.ReasoningText.Signature)

🤖 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 `@core/providers/bedrock/reasoning_replay_test.go` at line 30, In the reasoning
replay test, add a require.NotNil assertion for
blocks[0].ReasoningContent.ReasoningText.Signature before the existing
require.Equal assertion, then retain the equality check against
encryptedContent.

🤖 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.

Outside diff comments:
In `@core/providers/bedrock/responses.go`:
- Around line 4553-4564: The Responses reasoning conversion must filter Summary
entries by non-empty Text rather than using Summary length. Update the logic
around the ResponsesReasoning summary conversion to append blocks only for
visible text, then fall back to EncryptedContent when no summary block was
emitted; mirror the sibling behavior in invoke.go and add a regression case
covering an empty summary item with non-empty encrypted content.

---

Nitpick comments:
In `@core/providers/bedrock/reasoning_replay_test.go`:
- Line 30: In the reasoning replay test, add a require.NotNil assertion for
blocks[0].ReasoningContent.ReasoningText.Signature before the existing
require.Equal assertion, then retain the equality check against
encryptedContent.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fb41fa1d-47df-44f4-9150-4799d65109ec

📥 Commits

Reviewing files that changed from the base of the PR and between 2fb58b7 and 4a42498.

📒 Files selected for processing (3)

• core/changelog.md
• core/providers/bedrock/reasoning_replay_test.go
• core/providers/bedrock/responses.go