From 3fedbdc5199a02e6a3f0e318c15903250e51d58b Mon Sep 17 00:00:00 2001 From: jgarrison929 Date: Fri, 13 Mar 2026 07:12:32 -0700 Subject: [PATCH 1/3] fix: preserve encrypted_content in reasoning parsing when summaries are present When _parse_response_from_openai encounters a reasoning item with both summary and encrypted_content, the encrypted_content was silently dropped because only the fallback branch (no content, no summary) captured it. This fix extracts encrypted_content once at the top of the reasoning case and propagates it through all three branches (content, summary-only, fallback) so it round-trips correctly via _prepare_content_for_openai. The streaming parser was already correct (not affected). Fixes #4644 --- .../agent_framework_openai/_chat_client.py | 17 +++- .../tests/openai/test_openai_chat_client.py | 77 +++++++++++++++++++ 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/python/packages/openai/agent_framework_openai/_chat_client.py b/python/packages/openai/agent_framework_openai/_chat_client.py index 86af86895e..ae5bdbb75f 100644 --- a/python/packages/openai/agent_framework_openai/_chat_client.py +++ b/python/packages/openai/agent_framework_openai/_chat_client.py @@ -1631,11 +1631,20 @@ def _parse_response_from_openai( ) case "reasoning": # ResponseOutputReasoning added_reasoning = False + # Extract encrypted_content once so it is propagated through + # whichever branch fires and can round-trip via + # _prepare_content_for_openai. Previously only the fallback + # (no-content, no-summary) branch captured it, so responses + # containing both summaries and encrypted_content silently + # dropped the encrypted payload. See #4644. + encrypted_content = getattr(item, "encrypted_content", None) if item_content := getattr(item, "content", None): for index, reasoning_content in enumerate(item_content): additional_properties: dict[str, Any] = {} if hasattr(item, "summary") and item.summary and index < len(item.summary): additional_properties["summary"] = item.summary[index] + if encrypted_content: + additional_properties["encrypted_content"] = encrypted_content contents.append( Content.from_text_reasoning( id=item.id, @@ -1647,11 +1656,15 @@ def _parse_response_from_openai( added_reasoning = True if item_summary := getattr(item, "summary", None): for summary in item_summary: + summary_additional: dict[str, Any] = {} + if encrypted_content: + summary_additional["encrypted_content"] = encrypted_content contents.append( Content.from_text_reasoning( id=item.id, text=summary.text, raw_representation=summary, # type: ignore[arg-type] + additional_properties=summary_additional or None, ) ) added_reasoning = True @@ -1659,8 +1672,8 @@ def _parse_response_from_openai( # Reasoning item with no visible text (e.g. encrypted reasoning). # Always emit an empty marker so co-occurrence detection can be done additional_properties_empty: dict[str, Any] = {} - if encrypted := getattr(item, "encrypted_content", None): - additional_properties_empty["encrypted_content"] = encrypted + if encrypted_content: + additional_properties_empty["encrypted_content"] = encrypted_content contents.append( Content.from_text_reasoning( id=item.id, diff --git a/python/packages/openai/tests/openai/test_openai_chat_client.py b/python/packages/openai/tests/openai/test_openai_chat_client.py index 897fe5f913..8d8097ffda 100644 --- a/python/packages/openai/tests/openai/test_openai_chat_client.py +++ b/python/packages/openai/tests/openai/test_openai_chat_client.py @@ -534,6 +534,83 @@ def test_response_content_creation_with_reasoning() -> None: assert response.messages[0].contents[0].text == "Reasoning step" +def test_response_reasoning_preserves_encrypted_content_with_summary() -> None: + """encrypted_content must survive when both content/summary and encrypted_content are present. + + Regression test for #4644: _parse_response_from_openai dropped encrypted_content + when reasoning summaries were also present because only the fallback (no-content, + no-summary) branch captured it. + """ + client = OpenAIResponsesClient(model_id="test-model", api_key="test-key") + + mock_response = MagicMock() + mock_response.output_parsed = None + mock_response.metadata = {} + mock_response.usage = None + mock_response.id = "test-id" + mock_response.model = "test-model" + mock_response.created_at = 1000000000 + + mock_reasoning_content = MagicMock() + mock_reasoning_content.text = "Reasoning step" + + mock_reasoning_item = MagicMock() + mock_reasoning_item.type = "reasoning" + mock_reasoning_item.id = "rs_enc" + mock_reasoning_item.content = [mock_reasoning_content] + mock_reasoning_item.summary = [Summary(text="Summary text", type="summary_text")] + mock_reasoning_item.encrypted_content = "gAAAA_encrypted_payload" + + mock_response.output = [mock_reasoning_item] + + response = client._parse_response_from_openai(mock_response, options={}) # type: ignore + + # The content branch should carry encrypted_content in additional_properties + reasoning_contents = [c for c in response.messages[0].contents if c.type == "text_reasoning"] + assert len(reasoning_contents) >= 1 + first_reasoning = reasoning_contents[0] + assert first_reasoning.text == "Reasoning step" + assert first_reasoning.additional_properties is not None + assert first_reasoning.additional_properties.get("encrypted_content") == "gAAAA_encrypted_payload" + assert first_reasoning.additional_properties.get("summary") is not None + + +def test_response_reasoning_preserves_encrypted_content_summary_only() -> None: + """encrypted_content must survive when only summary (no content) is present. + + Covers the case where the API returns summary + encrypted_content but no + clear-text reasoning content. + """ + client = OpenAIResponsesClient(model_id="test-model", api_key="test-key") + + mock_response = MagicMock() + mock_response.output_parsed = None + mock_response.metadata = {} + mock_response.usage = None + mock_response.id = "test-id" + mock_response.model = "test-model" + mock_response.created_at = 1000000000 + + mock_reasoning_item = MagicMock() + mock_reasoning_item.type = "reasoning" + mock_reasoning_item.id = "rs_enc2" + mock_reasoning_item.content = None # No clear-text content + mock_reasoning_item.summary = [Summary(text="Summary only", type="summary_text")] + mock_reasoning_item.encrypted_content = "gAAAA_encrypted_payload_2" + + mock_response.output = [mock_reasoning_item] + + response = client._parse_response_from_openai(mock_response, options={}) # type: ignore + + reasoning_contents = [c for c in response.messages[0].contents if c.type == "text_reasoning"] + assert len(reasoning_contents) >= 1 + # The summary branch should carry encrypted_content + summary_reasoning = reasoning_contents[0] + assert summary_reasoning.text == "Summary only" + assert summary_reasoning.additional_properties is not None + assert summary_reasoning.additional_properties.get("encrypted_content") == "gAAAA_encrypted_payload_2" + + def test_response_content_keeps_reasoning_and_function_calls_in_one_message() -> None: """Reasoning + function calls should parse into one assistant message.""" client = OpenAIChatClient(model="test-model", api_key="test-key") From 4f5af1df30d4cbce55b1c3b2eab0e54571304080 Mon Sep 17 00:00:00 2001 From: Josh Garrison Date: Tue, 17 Mar 2026 23:04:47 -0700 Subject: [PATCH 2/3] 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 --- .../agent_framework_openai/_chat_client.py | 11 ++++- .../tests/openai/test_openai_chat_client.py | 41 +++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/python/packages/openai/agent_framework_openai/_chat_client.py b/python/packages/openai/agent_framework_openai/_chat_client.py index ae5bdbb75f..02f017ce33 100644 --- a/python/packages/openai/agent_framework_openai/_chat_client.py +++ b/python/packages/openai/agent_framework_openai/_chat_client.py @@ -2270,6 +2270,11 @@ def _parse_chunk_from_openai( case "reasoning": # ResponseOutputReasoning reasoning_id = getattr(event_item, "id", None) added_reasoning = False + # Extract encrypted_content once so it is propagated + # through whichever branch fires – mirrors the + # non-streaming fix in _parse_response_from_openai. + # See #4644. + encrypted_content = getattr(event_item, "encrypted_content", None) if hasattr(event_item, "content") and event_item.content: for index, reasoning_content in enumerate(event_item.content): additional_properties: dict[str, Any] = {} @@ -2279,6 +2284,8 @@ def _parse_chunk_from_openai( and index < len(event_item.summary) ): additional_properties["summary"] = event_item.summary[index] + if encrypted_content: + additional_properties["encrypted_content"] = encrypted_content contents.append( Content.from_text_reasoning( id=reasoning_id or None, @@ -2292,8 +2299,8 @@ def _parse_chunk_from_openai( # Reasoning item with no visible text (e.g. encrypted reasoning). # Always emit an empty marker so co-occurrence detection can occur. additional_properties_empty: dict[str, Any] = {} - if encrypted := getattr(event_item, "encrypted_content", None): - additional_properties_empty["encrypted_content"] = encrypted + if encrypted_content: + additional_properties_empty["encrypted_content"] = encrypted_content contents.append( Content.from_text_reasoning( id=reasoning_id or None, diff --git a/python/packages/openai/tests/openai/test_openai_chat_client.py b/python/packages/openai/tests/openai/test_openai_chat_client.py index 8d8097ffda..511c2c2fc8 100644 --- a/python/packages/openai/tests/openai/test_openai_chat_client.py +++ b/python/packages/openai/tests/openai/test_openai_chat_client.py @@ -524,6 +524,7 @@ def test_response_content_creation_with_reasoning() -> None: mock_reasoning_item.type = "reasoning" mock_reasoning_item.content = [mock_reasoning_content] mock_reasoning_item.summary = [Summary(text="Summary", type="summary_text")] + mock_reasoning_item.encrypted_content = None mock_response.output = [mock_reasoning_item] @@ -574,6 +575,11 @@ def test_response_reasoning_preserves_encrypted_content_with_summary() -> None: assert first_reasoning.additional_properties.get("encrypted_content") == "gAAAA_encrypted_payload" assert first_reasoning.additional_properties.get("summary") is not None + # The summary branch Content should also carry encrypted_content + assert len(reasoning_contents) >= 2 + assert reasoning_contents[1].additional_properties is not None + assert reasoning_contents[1].additional_properties.get("encrypted_content") == "gAAAA_encrypted_payload" + def test_response_reasoning_preserves_encrypted_content_summary_only() -> None: """encrypted_content must survive when only summary (no content) is present. @@ -611,6 +617,40 @@ def test_response_reasoning_preserves_encrypted_content_summary_only() -> None: assert summary_reasoning.additional_properties.get("encrypted_content") == "gAAAA_encrypted_payload_2" +def test_response_reasoning_no_encrypted_content() -> None: + """When encrypted_content is None/missing, additional_properties should not contain it.""" + client = OpenAIResponsesClient(model_id="test-model", api_key="test-key") + + mock_response = MagicMock() + mock_response.output_parsed = None + mock_response.metadata = {} + mock_response.usage = None + mock_response.id = "test-id" + mock_response.model = "test-model" + mock_response.created_at = 1000000000 + + mock_reasoning_content = MagicMock() + mock_reasoning_content.text = "Reasoning step" + + mock_reasoning_item = MagicMock() + mock_reasoning_item.type = "reasoning" + mock_reasoning_item.id = "rs_no_enc" + mock_reasoning_item.content = [mock_reasoning_content] + mock_reasoning_item.summary = [Summary(text="Summary text", type="summary_text")] + mock_reasoning_item.encrypted_content = None + + mock_response.output = [mock_reasoning_item] + + response = client._parse_response_from_openai(mock_response, options={}) # type: ignore + + reasoning_contents = [c for c in response.messages[0].contents if c.type == "text_reasoning"] + assert len(reasoning_contents) >= 1 + for rc in reasoning_contents: + # additional_properties should either be None or not contain encrypted_content + if rc.additional_properties is not None: + assert "encrypted_content" not in rc.additional_properties + + def test_response_content_keeps_reasoning_and_function_calls_in_one_message() -> None: """Reasoning + function calls should parse into one assistant message.""" client = OpenAIChatClient(model="test-model", api_key="test-key") @@ -631,6 +671,7 @@ def test_response_content_keeps_reasoning_and_function_calls_in_one_message() -> mock_reasoning_item.id = "rs_123" mock_reasoning_item.content = [mock_reasoning_content] mock_reasoning_item.summary = [] + mock_reasoning_item.encrypted_content = None mock_function_call_item_1 = MagicMock() mock_function_call_item_1.type = "function_call" From 49a56cb9f688081978ef4425328e9a261d0c2483 Mon Sep 17 00:00:00 2001 From: Josh Garrison Date: Thu, 26 Mar 2026 19:45:00 -0700 Subject: [PATCH 3/3] style: run prek hooks per review feedback --- .../packages/openai/agent_framework_openai/_chat_client.py | 2 +- .../packages/openai/tests/openai/test_openai_chat_client.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/python/packages/openai/agent_framework_openai/_chat_client.py b/python/packages/openai/agent_framework_openai/_chat_client.py index 02f017ce33..112ec95bcb 100644 --- a/python/packages/openai/agent_framework_openai/_chat_client.py +++ b/python/packages/openai/agent_framework_openai/_chat_client.py @@ -2271,7 +2271,7 @@ def _parse_chunk_from_openai( reasoning_id = getattr(event_item, "id", None) added_reasoning = False # Extract encrypted_content once so it is propagated - # through whichever branch fires – mirrors the + # through whichever branch fires - mirrors the # non-streaming fix in _parse_response_from_openai. # See #4644. encrypted_content = getattr(event_item, "encrypted_content", None) diff --git a/python/packages/openai/tests/openai/test_openai_chat_client.py b/python/packages/openai/tests/openai/test_openai_chat_client.py index 511c2c2fc8..dc69d588ab 100644 --- a/python/packages/openai/tests/openai/test_openai_chat_client.py +++ b/python/packages/openai/tests/openai/test_openai_chat_client.py @@ -542,7 +542,7 @@ def test_response_reasoning_preserves_encrypted_content_with_summary() -> None: when reasoning summaries were also present because only the fallback (no-content, no-summary) branch captured it. """ - client = OpenAIResponsesClient(model_id="test-model", api_key="test-key") + client = OpenAIChatClient(model="test-model", api_key="test-key") mock_response = MagicMock() mock_response.output_parsed = None @@ -587,7 +587,7 @@ def test_response_reasoning_preserves_encrypted_content_summary_only() -> None: Covers the case where the API returns summary + encrypted_content but no clear-text reasoning content. """ - client = OpenAIResponsesClient(model_id="test-model", api_key="test-key") + client = OpenAIChatClient(model="test-model", api_key="test-key") mock_response = MagicMock() mock_response.output_parsed = None @@ -619,7 +619,7 @@ def test_response_reasoning_preserves_encrypted_content_summary_only() -> None: def test_response_reasoning_no_encrypted_content() -> None: """When encrypted_content is None/missing, additional_properties should not contain it.""" - client = OpenAIResponsesClient(model_id="test-model", api_key="test-key") + client = OpenAIChatClient(model="test-model", api_key="test-key") mock_response = MagicMock() mock_response.output_parsed = None