diff --git a/open-sse/services/compression/resultMemo.ts b/open-sse/services/compression/resultMemo.ts index 283733a31f..d763233bf5 100644 --- a/open-sse/services/compression/resultMemo.ts +++ b/open-sse/services/compression/resultMemo.ts @@ -41,22 +41,48 @@ export function makeMemoKey( supportsVision?: boolean | null ): string { const bodyHash = sha256hex(JSON.stringify(body)); - // model + supportsVision MUST be part of the key: the `lite` engine strips data:image - // URLs only when vision is unsupported (replaceImageUrls / modelSupportsVision), so the - // same (body, config) yields a DIFFERENT result per target — omitting them returns a - // wrong (image-stripped or image-kept) cached body across vision/non-vision targets. + + // #8137: Only include model + supportsVision in the cache key when the compression + // result actually depends on them. The `lite` engine strips data:image URLs only when + // vision is unsupported (replaceImageUrls / modelSupportsVision), so the same (body, + // config) yields a DIFFERENT result per target — omitting them would return a wrong + // (image-stripped or image-kept) cached body across vision/non-vision targets. + // + // For all other deterministic engines (caveman, rtk), the output is model-independent. + // Including model in the key defeats memoization across combo retries — the body is + // identical but the model changes each attempt, producing a fresh cache miss every time + // and re-running the full compression pipeline 5-8x per request. + const isVisionDependent = usesVisionDependentEngine(mode, config); + return sha256hex( JSON.stringify({ bodyHash, mode, config, principalId: principalId ?? null, - model: model ?? null, - supportsVision: supportsVision ?? null, + model: isVisionDependent ? (model ?? null) : null, + supportsVision: isVisionDependent ? (supportsVision ?? null) : null, }) ); } +/** + * Whether the compression pipeline for this mode/config includes the `lite` engine, + * whose output depends on the target's vision support (image-URL stripping). + * Only `lite` itself, `standard` (lite → caveman), and `stacked` pipelines containing + * a `lite` step are vision-dependent. + */ +function usesVisionDependentEngine(mode: CompressionMode, config?: CompressionConfig): boolean { + if (mode === "lite") return true; + if (mode === "standard") return true; // standard = lite → caveman pipeline + if (mode === "stacked") { + const pipeline = config?.stackedPipeline; + if (!pipeline || pipeline.length === 0) return false; + return pipeline.some((step) => step.engine === "lite"); + } + return false; +} + function boundedSet(key: string, value: CompressionResult): void { if (!memoMap.has(key) && memoMap.size >= MEMO_CAP) { const firstKey = memoMap.keys().next().value; diff --git a/tests/unit/compression/result-memo.test.ts b/tests/unit/compression/result-memo.test.ts index d24acc0df9..385e2b19c9 100644 --- a/tests/unit/compression/result-memo.test.ts +++ b/tests/unit/compression/result-memo.test.ts @@ -329,7 +329,7 @@ describe("resultMemo — core review hardening", () => { assert.equal((got!.body.messages as Array<{ content: string }>)[0].content, "original"); }); - it("key folds in model + supportsVision (lite image-strip depends on vision capability)", () => { + it("key folds in model + supportsVision for lite mode (vision-dependent engine)", () => { // Regression: lite strips data:image URLs only when vision is unsupported, so the same // (body, config, principal) yields a DIFFERENT result per target. The key MUST include // model + supportsVision, else a non-vision target's image-stripped body is served to a @@ -340,4 +340,46 @@ describe("resultMemo — core review hardening", () => { assert.notEqual(k("gpt-4", true), k("gemini-2", true), "model must change the key"); assert.equal(k("gpt-4", true), k("gpt-4", true), "same inputs => same key (deterministic)"); }); + + // #8137: model-independent memo keys for non-vision-dependent deterministic engines. + // The combo retry loop re-runs compression for every target even though the body and + // config are identical — only the model changes. For engines that don't depend on vision + // (caveman, rtk, stacked without lite), the compression result is identical regardless of + // model, so including model in the key defeats memoization and wastes CPU on re-compression. + it("#8137: rtk mode produces SAME key across different models (model-independent)", () => { + const k = (model?: string) => makeMemoKey(baseBody, "rtk", memoConfig, "p1", model); + assert.equal(k("gpt-4"), k("claude-3"), "rtk key must be model-independent"); + assert.equal(k("gpt-4"), k("gemini-pro"), "rtk key must be model-independent"); + assert.equal(k(), k("any-model"), "rtk key must be model-independent even vs undefined"); + }); + + it("#8137: caveman mode produces SAME key across different models", () => { + // caveman is deterministic and model-independent (no image/vision logic) + const k = (model?: string) => makeMemoKey(baseBody, "caveman" as never, memoConfig, "p1", model); + assert.equal(k("gpt-4"), k("claude-3"), "caveman key must be model-independent"); + }); + + it("#8137: stacked pipeline WITHOUT lite produces SAME key across different models", () => { + const cfg = { + ...memoConfig, + stackedPipeline: [ + { engine: "rtk" as const, intensity: "standard" as const }, + { engine: "caveman" as const, intensity: "full" as const }, + ], + }; + const k = (model?: string) => makeMemoKey(baseBody, "stacked", cfg, "p1", model); + assert.equal(k("gpt-4"), k("claude-3"), "stacked-without-lite key must be model-independent"); + }); + + it("#8137: stacked pipeline WITH lite produces DIFFERENT keys across different models", () => { + const cfg = { + ...memoConfig, + stackedPipeline: [ + { engine: "lite" as const, intensity: "standard" as const }, + { engine: "caveman" as const, intensity: "full" as const }, + ], + }; + const k = (model?: string) => makeMemoKey(baseBody, "stacked", cfg, "p1", model, false); + assert.notEqual(k("gpt-4"), k("claude-3"), "stacked-with-lite key must be model-dependent"); + }); });