Parcourir la source

fix: clarify provider retry delay contract

Tianyi Cui il y a 1 mois
Parent
commit
3293d56a06

+ 2 - 2
.agents/notes/implemented/architecture/2026-06-21-bounded-llm-request-recovery.md

@@ -29,7 +29,7 @@ interface LlmFailure {
   message: string
   code: string
   status?: number
-  retryAfterMs?: number
+  providerRetryAfterMs?: number
   requestId?: ProviderRequestId
 }
 ```
@@ -64,7 +64,7 @@ interface Config {
 
 The defaults are two transient retries, a 500 millisecond initial delay, a 10 second delay cap, 10 percent jitter, and the four transient codes above. The count and delay bounds match the conservative edge of the inspected implementations: [OpenCode uses two request retries with 500 ms/10 s bounds](https://github.com/anomalyco/opencode/blob/9976269ab1accfc9f9dc98a4a688c516934de422/%70ackages/llm/src/route/executor.ts#L36-L39), [Pi separates three agent-level retries from provider retries and defaults provider retries to zero](https://github.com/earendil-works/pi/blob/3da591ab74ab9ab407e72ed882600b2c851fae21/%70ackages/coding-agent/docs/settings.md#L139-L147), and [Codex uses finite request/stream budgets plus a five-minute idle timeout](https://github.com/openai/codex/blob/0fb559f0f6e231a88ac02ea002d3ecd248e2b515/codex-rs/model-provider-info/src/lib.rs#L25-L33). Ten percent follows [Codex's bounded jitter](https://github.com/openai/codex/blob/0fb559f0f6e231a88ac02ea002d3ecd248e2b515/codex-rs/codex-client/src/retry.rs#L40-L47). Two retries mean at most three provider requests when no other recovery policy applies. `maxTransientRetries` is a non-negative integer, delays are positive finite numbers with `initialDelayMs <= maxDelayMs`, `jitterRatio` is in `[0, 1]`, and codes are non-empty and unique. These are Cordis config fields rather than hidden constants so deployments can choose different cost and latency budgets.
 
-For an eligible failure with budget remaining, the one-based transient retry count uses bounded exponential backoff. A valid provider `retryAfterMs` replaces exponential backoff only when it does not exceed `maxDelayMs`; a longer provider delay causes delegation instead of an earlier retry that violates the provider instruction. Local backoff multiplies by an injected random factor in `[1 - jitterRatio, 1 + jitterRatio]` and clamps the final value to `maxDelayMs`; provider delay is not jittered.
+For an eligible failure with budget remaining, the one-based transient retry count uses bounded exponential backoff. A valid `providerRetryAfterMs` replaces exponential backoff only when it does not exceed `maxDelayMs`; a longer provider delay causes delegation instead of an earlier retry that violates the provider instruction. Local backoff multiplies by an injected random factor in `[1 - jitterRatio, 1 + jitterRatio]` and clamps the final value to `maxDelayMs`; provider delay is not jittered.
 
 The plugin owns a lifetime `AbortController` and tracks every active backoff callback. Each wait fuses the waterfall's turn signal with that lifetime signal. Effect cleanup first unregisters the listener, then aborts and awaits the active callbacks; a captured callback whose lifetime signal aborts returns `fail` and can neither retry nor enter the rest of its captured waterfall after disposal. This makes HMR disposal quiescent even though Cordis has already captured the listener.
 

+ 1 - 17
docs/core-data-structures/core.md

@@ -224,23 +224,7 @@ interface GenerateOptions {
 }
 ```
 
-Why a model response stopped is a merge-extensible reason:
-
-```ts type-equiv
-/** Serializable provider-boundary facts; policy decides whether they are retryable. */
-interface LlmFailure {
-  /** Human-readable provider or transport failure. */
-  readonly message: string
-  /** Stable provider-neutral machine-routing code. */
-  readonly code: string
-  /** HTTP status observed at the provider boundary, when available. */
-  readonly status?: number
-  /** Provider-requested delay in milliseconds, when valid and available. */
-  readonly retryAfterMs?: number
-  /** Opaque provider-issued request identifier for diagnostics. */
-  readonly requestId?: ProviderRequestId
-}
-```
+Why a model response stopped is a merge-extensible reason. Terminal provider failures carry the streaming contract's [`LlmFailure`](llm-streaming.md#llmfailure):
 
 ```ts type-equiv
 /**

+ 4 - 2
docs/core-data-structures/llm-streaming.md

@@ -31,7 +31,9 @@ type StreamChunk =
   }
 ```
 
-Every thrown or in-band final-adapter failure normalizes to one serializable provider-neutral payload. `retryAfterMs` is a validated positive delay observed at the provider boundary, not a retry decision; `ProviderRequestId` is an opaque branded string for diagnostics.
+## `LlmFailure`
+
+Every thrown or in-band final-adapter failure normalizes to one serializable provider-neutral payload. `providerRetryAfterMs` is a validated positive delay requested by the provider, not a retry decision; `ProviderRequestId` is an opaque branded string for diagnostics.
 
 ```ts type-equiv
 /** Serializable provider-boundary facts; policy decides whether they are retryable. */
@@ -43,7 +45,7 @@ interface LlmFailure {
   /** HTTP status observed at the provider boundary, when available. */
   readonly status?: number
   /** Provider-requested delay in milliseconds, when valid and available. */
-  readonly retryAfterMs?: number
+  readonly providerRetryAfterMs?: number
   /** Opaque provider-issued request identifier for diagnostics. */
   readonly requestId?: ProviderRequestId
 }

+ 1 - 1
packages/cordis/tool-cordis/src/api-catalog.ts

@@ -1186,7 +1186,7 @@ export const TYPE_API: readonly TypeApiEntry[] = [
   },
   {
     name: 'LlmFailure',
-    declaration: 'export interface LlmFailure {\n    readonly message: string;\n    readonly code: string;\n    readonly status?: number;\n    readonly retryAfterMs?: number;\n    readonly requestId?: ProviderRequestId;\n}',
+    declaration: 'export interface LlmFailure {\n    readonly message: string;\n    readonly code: string;\n    readonly status?: number;\n    readonly providerRetryAfterMs?: number;\n    readonly requestId?: ProviderRequestId;\n}',
   },
   {
     name: 'LlmModelInfo',

+ 3 - 1
packages/core/agent-loop/src/loop.ts

@@ -45,7 +45,9 @@ function finishError(finish: FinishReason): { error: RequestError; failure: LlmF
       const facts = finish.failure
       const error = new LlmError(facts.message, facts.code, {
         ...facts.status === undefined ? {} : { status: facts.status },
-        ...facts.retryAfterMs === undefined ? {} : { retryAfterMs: facts.retryAfterMs },
+        ...facts.providerRetryAfterMs === undefined
+          ? {}
+          : { providerRetryAfterMs: facts.providerRetryAfterMs },
         ...facts.requestId === undefined ? {} : { requestId: facts.requestId },
       })
       return { error, failure: error.failure }

+ 1 - 1
packages/core/agent-loop/tests/contract-regressions.spec.ts

@@ -929,7 +929,7 @@ describe('a finish-error stream chunk ends the turn as error, not completed', ()
       message: 'provider 401',
       code: 'AUTH',
       status: 401,
-      retryAfterMs: 2_000,
+      providerRetryAfterMs: 2_000,
       requestId: ProviderRequestId('finish-request-1'),
     }
     const errorStream: StreamChunk[] = [

+ 2 - 2
packages/core/agent-loop/tests/request-recovery.spec.ts

@@ -422,7 +422,7 @@ describe('agent post-step and request-error lifecycle', () => {
   it('passes structured facts beside the original Error and records them on exhaustion', async () => {
     const original = new LlmError('provider busy', 'RATE_LIMIT', {
       status: 429,
-      retryAfterMs: 2_000,
+      providerRetryAfterMs: 2_000,
       requestId: ProviderRequestId('req-9'),
     })
     Object.freeze(original)
@@ -448,7 +448,7 @@ describe('agent post-step and request-error lifecycle', () => {
       message: 'provider busy',
       code: 'RATE_LIMIT',
       status: 429,
-      retryAfterMs: 2_000,
+      providerRetryAfterMs: 2_000,
       requestId: ProviderRequestId('req-9'),
     })
     expect(seenHistory).toEqual([])

+ 3 - 3
packages/llm/llm-deepseek/src/adapter.ts

@@ -42,7 +42,7 @@ export interface DeepSeekAdapterOptions {
 export const DEFAULT_STREAM_IDLE_TIMEOUT_MS = 300_000
 const STREAM_IDLE_TIMEOUT_CODE = 'LLM_STREAM_IDLE_TIMEOUT'
 
-function retryAfterMs(value: string | null): number | undefined {
+function providerRetryAfterMs(value: string | null): number | undefined {
   if (value === null) return undefined
   if (/^\d+$/.test(value)) {
     const delay = Number(value) * 1_000
@@ -188,11 +188,11 @@ export class DeepSeekAdapter extends LlmAdapter {
         // Only swallow error-body parsing: the HTTP status still identifies the
         // failure, so malformed gateway JSON must not mask it.
       }
-      const delay = retryAfterMs(response.headers.get('retry-after'))
+      const delay = providerRetryAfterMs(response.headers.get('retry-after'))
       const id = requestId(response.headers)
       throw new LlmError(message, httpErrorCode(response.status, providerError), {
         status: response.status,
-        ...delay === undefined ? {} : { retryAfterMs: delay },
+        ...delay === undefined ? {} : { providerRetryAfterMs: delay },
         ...id === undefined ? {} : { requestId: id },
       })
     }

+ 2 - 2
packages/llm/llm-deepseek/tests/adapter.spec.ts

@@ -232,7 +232,7 @@ describe('DeepSeekAdapter against a mock server', () => {
       message: 'slow down',
       code: 'RATE_LIMIT',
       status: 429,
-      retryAfterMs: 2_000,
+      providerRetryAfterMs: 2_000,
       requestId: ProviderRequestId('req-429'),
     })
   })
@@ -257,7 +257,7 @@ describe('DeepSeekAdapter against a mock server', () => {
             message: 'come back later',
             code: 'SERVER',
             status: 503,
-            retryAfterMs: 3_000,
+            providerRetryAfterMs: 3_000,
             requestId: ProviderRequestId('deepseek-503'),
           },
         })

+ 1 - 1
packages/llm/llm-retry/README.md

@@ -2,7 +2,7 @@
 
 Function plugin that retries selected transient model-request failures on the agent loop's closed-step recovery seam. It does not wrap `ctx.llm.stream()`: every adapter call remains one provider attempt, and every retry opens a fresh numbered step.
 
-The default policy permits two retries for `RATE_LIMIT`, `SERVER`, `TIMEOUT`, and `TRANSPORT`, using bounded exponential backoff from 500 ms to 10 seconds with 10 percent jitter. Delay bounds must fit Node's supported timer range. A valid provider `retryAfterMs` replaces local backoff when it is within the configured cap; an over-cap instruction delegates to the next recovery policy instead.
+The default policy permits two retries for `RATE_LIMIT`, `SERVER`, `TIMEOUT`, and `TRANSPORT`, using bounded exponential backoff from 500 ms to 10 seconds with 10 percent jitter. Delay bounds must fit Node's supported timer range. A valid `providerRetryAfterMs` replaces local backoff when it is within the configured cap; an over-cap instruction delegates to the next recovery policy instead.
 
 Before waiting, the plugin appends a non-surface `llm/retry` event with the failure and scheduled delay. Cancellation and plugin disposal abort the wait; disposal drains the plugin's active backoffs, and a callback captured before disposal fails closed if invoked afterward.
 

+ 5 - 3
packages/llm/llm-retry/src/index.ts

@@ -190,9 +190,11 @@ export function apply(ctx: Context, config: Config = {}, internals: RetryInterna
 
     const retry = priorTransientFailures + 1
     let delayMs: number
-    if (failure.retryAfterMs !== undefined && Number.isFinite(failure.retryAfterMs) && failure.retryAfterMs > 0) {
-      if (failure.retryAfterMs > resolved.maxDelayMs) return next()
-      delayMs = failure.retryAfterMs
+    if (failure.providerRetryAfterMs !== undefined
+      && Number.isFinite(failure.providerRetryAfterMs)
+      && failure.providerRetryAfterMs > 0) {
+      if (failure.providerRetryAfterMs > resolved.maxDelayMs) return next()
+      delayMs = failure.providerRetryAfterMs
     } else {
       delayMs = localDelay(resolved, retry, random)
     }

+ 2 - 2
packages/llm/llm-retry/tests/retry.spec.ts

@@ -235,7 +235,7 @@ describe('bounded transient retry policy', () => {
   it('uses a bounded provider Retry-After verbatim and delegates an over-cap instruction', async () => {
     vi.useFakeTimers()
     const accepted = new ScriptedAdapter([
-      new LlmError('wait', 'RATE_LIMIT', { retryAfterMs: 2_000 }),
+      new LlmError('wait', 'RATE_LIMIT', { providerRetryAfterMs: 2_000 }),
       textResponse('done'),
     ])
     ;({ ctx: context } = await harness(accepted, { jitterRatio: 1 }))
@@ -250,7 +250,7 @@ describe('bounded transient retry policy', () => {
 
     await context.fiber.dispose()
     const rejected = new ScriptedAdapter([
-      new LlmError('wait too long', 'RATE_LIMIT', { retryAfterMs: 10_001 }),
+      new LlmError('wait too long', 'RATE_LIMIT', { providerRetryAfterMs: 10_001 }),
     ])
     ;({ ctx: context } = await harness(rejected))
     const rejectedAgent = context.agentLoop.create(SessionId('retry-after-rejected'), { provider: 'mock', model: 'mock' })

+ 4 - 3
packages/llm/llm/src/adapter-failure.ts

@@ -76,18 +76,19 @@ function failureSnapshot(value: unknown): LlmFailure | undefined {
     const message = candidate.message
     const code = candidate.code
     const status = candidate.status
-    const retryAfterMs = candidate.retryAfterMs
+    const providerRetryAfterMs = candidate.providerRetryAfterMs
     const requestId = candidate.requestId
     if (typeof message !== 'string' || message.length === 0
       || typeof code !== 'string' || code.length === 0
       || (status !== undefined && (!Number.isInteger(status) || status < 100 || status > 599))
-      || (retryAfterMs !== undefined && (!Number.isFinite(retryAfterMs) || retryAfterMs <= 0))
+      || (providerRetryAfterMs !== undefined
+        && (!Number.isFinite(providerRetryAfterMs) || providerRetryAfterMs <= 0))
       || (requestId !== undefined && (typeof requestId !== 'string' || requestId.length === 0))) return undefined
     return Object.freeze({
       message,
       code,
       ...status === undefined ? {} : { status },
-      ...retryAfterMs === undefined ? {} : { retryAfterMs },
+      ...providerRetryAfterMs === undefined ? {} : { providerRetryAfterMs },
       ...requestId === undefined ? {} : { requestId },
     })
   } catch (_sdkFailureGetter) {

+ 5 - 5
packages/llm/llm/src/index.ts

@@ -50,7 +50,7 @@ export interface LlmErrorOptions extends ErrorOptions {
   /** Valid HTTP status observed at the provider boundary. */
   status?: number
   /** Positive finite provider-requested delay in milliseconds. */
-  retryAfterMs?: number
+  providerRetryAfterMs?: number
   /** Non-empty opaque provider request id. */
   requestId?: ProviderRequestId
 }
@@ -75,9 +75,9 @@ export class LlmError extends HarnessError {
       && (!Number.isInteger(options.status) || options.status < 100 || options.status > 599)) {
       throw new Error('LlmError status must be an integer from 100 through 599')
     }
-    if (options?.retryAfterMs !== undefined
-      && (!Number.isFinite(options.retryAfterMs) || options.retryAfterMs <= 0)) {
-      throw new Error('LlmError retryAfterMs must be a positive finite number')
+    if (options?.providerRetryAfterMs !== undefined
+      && (!Number.isFinite(options.providerRetryAfterMs) || options.providerRetryAfterMs <= 0)) {
+      throw new Error('LlmError providerRetryAfterMs must be a positive finite number')
     }
     if (options?.requestId !== undefined
       && (typeof options.requestId !== 'string' || options.requestId.length === 0)) {
@@ -89,7 +89,7 @@ export class LlmError extends HarnessError {
       message,
       code,
       ...options?.status === undefined ? {} : { status: options.status },
-      ...options?.retryAfterMs === undefined ? {} : { retryAfterMs: options.retryAfterMs },
+      ...options?.providerRetryAfterMs === undefined ? {} : { providerRetryAfterMs: options.providerRetryAfterMs },
       ...options?.requestId === undefined ? {} : { requestId: options.requestId },
     })
   }

+ 1 - 1
packages/llm/llm/src/types.ts

@@ -16,7 +16,7 @@ export interface LlmFailure {
   /** HTTP status observed at the provider boundary, when available. */
   readonly status?: number
   /** Provider-requested delay in milliseconds, when valid and available. */
-  readonly retryAfterMs?: number
+  readonly providerRetryAfterMs?: number
   /** Opaque provider-issued request identifier for diagnostics. */
   readonly requestId?: ProviderRequestId
 }

+ 4 - 3
packages/llm/llm/tests/service.spec.ts

@@ -191,7 +191,7 @@ describe('LlmService', () => {
   it('keeps structured provider facts beside a frozen third-party Error', async () => {
     const original = new LlmError('provider busy', 'RATE_LIMIT', {
       status: 429,
-      retryAfterMs: 1_500,
+      providerRetryAfterMs: 1_500,
       requestId: ProviderRequestId('req-7'),
     })
     Object.freeze(original)
@@ -212,7 +212,7 @@ describe('LlmService', () => {
       message: 'provider busy',
       code: 'RATE_LIMIT',
       status: 429,
-      retryAfterMs: 1_500,
+      providerRetryAfterMs: 1_500,
       requestId: ProviderRequestId('req-7'),
     })
   })
@@ -747,7 +747,8 @@ describe('LlmService', () => {
 
   it('rejects non-serializable structured failure facts at construction', () => {
     expect(() => new LlmError('busy', 'RATE_LIMIT', { status: 42 })).toThrow(/status/)
-    expect(() => new LlmError('busy', 'RATE_LIMIT', { retryAfterMs: Number.NaN })).toThrow(/retryAfterMs/)
+    expect(() => new LlmError('busy', 'RATE_LIMIT', { providerRetryAfterMs: Number.NaN }))
+      .toThrow(/providerRetryAfterMs/)
     expect(() => new LlmError('busy', 'RATE_LIMIT', { requestId: ProviderRequestId('') })).toThrow(/requestId/)
     expect(() => new LlmError(1 as never, 'RATE_LIMIT')).toThrow(/message/)
     expect(() => new LlmError('busy', 1 as never)).toThrow(/code/)

+ 0 - 1
scripts/type-equiv.manifest.json

@@ -6,7 +6,6 @@
     { "doc": "docs/core-data-structures/core.md", "symbol": "AssistantProvenance", "source": "packages/llm/llm/src/types.ts" },
     { "doc": "docs/core-data-structures/core.md", "symbol": "Message", "source": "packages/llm/llm/src/types.ts" },
     { "doc": "docs/core-data-structures/core.md", "symbol": "MessageSourceMap", "source": "packages/llm/llm/src/types.ts" },
-    { "doc": "docs/core-data-structures/core.md", "symbol": "LlmFailure", "source": "packages/llm/llm/src/types.ts" },
     { "doc": "docs/core-data-structures/core.md", "symbol": "FinishReasonMap", "source": "packages/llm/llm/src/types.ts" },
     { "doc": "docs/core-data-structures/core.md", "symbol": "LlmProviderInfo", "source": "packages/llm/llm/src/types.ts" },
     { "doc": "docs/core-data-structures/core.md", "symbol": "LlmModelInfo", "source": "packages/llm/llm/src/types.ts" },