fix(llm): handle reasoning_content from OpenRouter reasoning models#5748
fix(llm): handle reasoning_content from OpenRouter reasoning models#5748NIK-TIGER-BILL wants to merge 2 commits intocrewAIInc:mainfrom
Conversation
OpenRouter reasoning models (e.g., Anthropic Sonnet 4.5, Gemini 3.1 Pro) return a reasoning_content field when the model produces chain-of-thought output. CrewAI previously only read message.content, which could be None for these responses, resulting in an empty string being returned. This change adds a fallback to getattr(message, 'reasoning_content') in both sync and async non-streaming and streaming chat completion paths, so that reasoning output is correctly surfaced when content is absent. Fixes crewAIInc#5537 Signed-off-by: NIK-TIGER-BILL <nik.tiger.bill@github.com>
|
Please run pre-commit hooks and do not use |
…content Replaces getattr(message, 'reasoning_content', '') and getattr(chunk_delta, 'reasoning_content', '') with try/except AttributeError blocks as requested by reviewer. Also ran ruff check/format. Signed-off-by: NIK-TIGER-BILL <nik.tiger.bill@github.com>
|
@greysonlalonde Thanks for the review! I've pushed a fix that removes all |
📝 WalkthroughWalkthroughUpdated OpenAI chat completion handler to extract final content from either ChangesReasoning Content Fallback
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
lib/crewai/tests/llms/openai/test_openai.py (1)
30-64: ⚡ Quick winExpand test coverage to include edge cases and validate attribute error handling.
The test validates the happy path but misses several edge cases:
- Both fields present: Verify that
contenttakes precedence when bothcontentandreasoning_contentexist- AttributeError path: Verify behavior when
reasoning_contentattribute doesn't exist (true AttributeError)- Both None/empty: Verify that an empty string is returned when both are None
🧪 Suggested edge case tests
def test_openai_completion_content_takes_precedence_over_reasoning(): """Test that content is used when both content and reasoning_content exist.""" llm = OpenAICompletion(model="gpt-4o", stream=False) message = ChatCompletionMessage.model_validate({ "role": "assistant", "content": "Final answer", "reasoning_content": "Chain of thought reasoning", }) completion = ChatCompletion.model_validate({ "id": "test-id", "object": "chat.completion", "created": 1234567890, "model": "gpt-4o", "choices": [{ "index": 0, "message": message.model_dump(), "finish_reason": "stop", }], "usage": { "prompt_tokens": 10, "completion_tokens": 5, "total_tokens": 15, }, }) with patch.object(llm, "_get_sync_client") as mock_get_client: mock_client = MagicMock() mock_client.chat.completions.create.return_value = completion mock_get_client.return_value = mock_client result = llm.call(messages=[{"role": "user", "content": "Hello"}]) assert result == "Final answer" def test_openai_completion_handles_missing_reasoning_content_attribute(): """Test that AttributeError is handled when reasoning_content doesn't exist.""" llm = OpenAICompletion(model="gpt-4o", stream=False) # Create a message without reasoning_content field at all message = MagicMock() message.content = None message.tool_calls = None # No reasoning_content attribute - will raise AttributeError on access del message.reasoning_content completion = MagicMock() completion.choices = [MagicMock()] completion.choices[0].message = message completion.usage = MagicMock(prompt_tokens=10, completion_tokens=5, total_tokens=15) with patch.object(llm, "_get_sync_client") as mock_get_client: mock_client = MagicMock() mock_client.chat.completions.create.return_value = completion mock_get_client.return_value = mock_client result = llm.call(messages=[{"role": "user", "content": "Hello"}]) assert result == ""🤖 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/test_openai.py` around lines 30 - 64, Add three tests for OpenAICompletion to cover edge cases: (1) test that when both ChatCompletionMessage.content and ChatCompletionMessage.reasoning_content exist the llm.call (OpenAICompletion.call) returns the content value (reference message and completion objects used in test_openai_completion_reasoning_content_fallback), (2) test that if the message lacks the reasoning_content attribute entirely the llm.call handles the AttributeError and returns an empty string (mock a message object without reasoning_content and ensure completion.choices[0].message is that mock), and (3) test that if both content and reasoning_content are None or empty the llm.call returns an empty string; use the same mocking pattern with patch.object(llm, "_get_sync_client") and mock_client.chat.completions.create.return_value to supply completions and assert expected return values.lib/crewai/src/crewai/llms/providers/openai/completion.py (1)
1926-1933: ⚡ Quick winAdd test coverage for the streaming path with reasoning_content.
The streaming completion handler now falls back to
reasoning_content, but the test suite only covers the non-streaming path (test_openai_completion_reasoning_content_fallback). This leaves the streaming logic untested.🧪 Suggested test to add
def test_openai_streaming_completion_reasoning_content_fallback(): """Test that streaming falls back to reasoning_content when content is empty.""" llm = OpenAICompletion(model="gpt-4o", stream=True) # Mock streaming chunks with reasoning_content mock_chunk = MagicMock() mock_chunk.choices = [MagicMock()] mock_chunk.choices[0].delta = MagicMock() mock_chunk.choices[0].delta.content = None mock_chunk.choices[0].delta.reasoning_content = "Reasoning chunk" mock_chunk.choices[0].delta.tool_calls = None mock_chunk.id = "test-id" mock_usage_chunk = MagicMock() mock_usage_chunk.choices = [] mock_usage_chunk.usage = MagicMock( prompt_tokens=10, completion_tokens=5, total_tokens=15, ) with patch.object(llm, "_get_sync_client") as mock_get_client: mock_client = MagicMock() mock_client.chat.completions.create.return_value = iter([mock_chunk, mock_usage_chunk]) mock_get_client.return_value = mock_client result = llm.call(messages=[{"role": "user", "content": "Hello"}]) assert "Reasoning chunk" in result🤖 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 1926 - 1933, Add a unit test named test_openai_streaming_completion_reasoning_content_fallback that instantiates OpenAICompletion(model="gpt-4o", stream=True), patches OpenAICompletion._get_sync_client to return a mock client whose chat.completions.create yields an iterator of two items: a streaming chunk where choices[0].delta.content is None and choices[0].delta.reasoning_content contains "Reasoning chunk" (and tool_calls is None), and a usage chunk with usage.prompt_tokens/completion_tokens/total_tokens set; call llm.call(messages=[{"role":"user","content":"Hello"}]) and assert the returned string contains "Reasoning chunk" to exercise the branch that falls back to reasoning_content and triggers _emit_stream_chunk_event.
🤖 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 1684-1688: The try/except around accessing
message.reasoning_content is misleading because standard OpenAI
ChatCompletionMessage doesn't have that field; update the logic in the message
handling code that sets content (the block that reads reasoning =
message.reasoning_content or "" / except AttributeError / content =
message.content or reasoning or "") to either remove the reasoning_content
access entirely, or gate it behind an explicit provider check (e.g., only
attempt message.reasoning_content for non-standard providers like OpenRouter) so
you don't rely on a caught AttributeError; locate and fix the four occurrences
(the message handling function(s) that reference message.reasoning_content at
the four spots) and ensure content falls back to message.content if
reasoning_content is not available.
---
Nitpick comments:
In `@lib/crewai/src/crewai/llms/providers/openai/completion.py`:
- Around line 1926-1933: Add a unit test named
test_openai_streaming_completion_reasoning_content_fallback that instantiates
OpenAICompletion(model="gpt-4o", stream=True), patches
OpenAICompletion._get_sync_client to return a mock client whose
chat.completions.create yields an iterator of two items: a streaming chunk where
choices[0].delta.content is None and choices[0].delta.reasoning_content contains
"Reasoning chunk" (and tool_calls is None), and a usage chunk with
usage.prompt_tokens/completion_tokens/total_tokens set; call
llm.call(messages=[{"role":"user","content":"Hello"}]) and assert the returned
string contains "Reasoning chunk" to exercise the branch that falls back to
reasoning_content and triggers _emit_stream_chunk_event.
In `@lib/crewai/tests/llms/openai/test_openai.py`:
- Around line 30-64: Add three tests for OpenAICompletion to cover edge cases:
(1) test that when both ChatCompletionMessage.content and
ChatCompletionMessage.reasoning_content exist the llm.call
(OpenAICompletion.call) returns the content value (reference message and
completion objects used in test_openai_completion_reasoning_content_fallback),
(2) test that if the message lacks the reasoning_content attribute entirely the
llm.call handles the AttributeError and returns an empty string (mock a message
object without reasoning_content and ensure completion.choices[0].message is
that mock), and (3) test that if both content and reasoning_content are None or
empty the llm.call returns an empty string; use the same mocking pattern with
patch.object(llm, "_get_sync_client") and
mock_client.chat.completions.create.return_value to supply completions and
assert expected return values.
🪄 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: 3e6789ed-b83e-4d30-af9d-3656927d1b96
📒 Files selected for processing (2)
lib/crewai/src/crewai/llms/providers/openai/completion.pylib/crewai/tests/llms/openai/test_openai.py
| try: | ||
| reasoning = message.reasoning_content or "" | ||
| except AttributeError: | ||
| reasoning = "" | ||
| content = message.content or reasoning or "" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify reasoning_content field in OpenAI SDK ChatCompletionMessage
# Check if reasoning_content is documented in the OpenAI Python SDK
rg -n "reasoning_content" --type py -g "!test*" -C3Repository: crewAIInc/crewAI
Length of output: 2801
🌐 Web query:
Does the OpenAI Python SDK version 2.30.0 ChatCompletionMessage include a reasoning_content field?
💡 Result:
No. In the OpenAI Python SDK (openai/openai-python) the ChatCompletionMessage type does not define a reasoning_content field. The class only includes content, refusal, role, annotations, audio, function_call, and tool_calls (among any optional tool-related fields), with no reasoning_content attribute present. [1] While other libraries/issues discuss a reasoning_content key for different APIs/providers, that is not part of the SDK’s ChatCompletionMessage model in the v2.30.0 codebase/spec. [1][2]
Citations:
- 1: https://github.com/openai/openai-python/blob/main/src/openai/types/chat/chat_completion_message.py
- 2: https://developers.openai.com/api/reference/python/resources/chat/
Remove or clarify the non-standard reasoning_content field usage.
The OpenAI SDK v2.30.0 does not include a reasoning_content field on ChatCompletionMessage. The try-except pattern at lines 1684-1688 will always catch AttributeError when using the standard OpenAI API, making it ineffective. This pattern appears in 4 locations (lines 1685, 1927, 2086, 2238).
Either remove the try-except block if reasoning_content is not needed, or clarify if this code is intended to support a non-standard provider (e.g., OpenRouter) and conditionally apply it only when appropriate.
🤖 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 1684
- 1688, The try/except around accessing message.reasoning_content is misleading
because standard OpenAI ChatCompletionMessage doesn't have that field; update
the logic in the message handling code that sets content (the block that reads
reasoning = message.reasoning_content or "" / except AttributeError / content =
message.content or reasoning or "") to either remove the reasoning_content
access entirely, or gate it behind an explicit provider check (e.g., only
attempt message.reasoning_content for non-standard providers like OpenRouter) so
you don't rely on a caught AttributeError; locate and fix the four occurrences
(the message handling function(s) that reference message.reasoning_content at
the four spots) and ensure content falls back to message.content if
reasoning_content is not available.
Description
OpenRouter reasoning models (e.g., Anthropic Sonnet 4.5, Gemini 3.1 Pro Preview) return a
reasoning_contentfield when the model produces chain-of-thought output. CrewAI previously only readmessage.content, which could beNonefor these responses, resulting in an empty string being returned to agents and causing failures.This change adds a fallback to
getattr(message, "reasoning_content")in both sync and async non-streaming and streaming chat completion paths, so that reasoning output is correctly surfaced whencontentis absent.Changes
lib/crewai/src/crewai/llms/providers/openai/completion.py_handle_completion:message.content→ fallback onmessage.reasoning_content_ahandle_completion: same fallback_handle_streaming_completion:chunk_delta.content→ fallback onchunk_delta.reasoning_content_ahandle_streaming_completion: same fallbacklib/crewai/tests/llms/openai/test_openai.pytest_openai_completion_reasoning_content_fallbackto verify the fallback with a mockedChatCompletioncontainingreasoning_content.Checklist
Summary by CodeRabbit
New Features
Tests