mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-09 17:19:02 +00:00
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 <qwen-code-dev-bot@users.noreply.github.com>
This commit is contained in:
parent
6352d97173
commit
245defbb01
6 changed files with 254 additions and 23 deletions
|
|
@ -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';
|
||||
|
|
|
|||
|
|
@ -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')]),
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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 },
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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([]);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -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<string>();
|
||||
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: {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue