Skip to content

Commit 81b9210

Browse files
committed
Address PR #19533 review: guard test on jinja2cpp; document empty tools default
- test/CMakeLists.txt: exclude test_jinja_chat_formatter.cpp from the test binary when jinja2cpp isn't built (mirrors the runner CMake guard), so building tests without the chat_template subdir doesn't fail to link with undefined JinjaChatFormatter symbols (review #4). - jinja_chat_formatter.cpp: document that the empty `tools` list is intentionally falsy so the normalized no-tools template path renders (review #1).
1 parent 7c3280c commit 81b9210

2 files changed

Lines changed: 13 additions & 0 deletions

File tree

extension/llm/runner/jinja_chat_formatter.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,11 @@ std::string JinjaChatFormatter::formatConversation(
359359
params["add_generation_prompt"] = conversation.add_generation_prompt;
360360
// Provide vLLM/HuggingFace-style defaults that templates often reference.
361361
// Templates that don't use these will simply ignore them.
362+
// `tools` is intentionally an empty list (not null): Jinja2Cpp, like Python,
363+
// treats an empty list as falsy, so `{% if tools %}` and the normalized
364+
// `tools is not none` -> `tools` checks render the "no tools" path when none
365+
// are supplied. See normalizeTemplate() and the UniversalJinjaToolsAware
366+
// test.
362367
params["tools"] = jinja2::ValuesList();
363368
params["tool_choice"] = jinja2::Value();
364369
params["date_string"] = conversation.date_string;

extension/llm/runner/test/CMakeLists.txt

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,14 @@ set(_test_srcs
2828
test_wav_loader.cpp
2929
)
3030

31+
# test_jinja_chat_formatter.cpp exercises JinjaChatFormatter, which is only
32+
# compiled into extension_llm_runner when jinja2cpp is available (see
33+
# ../CMakeLists.txt). Drop the test source when jinja2cpp is not built so the
34+
# test binary does not fail to link with undefined JinjaChatFormatter symbols.
35+
if(NOT TARGET jinja2cpp)
36+
list(FILTER _test_srcs EXCLUDE REGEX "test_jinja_chat_formatter\\.cpp$")
37+
endif()
38+
3139
# Add LSan stub for Apple platforms
3240
if(APPLE)
3341
list(APPEND _test_srcs lsan_stub.cpp)

0 commit comments

Comments
 (0)