[codex] Add Codex routes and first-token timeout - #56
Conversation
📝 WalkthroughWalkthroughAdds ChangesFirst-token timeout and Codex routing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@provider_adapters/openai_compatible.py`:
- Around line 216-231: The streaming branch in make_async_call_provider leaves
the httpx.AsyncClient created by stream_openai_compatible open when
first_token_timeout_ms is set. Update the streaming path to ensure the
self-created client is managed with async context or explicitly closed after the
await, and adjust stream_openai_compatible so its internal client lifecycle is
always cleaned up even when make_async_call_provider passes client=None.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 26613f57-2d62-43a7-a5eb-9b08f0e54a98
📒 Files selected for processing (10)
config.live.luallm_router_host.pyprovider_adapters/openai_compatible.pyshim.pystreaming.pytests/test_antseed_concurrency.pytests/test_host.pytests/test_live_wiring.pytests/test_shim_max_tokens.pytests/test_streaming.py
…mini fallback; fall-through test Addresses the review (BLOCK on Axis 2). Axis 2 — the cycle. The first-token-timeout reuse made the adapter LEAF import UP into streaming.py (`from streaming import stream_openai_compatible`, function-local), while streaming.py already imported `_prepare_openai_call`/`_classify_from_map` from openai_compatible — a direct two-module cycle, laundered past the import checker by the lazy import, breaking the leaf's "never a host module" invariant. Fix: the openai-compatible STREAM backend belongs in the leaf beside its non-stream sibling (it shares `_prepare_openai_call`), so move `stream_openai_compatible` into `provider_adapters/openai_compatible.py`; `streaming.py` re-exports the name (the existing, allowed direction streaming → provider_adapters). `call()` now uses its module-local sibling — no host import, no cycle. (Verified: streaming.stream_openai_compatible is provider_adapters.openai_compatible.stream_openai_compatible; the leaf imports only common + stdlib again.) Axis 8 — routing home. `gpt-5.4-mini` was served_by ONLY `openai_codex`, so it failed closed (503) when the subscription hit the 429 wall — unlike every other edge family. Add `openai` + `openrouter` fallback rows, matching gpt-5.4. Axis 7 — the fall-through wasn't tested end-to-end. Add test_execute_async_first_token_timeout_falls_through_to_next_candidate: the `timeout` the first-token guard produces triggers the core's retry (config.example.lua: timeout = next_candidate), so a stalled seller falls through. Also pin §3 response-shape parity on the stream-under-hood path (finish_reason + usage, not just text). Full suite 421/0 against the compose Postgres.
21c5eb0 to
0b715f2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@provider_adapters/openai_compatible.py`:
- Around line 328-344: The first-token deadline is only enforced inside the
line-reading loop, so `async with client.stream(...)` in `openai_compatible.py`
can still block during connect/request/headers and bypass
`first_token_timeout_ms`. Update the streaming path to apply the same remaining
budget around stream entry itself in the relevant request/response flow, using
the existing `t0`, `first_token_timeout_s`, and `_first_token_timeout_error()`
logic so the fallback can trigger before any output arrives.
- Around line 389-394: Catch httpx.TimeoutException explicitly in the streaming
path before the blanket except Exception in openai_compatible.py so timeouts are
classified as timeout instead of network_error. Update the stream handling
around the partial-text/error return logic to mirror the non-stream path, using
the same _err helper and preserving partial output behavior for interruptions in
the streaming loop.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: df8f0ec7-6aee-4ce2-9c53-bda9bcfbfa27
📒 Files selected for processing (10)
config.live.luallm_router_host.pyprovider_adapters/openai_compatible.pyshim.pystreaming.pytests/test_antseed_concurrency.pytests/test_host.pytests/test_live_wiring.pytests/test_shim_max_tokens.pytests/test_streaming.py
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/test_live_wiring.py
- tests/test_shim_max_tokens.py
- llm_router_host.py
- tests/test_streaming.py
- shim.py
| try: | ||
| async with client.stream("POST", url, json=body, headers=headers, | ||
| timeout=timeout) as resp: | ||
| if not (200 <= resp.status_code < 300): | ||
| raw = (await resp.aread()).decode("utf-8", "replace")[:500] | ||
| kind = _classify_from_map(raw, rules.get("error_map")) \ | ||
| or _classify_status(resp.status_code, raw) | ||
| return _err(kind, resp.status_code, _latency(), raw) | ||
|
|
||
| lines = resp.aiter_lines().__aiter__() | ||
| while True: | ||
| try: | ||
| if first_token_timeout_s is not None and not saw_output: | ||
| remaining = first_token_timeout_s - (time.monotonic() - t0) | ||
| if remaining <= 0: | ||
| return _first_token_timeout_error() | ||
| line = await asyncio.wait_for(lines.__anext__(), timeout=remaining) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether the pre-output timeout covers stream context entry.
rg -n -C4 'client\.stream|wait_for|first_token_timeout' provider_adapters/openai_compatible.py testsRepository: genlayerlabs/unhardcoded
Length of output: 13798
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the streaming implementation and the relevant tests/helpers.
sed -n '280,380p' provider_adapters/openai_compatible.py
printf '\n--- tests/test_streaming.py ---\n'
sed -n '1,240p' tests/test_streaming.py
printf '\n--- search for stream helpers ---\n'
rg -n -C3 'class SlowFirstLineStreamResponse|class FakeStreamResponse|stream\(' tests provider_adaptersRepository: genlayerlabs/unhardcoded
Length of output: 16649
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect how the HTTP timeout is computed and whether first-token timeout is used elsewhere.
sed -n '1,220p' provider_adapters/openai_compatible.py
printf '\n--- first-token timeout usages ---\n'
rg -n -C3 'first_token_timeout_ms|timeout_s|timeout=' provider_adapters/openai_compatible.py tests | sed -n '1,220p'Repository: genlayerlabs/unhardcoded
Length of output: 24075
Apply the first-token deadline to stream entry
async with client.stream(...) can still block on connect/request/headers before the first wait_for in the loop, so a stalled seller waits for the full HTTP timeout instead of first_token_timeout_ms. Wrap stream setup in the same remaining budget so fallback can happen before any output.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@provider_adapters/openai_compatible.py` around lines 328 - 344, The
first-token deadline is only enforced inside the line-reading loop, so `async
with client.stream(...)` in `openai_compatible.py` can still block during
connect/request/headers and bypass `first_token_timeout_ms`. Update the
streaming path to apply the same remaining budget around stream entry itself in
the relevant request/response flow, using the existing `t0`,
`first_token_timeout_s`, and `_first_token_timeout_error()` logic so the
fallback can trigger before any output arrives.
| except Exception as exc: # noqa: BLE001 — classified below | ||
| partial = "".join(text_parts) | ||
| if emitted: | ||
| return _err("stream_interrupted", 0, _latency(), | ||
| f"{type(exc).__name__}: {exc} (partial: {partial[:200]!r})") | ||
| return _err("network_error", 0, _latency(), f"{type(exc).__name__}: {exc}") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="provider_adapters/openai_compatible.py"
# Show the relevant regions around the cited lines.
nl -ba "$file" | sed -n '220,270p'
printf '\n----\n'
nl -ba "$file" | sed -n '360,410p'Repository: genlayerlabs/unhardcoded
Length of output: 202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="provider_adapters/openai_compatible.py"
awk 'NR>=220 && NR<=270 {printf "%6d %s\n", NR, $0}' "$file"
printf '\n----\n'
awk 'NR>=360 && NR<=410 {printf "%6d %s\n", NR, $0}' "$file"Repository: genlayerlabs/unhardcoded
Length of output: 5609
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="provider_adapters/openai_compatible.py"
# Find the streaming function definition.
awk 'NR>=300 && NR<=340 {printf "%6d %s\n", NR, $0}' "$file"
printf '\n----\n'
# Show every TimeoutException reference in the file.
awk '/TimeoutException/ {printf "%6d %s\n", NR, $0}' "$file"Repository: genlayerlabs/unhardcoded
Length of output: 2127
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="provider_adapters/openai_compatible.py"
awk 'NR>=337 && NR<=395 {printf "%6d %s\n", NR, $0}' "$file"Repository: genlayerlabs/unhardcoded
Length of output: 3494
Catch streaming timeouts explicitly Catch httpx.TimeoutException before the blanket except Exception; otherwise streaming timeouts are returned as network_error instead of timeout, which breaks parity with the non-stream path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@provider_adapters/openai_compatible.py` around lines 389 - 394, Catch
httpx.TimeoutException explicitly in the streaming path before the blanket
except Exception in openai_compatible.py so timeouts are classified as timeout
instead of network_error. Update the stream handling around the
partial-text/error return logic to mirror the non-stream path, using the same
_err helper and preserving partial output behavior for interruptions in the
streaming loop.
|
Reviewed under the repo's architecture doctrine. The feature is real and well-built (a first-token/TTFB bound that fails a stalled marketplace seller fast, reusing 1. Axis 2 — import cycle (the blocker). 2. Axis 8 — routing home. 3. Axis 7 — fall-through untested end-to-end. Added Design note (placement of the knob). We discussed whether Verdict moved from BLOCK to resolved. Ready to merge once approved. |
2 participants
Summary
gpt-5.4andgpt-5.4-minifirst_token_timeout_msand propagate it to provider callsWhy
gpt-5.4andgpt-5.4-miniare accepted by the deployed Codex backend but were not available in the router catalog. AntSeed can also cause long silent stalls before fallback;first_token_timeout_msgives callers a way to fail fast before first output.Validation
.venv/bin/python -m pytest tests/test_streaming.py tests/test_antseed_concurrency.py tests/test_shim_max_tokens.py tests/test_live_wiring.py tests/test_host.py::test_execute_async_threads_first_token_timeout_to_provider_request -q.venv/bin/python -m pytest tests/test_sources.py -k codex -qgit diff --checkSummary by CodeRabbit
first_token_timeout_msoption for both chat and responses requests to fail fast when no output begins in time.gpt-5.4and introducedgpt-5.4-miniwith Codex as an available serving path.