Repository navigation
test(llms): add bedrock content and usage contract tests - #9096
Lorenze Jay (lorenzejay) merged 3 commits into
Conversation
Strix Security ReviewWarning This pull request has 45 commits after the last Strix review ( ✅ No open security findings. PR summary: Adds a Bedrock-specific pytest contract suite that drives fake Converse responses to assert message content formatting, sync/async parity, and token usage accounting for `BedrockCompletion`. Review summaryThis change adds only a new pytest module under Review 1 on this PR · Reviewed by Strix |
|
Hi Lorenze Jay (@lorenzejay), this is ready for a look when you have time. It’s test-only for crewAIInc/crewAI#8128. Could you approve the CI run and add the llm-generated label? There are two Bedrock-specific assumptions under “Additional context” I’d like you to confirm. Thanks! |
Related issue
Fixes #8128
Summary
Test-only. Adds
lib/crewai/tests/llms/bedrock/test_bedrock_content_usage_contract.py, Bedrock's side of the shared native-provider content and usage contract, driven by fake Converse responses (no AWS credentials, no network)._BedrockContractuses the same adapter shape as test: shared plain-text contract tests for OpenAI and Anthropic providers #8037 (response/stub_sync/stub_async/user_texts/system_text), so it can joinPROVIDERSonce the shared suites (test: shared plain-text contract tests for OpenAI and Anthropic providers #8037 / test(llms): shared LLMMessage.content contract for OpenAI and Anthropic #8039 / test(llms): shared tool-call, streaming, usage and failure contract for OpenAI and Anthropic #8062) land. They aren't onmainyet, so I kept this standalone instead of stacking on three open PRs.Noneuser/assistant content keeps the turn and never becomes"None"; caller messages not mutated; system text goes to the top-levelsystemparameter.callandacallsend the same request and return the same text;acallis forced onto the aiobotocore path (otherwise it silently falls back tocallin a thread) and the test checks the sync client was never used.get_token_usage_summary(), oncall,acall, and both together.xfail(strict=True), not fixed here, so there's no competing runtime change: two for [BUG] Bedrock Converse formatter stringifies multimodal system and tool message content #7920 (list system content lands as the raw list insystem[0].text; a laterNonesystem message becomes"None"). Two more for [BUG] Bedrock Converse forwards OpenAI-style content parts in user and assistant messages unconverted #9095, which I filed while writing these: OpenAI-style{"type": "text"}andimage_urlparts on user/assistant messages are forwarded to Converse unconverted.Verification
Tests added or updated for the changed behavior
Relevant tests and quality checks pass locally
uv run pytest lib/crewai/tests/llms/bedrock -qon Python 3.13 and 3.10: 62 passed, 8 skipped, 4 xfaileduv run pytest lib/crewai/tests/llms -qon 3.13: 1113 passed, 20 skipped, 4 xfaileduv run ruff check lib/anduv run ruff format --check lib/: pass. Tests are excluded from ruff/mypy config, so I also ran ruff andmypy --stricton the new file directly: clean.Mutation check: stringifying list content, turning
Noneinto"None", double-counting async usage, and lettingacallfall back to sync each fail at least one test.The [BUG] Bedrock Converse formatter stringifies multimodal system and tool message content #7920 xfails flip to
XPASS(strict)with a fix(bedrock): collapse multimodal parts content in Converse system and tool messages #7930-style fix applied locally.Not tested: live Bedrock calls.
Additional context
Assumptions I made where Bedrock differs from the shared suites. Can you confirm these, or tell me if either should be treated as a defect instead?
LLMCallCompletedEvent.usageis the raw Converse dict (inputTokens/outputTokens/totalTokens), while test(llms): shared tool-call, streaming, usage and failure contract for OpenAI and Anthropic #8062 assertsusage["total_tokens"]for OpenAI and Anthropic. I treated this as Bedrock-specific and recorded it in the module docstring without asserting either shape.""(the shared rule in test: shared plain-text contract tests for OpenAI and Anthropic providers #8037). Same treatment: recorded, not asserted.This PR was written with AI assistance (Claude Code); I reviewed and can explain every line. Per CONTRIBUTING.md it needs the
llm-generatedlabel, which I can't apply as a contributor. Could a maintainer add it? Thanks!Note
Low Risk
Test-only change with no production code paths modified; risk is limited to CI behavior (including expected xfails).
Overview
Adds
test_bedrock_content_usage_contract.py, a standalone Bedrock adapter for the shared native-provider content/usage contract (issue #8128), using fake Converse recorders so tests never hit AWS.The suite asserts message shaping (verbatim strings, Converse text/image blocks,
Nonenot serialized as"None", system text on the top-levelsystemparam, no in-place mutation of caller messages) and sync/async parity (callvsacallon the aiobotocore path, identical requests, token usage accumulated inget_token_usage_summary()).Four strict
xfailtests document known gaps (#7920 system/parts handling, #9095 OpenAI-style parts not converted) without changing Bedrock runtime code.Reviewed by Cursor Bugbot for commit 2c7f46a. Bugbot is set up for automated code reviews on this repo. Configure here.