Merge pull request #85581 from bbednarski9/codex/fix-openai-sparse-response-objects
fix(openai): tolerate sparse response objects
This commit is contained in:
@@ -4140,8 +4140,9 @@ def interruptible_streaming_api_call(agent, api_kwargs: dict, *, on_first_delta=
|
||||
agent._fire_reasoning_delta(reasoning_text)
|
||||
|
||||
# Accumulate text content — fire callback only when no tool calls
|
||||
if delta and delta.content:
|
||||
content_parts.append(delta.content)
|
||||
delta_content = getattr(delta, "content", None)
|
||||
if delta_content:
|
||||
content_parts.append(delta_content)
|
||||
if not tool_calls_acc:
|
||||
if pending_text_parts or _provider_stream_text_may_be_sse(delta.content):
|
||||
pending_text_parts.append(delta.content)
|
||||
@@ -4151,7 +4152,7 @@ def interruptible_streaming_api_call(agent, api_kwargs: dict, *, on_first_delta=
|
||||
_flush_pending_stream_text()
|
||||
continue
|
||||
_fire_first_delta()
|
||||
agent._fire_stream_delta(delta.content)
|
||||
agent._fire_stream_delta(delta_content)
|
||||
deltas_were_sent["yes"] = True
|
||||
# Tool calls suppress regular content streaming (avoids
|
||||
# displaying chatty "I'll use the tool..." text alongside
|
||||
@@ -4166,17 +4167,19 @@ def interruptible_streaming_api_call(agent, api_kwargs: dict, *, on_first_delta=
|
||||
# box is already closed (tool boundary flush).
|
||||
elif agent.stream_delta_callback:
|
||||
try:
|
||||
agent.stream_delta_callback(delta.content)
|
||||
agent._record_streamed_assistant_text(delta.content)
|
||||
agent.stream_delta_callback(delta_content)
|
||||
agent._record_streamed_assistant_text(delta_content)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
# Accumulate tool call deltas — notify display on first name
|
||||
if delta and delta.tool_calls:
|
||||
delta_tool_calls = getattr(delta, "tool_calls", None)
|
||||
if delta_tool_calls:
|
||||
_flush_pending_stream_text()
|
||||
for tc_delta in delta.tool_calls:
|
||||
raw_idx = tc_delta.index if tc_delta.index is not None else 0
|
||||
delta_id = tc_delta.id or ""
|
||||
for tc_delta in delta_tool_calls:
|
||||
raw_index = getattr(tc_delta, "index", None)
|
||||
raw_idx = raw_index if raw_index is not None else 0
|
||||
delta_id = getattr(tc_delta, "id", None) or ""
|
||||
|
||||
# Ollama fix: detect a new tool call reusing the same
|
||||
# raw index (different id) and redirect to a fresh slot.
|
||||
@@ -4195,7 +4198,7 @@ def interruptible_streaming_api_call(agent, api_kwargs: dict, *, on_first_delta=
|
||||
|
||||
if idx not in tool_calls_acc:
|
||||
# Poolside may send integer id instead of string
|
||||
_tc_id = tc_delta.id
|
||||
_tc_id = getattr(tc_delta, "id", None)
|
||||
if isinstance(_tc_id, int):
|
||||
_tc_id = str(_tc_id)
|
||||
tool_calls_acc[idx] = {
|
||||
@@ -4205,14 +4208,17 @@ def interruptible_streaming_api_call(agent, api_kwargs: dict, *, on_first_delta=
|
||||
"extra_content": None,
|
||||
}
|
||||
entry = tool_calls_acc[idx]
|
||||
if tc_delta.id is not None:
|
||||
_new_id = tc_delta.id
|
||||
tc_id = getattr(tc_delta, "id", None)
|
||||
if tc_id is not None:
|
||||
_new_id = tc_id
|
||||
if isinstance(_new_id, int):
|
||||
_new_id = str(_new_id)
|
||||
if _new_id:
|
||||
entry["id"] = _new_id
|
||||
if tc_delta.function:
|
||||
if tc_delta.function.name:
|
||||
tc_function = getattr(tc_delta, "function", None)
|
||||
if tc_function:
|
||||
function_name = getattr(tc_function, "name", None)
|
||||
if function_name:
|
||||
# Use assignment, not +=. Function names are
|
||||
# atomic identifiers delivered complete in the
|
||||
# first chunk (OpenAI spec). Some providers
|
||||
@@ -4221,9 +4227,10 @@ def interruptible_streaming_api_call(agent, api_kwargs: dict, *, on_first_delta=
|
||||
# produce "read_fileread_file". Assignment
|
||||
# (matching the OpenAI Node SDK / LiteLLM /
|
||||
# Vercel AI patterns) is immune to this.
|
||||
entry["function"]["name"] = tc_delta.function.name
|
||||
if tc_delta.function.arguments:
|
||||
entry["function"]["arguments"] += tc_delta.function.arguments
|
||||
entry["function"]["name"] = function_name
|
||||
function_arguments = getattr(tc_function, "arguments", None)
|
||||
if function_arguments:
|
||||
entry["function"]["arguments"] += function_arguments
|
||||
extra = getattr(tc_delta, "extra_content", None)
|
||||
if extra is None and hasattr(tc_delta, "model_extra"):
|
||||
extra = (tc_delta.model_extra if isinstance(tc_delta.model_extra, dict) else {}).get("extra_content")
|
||||
@@ -4249,8 +4256,9 @@ def interruptible_streaming_api_call(agent, api_kwargs: dict, *, on_first_delta=
|
||||
# discarding the attempted action.
|
||||
result["partial_tool_names"].append(name)
|
||||
|
||||
if chunk.choices[0].finish_reason:
|
||||
finish_reason = chunk.choices[0].finish_reason
|
||||
chunk_finish_reason = getattr(chunk.choices[0], "finish_reason", None)
|
||||
if chunk_finish_reason:
|
||||
finish_reason = chunk_finish_reason
|
||||
|
||||
# Usage in the final chunk
|
||||
if hasattr(chunk, "usage") and chunk.usage:
|
||||
|
||||
@@ -854,17 +854,25 @@ class ChatCompletionsTransport(ProviderTransport):
|
||||
preserved for downstream replay.
|
||||
"""
|
||||
choice = response.choices[0]
|
||||
msg = choice.message
|
||||
msg = getattr(choice, "message", None)
|
||||
# Poolside returns integer finish_reason (e.g. 24) instead of string
|
||||
_fr = choice.finish_reason
|
||||
_fr = getattr(choice, "finish_reason", None)
|
||||
if isinstance(_fr, int):
|
||||
_fr = str(_fr)
|
||||
finish_reason = _fr or "stop"
|
||||
|
||||
tool_calls = None
|
||||
if msg.tool_calls:
|
||||
message_tool_calls = getattr(msg, "tool_calls", None)
|
||||
if message_tool_calls:
|
||||
tool_calls = []
|
||||
for tc in msg.tool_calls:
|
||||
for tc in message_tool_calls:
|
||||
tc_function = getattr(tc, "function", None)
|
||||
function_name = getattr(tc_function, "name", None)
|
||||
# Match Relay's codec: skip absent function/name fields, but
|
||||
# preserve an explicit blank name for Hermes's recovery path.
|
||||
if tc_function is None or function_name is None:
|
||||
continue
|
||||
function_arguments = getattr(tc_function, "arguments", None)
|
||||
# Preserve provider-specific extras on the tool call.
|
||||
# Gemini 3 thinking models attach extra_content with
|
||||
# thought_signature — without replay on the next turn the API
|
||||
@@ -887,9 +895,13 @@ class ChatCompletionsTransport(ProviderTransport):
|
||||
tc_provider_data["extra_content"] = extra
|
||||
tool_calls.append(
|
||||
ToolCall(
|
||||
id=tc.id,
|
||||
name=tc.function.name,
|
||||
arguments=tc.function.arguments,
|
||||
id=getattr(tc, "id", None),
|
||||
name=function_name,
|
||||
arguments=(
|
||||
function_arguments
|
||||
if function_arguments is not None
|
||||
else "{}"
|
||||
),
|
||||
provider_data=tc_provider_data or None,
|
||||
)
|
||||
)
|
||||
@@ -932,7 +944,7 @@ class ChatCompletionsTransport(ProviderTransport):
|
||||
# Promote it to content + a ``content_filter`` finish reason so the
|
||||
# loop's refusal handler surfaces it clearly and stops. ``refusal`` is
|
||||
# ``None`` for normal responses, so this is a no-op in the common case.
|
||||
content = msg.content
|
||||
content = getattr(msg, "content", None)
|
||||
refusal = getattr(msg, "refusal", None)
|
||||
if refusal is None and hasattr(msg, "model_extra"):
|
||||
_msg_extra = getattr(msg, "model_extra", None) or {}
|
||||
|
||||
@@ -18,8 +18,70 @@ def transport():
|
||||
|
||||
|
||||
class TestChatCompletionsBasic:
|
||||
@pytest.mark.parametrize(
|
||||
"choice",
|
||||
[SimpleNamespace(message=SimpleNamespace()), SimpleNamespace()],
|
||||
)
|
||||
def test_normalize_response_allows_missing_optional_message_fields(
|
||||
self, transport, choice
|
||||
):
|
||||
response = SimpleNamespace(choices=[choice], usage=None)
|
||||
|
||||
normalized = transport.normalize_response(response)
|
||||
|
||||
assert normalized.content is None
|
||||
assert normalized.tool_calls is None
|
||||
assert normalized.finish_reason == "stop"
|
||||
|
||||
def test_normalize_response_allows_sparse_tool_call_fields(self, transport):
|
||||
response = SimpleNamespace(
|
||||
choices=[
|
||||
SimpleNamespace(
|
||||
message=SimpleNamespace(
|
||||
tool_calls=[
|
||||
SimpleNamespace(
|
||||
function=SimpleNamespace(arguments='{"city":"Paris"}')
|
||||
),
|
||||
SimpleNamespace(),
|
||||
SimpleNamespace(
|
||||
id="call-3",
|
||||
function=SimpleNamespace(name="lookup"),
|
||||
),
|
||||
SimpleNamespace(
|
||||
id="call-4",
|
||||
function=SimpleNamespace(name="", arguments="{}"),
|
||||
),
|
||||
SimpleNamespace(
|
||||
function=SimpleNamespace(
|
||||
name="weather", arguments="{}"
|
||||
)
|
||||
),
|
||||
]
|
||||
)
|
||||
)
|
||||
],
|
||||
usage=None,
|
||||
)
|
||||
|
||||
normalized = transport.normalize_response(response)
|
||||
|
||||
assert normalized.finish_reason == "stop"
|
||||
assert normalized.tool_calls is not None
|
||||
assert [tool.id for tool in normalized.tool_calls] == [
|
||||
"call-3",
|
||||
"call-4",
|
||||
None,
|
||||
]
|
||||
assert [tool.name for tool in normalized.tool_calls] == [
|
||||
"lookup",
|
||||
"",
|
||||
"weather",
|
||||
]
|
||||
assert [tool.arguments for tool in normalized.tool_calls] == [
|
||||
"{}",
|
||||
"{}",
|
||||
"{}",
|
||||
]
|
||||
|
||||
@pytest.mark.parametrize("provider", ["nous", "openrouter"])
|
||||
def test_gpt56_ultra_uses_max_wire_effort(self, transport, provider):
|
||||
|
||||
@@ -96,6 +96,94 @@ class TestStreamingAccumulator:
|
||||
assert response.usage is not None
|
||||
assert response.usage.completion_tokens == 3
|
||||
|
||||
@patch("run_agent.AIAgent._create_request_openai_client")
|
||||
@patch("run_agent.AIAgent._close_request_openai_client")
|
||||
def test_sparse_delta_allows_missing_optional_fields(self, mock_close, mock_create):
|
||||
"""Managed stream deltas may omit both content and tool_calls."""
|
||||
from run_agent import AIAgent
|
||||
|
||||
sparse_delta = SimpleNamespace(reasoning_content=None, reasoning=None)
|
||||
chunks = [
|
||||
SimpleNamespace(
|
||||
choices=[
|
||||
SimpleNamespace(
|
||||
index=0,
|
||||
delta=sparse_delta,
|
||||
finish_reason=None,
|
||||
)
|
||||
],
|
||||
model="test-model",
|
||||
usage=None,
|
||||
),
|
||||
_make_stream_chunk(
|
||||
content="done", finish_reason="stop", model="test-model"
|
||||
),
|
||||
]
|
||||
mock_client = MagicMock()
|
||||
mock_client.chat.completions.create.return_value = iter(chunks)
|
||||
mock_create.return_value = mock_client
|
||||
agent = AIAgent(
|
||||
api_key="test-key",
|
||||
base_url="https://openrouter.ai/api/v1",
|
||||
model="test/model",
|
||||
quiet_mode=True,
|
||||
skip_context_files=True,
|
||||
skip_memory=True,
|
||||
)
|
||||
agent.api_mode = "chat_completions"
|
||||
agent._interrupt_requested = False
|
||||
|
||||
response = agent._interruptible_streaming_api_call({})
|
||||
|
||||
assert response.choices[0].message.content == "done"
|
||||
assert response.choices[0].message.tool_calls is None
|
||||
|
||||
@patch("run_agent.AIAgent._create_request_openai_client")
|
||||
@patch("run_agent.AIAgent._close_request_openai_client")
|
||||
def test_sparse_tool_delta_allows_missing_nested_fields(
|
||||
self, mock_close, mock_create
|
||||
):
|
||||
"""A partial tool delta may contain arguments before its other fields."""
|
||||
from run_agent import AIAgent
|
||||
|
||||
sparse_tool_delta = SimpleNamespace(
|
||||
index=0,
|
||||
function=SimpleNamespace(arguments='{"city":"Paris"}'),
|
||||
)
|
||||
chunks = [
|
||||
SimpleNamespace(
|
||||
choices=[
|
||||
SimpleNamespace(
|
||||
index=0,
|
||||
delta=SimpleNamespace(tool_calls=[sparse_tool_delta]),
|
||||
)
|
||||
],
|
||||
model="test-model",
|
||||
usage=None,
|
||||
),
|
||||
_make_stream_chunk(finish_reason="tool_calls", model="test-model"),
|
||||
]
|
||||
mock_client = MagicMock()
|
||||
mock_client.chat.completions.create.return_value = iter(chunks)
|
||||
mock_create.return_value = mock_client
|
||||
agent = AIAgent(
|
||||
api_key="test-key",
|
||||
base_url="https://openrouter.ai/api/v1",
|
||||
model="test/model",
|
||||
quiet_mode=True,
|
||||
skip_context_files=True,
|
||||
skip_memory=True,
|
||||
)
|
||||
agent.api_mode = "chat_completions"
|
||||
agent._interrupt_requested = False
|
||||
|
||||
response = agent._interruptible_streaming_api_call({})
|
||||
|
||||
tool_call = response.choices[0].message.tool_calls[0]
|
||||
assert tool_call.function.name == ""
|
||||
assert tool_call.function.arguments == '{"city":"Paris"}'
|
||||
assert response.choices[0].finish_reason == "tool_calls"
|
||||
|
||||
@patch("run_agent.AIAgent._create_request_openai_client")
|
||||
@patch("run_agent.AIAgent._close_request_openai_client")
|
||||
def test_chat_stream_closes_original_provider_resource(
|
||||
@@ -1634,4 +1722,3 @@ class TestBedrockReasoningStaleFloor:
|
||||
from agent.chat_completion_helpers import _bedrock_reasoning_stale_floor
|
||||
|
||||
assert _bedrock_reasoning_stale_floor(model_id) == expected
|
||||
|
||||
|
||||
Reference in New Issue
Block a user