feat(bedrock): pass strict and additionalProperties to Converse API toolSpec - #25209
feat(bedrock): pass strict and additionalProperties to Converse API toolSpec#25209Ryze0323 wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@greptileai - Updated: applied P2 style feedback (dict-spread instead of post-construction mutation) |
Greptile SummaryThis PR closes the gap between the Anthropic direct API path (which already supports Key changes:
Confidence Score: 5/5Safe to merge — only one P2 style nit remains; no correctness or reliability issues. The change is narrow and correct: two optional fields are forwarded through an existing conversion function with proper falsy-safe guards. The test validates both the pass-through and absence paths. The only open finding is a P2 type-annotation nit ( No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/types/llms/bedrock.py | Adds additionalProperties: bool to ToolJsonSchemaBlock and strict: Optional[bool] to ToolSpecBlock; minor nit: Optional[bool] in a total=False TypedDict allows None values that the production code deliberately avoids. |
| litellm/litellm_core_utils/prompt_templates/factory.py | Correctly extracts additionalProperties from parameters and strict from tool["function"], passing each through only when non-None via dict-spread; logic is clean and handles False (falsy) values properly. |
| tests/llm_translation/test_bedrock_completion.py | New unit test covers both the "with strict + additionalProperties" and "without either field" paths; no network calls; could also test each field independently but the primary scenarios are exercised. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["OpenAI tool format\n{ function: { name, strict, parameters: { ..., additionalProperties } } }"] --> B["_bedrock_tools_pt()"]
B --> C{additionalProperties\nin parameters?}
C -- yes --> D["Include additionalProperties\nin BedrockToolJsonSchemaBlock"]
C -- no --> E["Omit additionalProperties"]
D --> F["BedrockToolInputSchemaBlock"]
E --> F
B --> G{strict in\nfunction?}
G -- yes --> H["Include strict\nin BedrockToolSpecBlock"]
G -- no --> I["Omit strict"]
F --> J["BedrockToolSpecBlock\n{ inputSchema, name, description, [strict] }"]
H --> J
I --> J
J --> K["BedrockToolBlock\n{ toolSpec: ... }"]
K --> L["Bedrock Converse API"]
Reviews (7): Last reviewed commit: "feat(bedrock): pass strict and additiona..." | Re-trigger Greptile
7f05979 to
71ef734
Compare
|
@ishaan-jaff @krrishdholakia This PR is ready for review — all CI checks pass and Greptile gave 5/5 confidence. Could you take a look when you get a chance? Thanks! |
71ef734 to
06b7252
Compare
|
@greptileai review |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
06b7252 to
ccae41e
Compare
ccae41e to
0b69b15
Compare
0b69b15 to
6e93431
Compare
6e93431 to
38c188c
Compare
…oolSpec Bedrock Converse API supports strict: true in toolSpec since 2026-02-04 (GA). Previously, both strict and additionalProperties were implicitly dropped during OpenAI-to-Bedrock tool conversion because ToolSpecBlock and ToolJsonSchemaBlock only whitelisted a subset of fields. This change: - Adds strict to ToolSpecBlock and passes it through when present - Adds additionalProperties to ToolJsonSchemaBlock and passes it through when present (required by Bedrock when strict: true is set) Without additionalProperties: false, Bedrock rejects strict: true requests with: "For 'object' type, 'additionalProperties' must be explicitly set to false" Fixes the gap where Anthropic direct API path already supports strict (PR BerriAI#16725) but Bedrock Converse path does not.
38c188c to
c51fb46
Compare
|
@yuneng-berri Could you take a look when you get a chance? All CI checks pass and Greptile gave 5/5 confidence. Thanks! |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
Relevant issues
Closes the gap where Anthropic direct API path already supports
strict(PR #16725) but Bedrock Converse path does not.Related: #10062, #16725, #6136
Summary
Bedrock Converse API supports
strict: trueintoolSpecsince 2026-02-04 (GA) — AWS API Reference: ToolSpecification.Previously, both
strictandadditionalPropertieswere implicitly dropped during OpenAI-to-Bedrock tool conversion becauseToolSpecBlockandToolJsonSchemaBlockonly whitelisted a subset of fields (introduced in #10062, which was correct at the time since Bedrock didn't supportstrict).Without
additionalProperties: falsein the schema, Bedrock rejectsstrict: truerequests with:This PR:
strict: Optional[bool]toToolSpecBlockTypedDictadditionalProperties: booltoToolJsonSchemaBlockTypedDict_bedrock_tools_pt()when presentPre-Submission checklist
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewType
🆕 New Feature
Changes
litellm/types/llms/bedrock.py: Addedstrict: Optional[bool]toToolSpecBlock,additionalProperties: booltoToolJsonSchemaBlocklitellm/litellm_core_utils/prompt_templates/factory.py: Pass throughstrictandadditionalPropertiesfrom OpenAI tool format to BedrocktoolSpectests/llm_translation/test_bedrock_completion.py: Updatedtest_bedrock_tools_pt_strict_parametertest to cover both fields