fix: align empty-response retry with current master

This commit is contained in:
Tianyi Cui
2026-07-25 14:39:57 +08:00
parent 4d9cf310f3
commit 9873240390
9 changed files with 24 additions and 26 deletions

View File

@@ -42,7 +42,7 @@ The agent loop keeps `RequestError` as that exact error object and passes `LlmFa
Adapters extract structured facts before falling back to message inspection. They validate HTTP status, parse `Retry-After` seconds or dates into a positive finite millisecond delay, brand the provider request id when exposed, and distinguish their own timeout from the caller's abort. Provider-specific codes and messages may refine a mapping, but no recovery listener parses them.
The initial shared transient-code set is intentionally small: the adapters' existing `RATE_LIMIT` and `SERVER` mappings plus explicit `TIMEOUT` and `TRANSPORT` codes for the two missing remote-failure families. Authentication, quota, invalid request, context overflow, protocol, abort, and unknown failures keep distinct stable codes and are not transient by default. Adding a code requires adapter fixtures and a documented policy decision; it does not require expanding a second failure-class enum. A later decision added `EMPTY_RESPONSE` as a fifth default transient code — a completed provider response with no content blocks, which both adapters now classify as an error finish; see [empty model responses are retryable](../bug-fix/2026-07-24-empty-model-response-is-retryable.md).
The shared transient-code set is intentionally small: adapter mappings for `RATE_LIMIT` and `SERVER`, explicit `TIMEOUT` and `TRANSPORT` codes for remote failures, and `EMPTY_RESPONSE` for a completed provider response with no content blocks. Both adapters classify the last case as an error finish; see [empty model responses are retryable](../bug-fix/2026-07-24-empty-model-response-is-retryable.md). Authentication, quota, invalid request, context overflow, protocol, abort, and unknown failures keep distinct stable codes and are not transient by default. Adding a code requires adapter fixtures and a documented policy decision; it does not require expanding a second failure-class enum.
### Put retry policy on the existing failed-step seam
@@ -62,7 +62,7 @@ interface Config {
}
```
The defaults are two transient retries, a 500 millisecond initial delay, a 10 second delay cap, 10 percent jitter, and the five transient codes above (`RATE_LIMIT`, `SERVER`, `TIMEOUT`, `TRANSPORT`, and the later `EMPTY_RESPONSE`). 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.
The defaults are two transient retries, a 500 millisecond initial delay, a 10 second delay cap, 10 percent jitter, and the five transient codes above (`RATE_LIMIT`, `SERVER`, `TIMEOUT`, `TRANSPORT`, and `EMPTY_RESPONSE`). 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 `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.
@@ -124,7 +124,7 @@ If recovery is exhausted, the final failure is stored once on `turn/end.reason`
- Pure unit tests cover transient-code selection, exponential backoff and jitter bounds, valid and over-cap `Retry-After`, exhausted budgets, deterministic timer/random seams, and abort during backoff.
- Real agent-loop tests cover failure before chunks, partial chunks then failure, thrown and in-band failures, retry to success in a new step, exhaustion to structured `turn/end.reason`, and composition with `dsh-compact-basic` context-overflow recovery.
- The partial-chunk integration test proves failed chunks remain attributed to the failed step, no assistant message or tool side effect is committed for that step, and the successful retry has distinct provenance.
- The plugin-owned `llm/retry` event is non-surface, survives JSONL and SQLite round trips, is ignored by message derivation, and drives TUI retraction plus durable discarded-attempt markers in append-only ACP and stdio streams. Keyless snapshots cover scheduling, cancellation, success, and exhaustion.
- The plugin-owned `llm/retry` event is non-surface, survives JSONL and SQLite round trips, is ignored by message derivation, and drives TUI retraction plus scheduled-retry rendering. Keyless snapshots cover scheduling, cancellation, success, and exhaustion; ACP automation snapshots confirm that a discarded attempt stays off the wire while the recovered reply is emitted.
- Idle-watchdog tests prove the stable signal is rearmed only while `next()` is outstanding, disarmed during consumer think time and in `finally`, and classified separately from a total-call deadline and an earlier caller abort; adapter tests prove the signal stops the underlying request rather than merely detaching it.
- Direct `ctx.llm.stream()` callers remain single-attempt and receive the same structured failure facts.

View File

@@ -2,5 +2,5 @@
# side as of the last confirmed-consistent state. Both languages carry equal authority;
# after editing either side, bring the other along and re-record with:
# pnpm run verify-translation-pairing --write
2026-07-24-empty-model-response-is-retryable.md: 1c9f6092efe7cc6702117c53ea3f1b7f14445100
2026-07-24-empty-model-response-is-retryable.zh.md: 8a124d6ac80c751fc2dbc46f1ed4d50ec5e7348f
2026-07-24-empty-model-response-is-retryable.md: f4a6373178efd5ca1ba5882fb2aaf97dffb2526b
2026-07-24-empty-model-response-is-retryable.zh.md: 4c3afe44140c029d274f34ade97803b958c6d669

View File

@@ -6,7 +6,7 @@ English | [中文](2026-07-24-empty-model-response-is-retryable.zh.md)
## Problem
Providers occasionally return a degenerate completion: a well-formed stream that carries a terminal `stop` finish and zero content blocks — no text, no reasoning, no tool calls. Before this change both adapters mapped it to a successful `{kind: 'stop'}` finish, so the loop logged an empty `assistant/message` and ended the turn as `completed`. Nothing retried, nothing failed loud, and a driver like goal-session counted the silent no-op as a consumed round. A live incident showed an openrouter-served model burning three of six goal rounds on empty completions before the goal blocked on its round limit.
Providers occasionally return a degenerate completion: a well-formed stream that carries a terminal `stop` finish and zero content blocks — no text, no reasoning, no tool calls. If an adapter maps this shape to a successful `{kind: 'stop'}` finish, the loop logs an empty `assistant/message` and ends the turn as `completed`. Retry never runs, no failure reaches the caller, and a driver such as goal-session consumes a round without progress.
## Decision
@@ -19,7 +19,7 @@ An adapter classifies a completed empty response as a provider-boundary failure,
Detection is scoped to `stop` finishes only. `max-tokens` with empty content keeps its existing meaning (pi-ai already normalizes the zero-output overflow case), `tool-calls` cannot be block-empty in practice, and error/aborted finishes already fail.
The classification rides the existing loop machinery — `finishError``agent/request-error``dsh-llm-retry`so no `agent-loop` change was needed, and after the retry budget exhausts, the turn fails loud with `EMPTY_RESPONSE` instead of silently completing empty.
The classification uses the existing loop machinery — `finishError``agent/request-error``dsh-llm-retry`and keeps `agent-loop` provider-neutral. Exhausting the retry budget ends the turn with an explicit `EMPTY_RESPONSE` failure instead of an empty success.
## Alternatives considered
@@ -31,6 +31,6 @@ The classification rides the existing loop machinery — `finishError` → `agen
## Consequences
- A transiently misbehaving provider now costs a bounded retry instead of a silently wasted turn; a persistently empty model surfaces as a loud `EMPTY_RESPONSE` turn failure users can act on.
- A model that genuinely intends to say nothing (rare, but possible after a tool result) is now retried and, if consistently empty, fails the turn. This trade was accepted deliberately: an empty assistant message is indistinguishable from the provider defect and has no value to the user.
- The `empty-response-retry` ACP snapshot (an authored keyless scenario with a deterministic 1 ms zero-jitter retry overlay, `examples/acp-agent/retry.cordis.yml`) pins the product-visible arc: durable `llm/retry` event, the discarded-attempt marker, and a clean completed turn.
- A transiently misbehaving provider consumes a bounded retry instead of a turn with no output; a persistently empty model surfaces an actionable `EMPTY_RESPONSE` turn failure.
- A model that genuinely intends to say nothing (rare, but possible after a tool result) is retried and, if consistently empty, fails the turn. This trade was accepted deliberately: an empty assistant message is indistinguishable from the provider defect and has no value to the user.
- The `empty-response-retry` ACP snapshot (an authored keyless scenario with a deterministic 1 ms zero-jitter retry overlay, `examples/acp-agent/retry.cordis.yml`) pins the product-visible behavior: a durable `llm/retry` event, no ACP output for the discarded attempt, the recovered reply, and a clean completed turn.

View File

@@ -6,7 +6,7 @@ Status: implemented
## Problem
提供方偶尔会返回一种退化的 completion流本身格式完好携带一个终止性的 `stop` 结束,却没有任何内容块——没有文本、没有 reasoning推理、没有工具调用。本次改动前,两个适配器都会把它映射为成功的 `{kind: 'stop'}` 结束,于是主循环记录一条空的 `assistant/message`,并把该轮次以 `completed` 结束。没有任何重试,也没有任何显式失败,而像 goal-session 这样的驱动方会把这次静默的空操作计为一次已消耗的 goal 轮数。一次线上事故显示,某个由 openrouter 提供的模型在触及 goal 的轮数上限而被阻塞前,把六轮 goal 中的三轮消耗在了空 completion 上
提供方偶尔会返回一种退化的 completion流本身格式完好携带一个终止性的 `stop` 结束,却没有任何内容块——没有文本、没有 reasoning推理、没有工具调用。如果适配器把这种形态映射为成功的 `{kind: 'stop'}` 结束,主循环就会记录一条空的 `assistant/message`,并把该轮次以 `completed` 结束。系统不会重试,失败也不会向调用方暴露,而像 goal-session 这样的驱动方会消耗一个轮次,却没有取得任何进展
## Decision
@@ -19,7 +19,7 @@ Status: implemented
检测仅限于 `stop` 结束。内容为空的 `max-tokens` 保持其既有含义pi-ai 已经把零输出的溢出场景归一化处理),`tool-calls` 在实践中不可能是空块,而 erroraborted 结束本身已经算失败。
这套归类沿用既有的主循环机制——`finishError``agent/request-error``dsh-llm-retry`——因此无需改动 `agent-loop`;在重试预算耗尽,该轮次会以 `EMPTY_RESPONSE` 显式失败,而不再静默地以空内容完成
这套归类使用既有的主循环机制——`finishError``agent/request-error``dsh-llm-retry`——并让 `agent-loop` 保持提供方无关。重试预算耗尽,该轮次会以显式的 `EMPTY_RESPONSE` 失败结束,而不是在没有内容的情况下成功结束
## Alternatives considered
@@ -31,6 +31,6 @@ Status: implemented
## Consequences
- 一个偶发异常的提供方现在只会花费一次有界重试,而不是一个被静默浪费的轮次;一个持续返回空内容的模型则会显式暴露为一次用户可据以行动的 `EMPTY_RESPONSE` 轮次失败。
- 一个确实打算什么都不说的模型(罕见,但在一次工具结果之后有可能出现)现在会被重试,若始终为空,则该轮次失败。这个取舍是经过审慎权衡后接受的:一条空的 assistant 消息与提供方缺陷无法区分,且对用户毫无价值。
- `empty-response-retry` ACP 快照(一个人工编写的无密钥场景,配有确定性的 1 ms 零抖动重试 overlay`examples/acp-agent/retry.cordis.yml`)钉住了产品可见的整个过程:持久的 `llm/retry` 事件、被丢弃尝试的标记,以及一次干净的已完成轮次。
- 一个偶发异常的提供方会消耗一次有界重试,而不是一个没有输出的轮次;一个持续返回空内容的模型则会暴露为用户可据以行动的 `EMPTY_RESPONSE` 轮次失败。
- 一个确实打算什么都不说的模型(罕见,但在一次工具结果之后有可能出现)会被重试,若始终为空,则该轮次失败。这个取舍是经过审慎权衡后接受的:一条空的 assistant 消息与提供方缺陷无法区分,且对用户毫无价值。
- `empty-response-retry` ACP 快照(一个人工编写的无密钥场景,配有确定性的 1 ms 零抖动重试 overlay`examples/acp-agent/retry.cordis.yml`)钉住了产品可见的行为:持久的 `llm/retry` 事件、被丢弃尝试不产生任何 ACP 输出、恢复后的回复,以及一次干净的已完成轮次。