mirror of
https://github.com/MoonshotAI/kimi-code.git
synced 2026-08-21 06:35:50 +00:00
fix: fail fast on provider-filtered empty responses (#3101)
* fix: fail fast on provider-filtered empty responses An APIEmptyResponseError carrying finishReason 'filtered' (OpenAI content_filter, Anthropic refusal) is deterministic: replaying the same request re-triggers the provider safety filter. Both isRetryableGenerateError implementations (kosong, agent-core-v2) treated every empty response as retryable, so step retry replayed the doomed request the full 10 attempts before the filter notice surfaced. Return non-retryable for filtered empty responses in both engines; the error already carries the provider.filtered code, so the turn fails immediately with the existing filter notice. * fix: skip the compaction shrink-retry for filtered empty responses Both full-compaction loops routed every APIEmptyResponseError into the shrink-and-continue branch before isRetryableGenerateError was consulted, so a filtered response was retried with shrinking input instead of failing fast. Exclude finishReason 'filtered' from the shrink branch in both engines; it now falls through to the retryability check and throws immediately. Add end-to-end tests (real kosong generate over a filtered think-only stream) asserting a single attempt with the history untouched. --------- Co-authored-by: kimi-agent-bot <kimi-agent-bot@users.noreply.github.com>
This commit is contained in:
parent
3d7762003a
commit
d96b4a0149
11 changed files with 149 additions and 12 deletions
5
.changeset/filtered-empty-response-fail-fast.md
Normal file
5
.changeset/filtered-empty-response-fail-fast.md
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
"@moonshot-ai/kimi-code": patch
|
||||
---
|
||||
|
||||
Stop retrying requests blocked by the provider content filter; the filter notice now shows immediately.
|
||||
|
|
@ -686,8 +686,11 @@ export class AgentFullCompactionService extends Service implements IAgentFullCom
|
|||
retryCount = 0;
|
||||
continue;
|
||||
}
|
||||
const unwrappedError = unwrapErrorCause(error);
|
||||
if (
|
||||
(error instanceof CompactionTruncatedError || unwrapErrorCause(error) instanceof APIEmptyResponseError) &&
|
||||
(error instanceof CompactionTruncatedError ||
|
||||
(unwrappedError instanceof APIEmptyResponseError &&
|
||||
unwrappedError.finishReason !== 'filtered')) &&
|
||||
messagesToCompact.length > 1
|
||||
) {
|
||||
emptyOrTruncatedShrinkCount += 1;
|
||||
|
|
@ -700,7 +703,7 @@ export class AgentFullCompactionService extends Service implements IAgentFullCom
|
|||
retryCount = 0;
|
||||
continue;
|
||||
}
|
||||
if (!isRetryableGenerateError(unwrapErrorCause(error))) {
|
||||
if (!isRetryableGenerateError(unwrappedError)) {
|
||||
throw error;
|
||||
}
|
||||
if (retryCount + 1 >= MAX_COMPACTION_RETRY_ATTEMPTS) {
|
||||
|
|
|
|||
|
|
@ -236,7 +236,7 @@ export function isRetryableGenerateError(error: unknown): boolean {
|
|||
return true;
|
||||
}
|
||||
if (error instanceof APIEmptyResponseError) {
|
||||
return true;
|
||||
return error.finishReason !== 'filtered';
|
||||
}
|
||||
if (error instanceof APIProviderOverloadedError) {
|
||||
return true;
|
||||
|
|
|
|||
|
|
@ -921,6 +921,37 @@ describe('FullCompaction', () => {
|
|||
]);
|
||||
});
|
||||
|
||||
it('fails fast without shrinking when the provider filters the compaction response', async () => {
|
||||
const inputs: string[][] = [];
|
||||
const generate = realKosongGenerate((_attempt, history) => {
|
||||
inputs.push(inputHistorySnapshot(history));
|
||||
return mockStreamedMessage(
|
||||
[{ type: 'think', think: 'Filtered while reasoning about the summary.' }],
|
||||
null,
|
||||
{ finishReason: 'filtered', rawFinishReason: 'content_filter' },
|
||||
);
|
||||
});
|
||||
const ctx = testAgent({ generate });
|
||||
ctx.configure({
|
||||
provider: CATALOGUED_PROVIDER,
|
||||
modelCapabilities: CATALOGUED_MODEL_CAPABILITIES,
|
||||
});
|
||||
ctx.appendExchange(1, 'old user one', 'old assistant one', 20);
|
||||
ctx.appendExchange(2, 'recent user two', 'recent assistant two', 80);
|
||||
const failed = ctx.once('error');
|
||||
|
||||
await ctx.rpc.beginCompaction({});
|
||||
await failed;
|
||||
|
||||
expect(inputs).toHaveLength(1);
|
||||
expect(ctx.compactHistory()).toEqual([
|
||||
{ role: 'user', text: 'old user one' },
|
||||
{ role: 'assistant', text: 'old assistant one' },
|
||||
{ role: 'user', text: 'recent user two' },
|
||||
{ role: 'assistant', text: 'recent assistant two' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('waits before retrying compaction generation after a retryable failure', async () => {
|
||||
vi.useFakeTimers();
|
||||
const firstAttemptFailed = deferred<void>();
|
||||
|
|
@ -3056,6 +3087,7 @@ function textResult(text: string, traceId: string | null = null): Awaited<Return
|
|||
function mockStreamedMessage(
|
||||
parts: readonly StreamedMessagePart[],
|
||||
traceId: string | null = null,
|
||||
opts?: { finishReason?: StreamedMessage['finishReason']; rawFinishReason?: string | null },
|
||||
): StreamedMessage {
|
||||
return {
|
||||
get id(): string | null {
|
||||
|
|
@ -3064,8 +3096,8 @@ function mockStreamedMessage(
|
|||
get usage() {
|
||||
return null;
|
||||
},
|
||||
finishReason: null,
|
||||
rawFinishReason: null,
|
||||
finishReason: opts?.finishReason ?? null,
|
||||
rawFinishReason: opts?.rawFinishReason ?? null,
|
||||
traceId,
|
||||
async *[Symbol.asyncIterator](): AsyncIterator<StreamedMessagePart> {
|
||||
for (const part of parts) {
|
||||
|
|
|
|||
|
|
@ -120,6 +120,16 @@ describe('isRetryableGenerateError', () => {
|
|||
expect(isRetryableGenerateError(new APIStatusError(400, 'Bad request'))).toBe(false);
|
||||
expect(isRetryableGenerateError(new APIStatusError(401, 'Unauthorized'))).toBe(false);
|
||||
});
|
||||
|
||||
it('does not retry provider-filtered empty responses', () => {
|
||||
expect(
|
||||
isRetryableGenerateError(new APIEmptyResponseError('filtered', { finishReason: 'filtered' })),
|
||||
).toBe(false);
|
||||
expect(
|
||||
isRetryableGenerateError(new APIEmptyResponseError('empty', { finishReason: 'completed' })),
|
||||
).toBe(true);
|
||||
expect(isRetryableGenerateError(new APIEmptyResponseError('empty'))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('classifyApiError', () => {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import { describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { APIEmptyResponseError } from '#/kosong/contract/errors';
|
||||
import { APIEmptyResponseError, isRetryableGenerateError } from '#/kosong/contract/errors';
|
||||
import { generate, type GenerateResult } from '#/kosong/contract/generate';
|
||||
import type { Message, StreamedMessagePart, ToolCall } from '#/kosong/contract/message';
|
||||
import type {
|
||||
|
|
@ -198,6 +198,22 @@ describe('generate() stream normalization', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('marks a provider-filtered thinking-only response as non-retryable', async () => {
|
||||
class FilteredStream extends FakeStreamedMessage {
|
||||
override readonly finishReason: FinishReason | null = 'filtered';
|
||||
override readonly rawFinishReason: string | null = 'content_filter';
|
||||
}
|
||||
const stream = new FilteredStream([{ type: 'think', think: 'filtered mid-thought' }]);
|
||||
const { provider } = createFakeProvider(stream);
|
||||
|
||||
const caught = await generate(provider, SYSTEM_PROMPT, NO_TOOLS, HISTORY).catch(
|
||||
(error: unknown) => error,
|
||||
);
|
||||
|
||||
expect(caught).toBeInstanceOf(APIEmptyResponseError);
|
||||
expect(isRetryableGenerateError(caught)).toBe(false);
|
||||
});
|
||||
|
||||
it('forwards the trace id to onTraceId and the result', async () => {
|
||||
const stream = new FakeStreamedMessage([{ type: 'text', text: 'ok' }], {
|
||||
traceId: 'trace-123',
|
||||
|
|
|
|||
|
|
@ -547,9 +547,13 @@ export class FullCompaction {
|
|||
retryCount = 0;
|
||||
continue;
|
||||
}
|
||||
// A filtered response is not a size problem: shrinking the input
|
||||
// cannot get a safety-filtered request through, so exclude it here
|
||||
// and let it fall through to the retryability check, which fails it
|
||||
// fast instead of burning the shrink budget.
|
||||
const shouldShrinkAfterEmptyOrTruncated =
|
||||
error instanceof CompactionTruncatedError ||
|
||||
error instanceof APIEmptyResponseError;
|
||||
(error instanceof APIEmptyResponseError && error.finishReason !== 'filtered');
|
||||
if (shouldShrinkAfterEmptyOrTruncated && historyForModel.length > 1) {
|
||||
// Each empty/truncated summary drops the oldest message and retries,
|
||||
// but without its own bound this would issue ~one request per message
|
||||
|
|
|
|||
|
|
@ -900,6 +900,42 @@ describe('FullCompaction', () => {
|
|||
]);
|
||||
});
|
||||
|
||||
it('fails fast without shrinking when the provider filters the compaction response', async () => {
|
||||
// End-to-end through the real kosong generate(): a think-only stream whose
|
||||
// finishReason is 'filtered' (content_filter) throws APIEmptyResponseError,
|
||||
// and the retry predicate now marks it non-retryable. Compaction must NOT
|
||||
// route it into the shrink-and-retry branch either — replaying the same
|
||||
// filtered request would just re-trigger the filter — so it fails on the
|
||||
// very first attempt with the history untouched.
|
||||
const inputs: string[][] = [];
|
||||
const generate = realKosongGenerate((_attempt, history) => {
|
||||
inputs.push(inputHistorySnapshot(history));
|
||||
return mockStreamedMessage(
|
||||
[{ type: 'think', think: 'Filtered while reasoning about the summary.' }],
|
||||
{ finishReason: 'filtered', rawFinishReason: 'content_filter' },
|
||||
);
|
||||
});
|
||||
const ctx = testAgent({ generate });
|
||||
ctx.configure({
|
||||
provider: CATALOGUED_PROVIDER,
|
||||
modelCapabilities: CATALOGUED_MODEL_CAPABILITIES,
|
||||
});
|
||||
ctx.appendExchange(1, 'old user one', 'old assistant one', 20);
|
||||
ctx.appendExchange(2, 'recent user two', 'recent assistant two', 80);
|
||||
const failed = ctx.once('error');
|
||||
|
||||
await ctx.rpc.beginCompaction({});
|
||||
await failed;
|
||||
|
||||
expect(inputs).toHaveLength(1);
|
||||
expect(ctx.compactHistory()).toEqual([
|
||||
{ role: 'user', text: 'old user one' },
|
||||
{ role: 'assistant', text: 'old assistant one' },
|
||||
{ role: 'user', text: 'recent user two' },
|
||||
{ role: 'assistant', text: 'recent assistant two' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('waits before retrying compaction generation after a retryable failure', async () => {
|
||||
vi.useFakeTimers();
|
||||
const firstAttemptFailed = deferred<void>();
|
||||
|
|
@ -2609,7 +2645,10 @@ function textResult(text: string): Awaited<ReturnType<GenerateFn>> {
|
|||
};
|
||||
}
|
||||
|
||||
function mockStreamedMessage(parts: readonly StreamedMessagePart[]): StreamedMessage {
|
||||
function mockStreamedMessage(
|
||||
parts: readonly StreamedMessagePart[],
|
||||
opts?: { finishReason?: StreamedMessage['finishReason']; rawFinishReason?: string | null },
|
||||
): StreamedMessage {
|
||||
return {
|
||||
get id(): string | null {
|
||||
return 'mock-stream';
|
||||
|
|
@ -2617,8 +2656,8 @@ function mockStreamedMessage(parts: readonly StreamedMessagePart[]): StreamedMes
|
|||
get usage() {
|
||||
return null;
|
||||
},
|
||||
finishReason: null,
|
||||
rawFinishReason: null,
|
||||
finishReason: opts?.finishReason ?? null,
|
||||
rawFinishReason: opts?.rawFinishReason ?? null,
|
||||
async *[Symbol.asyncIterator](): AsyncIterator<StreamedMessagePart> {
|
||||
for (const part of parts) {
|
||||
yield part;
|
||||
|
|
|
|||
|
|
@ -215,7 +215,10 @@ export function isRetryableGenerateError(error: unknown): boolean {
|
|||
return true;
|
||||
}
|
||||
if (error instanceof APIEmptyResponseError) {
|
||||
return true;
|
||||
// A filtered response is deterministic: replaying the same request just
|
||||
// re-triggers the provider's safety filter, so fail fast and surface the
|
||||
// filter notice instead of burning the whole step-retry budget.
|
||||
return error.finishReason !== 'filtered';
|
||||
}
|
||||
if (error instanceof APIStatusError) {
|
||||
// Quota/balance exhaustion is a 429 but deterministic until the account
|
||||
|
|
|
|||
|
|
@ -139,6 +139,18 @@ describe('isRetryableGenerateError', () => {
|
|||
expect(isRetryableGenerateError(new APIEmptyResponseError('empty'))).toBe(true);
|
||||
});
|
||||
|
||||
it('does not retry empty responses blocked by the provider content filter', () => {
|
||||
expect(
|
||||
isRetryableGenerateError(new APIEmptyResponseError('filtered', { finishReason: 'filtered' })),
|
||||
).toBe(false);
|
||||
expect(
|
||||
isRetryableGenerateError(new APIEmptyResponseError('empty', { finishReason: 'completed' })),
|
||||
).toBe(true);
|
||||
expect(
|
||||
isRetryableGenerateError(new APIEmptyResponseError('empty', { finishReason: null })),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it.each([408, 409, 429, 500, 502, 503, 504, 529])('treats HTTP %i as retryable', (statusCode) => {
|
||||
expect(isRetryableGenerateError(new APIStatusError(statusCode, 'retryable'))).toBe(true);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { APIEmptyResponseError } from '#/errors';
|
||||
import { APIEmptyResponseError, isRetryableGenerateError } from '#/errors';
|
||||
import { generate } from '#/generate';
|
||||
import type { Message, StreamedMessagePart, ToolCall } from '#/message';
|
||||
import type { ChatProvider, StreamedMessage, ThinkingEffort } from '#/provider';
|
||||
|
|
@ -244,6 +244,19 @@ describe('generate()', () => {
|
|||
expect(err.message).toContain('provider filtered the response');
|
||||
});
|
||||
|
||||
it('marks a provider-filtered think-only response as non-retryable', async () => {
|
||||
const stream = createMockStream([{ type: 'think', think: 'filtered mid-thought' }], {
|
||||
finishReason: 'filtered',
|
||||
rawFinishReason: 'content_filter',
|
||||
});
|
||||
const provider = createMockProvider(stream);
|
||||
|
||||
const caught = await generate(provider, '', [], []).catch((error: unknown) => error);
|
||||
|
||||
expect(caught).toBeInstanceOf(APIEmptyResponseError);
|
||||
expect(isRetryableGenerateError(caught)).toBe(false);
|
||||
});
|
||||
|
||||
it('throws APIEmptyResponseError for think + empty/whitespace text', async () => {
|
||||
const stream = createMockStream([
|
||||
{ type: 'think', think: 'Thinking...' },
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue