* 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>
This commit is contained in:
parent
76be770091
commit
88d1d6206f
2 changed files with 69 additions and 8 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue