Repository navigation
feat(openai_model): relay streaming Responses calls unchanged - #5457
gwarmstrong wants to merge 7 commits into
Conversation
Add `responses_stream_passthrough` to openai_model. When set, a streaming /v1/responses request is relayed to the upstream Responses endpoint as sent, with `model` replaced by openai_model, and the upstream SSE is streamed back unchanged. Model-call capture still records each exchange. The typed path cannot carry the Codex CLI's own requests for models in its bundled catalog. Codex 0.155.1 sends catalog-model requests with code-mode tools in an `additional_tools` input item, which the streaming sanitizer drops (leaving the model no shell), and `reasoning.effort: "max"`, which the pinned OpenAI SDK rejects with 422. The relay lets such a harness reach an OpenAI-compatible Responses backend while Gym keeps model substitution and capture. An upstream HTTP error before the stream starts goes through the app's ClientResponseError handler, so propagate_upstream_http_status_codes returns the provider's status and body as on the typed path. A relayed stream lasts as long as the model's answer. aiohttp's total timeout covers the body, so the default 30-minute upstream_request_timeout_seconds would cut a longer, still-live stream. For a relayed stream that setting now bounds the wait for the response headers and for each read of the body instead (ClientTimeout sock_read), which still ends a stalled stream. Long agent runs can set it to null to leave every limit to the client. A test against a real upstream checks that a stream longer than the limit completes and a stalled one is cut. Evaluation only: training token capture is rejected, and a multi-attempt upstream_retry_policy or drop_input_reasoning_items cannot be combined with it. Non-streaming requests and the default behavior are unchanged. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
…ent lines The `event:` field is optional in SSE. OpenAI-compatible gateways such as LiteLLM stream Responses events as `data:` lines only and end with `data: [DONE]`, so model-call capture never saw a terminal event and labeled every completed call relayed through such a gateway `stream_truncated`. For the responses dialect, also read the terminal type from the data payload. Tests: a parametrized case feeds data-only completed, incomplete, failed, error and non-terminal events to the parser. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
…l-responses-passthrough Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com> # Conflicts: # responses_api_models/openai_model/app.py
|
/ok to test a06ebba |
svcnvidia-nemo-ci
left a comment
There was a problem hiding this comment.
SHIP WITH CARE — three RISKs and one documentation NOTE.
I inspected all six changed files and traced the affected dispatch, transport, error-handling, concurrency, and capture paths. The findings cover debug-mode error propagation, an ignored jitter setting, and multiline data-only SSE classification. The SSE issue is a remaining parser limitation exposed by the raw relay, not a regression of explicit-event streams.
This was a static review; I didn't execute tests. All acceptance checks below are proposed.
Inline comments (escaped v1):
-
nemo_gym/openai_utils.pyline 349: RISK — Debug logging can replace the provider's HTTP error.With
global_aiohttp_client_request_debug=true, a finite timeout, and a non-retryable upstream error such as HTTP 400, the new streaming method passes aClientTimeoutinsiderequest_kwargsto this diagnostic serializer.json.dumpshas no conversion for that transport object, so it raisesTypeErrorbeforeraise_for_statuspreserves the provider's status/body. Evenpropagate_upstream_http_status_codes: [400]consequently returns a generic 500. Existing typed methods don't put their timeout in this diagnostic dictionary:_requestadds it to its local copy.Could you make optional diagnostics tolerate transport objects? The replacement at this line is:
print(f"Request kwargs: {json.dumps(request_kwargs, default=str)}")The invariant is that logging must not replace the upstream exception or mutate the request. Please verify this suggestion before adopting it.
Acceptance: With debugging enabled and a finite timeout, make
create_response_streamreceive a 400 JSON error and assert it raisesClientResponseErrorwith the original status andresponse_content; through the relay with propagation enabled, expect HTTP 400 and the same JSON body. Repeat with debugging disabled and with a null timeout. Preserve the debug-on/debug-off logging assertion in the existing diagnostic test, selected/unselected status propagation, and the live-stream/stalled-read timeout outcomes. -
responses_api_models/openai_model/app.pyline 402: RISK — The relay silently skips an accepted pre-request jitter setting.With
responses_stream_passthrough: trueandupstream_retry_policy: {max_attempts: 1, pre_request_jitter_seconds: [1, 2]}, streaming calls reach the provider without the configured delay. The field promises a delay before every provider attempt, including the first, and the ordinary one-attempt path applies it before acquiring a slot. Validation accepts this combination, so enabling the relay silently removes configured request spreading.Could you apply the same one-call jitter immediately before
slot = AsyncExitStack()?retry_policy = self.config.upstream_retry_policy if retry_policy.pre_request_jitter_seconds is not None: await asyncio.sleep(random.uniform(*retry_policy.pre_request_jitter_seconds))
Keep the delay outside the provider slot, leave the default
Nonedelay-free, and retain rejection of multi-attempt policies. Don't route this through_call_upstreamunchanged: that would release the slot before streaming finishes. Payload and SSE forwarding must remain unchanged. Please verify the suggested insertion before adopting it.Acceptance: Configure one-attempt
[1, 2]jitter, sample a deterministic1.5, and assert the provider isn't called until the observed async sleep completes; it must then return the original SSE bytes. With jitter omitted, expect no added sleep. Retain the typed-path jitter/backoff assertion and the relay assertion that its slot stays held until the stream ends and is then released. -
nemo_gym/base_responses_api_model.pyline 1223: RISK — Data-only terminal detection still misses multiline event payloads.For example, relay this complete SSE event:
data: {"type":"response.completed", data: "response":{"id":"r","status":"completed","output":[]}}Each individual line fails
json.loads, so this helper returnsNone. However, response reconstruction joins the data lines and reconstructs the completed response. Capture finalization therefore labels the successful exchangestream_truncated, rather than leavingerror_categoryunset. This affects capture classification, not forwarded bytes. It's a remaining pre-existing parser limitation now reachable through the new raw relay.Could you replace the per-line Responses fallback at lines 1219–1230 with event-level parsing matching reconstruction?
data = b"\n".join(line[5:].lstrip() for line in lines if line.startswith(b"data:")) try: payload = json.loads(data) except Exception: continue terminal = isinstance(payload, dict) and terminal_lines.get( b"event: " + str(payload.get("type")).encode() ) if terminal: return terminal
The invariant is that terminal detection and reconstruction interpret the same event payload. Keep explicit
event:detection first and leave Chat/Messages behavior unchanged. Please verify this suggested replacement before adopting it.Acceptance: Feed the example through capture middleware, also split across transport chunks, and expect the reconstructed response plus
error_category is None. Multiline incomplete/failed events should yieldincomplete/upstream_error. Preserve all five single-line mappings in the new test, existing explicit-event and Chat error/[DONE]assertions, and rejection of malformed/nonterminal payloads. -
responses_api_models/openai_model/README.mdline 30: NOTE — Please document the opt-in relay in the published model-server guide too.The README explains the feature, but the OpenAI user guide has no
responses_stream_passthroughentry and describesupstream_request_timeout_secondsexclusively as a total-attempt limit. Users following that configuration reference can't discover the supported route for catalog-model requests or its different timeout contract. The pinned rubric requires Fern user docs for notable user-facing features.Suggested correction: add a short streaming-passthrough section and table entry to that guide, including this minimal override:
policy_model: responses_api_models: openai_model: responses_stream_passthrough: true
State the false default, native-Responses upstream requirement, fixed-model/request-field precedence, evaluation-only restriction, rejected controls, and per-read timeout semantics. Qualify rather than globally replace the existing timeout description: non-streaming requests and passthrough-disabled streaming must retain their typed-path/total-timeout documentation.
Static acceptance: Inspect the guide source/rendering for the option and nested YAML, and confirm it distinguishes relay timeouts from typed-path timeouts and explicitly states that passthrough defaults to false.
Review generated by codex. This was a full review. Request a full review with /review reset=true.
…l-responses-passthrough Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com> # Conflicts: # responses_api_models/openai_model/app.py
|
/ok to test 2f5056c |
What does this PR do?
Gym's typed Responses path cannot carry some harnesses' own requests. The Codex CLI with a model from its bundled catalog sends its code-mode tools in
additional_toolsinput items. The streaming sanitizer keeps only the function tools among them, plus custom tools when #4880's function bridge lowers them for a chat-only backend, which a Responses upstream does not need. Codex also sendsreasoning.effort: "max", which the pinned OpenAI SDK rejects with 422.responses_stream_passthrough(off by default) relays a streaming/v1/responsesrequest to the upstream Responses endpoint as sent, withmodelreplaced byopenai_model, and streams the upstream SSE back unchanged. Model-call capture still records each exchange. For a relayed stream,upstream_request_timeout_secondsbounds the wait for the response headers and for each read of the body rather than the whole stream, so the 1800-second default no longer cuts a long, still-live answer; set it tonullto leave every limit to the client. Evaluation only: training token capture, a multi-attemptupstream_retry_policyanddrop_input_reasoning_itemsare rejected with it.One fix from the same runs: model-call capture read the terminal event type only from
event:lines, so streams from gateways that send data-only Responses SSE (LiteLLM-style) were labeledstream_truncated. It now also reads the type from the data.No issue: an opt-in relay for harnesses the typed path cannot serve, plus a capture fix found while using it.
Usage, for example Codex in Terminal-Bench-Science's task sandbox (its Codex profile,
terminal_bench_science/codex, #5219) against a Responses-native endpoint, with a catalog model at effortmax; confirmreasoning.effortin the captured model calls (++observability_enabled=true ++model_call_capture_dir=/abs/dir):A chat-completions server does not need the relay:
vllm_modelwith #5155's Codex settings serves Codex directly, as Terminal-Bench-Science's DeepSeek run config does.Validation
mainmerged 2026-11-03):openai_model: 143 passed, including a stream longer than the per-read limit that completes and a stalled one that is cut, against a real upstream.tests/unit_tests/rollouts/test_model_calls.py: 12 passed;tests/unit_tests/trajectory/test_model_call_record.py: 32 passed;tests/unit_tests/client/test_openai_client.py: 93 passed;tests/unit_tests/models/test_base_responses_api_model.py: 108 passed.pre-commitclean.main: Codex with GPT-6 Astra through this relay on 10 Terminal-Bench-Science tasks scored 7/10 against stock Harbor's 7/9. Codex with self-hosted DeepSeek V4.1 Flash through this relay in front of a separate convertingvllm_model, 70 tasks × 3 trials, 8 h, scored 31/210 against the public leaderboard's 33/210; that path now usesvllm_modelalone (feat(vllm_model): serve Codex through the Responses-to-chat conversion #5155), which sends the chat server the same requests.Rollouts
Two capped Terminal-Bench-Science trials at a06ebba (
mainmerged 2026-11-02), with main's Terminal-Bench-Science Codex profile (#5219) unchanged apart from the model settings: Codex 0.155.1 in the OpenSandbox task sandbox, catalog modelgpt-6-astraat effortmax, relayed by this passthrough to a Responses-native endpoint serving GPT-6 Astra, with a 30-minute agent cap and model-call capture on. Rows were prepared withgym eval prepare --benchmark terminal_bench_science/codex.neo-orbit-determination: reward 1 (4/4 tests), 37 model calls, about 22 minutes of agent time.eeg-erp-recovery: reward 1 (43/43 tests), 21 model calls, about 14 minutes of agent time.reasoning.effort: maxon every request. Every request carried Codex'sadditional_toolsitem (its three code-mode namespace tools), and the model used them: 56 code-mode tool calls came back. Every stream endedcompletedwith no capture error; the longest call took 204 s.The same two trials at 8cc1cbc, before
mainwas merged in, also scored 1 each (51 calls, all HTTP 200 at effortmax, about $8).Compatibility and benchmark impact
Off by default; non-streaming requests and the typed path are unchanged. Capture now labels data-only SSE streams by their terminal event.