unsloth/tests/python/test_construct_chat_template_validation.py
Andrew Chen b307823b1d
fix(chat_templates): bind loop_messages when default_system_message is None (#7199)
* fix(chat_templates): bind loop_messages when default_system_message is None

construct_chat_template(default_system_message=None) built a system part that
binds loop_messages only inside the `{% if messages[0]['role'] == 'system' %}`
arm. The `Fix missing loop_messages` step right below then found no
unconditional `{% set loop_messages = messages %}`, concluded loop_messages was
missing, and rewrote `{% for message in loop_messages %}` back to
`{% for message in messages %}` -- undoing the `messages[1:]` skip.

A caller-supplied system message therefore reached the loop and tripped
raise_exception:

    Only user and assistant roles are supported!

Add the `{% else %}` arm so loop_messages is always bound, mirroring the
default_system_message is not None branch minus the default text. That also
stops the rewrite from firing, since the unconditional binding is now present.

Renders before / after, same template, same inputs:

    default_system_message  input        before                       after
    None                    system msg   raise_exception              'Be terse.\n### User: Hi\n'
    None                    no system    '### User: Hi\n'             unchanged
    'You are helpful.'      system msg   'Be terse.\n### User: Hi\n'  unchanged
    'You are helpful.'      no system    'You are helpful.\n...'      unchanged

The rewrite still fires for templates with no {SYSTEM} part, which is what it
was there for -- verified unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Scope loop_messages binding to {SYSTEM} templates for PR #7199

The None branch now only adds the else arm when system_part contains
{SYSTEM}, so a static prefix with no {SYSTEM} placeholder keeps raising on a
caller system message instead of silently dropping it. Strengthen the tests:
assert the default does not leak when a caller system message is present, and
add a regression test for the static prefix case.

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: danielhanchen <danielhanchen@gmail.com>
2026-07-19 06:33:48 -07:00

196 lines
7.2 KiB
Python

"""Negative-path validation tests for unsloth.chat_templates.construct_chat_template.
Regression coverage for the no-match guards added in the PR #5763 follow-up:
missing placeholders or unrecoverable two-example structures must raise
RuntimeError with a clear message (not IndexError/AttributeError) and must
not silently drop the last char via s[:-1]. A minimal fake tokenizer keeps
the cases CPU-only (no HF_TOKEN, no gated download).
"""
from types import SimpleNamespace
import pytest
from unsloth.chat_templates import construct_chat_template
class _FakeTokenizer:
"""Minimal surface construct_chat_template touches before the guards fire."""
name_or_path = "fake/tokenizer"
eos_token = "</s>"
def get_vocab(self):
return {"</s>": 0}
@pytest.mark.parametrize(
"template, expected_in_message",
[
("only {INPUT} here, no output marker", "{OUTPUT}"),
("only {OUTPUT} here, no input marker", "{INPUT}"),
("neither sentinel here, just literal text", "{INPUT}"),
("neither sentinel here, just literal text", "{OUTPUT}"),
],
)
def test_missing_placeholder_in_chat_template_raises(template, expected_in_message):
with pytest.raises(RuntimeError) as exc_info:
construct_chat_template(
tokenizer = _FakeTokenizer(),
chat_template = template,
extra_eos_tokens = ["</s>"],
)
assert expected_in_message in str(exc_info.value)
def test_single_pair_template_raises_clear_error_not_attribute_error():
"""A single {INPUT}/{OUTPUT} pair must raise RuntimeError, not the old
AttributeError on `found.group(1)` when the loop broke without setting `found`."""
template = "user: {INPUT}\nassistant: {OUTPUT}\n"
with pytest.raises(RuntimeError):
construct_chat_template(
tokenizer = _FakeTokenizer(),
chat_template = template,
extra_eos_tokens = ["</s>"],
)
def test_error_message_excerpt_is_bounded():
"""Error messages must include a bounded excerpt of the offending
template, not dump arbitrarily large content into the traceback."""
huge = ("garbage " * 5000) + "{INPUT}" # ~40 KB, missing {OUTPUT}
with pytest.raises(RuntimeError) as exc_info:
construct_chat_template(
tokenizer = _FakeTokenizer(),
chat_template = huge,
extra_eos_tokens = ["</s>"],
)
msg = str(exc_info.value)
# Excerpt is capped well under the template length.
assert len(msg) < 1000
assert "{OUTPUT}" in msg
class _SuccessFakeTokenizer(_FakeTokenizer):
"""Adds the surface construct_chat_template touches on the success path."""
bos_token = "<s>"
bos_token_id = 1
added_tokens_decoder: dict = {}
def __call__(self, text):
# input_ids[0] must differ from bos_token_id so the BOS-handling branch is skipped.
return SimpleNamespace(input_ids = [5])
@pytest.mark.parametrize(
"chat_template",
[
# User turn begins with {INPUT} (no prefix before the sentinel).
"{INPUT} [/INST] {OUTPUT}</s>{INPUT} [/INST] {OUTPUT}</s>",
# Assistant turn begins with {OUTPUT} (no prefix before the sentinel).
"User: {INPUT}\n{OUTPUT}</s>User: {INPUT}\n{OUTPUT}</s>",
],
)
def test_chat_template_does_not_leak_sentinel_when_section_starts_with_it(chat_template):
"""When an input/output section begins with the {INPUT}/{OUTPUT} sentinel, the
generated Jinja template must not keep the literal sentinel text. The `startswith`
branch in the internal `process()` helper used to slice from `find()` (which is 0
here) instead of past the sentinel, re-including the literal `{INPUT}`/`{OUTPUT}`."""
_, jinja_template, _, _ = construct_chat_template(
tokenizer = _SuccessFakeTokenizer(),
chat_template = chat_template,
extra_eos_tokens = ["</s>"],
)
assert "{INPUT}" not in jinja_template
assert "{OUTPUT}" not in jinja_template
_SYSTEM_CHAT_TEMPLATE = (
"{SYSTEM}\n"
"### User: {INPUT}\n### Assistant: {OUTPUT}</s>"
"### User: {INPUT}\n### Assistant: {OUTPUT}</s>"
)
def _render(jinja_template, messages):
from jinja2.sandbox import ImmutableSandboxedEnvironment
env = ImmutableSandboxedEnvironment()
env.globals["raise_exception"] = lambda message: (_ for _ in ()).throw(RuntimeError(message))
return env.from_string(jinja_template).render(
messages = messages,
bos_token = "<s>",
eos_token = "</s>",
add_generation_prompt = False,
)
@pytest.mark.parametrize("default_system_message", [None, "You are helpful."])
def test_system_message_is_consumed_by_the_system_part(default_system_message):
"""A caller-supplied system message must be rendered by the system part and
skipped by the message loop, whatever `default_system_message` is.
With `default_system_message = None` the generated template used to bind
`loop_messages` only inside the `{% if %}` arm. The `Fix missing
loop_messages` step then saw no unconditional binding, rewrote the loop back
to `messages`, and the system message reached the loop and tripped
`raise_exception`.
"""
_, jinja_template, _, _ = construct_chat_template(
tokenizer = _SuccessFakeTokenizer(),
chat_template = _SYSTEM_CHAT_TEMPLATE,
default_system_message = default_system_message,
extra_eos_tokens = ["</s>"],
)
rendered = _render(
jinja_template,
[
{"role": "system", "content": "Be terse."},
{"role": "user", "content": "Hi"},
],
)
assert rendered.count("Be terse.") == 1
assert rendered.count("Hi") == 1
# A caller system message overrides the default; the default must not leak in.
if default_system_message is not None:
assert default_system_message not in rendered
def test_absent_system_message_still_renders_without_default():
"""`default_system_message = None` with no system message in the input must
keep working -- the `{% else %}` arm has to bind `loop_messages = messages`."""
_, jinja_template, _, _ = construct_chat_template(
tokenizer = _SuccessFakeTokenizer(),
chat_template = _SYSTEM_CHAT_TEMPLATE,
default_system_message = None,
extra_eos_tokens = ["</s>"],
)
rendered = _render(jinja_template, [{"role": "user", "content": "Hi"}])
assert "Hi" in rendered
_NO_SYSTEM_CHAT_TEMPLATE = (
"PREAMBLE\n"
"### User: {INPUT}\n### Assistant: {OUTPUT}</s>"
"### User: {INPUT}\n### Assistant: {OUTPUT}</s>"
)
def test_static_prefix_without_system_still_rejects_system_message():
"""A template with a static prefix but no {SYSTEM} placeholder cannot render a
caller system message, so it must still raise rather than silently drop it."""
_, jinja_template, _, _ = construct_chat_template(
tokenizer = _SuccessFakeTokenizer(),
chat_template = _NO_SYSTEM_CHAT_TEMPLATE,
default_system_message = None,
extra_eos_tokens = ["</s>"],
)
with pytest.raises(RuntimeError, match = "Only user and assistant roles are supported!"):
_render(
jinja_template,
[
{"role": "system", "content": "Be terse."},
{"role": "user", "content": "Hi"},
],
)