diff --git a/.changeset/filtered-empty-response-fail-fast.md b/.changeset/filtered-empty-response-fail-fast.md new file mode 100644 index 000000000..b5f946595 --- /dev/null +++ b/.changeset/filtered-empty-response-fail-fast.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Stop retrying requests blocked by the provider content filter; the filter notice now shows immediately. diff --git a/packages/agent-core-v2/src/agent/fullCompaction/fullCompactionService.ts b/packages/agent-core-v2/src/agent/fullCompaction/fullCompactionService.ts index 94efa719e..61c147dda 100644 --- a/packages/agent-core-v2/src/agent/fullCompaction/fullCompactionService.ts +++ b/packages/agent-core-v2/src/agent/fullCompaction/fullCompactionService.ts @@ -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) { diff --git a/packages/agent-core-v2/src/kosong/contract/errors.ts b/packages/agent-core-v2/src/kosong/contract/errors.ts index e2db1f28a..3a6bafde1 100644 --- a/packages/agent-core-v2/src/kosong/contract/errors.ts +++ b/packages/agent-core-v2/src/kosong/contract/errors.ts @@ -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; diff --git a/packages/agent-core-v2/test/agent/fullCompaction/fullCompaction.test.ts b/packages/agent-core-v2/test/agent/fullCompaction/fullCompaction.test.ts index 59d736a08..60b0800c1 100644 --- a/packages/agent-core-v2/test/agent/fullCompaction/fullCompaction.test.ts +++ b/packages/agent-core-v2/test/agent/fullCompaction/fullCompaction.test.ts @@ -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(); @@ -3056,6 +3087,7 @@ function textResult(text: string, traceId: string | null = null): Awaited { for (const part of parts) { diff --git a/packages/agent-core-v2/test/kosong/contract/errors.test.ts b/packages/agent-core-v2/test/kosong/contract/errors.test.ts index 5be891897..1d1be2b06 100644 --- a/packages/agent-core-v2/test/kosong/contract/errors.test.ts +++ b/packages/agent-core-v2/test/kosong/contract/errors.test.ts @@ -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', () => { diff --git a/packages/agent-core-v2/test/kosong/contract/generate.test.ts b/packages/agent-core-v2/test/kosong/contract/generate.test.ts index 70081724c..55c664264 100644 --- a/packages/agent-core-v2/test/kosong/contract/generate.test.ts +++ b/packages/agent-core-v2/test/kosong/contract/generate.test.ts @@ -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', diff --git a/packages/agent-core/src/agent/compaction/full.ts b/packages/agent-core/src/agent/compaction/full.ts index f56c7bc91..a3c65bcb1 100644 --- a/packages/agent-core/src/agent/compaction/full.ts +++ b/packages/agent-core/src/agent/compaction/full.ts @@ -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 diff --git a/packages/agent-core/test/agent/compaction/full.test.ts b/packages/agent-core/test/agent/compaction/full.test.ts index abc4138df..cdc83afb3 100644 --- a/packages/agent-core/test/agent/compaction/full.test.ts +++ b/packages/agent-core/test/agent/compaction/full.test.ts @@ -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(); @@ -2609,7 +2645,10 @@ function textResult(text: string): Awaited> { }; } -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 { for (const part of parts) { yield part; diff --git a/packages/kosong/src/errors.ts b/packages/kosong/src/errors.ts index 1ad2c0074..467a6301f 100644 --- a/packages/kosong/src/errors.ts +++ b/packages/kosong/src/errors.ts @@ -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 diff --git a/packages/kosong/test/errors.test.ts b/packages/kosong/test/errors.test.ts index c2ad36f89..3c75a9305 100644 --- a/packages/kosong/test/errors.test.ts +++ b/packages/kosong/test/errors.test.ts @@ -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); }); diff --git a/packages/kosong/test/generate.test.ts b/packages/kosong/test/generate.test.ts index 4f84b9572..d22bd065e 100644 --- a/packages/kosong/test/generate.test.ts +++ b/packages/kosong/test/generate.test.ts @@ -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...' },