Skip to content
Merged
106 changes: 106 additions & 0 deletions packages/core/src/core/geminiChat.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1520,6 +1520,76 @@ describe('GeminiChat', async () => {
);
});

it('keeps historical image refs stable and reattaches only recent image bytes', async () => {
vi.mocked(mockConfig.getChatCompression).mockReturnValue({
maxRecentImagesToRetain: 1,
});
chat.setHistory([
{
role: 'user',
parts: [{ inlineData: { mimeType: 'image/png', data: 'old-shot' } }],
},
{
role: 'user',
parts: [{ inlineData: { mimeType: 'image/png', data: 'new-shot' } }],
},
]);
const response = (async function* () {
yield {
candidates: [
{
content: {
parts: [{ text: 'response' }],
role: 'model',
},
finishReason: 'STOP',
index: 0,
safetyRatings: [],
},
],
text: () => 'response',
} as unknown as GenerateContentResponse;
})();
vi.mocked(mockContentGenerator.generateContentStream).mockResolvedValue(
response,
);

const stream = await chat.sendMessageStream(
'test-model',
{ message: 'continue' },
'prompt-id-image-refs',
);
for await (const _ of stream) {
// consume stream
}

const request = vi.mocked(mockContentGenerator.generateContentStream).mock
.calls[0]?.[0];
const contents = request?.contents as Content[];
const serialized = JSON.stringify(contents);
expect(serialized).toMatch(
/\[Image #[a-f0-9]{12}: image\/png, \d+ bytes\]/,
);
expect(serialized).not.toContain('"data":"old-shot"');
expect(serialized?.match(/"data":"new-shot"/g)).toHaveLength(1);
expect(contents.at(-1)).toEqual({
role: 'user',
parts: expect.arrayContaining([
{ text: 'continue' },
{
text: expect.stringContaining('Recent images reattached'),
},
{
inlineData: {
mimeType: 'image/png',
data: 'new-shot',
displayName: undefined,
},
},
]),
});
});

it('coalesces startup reminders with the first user prompt for provider requests', async () => {
chat.setHistory([
{
Expand Down Expand Up @@ -7232,6 +7302,42 @@ describe('GeminiChat', async () => {
);
});

it('preserves current user image bytes during output recovery', async () => {
vi.mocked(mockConfig.getChatCompression).mockReturnValue({
maxRecentImagesToRetain: 0,
});
const streams = [
makeStream([makeChunk([{ text: 'initial' }], 'MAX_TOKENS')]),
makeStream([makeChunk([{ text: 'escalated' }], 'MAX_TOKENS')]),
makeStream([makeChunk([{ text: 'done' }], 'STOP')]),
];
let callIndex = 0;
vi.mocked(mockContentGenerator.generateContentStream).mockImplementation(
async () => streams[callIndex++]!,
);

const stream = await chat.sendMessageStream(
'gemini-3-pro',
{
message: [
{ text: 'describe this image' },
{ inlineData: { mimeType: 'image/png', data: 'current-shot' } },
],
},
'prompt-recovery-image',
);

for await (const _event of stream) {
// consume
}

const recoveryRequest = vi.mocked(
mockContentGenerator.generateContentStream,
).mock.calls[2]?.[0];
const serialized = JSON.stringify(recoveryRequest?.contents);
expect(serialized).toContain('"data":"current-shot"');
});

it('should coalesce overlapping recovery continuation text', async () => {
const streams = [
makeStream([makeChunk([{ text: 'discarded initial' }], 'MAX_TOKENS')]),
Expand Down
47 changes: 40 additions & 7 deletions packages/core/src/core/geminiChat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,14 @@ import {
type CompactTrigger,
} from '../services/chatCompressionService.js';
import { acquireSleepInhibitor } from '../services/sleepInhibitor.js';
import { resolveSlimmingConfig } from '../services/compactionInputSlimming.js';
import {
resolveCompactionTuning,
resolveSlimmingConfig,
} from '../services/compactionInputSlimming.js';
import {
InMemoryImagePayloadStore,
prepareImagePayloadsForRequest,
} from '../services/image-payload-references.js';
import {
estimateContentTokens,
estimatePromptTokens,
Expand Down Expand Up @@ -1429,6 +1436,8 @@ export class GeminiChat {
| Parameters<ChatRecordingService['recordAssistantTurn']>[0]
| null = null;

private readonly imagePayloadStore = new InMemoryImagePayloadStore();
Comment thread
LaZzyMan marked this conversation as resolved.

/**
* Monotonically counts user-content pushes that survived into history.
* Incremented when `sendMessageStream` pushes the user content and decremented
Expand Down Expand Up @@ -1491,8 +1500,30 @@ export class GeminiChat {
* Public history readers still use {@link getHistory}, which returns a
* defensive deep copy for caller mutation safety.
*/
private getRequestHistory(): Content[] {
return extractCuratedHistory(this.history).map(copyContentContainer);
private getRequestHistory(currentUserContent?: Content): Content[] {
const curatedHistory = extractCuratedHistory(this.history);
const preserveImagePartsForContentIndex = currentUserContent

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] currentUserContent is looked up by object identity after extractCuratedHistory(), but curation can merge adjacent user turns into a new object. For example, post-compact history can end with a restored-attachments user turn; the next image prompt is merged with it, so findIndex(content === currentUserContent) returns -1. During output recovery a synthetic recovery user turn is appended, and the fallback tail-count preservation applies to that synthetic turn instead of the original multimodal turn. With maxRecentImagesToRetain: 0, the recovery request can replace the user's current image bytes with a text ref and continue without the image.

Please preserve the current image parts by part identity or part range through curation rather than by the Content object identity alone, and add a regression test with adjacent user turns before output recovery.

— gpt-5 via Qwen Code /review

? curatedHistory.findIndex((content) => content === currentUserContent)
: -1;
const requestHistory = curatedHistory.map(copyContentContainer);
const preserveLastUserImagePartCount =
preserveImagePartsForContentIndex === -1
? (currentUserContent?.parts?.length ?? 0)
: 0;
const preserveImagePartsForContentIndexOption =
preserveImagePartsForContentIndex === -1
? undefined
: preserveImagePartsForContentIndex;
const { maxRecentImages } = resolveCompactionTuning(
this.config.getChatCompression(),
);
Comment thread
LaZzyMan marked this conversation as resolved.
return prepareImagePayloadsForRequest(requestHistory, {
maxRecentImages,
preserveImagePartsForContentIndex:
preserveImagePartsForContentIndexOption,
preserveLastUserImagePartCount,
store: this.imagePayloadStore,
});
}

/**
Expand Down Expand Up @@ -1779,7 +1810,7 @@ export class GeminiChat {
parsedEnvMaxTokensForThreshold ??
0)
: Math.max(ESCALATED_MAX_TOKENS, tokenLimit(model, 'output')));

let currentUserContent: Content | undefined;
try {
// The send-lock above is held but the generator's `finally` (which
// resolves it) has not run yet. Any setup error before returning the
Expand Down Expand Up @@ -1953,6 +1984,7 @@ export class GeminiChat {

// Add user content to history ONCE before any attempts.
this.history.push(userContent);
currentUserContent = userContent;
userContentAdded = true;
// Record that the user content landed (see `userContentPushCount`). The
// setup-error path below decrements this if it rolls the push back.
Expand Down Expand Up @@ -1984,7 +2016,7 @@ export class GeminiChat {
.join(', '),
);
}
requestContents = this.getRequestHistory();
requestContents = this.getRequestHistory(currentUserContent);
} catch (error) {
if (userContentAdded) {
this.history.pop();
Expand Down Expand Up @@ -2287,7 +2319,8 @@ export class GeminiChat {
// other retry branches in case a future in-place
// tryCompress stops resetting it.
popPartialIfPushed();
requestContents = self.getRequestHistory();
requestContents =
self.getRequestHistory(currentUserContent);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The latest change threads currentUserContent through the reactive-compression retry path too, but the new regression coverage only exercises output recovery. Reactive compression replaces history via tryCompress() before rebuilding request contents, so a future regression in the fallback or identity behavior could still drop the current image bytes on the retry without a test catching it.

Please add a focused GeminiChat test that sends a current inline image, triggers context-length reactive compression, and asserts the retry request still includes that image with maxRecentImagesToRetain: 0.

— gpt-5 via Qwen Code /review

debugLogger.info(
`Reactive compression succeeded: ` +
`${reactiveInfo.originalTokenCount} -> ` +
Expand Down Expand Up @@ -2530,7 +2563,7 @@ export class GeminiChat {
// model's continuation appends to the previous partial output.
yield { type: StreamEventType.RETRY, isContinuation: true };
// Re-send with the updated history (includes partial + recovery)
const recoveryContents = self.getRequestHistory();
const recoveryContents = self.getRequestHistory(currentUserContent);
escalatedFinishReason = undefined;
try {
const recoveryStream = await self.makeApiCallAndProcessStream(
Expand Down
Loading
Loading