Skip to content

Commit 5052e8c

Browse files
mzeglamzegla
authored
Change default chat template (#3723)
Intermediate change for merging: #3722
1 parent 14cb469 commit 5052e8c

File tree

4 files changed

+48
-335
lines changed

4 files changed

+48
-335
lines changed

‎src/llm/servable_initializer.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
namespace ovms {
5252

5353
static const std::string CHAT_TEMPLATE_WARNING_MESSAGE = "Warning: Chat template has not been loaded properly. Servable will not respond to /chat/completions endpoint.";
54-
static const std::string DEFAULT_CHAT_TEMPLATE = R"({% if messages|length != 1 %} {{ raise_exception('This servable accepts only single message requests') }}{% endif %}{{ messages[0]['content'] }})";
54+
static const std::string DEFAULT_CHAT_TEMPLATE = R"({% for message in messages %}{% if message['role'] == 'user' %}{{ 'User: ' + message['content'] }}{% elif message['role'] == 'system' %}{{ '<|system|>\n' + message['content'] + eos_token }}{% elif message['role'] == 'assistant' %}{{ message['content'] + eos_token }}{% endif %}{% endfor %})";
5555

5656
void GenAiServableInitializer::loadChatTemplate(std::shared_ptr<GenAiServableProperties> properties, const std::string& chatTemplateDirectory) {
5757
#if (PYTHON_DISABLE == 0)
@@ -217,7 +217,7 @@ void GenAiServableInitializer::loadPyTemplateProcessor(std::shared_ptr<GenAiServ
217217
218218
# Default chat template accepts only single message and outputs only it's 'content'
219219
# effectively turning it into a regular prompt.
220-
default_chat_template = "{% if messages|length != 1 %} {{ raise_exception('This servable accepts only single message requests') }}{% endif %}{{ messages[0]['content'] }}"
220+
default_chat_template = "{% for message in messages %}{% if message['role'] == 'user' %}{{ 'User: ' + message['content'] }}{% elif message['role'] == 'system' %}{{ '<|system|>\n' + message['content'] + eos_token }}{% elif message['role'] == 'assistant' %}{{ message['content'] + eos_token }}{% endif %}{% endfor %}"
221221
222222
bos_token = ""
223223
eos_token = ""

‎src/test/llm/assisted_decoding_test.cpp‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,12 @@ class AssistedDecodingPipelinesHttpTest : public ::testing::Test {
9494
}
9595
}
9696

97-
int generateExpectedText(std::string prompt, bool addSpecialTokens) {
97+
int generateExpectedText(std::string prompt, bool addSpecialTokens, bool applyChatTemplate) {
9898
try {
99+
if (applyChatTemplate) {
100+
ov::genai::ChatHistory chatHistory({{{"role", "user"}, {"content", prompt}}});
101+
prompt = cbPipe->get_tokenizer().apply_chat_template(chatHistory, true);
102+
}
99103
ov::Tensor promptIds = cbPipe->get_tokenizer().encode(prompt, ov::genai::add_special_tokens(addSpecialTokens)).input_ids;
100104
std::cout << "Generated prompt ids: " << getPromptTokensString(promptIds) << std::endl;
101105
auto generationHandle = cbPipe->add_request(
@@ -162,7 +166,7 @@ TEST_F(AssistedDecodingPipelinesHttpTest, unaryCompletionsJsonSpeculativeDecodin
162166
// Generate reference from the base model (unassisted generation)
163167
config.max_new_tokens = 10;
164168
config.temperature = 0;
165-
ASSERT_EQ(generateExpectedText("What is OpenVINO?", true), 0);
169+
ASSERT_EQ(generateExpectedText("What is OpenVINO?", true, false), 0);
166170
ASSERT_EQ(config.num_return_sequences, expectedMessages.size());
167171

168172
// Static number of candidates
@@ -185,8 +189,7 @@ TEST_F(AssistedDecodingPipelinesHttpTest, unaryCompletionsJsonSpeculativeDecodin
185189
ASSERT_EQ(parsedResponse["choices"].Capacity(), 1);
186190
auto& choice = parsedResponse["choices"].GetArray()[0];
187191
ASSERT_TRUE(choice["text"].IsString());
188-
// TODO: awaiting OV/GenAI fix, uncomment when fixed
189-
// EXPECT_STREQ(choice["text"].GetString(), expectedMessages[0].c_str());
192+
EXPECT_STREQ(choice["text"].GetString(), expectedMessages[0].c_str());
190193

191194
// Dynamic number of candidates
192195
requestBody = R"(
@@ -208,15 +211,15 @@ TEST_F(AssistedDecodingPipelinesHttpTest, unaryCompletionsJsonSpeculativeDecodin
208211
ASSERT_EQ(parsedResponse["choices"].Capacity(), 1);
209212
choice = parsedResponse["choices"].GetArray()[0];
210213
ASSERT_TRUE(choice["text"].IsString());
211-
// TODO: awaiting OV/GenAI fix, uncomment when fixed
212-
// EXPECT_STREQ(choice["text"].GetString(), expectedMessages[0].c_str());
214+
EXPECT_STREQ(choice["text"].GetString(), expectedMessages[0].c_str());
213215
}
214216

215217
TEST_F(AssistedDecodingPipelinesHttpTest, unaryChatCompletionsJsonSpeculativeDecoding) {
218+
GTEST_SKIP(); // TODO: Temporary skip to synchronize CI workers
216219
// Generate reference from the base model (unassisted generation)
217220
config.max_new_tokens = 10;
218221
config.temperature = 0;
219-
ASSERT_EQ(generateExpectedText("What is OpenVINO?", false), 0);
222+
ASSERT_EQ(generateExpectedText("What is OpenVINO?", false, true), 0);
220223
ASSERT_EQ(config.num_return_sequences, expectedMessages.size());
221224

222225
// Static number of candidates
@@ -247,8 +250,7 @@ TEST_F(AssistedDecodingPipelinesHttpTest, unaryChatCompletionsJsonSpeculativeDec
247250
ASSERT_TRUE(choice["message"]["content"].IsString());
248251
ASSERT_TRUE(choice["finish_reason"].IsString());
249252
ASSERT_FALSE(choice["logprobs"].IsObject());
250-
// TODO: awaiting OV/GenAI fix, uncomment when fixed
251-
// ASSERT_EQ(choice["message"]["content"].GetString(), expectedMessages[0]);
253+
ASSERT_EQ(choice["message"]["content"].GetString(), expectedMessages[0]);
252254

253255
// Dynamic number of candidates
254256
requestBody = R"(
@@ -278,8 +280,7 @@ TEST_F(AssistedDecodingPipelinesHttpTest, unaryChatCompletionsJsonSpeculativeDec
278280
ASSERT_TRUE(choice["message"]["content"].IsString());
279281
ASSERT_TRUE(choice["finish_reason"].IsString());
280282
ASSERT_FALSE(choice["logprobs"].IsObject());
281-
// TODO: awaiting OV/GenAI fix, uncomment when fixed
282-
// ASSERT_EQ(choice["message"]["content"].GetString(), expectedMessages[0]);
283+
ASSERT_EQ(choice["message"]["content"].GetString(), expectedMessages[0]);
283284
}
284285

285286
TEST_F(AssistedDecodingPipelinesHttpTest, speculativeDecodingExclusiveParametersProvided) {
@@ -318,7 +319,7 @@ TEST_F(AssistedDecodingPipelinesHttpTest, unaryCompletionsJsonPromptLookupDecodi
318319
// Generate reference from the base model (unassisted generation)
319320
config.max_new_tokens = 10;
320321
config.temperature = 0;
321-
ASSERT_EQ(generateExpectedText("What is OpenVINO?", true), 0);
322+
ASSERT_EQ(generateExpectedText("What is OpenVINO?", true, false), 0);
322323
ASSERT_EQ(config.num_return_sequences, expectedMessages.size());
323324

324325
std::string requestBody = R"(
@@ -341,15 +342,15 @@ TEST_F(AssistedDecodingPipelinesHttpTest, unaryCompletionsJsonPromptLookupDecodi
341342
ASSERT_EQ(parsedResponse["choices"].Capacity(), 1);
342343
auto& choice = parsedResponse["choices"].GetArray()[0];
343344
ASSERT_TRUE(choice["text"].IsString());
344-
// TODO: awaiting OV/GenAI fix, uncomment when fixed
345-
// EXPECT_STREQ(choice["text"].GetString(), expectedMessages[0].c_str());
345+
EXPECT_STREQ(choice["text"].GetString(), expectedMessages[0].c_str());
346346
}
347347

348348
TEST_F(AssistedDecodingPipelinesHttpTest, unaryChatCompletionsJsonPromptLookupDecoding) {
349+
GTEST_SKIP(); // TODO: Temporary skip to synchronize CI workers
349350
// Generate reference from the base model (unassisted generation)
350351
config.max_new_tokens = 10;
351352
config.temperature = 0;
352-
ASSERT_EQ(generateExpectedText("What is OpenVINO?", false), 0);
353+
ASSERT_EQ(generateExpectedText("What is OpenVINO?", false, true), 0);
353354
ASSERT_EQ(config.num_return_sequences, expectedMessages.size());
354355

355356
auto requestBody = R"(
@@ -380,8 +381,7 @@ TEST_F(AssistedDecodingPipelinesHttpTest, unaryChatCompletionsJsonPromptLookupDe
380381
ASSERT_TRUE(choice["message"]["content"].IsString());
381382
ASSERT_TRUE(choice["finish_reason"].IsString());
382383
ASSERT_FALSE(choice["logprobs"].IsObject());
383-
// TODO: awaiting OV/GenAI fix, uncomment when fixed
384-
// ASSERT_EQ(choice["message"]["content"].GetString(), expectedMessages[0]);
384+
ASSERT_EQ(choice["message"]["content"].GetString(), expectedMessages[0]);
385385
}
386386

387387
// Consider parametrization of negative tests with request body and endpoint as parameters

‎src/test/llm/llmnode_test.cpp‎

Lines changed: 13 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,12 @@ class LLMFlowHttpTest : public ::testing::Test {
108108
}
109109
}
110110

111-
int generateExpectedText(std::string prompt, bool addSpecialTokens = true) {
111+
int generateExpectedText(std::string prompt, bool addSpecialTokens = true, bool applyChatTemplate = false) {
112112
try {
113+
if (applyChatTemplate) {
114+
ov::genai::ChatHistory chatHistory({{{"role", "user"}, {"content", prompt}}});
115+
prompt = cbPipe->get_tokenizer().apply_chat_template(chatHistory, true);
116+
}
113117
ov::Tensor promptIds = cbPipe->get_tokenizer().encode(prompt, ov::genai::add_special_tokens(addSpecialTokens)).input_ids;
114118
std::cout << "Generated prompt ids: " << getPromptTokensString(promptIds) << std::endl;
115119
auto generationHandle = cbPipe->add_request(
@@ -737,14 +741,15 @@ TEST_P(LLMFlowHttpTestParameterized, unaryChatCompletionsJsonNFail) {
737741
}
738742

739743
TEST_P(LLMFlowHttpTestParameterized, unaryChatCompletionsJsonN) {
744+
GTEST_SKIP(); // TODO: Temporary skip to synchronize CI workers
740745
auto params = GetParam();
741746
config.max_new_tokens = 5;
742747
config.rng_seed = 1;
743748
config.num_beams = 16;
744749
config.num_return_sequences = 8;
745750
config.echo = false;
746751
if (params.generateExpectedOutput) {
747-
ASSERT_EQ(generateExpectedText("What is OpenVINO?", false), 0);
752+
ASSERT_EQ(generateExpectedText("What is OpenVINO?", false, true), 0);
748753
ASSERT_EQ(config.num_return_sequences, expectedMessages.size());
749754
}
750755
std::string requestBody = R"(
@@ -2754,34 +2759,7 @@ TEST_P(LLMHttpParametersValidationTest, missingContentInMessage) {
27542759
)";
27552760

27562761
ovms::Status status = handler->dispatchToProcessor(endpointChatCompletions, requestBody, &response, comp, responseComponents, writer, multiPartParser);
2757-
#if (PYTHON_DISABLE == 0)
2758-
bool genAiTemplateParsing = false; // With Python enabled, we use native Jinja2 template parsing
2759-
#else
2760-
bool genAiTemplateParsing = true; // With Python disabled, we use GenAI template parsing
2761-
#endif
2762-
2763-
if (params.modelName.find("vlm") != std::string::npos) {
2764-
ASSERT_EQ(status.getCode(), ovms::StatusCode::OK); // GenAI accepts such messages, so we expect a successful response
2765-
return;
2766-
}
2767-
2768-
if (genAiTemplateParsing) {
2769-
/*
2770-
This test checks if API handler validation allows messages without content.
2771-
The reason why we expect generic error here is that with GenAI template rendering missing content is unexpected.
2772-
On the API handler level this is a positive path as this test confirms that request reaches template processing phase.
2773-
*/
2774-
ASSERT_EQ(status.getCode(), ovms::StatusCode::MEDIAPIPE_EXECUTION_ERROR);
2775-
ASSERT_NE(status.string().find("Response generation failed"), std::string::npos);
2776-
} else {
2777-
/*
2778-
This test checks if API handler validation allows messages without content.
2779-
The reason why we expect error here is that for the tested LLM model, lack of content means that pipeline input is empty.
2780-
On the API handler level this is a positive path as this test confirms that request reaches template processing phase.
2781-
*/
2782-
ASSERT_EQ(status.getCode(), ovms::StatusCode::MEDIAPIPE_EXECUTION_ERROR);
2783-
ASSERT_NE(status.string().find("Final prompt after applying chat template is empty"), std::string::npos);
2784-
}
2762+
ASSERT_EQ(status.getCode(), ovms::StatusCode::OK);
27852763
}
27862764

27872765
TEST_P(LLMHttpParametersValidationTest, roleNotAString) {
@@ -3267,19 +3245,13 @@ TEST_P(LLMHttpParametersValidationTest, MessagesWithOnlyRole) {
32673245
{
32683246
"model": ")" + params.modelName +
32693247
R"(",
3270-
"messages": [{"role": "abc"}]
3248+
"messages": [{"role": "user"}]
32713249
}
32723250
)";
32733251

3274-
if (params.modelName.find("vlm") != std::string::npos) {
3275-
ASSERT_EQ(
3276-
handler->dispatchToProcessor(endpointChatCompletions, requestBody, &response, comp, responseComponents, writer, multiPartParser),
3277-
ovms::StatusCode::OK); // GenAI supports such messages
3278-
} else {
3279-
ASSERT_EQ(
3280-
handler->dispatchToProcessor(endpointChatCompletions, requestBody, &response, comp, responseComponents, writer, multiPartParser),
3281-
ovms::StatusCode::MEDIAPIPE_EXECUTION_ERROR);
3282-
}
3252+
ASSERT_EQ(
3253+
handler->dispatchToProcessor(endpointChatCompletions, requestBody, &response, comp, responseComponents, writer, multiPartParser),
3254+
ovms::StatusCode::OK); // GenAI supports such messages
32833255
}
32843256

32853257
TEST_P(LLMHttpParametersValidationTest, SpeculativeDecodingNoSDSpecificParametersProvided) {
@@ -3345,7 +3317,7 @@ TEST_P(LLMHttpParametersValidationTest, MessagesWithMoreMessageFields) {
33453317
"model": ")" + params.modelName +
33463318
R"(",
33473319
"max_tokens": 1,
3348-
"messages": [{"role": "123", "content": "def", "unexpected": "123"}]
3320+
"messages": [{"role": "user", "content": "def", "unexpected": "123"}]
33493321
}
33503322
)";
33513323

0 commit comments

Comments
 (0)