Skip to content

Commit 4e49d9a

Browse files
authored
fix: (bug): Drop reasoningContent from request (strands-agents#1099)
* fix: (bug): Drop reasoningContent from request * fix: (bug): Drop reasoningContent from request * fix: (bug): Drop reasoningContent from request
1 parent c2ba0f7 commit 4e49d9a

File tree

2 files changed

+122
-44
lines changed

2 files changed

+122
-44
lines changed

src/strands/models/openai.py

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -214,10 +214,16 @@ def format_request_messages(cls, messages: Messages, system_prompt: Optional[str
214214
for message in messages:
215215
contents = message["content"]
216216

217+
# Check for reasoningContent and warn user
218+
if any("reasoningContent" in content for content in contents):
219+
logger.warning(
220+
"reasoningContent is not supported in multi-turn conversations with the Chat Completions API."
221+
)
222+
217223
formatted_contents = [
218224
cls.format_request_message_content(content)
219225
for content in contents
220-
if not any(block_type in content for block_type in ["toolResult", "toolUse"])
226+
if not any(block_type in content for block_type in ["toolResult", "toolUse", "reasoningContent"])
221227
]
222228
formatted_tool_calls = [
223229
cls.format_request_message_tool_call(content["toolUse"]) for content in contents if "toolUse" in content
@@ -405,38 +411,46 @@ async def stream(
405411

406412
logger.debug("got response from model")
407413
yield self.format_chunk({"chunk_type": "message_start"})
408-
yield self.format_chunk({"chunk_type": "content_start", "data_type": "text"})
409-
410414
tool_calls: dict[int, list[Any]] = {}
415+
data_type = None
416+
finish_reason = None # Store finish_reason for later use
417+
event = None # Initialize for scope safety
411418

412419
async for event in response:
413420
# Defensive: skip events with empty or missing choices
414421
if not getattr(event, "choices", None):
415422
continue
416423
choice = event.choices[0]
417424

418-
if choice.delta.content:
419-
yield self.format_chunk(
420-
{"chunk_type": "content_delta", "data_type": "text", "data": choice.delta.content}
421-
)
422-
423425
if hasattr(choice.delta, "reasoning_content") and choice.delta.reasoning_content:
426+
chunks, data_type = self._stream_switch_content("reasoning_content", data_type)
427+
for chunk in chunks:
428+
yield chunk
424429
yield self.format_chunk(
425430
{
426431
"chunk_type": "content_delta",
427-
"data_type": "reasoning_content",
432+
"data_type": data_type,
428433
"data": choice.delta.reasoning_content,
429434
}
430435
)
431436

437+
if choice.delta.content:
438+
chunks, data_type = self._stream_switch_content("text", data_type)
439+
for chunk in chunks:
440+
yield chunk
441+
yield self.format_chunk(
442+
{"chunk_type": "content_delta", "data_type": data_type, "data": choice.delta.content}
443+
)
444+
432445
for tool_call in choice.delta.tool_calls or []:
433446
tool_calls.setdefault(tool_call.index, []).append(tool_call)
434447

435448
if choice.finish_reason:
449+
finish_reason = choice.finish_reason # Store for use outside loop
450+
if data_type:
451+
yield self.format_chunk({"chunk_type": "content_stop", "data_type": data_type})
436452
break
437453

438-
yield self.format_chunk({"chunk_type": "content_stop", "data_type": "text"})
439-
440454
for tool_deltas in tool_calls.values():
441455
yield self.format_chunk({"chunk_type": "content_start", "data_type": "tool", "data": tool_deltas[0]})
442456

@@ -445,17 +459,37 @@ async def stream(
445459

446460
yield self.format_chunk({"chunk_type": "content_stop", "data_type": "tool"})
447461

448-
yield self.format_chunk({"chunk_type": "message_stop", "data": choice.finish_reason})
462+
yield self.format_chunk({"chunk_type": "message_stop", "data": finish_reason or "end_turn"})
449463

450464
# Skip remaining events as we don't have use for anything except the final usage payload
451465
async for event in response:
452466
_ = event
453467

454-
if event.usage:
468+
if event and hasattr(event, "usage") and event.usage:
455469
yield self.format_chunk({"chunk_type": "metadata", "data": event.usage})
456470

457471
logger.debug("finished streaming response from model")
458472

473+
def _stream_switch_content(self, data_type: str, prev_data_type: str | None) -> tuple[list[StreamEvent], str]:
474+
"""Handle switching to a new content stream.
475+
476+
Args:
477+
data_type: The next content data type.
478+
prev_data_type: The previous content data type.
479+
480+
Returns:
481+
Tuple containing:
482+
- Stop block for previous content and the start block for the next content.
483+
- Next content data type.
484+
"""
485+
chunks = []
486+
if data_type != prev_data_type:
487+
if prev_data_type is not None:
488+
chunks.append(self.format_chunk({"chunk_type": "content_stop", "data_type": prev_data_type}))
489+
chunks.append(self.format_chunk({"chunk_type": "content_start", "data_type": data_type}))
490+
491+
return chunks, data_type
492+
459493
@override
460494
async def structured_output(
461495
self, output_model: Type[T], prompt: Messages, system_prompt: Optional[str] = None, **kwargs: Any

tests/strands/models/test_openai.py

Lines changed: 75 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -561,11 +561,13 @@ async def test_stream(openai_client, model_id, model, agenerator, alist):
561561
tru_events = await alist(response)
562562
exp_events = [
563563
{"messageStart": {"role": "assistant"}},
564-
{"contentBlockStart": {"start": {}}},
564+
{"contentBlockStart": {"start": {}}}, # reasoning_content starts
565565
{"contentBlockDelta": {"delta": {"reasoningContent": {"text": "\nI'm thinking"}}}},
566+
{"contentBlockStop": {}}, # reasoning_content ends
567+
{"contentBlockStart": {"start": {}}}, # text starts
566568
{"contentBlockDelta": {"delta": {"text": "I'll calculate"}}},
567569
{"contentBlockDelta": {"delta": {"text": "that for you"}}},
568-
{"contentBlockStop": {}},
570+
{"contentBlockStop": {}}, # text ends
569571
{
570572
"contentBlockStart": {
571573
"start": {
@@ -631,9 +633,7 @@ async def test_stream_empty(openai_client, model_id, model, agenerator, alist):
631633
tru_events = await alist(response)
632634
exp_events = [
633635
{"messageStart": {"role": "assistant"}},
634-
{"contentBlockStart": {"start": {}}},
635-
{"contentBlockStop": {}},
636-
{"messageStop": {"stopReason": "end_turn"}},
636+
{"messageStop": {"stopReason": "end_turn"}}, # No content blocks when no content
637637
]
638638

639639
assert len(tru_events) == len(exp_events)
@@ -678,10 +678,10 @@ async def test_stream_with_empty_choices(openai_client, model, agenerator, alist
678678
tru_events = await alist(response)
679679
exp_events = [
680680
{"messageStart": {"role": "assistant"}},
681-
{"contentBlockStart": {"start": {}}},
681+
{"contentBlockStart": {"start": {}}}, # text content starts
682682
{"contentBlockDelta": {"delta": {"text": "content"}}},
683683
{"contentBlockDelta": {"delta": {"text": "content"}}},
684-
{"contentBlockStop": {}},
684+
{"contentBlockStop": {}}, # text content ends
685685
{"messageStop": {"stopReason": "end_turn"}},
686686
{
687687
"metadata": {
@@ -756,6 +756,74 @@ def test_tool_choice_none_no_warning(model, messages, captured_warnings):
756756
assert len(captured_warnings) == 0
757757

758758

759+
@pytest.mark.parametrize(
760+
"new_data_type, prev_data_type, expected_chunks, expected_data_type",
761+
[
762+
("text", None, [{"contentBlockStart": {"start": {}}}], "text"),
763+
(
764+
"reasoning_content",
765+
"text",
766+
[{"contentBlockStop": {}}, {"contentBlockStart": {"start": {}}}],
767+
"reasoning_content",
768+
),
769+
("text", "text", [], "text"),
770+
],
771+
)
772+
def test__stream_switch_content(model, new_data_type, prev_data_type, expected_chunks, expected_data_type):
773+
"""Test _stream_switch_content method for content type switching."""
774+
chunks, data_type = model._stream_switch_content(new_data_type, prev_data_type)
775+
assert chunks == expected_chunks
776+
assert data_type == expected_data_type
777+
778+
779+
def test_format_request_messages_excludes_reasoning_content():
780+
"""Test that reasoningContent is excluded from formatted messages."""
781+
messages = [
782+
{
783+
"content": [
784+
{"text": "Hello"},
785+
{"reasoningContent": {"reasoningText": {"text": "excluded"}}},
786+
],
787+
"role": "user",
788+
},
789+
]
790+
791+
tru_result = OpenAIModel.format_request_messages(messages)
792+
793+
# Only text content should be included
794+
exp_result = [
795+
{
796+
"content": [{"text": "Hello", "type": "text"}],
797+
"role": "user",
798+
},
799+
]
800+
assert tru_result == exp_result
801+
802+
803+
@pytest.mark.asyncio
804+
async def test_structured_output_context_overflow_exception(openai_client, model, messages, test_output_model_cls):
805+
"""Test that structured output also handles context overflow properly."""
806+
# Create a mock OpenAI BadRequestError with context_length_exceeded code
807+
mock_error = openai.BadRequestError(
808+
message="This model's maximum context length is 4096 tokens. However, your messages resulted in 5000 tokens.",
809+
response=unittest.mock.MagicMock(),
810+
body={"error": {"code": "context_length_exceeded"}},
811+
)
812+
mock_error.code = "context_length_exceeded"
813+
814+
# Configure the mock client to raise the context overflow error
815+
openai_client.beta.chat.completions.parse.side_effect = mock_error
816+
817+
# Test that the structured_output method converts the error properly
818+
with pytest.raises(ContextWindowOverflowException) as exc_info:
819+
async for _ in model.structured_output(test_output_model_cls, messages):
820+
pass
821+
822+
# Verify the exception message contains the original error
823+
assert "maximum context length" in str(exc_info.value)
824+
assert exc_info.value.__cause__ == mock_error
825+
826+
759827
@pytest.mark.asyncio
760828
async def test_stream_context_overflow_exception(openai_client, model, messages):
761829
"""Test that OpenAI context overflow errors are properly converted to ContextWindowOverflowException."""
@@ -803,30 +871,6 @@ async def test_stream_other_bad_request_errors_passthrough(openai_client, model,
803871
assert exc_info.value == mock_error
804872

805873

806-
@pytest.mark.asyncio
807-
async def test_structured_output_context_overflow_exception(openai_client, model, messages, test_output_model_cls):
808-
"""Test that structured output also handles context overflow properly."""
809-
# Create a mock OpenAI BadRequestError with context_length_exceeded code
810-
mock_error = openai.BadRequestError(
811-
message="This model's maximum context length is 4096 tokens. However, your messages resulted in 5000 tokens.",
812-
response=unittest.mock.MagicMock(),
813-
body={"error": {"code": "context_length_exceeded"}},
814-
)
815-
mock_error.code = "context_length_exceeded"
816-
817-
# Configure the mock client to raise the context overflow error
818-
openai_client.beta.chat.completions.parse.side_effect = mock_error
819-
820-
# Test that the structured_output method converts the error properly
821-
with pytest.raises(ContextWindowOverflowException) as exc_info:
822-
async for _ in model.structured_output(test_output_model_cls, messages):
823-
pass
824-
825-
# Verify the exception message contains the original error
826-
assert "maximum context length" in str(exc_info.value)
827-
assert exc_info.value.__cause__ == mock_error
828-
829-
830874
@pytest.mark.asyncio
831875
async def test_stream_rate_limit_as_throttle(openai_client, model, messages):
832876
"""Test that all rate limit errors are converted to ModelThrottledException."""

0 commit comments

Comments
 (0)