fix(compression): make memo key model-independent for non-vision engines (#8196)

* 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

@@ -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;

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");
+  });
 });