fix(llm): classify empty model completions as retryable EMPTY_RESPONSE
A well-formed provider stream that ends with finish_reason stop and zero
content blocks previously became a successful empty assistant message: the
turn completed silently, and drivers like goal-session counted the no-op
round. Both adapters now map that degenerate completion to a finish
{kind:'error'} with the new canonical EMPTY_RESPONSE code from dsh-llm, and
dsh-llm-retry adds the code to its default retryable set, so the existing
closed-step recovery path retries it and fails loud once the budget is
exhausted.
Covered by adapter unit tests, an llm-retry default-policy test, and a new
authored keyless ACP snapshot (empty-response-retry) with a deterministic
1 ms zero-jitter retry overlay.
This commit is contained in:
@@ -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 `providerRetryAfterMs` 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 `EMPTY_RESPONSE`, `RATE_LIMIT`, `SERVER`, `TIMEOUT`, and `TRANSPORT`, using bounded exponential backoff from 500 ms to 10 seconds with 10 percent jitter. `EMPTY_RESPONSE` is the adapters' classification of a degenerate provider completion (a terminal stop with zero content blocks); the attempt produced nothing durable, so repeating it is safe. 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.
|
||||
|
||||
@@ -15,7 +15,7 @@ The separately published `./invariant` companion checks that every retry record
|
||||
initialDelayMs: 500
|
||||
maxDelayMs: 10000
|
||||
jitterRatio: 0.1
|
||||
retryableCodes: [RATE_LIMIT, SERVER, TIMEOUT, TRANSPORT]
|
||||
retryableCodes: [EMPTY_RESPONSE, RATE_LIMIT, SERVER, TIMEOUT, TRANSPORT]
|
||||
```
|
||||
|
||||
## Model Experience
|
||||
|
||||
@@ -33,7 +33,7 @@ const DEFAULT_MAX_TRANSIENT_RETRIES = 2
|
||||
const DEFAULT_INITIAL_DELAY_MS = 500
|
||||
const DEFAULT_MAX_DELAY_MS = 10_000
|
||||
const DEFAULT_JITTER_RATIO = 0.1
|
||||
const DEFAULT_RETRYABLE_CODES = Object.freeze(['RATE_LIMIT', 'SERVER', 'TIMEOUT', 'TRANSPORT'])
|
||||
const DEFAULT_RETRYABLE_CODES = Object.freeze(['EMPTY_RESPONSE', 'RATE_LIMIT', 'SERVER', 'TIMEOUT', 'TRANSPORT'])
|
||||
|
||||
/** Deployment-owned limits and classification for transient request recovery. */
|
||||
export interface Config {
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { Context } from 'cordis'
|
||||
import type { Fiber } from 'cordis'
|
||||
import LlmService, { CallId, LlmAdapter, LlmError } from '@deepseek-ai/dsh-llm'
|
||||
import LlmService, { CallId, EMPTY_RESPONSE_CODE, LlmAdapter, LlmError } from '@deepseek-ai/dsh-llm'
|
||||
import type { GenerateOptions, StreamChunk } from '@deepseek-ai/dsh-llm'
|
||||
import SessionStore, { SessionId } from '@deepseek-ai/dsh-session'
|
||||
import type { SessionEvent } from '@deepseek-ai/dsh-session'
|
||||
@@ -51,6 +51,25 @@ function textResponse(text: string): StreamChunk[] {
|
||||
]
|
||||
}
|
||||
|
||||
/**
|
||||
* A degenerate empty provider completion as an error finish chunk. Both
|
||||
* adapters emit this shape and the EMPTY_RESPONSE code (the field the policy
|
||||
* routes on); the message text here is the deepseek adapter's phrasing (pi-ai
|
||||
* qualifies it with the model name).
|
||||
*/
|
||||
function emptyCompletion(): StreamChunk[] {
|
||||
return [
|
||||
{ type: 'usage', usage: { inputTokens: 0, outputTokens: 0 } },
|
||||
{
|
||||
type: 'finish',
|
||||
reason: {
|
||||
kind: 'error',
|
||||
failure: { message: 'model returned a completed response with no content', code: EMPTY_RESPONSE_CODE },
|
||||
},
|
||||
},
|
||||
]
|
||||
}
|
||||
|
||||
async function harness(
|
||||
adapter: LlmAdapter,
|
||||
config: retry.Config = {},
|
||||
@@ -158,6 +177,39 @@ describe('bounded transient retry policy', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('retries an EMPTY_RESPONSE error finish under the default retryable codes', async () => {
|
||||
vi.useFakeTimers()
|
||||
const adapter = new ScriptedAdapter([
|
||||
emptyCompletion(),
|
||||
textResponse('recovered'),
|
||||
])
|
||||
// No retryableCodes override: this proves the default policy covers the
|
||||
// adapters' empty-completion classification end to end (finish-chunk error
|
||||
// delivery, not a thrown stream error).
|
||||
;({ ctx: context } = await harness(adapter))
|
||||
const agent = context.agentLoop.create(SessionId('retry-empty-response'), { provider: 'mock', model: 'mock' })
|
||||
const scheduled = waitForRetry(context, agent, 1)
|
||||
|
||||
agent.send([{ type: 'text', text: 'go' }])
|
||||
const event = await scheduled
|
||||
expect(event.data.failure).toEqual({
|
||||
message: 'model returned a completed response with no content',
|
||||
code: EMPTY_RESPONSE_CODE,
|
||||
})
|
||||
|
||||
const idle = waitForIdle(context, agent)
|
||||
await vi.advanceTimersByTimeAsync(500)
|
||||
await idle
|
||||
|
||||
expect(adapter.requests).toHaveLength(2)
|
||||
expect(agent.session.events.filter(event => event.type === 'assistant/message').map(event => event.data.step))
|
||||
.toEqual([2])
|
||||
expect(agent.session.deriveMessages().at(-1)).toMatchObject({
|
||||
role: 'assistant',
|
||||
content: [{ type: 'text', text: 'recovered' }],
|
||||
})
|
||||
})
|
||||
|
||||
it('leaves partial failed chunks on their step without committing a message or tool side effect', async () => {
|
||||
vi.useFakeTimers()
|
||||
const adapter = new ScriptedAdapter([
|
||||
|
||||
Reference in New Issue
Block a user