fix(providers): normalize tool schemas for JSON Schema 2020-12 compliance (Bedrock/LiteLLM)#879
fix(providers): normalize tool schemas for JSON Schema 2020-12 compliance (Bedrock/LiteLLM)#879umi008 wants to merge 5 commits into
Conversation
…nAI Compatible path Call normalizeToolSchema() before applying OpenAI-specific strict mode transforms in convertToolSchemaForOpenAI(). This ensures tool schemas are 2020-12 compliant (type arrays -> anyOf, unsupported formats stripped) before backends like Bedrock validate them through OpenAI-compatible proxies like LiteLLM. Previously, only the direct provider paths (bedrock, vscode-lm) applied this normalization. The 15+ providers that inherit from BaseProvider and use convertToolsForOpenAI() were sending raw schemas that Bedrock rejects with 'maxItems is not supported' errors. Fixes Zoo-Code-Org#869
📝 WalkthroughWalkthrough
ChangesOpenAI tool schema normalization
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/providers/base-provider.ts`:
- Around line 70-84: Update the schema handling around normalizeToolSchema so
object-like normalized schemas with properties and additionalProperties: false
are processed even when type is absent, rather than returning early; use an
object-shape check that also covers flattened compositions and nullable object
conversions. In the same logic, remove the redundant Array.isArray(prop.type)
branch and rely on normalizeToolSchema’s recursive type-array normalization
before applying strict required-field and nested-property processing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8f75b99a-dfae-41c8-8a20-c5926d1ab896
📒 Files selected for processing (1)
src/api/providers/base-provider.ts
… call
The spread of normalied (Record<string, unknown>) caused TS7053
because the inferred type was {} and string indexing was not allowed.
Added explicit Record<string, any> type annotation to fix compilation.
1a45e3a to
feb79dd
Compare
…ation Two test updates after adding normalizeToolSchema() to the schema pipeline: 1. Nullable types test: Now asserts anyOf conversion instead of null stripping. normalizeToolSchema() converts type arrays to anyOf for JSON Schema 2020-12 compliance, which is correct behavior. 2. Non-object schemas test: Verifies type preservation through normalization. additionalProperties is NOT added to non-object schemas (strings, numbers, etc.) because the JSON Schema spec says it's only valid on object types.
9c7204b to
826eb83
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
@roomote Do a review please |
|
I saw the mention, but I could not start work on this PR with the current Roomote GitHub setup. |
edelauna
left a comment
There was a problem hiding this comment.
Thanks for your contribution, had some comments on the implementation and how it related to your inspiration for opening the PR.
…bjects, improve tests - Remove minItems/maxItems from ARRAY_SPECIFIC_PROPERTIES so they dont get forwarded to array-branch schemas (Bedrock rejects them) - Also strip minLength, maxLength, pattern from normalize output - Fix nullable object handling: check for properties field as well as type=object so nullable schemas (converted to anyOf) still get strict-mode processing (required/boolean/null removal) - Add test for maxItems stripping on array schemas - Add assertion that stale type array field is removed after normalization - Fix non-object schema test to use type: ["string"] (actually changed by normalization) and assert anyOf output
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/api/providers/__tests__/base-provider.spec.ts (1)
175-181: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueDrop the redundant nullable-type branch
normalizeToolSchema()already normalizes nested schemas recursively, soconvertToolSchemaForOpenAI()doesn’t need theArray.isArray(prop.type) && prop.type.includes("null")path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/providers/__tests__/base-provider.spec.ts` around lines 175 - 181, Remove the redundant nullable-type handling branch that checks Array.isArray(prop.type) and prop.type.includes("null") from convertToolSchemaForOpenAI(). Rely on normalizeToolSchema() to recursively convert nullable nested schemas, while preserving the existing normalization and OpenAI schema conversion behavior for other property types.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/api/providers/__tests__/base-provider.spec.ts`:
- Around line 175-181: Remove the redundant nullable-type handling branch that
checks Array.isArray(prop.type) and prop.type.includes("null") from
convertToolSchemaForOpenAI(). Rely on normalizeToolSchema() to recursively
convert nullable nested schemas, while preserving the existing normalization and
OpenAI schema conversion behavior for other property types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8f88462f-0d9a-4a62-98b1-35ab2cf903bb
📒 Files selected for processing (3)
src/api/providers/__tests__/base-provider.spec.tssrc/api/providers/base-provider.tssrc/utils/json-schema.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/api/providers/base-provider.ts
Related GitHub Issue
Closes: #869
Description
Tool requests through the OpenAI Compatible provider path (used by 15+ providers including LiteLLM, OpenRouter, DeepSeek, xAI, etc.) were sending raw tool schemas without JSON Schema 2020-12 normalization. When the backend is AWS Bedrock (directly or through a LiteLLM proxy), it rejects schemas containing keywords like
maxItemson array properties because Bedrock enforces strict 2020-12 compliance.normalizeToolSchema()was already applied in the direct Bedrock and VSCode LM provider paths, butBaseProvider.convertToolSchemaForOpenAI()— the shared method used by all OpenAI-compatible providers — was not calling it.Fix:
convertToolSchemaForOpenAI()now callsnormalizeToolSchema()first to convert deprecated type arrays toanyOf, strip unsupported format values, and enforceadditionalProperties: false, before applying OpenAI-specific strict mode transforms (all-required, null removal). The two passes compose safely: normalization handles 2020-12 compliance, and the existing OpenAI pass adds the strict-mode constraints on top.Test Procedure
maxItemsManual verification of schema transformation:
Pre-Submission Checklist
Screenshots / Videos
N/A — backend schema normalization, no UI changes.
Documentation Updates
Additional Notes
This fix benefits all providers that inherit from
BaseProviderand useconvertToolsForOpenAI()— OpenRouter, DeepSeek, xAI, Groq, Fireworks, Together, SambaNova, Baseten, Friendli, and others. The normalization is idempotent for well-formed schemas, so the risk of regression is minimal.Summary by CodeRabbit