fix(llm): fall back when a provider rejects json_schema response_format#6191
Open
HumphreySun98 wants to merge 3 commits into
Open
fix(llm): fall back when a provider rejects json_schema response_format#6191HumphreySun98 wants to merge 3 commits into
HumphreySun98 wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe change adds provider capability checks for native JSON-schema structured output, disables that path for DeepSeek, and updates sync, async, and streaming completion handling to fall back to client-side validation. Tests cover provider flags and fallback behavior. ChangesStructured output fallback for OpenAI-compatible providers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/crewai/tests/llms/openai_compatible/test_openai_compatible.py (1)
321-386: ⚡ Quick winAdd a DeepSeek streaming fallback regression test (
stream=True+response_model).Current coverage validates the non-streaming fallback well, but it does not exercise the sync streaming fallback path. A targeted test here would lock in the intended contract and catch the current mismatch early.
🤖 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 `@lib/crewai/tests/llms/openai_compatible/test_openai_compatible.py` around lines 321 - 386, Add a new test method to the TestStructuredOutputFallback class that validates DeepSeek's streaming fallback behavior when using response_model. Create a test similar to test_deepseek_completion_skips_native_parse_and_validates_client_side but pass stream=True in the _handle_completion parameters to exercise the sync streaming fallback path. Mock the streaming response appropriately (using an iterator or stream chunks) and verify that the client.chat.completions.create is called with stream=True, that client.beta.chat.completions.parse is not called, and that the final result is still a validated _Answer instance.
🤖 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 `@lib/crewai/src/crewai/llms/providers/openai/completion.py`:
- Line 1914: The sync streaming fallback path for unsupported providers drops
structured parsing validation, causing stream=True with response_model to return
raw str instead of BaseModel. At
lib/crewai/src/crewai/llms/providers/openai/completion.py line 1914, the native
streaming parse is correctly gated by the response_model check, which is the
anchor pattern. The issue is in the sibling fallback path at lines 2039-2054
where _finalize_streaming_response(...) is called without validating
response_model. Add response_model parsing validation in the
unsupported-provider path (2039-2054) to ensure that when response_model is
provided with stream=True on DeepSeek-like providers, the result is properly
parsed into a BaseModel instance, matching the behavior of non-streaming and
async-streaming fallbacks.
---
Nitpick comments:
In `@lib/crewai/tests/llms/openai_compatible/test_openai_compatible.py`:
- Around line 321-386: Add a new test method to the TestStructuredOutputFallback
class that validates DeepSeek's streaming fallback behavior when using
response_model. Create a test similar to
test_deepseek_completion_skips_native_parse_and_validates_client_side but pass
stream=True in the _handle_completion parameters to exercise the sync streaming
fallback path. Mock the streaming response appropriately (using an iterator or
stream chunks) and verify that the client.chat.completions.create is called with
stream=True, that client.beta.chat.completions.parse is not called, and that the
final result is still a validated _Answer instance.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 92196d58-34cc-4d5b-b435-b3b948a52c64
📒 Files selected for processing (3)
lib/crewai/src/crewai/llms/providers/openai/completion.pylib/crewai/src/crewai/llms/providers/openai_compatible/completion.pylib/crewai/tests/llms/openai_compatible/test_openai_compatible.py
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/crewai/src/crewai/llms/providers/openai/completion.py (1)
1572-1582: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGate dict-based
json_schemaformats too.Line 1581 still forwards any dict
response_format, so DeepSeek-like providers will still 400 if callers pass{"type": "json_schema", ...}directly instead of a Pydantic model. Preserve non-native formats likejson_object, but suppressjson_schemawhensupports_native_structured_output()is false.Proposed fix
elif isinstance(self.response_format, dict): - params["response_format"] = self.response_format + if ( + self.response_format.get("type") != "json_schema" + or self.supports_native_structured_output() + ): + params["response_format"] = self.response_format🤖 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 `@lib/crewai/src/crewai/llms/providers/openai/completion.py` around lines 1572 - 1582, The dict-based response_format handling in completion.py still forwards native json_schema payloads even when the provider does not support structured output. Update the response_format branch in the completion path (around self.response_format / supports_native_structured_output) so dict inputs with type json_schema are only passed through when supports_native_structured_output() is true, while still allowing non-native formats like json_object. Keep the existing BaseModel path using generate_model_description and preserve other dict formats unchanged.
🤖 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 `@lib/crewai/src/crewai/llms/providers/openai/completion.py`:
- Around line 1808-1809: The sync streaming fallback in the OpenAI completion
flow is ignoring the configured response format, so
`_finalize_streaming_response` only sees `response_model` and skips
validation/parsing when `self.response_format` is set. Update the streaming path
in the completion logic to pass or resolve `response_model or
self.response_format` consistently, matching the non-streaming behavior and the
related sync/async finalize calls, so `_Answer`-style configured formats are
parsed instead of returning raw JSON text.
---
Outside diff comments:
In `@lib/crewai/src/crewai/llms/providers/openai/completion.py`:
- Around line 1572-1582: The dict-based response_format handling in
completion.py still forwards native json_schema payloads even when the provider
does not support structured output. Update the response_format branch in the
completion path (around self.response_format /
supports_native_structured_output) so dict inputs with type json_schema are only
passed through when supports_native_structured_output() is true, while still
allowing non-native formats like json_object. Keep the existing BaseModel path
using generate_model_description and preserve other dict formats unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3518cfdb-3364-42ac-a581-379a36c739aa
📒 Files selected for processing (2)
lib/crewai/src/crewai/llms/providers/openai/completion.pylib/crewai/tests/llms/openai_compatible/test_openai_compatible.py
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/crewai/tests/llms/openai_compatible/test_openai_compatible.py
…hat reject it DeepSeek and some other OpenAI-compatible endpoints reject OpenAI's json_schema structured-output response_format with "This response_format type is unavailable now". The native OpenAI provider unconditionally sent it via beta.chat.completions.parse/stream and via a Pydantic response_format, breaking any crew using structured output against those endpoints. Add a supports_native_structured_output() capability (True for OpenAI) and a per-provider supports_json_schema flag on the OpenAI-compatible config (DeepSeek = False). When unsupported, skip the native json_schema paths and fall back to a plain completion; the non-streaming path then validates the result against the requested model client-side, and the Task converter reconciles it otherwise. OpenAI behavior is unchanged. Fixes crewAIInc#5990 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When native json_schema streaming is skipped for a provider that rejects it (e.g. DeepSeek), the sync streaming path fell through to a plain stream and returned the raw text, while the async streaming and non-streaming paths returned the parsed model. Validate the accumulated text against response_model in _finalize_streaming_response so all paths behave consistently. Adds a sync-streaming fallback regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The streaming fallback validated only against a per-call response_model, so stream=True with a configured self.response_format on a provider that rejects native json_schema returned raw text instead of the parsed model. Validate against `response_model or self.response_format`, matching the non-streaming fallback. Adds a regression test for the configured-response_format case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
HumphreySun98
force-pushed
the
fix/openai-compatible-structured-output-fallback
branch
from
fbda756 to
cb13afe
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
DeepSeek and some other OpenAI-compatible endpoints reject OpenAI's
json_schemastructured-outputresponse_format:The native OpenAI provider (
OpenAICompatibleCompletionextendsOpenAICompletion) unconditionally sent it viabeta.chat.completions.parse/.streamand via a Pydanticresponse_format, so any crew using structured output against those endpoints failed.Fix
Add a capability gate:
OpenAICompletion.supports_native_structured_output()→True.supports_json_schemaflag on the OpenAI-compatibleProviderConfig(DeepSeek =False); the subclass override reads it.When unsupported, the three
json_schema-sending branches are skipped and the call falls back to a plain completion. The non-streaming path then validates the result against the requested model client-side, and theTaskconverter reconciles it otherwise. OpenAI behavior is unchanged. (The async-streaming path already used a plain completion + client-side validation.)Tests
Added
TestStructuredOutputFallbackintest_openai_compatible.py: capability flags,json_schemasuppression in_prepare_completion_params, and a mock asserting DeepSeek skips the native parse and still returns the validated model. Verified failing-without/passing-with the fix; 137 OpenAI/compatible provider tests still pass;ruffandmypyclean.Fixes #5990
This PR was authored with Claude Code. Per
CONTRIBUTING.md, AI-generated contributions require thellm-generatedlabel — I don't have triage permission to set it, so could a maintainer please add it? 🤖 Generated with Claude CodeSummary by CodeRabbit
Release Notes
New Features
Tests