fix(core): bypass routing classifiers to prevent orphaned function response errors (#27389)

This commit is contained in:
Daniel Weis
2026-05-26 12:19:41 -04:00
committed by GitHub
parent 4a5d5cfc9f
commit 85563dabe8
4 changed files with 214 additions and 1 deletions

View File

@@ -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);

View File

@@ -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),
);

View File

@@ -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)' }] },

View File

@@ -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 };