feat(py/plugins/amazon-bedrock): add Converse non-streaming generate path - #5876
feat(py/plugins/amazon-bedrock): add Converse non-streaming generate path#5876hilariie wants to merge 7 commits into
Conversation
Package skeleton for the Bedrock port from the Go plugin (genkit-ai/aws-bedrock-go-plugin): plugin class with constructor and per-call config types, workspace wiring, and scaffold tests. Action resolution, Converse generate/stream, embedders, image generation and reranker land in follow-up slices.
boto3 + thin async bridge instead of aioboto3: aiobotocore pins botocore to a narrow patch range, which would dictate boto3 versions for every user of the plugin. A thread bridge keeps the event loop unblocked at LLM concurrency levels, matching Strands, pydantic-ai and langchain-aws. Transport goes behind an internal seam so the smithy-based official async SDK can slot in once it matures.
Port of the Go plugin's models.go: 47-entry capability registry (verified key-for-key and flag-for-flag against the source), cross-region inference-profile prefix stripping for capability lookup only, and get_model_info() with modern Converse defaults at the unstable stage for unknown chat/text models so newer or profile-only models stay callable.
…k ARNs Reduce foundation-model and inference-profile ARNs to their resource ID before capability lookup, across all AWS partitions (aws, aws-us-gov, aws-cn). Lookup-only, requests still carry the original identifier. Addresses PR review feedback.
…path Transport seam bridging sync boto3 onto worker threads, pure Converse converters ported from the Go plugin's generate.go, lazy model resolution, and AWS error mapping. Region resolution now fails loudly instead of defaulting to us-east-1, matching Go. boto3 floor moves to 1.37.24 for cache points and reasoningContent.
…tionals Both land from earlier slices: the scaffold pins 0.1.0 where check_consistency.py wants the core version, and model_info_test.py trips ty and pyright on the optional Supports field.
lock refreshed with uv 0.12.0; drops exclude-newer options released uv cannot reproduce
There was a problem hiding this comment.
Code Review
This pull request introduces the genkit-amazon-bedrock plugin, enabling support for Amazon Bedrock models via the Converse API. The implementation includes the plugin structure, configuration, type converters, model capability registry, and comprehensive tests. Feedback highlights a potential namespace hijacking issue in the resolve method and a runtime safety concern in _reasoning_block_to_part when handling unexpected API response types.
| if action_type != ActionKind.MODEL: | ||
| return None | ||
| model_id = name.removeprefix(f'{BEDROCK_PLUGIN_NAME}/') |
There was a problem hiding this comment.
The resolve method does not verify if the requested action name starts with the bedrock/ namespace prefix before attempting to resolve it. If another plugin's model is requested (e.g., openai/gpt-4), this method will strip the prefix (which does nothing since it doesn't start with bedrock/), identify it as a 'chat' model, and return a BedrockModel action for it. This will cause the Bedrock plugin to greedily hijack and attempt to resolve actions belonging to other namespaces.\n\nTo fix this, explicitly check if the action name starts with the bedrock/ namespace prefix.
if action_type != ActionKind.MODEL:\n return None\n if not name.startswith(f'{BEDROCK_PLUGIN_NAME}/'):\n return None\n model_id = name.removeprefix(f'{BEDROCK_PLUGIN_NAME}/')| if isinstance(reasoning_text, str): | ||
| text, signature = reasoning_text, None | ||
| else: | ||
| text = reasoning_text.get('text') or '' | ||
| signature = reasoning_text.get('signature') |
There was a problem hiding this comment.
In _reasoning_block_to_part, if reasoning_text is not a string, the code assumes it is a dictionary and calls .get('text') and .get('signature') on it. If the API returns an unexpected type (such as a list or an integer) for reasoningText, this will raise an AttributeError. It is safer to explicitly check if reasoning_text is a dictionary before calling .get() to prevent potential runtime crashes.
if isinstance(reasoning_text, str):\n text, signature = reasoning_text, None\n elif isinstance(reasoning_text, dict):\n text = reasoning_text.get('text') or ''\n signature = reasoning_text.get('signature')\n else:\n return None
Slice 3 of #5820. Non-streaming Converse generate path for the Amazon Bedrock Python plugin, ported from
genkit-ai/aws-bedrock-go-plugin(generate.go,types.go). Stacked on #5699.What lands
transport.py: every boto3 call goes through here. One shared client built lazily under a lock, each call pushed onto a worker thread withasyncio.to_threadso the loop stays free for the seconds-to-minutes a generate takes. Keeping boto3 behind one module means we can swap in AWS's async SDK later without touching converters or models.converters.py: pure request/response conversion. Roles, system extraction, media, tools and tool choice, cache points, stop reasons, token usage, tool-input coercion, reasoning parts.models.py:BedrockModel.generateplus AWS error toGenkitErrorstatus mapping.plugin.py:resolve()andlist_actions()wired to real model actions.samples/amazon-bedrock-sample: Dev UI sample with text, structured output, tool calling, and reasoning flows, over Nova Lite, Llama 3.3, and DeepSeek R1.Region resolution now fails loudly instead of defaulting to
us-east-1, same as the Go plugin. A silent default sends traffic, and data, to a region nobody picked.Reasoning
Still working on Genkit
ReasoningParts.Divergences from Go
read_timeout/connect_timeout/max_pool_connections(3600/60/50) rather than Go's whole-call deadline, which kills long generations mid-flight.resolve()is lazy and serves any model ID, inference profile or ARN.init()returns[], the Bedrock catalogue can't be enumerated.GenkitErrorstatuses. Go has no mapping.request.tool_choiceis a fallback; the Bedrock config wins because it can name a specific tool the core enum can't express.temperature=0.0is sent, not dropped as unset.AWS_REGIONis read directly. botocore only started honoring it in 1.41, above our floor, so leaving it to the SDK chain would silently ignore the var our own error message tells you to set.maxTokensfor Claude models. We don't: the requirement and the per-family values couldn't be verified here, and guessing one silently caps output. Callers setmaxTokensthemselves.>=1.37.24, first release with cache points andreasoningContent.Structured output needs
output_instructions=Truepassed alongsideoutput_formatandoutput_schema.supports.constrainedisNONEbecause Converse has no constrained-decoding mode, and core's json format injects no instructions on its own, so without the flag the schema never reaches the model. The sample and its README both show the working call.Lockfile
uv.lockrefreshed with uv 0.12.0, what CI's unpinnedsetup-uvinstalls today. That drops theexclude-newer/exclude-newer-spanoptions block this branch inherited (no released uv acceptsexclude-newer-span, main's lock doesn't carry the block, anduv lock --checkfails while it's there) and records the plugin's real version. Rest of the churn is 0.12.0 marker normalization, no dependency versions changed.Tests
159 unit tests, no AWS, plus 3 opt-in live tests (
BEDROCK_LIVE_TESTS=1).Ran Nova Lite for text and the undocumented-tool round trip, DeepSeek R1 for reasoning, Llama 3.3 for structured output.
tests/request_validation_test.pyruns botocore's ownParamValidatorover built requests against the bedrock-runtime service model. Several Converse fields areNonEmptyString(min: 1) and botocore enforces that client-side, so an empty tool description, system text ortoolUseIdfails before it reaches AWS, asParamValidationError(aBotoCoreError, not aClientError). Go never hits this, its SDK does no client-side length check. The corpus covers every request-shaping branch the builder has: undocumented tool, blank system prompt, empty assistant text, tool round trip, media and cache points, dropped reasoning, and inference config on two model families.Dev UI
Declared models resolve, the plugin's
customOptionsschema (Max tokens, Tool choice) renders in the model runner, and the sample registers four flows and one tool.Notes for the stack
0.1.0wherecheck_consistency.pywants the core version, andmodel_info_test.pyreaching through the optionalSupportsfield (17ty+ 17 pyright errors). Happy to move that commit down the stack instead.exclude-newerblock and stale0.1.0version, so itsuv lock --checkfails under current uv. A plainuv lockthere fixes it the same way.