From a0f6104efcbfb9d54f43f330230fe5c408d82eea Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Thu, 21 May 2026 23:49:27 -0300 Subject: [PATCH] fix(tests,translator): repair post-merge regressions on release/v3.8.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two corrections to PRs that merged without their review fixes: 1. #2536 deleted the isCacheable export but tests/unit/semantic-cache.test.ts still imported it — the whole unit suite failed to load. Rewrote the block against the current isCacheableForRead / isCacheableForWrite API (caches streaming + non-streaming; requires explicit temperature: 0). 2. #2474's extractSystemRoleMessages only lifted role=system, dropping role=developer (OpenAI Responses' system alias) into the Anthropic message array — the exact 400 the PR set out to prevent. Now lifts both, matching the normal-path extractSystemMessagesToBody. Adds a developer-role test. --- open-sse/handlers/chatCore.ts | 15 ++++--- tests/unit/semantic-cache.test.ts | 53 +++++++++++++++-------- tests/unit/system-role-extraction.test.ts | 18 ++++++++ 3 files changed, 63 insertions(+), 23 deletions(-) diff --git a/open-sse/handlers/chatCore.ts b/open-sse/handlers/chatCore.ts index 2a189975eccf..6ca8530676c8 100644 --- a/open-sse/handlers/chatCore.ts +++ b/open-sse/handlers/chatCore.ts @@ -1291,9 +1291,14 @@ function isCopilotClient( export function extractSystemRoleMessages(payload: Record): void { if (!Array.isArray(payload.messages)) return; const messages = payload.messages as Array<{ role?: unknown; content?: unknown }>; - const systemMessages = messages.filter( - (m) => typeof m.role === "string" && m.role.toLowerCase() === "system" - ); + // Treat both `system` and `developer` as system-equivalent (OpenAI's Responses + // API renamed system → developer). Anthropic rejects either as a chat role, so + // both must be lifted into the top-level `system` field — parity with the + // normal-path extractSystemMessagesToBody closure. + const isSystemRole = (role: unknown): boolean => + typeof role === "string" && + (role.toLowerCase() === "system" || role.toLowerCase() === "developer"); + const systemMessages = messages.filter((m) => isSystemRole(m.role)); if (systemMessages.length === 0) return; const extraBlocks: Array> = []; @@ -1318,9 +1323,7 @@ export function extractSystemRoleMessages(payload: Record): voi payload.system = extraBlocks; } } - payload.messages = messages.filter( - (m) => typeof m.role !== "string" || m.role.toLowerCase() !== "system" - ); + payload.messages = messages.filter((m) => !isSystemRole(m.role)); } export async function handleChatCore({ diff --git a/tests/unit/semantic-cache.test.ts b/tests/unit/semantic-cache.test.ts index f27a23a5ec75..69185613a0d2 100644 --- a/tests/unit/semantic-cache.test.ts +++ b/tests/unit/semantic-cache.test.ts @@ -1,6 +1,10 @@ import { describe, it } from "node:test"; import assert from "node:assert/strict"; -import { generateSignature, isCacheable } from "../../src/lib/semanticCache.ts"; +import { + generateSignature, + isCacheableForRead, + isCacheableForWrite, +} from "../../src/lib/semanticCache.ts"; describe("Semantic Cache", () => { describe("generateSignature", () => { @@ -69,35 +73,50 @@ describe("Semantic Cache", () => { }); }); - describe("isCacheable", () => { - it("returns true for non-streaming temp=0 requests", () => { - assert.equal(isCacheable({ stream: false, temperature: 0 }, null), true); + describe("isCacheableForRead", () => { + // #2536 superseded isCacheable with read/write variants that cache both + // streaming and non-streaming requests and require an explicit numeric + // temperature: 0 (omitted temperature is treated as non-deterministic). + it("returns true for temperature=0 (streaming or not)", () => { + assert.equal(isCacheableForRead({ stream: false, temperature: 0 }, null), true); + assert.equal(isCacheableForRead({ stream: true, temperature: 0 }, null), true); }); - it("returns true when temperature is undefined (defaults to 0)", () => { - assert.equal(isCacheable({ stream: false }, null), true); - }); - - it("returns false for streaming requests", () => { - assert.equal(isCacheable({ stream: true, temperature: 0 }, null), false); - }); - - it("returns false when stream is not explicitly false", () => { - assert.equal(isCacheable({ temperature: 0 }, null), false); + it("returns false when temperature is omitted (provider default may be non-deterministic)", () => { + assert.equal(isCacheableForRead({ stream: false }, null), false); }); it("returns false for non-zero temperature", () => { - assert.equal(isCacheable({ stream: false, temperature: 0.7 }, null), false); + assert.equal(isCacheableForRead({ temperature: 0.7 }, null), false); }); it("returns false when no-cache header is set", () => { const headers = new Headers({ "x-omniroute-no-cache": "true" }); - assert.equal(isCacheable({ stream: false, temperature: 0 }, headers), false); + assert.equal(isCacheableForRead({ temperature: 0 }, headers), false); }); it("returns true when no-cache header is absent", () => { const headers = new Headers({}); - assert.equal(isCacheable({ stream: false, temperature: 0 }, headers), true); + assert.equal(isCacheableForRead({ temperature: 0 }, headers), true); + }); + }); + + describe("isCacheableForWrite", () => { + it("returns true for temperature=0 responses", () => { + assert.equal(isCacheableForWrite({ temperature: 0 }, null), true); + }); + + it("returns false when temperature is omitted", () => { + assert.equal(isCacheableForWrite({ stream: false }, null), false); + }); + + it("returns false for non-zero temperature", () => { + assert.equal(isCacheableForWrite({ temperature: 0.7 }, null), false); + }); + + it("returns false when no-cache header is set", () => { + const headers = new Headers({ "x-omniroute-no-cache": "true" }); + assert.equal(isCacheableForWrite({ temperature: 0 }, headers), false); }); }); }); diff --git a/tests/unit/system-role-extraction.test.ts b/tests/unit/system-role-extraction.test.ts index 306fb7feef27..e2f9f09381c0 100644 --- a/tests/unit/system-role-extraction.test.ts +++ b/tests/unit/system-role-extraction.test.ts @@ -16,6 +16,24 @@ test("extractSystemRoleMessages moves role=system to top-level system", () => { assert.deepEqual(payload.system, [{ type: "text", text: "Memory context: foo" }]); }); +test("extractSystemRoleMessages also lifts role=developer (OpenAI Responses system alias)", () => { + const payload = { + messages: [ + { role: "developer", content: "Dev instructions" }, + { role: "system", content: "Sys context" }, + { role: "user", content: "hello" }, + ], + }; + extractSystemRoleMessages(payload); + // both developer and system are removed from messages and lifted into system + assert.equal(payload.messages.length, 1); + assert.equal(payload.messages[0].role, "user"); + assert.deepEqual(payload.system, [ + { type: "text", text: "Dev instructions" }, + { type: "text", text: "Sys context" }, + ]); +}); + test("extractSystemRoleMessages merges with existing top-level system string", () => { const payload = { system: "You are Claude.",