docs: narrow LLM request recovery proposal
This commit is contained in:
Tianyi Cui
2 files changed
+147
-219
No files matched your search
@@ -0,0 +1,147 @@
|
||||
# Agent Note: Bounded recovery for transient LLM request failures
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
`dsh-llm` can report provider failures either by throwing during adapter dispatch or iteration or by ending with `finish { kind: 'error' | 'aborted' }`. The final adapter boundary tags thrown failures so `dsh-agent-loop` can distinguish them from middleware and result-processing defects, and the loop normalizes both delivery forms into `agent/request-error` after closing the failed step. The default decision is `fail`; `dsh-compact-basic` is the only shipped recovery listener, and it retries a canonical context-window overflow only after compaction proves that the durable surface shrank.
|
||||
|
||||
That boundary is already safe for another request attempt. Raw `assistant/chunk` events carry the failed `turn` and `step`, message derivation ignores them unless a successful `assistant/message` cites them, tool calls are dispatched only after a successful terminal finish and assembly, and a retry opens a new numbered step from the durable log. The harness therefore does not need a second response lifecycle or tentative-output protocol to keep two attempts separate.
|
||||
|
||||
Three narrower gaps remain.
|
||||
|
||||
- Provider failures retain only a message and usually a code. HTTP status, retry delay, and provider request id are discarded or recoverable only through provider-specific error objects, so generic recovery cannot make or explain a decision without parsing text.
|
||||
- Retry ownership differs by adapter. The hand-written DeepSeek adapter makes one attempt, while pi-ai profiles can enable opaque library retries. Combining hidden transport retries with a future `agent/request-error` listener would multiply attempts and omit intermediate failures from the session log.
|
||||
- A recovered failure has no durable status fact. The failed step and chunks remain reconstructable, but an observer cannot tell whether the agent is deliberately backing off, for how long, or why. A long silent wait looks like a stalled loop.
|
||||
|
||||
The goal is bounded recovery from transient failures of the same explicit provider/model request. Provider or model failover, response splicing, and semantic output repair are different problems and have no current consumer.
|
||||
|
||||
## Proposal
|
||||
|
||||
### Preserve failure facts without embedding policy
|
||||
|
||||
Add one JSON-serializable `LlmFailure` payload to `@deepseek-ai/dsh-llm`:
|
||||
|
||||
```ts ignore-check
|
||||
type ProviderRequestId = Branded<'ProviderRequestId'>
|
||||
|
||||
interface LlmFailure {
|
||||
message: string
|
||||
code: string
|
||||
status?: number
|
||||
retryAfterMs?: number
|
||||
requestId?: ProviderRequestId
|
||||
}
|
||||
```
|
||||
|
||||
`code` remains the provider-neutral machine-routing taxonomy established by `HarnessError`; the new fields are observations from the provider boundary. `ProviderRequestId` is owned and constructed by `dsh-llm`, then serializes as its provider-issued string. The payload deliberately has no `retryable`, `failover`, `partialOutput`, provider, model, phase, or route id fields. Retryability belongs to policy, provider/model are already in the durable request header, and partial output is derived from the failed step's `assistant/chunk` events.
|
||||
|
||||
`LlmError` carries `failure: LlmFailure` and preserves `failure.code === error.code`. `FinishReasonMap.error` and `FinishReasonMap.aborted` carry the same payload instead of parallel failure shapes. An adapter-thrown `Error` keeps its exact object identity: the final-adapter scope associates the normalized facts with that object in call-local sidecar state and rethrows it unchanged; a non-`Error` throw is wrapped as today. `llmFailureOf(stream, error)` retrieves those facts alongside the existing provenance check, while an in-band finish without an error object becomes a new `LlmError`. This preserves listeners that key on error type or identity while giving all final-adapter failures, including unknown SDK exceptions, an `UNKNOWN` terminal payload.
|
||||
|
||||
The agent loop keeps `RequestError` as that exact error object and passes `LlmFailure` as a separate argument to `agent/request-error`; it does not mutate possibly frozen third-party errors. It also uses the payload when converting an in-band finish and when recording an unrecovered `turn/end.reason`.
|
||||
|
||||
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.
|
||||
|
||||
### Put retry policy on the existing failed-step seam
|
||||
|
||||
Add a function plugin, `@deepseek-ai/dsh-llm-retry`, that listens to `agent/request-error`. It introduces no service or new loop branch; the agent-loop package changes only the data carried through its existing failed-step recovery control flow.
|
||||
|
||||
Replace the scalar `retryAttempt` argument with the current `LlmFailure` and an immutable list of prior failures that led to another request attempt in this consecutive recovery sequence. `dsh-llm-retry` counts only prior failures whose codes are in its configured transient set, while `dsh-compact-basic` counts only prior context-overflow failures. A successful model request clears the history as it clears the current scalar. Alternating transient and context-overflow failures therefore consume their owning policy budgets independently; the maximum request count is one plus the sum of the finite budgets of the loaded recovery policies.
|
||||
|
||||
The plugin resolves and validates this deployment configuration at load:
|
||||
|
||||
```ts ignore-check
|
||||
interface Config {
|
||||
maxTransientRetries?: number
|
||||
initialDelayMs?: number
|
||||
maxDelayMs?: number
|
||||
jitterRatio?: number
|
||||
retryableCodes?: string[]
|
||||
}
|
||||
```
|
||||
|
||||
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.
|
||||
|
||||
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.
|
||||
|
||||
Before sleeping, `dsh-llm-retry` appends one non-surface `llm/retry` session event containing the turn, failed step, one-based transient retry number, configured maximum, scheduled delay, and `LlmFailure`. The plugin owns the `SessionEventMap` augmentation; `dsh-session` remains generic persistence and does not absorb the optional policy's vocabulary. The event says what was scheduled, not that the next request completed; cancellation during the delay is subsequently visible on `turn/end`. The event ships only with a production renderer and replay/snapshot coverage, because its purpose is operational state rather than trace collection.
|
||||
|
||||
The listener calls `next()` for a non-transient code, an exhausted policy budget, or an over-cap provider delay. This preserves composition with context-overflow recovery and later policy plugins. It returns `{ action: 'retry' }` only after the delay completes under both signals; turn cancellation and plugin disposal return `fail`, after which the loop's cancellation/disposal checks remain authoritative.
|
||||
|
||||
The agent-spine demo bundle loads the plugin so the shared stdio/TUI, one-shot CLI, and ACP example compositions use the same bounded policy. Library consumers retain explicit plugin composition: omitting the plugin leaves `agent/request-error` at its current fail default.
|
||||
|
||||
### Make one layer own visible attempts
|
||||
|
||||
Adapters perform one provider request per `stream()` call. The pi-ai adapter removes public `maxRetries` and `maxRetryDelayMs` profile fields and disables library retries; the hand-written adapter keeps its current single-attempt behavior. This prevents an SDK budget from multiplying the agent budget and ensures every transient retry is represented by a closed failed step plus `llm/retry`.
|
||||
|
||||
`ctx.llm.stream()` remains the raw one-attempt waterfall. Direct callers such as compaction summarization receive the structured failure but do not gain automatic retry, because they have no agent step boundary or general durable place to separate attempts. A future direct-call consumer may justify a buffering helper that retries only before emitting a chunk, but this proposal does not add one speculatively.
|
||||
|
||||
### Bound stalled streams where they can be stopped
|
||||
|
||||
Each adapter exposes a validated `streamIdleTimeoutMs` configuration field with the five-minute prior-art default cited above. The interval covers each outstanding iterator `next()` from demand to the next valid `StreamChunk`; time a consumer spends between `next()` calls is not provider idle time.
|
||||
|
||||
Extend `@deepseek-ai/dsh-timeout` with a rearmable idle-watchdog primitive. One stable local `AbortController` is fused with the caller signal and passed to the transport for the whole adapter call; each outstanding `next()` arms the watchdog, resolution disarms it, and the next demand rearms it. Timeout aborts that stable controller with a capability-owned `TimeoutReason`, and `finally` clears the timer. The adapter classifies its watchdog as `TIMEOUT` and an earlier upstream abort as `ABORTED`. The existing one-shot `deadline()` is not presented as a sliding timer.
|
||||
|
||||
The two adapters must prove termination at their actual boundaries. The hand-written adapter aborts its fetch/reader, and the pi-ai adapter maps the stable signal through the SDK only after a test proves the SDK stops the request. A timer that merely rejects a consumer promise while leaving the request running does not satisfy the contract.
|
||||
|
||||
### Keep attempts separate in the existing log
|
||||
|
||||
A failed attempt may leave `assistant/chunk` events in its closed step, but it never appends `assistant/message` and never dispatches a tool. A retry opens the next numbered step, reconstructs the request from the durable surface, and produces its own chunks. UIs may render live chunks while a step is open, then mark or clear that transient view when `llm/retry` identifies the failed step or `turn/end` records terminal failure; message derivation continues to ignore the failed chunks.
|
||||
|
||||
If recovery is exhausted, the final failure is stored once on `turn/end.reason` with the structured facts. If transient recovery continues, `llm/retry` is the durable home for that attempt's failure and delay. No standalone final-error event or response-id vocabulary is added.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Automatic provider or model failover. Requests already select one explicit provider and model, and the provider registry deliberately has one adapter owner per provider.
|
||||
- Retrying or continuing after a successful terminal finish, or splicing chunks from two attempts into one assistant message.
|
||||
- Repairing malformed tool arguments, refusals, content filters, or other semantic model output.
|
||||
- Unbounded retries, unattended retry-until-cancelled behavior, circuit breakers, shared provider health, or cross-agent retry budgets.
|
||||
- Changing `llm/stream` into a response lifecycle or adding convenience generation APIs without a production consumer.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **Retry inside `llm/stream` or the provider SDK** — rejected because a raw stream has no durable attempt boundary after emitting chunks, hidden SDK retries multiply budgets, and neither path can record each failed attempt consistently.
|
||||
- **Add response start, interrupted, discarded, failed, and committed events to `dsh-llm`** — rejected because the agent log already separates raw chunks, successful messages, and numbered attempts. A second state machine would duplicate ownership without enabling the bounded same-route retry.
|
||||
- **Add logical routes, capability matrices, and failover selection** — rejected because current requests already name provider and model explicitly, one adapter owns each provider, and no current consumer requires automatic fallback or can prove semantic compatibility.
|
||||
- **Put `retryable` or `failover` on `LlmFailure`** — rejected because adapters report facts while deployment policy decides action. The same 429 may be retried in an interactive bundle and rejected in a cost-capped batch.
|
||||
- **Retry forever while the caller remains active** — rejected because it gives one request unbounded cost and latency. Visible status makes bounded waiting understandable; it does not make an unlimited budget safe.
|
||||
- **Log retry status only through the process logger** — rejected because process logs do not reconstruct session behavior and cannot drive replayed UI state.
|
||||
- **Keep only flat codes** — rejected because retry delay and provider request id are structured provider facts, and HTTP status is necessary for diagnosis when different wire failures share one stable code.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- `LlmFailure` is the single serializable payload for thrown, error-finish, and aborted-finish final-adapter failures; normalization preserves stable code, status, retry delay, branded provider request id, error cause, and caller-abort versus adapter-timeout classification where available.
|
||||
- An adapter-thrown `Error` reaches `agent/request-error` as the exact same object while its sidecar `LlmFailure` reaches the adjacent argument; tests retain the existing identity assertion for extensible and frozen third-party errors.
|
||||
- DeepSeek and pi-ai adapter tests cover representative 400, 401/403, 429, 5xx, connection, malformed/truncated stream, timeout, abort, retry-after seconds/date, request-id, and unknown-SDK-error paths without recovery policy parsing message text.
|
||||
- Pi-ai performs one wire attempt per adapter call, and a wire-level test rejects any regression that silently restores SDK retries.
|
||||
- `agent/request-error` carries current failure facts plus immutable prior-retried failure facts; a success clears that history, and alternating transient/context-overflow integration tests prove the two policies consume only their own finite budgets.
|
||||
- `dsh-llm-retry` validates every config field at Loader startup, delegates all ineligible paths with `next()`, and makes at most `maxTransientRetries + 1` provider requests when no other policy applies.
|
||||
- HMR-during-backoff tests prove disposal unregisters the listener, aborts and awaits its captured callbacks, emits no retry decision after disposal, and leaves no timer or promise alive.
|
||||
- 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 has a production UI consumer with keyless ACP or TUI snapshot coverage for scheduled retry, cancellation during delay, and eventual success or exhaustion.
|
||||
- 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.
|
||||
- The architecture LLM section, the implemented request-recovery and timeout notes, agent-loop and adapter READMEs, package catalogs, example configuration, persistence catalog, and testing documentation are updated in the implementation change; all generated outputs and bilingual counterparts required by those files are refreshed together.
|
||||
|
||||
## Risks
|
||||
|
||||
- A retry can duplicate provider billing even when no chunk arrived; the finite attempt budget limits but cannot remove that risk.
|
||||
- Provider SDKs may hide status or retry headers. Those adapters must use `UNKNOWN` or a stable coarse code rather than infer policy from fragile text.
|
||||
- Durable retry events expand the session protocol and UI state machine. Shipping the event and its consumer together prevents an unused telemetry vocabulary, but later schema changes still require persistence and replay work.
|
||||
- Clearing a failed step's live chunks can visibly retract output. That is preferable to presenting discarded text or partial tool JSON as committed history, and snapshots must make the transition explicit.
|
||||
- Adapter-local timeout enforcement can drift across transport libraries. Contract tests at the termination boundary are required for both implementations.
|
||||
- Multiple recovery plugins add their finite budgets. Their classifiers should remain disjoint; an overlapping classifier is registration-order policy and must be documented and tested by the plugins that introduce it.
|
||||
|
||||
## Related
|
||||
|
||||
- [Structured error taxonomy](../../implemented/architecture/2026-06-11-structured-error-taxonomy.md) owns stable machine-routable codes and cause chaining.
|
||||
- [Reconstructable requests](../../implemented/architecture/2026-07-05-reconstructable-requests.md) makes provider/model and complete request inputs durable before dispatch.
|
||||
- [Timeout deadline library](../../implemented/architecture/2026-07-06-timeout-deadline-library.md) separates shared deadline classification from capability-owned termination.
|
||||
- [After-call compaction pressure and context-overflow recovery](../../implemented/architecture/2026-07-10-after-call-compaction-pressure-and-overflow-recovery.md) owns the current closed-step request-recovery seam and bounded overflow retry.
|
||||
- [Provider-routed LLM adapters](../../implemented/architecture/2026-07-14-provider-routed-llm-adapters.md) owns explicit provider/model routing and the one-adapter-per-provider invariant.
|
||||
@@ -1,219 +0,0 @@
|
||||
# Agent Note: Treat unstable LLM APIs as a first-class failure mode
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
LLM APIs are not a stable local function call. They rate-limit, overload, return 5xx/502/503 from gateways, close streaming sockets before `[DONE]`, emit malformed or provider-specific error payloads, hang mid-stream, surface SDK errors as in-band events, and sometimes require a delayed retry using `Retry-After`. The harness currently contains good error containment, but it does not yet treat this API instability as a first-class design problem.
|
||||
|
||||
`dsh-llm` defines two sanctioned adapter failure paths - throw from `stream()` or end with `finish { kind: 'error' | 'aborted' }` - and downstream consumers are expected to treat both as failed model calls. That was the right MVP containment baseline, documented in [the architecture](../../../../docs/architecture.md) and reinforced by [the twin-adapter RFC](../../implemented/architecture/2026-06-13-twin-llm-adapters.md). It is not enough for callers that depend on an unstable remote model API for every turn, and it forces every caller to understand two failure delivery mechanisms.
|
||||
|
||||
The audit found three load-bearing gaps.
|
||||
|
||||
- `LlmError` and `FinishReasonMap.error` carry only `message`, `code`, and sometimes HTTP `status` ([packages/llm/llm/src/index.ts](../../../../packages/llm/llm/src/index.ts), [packages/llm/llm/src/types.ts](../../../../packages/llm/llm/src/types.ts)). A caller cannot reliably tell "retry this after 800 ms on the same endpoint", "fail over to another route for the same model", "ask the user for credentials", "never retry because the request is invalid", "provider truncated the stream after committed output", or "adapter protocol bug" without provider-specific heuristics.
|
||||
- The `llm/stream` waterfall is documented as the place for retry/routing/caching, but its value is one raw `AsyncIterable<StreamChunk>`. A listener can technically catch an API error and call `next()` again, but once it has yielded chunks, callers may already have rendered them, buffered them as output, or executed side effects based on completed tool calls. Retrying after that point can concatenate chunks from two provider responses into one apparent model output. The current surface has no canonical way to say "the tokens you saw were tentative; this response timed out, so discard them and restart."
|
||||
- The adapter registry is one adapter per model name. That makes "logical model" and "concrete API route" the same thing, so the service has no vocabulary for "same model through another endpoint or SDK", "same provider region with different health", "fallback route with compatible capabilities", provider request ids, or per-route backoff state.
|
||||
|
||||
The result is a package that can surface an unstable LLM API failure, but cannot make principled recovery decisions for its callers. Ordinary provider turbulence should not require every consumer to reinvent retries around an unsafe stream boundary.
|
||||
|
||||
## Proposal
|
||||
|
||||
Introduce an LLM-call v2 contract centered on API-instability recovery: classify provider/API failures, separate provider responses from committed model output, route logical models through recoverable API routes, and make conservative retry/failover the default behavior in `dsh-llm`. Because the harness is unreleased, this should be a breaking cleanup rather than a compatibility layer around the underspecified v1 surface.
|
||||
|
||||
### 1. Replace flat error codes with a serializable `LlmFailure`
|
||||
|
||||
Keep `HarnessError` as the common thrown-error base, but make LLM failures carry a structured, JSON-serializable payload. `code` remains a stable leaf label for logs and provider-specific matching; retry/failover policy branches on the structured fields.
|
||||
|
||||
```ts
|
||||
type LlmFailureClass =
|
||||
| 'auth'
|
||||
| 'rate-limit'
|
||||
| 'quota'
|
||||
| 'invalid-request'
|
||||
| 'unsupported'
|
||||
| 'timeout'
|
||||
| 'transport'
|
||||
| 'provider-overloaded'
|
||||
| 'provider-unavailable'
|
||||
| 'provider-bug'
|
||||
| 'protocol'
|
||||
| 'safety'
|
||||
| 'aborted'
|
||||
| 'unknown'
|
||||
|
||||
type LlmFailurePhase =
|
||||
| 'request-build'
|
||||
| 'connect'
|
||||
| 'response-headers'
|
||||
| 'stream'
|
||||
| 'finish'
|
||||
|
||||
interface LlmFailure {
|
||||
message: string
|
||||
code: string
|
||||
class: LlmFailureClass
|
||||
phase: LlmFailurePhase
|
||||
retryable: boolean
|
||||
failover: 'never' | 'same-model' | 'compatible-model'
|
||||
partialOutput: 'none' | 'uncommitted' | 'committed'
|
||||
provider?: string
|
||||
routeId?: string
|
||||
model?: string
|
||||
wireModel?: string
|
||||
status?: number
|
||||
retryAfterMs?: number
|
||||
requestId?: string
|
||||
}
|
||||
```
|
||||
|
||||
`LlmError` should carry `failure: LlmFailure`; `FinishReasonMap.error` should carry the same payload instead of a parallel `{ message, code? }` shape. Adapter-thrown errors and in-band finish errors are input forms to the recovery layer. The public lifecycle stream should convert classified LLM API failures into terminal lifecycle events rather than requiring callers to catch thrown provider errors. The failure payload is serializable so callers can log or persist it if they choose, while the internal thrown `LlmError` may still carry a non-serializable `cause` chain for local debugging.
|
||||
|
||||
Adapters are responsible for faithful provider/API classification at their boundary: HTTP status, `Retry-After`, provider request id headers, SDK error type, timeout vs caller abort, malformed SSE, missing `[DONE]`, unknown finish reason, and unsupported local request shape. The current pi-ai adapter's regex over message text is acceptable only as a temporary fallback when the SDK hides the real status; the adapter should prefer structured SDK/provider fields when available.
|
||||
|
||||
### 2. Split provider responses from committed model output
|
||||
|
||||
Make "response" a first-class boundary in `ctx.llm.stream()`. An adapter streams one provider API response. The LLM service runs zero or more responses according to recovery policy and exposes one canonical response-lifecycle stream to consumers. Convenience APIs may expose a committed-or-failed result for simple callers, but that result must be derived from the lifecycle stream, not a parallel contract.
|
||||
|
||||
The primary response id is generated by the harness before the adapter call starts. Provider response ids and request ids are metadata attached when known; they are not the primary key because providers may omit them, report them only after the stream starts, reuse them in surprising ways, or fail before one exists.
|
||||
|
||||
The important invariant: chunks from a failed response must never be silently spliced together with chunks from a later response as one apparent model result. Token deltas from a response are tentative until that response reaches a committing terminal finish (`stop`, `tool-calls`, or `max-tokens`). The lifecycle stream must be able to report that tentative tokens were shown live, then discarded because the response timed out, disconnected, or otherwise failed before commit.
|
||||
|
||||
The event vocabulary should keep the familiar `assistant/chunk` concept but stop pretending every chunk is already final output. A possible spelling is:
|
||||
|
||||
```ts ignore-check
|
||||
type LlmStreamEvent =
|
||||
| { type: 'response/start'; responseId: ResponseId; responseIndex: number; routeId: string }
|
||||
| { type: 'assistant/chunk'; responseId: ResponseId; commitment: 'uncommitted'; chunk: StreamChunk }
|
||||
| { type: 'response/interrupted'; responseId: ResponseId; failure: LlmFailure; scheduledRetryMs?: number }
|
||||
| { type: 'response/failed'; responseId?: ResponseId; failure: LlmFailure }
|
||||
| { type: 'response/committed'; responseId: ResponseId; message: Message; finish: FinishReason; usage?: TokenUsage }
|
||||
|
||||
type LlmCallOutcome =
|
||||
| { type: 'committed'; responseId: ResponseId; message: Message; finish: FinishReason; usage?: TokenUsage }
|
||||
| { type: 'failed'; responseId?: ResponseId; failure: LlmFailure }
|
||||
```
|
||||
|
||||
The implementation may choose the exact names, but the type shape should make the state transition obvious: assistant chunks start uncommitted, then the enclosing response becomes interrupted/discarded, failed, or committed.
|
||||
|
||||
- **Lifecycle assistant chunks.** The lifecycle stream should make the old ambiguity explicit: these events are assistant chunk messages, but each one belongs to a response and has a commitment state. Most arrive as uncommitted live UI state; a response that fails before commit marks them interrupted/discarded, and a response that reaches a committing finish lets the UI mark that response committed.
|
||||
- **Retry before commit.** If an API response fails before a committing finish, the service may retry or fail over and exclude the failed response from the committed result, while surfacing response diagnostics separately.
|
||||
- **Commit on terminal finish.** Once a response reaches a committing finish, the response owns the visible result. `dsh-llm` emits a `response/committed` event carrying the fully assembled assistant `Message`, final `FinishReason`, usage, and response metadata. Callers that persist messages, execute tool calls, or otherwise take side effects should use this committed event rather than rebuilding output from lifecycle chunks.
|
||||
- **Terminal failure.** If recovery reaches a non-retryable failure, is aborted by the caller, or otherwise stops without a committing finish, `dsh-llm` emits `response/failed` carrying the final `LlmFailure` and ends the lifecycle stream normally. Throwing is reserved for defects outside the classified LLM API failure contract.
|
||||
- **Fail after commit.** If a later failure is ever observable after commit, the lifecycle reports `response/failed` with `partialOutput: 'committed'`; automatic retry is not allowed unless a later RFC designs an explicit continuation/repair protocol.
|
||||
|
||||
This means replacing the single overloaded raw-chunk `llm/stream` waterfall with a lifecycle stream and narrower hooks: one around a single provider response, one around recovery policy decisions, and one around convenience APIs that only expose the terminal outcome. Names are implementation details for the follow-up PR, but the semantics are not: plugins must be able to wrap "one API response" without pretending they can safely retry already-committed chunks.
|
||||
|
||||
### 3. Route logical models through recoverable API routes
|
||||
|
||||
Separate the logical model a caller requests from the concrete provider route that serves a response. Replace "one adapter per model name" with route registration, for example:
|
||||
|
||||
```ts ignore-check
|
||||
ctx.llm.registerRoute({
|
||||
routeId: 'deepseek-direct:deepseek-v4-flash',
|
||||
model: 'deepseek-v4-flash',
|
||||
wireModel: 'deepseek-v4-flash',
|
||||
provider: 'deepseek',
|
||||
adapter,
|
||||
priority: 0,
|
||||
capabilities: { tools: true, reasoning: true, images: false, prefill: false },
|
||||
})
|
||||
```
|
||||
|
||||
`GenerateOptions.model` remains the logical model. The service resolves it to a route for each API response, records the route in failure/response diagnostics, and can retry on the same route or fail over to another route with compatible capabilities. Duplicate model names become normal; duplicate route ids are the conflict. This is the smallest vocabulary that can express direct endpoint vs SDK-backed endpoint, regional endpoints, and future fallback models without making every caller own routing.
|
||||
|
||||
The route registry must keep the lifecycle guarantees of the current adapter registry: `registerRoute()` is effect-scoped, returns a disposer, and has an HMR-safety test proving disposal removes the route. It should not preserve `llm/adapter-change`; if [PR #82](https://github.com/deepseek-ai/deepseek-harness/pull/82) lands first, that event is already gone, and the route registry should not reintroduce it without a concrete consumer.
|
||||
|
||||
The new ids should follow the branded-id policy. `ResponseId`, `RouteId`, and the logical/wire model ids cross package boundaries and are easy to swap accidentally, so the implementation should deliberately brand or explicitly decline to brand each one in line with `2026-06-20-branded-ids` and its implementation stack ([PR #84](https://github.com/deepseek-ai/deepseek-harness/pull/84)).
|
||||
|
||||
### 4. Put default API recovery policy in `dsh-llm`
|
||||
|
||||
Adapters should not perform hidden SDK retries unless those retries are surfaced as response lifecycle events with classified failures. The service owns the default policy so every consumer gets the same behavior and the same audit trail.
|
||||
|
||||
Default policy should be conservative:
|
||||
|
||||
- Retry transient API failures (`rate-limit`, `timeout`, `transport`, `provider-overloaded`, `provider-unavailable`) only before committed output, and keep retrying until the caller aborts or the failure class changes to a non-retryable one.
|
||||
- Honor `retryAfterMs` up to `maxRetryDelayMs`, otherwise use bounded exponential backoff with jitter. The same cap applies to both provider-supplied retry hints and ordinary exponential backoff; diagnostics record whether the delay source was `provider-retry-after` or `exponential-backoff`.
|
||||
- Treat 429/408/409/425/500/502/503/504 and connection resets as potentially recoverable unless the provider payload says otherwise; treat 400/401/403, unsupported local options, caller abort, and adapter protocol bugs as non-retryable.
|
||||
- Fail over only when the failure says failover is safe and the candidate route advertises compatible capabilities for the request (`tools`, reasoning passback, images, prefill, stop sequences, strict tools).
|
||||
- Share the caller's `AbortSignal` across the whole recovered call, and expose per-response timeouts as explicit policy. A stuck stream must time out in a controlled way instead of hanging the turn forever.
|
||||
- Surface every retry decision to the UI with retry count, backoff delay, route, and failure summary, so an actively watching user can tell the agent is waiting on provider capacity instead of frozen.
|
||||
|
||||
The policy should be configurable through a typed service option and an event/waterfall seam so product plugins can adjust timing/backoff details, but the default retry posture is not opt-in: a basic agent should keep recovering from retryable 429/5xx/connectivity noise until cancelled.
|
||||
|
||||
The zero-config defaults should be sensible production behavior, not placeholders:
|
||||
|
||||
```ts
|
||||
const defaultLlmRecoveryConfig = {
|
||||
maxResponses: 'unbounded',
|
||||
maxElapsedMs: 'unbounded',
|
||||
connectTimeoutMs: 15_000,
|
||||
responseHeaderTimeoutMs: 60_000,
|
||||
streamIdleTimeoutMs: 5 * 60_000,
|
||||
initialBackoffMs: 200,
|
||||
maxRetryDelayMs: 10 * 60_000,
|
||||
jitterRatio: 0.1,
|
||||
}
|
||||
```
|
||||
|
||||
The retry-delay cap is deliberate. The survey found mixed precedent: Codex parses retry delays out of streamed OpenAI rate-limit error messages and uses that requested delay, but Codex also has finite stream retry counts; the official OpenAI and Anthropic TypeScript SDKs parse `retry-after-ms`, `Retry-After` seconds, and `Retry-After` dates and then sleep for the provider-specified duration; the official OpenAI and Anthropic Python SDKs only honor `Retry-After` when it is greater than zero and at most 60 seconds, otherwise falling back to ordinary exponential backoff. Because this RFC's default retry posture is unbounded, blindly honoring a multi-hour provider delay can make the agent look dead, while ignoring the hint entirely can retry too aggressively. The service should therefore record both `providerRetryAfterMs` and `scheduledRetryMs`, cap the scheduled sleep at `maxRetryDelayMs`, and surface that choice to the UI.
|
||||
|
||||
The service cannot reliably infer whether the user is actively watching or away from the keyboard, so the default should not fail a retryable model call merely because a short interactive budget expired. A clear UI can make long waits tolerable even in interactive sessions: "retried 8 times; next retry in 10 minutes" is better than silently failing recoverable provider turbulence and asking the user to resubmit.
|
||||
|
||||
### 5. Define the caller contract, not the product transcript
|
||||
|
||||
This RFC is deliberately about the `dsh-llm` API and how callers use it, not about the final transcript/event architecture of the product. The LLM package should guarantee these caller-visible semantics:
|
||||
|
||||
- `ctx.llm.stream()` is the live response-lifecycle API. It reports response starts, uncommitted assistant chunks, retries/backoff, interruptions/discards, terminal failures, and the one committed result.
|
||||
- `response/committed` is the only event that makes model output safe for history or side effects. It carries the assembled `Message`, finish reason, usage, response id, route metadata, and provider ids known to the service.
|
||||
- `response/failed` is the terminal event for classified failures. Callers should not need `try`/`catch` to learn that a provider was rate-limited, unavailable, misconfigured, aborted, or otherwise unable to produce a committed response.
|
||||
- Response diagnostics are JSON-serializable so callers can store, display, or ignore them. The LLM package does not decide whether those diagnostics become session events, agent events, telemetry rows, or UI-only state.
|
||||
- Any assembled convenience helper that survives or is reintroduced returns a terminal union (`committed` or `failed`) rather than throwing for classified LLM failures. It must be derived from the lifecycle stream so recovery semantics stay single-sourced.
|
||||
|
||||
The session log shape, agent event taxonomy, ACP rendering, snapshot/replay fixtures, and whether live uncommitted chunks are ever durably recorded are downstream integration decisions. So is the fate of today's assembled public helper methods: [PR #82](https://github.com/deepseek-ai/deepseek-harness/pull/82) implements the proposed removal of `generate()`, `streamBlocks()`, `GenerateResult`, and `llm/generate`, and this RFC should not resurrect them without a real caller. This RFC should constrain downstream work only by the LLM API contract above.
|
||||
|
||||
The current simplification stack was checked while drafting this proposal. [PR #83](https://github.com/deepseek-ai/deepseek-harness/pull/83) and [PR #85](https://github.com/deepseek-ai/deepseek-harness/pull/85) do not change this RFC's LLM API assumptions. [PR #86](https://github.com/deepseek-ai/deepseek-harness/pull/86) does matter for later integration because it folds durable token usage onto `assistant/message` and operational errors onto `turn/end.reason`; if it lands first, the LLM recovery implementation should still stop at the `dsh-llm` lifecycle contract here and let the agent/session layer decide how committed usage and terminal failures map onto those load-bearing product events.
|
||||
|
||||
## Out of scope
|
||||
|
||||
This RFC does not propose silent mid-stream continuation after user-visible output. That requires a separate model-history design: either provider-supported prefill/continuation, a recovery prompt that explicitly shows the partial assistant output, or a UI affordance that marks the partial answer as failed and asks the model to continue in a new step. Splicing two API responses into one assistant message is rejected.
|
||||
|
||||
This RFC also does not solve semantic model-output repair: malformed tool-call JSON, refusal handling, or content-filter fallbacks. Those may use the same failure vocabulary later, but they are higher-level agent behaviors, not unstable-API recovery.
|
||||
|
||||
This RFC does not decide whether product UIs consume LLM lifecycle events directly, through agent events, or through session events. It also does not decide which response diagnostics belong in the durable session log. Those decisions belong in narrower integration RFCs once the `dsh-llm` contract exists.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
<!-- agent-note-format: alternatives-not-recorded (pre-format Agent Note) -->
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- Adapter-thrown `LlmError`s and in-band finish errors carry one structured, JSON-serializable `LlmFailure` payload.
|
||||
- Recovery policy can distinguish retry, failover, credential/user-action, unsupported request, caller abort, adapter/protocol bug, and post-commit partial stream failure without parsing message text.
|
||||
- `ctx.llm.stream()` exposes the response lifecycle as the canonical stream, including terminal `response/failed` events for classified failures; convenience APIs are derived views for callers that only want the terminal outcome.
|
||||
- `response/committed` carries the assembled assistant `Message`; callers do not need to rebuild committed output from lifecycle chunks.
|
||||
- Any assembled convenience API that survives or is reintroduced returns a committed/failed union derived from the lifecycle stream, rather than throwing for classified LLM failures.
|
||||
- The LLM service has an explicit API-response boundary; no retry path can present output from two provider responses as one committed assistant result.
|
||||
- The route registry allows multiple concrete API routes for one logical model and records the selected route on responses/failures.
|
||||
- `registerRoute()` is effect-scoped, returns a disposer, and has an HMR-safety test proving route cleanup; `llm/adapter-change` is not reintroduced unless a concrete consumer needs it.
|
||||
- New LLM ids are deliberately branded or explicitly left unbranded according to the branded-id policy, with `ResponseId`, `RouteId`, and logical/wire model ids decided together.
|
||||
- Default recovery retries transient pre-commit failures with bounded backoff, honors provider retry-after hints, times out stuck streams, disables hidden SDK retries or surfaces them as response lifecycle events, and never retries after committed chunks without an explicit continuation design.
|
||||
- Unit tests cover thrown errors and finish-error chunks through `dsh-llm`, retry-before-first-commit, failover to a second compatible route, unbounded retry status/backoff visibility, abort during backoff, stream timeout, and the "partial chunks then failure does not retry/splice" invariant.
|
||||
- Adapter tests classify representative HTTP statuses, retry-after headers, request ids, malformed/truncated SSE streams, SDK in-stream errors, caller aborts, and unsupported options into `LlmFailure`.
|
||||
- Docs updated in the same change: [the architecture LLM section](../../../../docs/architecture.md), [the LLM adapter cookbook](../../../../docs/cookbook/adding-an-llm-adapter.md), and the LLM package README.
|
||||
|
||||
## Risks
|
||||
|
||||
- **More surface area in the LLM core.** API recovery adds policy, route state, response diagnostics, and tests. That complexity belongs in `dsh-llm` because every consumer otherwise reinvents it around the same unsafe stream boundary.
|
||||
- **Committed result lags live UI.** Safe recovery means callers cannot treat streamed tokens as final assistant output until the response commits. UIs can still stream eagerly from lifecycle assistant chunks, but side-effecting consumers must wait for `response/committed`.
|
||||
- **Breaking adapter churn.** Existing adapters will change from "stream chunks or throw a flat `LlmError`" to "stream one classified API response." Pre-release rules favor the correct seam over shims.
|
||||
- **Route compatibility is easy to overclaim.** A route must advertise concrete capabilities, and failover must check the request actually fits them. "Same model name" is not enough when one route lacks strict tools, reasoning passback, images, stop sequences, or prefill.
|
||||
|
||||
## Related
|
||||
|
||||
- Builds on [Provider-neutral content-block vocabulary](../../implemented/architecture/2026-06-11-content-block-vocabulary.md): the content vocabulary stays provider-neutral; this adds a provider-neutral failure/recovery vocabulary beside it.
|
||||
- Revises the scope implied by [Two LLM adapters as a design-verification twin](../../implemented/architecture/2026-06-13-twin-llm-adapters.md): the twin validated chunk shape and error delivery paths, but it also exposed that delivery paths are not enough for unstable API recovery.
|
||||
- Extends [Structured error taxonomy](../../implemented/architecture/2026-06-11-structured-error-taxonomy.md): `HarnessError.code` was the foundation; LLM API recovery needs a richer payload because retry/failover policy cannot safely branch on one flat string.
|
||||
- Coordinates with [PR #82](https://github.com/deepseek-ai/deepseek-harness/pull/82), which implements the `drop-unconsumed-llm-assembled-surfaces` and `drop-unconsumed-llm-adapter-change-event` simplification RFCs. If that PR lands first, this RFC starts from a narrower `dsh-llm`: no `generate()`, no `streamBlocks()`, no `GenerateResult`, no `llm/generate`, and no `llm/adapter-change`. Recovery should build on that baseline rather than revive removed convenience or change-notification surfaces speculatively.
|
||||
- Is orthogonal to [PR #81](https://github.com/deepseek-ai/deepseek-harness/pull/81), which proposes provider-request app attribution headers. Recovery route metadata and provider request construction can carry attribution policy later, but this RFC does not define request headers.
|
||||
- Coordinates with [PR #84](https://github.com/deepseek-ai/deepseek-harness/pull/84), which implements the branded-id RFC. The new response/route/model ids introduced here are exactly the sort of cross-boundary ids that need an explicit branding decision before implementation.
|
||||
- Coordinates with [PR #86](https://github.com/deepseek-ai/deepseek-harness/pull/86), which implements the `collapse-trace-only-session-events` simplification RFC. If that stack lands first, downstream recovery integration should map committed usage and terminal error facts onto the surviving load-bearing session events instead of reintroducing standalone trace-only records from inside `dsh-llm`.
|
||||
Reference in New Issue
Block a user