From 5e8b130e77c774b420765402fbc3e8d904d3f095 Mon Sep 17 00:00:00 2001 From: Rafael Dias Zendron Date: Thu, 23 Jul 2026 05:36:49 -0300 Subject: [PATCH] fix(compression): make memo key model-independent for non-vision engines (#8196) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(ci): resolve upstream-inherited check failures * fix(compression): make memo key model-independent for non-vision engines (#8137) The compression result memo included `model` and `supportsVision` in the cache key for ALL deterministic modes. This was correct for the `lite` engine (which strips data:image URLs based on vision support) but unnecessary for model-independent engines like `rtk`, `caveman`, and stacked pipelines without a `lite` step. In the combo retry loop, the body and config are identical across targets — only the model changes each attempt. Including model in the key forced a fresh cache miss on every retry, re-running the full compression pipeline 5-8x per request instead of serving the cached result. Fix: `makeMemoKey` now only includes model + supportsVision when the compression pipeline actually uses a vision-dependent engine (lite, standard, or stacked containing lite). All other deterministic engines use a model-independent key. - Add `usesVisionDependentEngine()` helper to classify modes - `makeMemoKey` conditionally includes model/vision fields - 5 new tests covering rtk, caveman, stacked-with-lite, stacked-without-lite Closes #8137 --------- Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --- open-sse/services/compression/resultMemo.ts | 38 +++++++++++++++--- tests/unit/compression/result-memo.test.ts | 44 ++++++++++++++++++++- 2 files changed, 75 insertions(+), 7 deletions(-) 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"); + }); });