Skip to content

fix: reject repeated and out-of-order template placeholders #673

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
ktz03 wants to merge 2 commits into
base: master
Choose a base branch
from
+73 −0
Open

fix: reject repeated and out-of-order template placeholders #673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0690871
fix: reject repeated and out-of-order template placeholders
ktz03 Oct 9, 2026
819189f
Merge branch 'master' into fix/template-trailing-placeholders
giladgd Oct 19, 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
9 changes: 9 additions & 0 deletions src/utils/parseTextTemplate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,15 @@ export function parseTextTemplate<const Parts extends readonly TextTemplatePart[
if (typeof textPart === "string")
continue;

const matchingPartIndex = parts.findIndex((part) => part.text === textPart.separator);
if (matchingPartIndex < partIndex) {
const matchingPart = parts[matchingPartIndex]!;
if (result[matchingPart.key as Parts[number]["key"]] != null)
throw new Error(`Template must contain exactly one "${matchingPart.text}"`);

throw new Error(`Template must contain "${matchingPart.text}" before "${parts[partIndex - 1]!.text}"`);
}

@giladgd giladgd Oct 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this is the right fix since this function can support receiving the same separator more than once (with different keys).
What we should validate and throw on is whether after iterating all the parts we're still left with unused text separators (from splitText) which would indicate that the template contains additional instances of the separators we support (and already parsed), but after the expected sequence

for (; partIndex < parts.length; partIndex++) {
const part = parts[partIndex]!;

Expand Down
36 changes: 36 additions & 0 deletions test/standalone/chatWrappers/generic/TemplateChatWrapper.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,42 @@ import {defaultChatSystemPrompt} from "../../../../src/config.js";


describe("TemplateChatWrapper", () => {
test.each([
{template: "{{history}}model: {{completion}} done {{completion}}", part: "{{completion}}"},
{template: "{{history}}model: {{completion}} done {{history}}", part: "{{history}}"}
] as const)("rejects a trailing $part", ({template, part}) => {
expect(() => new TemplateChatWrapper({
template,
historyTemplate: {
system: "system: {{message}}\n",
user: "user: {{message}}\n",
model: "model: {{message}}\n"
}
})).toThrow(`Template must contain exactly one "${part}"`);
});

test("rejects duplicate history message placeholders", () => {
expect(() => new TemplateChatWrapper({
template: "{{history}}model: {{completion}}\nuser: ",
historyTemplate: {
system: "system: {{message}}\n",
user: "user: {{message}} / {{message}}\n",
model: "model: {{message}}\n"
}
})).toThrow('Template must contain exactly one "{{message}}"');
});

test("rejects a system prompt after completion", () => {
expect(() => new TemplateChatWrapper({
template: "{{history}}model: {{completion}}\nuser: {{systemPrompt}}",
historyTemplate: {
system: "system: {{message}}\n",
user: "user: {{message}}\n",
model: "model: {{message}}\n"
}
})).toThrow('Template must contain "{{systemPrompt}}" before "{{completion}}"');
});

const conversationHistory: ChatHistoryItem[] = [{
type: "system",
text: defaultChatSystemPrompt
Expand Down
28 changes: 28 additions & 0 deletions test/standalone/utils/parseTextTemplate.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,34 @@ import {parseTextTemplate} from "../../../src/utils/parseTextTemplate.js";

describe("utils", () => {
describe("parseTextTemplate", () => {
test.each(["{{name}}", "{{thing}}"])("rejects trailing duplicate %s", (part) => {
expect(() => parseTextTemplate(
`Hello, {{name}}! What is the {{thing}}? ${part} ignored text`,
[{key: "name", text: "{{name}}"}, {key: "thing", text: "{{thing}}"}]
)).toThrow(`Template must contain exactly one "${part}"`);
});

test("rejects a duplicate in a single-part template", () => {
expect(() => parseTextTemplate(
"{{message}} / {{message}} trailing text",
[{key: "message", text: "{{message}}"}]
)).toThrow('Template must contain exactly one "{{message}}"');
});

test("rejects a duplicate before a trailing optional part", () => {
expect(() => parseTextTemplate(
"{{name}} / {{name}} trailing text",
[{key: "name", text: "{{name}}"}, {key: "thing", text: "{{thing}}", optional: true}]
)).toThrow('Template must contain exactly one "{{name}}"');
});

test("rejects a skipped optional part after later parts", () => {
expect(() => parseTextTemplate(
"{{thing}} / {{name}} trailing text",
[{key: "name", text: "{{name}}", optional: true}, {key: "thing", text: "{{thing}}"}]
)).toThrow('Template must contain "{{name}}" before "{{thing}}"');
});

test("normal", async () => {
const res = parseTextTemplate(
"Hello, {{name}}! What is the {{thing}}?",
Expand Down
Loading