From 245defbb017891c2324a0c411b2dbaabe18b6176 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=A1=BE=E7=9B=BC?= Date: Tue, 7 Jul 2026 10:39:15 +0800 Subject: [PATCH] fix(core): gate image payload replacement behind threshold (#6380) * fix(core): reduce multimodal history payload size * fix(core): use kebab-case image payload filenames * fix(core): address image payload review blockers * fix(core): preserve current request image payloads * ci: disable implicit actionlint pyflakes integration * fix(core): reattach recent unique image payloads * fix(core): preserve referenced image payloads * fix(core): tolerate partial config mocks in MCP discovery * fix(core): preserve current images during recovery * fix(core): gate image payload replacement behind threshold The always-on image payload replacement introduced by PR #6045 replaced ALL historical images with text references on every request, causing users' old screenshots to be reattached and triggering infinite fix loops when the model mistook stale buggy screenshots for current state. Replace the always-on approach with a threshold-gated design: - Below 20 images (configurable): zero transformation, images stay in-place in history - At or above 20: in-place replace historical images with text references, reattach only the most recent 3 unique images - Replacement is persistent (mutates this.history), so the count resets and won't re-trigger until 20 new images accumulate - Current user request images are protected via skipContent Also lower DEFAULT_SCREENSHOT_TRIGGER_THRESHOLD from 50 to 20 to align with the new image payload threshold. --------- Co-authored-by: qwen-code-dev-bot --- packages/core/src/config/config.ts | 9 +- packages/core/src/core/geminiChat.test.ts | 15 +++ packages/core/src/core/geminiChat.ts | 57 +++++++----- .../src/services/compactionInputSlimming.ts | 15 ++- .../services/image-payload-references.test.ts | 89 ++++++++++++++++++ .../src/services/image-payload-references.ts | 92 +++++++++++++++++++ 6 files changed, 254 insertions(+), 23 deletions(-) diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index f0c4e19b8b..02426bebb6 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -412,10 +412,17 @@ export interface ChatCompressionSettings { enableScreenshotTrigger?: boolean; /** * Tool-returned image count at or above which the screenshot trigger - * fires (only when `enableScreenshotTrigger`). Default 50. + * fires (only when `enableScreenshotTrigger`). Default 20. * Env override: `QWEN_COMPACT_SCREENSHOT_THRESHOLD`. */ screenshotTriggerThreshold?: number; + /** + * Inline image count at or above which historical image payloads are + * replaced with text references and only recent images are reattached. + * Below this threshold images stay in-place untouched. Default 20. + * Env override: `QWEN_IMAGE_PAYLOAD_THRESHOLD`. + */ + imagePayloadThreshold?: number; } export { DEFAULT_TOOL_RESULTS_TOTAL_CHARS_THRESHOLD } from './clearContextDefaults.js'; diff --git a/packages/core/src/core/geminiChat.test.ts b/packages/core/src/core/geminiChat.test.ts index d83b30bc53..5c0558ec0b 100644 --- a/packages/core/src/core/geminiChat.test.ts +++ b/packages/core/src/core/geminiChat.test.ts @@ -1524,16 +1524,27 @@ describe('GeminiChat', async () => { it('keeps historical image refs stable and reattaches only recent image bytes', async () => { vi.mocked(mockConfig.getChatCompression).mockReturnValue({ maxRecentImagesToRetain: 1, + imagePayloadThreshold: 1, + }); chat.setHistory([ { role: 'user', parts: [{ inlineData: { mimeType: 'image/png', data: 'old-shot' } }], }, + { + role: 'model', + parts: [{ text: 'I see the first image' }], + }, { role: 'user', parts: [{ inlineData: { mimeType: 'image/png', data: 'new-shot' } }], }, + { + role: 'model', + parts: [{ text: 'I see the second image' }], + }, + ]); const response = (async function* () { yield { @@ -8051,6 +8062,8 @@ describe('GeminiChat', async () => { it('preserves current user image bytes during output recovery', async () => { vi.mocked(mockConfig.getChatCompression).mockReturnValue({ maxRecentImagesToRetain: 0, + imagePayloadThreshold: 1, + }); const streams = [ makeStream([makeChunk([{ text: 'initial' }], 'MAX_TOKENS')]), @@ -8064,6 +8077,7 @@ describe('GeminiChat', async () => { const stream = await chat.sendMessageStream( 'gemini-pro', + { message: [ { text: 'describe this image' }, @@ -8126,6 +8140,7 @@ describe('GeminiChat', async () => { expect(text).toBe('Hello ending.'); }); + it('should coalesce overlapping recovery continuation text', async () => { const streams = [ makeStream([makeChunk([{ text: 'discarded initial' }], 'MAX_TOKENS')]), diff --git a/packages/core/src/core/geminiChat.ts b/packages/core/src/core/geminiChat.ts index 060fc6000a..3a60aef730 100644 --- a/packages/core/src/core/geminiChat.ts +++ b/packages/core/src/core/geminiChat.ts @@ -70,7 +70,10 @@ import { } from '../services/compactionInputSlimming.js'; import { InMemoryImagePayloadStore, - prepareImagePayloadsForRequest, + buildReattachParts, + countAllInlineImages, + replaceImagePayloadsInPlace, + } from '../services/image-payload-references.js'; import { estimateContentTokens, @@ -1543,28 +1546,37 @@ export class GeminiChat { */ private getRequestHistory(currentUserContent?: Content): Content[] { const curatedHistory = extractCuratedHistory(this.history); - const preserveImagePartsForContentIndex = currentUserContent - ? 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( + const { maxRecentImages, imagePayloadThreshold } = resolveCompactionTuning( this.config.getChatCompression(), ); - return prepareImagePayloadsForRequest(requestHistory, { - maxRecentImages, - preserveImagePartsForContentIndex: - preserveImagePartsForContentIndexOption, - preserveLastUserImagePartCount, - store: this.imagePayloadStore, - }); + if (countAllInlineImages(curatedHistory) >= imagePayloadThreshold) { + const skipEntry = currentUserContent + ? curatedHistory.find( + (c) => + c === currentUserContent || + (c.role === 'user' && + currentUserContent.parts?.some((p) => c.parts?.includes(p))), + ) + : undefined; + const replaced = replaceImagePayloadsInPlace( + curatedHistory, + this.imagePayloadStore, + skipEntry, + ); + const requestHistory = curatedHistory.map(copyContentContainer); + const reattachParts = buildReattachParts(replaced, maxRecentImages); + if (reattachParts.length > 0) { + const last = requestHistory.at(-1); + if (last?.role === 'user') { + last.parts = [...(last.parts ?? []), ...reattachParts]; + } else { + requestHistory.push({ role: 'user', parts: reattachParts }); + } + } + return requestHistory; + } + return curatedHistory.map(copyContentContainer); + } /** @@ -1857,6 +1869,7 @@ export class GeminiChat { model, cgConfigForThresholds?.contextWindowSize, )); + let currentUserContent: Content | undefined; try { // The send-lock above is held but the generator's `finally` (which @@ -2320,6 +2333,7 @@ export class GeminiChat { // other retry branches in case a future in-place // tryCompress stops resetting it. self.popPendingPartialAssistantTurn(); + requestContents = self.getRequestHistory(currentUserContent); debugLogger.info( @@ -2583,6 +2597,7 @@ export class GeminiChat { // Re-send with the updated history (includes partial + recovery) const recoveryContents = self.getRequestHistory(currentUserContent); recoveryFinishReason = undefined; + try { const recoveryStream = await self.makeApiCallAndProcessStream( model, diff --git a/packages/core/src/services/compactionInputSlimming.ts b/packages/core/src/services/compactionInputSlimming.ts index 52b44098e9..8a1dae32d7 100644 --- a/packages/core/src/services/compactionInputSlimming.ts +++ b/packages/core/src/services/compactionInputSlimming.ts @@ -111,7 +111,8 @@ function resolveNumber( export const DEFAULT_MAX_RECENT_FILES = 5; export const DEFAULT_MAX_RECENT_IMAGES = 3; export const DEFAULT_SCREENSHOT_TRIGGER_ENABLED = true; -export const DEFAULT_SCREENSHOT_TRIGGER_THRESHOLD = 50; +export const DEFAULT_SCREENSHOT_TRIGGER_THRESHOLD = 20; +export const DEFAULT_IMAGE_PAYLOAD_THRESHOLD = 20; export interface ResolvedCompactionTuning { /** Recent files restored after compaction (0 = restore none). */ @@ -122,6 +123,12 @@ export interface ResolvedCompactionTuning { enableScreenshotTrigger: boolean; /** Tool-image count at or above which the trigger fires (≥ 1). */ screenshotTriggerThreshold: number; + /** + * Inline image count at or above which historical image payloads + * are replaced with text references and only recent images are + * reattached. Below this threshold images stay in-place untouched. + */ + imagePayloadThreshold: number; } /** @@ -164,6 +171,12 @@ export function resolveCompactionTuning( DEFAULT_SCREENSHOT_TRIGGER_THRESHOLD, { integer: true, minInclusive: 1 }, ), + imagePayloadThreshold: resolveNumber( + process.env['QWEN_IMAGE_PAYLOAD_THRESHOLD'], + settings?.imagePayloadThreshold, + DEFAULT_IMAGE_PAYLOAD_THRESHOLD, + { integer: true, minInclusive: 1 }, + ), }; } diff --git a/packages/core/src/services/image-payload-references.test.ts b/packages/core/src/services/image-payload-references.test.ts index f84d675b57..3f365f09db 100644 --- a/packages/core/src/services/image-payload-references.test.ts +++ b/packages/core/src/services/image-payload-references.test.ts @@ -8,7 +8,11 @@ import type { Content, Part } from '@google/genai'; import { describe, expect, it } from 'vitest'; import { InMemoryImagePayloadStore, + buildReattachParts, + countAllInlineImages, prepareImagePayloadsForRequest, + replaceImagePayloadsInPlace, + } from './image-payload-references.js'; function toolImageTurn(data: string): Content { @@ -210,3 +214,88 @@ describe('prepareImagePayloadsForRequest', () => { expect(serialized).not.toContain('ignore all prior instructions'); }); }); + +describe('countAllInlineImages', () => { + it('counts top-level and tool-nested images', () => { + const contents: Content[] = [ + { + role: 'user', + parts: [ + { inlineData: { mimeType: 'image/png', data: 'user-shot' } }, + { text: 'look at this' }, + ], + }, + toolImageTurn('tool-shot-1'), + toolImageTurn('tool-shot-2'), + { role: 'model', parts: [{ text: 'ok' }] }, + ]; + expect(countAllInlineImages(contents)).toBe(3); + }); + + it('returns zero for text-only history', () => { + expect( + countAllInlineImages([ + { role: 'user', parts: [{ text: 'hello' }] }, + { role: 'model', parts: [{ text: 'hi' }] }, + ]), + ).toBe(0); + }); +}); + +describe('replaceImagePayloadsInPlace', () => { + it('mutates contents in-place and returns replaced payloads', () => { + const store = new InMemoryImagePayloadStore(); + const contents: Content[] = [ + toolImageTurn('shot-a'), + toolImageTurn('shot-b'), + { role: 'user', parts: [{ text: 'continue' }] }, + ]; + const replaced = replaceImagePayloadsInPlace(contents, store); + expect(replaced).toHaveLength(2); + expect(countAllInlineImages(contents)).toBe(0); + const serialized = JSON.stringify(contents); + expect(serialized).toMatch( + /\[Image #[a-f0-9]{12}: image\/png, \d+ bytes\]/, + ); + }); + + it('skips the specified content entry', () => { + const store = new InMemoryImagePayloadStore(); + const current: Content = { + role: 'user', + parts: [{ inlineData: { mimeType: 'image/png', data: 'current-shot' } }], + }; + const contents: Content[] = [toolImageTurn('old-shot'), current]; + replaceImagePayloadsInPlace(contents, store, current); + expect(countAllInlineImages(contents)).toBe(1); + expect(JSON.stringify(contents)).toContain('"data":"current-shot"'); + expect(JSON.stringify(contents)).not.toContain('"data":"old-shot"'); + }); +}); + +describe('buildReattachParts', () => { + it('picks the most recent unique images', () => { + const store = new InMemoryImagePayloadStore(); + const contents: Content[] = [ + toolImageTurn('a'), + toolImageTurn('b'), + toolImageTurn('c'), + toolImageTurn('c'), + ]; + const replaced = replaceImagePayloadsInPlace(contents, store); + const parts = buildReattachParts(replaced, 2); + expect(parts).toHaveLength(3); + expect(parts[0]?.text).toContain('Recent images reattached'); + const data = parts + .filter((p) => p.inlineData) + .map((p) => p.inlineData?.data); + expect(data).toEqual(['b', 'c']); + }); + + it('returns empty when maxRecentImages is zero', () => { + const store = new InMemoryImagePayloadStore(); + const replaced = replaceImagePayloadsInPlace([toolImageTurn('a')], store); + expect(buildReattachParts(replaced, 0)).toEqual([]); + }); +}); + diff --git a/packages/core/src/services/image-payload-references.ts b/packages/core/src/services/image-payload-references.ts index 91d86c1530..ac9d7ccfa8 100644 --- a/packages/core/src/services/image-payload-references.ts +++ b/packages/core/src/services/image-payload-references.ts @@ -47,6 +47,98 @@ export class InMemoryImagePayloadStore implements ImagePayloadStore { } } +export function countAllInlineImages(contents: Content[]): number { + let count = 0; + for (const content of contents) { + for (const part of content.parts ?? []) { + if (part.inlineData?.mimeType?.startsWith('image/')) count++; + const nested = getFunctionResponseParts(part); + if (!nested) continue; + for (const inner of nested) { + if (inner.inlineData?.mimeType?.startsWith('image/')) count++; + } + } + } + return count; +} + +/** + * Replace image payloads in-place with text references, storing the + * originals in the provided store. This mutates the history so that + * subsequent `countAllInlineImages` returns a lower count. + * + * Returns the stored payloads in order of appearance for downstream + * reattach decisions. + */ +export function replaceImagePayloadsInPlace( + contents: Content[], + store: ImagePayloadStore, + skipContent?: Content, +): StoredImagePayload[] { + const replaced: StoredImagePayload[] = []; + for (const content of contents) { + if (content === skipContent) continue; + if (!content.parts) continue; + for (let i = 0; i < content.parts.length; i++) { + const part = content.parts[i]!; + if ( + part.inlineData?.mimeType?.startsWith('image/') && + part.inlineData.data + ) { + const stored = store.put(part); + replaced.push(stored); + content.parts[i] = { text: imageReferenceText(stored) }; + continue; + } + const nested = getFunctionResponseParts(part); + if (!nested) continue; + for (let j = 0; j < nested.length; j++) { + const inner = nested[j]!; + if ( + inner.inlineData?.mimeType?.startsWith('image/') && + inner.inlineData.data + ) { + const stored = store.put(inner); + replaced.push(stored); + nested[j] = { text: imageReferenceText(stored) }; + } + } + } + } + return replaced; +} + +/** + * Build the reattach parts for the most recent unique images from a + * replacement pass. Used after `replaceImagePayloadsInPlace` to append + * recent image bytes to the outgoing request. + */ +export function buildReattachParts( + replaced: StoredImagePayload[], + maxRecentImages: number, +): Part[] { + if (maxRecentImages <= 0 || replaced.length === 0) return []; + const recent: StoredImagePayload[] = []; + const seen = new Set(); + for (let i = replaced.length - 1; i >= 0; i--) { + const img = replaced[i]!; + if (seen.has(img.id)) continue; + seen.add(img.id); + recent.push(img); + if (recent.length === maxRecentImages) break; + } + recent.reverse(); + return [ + { + text: + 'Recent images reattached for visual context: ' + + recent.map((img) => `Image #${img.id}`).join(', '), + }, + ...recent.map(storedImageToPart), + ]; +} + + export function prepareImagePayloadsForRequest( contents: Content[], options: {