Skip to content

Commit ff01ba3

Browse files
committed
Address review: fix streaming path + improve test coverage
- Apply same encrypted_content extraction to streaming _process_streaming_events - Fix MagicMock leak in existing reasoning tests (set encrypted_content=None) - Assert encrypted_content on summary branch Content, not just content branch - Add negative test for None encrypted_content case
1 parent 1519085 commit ff01ba3

2 files changed

Lines changed: 50 additions & 2 deletions

File tree

python/packages/core/agent_framework/openai/_responses_client.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2092,6 +2092,11 @@ def _parse_chunk_from_openai(
20922092
case "reasoning": # ResponseOutputReasoning
20932093
reasoning_id = getattr(event_item, "id", None)
20942094
added_reasoning = False
2095+
# Extract encrypted_content once so it is propagated
2096+
# through whichever branch fires – mirrors the
2097+
# non-streaming fix in _parse_response_from_openai.
2098+
# See #4644.
2099+
encrypted_content = getattr(event_item, "encrypted_content", None)
20952100
if hasattr(event_item, "content") and event_item.content:
20962101
for index, reasoning_content in enumerate(event_item.content):
20972102
additional_properties: dict[str, Any] = {}
@@ -2101,6 +2106,8 @@ def _parse_chunk_from_openai(
21012106
and index < len(event_item.summary)
21022107
):
21032108
additional_properties["summary"] = event_item.summary[index]
2109+
if encrypted_content:
2110+
additional_properties["encrypted_content"] = encrypted_content
21042111
contents.append(
21052112
Content.from_text_reasoning(
21062113
id=reasoning_id or None,
@@ -2114,8 +2121,8 @@ def _parse_chunk_from_openai(
21142121
# Reasoning item with no visible text (e.g. encrypted reasoning).
21152122
# Always emit an empty marker so co-occurrence detection can occur.
21162123
additional_properties_empty: dict[str, Any] = {}
2117-
if encrypted := getattr(event_item, "encrypted_content", None):
2118-
additional_properties_empty["encrypted_content"] = encrypted
2124+
if encrypted_content:
2125+
additional_properties_empty["encrypted_content"] = encrypted_content
21192126
contents.append(
21202127
Content.from_text_reasoning(
21212128
id=reasoning_id or None,

python/packages/core/tests/openai/test_openai_responses_client.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,7 @@ def test_response_content_creation_with_reasoning() -> None:
518518
mock_reasoning_item.type = "reasoning"
519519
mock_reasoning_item.content = [mock_reasoning_content]
520520
mock_reasoning_item.summary = [Summary(text="Summary", type="summary_text")]
521+
mock_reasoning_item.encrypted_content = None
521522

522523
mock_response.output = [mock_reasoning_item]
523524

@@ -568,6 +569,11 @@ def test_response_reasoning_preserves_encrypted_content_with_summary() -> None:
568569
assert first_reasoning.additional_properties.get("encrypted_content") == "gAAAA_encrypted_payload"
569570
assert first_reasoning.additional_properties.get("summary") is not None
570571

572+
# The summary branch Content should also carry encrypted_content
573+
assert len(reasoning_contents) >= 2
574+
assert reasoning_contents[1].additional_properties is not None
575+
assert reasoning_contents[1].additional_properties.get("encrypted_content") == "gAAAA_encrypted_payload"
576+
571577

572578
def test_response_reasoning_preserves_encrypted_content_summary_only() -> None:
573579
"""encrypted_content must survive when only summary (no content) is present.
@@ -605,6 +611,40 @@ def test_response_reasoning_preserves_encrypted_content_summary_only() -> None:
605611
assert summary_reasoning.additional_properties.get("encrypted_content") == "gAAAA_encrypted_payload_2"
606612

607613

614+
def test_response_reasoning_no_encrypted_content() -> None:
615+
"""When encrypted_content is None/missing, additional_properties should not contain it."""
616+
client = OpenAIResponsesClient(model_id="test-model", api_key="test-key")
617+
618+
mock_response = MagicMock()
619+
mock_response.output_parsed = None
620+
mock_response.metadata = {}
621+
mock_response.usage = None
622+
mock_response.id = "test-id"
623+
mock_response.model = "test-model"
624+
mock_response.created_at = 1000000000
625+
626+
mock_reasoning_content = MagicMock()
627+
mock_reasoning_content.text = "Reasoning step"
628+
629+
mock_reasoning_item = MagicMock()
630+
mock_reasoning_item.type = "reasoning"
631+
mock_reasoning_item.id = "rs_no_enc"
632+
mock_reasoning_item.content = [mock_reasoning_content]
633+
mock_reasoning_item.summary = [Summary(text="Summary text", type="summary_text")]
634+
mock_reasoning_item.encrypted_content = None
635+
636+
mock_response.output = [mock_reasoning_item]
637+
638+
response = client._parse_response_from_openai(mock_response, options={}) # type: ignore
639+
640+
reasoning_contents = [c for c in response.messages[0].contents if c.type == "text_reasoning"]
641+
assert len(reasoning_contents) >= 1
642+
for rc in reasoning_contents:
643+
# additional_properties should either be None or not contain encrypted_content
644+
if rc.additional_properties is not None:
645+
assert "encrypted_content" not in rc.additional_properties
646+
647+
608648
def test_response_content_keeps_reasoning_and_function_calls_in_one_message() -> None:
609649
"""Reasoning + function calls should parse into one assistant message."""
610650
client = OpenAIResponsesClient(model_id="test-model", api_key="test-key")
@@ -625,6 +665,7 @@ def test_response_content_keeps_reasoning_and_function_calls_in_one_message() ->
625665
mock_reasoning_item.id = "rs_123"
626666
mock_reasoning_item.content = [mock_reasoning_content]
627667
mock_reasoning_item.summary = []
668+
mock_reasoning_item.encrypted_content = None
628669

629670
mock_function_call_item_1 = MagicMock()
630671
mock_function_call_item_1.type = "function_call"

0 commit comments

Comments
 (0)