Skip to content

fix(sdk): support multimodal array inputs (e.g. image URLs) in prompt compilation #756

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ckr-rai73 wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix(sdk): support multimodal array inputs (e.g. image URLs) in prompt compilation #756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2b8edc3
fix(sdk): resolve string vs. object type mismatch in prompt compilation
ckr-rai73 Mar 21, 2026
7a4d87f
test: add coverage for multimodal main-loop and langchain paths
ckr-rai73 Mar 21, 2026
28a0e42
test: add coverage for multimodal main-loop and langchain paths
ckr-rai73 Mar 21, 2026
0fc6ae6
test: add coverage for multimodal main-loop and langchain paths
ckr-rai73 Mar 21, 2026
49b5609
fix(sdk): implement FIFO cache eviction and unique test signatures
ckr-rai73 Mar 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 81 additions & 45 deletions packages/client/src/prompt/promptClients.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,8 @@ export class TextPromptClient extends BasePromptClient {
* @public
*/
export class ChatPromptClient extends BasePromptClient {
private static readonly MAX_CACHE_SIZE = 500;
private static readonly mustacheTokenCache = new Map<string, any[]>();
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Unbounded static cache is a memory leak

mustacheTokenCache is a static Map with no maximum size and no eviction policy. Because it lives for the entire lifetime of the process, applications that process many distinct prompt templates (e.g. dynamically generated templates, per-tenant variations, or prompts whose text changes over time) will accumulate entries indefinitely, potentially causing out-of-memory failures in long-running servers.

A Map in JavaScript preserves insertion order, so implementing a simple bounded cache is straightforward: before adding a new entry, check if the map has reached its maximum size and evict the oldest key (the first entry returned by the iterator) if so. A cap of a few hundred entries is typically enough to warm the common case while preventing unbounded growth.

/** The original prompt response from the API */
public readonly promptResponse: Prompt.Chat;
/** The chat messages that make up the prompt */
Expand Down Expand Up @@ -328,13 +330,68 @@ export class ChatPromptClient extends BasePromptClient {
variables?: Record<string, string>,
placeholders?: Record<string, any>,
): (ChatMessageOrPlaceholder | any)[] {
const messagesWithPlaceholdersReplaced: (ChatMessageOrPlaceholder | any)[] =
[];
const placeholderValues = placeholders ?? {};
const vars = variables ?? {};
const phs = placeholders ?? {};

// 4. ZERO-ALLOCATION CHECK: Skip entirely if no injections needed
if (Object.keys(vars).length === 0 && Object.keys(phs).length === 0) {
return this.prompt.map((item) => {
if ("type" in item && item.type === ChatMessageType.ChatMessage) {
return { role: item.role, content: item.content };
}
return item;
});
}

const result: (ChatMessageOrPlaceholder | any)[] = [];
const writer = new mustache.Writer();

for (const item of this.prompt) {
if ("type" in item && item.type === ChatMessageType.Placeholder) {
const placeholderValue = placeholderValues[item.name];
if (item.type === ChatMessageType.ChatMessage) {
// 3. REFINED TYPE-GUARD: Check content type as the very first operation
if (typeof item.content !== "string") {
result.push({
role: item.role,
content: item.content,
});
continue;
}

// 2. TOKEN CACHING: Use static Map to bypass expensive parsing (Memory-Safe)
let tokens = ChatPromptClient.mustacheTokenCache.get(item.content);
if (!tokens) {
if (
ChatPromptClient.mustacheTokenCache.size >=
ChatPromptClient.MAX_CACHE_SIZE
) {
// FIFO Eviction: Eject the oldest entry
const oldestKey = ChatPromptClient.mustacheTokenCache
.keys()
.next().value;
if (oldestKey !== undefined) {
ChatPromptClient.mustacheTokenCache.delete(oldestKey);
}
}
tokens = mustache.parse(item.content);
ChatPromptClient.mustacheTokenCache.set(item.content, tokens);
}

const rendered = writer.renderTokens(
tokens,
new mustache.Context(vars),
undefined,
item.content,
);
result.push({
role: item.role,
content: rendered,
});
continue;
}

// Handle Placeholders (preserved logic)
if (item.type === ChatMessageType.Placeholder) {
const placeholderValue = phs[item.name];
if (
Array.isArray(placeholderValue) &&
placeholderValue.length > 0 &&
Expand All @@ -343,54 +400,27 @@ export class ChatPromptClient extends BasePromptClient {
typeof msg === "object" && "role" in msg && "content" in msg,
)
) {
messagesWithPlaceholdersReplaced.push(
...(placeholderValue as ChatMessage[]),
);
result.push(...(placeholderValue as ChatMessage[]));
} else if (
Array.isArray(placeholderValue) &&
placeholderValue.length === 0
) {
// Empty array provided - skip placeholder (don't include it)
// Skip empty placeholder array
} else if (placeholderValue !== undefined) {
// Non-standard placeholder value format, just stringfiy
messagesWithPlaceholdersReplaced.push(
JSON.stringify(placeholderValue),
);
// Stringify non-standard formats
result.push(JSON.stringify(placeholderValue));
} else {
// Keep unresolved placeholder in the output
messagesWithPlaceholdersReplaced.push(
item as { type: ChatMessageType.Placeholder } & typeof item,
);
// Keep unresolved placeholder
result.push(item);
}
} else if (
"role" in item &&
"content" in item &&
item.type === ChatMessageType.ChatMessage
) {
messagesWithPlaceholdersReplaced.push({
role: item.role,
content: item.content,
});
continue;
Comment on lines 393 to +416
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Regression: placeholder-injected messages no longer receive Mustache variable substitution

In the original code, the compile pipeline was a two-pass process:

  1. First loop resolved placeholders → collected all messages (including injected ones) into messagesWithPlaceholdersReplaced.
  2. Second .map() pass applied mustache.render(item.content, variables) to every message in that array — including ones that had just been injected via a placeholder.

In the refactored code the two passes are merged into one loop. ChatMessage items from this.prompt are rendered correctly, but messages that arrive through a Placeholder value are pushed directly into result without any Mustache rendering:

result.push(...(placeholderValue as ChatMessage[]));
// ↑ No variable substitution applied to these messages

Any user who combines variables with placeholders where the injected messages contain {{variable}} patterns will silently receive unrendered templates instead of the expected output. For example:

// compile({ user: "Alice" }, { history: [{ role: "user", content: "Hello {{user}}" }] })
// Before: [{ role: "user", content: "Hello Alice" }]
// After:  [{ role: "user", content: "Hello {{user}}" }]   ← regression

The injected messages need to go through the same Mustache rendering step before being pushed to result.

}

// Catch-all for any other message types
result.push(item);
}

return messagesWithPlaceholdersReplaced.map((item) => {
if (
typeof item === "object" &&
item !== null &&
"role" in item &&
"content" in item &&
typeof item.content === 'string'
) {
return {
...item,
content: mustache.render(item.content, variables ?? {}),
};
} else {
// Return placeholder or stringified value as-is
return item;
}
});
return result;
}

/**
Expand Down Expand Up @@ -438,7 +468,10 @@ export class ChatPromptClient extends BasePromptClient {
...(placeholderValue as ChatMessage[]).map((msg) => {
return {
role: msg.role,
content: this._transformToLangchainVariables(msg.content),
content:
typeof msg.content === "string"
? this._transformToLangchainVariables(msg.content)
: msg.content,
};
}),
);
Expand Down Expand Up @@ -469,7 +502,10 @@ export class ChatPromptClient extends BasePromptClient {
) {
messagesWithPlaceholdersReplaced.push({
role: item.role,
content: this._transformToLangchainVariables(item.content),
content:
typeof item.content === "string"
? this._transformToLangchainVariables(item.content)
: item.content,
});
}
}
Expand Down
18 changes: 18 additions & 0 deletions packages/client/tsup.config.bundled_nv8l3rehalb.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// tsup.config.ts
import { defineConfig } from "tsup";
var tsup_config_default = defineConfig({
entry: ["src/index.ts"],
format: ["cjs", "esm"],
dts: true,
splitting: false,
sourcemap: true,
clean: true,
outDir: "dist",
outExtension: ({ format }) => ({
js: format === "cjs" ? ".cjs" : ".mjs"
})
});
export {
tsup_config_default as default
};
//# sourceMappingURL=data:application/json;base64,ewogICJ2ZXJzaW9uIjogMywKICAic291cmNlcyI6IFsidHN1cC5jb25maWcudHMiXSwKICAic291cmNlc0NvbnRlbnQiOiBbImNvbnN0IF9faW5qZWN0ZWRfZmlsZW5hbWVfXyA9IFwiQzpcXFxcVXNlcnNcXFxcUFJBVkVFTiBSQUlcXFxcbGFuZ2Z1c2UtanNcXFxccGFja2FnZXNcXFxcY2xpZW50XFxcXHRzdXAuY29uZmlnLnRzXCI7Y29uc3QgX19pbmplY3RlZF9kaXJuYW1lX18gPSBcIkM6XFxcXFVzZXJzXFxcXFBSQVZFRU4gUkFJXFxcXGxhbmdmdXNlLWpzXFxcXHBhY2thZ2VzXFxcXGNsaWVudFwiO2NvbnN0IF9faW5qZWN0ZWRfaW1wb3J0X21ldGFfdXJsX18gPSBcImZpbGU6Ly8vQzovVXNlcnMvUFJBVkVFTiUyMFJBSS9sYW5nZnVzZS1qcy9wYWNrYWdlcy9jbGllbnQvdHN1cC5jb25maWcudHNcIjtpbXBvcnQgeyBkZWZpbmVDb25maWcgfSBmcm9tIFwidHN1cFwiO1xuXG5leHBvcnQgZGVmYXVsdCBkZWZpbmVDb25maWcoe1xuICBlbnRyeTogW1wic3JjL2luZGV4LnRzXCJdLFxuICBmb3JtYXQ6IFtcImNqc1wiLCBcImVzbVwiXSxcbiAgZHRzOiB0cnVlLFxuICBzcGxpdHRpbmc6IGZhbHNlLFxuICBzb3VyY2VtYXA6IHRydWUsXG4gIGNsZWFuOiB0cnVlLFxuICBvdXREaXI6IFwiZGlzdFwiLFxuICBvdXRFeHRlbnNpb246ICh7IGZvcm1hdCB9KSA9PiAoe1xuICAgIGpzOiBmb3JtYXQgPT09IFwiY2pzXCIgPyBcIi5janNcIiA6IFwiLm1qc1wiLFxuICB9KSxcbn0pO1xuIl0sCiAgIm1hcHBpbmdzIjogIjtBQUE0UyxTQUFTLG9CQUFvQjtBQUV6VSxJQUFPLHNCQUFRLGFBQWE7QUFBQSxFQUMxQixPQUFPLENBQUMsY0FBYztBQUFBLEVBQ3RCLFFBQVEsQ0FBQyxPQUFPLEtBQUs7QUFBQSxFQUNyQixLQUFLO0FBQUEsRUFDTCxXQUFXO0FBQUEsRUFDWCxXQUFXO0FBQUEsRUFDWCxPQUFPO0FBQUEsRUFDUCxRQUFRO0FBQUEsRUFDUixjQUFjLENBQUMsRUFBRSxPQUFPLE9BQU87QUFBQSxJQUM3QixJQUFJLFdBQVcsUUFBUSxTQUFTO0FBQUEsRUFDbEM7QUFDRixDQUFDOyIsCiAgIm5hbWVzIjogW10KfQo=
Comment on lines +1 to +18
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0 Accidental build artifact committed

This is a generated tsup intermediary file with a randomised suffix (bundled_nv8l3rehalb) that was unintentionally committed. It should never live in source control.

More critically, the embedded base64 source map decodes to a Windows absolute path that leaks the developer's local machine username and directory layout:

"C:\\Users\\PRAVEEN RAI\\langfuse-js\\packages\\client\\tsup.config.ts"

This file should be deleted from the branch and added to .gitignore (pattern tsup.config.bundled_*.mjs) to prevent re-occurrence.

123 changes: 123 additions & 0 deletions tests/e2e/prompts.e2e.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2151,5 +2151,128 @@ Configuration:
expect(formattedMessages[1].content).toBe(expectedUser);
});
});

it("should handle multimodal array inputs for ChatMessage types", () => {
const promptClient = new ChatPromptClient({
name: "multimodal-test",
type: "chat",
version: 1,
prompt: [
{
type: ChatMessageType.ChatMessage,
role: "user",
content: {
attachments: [
{
type: "image_url",
image_url: { url: "https://example.com/image.png" },
},
],
} as any,
},
],
config: {},
labels: [],
tags: [],
});

// After fix: Should return the multimodal content as-is without crashing
const compiled = promptClient.compile();
expect(compiled[0].content).toEqual({
attachments: [
{
type: "image_url",
image_url: { url: "https://example.com/image.png" },
},
],
});
Comment on lines +2180 to +2188
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Test only exercises early-exit path, not the actual type guard

promptClient.compile() is called with no arguments, so Object.keys(vars).length === 0 && Object.keys(phs).length === 0 is true and the function returns early at line 336. The new typeof item.content !== "string" type guard added to the main loop (line 351-357) — which handles the case when variables are provided alongside multimodal content — is never exercised.

To cover the primary fix, you should also test the main-loop path:

// Also test when variables are provided (exercises the main loop type guard)
const compiledWithVars = promptClient.compile({ someVar: "value" });
expect(compiledWithVars[0].content).toEqual({
  attachments: [
    {
      type: "image_url",
      image_url: { url: "https://example.com/image.png" },
    },
  ],
});

Without this, the regression test does not guard against future regressions specifically in the main loop path.

});

it("should handle multimodal array inputs in the main loop with variables", () => {
const promptClient = new ChatPromptClient({
name: "multimodal-test",
type: "chat",
version: 1,
prompt: [
{
type: ChatMessageType.ChatMessage,
role: "user",
content: {
attachments: [
{
type: "image_url",
image_url: { url: "https://example.com/image.png" },
},
],
} as any,
},
{
type: ChatMessageType.ChatMessage,
role: "user",
content: "Dummy: {{dummy}}",
},
],
config: {},
labels: [],
tags: [],
});

// Passing a variable forces execution into the main loop
const compiled = promptClient.compile({ dummy: "test" });
expect(compiled[0].content).toEqual({
attachments: [
{
type: "image_url",
image_url: { url: "https://example.com/image.png" },
},
],
});
expect(compiled[1].content).toEqual("Dummy: test");
});

it("should handle multimodal content in getLangchainPrompt safely", () => {
const promptClient = new ChatPromptClient({
name: "multimodal-langchain-test",
type: "chat",
version: 1,
prompt: [
{
type: ChatMessageType.ChatMessage,
role: "user",
content: {
attachments: [
{
type: "image_url",
image_url: { url: "https://example.com/image.png" },
},
],
} as any,
},
],
config: {},
labels: [],
tags: [],
});
});
Comment on lines +2233 to +2256
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Empty test body — getLangchainPrompt() is never called

The test creates a ChatPromptClient but then the function ends without ever invoking promptClient.getLangchainPrompt() or making any assertions. As written, this test will always pass regardless of regressions in getLangchainPrompt.

The fix added in getLangchainPrompt() (the typeof item.content === "string" guard at lines 506-508 in promptClients.ts) is the higher-impact change because _transformToLangchainVariables calls .replace() which throws at runtime on a non-string value — yet this guard has zero effective coverage.

The method must be called and its output verified:

it("should handle multimodal content in getLangchainPrompt safely", () => {
  const promptClient = new ChatPromptClient({ /* same setup */ });

  // This call would throw before the fix
  const result = promptClient.getLangchainPrompt();
  expect(result[0].content).toEqual({
    attachments: [
      {
        type: "image_url",
        image_url: { url: "https://example.com/image.png" },
      },
    ],
  });
});


it("should handle multimodal content in getLangchainPrompt for non-ChatMessage items", () => {
const promptClient = new ChatPromptClient({
name: "multimodal-non-chat-test",
type: "chat",
version: 1,
prompt: [
{
type: ChatMessageType.Placeholder,
name: "multimodal_placeholder",
},
],
config: {},
labels: [],
tags: [],
});

const result = promptClient.getLangchainPrompt();
expect(result[0]).toEqual(["placeholder", "{multimodal_placeholder}"]);
});
});
});
Loading