Repository navigation
feat: add Bedrock :invoke_anthropic protocol spike (InvokeModel) - #30
staging-supernova-dx-appf-io[bot] wants to merge 1 commit into
Conversation
…ssages bodies)
Opt-in third Bedrock protocol that POSTs Anthropic Messages bodies to
/model/{id}/invoke and /invoke-with-response-stream (SigV4-signed),
reusing the Anthropic protocol's payload assembly and parsing. Converse
stays the default; protocol_for never selects it.
- Extract Converse::EventStream (eventstream POST/decoding/exception
mapping) and share it between ConverseStream and InvokeModel streaming
- finalize_invoke_payload is the single envelope hook (anthropic_version,
de-duplicated anthropic_beta) for later beta/context_management work
- Thinking/redacted_thinking blocks captured verbatim and replayed on
either endpoint (Converse <-> Messages shape translation both ways)
- Bedrock#complete lifts additionalModelRequestFields to top level for
invoke requests
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Sam Boland <sam.boland@appfolio.com>
|
🤖 Supernova Code Review — View trace — via automation "Code Review Opus 5.5 Medium" (run), installed by Sam Boland (sam.boland@appfolio.com) |
There was a problem hiding this comment.
🤖 Supernova adversarial review: 4 inline findings.
The biggest risks when turning on :invoke_anthropic:
- Routing: setting
config.bedrock_protocolsends non-Claude Bedrock models to InvokeAnthropic. - Output cap: the 4096
max_tokensfallback truncates output and rejects thinking budgets of 4096 or more. - Param translation: top-level Converse params such as
inferenceConfigare passed through untranslated. - Render mismatch:
renderandcompleteproduce different bodies.
The EventStream extraction and the thinking-block translation between Converse and InvokeAnthropic looked correct to me.
— via automation "Code Review Opus 5.5 Medium" (run), installed by Sam Boland (sam.boland@appfolio.com)
| def max_tokens_for(model) | ||
| model.max_tokens || DEFAULT_MAX_TOKENS | ||
| end |
There was a problem hiding this comment.
When the model has no max_tokens, this caps output at 4096. Converse doesn't do that, so the two protocols behave differently here.
Converse's format_inference_config never sets maxTokens, so Bedrock falls back to the model's own default. InvokeModel requires max_tokens, and this code fills it with model.max_tokens || 4096. Bedrock registry entries often have no max_tokens, so Claude requests would hit the 4096 fallback. That causes two problems:
- Long answers get cut off with
stop_reason: max_tokenson requests that work fine over Converse. - Any budget thinking request with
budget_tokens >= 4096gets a 400, because Anthropic requiresmax_tokens > thinking.budget_tokens.
Fix: use a Claude-appropriate default (for example the model family's max output). At minimum, when a thinking budget is set, bump max_tokens above it. Also honor a caller-supplied max_tokens.
| def normalize_invoke_params(params) | ||
| normalized = RubyLLM::Utils.deep_symbolize_keys(params || {}) | ||
| additional_fields = normalized.delete(:additionalModelRequestFields) || {} | ||
| normalized = RubyLLM::Utils.deep_merge(normalized, additional_fields) | ||
| normalized.delete(:model) | ||
| normalized.delete(:stream) | ||
| normalized | ||
| end |
There was a problem hiding this comment.
This only unwraps additionalModelRequestFields. Other top-level Converse params go into the InvokeModel body unchanged.
The docstring says callers build params in the Converse shape. But keys like inferenceConfig (maxTokens, topP, stopSequences), guardrailConfig, performanceConfig, requestMetadata and additionalModelResponseFieldPaths get deep-merged into the Anthropic Messages body as they are. Bedrock then rejects them with a 400 (extraneous key ... is not permitted). So a caller that works on Converse today breaks as soon as it opts into :invoke_anthropic.
Fix: translate inferenceConfig to max_tokens / top_p / stop_sequences. For the Converse-only keys, either strip them or raise a clear ArgumentError, the way strip_converse_only_params does for mantle.
| def invoke_anthropic_request?(protocol) | ||
| (protocol || configured_protocol)&.to_sym == :invoke_anthropic | ||
| end |
There was a problem hiding this comment.
Setting config.bedrock_protocol = :invoke_anthropic sends every Bedrock model to InvokeAnthropic, not just Claude.
Provider#resolve_protocol lets an explicit or configured protocol win over protocol_for. With this config set, Llama, Nova, Mistral and openai.gpt-oss-* all get an Anthropic Messages body posted to /model/{id}/invoke. Mantle-only openai.gpt-5* ids get their params run through strip_converse_only_params, then still go to the InvokeAnthropic protocol on bedrock-runtime. The PR advertises this config key as one of the two opt-in paths, so any non-Anthropic Bedrock call would fail with 400s.
Fix: only honor :invoke_anthropic for Anthropic model ids (match anthropic. including region prefixes like us./global., plus inference-profile ARNs), and fall back to protocol_for otherwise. Or raise an ArgumentError naming the model.
| def complete(messages, model:, params: {}, **rest, &) | ||
| params = mantle_only_model?(model) ? strip_converse_only_params(params) : normalize_params(params, model:) | ||
| params = if mantle_only_model?(model) | ||
| strip_converse_only_params(params) | ||
| elsif invoke_anthropic_request?(rest[:protocol]) | ||
| normalize_invoke_params(params) | ||
| else | ||
| normalize_params(params, model:) | ||
| end |
There was a problem hiding this comment.
The InvokeModel param normalization only runs in complete, not in render, so render and complete produce different bodies.
Bedrock doesn't override Provider#render. A dry-run render with protocol: :invoke_anthropic keeps additionalModelRequestFields nested and never lifts thinking / anthropic_beta / context_management to the top level. The rendered body then doesn't match what's actually sent, which undermines using render to check parity. Fix: apply the same params = branching in a render override, or pull it into a shared helper both methods call.
What this does
Adds a new, opt-in
:invoke_anthropicprotocol to the Bedrock provider that talks to Claude via Bedrock's InvokeModel / InvokeModelWithResponseStream APIs using Anthropic's native Messages request format, instead of the Converse API. Supernova currently talks to Claude on Bedrock via Converse, but newer server-side context-management features (compactioncompact-2026-09-04, tool-result clearingclear_tool_uses_20250919/context-management-2025-06-27, and tool search withdefer_loading) are only reachable through InvokeModel/InvokeModelWithResponseStream, not Converse. This PR is a spike that builds and tests the alternate wire protocol at parity with Converse, as a foundation for a later PR to actually turn on compaction/context-editing/tool-search. It does not enable any of those features yet.Key pieces:
lib/ruby_llm/providers/bedrock.rb: registers:invoke_anthropicalongside:converseand:mantle_responses.protocol_fornever auto-selects it — it is opt-in only viaprotocol: :invoke_anthropic(orconfig.bedrock_protocol), so Converse remains the default with no behavior change unless explicitly requested. Addsnormalize_invoke_params, which lifts everything underadditionalModelRequestFieldsto the top level and stripsmodel/stream(the single place intended for futureanthropic_beta/context_managementadditions).lib/ruby_llm/protocols/invoke_anthropic.rb(new): protocol class subclassingAnthropic, mixing inChatandStreaming, POSTing to/model/{id}/invoke, SigV4-signed via the existing Bedrock signer.lib/ruby_llm/protocols/invoke_anthropic/chat.rb(new): request rendering/response parsing matching Converse's replay guarantees (thinking blocks always replayed verbatim, consecutive same-role turns folded, tool results batched). Builds the InvokeModel envelope (anthropic_version: "bedrock-2023-05-31", merged/de-dupedanthropic_beta, nomodel/stream). Images/PDFs are always inline base64 (supports_provider_file_references?returns false).lib/ruby_llm/protocols/invoke_anthropic/streaming.rb(new): decodes the AWS eventstream (via the extractedConverse::EventStreamhelper) whosechunkevents wrap base64-encoded Anthropic SSE-shaped events, including thinking/redacted_thinking finalize/salvage logic mirroring Converse streaming.lib/ruby_llm/protocols/converse/event_stream.rb(new): extracted eventstream decoding/base64 unwrapping/error handling shared by both Converse and InvokeAnthropic streaming;converse/streaming.rbnow includes it (net −100 lines), validated by re-running the full existing 50-spec Converse suite before building on top of it.lib/ruby_llm/protocols/converse/chat.rb: addsconverse_thinking_blockso thinking/redacted_thinking blocks captured under InvokeModel can be replayed correctly if a later message on the same chat falls back to Converse (cross-protocol replay for Supernova's endpoint-pin/kill-switch mechanism).spec/ruby_llm/protocols/invoke_anthropic_spec.rb(new, 358 lines) covering URL construction, request-body shape, tool formatting/folding, inline images, verbatim/cross-protocol thinking replay, SigV4-signed completion parsing, throttling→RateLimitError mapping, full synthetic streaming decode, thinking-block salvage rules, and streaming error mapping.spec/ruby_llm/providers/bedrock_spec.rb(+35 lines) covers#completeparam normalization for the new protocol.Reviewer notes: default behavior is unchanged by construction (no config flag to flip in this fork — Supernova gates it via its own feature flag in a separate PR). No beta/context-management values are set anywhere in this PR — that's left for a follow-up. Cross-protocol thinking replay (both directions) is implemented and unit-tested, but nothing has run against live Bedrock yet to confirm this end-to-end; that verification is planned as a staging probe on the Supernova side. The Supernova-side feature flag, admin page, and endpoint-pinning/kill-switch logic live entirely in the separate
appfolio/supernovaPR #7086 and are not part of this diff.Type of change
Scope check
Required for new features
N/A — no approved GitHub issue for this feature was identified; this should be confirmed with the author before merge.
Quality check
overcommit --installand all hooks passbundle exec rake vcr:record[provider_name]bundle exec rspecmodels.json,aliases.json)Note: this fork's tests use WebMock/RSpec stubs and synthetic AWS-eventstream payloads rather than VCR cassettes, so no cassette re-recording applies. 106 targeted specs pass and rubocop reports no offenses.
AI-generated code
API changes
New public classes/modules:
Protocols::InvokeAnthropic(plus itsChatandStreaming),Protocols::Converse::EventStream, and newBedrock#complete/protocol_forbranching logic for the:invoke_anthropicprotocol.In Supernova
Chat: open