From 88d1d6206f399c134d1f4c0b7db27733aaa3c50c Mon Sep 17 00:00:00 2001 From: kshitij <82637225+kshitijk4poor@users.noreply.github.com> Date: Thu, 2 Jul 2026 06:36:20 +0530 Subject: [PATCH] fix(streaming): handle completed responses with empty/None choices (#55933) (#56713) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(streaming): handle completed responses with empty/None choices The streaming fallback guard added in #55932 recognized a completed response object only when its `choices` was a non-empty list. But an adapter can return a completed response whose `choices` is `None` or an empty list (an error / content-filter / terminal frame) — still a whole, non-iterable response, not a token stream. Those shapes fell through to `for chunk in stream` and crashed with 'types.SimpleNamespace' object is not iterable which is exactly issue #55933 (MoA `openai-codex` aggregator on TUI/Desktop, where a stream consumer forces the streaming path). Broaden the guard to discriminate on the PRESENCE of a `choices` attribute (a genuine provider Stream object exposes none), disable streaming for the session, and return the completed object so the outer loop's normal invalid-response validation handles empty/None choices via its retry path instead of iterating. Based on the diagnosis in #56525 by @spiky02plateau (that PR normalized the MoA aggregator return with a one-shot chunk iterator; the common text/tool-call crash was already fixed at this seam by #55932, so this extends the existing guard to cover only the remaining empty/None-choices gap). Fixes #55933 * refactor(streaming): simplify empty-choices guard body and parametrize tests Post-review cleanup (no behavior change): - Inline the single-use `response_choices` local and drop the redundant `if first_choice is not None else None` guard (getattr(None, ...) already returns the default safely). - Collapse the two near-identical empty/None-choices regression tests into one `@pytest.mark.parametrize` case. Mutation-verified: reverting the guard to the old non-empty-list condition still makes both parametrized cases fail with the historical 'types.SimpleNamespace' object is not iterable. --------- Co-authored-by: spiky02plateau <155588579+spiky02plateau@users.noreply.github.com> --- agent/chat_completion_helpers.py | 30 ++++++++++++++------ tests/run_agent/test_streaming.py | 47 +++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 8 deletions(-) diff --git a/agent/chat_completion_helpers.py b/agent/chat_completion_helpers.py index 5eca94a32..7a7064d72 100644 --- a/agent/chat_completion_helpers.py +++ b/agent/chat_completion_helpers.py @@ -2017,13 +2017,21 @@ def interruptible_streaming_api_call(agent, api_kwargs: dict, *, on_first_delta= request_client_holder["diag"] = _diag stream = request_client.chat.completions.create(**stream_kwargs) - # Some OpenAI-compatible adapters (for example copilot-acp) accept - # stream=True but still return a completed response object rather than - # an iterator of chunks. Treat that as "streaming unsupported" for the - # rest of this session instead of crashing on ``for chunk in stream`` - # with ``'types.SimpleNamespace' object is not iterable`` (#11732). - response_choices = getattr(stream, "choices", None) - if isinstance(response_choices, list) and response_choices: + # Some OpenAI-compatible adapters (for example copilot-acp, and the MoA + # openai-codex aggregator) accept stream=True but still return a + # completed response object rather than an iterator of chunks. Treat + # that as "streaming unsupported" for the rest of this session instead + # of crashing on ``for chunk in stream`` with ``'types.SimpleNamespace' + # object is not iterable`` (#11732, #55933). + # + # Discriminate on the mere PRESENCE of a ``choices`` attribute, not on + # it being a non-empty list: an adapter may hand back a completed + # response whose ``choices`` is ``None`` or empty (an error / + # content-filter / terminal frame), and every such shape is still a + # whole response — not a token stream — that would crash iteration just + # the same. A genuine provider stream (SDK ``Stream`` object, + # generator) exposes no ``choices`` attribute, so it is left untouched. + if hasattr(stream, "choices"): logger.info( "Streaming request returned a final response object instead of " "an iterator; switching %s/%s to non-streaming for this session.", @@ -2031,7 +2039,13 @@ def interruptible_streaming_api_call(agent, api_kwargs: dict, *, on_first_delta= agent.model or "unknown", ) agent._disable_streaming = True - message = getattr(response_choices[0], "message", None) + # An empty/None ``choices`` carries no message to surface; return the + # completed object as-is so the outer loop's normal invalid-response + # validation (conversation_loop.py) handles it via the retry path, + # never ``for chunk in stream``. + choices = stream.choices + first_choice = choices[0] if isinstance(choices, (list, tuple)) and choices else None + message = getattr(first_choice, "message", None) if message is not None: reasoning_text = ( getattr(message, "reasoning_content", None) diff --git a/tests/run_agent/test_streaming.py b/tests/run_agent/test_streaming.py index 2c4f8f8d0..7ccafff66 100644 --- a/tests/run_agent/test_streaming.py +++ b/tests/run_agent/test_streaming.py @@ -671,6 +671,53 @@ class TestStreamingFallback: assert agent._disable_streaming is True assert deltas == ["Hello from ACP"] + @pytest.mark.parametrize("choices", [[], None], ids=["empty", "none"]) + @patch("run_agent.AIAgent._create_request_openai_client") + @patch("run_agent.AIAgent._close_request_openai_client") + def test_completed_response_no_usable_choices_returned_not_iterated( + self, mock_close, mock_create, choices + ): + """A completed response whose ``choices`` is empty ``[]`` or ``None`` is + still a whole (non-iterable) response, not a token stream. + + The pre-existing guard (#55932) recognized a completed response only + when ``choices`` was a *non-empty* list, so an empty/terminal or + error/content-filter frame fell through to ``for chunk in stream`` and + crashed with ``'types.SimpleNamespace' object is not iterable`` (#55933, + hit by the MoA openai-codex aggregator). It must now disable streaming + and return the object for the outer loop's invalid-response retry path + instead of iterating it. + """ + from run_agent import AIAgent + + final_response = SimpleNamespace(model="gpt-5.5", choices=choices, usage=None) + + mock_client = MagicMock() + mock_client.chat.completions.create.return_value = final_response + mock_create.return_value = mock_client + + agent = AIAgent( + model="default", + provider="moa", + api_key="test-key", + base_url="moa://local", + quiet_mode=True, + skip_context_files=True, + skip_memory=True, + ) + agent.api_mode = "chat_completions" + agent._interrupt_requested = False + + deltas = [] + agent._stream_callback = lambda text: deltas.append(text) + + # Must NOT raise "'types.SimpleNamespace' object is not iterable". + response = agent._interruptible_streaming_api_call({}) + + assert response is final_response + assert agent._disable_streaming is True + assert deltas == [] + @patch("run_agent.AIAgent._create_request_openai_client") @patch("run_agent.AIAgent._close_request_openai_client") def test_stream_error_propagates_original(self, mock_close, mock_create):