Skip to content

fix(bedrock): capture and replay native Claude reasoning blocks - #4665

Closed
aheritier wants to merge 9 commits into
feat/claude5x-model-tablefrom
feat/claude5x-bedrock-reasoning
Closed

aheritier wants to merge 9 commits into
feat/claude5x-model-tablefrom
feat/claude5x-bedrock-reasoning

Conversation

@aheritier

Copy link
Copy Markdown
Collaborator

Part (a) of #4653: Bedrock native reasoning blocks. Part of #4583 and epic #4580. Parts (b) and (c) follow as separate stacked PRs (see the plan in #4653).

Stacked on #4658 (base branch feat/claude5x-safety-defaults). This PR's diff shows only its own commits.

What

Bedrock Converse now captures, persists and replays Claude's reasoning blocks exactly, instead of flattening them into one string and one signature.

  • Capture: the stream adapter records every content block by ContentBlockIndex (reasoning text and signature, redacted bytes, text, tool use). When a message completes, it attaches the ordered blocks as ProviderState{provider: "amazon-bedrock"}. Incomplete or failed streams record nothing.
  • Replay: convertAssistantContent rebuilds the recorded blocks in order when the message still matches them: same text, reasoning and tool calls, compared with the runtime's own accumulation rules. Otherwise it falls back to the flattened fields.
  • Persistence: the runtime already seals and stores ProviderState for any provider. Nothing changes there.

Bugs fixed (each with a test that failed first)

# Before After
R1 Redacted bytes were stored in ThinkingSignature, a string; JSON replaced non-UTF-8 bytes on save Kept as base64 in the native blocks; a SQLite round trip is byte-identical
R2 Several thinking blocks collapsed into one string with the last signature, which can't verify Each block replays with its own signature
R3 Signed thinking with empty text (omitted display, the Claude 5 default on Bedrock) was replayed as redacted content Replayed as reasoning text with its signature, also in the flattened fallback
R4 Replay order was always reasoning, then text, then tool calls Original order, including thinking between tool calls

Two regressions found in review and fixed before opening this PR:

  • Anthropic after a fallback or model switch from Bedrock. The Anthropic converters dropped flattened thinking whenever any ProviderState was present; now only Anthropic's own state suppresses it (e623364).
  • Unsigned reasoning (thinking cut off by max_tokens, or another vendor's reasoning on Bedrock) is never replayed, matching the Anthropic replay path (88f0a77).

Compatibility

  • Sessions saved by older versions have no native blocks and keep the flattened replay.
  • Older binaries reading new sessions ignore the native blocks and use the flattened fields.
  • Legacy ambiguity (deliberate): the old adapter stored both signature-only thinking and redacted bytes in ThinkingSignature. The fallback now treats them as signed thinking, the common case on Claude 5. A legacy session that held genuinely redacted thinking would get a 400 on replay; those bytes were usually already corrupted by JSON.

References

Vendor documentation behind each change (all links checked on 2026-10-20):

Change Documentation
Converse reasoning blocks: reasoningText (text + signature) vs redactedContent (bytes) ReasoningContentBlock, ReasoningTextBlock
Blocks are identified by contentBlockIndex in stream events ContentBlockDeltaEvent
Signed thinking must be passed back unchanged; adaptive thinking on Bedrock AWS: adaptive thinking, Preserved thinking
Redacted thinking is opaque and must be replayed as is; signatures are verified Extended thinking

Validation

  • task build passes.
  • task test passes. The 10 Gemini TestLoadExamples failures seen in earlier PRs didn't appear in this run.
  • golangci-lint v2.14.0 reports 0 issues, and go run ./lint . reports no offenses.
  • go test -race -shuffle=on passes on pkg/model/provider/bedrock, pkg/session and pkg/runtime.
  • go test ./e2e -run '^TestWasmProviderDependencies$|^TestDependencies$' passes.
  • An independent review found 0 critical and 2 important issues (both fixed above). Four minor findings are deferred; see below.

Deferred minor findings:

  • Empty text blocks are not skipped on native replay.
  • Tool inputs lose integers above 2^53 on replay, as the existing fallback already does.
  • Every Bedrock message now stores a ProviderState, including replies without reasoning.
  • A redacted delta arriving at an index first typed as reasoning is dropped; the API doesn't send that.

Not verified against live AWS: replay of native blocks on Converse.

#4584

Signed-off-by: Arnaud Héritier <aheritier@apache.org>
Signed-off-by: Arnaud Héritier <aheritier@apache.org>
- Drop the Fable example from the server-side fallback comment; that
  configuration is now rejected.
- Build the Bedrock Mantle-only error from the configured model ID.
- Move unit-test fixtures off Claude 5.x fallbacks that the constructor
  now rejects.

Signed-off-by: Arnaud Héritier <aheritier@apache.org>
Reasoning was flattened into one string and the last signature, and
redacted bytes were stored in the signature string, where JSON
persistence corrupted them. Record every content block by index and
emit the ordered blocks as ProviderState on a complete message.

Signed-off-by: Arnaud Héritier <aheritier@apache.org>
Replay the recorded Converse blocks when the message still matches them,
so interleaved, multi-signature and redacted reasoning round-trips
exactly. The flattened fallback now sends signed thinking with empty
text as reasoning text instead of redacted content.

Signed-off-by: Arnaud Héritier <aheritier@apache.org>
Signed-off-by: Arnaud Héritier <aheritier@apache.org>
Signed-off-by: Arnaud Héritier <aheritier@apache.org>
Bedrock messages now carry a ProviderState, and the Anthropic converters
only replayed flattened thinking when the state was nil. After a fallback
or a model switch from Bedrock, the thinking block was dropped and the
tool loop was rejected. Only Anthropic's own state suppresses it.

Signed-off-by: Arnaud Héritier <aheritier@apache.org>
Thinking cut off by max_tokens, or another vendor's reasoning on Bedrock,
has no signature the API can verify. Skip it on replay, as the Anthropic
path does, while keeping it in the message match.

Signed-off-by: Arnaud Héritier <aheritier@apache.org>
@aheritier
aheritier requested a review from a team as a code owner October 20, 2026 08:47
Copilot AI balanced review requested due to automatic review settings October 20, 2026 08:47
Base automatically changed from feat/claude5x-safety-defaults to feat/claude5x-model-table October 20, 2026 08:47
@aheritier
aheritier deleted the branch feat/claude5x-model-table October 20, 2026 08:47
@aheritier aheritier closed this Oct 20, 2026
@aheritier

Copy link
Copy Markdown
Collaborator Author

Superseded by #4666: this PR was closed automatically when its stack base (#4657, #4658) merged and the base branches were deleted. Same commits, rebased onto main.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Per-delta string concatenation introduces quadratic allocation overhead for long Bedrock text and reasoning streams.

1 open issue
What changed in this PR

Adds native Bedrock reasoning capture and replay through the existing provider-state mechanism, supporting the signed-history work in #4653.

Changes:

  • Preserves ordered reasoning, redacted bytes, text, and tool calls across persistence.
  • Updates flattened replay and Anthropic handling of foreign provider state.
  • Includes Claude 5.x safety checks, regression tests, and documentation.
File Description
pkg/​session/​provider_state_persist_test.go Tests opaque-byte persistence.
pkg/​modelinfo/​claude5.go Marks Mythos 5 as Mantle-only.
pkg/​modelinfo/​claude5_test.go Tests updated model metadata.
pkg/​model/​provider/​bedrock/​roundtrip_test.go Tests streamed-state replay after serialization.
pkg/​model/​provider/​bedrock/​native_blocks.go Records and replays ordered native blocks.
pkg/​model/​provider/​bedrock/​native_blocks_test.go Tests replay and fallback behavior.
pkg/​model/​provider/​bedrock/​convert.go Uses native replay and fixes signed-empty fallback.
pkg/​model/​provider/​bedrock/​client.go Rejects Mantle-only Mythos 5.
pkg/​model/​provider/​bedrock/​client_test.go Tests signed-empty replay and model rejection.
pkg/​model/​provider/​bedrock/​adapter.go Captures native state from stream events.
pkg/​model/​provider/​bedrock/​adapter_test.go Tests recording and incomplete streams.
pkg/​model/​provider/​anthropic/​vertex/​vertex_test.go Tests Claude 5.x fallback rejection.
pkg/​model/​provider/​anthropic/​thinking_test.go Adjusts display-validation fixtures.
pkg/​model/​provider/​anthropic/​thinking_binding.go Adds fallback validation.
pkg/​model/​provider/​anthropic/​thinking_binding_test.go Adjusts binding test fixtures.
pkg/​model/​provider/​anthropic/​response_content.go Identifies Anthropic-owned state.
pkg/​model/​provider/​anthropic/​features_integration_test.go Tests safety checks and foreign-state thinking.
pkg/​model/​provider/​anthropic/​fallbacks.go Rejects Claude 5.x fallback chains.
pkg/​model/​provider/​anthropic/​fallbacks_test.go Tests fallback validation.
pkg/​model/​provider/​anthropic/​client.go Preserves flattened thinking for foreign state.
pkg/​model/​provider/​anthropic/​beta_converter.go Applies foreign-state handling to beta requests.
examples/​anthropic-fallbacks.yaml Uses a supported fallback chain.
docs/​providers/​bedrock/​index.md Documents native replay and model restrictions.
docs/​providers/​anthropic/​index.md Documents fallback restrictions.
agent-schema.json Updates fallback option documentation.

Review effort: Balanced

switch d := d.(type) {
case *types.ContentBlockDeltaMemberText:
b := r.get(idx, "text")
b.block.Text += d.Value
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.

2 participants