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.",