From f3679019ab6fc30a6dcd1cd2d2287e536a3f9cff Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Mon, 15 Jun 2026 20:05:23 -0300 Subject: [PATCH] fix(compression): preserve upstream prompt cache for memory + compression (#3890) (#3936) --- CHANGELOG.md | 1 + config/quality/file-size-baseline.json | 3 +- open-sse/handlers/chatCore.ts | 12 +- .../chatCore/memorySkillsInjection.ts | 8 +- .../services/compression/strategySelector.ts | 22 ++++ src/lib/memory/injection.ts | 39 ++++++- .../strategySelector-cache-aware.test.ts | 58 ++++++++++ .../unit/memory-cache-safe-injection.test.ts | 105 ++++++++++++++++++ 8 files changed, 239 insertions(+), 9 deletions(-) create mode 100644 tests/unit/compression/strategySelector-cache-aware.test.ts create mode 100644 tests/unit/memory-cache-safe-injection.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index a3c327a8af..71fcb30c2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ ### 🐛 Fixed +- **fix(compression/memory): stop memory + compression from poisoning the upstream prompt cache** — with compression and/or memory enabled, requests to caching providers (Anthropic-family) missed the prompt cache on every turn, multiplying cost. Two root causes: (1) memory injection prepended the retrieved memories — which **vary per user query** — at index 0 of the message array, shifting the entire cacheable prefix every turn; memory is now inserted just before the last user message when the request carries `cache_control` breakpoints, keeping the cacheable prefix (system prompt + prior turns) byte-stable. (2) the cache-aware `skipSystemPrompt` flag computed by `getCacheAwareStrategy()` was dropped by `selectCompressionStrategy()` (which can only return a mode), so the system prompt could still be compressed under caching; a new `resolveCacheAwareConfig()` now forces `preserveSystemPrompt` on for caching requests. ([#3890](https://github.com/diegosouzapw/OmniRoute/issues/3890) — thanks @xenstar) - **fix(providers): register BytePlus ModelArk so its API key can be added** — adding a BytePlus (`ark-…`) key reported "invalid". `byteplus` was present in the provider catalog (`APIKEY_PROVIDERS`) but **never registered in the routing registry**, so key validation fell through to `{ unsupported: true }` → HTTP 400 → the UI rendered every key as invalid (and the provider was unusable for inference). Added a registry entry modeled on the existing Volcengine Ark provider: OpenAI-compatible format, base `https://ark.ap-southeast.bytepluses.com/api/v3` (region `ap-southeast-1`), `Authorization: Bearer` auth, seeded with the catalog's advertised models (Seed 2.0, Kimi K2 Thinking, GLM 4.7, GPT-OSS-120B). ([#3877](https://github.com/diegosouzapw/OmniRoute/issues/3877) — thanks @nikohd12) - **fix(providers): Nous Research key validation no longer fails on a stale probe model** — adding a valid Nous Research API key reported "invalid" even though the same key worked via the portal's copy-shell `curl`. The validation probe sent `model: "nousresearch/hermes-4-70b"`, which Nous does not serve, so the API returned `400` and the validator (which only treated `200`/`429` as success) reported the key invalid. The probe now uses the real `Hermes-4-70B` slug, and any non-auth 4xx (`400`/`404`/`422`) is treated as a valid key (the request shape was wrong, not the credentials) — mirroring the longcat/nvidia validators so a future model rename can't re-break key validation. ([#3881](https://github.com/diegosouzapw/OmniRoute/issues/3881) — thanks @FerLuisxd) - **test(oauth): prove refresh_token preservation for the real gemini-cli / antigravity dispatch** — the #3679/#3766 regression test used a synthetic provider that routes through the generic `tokenUrl` path, so the fix was never proven for the actual Google-family providers, which dispatch through `refreshGoogleToken()` against the hardcoded `OAUTH_ENDPOINTS.google.token`. Added a test that drives `checkConnection` through the real `gemini-cli`/`antigravity` path (redirecting the Google token endpoint to a local server returning `invalid_grant`) and asserts the `refresh_token` is preserved (not nulled) — confirming these connections are not spuriously destroyed on a failed refresh. ([#3850](https://github.com/diegosouzapw/OmniRoute/issues/3850) — thanks @3xa228148) diff --git a/config/quality/file-size-baseline.json b/config/quality/file-size-baseline.json index 8ba2e193d8..f4d36fe37a 100644 --- a/config/quality/file-size-baseline.json +++ b/config/quality/file-size-baseline.json @@ -14,6 +14,7 @@ "_rebaseline_2026_06_15_3910_nested_combo_ctx": "PR #3910 net +1: providerRegistry.ts 4730->4731 (test-registry contextLength metadata for the nested combo-ref LCD regression test). opencode-plugin-only behavioral change; no core routing/virtualFactory touched.", "_rebaseline_2026_06_15_3929_vertex_media": "PR #3929 own growth: audioSpeech.ts 952->965 (+13) and videoGeneration.ts 1026->1078 (+52) = vertex/* media branches (Gemini TTS, Veo predictLongRunning poll) wired into the speech/video handlers; new logic lives in open-sse/executors/vertexMedia.ts (341, under cap). Cohesive media-provider feature.", "_rebaseline_2026_06_15_3879_redact_thinking": "PR #3879 + #3921 reconcile: AddApiKeyModal.tsx 843->845 (+2 = merging #3879's CcCompatibleRequestDefaultsFields (context1m + opt-in redact-thinking toggle) into #3921's preset-input block in the cc-compatible settings group). Cohesive UI; not extractable.", + "_rebaseline_2026_06_15_3890_cache_preserve": "Issue #3890 own growth: chatCore.ts 5815->5823 (+8 = wire resolveCacheAwareConfig() into the compression apply step so the system prompt is never compressed in a caching context — honors the cache-aware skipSystemPrompt flag that selectCompressionStrategy could not carry). Cohesive cache-preservation guard at the existing compression chokepoint; not extractable.", "cap": 800, "frozen": { "open-sse/config/providerRegistry.ts": 4731, @@ -29,7 +30,7 @@ "open-sse/executors/muse-spark-web.ts": 1284, "open-sse/executors/perplexity-web.ts": 868, "open-sse/handlers/audioSpeech.ts": 965, - "open-sse/handlers/chatCore.ts": 5815, + "open-sse/handlers/chatCore.ts": 5823, "open-sse/handlers/imageGeneration.ts": 3777, "open-sse/handlers/responseSanitizer.ts": 1103, "open-sse/handlers/search.ts": 1442, diff --git a/open-sse/handlers/chatCore.ts b/open-sse/handlers/chatCore.ts index 4d7e5016c2..fe28551589 100644 --- a/open-sse/handlers/chatCore.ts +++ b/open-sse/handlers/chatCore.ts @@ -2391,7 +2391,7 @@ export async function handleChatCore({ // --- Modular Compression Pipeline (Phase 1 Lite + Phase 2 Standard/Caveman + Phase 3 Aggressive) --- // Runs BEFORE the existing reactive compressContext() to proactively reduce tokens. try { - const { selectCompressionStrategy, applyCompressionAsync } = + const { selectCompressionStrategy, applyCompressionAsync, resolveCacheAwareConfig } = await import("../services/compression/strategySelector.ts"); const { trackCompressionStats } = await import("../services/compression/stats.ts"); let config: CompressionConfig = compressionSettings ?? { @@ -2629,9 +2629,17 @@ export async function handleChatCore({ ); let compressionAnalyticsRecorded = false; if (mode !== "off") { + // #3890: in a caching context, never compress the system prompt (cacheable prefix) + // even if the operator disabled preserveSystemPrompt — honors the cache-aware flag + // that selectCompressionStrategy can only partially apply via the mode string. + const compressionConfig = resolveCacheAwareConfig(config, compressionInputBody, { + provider, + targetFormat, + model: effectiveModel, + }); const result = await applyCompressionAsync(compressionInputBody, mode, { model: effectiveModel, - config, + config: compressionConfig, principalId: apiKeyInfo?.id ? String(apiKeyInfo.id) : undefined, }); if (result.stats) { diff --git a/open-sse/handlers/chatCore/memorySkillsInjection.ts b/open-sse/handlers/chatCore/memorySkillsInjection.ts index 28719d810d..ae0fa35f58 100644 --- a/open-sse/handlers/chatCore/memorySkillsInjection.ts +++ b/open-sse/handlers/chatCore/memorySkillsInjection.ts @@ -3,6 +3,7 @@ import { getMemorySettings, DEFAULT_MEMORY_SETTINGS, toMemoryRetrievalConfig } f import { injectMemory, shouldInjectMemory } from "@/lib/memory/injection"; import { injectSkills } from "@/lib/skills/injection"; import { FORMATS } from "../../translator/formats.ts"; +import { detectCachingContext } from "../../services/compression/cachingAware.ts"; export function getSkillsProviderForFormat(format: string): "openai" | "anthropic" | "google" | "other" { switch (format) { @@ -113,10 +114,15 @@ export async function injectMemoryAndSkills({ toMemoryRetrievalConfig(memorySettings, { query: lastUserQuery }) ); if (memories.length > 0) { + // #3890: when the client uses prompt caching (cache_control breakpoints), inject + // memory cache-safely (before the last user message) so the per-query memory text + // does not poison the cacheable prefix and force a cache miss on every turn. + const cacheSafe = detectCachingContext(body, { provider, targetFormat }).hasCacheControl; const injected = injectMemory( body as Parameters[0], memories, - provider + provider, + { cacheSafe } ); body = injected as typeof body; log?.debug?.("MEMORY", `Injected ${memories.length} memories for key=${memoryOwnerId}`); diff --git a/open-sse/services/compression/strategySelector.ts b/open-sse/services/compression/strategySelector.ts index 105e3da4df..20e15580a1 100644 --- a/open-sse/services/compression/strategySelector.ts +++ b/open-sse/services/compression/strategySelector.ts @@ -67,6 +67,28 @@ export function selectCompressionStrategy( return selectedMode; } +/** + * #3890: honor the cache-aware `skipSystemPrompt` decision that `getCacheAwareStrategy` + * already computes but that `selectCompressionStrategy` (which can only return a mode + * string) previously discarded. In a caching context the system prompt is part of the + * cacheable prefix, so compressing it breaks the upstream prompt cache. This forces + * `preserveSystemPrompt` on for caching requests even when the operator turned it off, + * and leaves non-caching requests untouched. + */ +export function resolveCacheAwareConfig( + config: CompressionConfig, + body?: Record, + context?: CachingDetectionContext +): CompressionConfig { + if (!body) return config; + const ctx = detectCachingContext(body, context); + const cacheAware = getCacheAwareStrategy(config.defaultMode, ctx); + if (cacheAware.skipSystemPrompt && config.preserveSystemPrompt === false) { + return { ...config, preserveSystemPrompt: true }; + } + return config; +} + export function applyCompression( body: Record, mode: CompressionMode, diff --git a/src/lib/memory/injection.ts b/src/lib/memory/injection.ts index 283bb28bd2..b9a6199bb5 100644 --- a/src/lib/memory/injection.ts +++ b/src/lib/memory/injection.ts @@ -79,10 +79,24 @@ export function formatMemoryContext(memories: Memory[]): string { * @param provider - Provider identifier used to choose injection strategy * @returns A new request body with memories prepended to messages */ +export interface InjectMemoryOptions { + /** + * #3890: when the request uses prompt caching (cache_control breakpoints), prepending + * the memory message at index 0 shifts the entire cacheable prefix — and since the + * retrieved memories vary per user query, every cache breakpoint then misses on each + * turn (observed as a sustained cache-miss / cost spike). When `cacheSafe` is set, the + * memory message is inserted immediately before the LAST user message instead, so the + * cacheable prefix (system prompt + prior turns) stays byte-stable while memory still + * contextualizes the current turn. + */ + cacheSafe?: boolean; +} + export function injectMemory( request: ChatRequest, memories: Memory[], - provider: string | null | undefined + provider: string | null | undefined, + options: InjectMemoryOptions = {} ): ChatRequest { if (!memories || memories.length === 0) { log.info("memory.injection.skipped", { reason: "no_memories", model: request.model }); @@ -97,26 +111,41 @@ export function injectMemory( const messages: ChatMessage[] = Array.isArray(request.messages) ? [...request.messages] : []; + // #3890: in a caching context, anchor the injection just before the LAST user message so + // the cacheable prefix (system prompt + prior turns) is preserved byte-for-byte. Falls + // back to a leading message when caching is off or there is no user turn to anchor on. + const cacheSafeIndex = options.cacheSafe ? messages.findLastIndex((m) => m.role === "user") : -1; + if (providerSupportsSystemMessage(provider)) { - // Strategy 1: inject as a leading system message. + // Strategy 1: inject as a system message. // Prepending before any existing system messages keeps memory context // accessible without overriding the caller's own system instructions. const memorySystemMessage: ChatMessage = { role: "system", content: memoryText }; log.info("memory.injection.injected", { count: memories.length, - strategy: "system", + strategy: cacheSafeIndex >= 0 ? "system-cache-safe" : "system", model: request.model, }); + if (cacheSafeIndex >= 0) { + const next = [...messages]; + next.splice(cacheSafeIndex, 0, memorySystemMessage); + return { ...request, messages: next }; + } return { ...request, messages: [memorySystemMessage, ...messages] }; } else { - // Strategy 2 (fallback): inject as the first user message. + // Strategy 2 (fallback): inject as a user message. // Used for providers like o1-mini that reject the system role. const memoryUserMessage: ChatMessage = { role: "user", content: memoryText }; log.info("memory.injection.injected", { count: memories.length, - strategy: "user", + strategy: cacheSafeIndex >= 0 ? "user-cache-safe" : "user", model: request.model, }); + if (cacheSafeIndex >= 0) { + const next = [...messages]; + next.splice(cacheSafeIndex, 0, memoryUserMessage); + return { ...request, messages: next }; + } return { ...request, messages: [memoryUserMessage, ...messages] }; } } diff --git a/tests/unit/compression/strategySelector-cache-aware.test.ts b/tests/unit/compression/strategySelector-cache-aware.test.ts new file mode 100644 index 0000000000..c7d1963282 --- /dev/null +++ b/tests/unit/compression/strategySelector-cache-aware.test.ts @@ -0,0 +1,58 @@ +/** + * Tests for #3890: the cache-aware `skipSystemPrompt` flag was computed by + * getCacheAwareStrategy() but dropped by selectCompressionStrategy() (which can only + * return a mode string). resolveCacheAwareConfig() applies it: in a caching context the + * system prompt must stay uncompressed (it is part of the cacheable prefix) even when the + * operator disabled preserveSystemPrompt. + */ + +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { resolveCacheAwareConfig } from "../../../open-sse/services/compression/strategySelector.ts"; +import type { CompressionConfig } from "../../../open-sse/services/compression/types.ts"; + +function cfg(overrides: Partial = {}): CompressionConfig { + return { + enabled: true, + defaultMode: "standard", + autoTriggerTokens: 0, + cacheMinutes: 5, + preserveSystemPrompt: true, + comboOverrides: {}, + ...overrides, + } as CompressionConfig; +} + +describe("resolveCacheAwareConfig (#3890)", () => { + it("forces preserveSystemPrompt on for a caching request that disabled it", () => { + const out = resolveCacheAwareConfig( + cfg({ preserveSystemPrompt: false }), + { messages: [{ role: "system", content: "x", cache_control: { type: "ephemeral" } }] }, + { provider: "anthropic", targetFormat: "claude" } + ); + assert.equal(out.preserveSystemPrompt, true); + }); + + it("leaves a non-caching request untouched (preserveSystemPrompt stays false)", () => { + const out = resolveCacheAwareConfig( + cfg({ preserveSystemPrompt: false }), + { messages: [{ role: "system", content: "x" }] }, + { provider: "openai" } + ); + assert.equal(out.preserveSystemPrompt, false); + }); + + it("returns the same config object when there is no body", () => { + const base = cfg({ preserveSystemPrompt: false }); + assert.equal(resolveCacheAwareConfig(base), base); + }); + + it("does not change a config that already preserves the system prompt", () => { + const out = resolveCacheAwareConfig( + cfg({ preserveSystemPrompt: true }), + { messages: [{ role: "system", content: "x", cache_control: { type: "ephemeral" } }] }, + { provider: "anthropic", targetFormat: "claude" } + ); + assert.equal(out.preserveSystemPrompt, true); + }); +}); diff --git a/tests/unit/memory-cache-safe-injection.test.ts b/tests/unit/memory-cache-safe-injection.test.ts new file mode 100644 index 0000000000..c86e39df08 --- /dev/null +++ b/tests/unit/memory-cache-safe-injection.test.ts @@ -0,0 +1,105 @@ +/** + * Tests for #3890: prompt-cache misses when memory injection is enabled. + * + * When the client uses prompt caching (cache_control breakpoints), the previous behavior + * prepended the (per-query, varying) memory message at index 0 — shifting the entire + * cacheable prefix and forcing a cache miss on every turn. With `cacheSafe`, memory is + * inserted just before the last user message so the cacheable prefix stays byte-stable. + */ + +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { injectMemory } from "../../src/lib/memory/injection.ts"; +import type { ChatRequest } from "../../src/lib/memory/injection.ts"; +import type { Memory } from "../../src/lib/memory/types.ts"; + +function mem(content: string): Memory { + return { + id: `mem-${content}`, + content, + type: "factual" as any, + apiKeyId: "k", + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + importance: 0.5, + }; +} + +function multiTurn(): ChatRequest { + return { + model: "anthropic/claude-sonnet-4-6", + messages: [ + { role: "system", content: "SYSTEM PROMPT", cache_control: { type: "ephemeral" } } as any, + { role: "user", content: "turn 1 question" }, + { role: "assistant", content: "turn 1 answer" }, + { role: "user", content: "turn 2 question" }, + ], + }; +} + +describe("injectMemory cache-safe positioning (#3890)", () => { + it("default (cacheSafe off) prepends memory at index 0 — unchanged legacy behavior", () => { + const out = injectMemory(multiTurn(), [mem("dark mode")], "anthropic"); + assert.equal(out.messages[0].role, "system"); + assert.ok(out.messages[0].content.includes("Memory context")); + assert.equal(out.messages[1].content, "SYSTEM PROMPT"); + }); + + it("cacheSafe inserts memory just before the last user message, preserving the prefix", () => { + const req = multiTurn(); + const prefixBefore = JSON.stringify(req.messages.slice(0, 3)); // sys, u1, a1 + const out = injectMemory(req, [mem("dark mode")], "anthropic", { cacheSafe: true }); + + // The cacheable prefix (system + prior turns up to the last assistant) is byte-identical. + assert.equal(JSON.stringify(out.messages.slice(0, 3)), prefixBefore); + // Memory is injected right before the last user message... + assert.equal(out.messages[3].role, "system"); + assert.ok(out.messages[3].content.includes("Memory context")); + // ...and the last user message is preserved at the tail. + assert.equal(out.messages[4].content, "turn 2 question"); + assert.equal(out.messages.length, 5); + // Memory must NOT be at index 0 (that is what broke caching). + assert.notEqual(out.messages[0].content, out.messages[3].content); + assert.equal(out.messages[0].content, "SYSTEM PROMPT"); + }); + + it("cacheSafe keeps the cacheable prefix identical across two turns despite different memories", () => { + // Turn 1: [sys, u1]; Turn 2: [sys, u1, a1, u2]. Different memories retrieved per query. + // Same conversation, observed on two consecutive turns. + const turn1: ChatRequest = { + model: "anthropic/claude-sonnet-4-6", + messages: [ + { role: "system", content: "SYSTEM PROMPT", cache_control: { type: "ephemeral" } } as any, + { role: "user", content: "turn 1 question" }, + ], + }; + const turn2 = multiTurn(); + + const out1 = injectMemory(turn1, [mem("A")], "anthropic", { cacheSafe: true }); + const out2 = injectMemory(turn2, [mem("B")], "anthropic", { cacheSafe: true }); + + // The cache-breakpoint-bearing system message stays at the head, byte-identical, in + // both turns (and is NOT displaced by the per-query memory) — so the prompt cache + // created on turn 1 still matches on turn 2. (Memory is inserted before the last user + // turn: [SYS, MEM_A, u1] and [SYS, u1, a1, MEM_B, u2] respectively.) + assert.deepEqual(out1.messages[0], out2.messages[0]); + assert.equal(out1.messages[0].content, "SYSTEM PROMPT"); + assert.equal((out1.messages[0] as any).cache_control?.type, "ephemeral"); + // In turn 2 the earlier turns up to the last assistant are preserved before memory. + assert.equal(out2.messages[1].content, "turn 1 question"); + assert.equal(out2.messages[2].content, "turn 1 answer"); + assert.ok(out2.messages[3].content.includes("Memory context")); + assert.equal(out2.messages[4].content, "turn 2 question"); + }); + + it("cacheSafe falls back to leading injection when there is no user message", () => { + const req: ChatRequest = { + model: "anthropic/claude-sonnet-4-6", + messages: [{ role: "system", content: "SYS" }], + }; + const out = injectMemory(req, [mem("x")], "anthropic", { cacheSafe: true }); + assert.equal(out.messages[0].role, "system"); + assert.ok(out.messages[0].content.includes("Memory context")); + assert.equal(out.messages[1].content, "SYS"); + }); +});