Skip to content

Commit 70b9d1d

Browse files
committed
fix(core): preserve structured thoughts during session resume (#24106)
This change ensures that AI thinking blocks are correctly handled during session serialization and deserialization, preventing string contamination like '[Thought: true]'. By removing unnecessary transformations in the API conversion layer, we maintain the structured 'thought: true' property, which is supported by the CountToken and other generation APIs. Verified by running Gemini CLI with real models and added a new unit test to prevent regressions. Fixes #23046 Partial revert of fef89f5 (#6859)
1 parent 07ab16d commit 70b9d1d

3 files changed

Lines changed: 57 additions & 124 deletions

File tree

packages/core/src/code_assist/converter.test.ts

Lines changed: 0 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import {
1717
BlockedReason,
1818
type ContentListUnion,
1919
type GenerateContentParameters,
20-
type Part,
2120
} from '@google/genai';
2221

2322
describe('converter', () => {
@@ -392,94 +391,5 @@ describe('converter', () => {
392391
{ role: 'user', parts: [{ text: 'string 2' }] },
393392
]);
394393
});
395-
396-
it('should convert thought parts to text parts for API compatibility', () => {
397-
const contentWithThought: ContentListUnion = {
398-
role: 'model',
399-
parts: [
400-
{ text: 'regular text' },
401-
{ thought: 'thinking about the problem' } as Part & {
402-
thought: string;
403-
},
404-
{ text: 'more text' },
405-
],
406-
};
407-
expect(toContents(contentWithThought)).toEqual([
408-
{
409-
role: 'model',
410-
parts: [
411-
{ text: 'regular text' },
412-
{ text: '[Thought: thinking about the problem]' },
413-
{ text: 'more text' },
414-
],
415-
},
416-
]);
417-
});
418-
419-
it('should combine text and thought for text parts with thoughts', () => {
420-
const contentWithTextAndThought: ContentListUnion = {
421-
role: 'model',
422-
parts: [
423-
{
424-
text: 'Here is my response',
425-
thought: 'I need to be careful here',
426-
} as Part & { thought: string },
427-
],
428-
};
429-
expect(toContents(contentWithTextAndThought)).toEqual([
430-
{
431-
role: 'model',
432-
parts: [
433-
{
434-
text: 'Here is my response\n[Thought: I need to be careful here]',
435-
},
436-
],
437-
},
438-
]);
439-
});
440-
441-
it('should preserve non-thought properties while removing thought', () => {
442-
const contentWithComplexPart: ContentListUnion = {
443-
role: 'model',
444-
parts: [
445-
{
446-
functionCall: { name: 'calculate', args: { x: 5, y: 10 } },
447-
thought: 'Performing calculation',
448-
} as Part & { thought: string },
449-
],
450-
};
451-
expect(toContents(contentWithComplexPart)).toEqual([
452-
{
453-
role: 'model',
454-
parts: [
455-
{
456-
functionCall: { name: 'calculate', args: { x: 5, y: 10 } },
457-
},
458-
],
459-
},
460-
]);
461-
});
462-
463-
it('should convert invalid text content to valid text part with thought', () => {
464-
const contentWithInvalidText: ContentListUnion = {
465-
role: 'model',
466-
parts: [
467-
{
468-
text: 123, // Invalid - should be string
469-
thought: 'Processing number',
470-
} as Part & { thought: string; text: number },
471-
],
472-
};
473-
expect(toContents(contentWithInvalidText)).toEqual([
474-
{
475-
role: 'model',
476-
parts: [
477-
{
478-
text: '123\n[Thought: Processing number]',
479-
},
480-
],
481-
},
482-
]);
483-
});
484394
});
485395
});

packages/core/src/code_assist/converter.ts

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -244,40 +244,6 @@ function toPart(part: PartUnion): Part {
244244
return { text: part };
245245
}
246246

247-
// Handle thought parts for CountToken API compatibility
248-
// The CountToken API expects parts to have certain required "oneof" fields initialized,
249-
// but thought parts don't conform to this schema and cause API failures
250-
if ('thought' in part && part.thought) {
251-
const thoughtText = `[Thought: ${part.thought}]`;
252-
253-
const newPart = { ...part };
254-
delete (newPart as Record<string, unknown>)['thought'];
255-
256-
const hasApiContent =
257-
'functionCall' in newPart ||
258-
'functionResponse' in newPart ||
259-
'inlineData' in newPart ||
260-
'fileData' in newPart;
261-
262-
if (hasApiContent) {
263-
// It's a functionCall or other non-text part. Just strip the thought.
264-
return newPart;
265-
}
266-
267-
// If no other valid API content, this must be a text part.
268-
// Combine existing text (if any) with the thought, preserving other properties.
269-
const text = (newPart as { text?: unknown }).text;
270-
const existingText = text ? String(text) : '';
271-
const combinedText = existingText
272-
? `${existingText}\n${thoughtText}`
273-
: thoughtText;
274-
275-
return {
276-
...newPart,
277-
text: combinedText,
278-
};
279-
}
280-
281247
return part;
282248
}
283249

packages/core/src/utils/sessionUtils.test.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { describe, it, expect } from 'vitest';
77
import { convertSessionToClientHistory } from './sessionUtils.js';
88
import { type ConversationRecord } from '../services/chatRecordingService.js';
99
import { CoreToolCallStatus } from '../scheduler/types.js';
10+
import { toContents } from '../code_assist/converter.js';
1011

1112
describe('convertSessionToClientHistory', () => {
1213
it('should convert a simple conversation without tool calls', () => {
@@ -182,4 +183,60 @@ describe('convertSessionToClientHistory', () => {
182183
},
183184
]);
184185
});
186+
187+
it('integration: should ensure thoughts survive conversion back to API format without string mutation on session resume', () => {
188+
// 1. A simulated conversation pulled from storage (like session resume)
189+
const messages: ConversationRecord['messages'] = [
190+
{
191+
id: '1',
192+
type: 'user',
193+
timestamp: '2024-01-01T10:00:00Z',
194+
content: 'Hello, can you help me?',
195+
},
196+
{
197+
id: '2',
198+
type: 'gemini',
199+
timestamp: '2024-01-01T10:01:00Z',
200+
content: 'AI Response',
201+
thoughts: [
202+
{
203+
subject: 'Thinking',
204+
description: 'I need to process this',
205+
timestamp: '2024-01-01T10:00:50Z',
206+
},
207+
],
208+
},
209+
{
210+
id: '3',
211+
type: 'user',
212+
timestamp: '2024-01-01T10:02:00Z',
213+
content: 'This is the first message after session resume.',
214+
},
215+
];
216+
217+
// 2. convertSessionToClientHistory (what GeminiChat uses to load history internally)
218+
const history = convertSessionToClientHistory(messages);
219+
220+
// 3. toContents (what the CodeAssist API / external AI provider uses internally to format network requests)
221+
const generatedContents = toContents(history);
222+
223+
// 4. Assert that we don't accidentally mutate or inject '[Thought: true]' as string text.
224+
expect(generatedContents).toEqual([
225+
{
226+
role: 'user',
227+
parts: [{ text: 'Hello, can you help me?' }],
228+
},
229+
{
230+
role: 'model',
231+
parts: [
232+
{ text: '**Thinking** I need to process this', thought: true },
233+
{ text: 'AI Response' },
234+
],
235+
},
236+
{
237+
role: 'user',
238+
parts: [{ text: 'This is the first message after session resume.' }],
239+
},
240+
]);
241+
});
185242
});

0 commit comments

Comments
 (0)