-
Notifications
You must be signed in to change notification settings - Fork 975
feat(providers): add opt-in transient-5xx retry with a shared total-send budget #2981
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f4efa50
af388c1
76e4eca
36440dc
3eb5d76
8101999
1f047c2
2f298d7
9b0abb6
5701e66
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -118,6 +118,7 @@ differing backup and rewrites known legacy namespaced selected ids to bare ids. | |
| | `responsesItemIdRepair?` | `{ message?: string[]; reasoning?: string[]; repairMissingTerminalIds?: boolean; repairInvalidIds?: boolean }` | Disabled-by-default downstream SSE repair for exact placeholder ids, missing terminal ids, and (with `repairInvalidIds`) message/reasoning ids missing the canonical `msg_`/`rs_` prefix. Function-call ids are never rewritten. Built-in DeepSeek enables the last two by default. | | ||
| | `responsesSnapshotRepair?` | `boolean` | Disabled-by-default client-facing repair for sparse Responses lifecycle snapshots in SSE and JSON. Fills missing canonical status, output, and tool metadata while raw inspection and persistence remain unchanged. | | ||
| | `retryOn429?` | `{ enabled?: boolean; attempts?: number; intervalMs?: number; maxIntervalMs?: number; respectRetryAfter?: boolean }` | API-key providers only (`authMode: "key"`). Opt-in same-target 429 retry: when `retryOn429` is absent the feature is off; object presence enables it unless `enabled: false`. On 429 the proxy waits (upstream `Retry-After` or the fixed interval) and replays the identical request on the same key before any key failover — across the main text-turn recovery loop, the Responses passthrough wire, the image/video bridge, the web-search sidecar, and terminal continuations. Only pre-stream HTTP 429 responses are eligible for replay; custom `runTurn` transports are outside the HTTP retry loop. `attempts` counts same-key replays after the first 429 (total sends = `attempts` + 1) and is one request-wide budget shared by the main recovery loop, the terminal-guard continuation, and bridge retries. Exhausting `attempts` only stops further same-key replays: normal key failover or final-error handling then applies per the available targets — on the key-auth passthrough wire there is no failover, so the exhausted 429 surfaces as-is. Codex itself never retries 429, so this is the only defense for single-key providers. Defaults: `enabled: true`, `attempts: 3`, `intervalMs: 5000`, `maxIntervalMs: 60000` (any single wait is capped at `maxIntervalMs`, itself capped at 600000), `respectRetryAfter: true`. | | ||
| | `transientRetryOn5xx?` | `{ enabled?: boolean; attempts?: number }` | Key-auth `openai-chat` providers only. Opt-in retry for pre-stream transient upstream statuses (500, 502, 503, 504, 520, 521, 522): absent means off, object presence enables it unless `enabled: false`. Covers the initial Responses request, the terminal-guard continuation, and native `/v1/chat/completions`. `attempts` is the TOTAL number of upstream sends allowed for one request including the first (1..10, default 3) — it is one budget shared with connection-reset recovery, so `3` means at most three real requests reach the provider. Waits use a fixed 400 ms exponential backoff capped at 5 s and honor `Retry-After`. Separate from `retryOn429`, which handles rate limiting; mid-stream failures are never replayed. | | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -euo pipefail
root="docs-site/src/content/docs"
echo "English entry:"
rg -n 'transientRetryOn5xx|openai-chat|openai-responses|runTurn|reload|restart' \
"$root/reference/configuration/providers.md"
echo "Localized provider pages:"
fd --type f --extension md . "$root" \
| rg '/(ja|ko|ru|zh-cn)/|/(ja|ko|ru|zh-cn)[^/]*\.md$' \
| while IFS= read -r file; do
echo "== $file =="
rg -n 'transientRetryOn5xx|openai-chat|openai-responses|runTurn|reload|restart' "$file" || true
doneRepository: lidge-jun/opencodex Length of output: 50375 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Repository convention and learning scopes:"
find /tmp/coderabbit-repo-knowledge/lidge-jun-opencodex-7afea732 -maxdepth 2 -type f -name '*.md' -print
echo
echo "Retry implementation references:"
rg -n -C 4 'transientRetryOn5xx|retryOn5xx|connection-reset|Retry-After' \
--glob '!docs-site/**' .
echo
echo "English provider table:"
sed -n '112,124p' docs-site/src/content/docs/reference/configuration/providers.md
echo
echo "Localized provider table entries:"
for locale in ja ko ru zh-cn; do
file="docs-site/src/content/docs/$locale/reference/configuration/providers.md"
echo "== $file =="
sed -n '96,110p' "$file"
doneRepository: lidge-jun/opencodex Length of output: 50378 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Exact implementation references:"
rg -n -C 6 'transientRetryOn5xx' src tests docs-site/src/content/docs \
--glob '!**/devlog/**' || true
echo
echo "Provider configuration files:"
for locale in reference ja ko ru zh-cn; do
file="docs-site/src/content/docs/$locale/reference/configuration/providers.md"
echo "== $file =="
rg -n -C 2 'retryOn429|transientRetryOn5xx|reload|restart|runTurn|openai-responses' "$file" || true
done
echo
echo "Applicable repository guidance:"
find /tmp/coderabbit-repo-knowledge/lidge-jun-opencodex-7afea732 \
-maxdepth 2 -type f -name '*.md' -print \
| sort \
| while IFS= read -r file; do
case "$file" in
*/docs-site/*|*/learnings/*|*/architecture/*)
echo "== $file =="
sed -n '1,160p' "$file"
;;
esac
doneRepository: lidge-jun/opencodex Length of output: 50377 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Retry policy scope:"
sed -n '126,151p' src/providers/key-failover.ts
echo
echo "Retry operation and callers:"
rg -n -C 5 'fetchWithTransientRetry|transientRetryPolicyFor|runTurn|fetchResponse' \
src/lib/upstream-retry.ts src/adapters src \
--glob '!src/**/*.map' \
| head -n 220
echo
echo "Configuration reload/restart guidance:"
rg -n -C 3 'reload|restart|configuration changes|config.*change|loadConfig' \
docs-site/src/content/docs/reference/configuration/providers.md \
docs-site/src/content/docs/reference/cli/lifecycle.md \
src \
| head -n 180Repository: lidge-jun/opencodex Length of output: 38587 Clarify the At 🤖 Prompt for AI AgentsSource: Path instructions |
||
| | `autoToolChoiceOnlyModels?` | `string[]` | Models whose `tool_choice` accepts only `auto` or `none`; forced choices are downgraded. | | ||
| | `preserveReasoningContentModels?` | `string[]` | Models requiring prior assistant `reasoning_content` in chat history. | | ||
| | `requiresReasoningPlaceholderModels?` | `string[]` | Models whose upstream rejects a tool_call continuation missing `reasoning_content` (DeepSeek thinking mode); a minimal placeholder is injected when the replay cache misses. Defaults to `preserveReasoningContentModels`; set `[]` to opt out. | | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify the adapter boundary in every translated provider row.
The rows describe “Responses” coverage without stating that these are Responses API requests routed through the eligible OpenAI-compatible
openai-chatadapter. Theopenai-responsesadapter is excluded bysrc/providers/key-failover.tsLines 136-147. Keep native/v1/chat/completionsas a separate path, as implemented insrc/server/chat-native.tsLines 207-241.docs-site/src/content/docs/fr/reference/configuration/providers.md#L117-L117: clarify that “requête Responses initiale” usesopenai-chat, notopenai-responses.docs-site/src/content/docs/ja/reference/configuration/providers.md#L104-L104: clarify that “最初の Responses リクエスト” usesopenai-chat, notopenai-responses.docs-site/src/content/docs/ko/reference/configuration/providers.md#L104-L104: clarify that “최초 Responses 요청” usesopenai-chat, notopenai-responses.docs-site/src/content/docs/zh-tw/reference/configuration/providers.md#L81-L81: clarify that “初始Responses請求” usesopenai-chat, notopenai-responses.As per path instructions, the docs must distinguish
openai-chatfromopenai-responsesand keep translated pages aligned with actual behavior.🧰 Tools
🪛 LanguageTool
[typographical] ~117-~117: Caractère d’apostrophe incorrect.
Context: ...présence d'un objet l'active, sauf avec
enabled: false. Ce comportement couvre la requête Resp...(APOS_INCORRECT)
[typographical] ~117-~117: Caractère d’apostrophe incorrect.
Context: ... un 429 ou à la récupération de compte.
attemptsreprésente le nombre TOTAL d'e...(APOS_INCORRECT)
[typographical] ~117-~117: Caractère d’apostrophe incorrect.
Context: ... 400 ms, plafonnée à 5 s, et respectent
Retry-After. Cette option est distincte de `retryOn...(APOS_INCORRECT)
[typographical] ~117-~117: Caractère d’apostrophe incorrect.
Context: ...y-After
. Cette option est distincte deretryOn429`, qui traite la limitation de débit ; le...(APOS_INCORRECT)
📍 Affects 4 files
docs-site/src/content/docs/fr/reference/configuration/providers.md#L117-L117(this comment)docs-site/src/content/docs/ja/reference/configuration/providers.md#L104-L104docs-site/src/content/docs/ko/reference/configuration/providers.md#L104-L104docs-site/src/content/docs/zh-tw/reference/configuration/providers.md#L81-L81🤖 Prompt for AI Agents
Source: Path instructions