perf(compression): avoid cloning memo misses twice (#11727)

Obrigado! Validado em lote combinado (8 PRs, release/v3.8.51):

- TDD claro: sem o fix, o teste focado falha nas asserções de contagem exata de lookup sync/async; com o fix, `tests/unit/compression/result-memo.test.ts` passa (34/34).
- Remove supressão eslint agora obsoleta (`no-unused-vars` no arquivo de teste).
- `⚠️ base-red inherited: #11449` reconhecido e verificado — não é responsabilidade desta PR (confirmado via probe-worktree do tip puro).
- Gates estáticos do lote OK.
This commit is contained in:
Ravi Tharuma
2026-08-28 03:14:57 +02:00
committed by GitHub
parent 09fa818365
commit a14a91dcfe
4 changed files with 47 additions and 9 deletions

View File

@@ -4539,11 +4539,6 @@
"count": 1
}
},
"tests/unit/compression/result-memo.test.ts": {
"@typescript-eslint/no-unused-vars": {
"count": 1
}
},
"tests/unit/compression/rtk-grouping.test.ts": {
"@typescript-eslint/no-unused-vars": {
"count": 1

View File

@@ -4,6 +4,7 @@ import type { CompressionConfig, CompressionMode, CompressionResult } from "./ty
export const MEMO_CAP = 5_000;
const memoMap = new Map<string, CompressionResult>();
let lookupCountForTests = 0;
// Opt-IN whitelist (NOT opt-out): cache only engines proven pure + STATELESS across
// requests. Excluded on purpose: `ccr` and `session-dedup` write to the cross-request
@@ -94,6 +95,7 @@ function boundedSet(key: string, value: CompressionResult): void {
}
export function memoLookup(key: string): CompressionResult | null {
lookupCountForTests++;
const hit = memoMap.get(key);
if (!hit) return null;
// Return a clone so downstream mutation cannot corrupt the cached value.
@@ -110,4 +112,10 @@ export function memoStore(key: string, result: CompressionResult): void {
/** For tests only — clears the in-process memo store. */
export function clearMemoStore(): void {
memoMap.clear();
lookupCountForTests = 0;
}
export const resultMemoForTests = {
get lookupCount(): number {
return lookupCountForTests;
},
};

View File

@@ -332,7 +332,7 @@ function runCompression(
config: { ...options.config, memoizeCompressionResults: false },
});
memoStore(key, result);
return memoLookup(key)!;
return result;
}
if (mode === "rtk") {
return applyRtkCompression(body, {
@@ -565,7 +565,7 @@ async function runCompressionAsync(
config: { ...options.config, memoizeCompressionResults: false },
});
memoStore(key, result);
return memoLookup(key)!;
return result;
}
// Single-mode omniglyph (async-only) — resolution lives in engines/omniglyphSingleMode.ts.
if (mode === "omniglyph") return applyOmniglyphSingleMode(body, options);

View File

@@ -6,11 +6,15 @@ import {
makeMemoKey,
isDeterministicMode,
clearMemoStore,
resultMemoForTests,
MEMO_CAP,
} from "../../../open-sse/services/compression/resultMemo.ts";
import type { CompressionResult } from "../../../open-sse/services/compression/types.ts";
import { DEFAULT_COMPRESSION_CONFIG } from "../../../open-sse/services/compression/types.ts";
import { applyCompression } from "../../../open-sse/services/compression/strategySelector.ts";
import {
applyCompression,
applyCompressionAsync,
} from "../../../open-sse/services/compression/strategySelector.ts";
const baseBody = {
messages: [{ role: "user", content: "hello world compress me please" }],
@@ -218,7 +222,6 @@ describe("applyCompression with memoization", () => {
});
it("flag OFF: two identical calls both compute (no caching path)", () => {
let callCount = 0;
// We can't easily spy on internal engine, so we verify via deterministic output
// equality between independent calls (proving cache isn't interfering).
// Use a body that will be lightly compressed.
@@ -257,6 +260,38 @@ describe("applyCompression with memoization", () => {
assert.notEqual(memoLookup(key), null);
});
it("memo misses and hits cannot mutate the cached result", () => {
const body = {
messages: [{ role: "user", content: "Mutation isolation test content. ".repeat(15) }],
model: "gpt-4",
};
const miss = applyCompression(body, "lite", { config: memoConfig, principalId: "u1" });
const expected = structuredClone(miss.body);
miss.body.messages[0]!.content = "mutated miss";
const hit = applyCompression(body, "lite", { config: memoConfig, principalId: "u1" });
assert.deepEqual(hit.body, expected);
hit.body.messages[0]!.content = "mutated hit";
assert.deepEqual(
applyCompression(body, "lite", { config: memoConfig, principalId: "u1" }).body,
expected
);
assert.equal(resultMemoForTests.lookupCount, 3);
});
it("async memo misses and hits perform one lookup per call", async () => {
const body = {
messages: [{ role: "user", content: "Async lookup count test content. ".repeat(15) }],
model: "gpt-4",
};
await applyCompressionAsync(body, "lite", { config: memoConfig, principalId: "u1" });
assert.equal(resultMemoForTests.lookupCount, 1);
await applyCompressionAsync(body, "lite", { config: memoConfig, principalId: "u1" });
assert.equal(resultMemoForTests.lookupCount, 2);
});
it("flag ON + deterministic mode: different principalId = MISS", () => {
const body = {
messages: [{ role: "user", content: "Cross-principal test content. ".repeat(10) }],