From 85563dabe8df8f771212c31398d332a97ef7970c Mon Sep 17 00:00:00 2001 From: Daniel Weis Date: Tue, 26 May 2026 12:19:41 -0400 Subject: [PATCH] fix(core): bypass routing classifiers to prevent orphaned function response errors (#27389) --- .../strategies/classifierStrategy.test.ts | 91 +++++++++++++++++ .../routing/strategies/classifierStrategy.ts | 12 ++- .../numericalClassifierStrategy.test.ts | 99 +++++++++++++++++++ .../strategies/numericalClassifierStrategy.ts | 13 +++ 4 files changed, 214 insertions(+), 1 deletion(-) diff --git a/packages/core/src/routing/strategies/classifierStrategy.test.ts b/packages/core/src/routing/strategies/classifierStrategy.test.ts index ebae62696b..dbd4b3740a 100644 --- a/packages/core/src/routing/strategies/classifierStrategy.test.ts +++ b/packages/core/src/routing/strategies/classifierStrategy.test.ts @@ -386,6 +386,97 @@ describe('ClassifierStrategy', () => { expect(decision?.model).toBe(DEFAULT_GEMINI_FLASH_MODEL); }); + it('should return null (bypass classifier) if history is only tool turns and request is a function response', async () => { + const history: Content[] = [ + { role: 'model', parts: [{ functionCall: { name: 'tool' } }] }, + { + role: 'user', + parts: [{ functionResponse: { name: 'tool', response: { ok: true } } }], + }, + { role: 'model', parts: [{ functionCall: { name: 'tool2' } }] }, + ]; + mockContext.history = history; + mockContext.request = [ + { functionResponse: { name: 'tool2', response: { ok: true } } }, + ]; + + const decision = await strategy.route( + mockContext, + mockConfig, + mockBaseLlmClient, + mockLocalLiteRtLmClient, + ); + + expect(decision).toBeNull(); + expect(mockBaseLlmClient.generateJson).not.toHaveBeenCalled(); + }); + + it('should return null (bypass classifier) if history has text turns and request is a function response', async () => { + const history: Content[] = [ + { role: 'user', parts: [{ text: 'some task' }] }, + { role: 'model', parts: [{ functionCall: { name: 'tool' } }] }, + ]; + mockContext.history = history; + mockContext.request = [ + { functionResponse: { name: 'tool', response: { ok: true } } }, + ]; + + const decision = await strategy.route( + mockContext, + mockConfig, + mockBaseLlmClient, + mockLocalLiteRtLmClient, + ); + + expect(decision).toBeNull(); + expect(mockBaseLlmClient.generateJson).not.toHaveBeenCalled(); + }); + + it('should still route if history is only tool turns but request is text', async () => { + const history: Content[] = [ + { role: 'model', parts: [{ functionCall: { name: 'tool' } }] }, + { + role: 'user', + parts: [{ functionResponse: { name: 'tool', response: { ok: true } } }], + }, + { role: 'model', parts: [{ functionCall: { name: 'tool2' } }] }, + ]; + mockContext.history = history; + mockContext.request = [{ text: 'simple task' }]; + + const mockApiResponse = { + reasoning: 'Simple.', + model_choice: 'flash', + }; + vi.mocked(mockBaseLlmClient.generateJson).mockResolvedValue( + mockApiResponse, + ); + + const decision = await strategy.route( + mockContext, + mockConfig, + mockBaseLlmClient, + mockLocalLiteRtLmClient, + ); + + expect(decision).not.toBeNull(); + expect(mockBaseLlmClient.generateJson).toHaveBeenCalled(); + + const generateJsonCall = vi.mocked(mockBaseLlmClient.generateJson).mock + .calls[0][0]; + const contents = generateJsonCall.contents; + + // History should be empty because all turns were tool turns and stripped. + // Request should be present. + const expectedContents = [ + { + role: 'user', + parts: [{ text: 'simple task' }], + }, + ]; + expect(contents).toEqual(expectedContents); + }); + describe('Gemini 3.1 and Custom Tools Routing', () => { it('should route to PREVIEW_GEMINI_3_1_MODEL when Gemini 3.1 is launched', async () => { vi.mocked(mockConfig.getGemini31Launched).mockResolvedValue(true); diff --git a/packages/core/src/routing/strategies/classifierStrategy.ts b/packages/core/src/routing/strategies/classifierStrategy.ts index d2edf727dc..c9615a1b2c 100644 --- a/packages/core/src/routing/strategies/classifierStrategy.ts +++ b/packages/core/src/routing/strategies/classifierStrategy.ts @@ -145,12 +145,22 @@ export class ClassifierStrategy implements RoutingStrategy { return null; } + // TODO - Consider using function req/res if they help accuracy. + // Bypass the classifier if the request is a function response. + // Since we prune all tool turns from history, sending a function response + // request would result in an invalid payload (missing the preceding function call). + if (isFunctionResponse(createUserContent(context.request))) { + debugLogger.log( + '[Routing] Bypassing Classifier: request is FunctionResponse.', + ); + return null; + } + const promptId = getPromptIdWithFallback('classifier-router'); const historySlice = context.history.slice(-HISTORY_SEARCH_WINDOW); // Filter out tool-related turns. - // TODO - Consider using function req/res if they help accuracy. const cleanHistory = historySlice.filter( (content) => !isFunctionCall(content) && !isFunctionResponse(content), ); diff --git a/packages/core/src/routing/strategies/numericalClassifierStrategy.test.ts b/packages/core/src/routing/strategies/numericalClassifierStrategy.test.ts index 6b60b04329..cf6c2b2856 100644 --- a/packages/core/src/routing/strategies/numericalClassifierStrategy.test.ts +++ b/packages/core/src/routing/strategies/numericalClassifierStrategy.test.ts @@ -475,6 +475,105 @@ describe('NumericalClassifierStrategy', () => { expect(contents).toEqual(expectedContents); }); + it('should return null (bypass classifier) if history is only tool turns and request is a function response', async () => { + const history: Content[] = [ + { role: 'model', parts: [{ functionCall: { name: 'tool' } }] }, + { + role: 'user', + parts: [{ functionResponse: { name: 'tool', response: { ok: true } } }], + }, + { role: 'model', parts: [{ functionCall: { name: 'tool2' } }] }, + ]; + mockContext.history = history; + mockContext.request = [ + { functionResponse: { name: 'tool2', response: { ok: true } } }, + ]; + + const decision = await strategy.route( + mockContext, + mockConfig, + mockBaseLlmClient, + mockLocalLiteRtLmClient, + ); + + expect(decision).toBeNull(); + expect(mockBaseLlmClient.generateJson).not.toHaveBeenCalled(); + }); + + it('should still route if history is only tool turns but request is text', async () => { + const history: Content[] = [ + { role: 'model', parts: [{ functionCall: { name: 'tool' } }] }, + { + role: 'user', + parts: [{ functionResponse: { name: 'tool', response: { ok: true } } }], + }, + { role: 'model', parts: [{ functionCall: { name: 'tool2' } }] }, + ]; + mockContext.history = history; + mockContext.request = [{ text: 'simple task' }]; + + const mockApiResponse = { + complexity_reasoning: 'Simple.', + complexity_score: 10, + }; + vi.mocked(mockBaseLlmClient.generateJson).mockResolvedValue( + mockApiResponse, + ); + + const decision = await strategy.route( + mockContext, + mockConfig, + mockBaseLlmClient, + mockLocalLiteRtLmClient, + ); + + expect(decision).not.toBeNull(); + expect(mockBaseLlmClient.generateJson).toHaveBeenCalled(); + + const generateJsonCall = vi.mocked(mockBaseLlmClient.generateJson).mock + .calls[0][0]; + const contents = generateJsonCall.contents; + + // History should be empty because all turns were tool turns and stripped. + // Request should be present. + const expectedContents = [ + { + role: 'user', + parts: [{ text: 'simple task' }], + }, + ]; + expect(contents).toEqual(expectedContents); + }); + + it('should still route if history has text turns and request is a function response', async () => { + const history: Content[] = [ + { role: 'user', parts: [{ text: 'some task' }] }, + { role: 'model', parts: [{ functionCall: { name: 'tool' } }] }, + ]; + mockContext.history = history; + mockContext.request = [ + { functionResponse: { name: 'tool', response: { ok: true } } }, + ]; + + const mockApiResponse = { + complexity_reasoning: 'Simple.', + complexity_score: 10, + }; + vi.mocked(mockBaseLlmClient.generateJson).mockResolvedValue( + mockApiResponse, + ); + + const decision = await strategy.route( + mockContext, + mockConfig, + mockBaseLlmClient, + mockLocalLiteRtLmClient, + ); + + expect(decision).not.toBeNull(); + expect(mockBaseLlmClient.generateJson).toHaveBeenCalled(); + }); + it('should preserve tool turns when they appear after a non-tool turn in the middle of history', async () => { const history: Content[] = [ { role: 'user', parts: [{ text: 'turn 0 (before)' }] }, diff --git a/packages/core/src/routing/strategies/numericalClassifierStrategy.ts b/packages/core/src/routing/strategies/numericalClassifierStrategy.ts index bd60f956d7..89fb8069a4 100644 --- a/packages/core/src/routing/strategies/numericalClassifierStrategy.ts +++ b/packages/core/src/routing/strategies/numericalClassifierStrategy.ts @@ -142,6 +142,19 @@ export class NumericalClassifierStrategy implements RoutingStrategy { ? context.request : [context.request]; + // Bypass the classifier if the request is a function response and history is empty. + // Since we prune leading tool turns, if the history becomes empty, sending a + // function response request would result in an invalid payload (starts with function response). + if ( + finalHistory.length === 0 && + isFunctionResponse(createUserContent(context.request)) + ) { + debugLogger.log( + '[Routing] Bypassing NumericalClassifier: request is FunctionResponse but history is empty after slicing.', + ); + return null; + } + const sanitizedRequest = requestParts.map((part) => { if (typeof part === 'string') { return { text: part };