Remove unused code. (#8497)

This commit is contained in:
Tommaso Sciortino
2025-09-16 12:12:45 -07:00
committed by GitHub
parent 986b9fe7e9
commit 4e5c1fce8d
2 changed files with 29 additions and 299 deletions

View File

@@ -8,7 +8,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import type {
Content,
GenerateContentConfig,
Part,
GenerateContentResponse,
} from '@google/genai';
import type { ContentGenerator } from '../core/contentGenerator.js';
@@ -286,67 +285,6 @@ describe('GeminiChat', () => {
expect(modelTurn?.parts?.length).toBe(1);
expect(modelTurn?.parts![0]!.text).toBe('Initial valid content...');
});
it('should not consolidate text into a part that also contains a functionCall', async () => {
// 1. Mock the API to stream a malformed part followed by a valid text part.
const multiChunkStream = (async function* () {
// This malformed part has both text and a functionCall.
yield {
candidates: [
{
content: {
role: 'model',
parts: [
{
text: 'Some text',
functionCall: { name: 'do_stuff', args: {} },
},
],
},
},
],
} as unknown as GenerateContentResponse;
// This valid text part should NOT be merged into the malformed one.
yield {
candidates: [
{
content: {
role: 'model',
parts: [{ text: ' that should not be merged.' }],
},
},
],
} as unknown as GenerateContentResponse;
})();
vi.mocked(mockContentGenerator.generateContentStream).mockResolvedValue(
multiChunkStream,
);
// 2. Action: Send a message and consume the stream.
const stream = await chat.sendMessageStream(
'test-model',
{ message: 'test message' },
'prompt-id-malformed-chunk',
);
for await (const _ of stream) {
// Consume the stream to trigger history recording.
}
// 3. Assert: Check that the final history was not incorrectly consolidated.
const history = chat.getHistory();
expect(history.length).toBe(2);
const modelTurn = history[1]!;
// CRUCIAL ASSERTION: There should be two separate parts.
// The old, non-strict logic would incorrectly merge them, resulting in one part.
expect(modelTurn?.parts?.length).toBe(2);
// Verify the contents of each part.
expect(modelTurn?.parts![0]!.text).toBe('Some text');
expect(modelTurn?.parts![0]!.functionCall).toBeDefined();
expect(modelTurn?.parts![1]!.text).toBe(' that should not be merged.');
});
it('should consolidate subsequent text chunks after receiving an empty text chunk', async () => {
// 1. Mock the API to return a stream where one chunk is just an empty text part.
@@ -620,151 +558,6 @@ describe('GeminiChat', () => {
});
});
describe('recordHistory', () => {
const userInput: Content = {
role: 'user',
parts: [{ text: 'User input' }],
};
it('should consolidate all consecutive model turns into a single turn', () => {
const userInput: Content = {
role: 'user',
parts: [{ text: 'User input' }],
};
// This simulates a multi-part model response with different part types.
const modelOutput: Content[] = [
{ role: 'model', parts: [{ text: 'Thinking...' }] },
{
role: 'model',
parts: [{ functionCall: { name: 'do_stuff', args: {} } }],
},
];
// @ts-expect-error Accessing private method for testing
chat.recordHistory(userInput, modelOutput);
const history = chat.getHistory();
// The history should contain the user's turn and ONE consolidated model turn.
// The old code would fail here, resulting in a length of 3.
//expect(history).toBe([]);
expect(history.length).toBe(2);
const modelTurn = history[1]!;
expect(modelTurn.role).toBe('model');
// The consolidated turn should contain both the text part and the functionCall part.
expect(modelTurn?.parts?.length).toBe(2);
expect(modelTurn?.parts![0]!.text).toBe('Thinking...');
expect(modelTurn?.parts![1]!.functionCall).toBeDefined();
});
it('should add a placeholder model turn when a tool call is followed by an empty response', () => {
// 1. Setup: A history where the model has just made a function call.
const initialHistory: Content[] = [
{ role: 'user', parts: [{ text: 'Initial prompt' }] },
{
role: 'model',
parts: [{ functionCall: { name: 'test_tool', args: {} } }],
},
];
chat.setHistory(initialHistory);
// 2. Action: The user provides the tool's response, and the model's
// final output is empty (e.g., just a thought, which gets filtered out).
const functionResponse: Content = {
role: 'user',
parts: [{ functionResponse: { name: 'test_tool', response: {} } }],
};
const emptyModelOutput: Content[] = [];
// @ts-expect-error Accessing private method for testing
chat.recordHistory(functionResponse, emptyModelOutput, [
functionResponse,
]);
// 3. Assert: The history should now have four valid, alternating turns.
const history = chat.getHistory();
expect(history.length).toBe(4);
// The final turn must be the empty model placeholder.
const lastTurn = history[3]!;
expect(lastTurn.role).toBe('model');
expect(lastTurn?.parts?.length).toBe(0);
// The second-to-last turn must be the function response we provided.
const secondToLastTurn = history[2]!;
expect(secondToLastTurn.role).toBe('user');
expect(secondToLastTurn?.parts![0]!.functionResponse).toBeDefined();
});
it('should add user input and a single model output to history', () => {
const modelOutput: Content[] = [
{ role: 'model', parts: [{ text: 'Model output' }] },
];
// @ts-expect-error Accessing private method for testing
chat.recordHistory(userInput, modelOutput);
const history = chat.getHistory();
expect(history.length).toBe(2);
expect(history[0]).toEqual(userInput);
expect(history[1]).toEqual(modelOutput[0]);
});
it('should consolidate adjacent text parts from multiple content objects', () => {
const modelOutput: Content[] = [
{ role: 'model', parts: [{ text: 'Part 1.' }] },
{ role: 'model', parts: [{ text: ' Part 2.' }] },
{ role: 'model', parts: [{ text: ' Part 3.' }] },
];
// @ts-expect-error Accessing private method for testing
chat.recordHistory(userInput, modelOutput);
const history = chat.getHistory();
expect(history.length).toBe(2);
expect(history[1]!.role).toBe('model');
expect(history[1]!.parts).toEqual([{ text: 'Part 1. Part 2. Part 3.' }]);
});
it('should add an empty placeholder turn if modelOutput is empty', () => {
// This simulates receiving a pre-filtered, thought-only response.
const emptyModelOutput: Content[] = [];
// @ts-expect-error Accessing private method for testing
chat.recordHistory(userInput, emptyModelOutput);
const history = chat.getHistory();
expect(history.length).toBe(2);
expect(history[0]).toEqual(userInput);
expect(history[1]!.role).toBe('model');
expect(history[1]!.parts).toEqual([]);
});
it('should preserve model outputs with undefined or empty parts arrays', () => {
const malformedOutput: Content[] = [
{ role: 'model', parts: [{ text: 'Text part' }] },
{ role: 'model', parts: undefined as unknown as Part[] },
{ role: 'model', parts: [] },
];
// @ts-expect-error Accessing private method for testing
chat.recordHistory(userInput, malformedOutput);
const history = chat.getHistory();
expect(history.length).toBe(4); // userInput + 3 model turns
expect(history[1]!.parts).toEqual([{ text: 'Text part' }]);
expect(history[2]!.parts).toBeUndefined();
expect(history[3]!.parts).toEqual([]);
});
it('should not consolidate content with different roles', () => {
const mixedOutput: Content[] = [
{ role: 'model', parts: [{ text: 'Model 1' }] },
{ role: 'user', parts: [{ text: 'Unexpected User' }] },
{ role: 'model', parts: [{ text: 'Model 2' }] },
];
// @ts-expect-error Accessing private method for testing
chat.recordHistory(userInput, mixedOutput);
const history = chat.getHistory();
expect(history.length).toBe(4); // userInput, model1, unexpected_user, model2
expect(history[1]).toEqual(mixedOutput[0]);
expect(history[2]).toEqual(mixedOutput[1]);
expect(history[3]).toEqual(mixedOutput[2]);
});
});
describe('addHistory', () => {
it('should add a new content item to the history', () => {
const newContent: Content = {

View File

@@ -81,6 +81,19 @@ function isValidResponse(response: GenerateContentResponse): boolean {
return isValidContent(content);
}
export function isValidNonThoughtTextPart(part: Part): boolean {
return (
typeof part.text === 'string' &&
!part.thought &&
// Technically, the model should never generate parts that have text and
// any of these but we don't trust them so check anyways.
!part.functionCall &&
!part.functionResponse &&
!part.inlineData &&
!part.fileData
);
}
function isValidContent(content: Content): boolean {
if (content.parts === undefined || content.parts.length === 0) {
return false;
@@ -261,7 +274,6 @@ export class GeminiChat {
requestContents,
params,
prompt_id,
userContent,
);
for await (const chunk of stream) {
@@ -326,7 +338,6 @@ export class GeminiChat {
requestContents: Content[],
params: SendMessageParameters,
prompt_id: string,
userContent: Content,
): Promise<AsyncGenerator<GenerateContentResponse>> {
const apiCall = () => {
const modelToUse = getEffectiveModel(
@@ -371,7 +382,7 @@ export class GeminiChat {
authType: this.config.getContentGeneratorConfig()?.authType,
});
return this.processStreamResponse(model, streamResponse, userContent);
return this.processStreamResponse(model, streamResponse);
}
/**
@@ -476,7 +487,6 @@ export class GeminiChat {
private async *processStreamResponse(
model: string,
streamResponse: AsyncGenerator<GenerateContentResponse>,
userInput: Content,
): AsyncGenerator<GenerateContentResponse> {
const modelResponseParts: Part[] = [];
let hasReceivedAnyChunk = false;
@@ -501,8 +511,10 @@ export class GeminiChat {
if (content.parts.some((part) => part.functionCall)) {
hasToolCall = true;
}
// Always add parts - thoughts will be filtered out later in recordHistory
modelResponseParts.push(...content.parts);
modelResponseParts.push(
...content.parts.filter((part) => !part.thought),
);
}
} else {
logInvalidChunk(
@@ -547,7 +559,7 @@ export class GeminiChat {
// Record model response text from the collected parts
if (modelResponseParts.length > 0) {
const responseText = modelResponseParts
.filter((part) => part.text && !part.thought)
.filter((part) => part.text)
.map((part) => part.text)
.join('');
@@ -560,97 +572,22 @@ export class GeminiChat {
}
}
// Bundle all streamed parts into a single Content object
const modelOutput: Content[] =
modelResponseParts.length > 0
? [{ role: 'model', parts: modelResponseParts }]
: [];
// Pass the raw, bundled data to the new, robust recordHistory
this.recordHistory(userInput, modelOutput);
}
private recordHistory(userInput: Content, modelOutput: Content[]) {
// Part 1: Handle the user's turn.
const lastTurn = this.history[this.history.length - 1];
// The only time we don't push is if it's the *exact same* object,
// which happens in streaming where we add it preemptively.
if (lastTurn !== userInput) {
if (lastTurn?.role === 'user') {
// This is an invalid sequence.
throw new Error('Cannot add a user turn after another user turn.');
}
this.history.push(userInput);
}
// Part 2: Process the model output into a final, consolidated list of turns.
const finalModelTurns: Content[] = [];
for (const content of modelOutput) {
// A. Preserve malformed content that has no 'parts' array.
if (!content.parts) {
finalModelTurns.push(content);
continue;
}
// B. Filter out 'thought' parts.
const visibleParts = content.parts.filter((part) => !part.thought);
const newTurn = { ...content, parts: visibleParts };
const lastTurnInFinal = finalModelTurns[finalModelTurns.length - 1];
// Consolidate this new turn with the PREVIOUS turn if they are adjacent model turns.
// String thoughts and consolidate text parts.
const consolidatedParts: Part[] = [];
for (const part of modelResponseParts) {
const lastPart = consolidatedParts[consolidatedParts.length - 1];
if (
lastTurnInFinal &&
lastTurnInFinal.role === 'model' &&
newTurn.role === 'model' &&
lastTurnInFinal.parts && // SAFETY CHECK: Ensure the destination has a parts array.
newTurn.parts
lastPart?.text &&
isValidNonThoughtTextPart(lastPart) &&
isValidNonThoughtTextPart(part)
) {
lastTurnInFinal.parts.push(...newTurn.parts);
lastPart.text += part.text;
} else {
finalModelTurns.push(newTurn);
consolidatedParts.push(part);
}
}
// Part 3: Add the processed model turns to the history, with one final consolidation pass.
if (finalModelTurns.length > 0) {
// Re-consolidate parts within any turns that were merged in the previous step.
for (const turn of finalModelTurns) {
if (turn.parts && turn.parts.length > 1) {
const consolidatedParts: Part[] = [];
for (const part of turn.parts) {
const lastPart = consolidatedParts[consolidatedParts.length - 1];
if (
lastPart &&
// Ensure lastPart is a pure text part
typeof lastPart.text === 'string' &&
!lastPart.functionCall &&
!lastPart.functionResponse &&
!lastPart.inlineData &&
!lastPart.fileData &&
!lastPart.thought &&
// Ensure current part is a pure text part
typeof part.text === 'string' &&
!part.functionCall &&
!part.functionResponse &&
!part.inlineData &&
!part.fileData &&
!part.thought
) {
lastPart.text += part.text;
} else {
consolidatedParts.push({ ...part });
}
}
turn.parts = consolidatedParts;
}
}
this.history.push(...finalModelTurns);
} else {
// If, after all processing, there's NO model output, add the placeholder.
this.history.push({ role: 'model', parts: [] });
}
this.history.push({ role: 'model', parts: consolidatedParts });
}
/**