From ca55f473ee8facdae63ef1b0d7cc26d96ea82b38 Mon Sep 17 00:00:00 2001 From: ran Date: Wed, 15 Jul 2026 17:16:25 -0700 Subject: [PATCH 1/3] fix(router): match the prompt when a multimodal turn is split into separate user messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some SDKs (e.g. Microsoft Agent Framework's agent_framework_openai image path) serialize a single multimodal user turn (prompt text + attachment) into TWO consecutive user messages — a text-only one followed by an attachment-only one (content: [{type: 'image_url', ...}]). The trailing attachment-only message has no extractable text, so the naive last-user- message lookup returned null and — since no fixture can key on empty text — every such multimodal fixture missed (404/503). getLastUserText now skips trailing user messages that carry no text and uses the nearest preceding user message that does. This is deliberately narrow: it only skips text-LESS user messages, so a genuine multi-message user turn (each message has text) is unaffected and still matched on its final message. Adds two router tests (image split + a text-bearing trailing message stays the match target). --- src/__tests__/router.test.ts | 36 ++++++++++++++++++++++++++++++++++++ src/router.ts | 31 +++++++++++++++++++++++++++++-- 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/src/__tests__/router.test.ts b/src/__tests__/router.test.ts index 50885730..5f63a75b 100644 --- a/src/__tests__/router.test.ts +++ b/src/__tests__/router.test.ts @@ -213,6 +213,42 @@ describe("matchFixture — userMessage (array content)", () => { }); expect(matchFixture([fixture], req)).toBeNull(); }); + + it("matches the text prompt when a trailing user message is attachment-only (multimodal image split)", () => { + // Some SDKs (e.g. Microsoft Agent Framework's agent_framework_openai image + // path) serialise a single multimodal turn into a text-only user message + // FOLLOWED by a separate attachment-only user message. The trailing + // image-only message must not shadow the real prompt. + const fixture = makeFixture({ userMessage: "describe this image" }); + const req = makeReq({ + messages: [ + { role: "user", content: "please describe this image" }, + { + role: "user", + content: [{ type: "image_url", image_url: { url: "data:image/png;base64,AAAA" } }], + }, + ], + }); + expect(matchFixture([fixture], req)).toBe(fixture); + }); + + it("keeps matching a trailing user message that HAS text (does not skip a flattened attachment)", () => { + // Contrast with the image split above: when the trailing user message + // carries text (e.g. a pdf flattened to `[Attached document]\n…` by the + // agent) it is NOT skipped — it is the match target. Fixtures for such a + // turn therefore key on the flattened body, not the original prompt. + const fixture = makeFixture({ userMessage: "CopilotKit Quickstart" }); + const req = makeReq({ + messages: [ + { role: "user", content: "can you tell me what is in this demo pdf I just attached" }, + { + role: "user", + content: "[Attached document]\nCopilotKit Quickstart\nAdd AI copilots to your app.", + }, + ], + }); + expect(matchFixture([fixture], req)).toBe(fixture); + }); }); describe("matchFixture — userMessage (RegExp)", () => { diff --git a/src/router.ts b/src/router.ts index fe6ec21b..d577ffcd 100644 --- a/src/router.ts +++ b/src/router.ts @@ -70,6 +70,31 @@ export function getTextContent(content: string | ContentPart[] | null): string | return null; } +/** + * Text of the last user message, for `userMessage` fixture matching. Normally + * this is the text of the final `user` message. But some SDKs serialise a + * single multimodal user turn (prompt text + attachment) into TWO consecutive + * user messages — a text-only one FOLLOWED by an attachment-only one, e.g. + * `[{role:"user", content:"describe this"}, {role:"user", content:[{type:"image_url",...}]}]` + * (observed with Microsoft Agent Framework's `agent_framework_openai` image + * path). The trailing attachment-only message has NO extractable text, so the + * naive "last user message" lookup returns `null` and — because no fixture can + * key on empty text — every such multimodal fixture misses. We therefore skip + * trailing user messages that carry no text and use the nearest preceding user + * message that does. This is deliberately narrow: it only skips text-LESS user + * messages, so a genuine multi-message user turn (each message HAS text) is + * unaffected and still matched on its final message. Returns `null` when no + * user message carries text. + */ +export function getLastUserText(messages: ChatMessage[]): string | null { + for (let i = messages.length - 1; i >= 0; i--) { + if (messages[i].role !== "user") continue; + const text = getTextContent(messages[i].content); + if (text !== null && text !== "") return text; + } + return null; +} + /** * Result of {@link matchFixtureDiagnostic}: the matched fixture (or `null`) plus * the number of fixtures that matched the request SHAPE (every predicate above @@ -276,8 +301,10 @@ export function matchFixtureDiagnostic( // matchesPattern() in helpers.ts, which is used for search/rerank/moderation // where exact casing rarely matters. if (match.userMessage !== undefined) { - const msg = getLastMessageByRole(effective.messages, "user"); - const text = msg ? getTextContent(msg.content) : null; + // Use the last user message that actually carries text — see + // getLastUserText for why a trailing attachment-only user message + // (multimodal serialisation split) must not shadow the real prompt. + const text = getLastUserText(effective.messages); if (!text) continue; if (typeof match.userMessage === "string") { if (useExactMatch) { From aa861da0f5d63a3519b728a01b8e6760c0d7035e Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Wed, 15 Jul 2026 23:07:34 -0700 Subject: [PATCH 2/3] fix(router): match fixtures keyed on empty user/embedding text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The userMessage and inputText matcher branches discarded the null-vs-"" distinction at two layers, so a fixture keyed on an empty body could never fire even though such fixtures load fine: - getLastUserText skipped a user message whose text extracted to "" (the `&& text !== ""` clause), conflating a present-but-empty text turn with a text-less (attachment-only) split turn. It now skips only the genuinely text-LESS (`null`) case — matching what its own doc-comment already claimed — so `content: ""` is returned as "". - The userMessage guard `if (!text) continue;` became `if (text === null) continue;` so an empty-string body flows into the exact / substring / regex match forms (all of which handle "" correctly: exact "" === "", substring "".includes("") === true, /^$/.test("") === true). - The inputText guard `if (!embeddingInput) continue;` became `if (embeddingInput === undefined) continue;` — embeddingInput is `string | undefined`, undefined = absent, "" = a genuinely-empty embedding request. systemMessage is intentionally NOT changed. getSystemText returns "" for BOTH "a present but empty system message" AND "no system message at all", so it exposes no absent-vs-empty distinction at the request level; allowing "" through would turn a `systemMessage: ""` / `/^$/` fixture into a catch-all firing on every no-system-message request. There is no shipped-fixture demand for it, so the falsy guard stays, with a comment explaining the asymmetry and how to add it later. Call sites checked (grep for the extractor guards): - userMessage guard src/router.ts (matcher) — FIXED - systemMessage guard src/router.ts (matcher) — UNCHANGED (see above) - inputText guard src/router.ts (matcher) — FIXED - model branch reads effective.model (required, defaulted ""), no null-vs-"" semantics — untouched - getLastUserText only caller is the userMessage branch Fixtures audit (fixtures/ docs/fixtures/ examples/) finds zero empty-string userMessage/systemMessage/inputText matchers, so no shipped fixture changes behavior. --- src/__tests__/router.test.ts | 87 +++++++++++++++++++++++++++++++++++- src/router.ts | 33 +++++++++++--- 2 files changed, 112 insertions(+), 8 deletions(-) diff --git a/src/__tests__/router.test.ts b/src/__tests__/router.test.ts index 5f63a75b..d68f8280 100644 --- a/src/__tests__/router.test.ts +++ b/src/__tests__/router.test.ts @@ -1,5 +1,11 @@ import { describe, it, expect } from "vitest"; -import { matchFixture, getLastMessageByRole, getSystemText, getTextContent } from "../router.js"; +import { + matchFixture, + getLastMessageByRole, + getSystemText, + getTextContent, + getLastUserText, +} from "../router.js"; import type { ChatCompletionRequest, ChatMessage, ContentPart, Fixture } from "../types.js"; // --------------------------------------------------------------------------- @@ -1224,3 +1230,82 @@ describe("matchFixture — first-match-wins", () => { expect(matchFixture([noMatch, match], req)).toBe(match); }); }); + +// --------------------------------------------------------------------------- +// Item 1 — null-vs-"" empty-body matching +// --------------------------------------------------------------------------- + +describe("getLastUserText — empty-string user message", () => { + it("returns '' for an explicit empty-text user message", () => { + expect(getLastUserText([{ role: "user", content: "" }])).toBe(""); + }); + it("still skips a trailing attachment-only (null-text) user message", () => { + const msgs: ChatMessage[] = [ + { role: "user", content: "describe this" }, + { role: "user", content: [{ type: "image_url", image_url: { url: "x" } }] as ContentPart[] }, + ]; + expect(getLastUserText(msgs)).toBe("describe this"); + }); + it("returns null when no user message has any text part", () => { + const msgs: ChatMessage[] = [ + { role: "user", content: [{ type: "image_url", image_url: { url: "x" } }] as ContentPart[] }, + ]; + expect(getLastUserText(msgs)).toBeNull(); + }); +}); + +describe("matchFixture — empty userMessage", () => { + it("matches userMessage:'' against an empty user message (exact)", () => { + const fx = makeFixture({ userMessage: "" }); + const req = makeReq({ messages: [{ role: "user", content: "" }] }); + expect(matchFixture([fx], req, undefined, (r) => r)).toBe(fx); + }); + it("matches userMessage:/^$/ against an empty user message", () => { + const fx = makeFixture({ userMessage: /^$/ }); + const req = makeReq({ messages: [{ role: "user", content: "" }] }); + expect(matchFixture([fx], req)).toBe(fx); + }); + it("does NOT match empty userMessage against a non-empty message", () => { + const fx = makeFixture({ userMessage: "" }); + const req = makeReq({ messages: [{ role: "user", content: "hello" }] }); + expect(matchFixture([fx], req, undefined, (r) => r)).toBeNull(); + }); + it("truly-absent user text still skips (attachment-only turn, no fixture match)", () => { + const fx = makeFixture({ userMessage: "" }); + const req = makeReq({ + messages: [ + { + role: "user", + content: [{ type: "image_url", image_url: { url: "x" } }] as ContentPart[], + }, + ], + }); + expect(matchFixture([fx], req, undefined, (r) => r)).toBeNull(); + }); +}); + +describe("matchFixture — empty inputText", () => { + it("matches inputText:'' against embeddingInput:'' (exact)", () => { + const fx = makeFixture({ inputText: "" }, { embedding: [0.1] }); + const req = makeReq({ embeddingInput: "" }); + expect(matchFixture([fx], req, undefined, (r) => r)).toBe(fx); + }); + it("matches inputText:/^$/ against embeddingInput:''", () => { + const fx = makeFixture({ inputText: /^$/ }, { embedding: [0.1] }); + const req = makeReq({ embeddingInput: "" }); + expect(matchFixture([fx], req)).toBe(fx); + }); + it("skips when embeddingInput is undefined (absent)", () => { + const fx = makeFixture({ inputText: "" }, { embedding: [0.1] }); + const req = makeReq({}); + expect(matchFixture([fx], req, undefined, (r) => r)).toBeNull(); + }); +}); + +describe("matchFixture — systemMessage empty behavior unchanged", () => { + it("systemMessage:'' does NOT match a request with no system message (documented catch-all avoidance)", () => { + const fx = makeFixture({ systemMessage: "" }); + const req = makeReq({ messages: [{ role: "user", content: "hi" }] }); + expect(matchFixture([fx], req, undefined, (r) => r)).toBeNull(); + }); +}); diff --git a/src/router.ts b/src/router.ts index d577ffcd..7698bbb8 100644 --- a/src/router.ts +++ b/src/router.ts @@ -81,16 +81,19 @@ export function getTextContent(content: string | ContentPart[] | null): string | * naive "last user message" lookup returns `null` and — because no fixture can * key on empty text — every such multimodal fixture misses. We therefore skip * trailing user messages that carry no text and use the nearest preceding user - * message that does. This is deliberately narrow: it only skips text-LESS user - * messages, so a genuine multi-message user turn (each message HAS text) is - * unaffected and still matched on its final message. Returns `null` when no - * user message carries text. + * message that does. This is deliberately narrow: it only skips text-LESS + * (`null`) user messages — a message whose text extracts to an explicit empty + * string `""` is a present-but-empty body and IS returned (so a fixture can key + * on it), while a genuine multi-message user turn (each message HAS text) is + * unaffected and still matched on its final message. Returns the nearest + * preceding user message whose text is non-null (including `""`); returns `null` + * only when no user message carries any text part at all. */ export function getLastUserText(messages: ChatMessage[]): string | null { for (let i = messages.length - 1; i >= 0; i--) { if (messages[i].role !== "user") continue; const text = getTextContent(messages[i].content); - if (text !== null && text !== "") return text; + if (text !== null) return text; } return null; } @@ -305,7 +308,11 @@ export function matchFixtureDiagnostic( // getLastUserText for why a trailing attachment-only user message // (multimodal serialisation split) must not shadow the real prompt. const text = getLastUserText(effective.messages); - if (!text) continue; + // `text === null` means no user message carried any text (e.g. a pure + // attachment turn) — skip. An explicit empty-string body (`""`) is a + // present-but-empty user message and must be allowed through so a fixture + // keyed on empty text can match it (see getLastUserText). + if (text === null) continue; if (typeof match.userMessage === "string") { if (useExactMatch) { if (text !== match.userMessage) continue; @@ -342,6 +349,15 @@ export function matchFixtureDiagnostic( // no constraint — fall through to the next predicate } else { const text = getSystemText(effective.messages); + // Deliberately `!text` (not `text === null`): unlike userMessage/inputText, + // getSystemText returns `""` for BOTH "a present but empty system message" + // AND "no system message at all", so it exposes no absent-vs-empty + // distinction at the request level. Allowing `""` through would make a + // `systemMessage: ""` / `/^$/` fixture a catch-all firing on every + // no-system-message request. There is no shipped-fixture demand for + // matching an empty system prompt, so we keep the falsy guard here. If a + // real need arises, add a getSystemText sibling that returns `null` when + // absent and `""` when present-empty, then switch to `text === null`. if (!text) continue; if (Array.isArray(sm)) { let allPresent = true; @@ -385,7 +401,10 @@ export function matchFixtureDiagnostic( // Same rationale as userMessage above: fixture authors specify exact strings. if (match.inputText !== undefined) { const embeddingInput = effective.embeddingInput; - if (!embeddingInput) continue; + // `undefined` means the request carried no embedding input (a non-embedding + // request) — skip. An explicit empty string (`""`) is a genuinely-empty + // embedding input and must be allowed through so `inputText: ""` can match. + if (embeddingInput === undefined) continue; if (typeof match.inputText === "string") { if (useExactMatch) { if (embeddingInput !== match.inputText) continue; From ae638da684f2b3f4691dc607c82e9014673db7f2 Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Wed, 15 Jul 2026 23:09:55 -0700 Subject: [PATCH 3/3] fix(router): don't mutate caller-supplied RegExp lastIndex when matching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The four RegExp matcher branches each did `match..lastIndex = 0; match..test(text)` in place. For a `/g` or `/y` regex, `RegExp.prototype.test` advances `lastIndex` as a side effect, so a successful match left the caller's own RegExp object with a non-zero `lastIndex` — clobbering a positional scan the caller was running with the same object, and making a `/g` regex reused across fixtures / successive matchFixture calls match intermittently. Introduce a `regexTest(re, text)` helper that tests a fresh `new RegExp(re.source, re.flags)` clone: the caller's object is never touched, every test starts at index 0, and all flag semantics (i/m/s/u and the harmless-for-a-single-test g/y) are preserved. Four sites replaced (reset-line dropped, .test → regexTest): - userMessage src/router.ts - systemMessage src/router.ts (sm narrowed to RegExp in the else branch) - inputText src/router.ts - model src/router.ts Call-site check: `grep lastIndex src/router.ts` now shows only the helper's doc-comment reference — no code sets or reads `match..lastIndex` after matching, so the clone is the sole mediator and no other path depended on the in-place reset. --- src/__tests__/router.test.ts | 62 ++++++++++++++++++++++++++++++++++++ src/router.ts | 25 ++++++++++----- 2 files changed, 79 insertions(+), 8 deletions(-) diff --git a/src/__tests__/router.test.ts b/src/__tests__/router.test.ts index d68f8280..17c963a6 100644 --- a/src/__tests__/router.test.ts +++ b/src/__tests__/router.test.ts @@ -1309,3 +1309,65 @@ describe("matchFixture — systemMessage empty behavior unchanged", () => { expect(matchFixture([fx], req, undefined, (r) => r)).toBeNull(); }); }); + +// --------------------------------------------------------------------------- +// Item 3 — matcher must not mutate caller-supplied RegExp lastIndex +// --------------------------------------------------------------------------- + +describe("matchFixture — does not mutate caller's RegExp lastIndex", () => { + it("leaves a /g userMessage regex lastIndex at 0 after a match", () => { + const re = /hello/g; + const fx = makeFixture({ userMessage: re }); + matchFixture([fx], makeReq({ messages: [{ role: "user", content: "hello" }] })); + expect(re.lastIndex).toBe(0); + }); + it("a /g regex reused across TWO match calls matches BOTH times (no leaked lastIndex)", () => { + const re = /world/g; + const fx = makeFixture({ userMessage: re }); + const req = makeReq({ messages: [{ role: "user", content: "world" }] }); + // Advance the caller's own lastIndex the way an external re.exec would: + re.exec("world world"); // lastIndex now > 0 + expect(matchFixture([fx], req)).toBe(fx); + re.exec("world world"); + expect(matchFixture([fx], req)).toBe(fx); + }); + it("does not clobber the caller's mid-scan lastIndex (userMessage /g)", () => { + const re = /a/g; + re.exec("aaa"); // caller mid-scan, lastIndex === 1 + const fx = makeFixture({ userMessage: re }); + matchFixture([fx], makeReq({ messages: [{ role: "user", content: "a" }] })); + expect(re.lastIndex).toBe(1); + }); + it("systemMessage /g regex lastIndex preserved", () => { + const re = /ctx/g; + re.exec("ctx ctx"); + const before = re.lastIndex; + matchFixture( + [makeFixture({ systemMessage: re })], + makeReq({ + messages: [ + { role: "system", content: "ctx" }, + { role: "user", content: "x" }, + ], + }), + ); + expect(re.lastIndex).toBe(before); + }); + it("inputText /g regex lastIndex preserved", () => { + const re = /q/g; + re.exec("q q"); + const before = re.lastIndex; + matchFixture( + [makeFixture({ inputText: re }, { embedding: [0.1] })], + makeReq({ embeddingInput: "q" }), + ); + expect(re.lastIndex).toBe(before); + }); + it("model /g regex lastIndex preserved", () => { + const re = /gpt/g; + re.exec("gpt gpt"); + const before = re.lastIndex; + matchFixture([makeFixture({ model: re })], makeReq({ model: "gpt-4o" })); + expect(re.lastIndex).toBe(before); + }); +}); diff --git a/src/router.ts b/src/router.ts index 7698bbb8..af3cae6a 100644 --- a/src/router.ts +++ b/src/router.ts @@ -98,6 +98,19 @@ export function getLastUserText(messages: ChatMessage[]): string | null { return null; } +/** + * Test a matcher RegExp against a string WITHOUT mutating the caller-supplied + * regex. `RegExp.prototype.test` advances `lastIndex` as a side effect on + * `/g` and `/y` regexes; fixtures hold caller-owned RegExp objects, so testing + * them in place would clobber the caller's positional state (and a `/g` regex + * reused across match calls would match intermittently). We test a fresh clone + * (same source + flags) so the caller's object is never touched and every test + * starts from index 0. + */ +function regexTest(re: RegExp, text: string): boolean { + return new RegExp(re.source, re.flags).test(text); +} + /** * Result of {@link matchFixtureDiagnostic}: the matched fixture (or `null`) plus * the number of fixtures that matched the request SHAPE (every predicate above @@ -320,8 +333,7 @@ export function matchFixtureDiagnostic( if (!text.includes(match.userMessage)) continue; } } else { - match.userMessage.lastIndex = 0; - if (!match.userMessage.test(text)) continue; + if (!regexTest(match.userMessage, text)) continue; } } @@ -375,8 +387,7 @@ export function matchFixtureDiagnostic( if (!text.includes(sm)) continue; } } else { - sm.lastIndex = 0; - if (!sm.test(text)) continue; + if (!regexTest(sm, text)) continue; } } } @@ -412,8 +423,7 @@ export function matchFixtureDiagnostic( if (!embeddingInput.includes(match.inputText)) continue; } } else { - match.inputText.lastIndex = 0; - if (!match.inputText.test(embeddingInput)) continue; + if (!regexTest(match.inputText, embeddingInput)) continue; } } @@ -434,8 +444,7 @@ export function matchFixtureDiagnostic( if (!/^-\d/.test(rest)) continue; } } else { - match.model.lastIndex = 0; - if (!match.model.test(effective.model ?? "")) continue; + if (!regexTest(match.model, effective.model ?? "")) continue; } }