Skip to content

Commit 6e30225

Browse files
authored
fix(streaming): coerce Responses API output field to list (#55) (#57)
When the Codex upstream SSE stream drifts and the ResponsesAccumulator silently drops response.completed / response.created events, the buffer was leaving response_obj["output"] as an empty dict (or missing), which caused every /codex/v1/chat/completions request to fail downstream with "ResponseObject.output Field required". Fixes: - _parse_collected_stream now always coerces non-list output values to [] both before and after accumulator rebuild, so the payload handed to the format chain always satisfies ResponseObject validation. - Silent contextlib.suppress and debug-only except blocks around accumulator.accumulate and rebuild_response_object are upgraded to warning-level logs so upstream SSE drift is visible in production. Adds regression tests covering: - unrecognized response.completed event shape (the production bug) - upstream sending output as a bare dict - happy path where accumulator successfully rebuilds message outputs
1 parent 97945a5 commit 6e30225

2 files changed

Lines changed: 245 additions & 4 deletions

File tree

ccproxy/streaming/buffer.py

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -560,8 +560,16 @@ async def _parse_collected_stream(
560560
continue
561561
event_type = payload.get("type")
562562
if isinstance(event_type, str) and stream_accumulator is not None:
563-
with contextlib.suppress(Exception):
563+
try:
564564
stream_accumulator.accumulate(event_type, payload)
565+
except Exception as exc: # pragma: no cover - defensive logging
566+
logger.warning(
567+
"streaming_buffer_accumulate_failed",
568+
event_type=event_type,
569+
error=str(exc),
570+
request_id=getattr(request_context, "request_id", None),
571+
category="streaming",
572+
)
565573
if event_type == "response.reasoning_summary_part.added":
566574
part = payload.get("part")
567575
if isinstance(part, dict):
@@ -584,7 +592,10 @@ async def _parse_collected_stream(
584592
response_obj.setdefault("created_at", 0)
585593
response_obj.setdefault("status", "completed")
586594
response_obj.setdefault("model", response_obj.get("model") or "")
587-
response_obj.setdefault("output", response_obj.get("output") or {})
595+
# ResponseObject.output must be a list; coerce stray dicts/None to [].
596+
existing_output = response_obj.get("output")
597+
if not isinstance(existing_output, list):
598+
response_obj["output"] = []
588599
response_obj.setdefault(
589600
"parallel_tool_calls", response_obj.get("parallel_tool_calls", False)
590601
)
@@ -602,8 +613,16 @@ async def _parse_collected_stream(
602613
continue
603614
event_type = payload.get("type")
604615
if isinstance(event_type, str):
605-
with contextlib.suppress(Exception):
616+
try:
606617
accumulator_for_rebuild.accumulate(event_type, payload)
618+
except Exception as exc: # pragma: no cover - defensive logging
619+
logger.warning(
620+
"streaming_buffer_rebuild_accumulate_failed",
621+
event_type=event_type,
622+
error=str(exc),
623+
request_id=getattr(request_context, "request_id", None),
624+
category="streaming",
625+
)
607626

608627
if accumulator_for_rebuild is not None:
609628
completed_payload = accumulator_for_rebuild.get_completed_response()
@@ -635,12 +654,21 @@ async def _parse_collected_stream(
635654
request_id=getattr(request_context, "request_id", None),
636655
)
637656
except Exception as exc: # pragma: no cover - defensive logging
638-
logger.debug(
657+
logger.warning(
639658
"response_rebuild_failed",
640659
error=str(exc),
641660
request_id=getattr(request_context, "request_id", None),
661+
category="streaming",
642662
)
643663

664+
# Final safety net: ResponseObject.output is required and must be a
665+
# list. If upstream event schema drift caused the accumulator to drop
666+
# the completed event and rebuild to leave a non-list output behind,
667+
# coerce it so downstream format chain validation doesn't explode
668+
# with a bare "Field required" error.
669+
if not isinstance(response_obj.get("output"), list):
670+
response_obj["output"] = []
671+
644672
if not response_obj.get("usage"):
645673
usage = self._extract_usage_from_chunks(chunks)
646674
if usage:
Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
"""Regression tests for StreamingBufferService._parse_collected_stream.
2+
3+
Covers issue #55: when the Codex Responses API SSE stream has drifted away
4+
from the shape ResponsesAccumulator knows about, the buffer must still emit
5+
a dict whose ``output`` field is a list so the downstream format chain's
6+
ResponseObject validation does not fail with ``Field required``.
7+
"""
8+
9+
from __future__ import annotations
10+
11+
import json
12+
from typing import Any
13+
14+
import httpx
15+
import pytest
16+
17+
from ccproxy.llms.models import openai as openai_models
18+
from ccproxy.llms.streaming.accumulators import ResponsesAccumulator
19+
from ccproxy.streaming.buffer import StreamingBufferService
20+
21+
22+
class _Ctx:
23+
request_id = "test-req"
24+
_tool_accumulator_class = ResponsesAccumulator
25+
26+
27+
def _sse(event_type: str, payload: dict[str, Any]) -> bytes:
28+
body = {"type": event_type, **payload}
29+
return f"event: {event_type}\ndata: {json.dumps(body)}\n\n".encode()
30+
31+
32+
@pytest.fixture
33+
def buffer() -> StreamingBufferService:
34+
return StreamingBufferService(http_client=httpx.AsyncClient())
35+
36+
37+
@pytest.mark.asyncio
38+
async def test_parse_collected_stream_output_is_always_a_list(
39+
buffer: StreamingBufferService,
40+
) -> None:
41+
"""A Codex stream whose completed event is unrecognizable must still
42+
yield a dict with ``output`` as a list (issue #55)."""
43+
44+
base_response = {
45+
"id": "resp_abc",
46+
"object": "response",
47+
"model": "gpt-5-codex",
48+
"parallel_tool_calls": True,
49+
"top_logprobs": 0,
50+
}
51+
chunks = [
52+
_sse(
53+
"response.created",
54+
{"sequence_number": 1, "response": base_response},
55+
),
56+
_sse(
57+
"response.in_progress",
58+
{"sequence_number": 2, "response": base_response},
59+
),
60+
_sse(
61+
"response.completed",
62+
{
63+
"sequence_number": 3,
64+
"response": {**base_response, "unexpected_future_field": True},
65+
},
66+
),
67+
]
68+
69+
parsed = await buffer._parse_collected_stream(
70+
chunks=chunks,
71+
handler_config=None, # type: ignore[arg-type]
72+
request_context=_Ctx(), # type: ignore[arg-type]
73+
)
74+
75+
assert parsed is not None
76+
assert isinstance(parsed.get("output"), list), (
77+
f"output must be list for ResponseObject validation, got {type(parsed.get('output'))}"
78+
)
79+
openai_models.ResponseObject.model_validate(parsed)
80+
81+
82+
@pytest.mark.asyncio
83+
async def test_parse_collected_stream_coerces_non_list_output(
84+
buffer: StreamingBufferService,
85+
) -> None:
86+
"""Even if upstream sends ``output`` as a bare dict, the buffer coerces it."""
87+
88+
base_response = {
89+
"id": "resp_xyz",
90+
"object": "response",
91+
"model": "gpt-5-codex",
92+
"parallel_tool_calls": False,
93+
"output": {},
94+
}
95+
chunks = [
96+
_sse(
97+
"response.created",
98+
{"sequence_number": 1, "response": base_response},
99+
),
100+
]
101+
102+
parsed = await buffer._parse_collected_stream(
103+
chunks=chunks,
104+
handler_config=None, # type: ignore[arg-type]
105+
request_context=_Ctx(), # type: ignore[arg-type]
106+
)
107+
108+
assert parsed is not None
109+
assert isinstance(parsed.get("output"), list)
110+
openai_models.ResponseObject.model_validate(parsed)
111+
112+
113+
@pytest.mark.asyncio
114+
async def test_parse_collected_stream_preserves_rebuilt_output(
115+
buffer: StreamingBufferService,
116+
) -> None:
117+
"""When the accumulator successfully rebuilds message outputs from
118+
valid events, those outputs must reach the parsed payload."""
119+
120+
response_dict: dict[str, Any] = {
121+
"id": "resp_done",
122+
"object": "response",
123+
"model": "gpt-5-codex",
124+
"parallel_tool_calls": False,
125+
"output": [],
126+
}
127+
chunks = [
128+
_sse(
129+
"response.created",
130+
{"sequence_number": 1, "response": response_dict},
131+
),
132+
_sse(
133+
"response.output_item.added",
134+
{
135+
"sequence_number": 2,
136+
"output_index": 0,
137+
"item": {
138+
"type": "message",
139+
"id": "msg_1",
140+
"status": "in_progress",
141+
"role": "assistant",
142+
"content": [],
143+
},
144+
},
145+
),
146+
_sse(
147+
"response.output_text.delta",
148+
{
149+
"sequence_number": 3,
150+
"item_id": "msg_1",
151+
"output_index": 0,
152+
"content_index": 0,
153+
"delta": "hello",
154+
},
155+
),
156+
_sse(
157+
"response.output_text.done",
158+
{
159+
"sequence_number": 4,
160+
"item_id": "msg_1",
161+
"output_index": 0,
162+
"content_index": 0,
163+
"text": "hello",
164+
},
165+
),
166+
_sse(
167+
"response.output_item.done",
168+
{
169+
"sequence_number": 5,
170+
"output_index": 0,
171+
"item": {
172+
"type": "message",
173+
"id": "msg_1",
174+
"status": "completed",
175+
"role": "assistant",
176+
"content": [{"type": "output_text", "text": "hello"}],
177+
},
178+
},
179+
),
180+
_sse(
181+
"response.completed",
182+
{
183+
"sequence_number": 6,
184+
"response": {
185+
**response_dict,
186+
"status": "completed",
187+
"output": [
188+
{
189+
"type": "message",
190+
"id": "msg_1",
191+
"status": "completed",
192+
"role": "assistant",
193+
"content": [{"type": "output_text", "text": "hello"}],
194+
}
195+
],
196+
},
197+
},
198+
),
199+
]
200+
201+
parsed = await buffer._parse_collected_stream(
202+
chunks=chunks,
203+
handler_config=None, # type: ignore[arg-type]
204+
request_context=_Ctx(), # type: ignore[arg-type]
205+
)
206+
207+
assert parsed is not None
208+
output = parsed.get("output")
209+
assert isinstance(output, list) and output, (
210+
"output should contain the rebuilt message"
211+
)
212+
validated = openai_models.ResponseObject.model_validate(parsed)
213+
assert validated.output

0 commit comments

Comments
 (0)