* 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>
* Fix construct_chat_template leaking {INPUT}/{OUTPUT} sentinel into the template
In construct_chat_template's inner process() helper, the branch handling a
section that starts with the {INPUT}/{OUTPUT} sentinel sliced the part from
part.find(which) (which is 0 in that branch), so the literal sentinel was
re-included in the generated Jinja chat template. The endswith branch already
slices correctly with part[:part.find(which)]; this slices past the sentinel
with part[len(which):], so a template whose input or output section begins
with the sentinel (for example a user turn that starts with {INPUT}) renders
correctly instead of emitting a literal {INPUT}/{OUTPUT}.
Added a regression test covering {INPUT}-leading and {OUTPUT}-leading sections.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Reduce and tighten comments and docstrings in tests
Shorten verbose comments and docstrings across the test suite without
changing any test logic. Remove narration that restates the next line,
collapse long module and test docstrings to a single line, and drop banner
separators. Keep regression context (issue and PR references, run ids),
skip reasons, mocking and timing rationale, license headers, lint and type
directives, and commented-out code.
Comments and docstrings only: an AST signature check confirms no code,
assertions, or string literals changed, and the suite byte-compiles cleanly.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* fix(chat_templates): check find() return value before slicing on placeholders
Two places in `construct_chat_template()` use `str.find()` for sentinel
placeholders (`{INPUT}` / `{OUTPUT}`) without checking the -1 return:
1. The `except:` fallback (around line 2464) computes
`chat_template[chat_template.find("{OUTPUT}") + len("{OUTPUT}"):]`.
If the template has no `{OUTPUT}` marker, `find()` returns -1 and the
slice starts at offset 7 (`-1 + len("{OUTPUT}")`), producing garbage
that's then `re.escape`-d and fed back into the template-recovery
regex. The user sees a confusing `IndexError` on
`response_part = response_part[0]` instead of the real problem.
2. The final trim before returning (`input_part[:input_part.find("{INPUT}")]`
and the matching `{OUTPUT}` line) silently drops the last character
when the placeholder is missing — `find()` returns -1, and `[:-1]`
slices everything except the last character, returning a corrupted
template prefix to the caller.
Replace both with an explicit `-1` check that raises a clear
`RuntimeError` naming the missing placeholder, matching the existing
guard pattern from #4923 (`try_fix_tokenizer`).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(chat_templates): also guard {INPUT} and fallback regex/separator paths
Builds on the {OUTPUT} / final-trim guards in this branch by closing
the three remaining ways the except-block fallback in
construct_chat_template() can still raise a confusing IndexError or
AttributeError on malformed templates:
1. Validate both {INPUT} and {OUTPUT} before deriving `ending`. The
regex two lines later (`{INPUT} + ending + ...`) still produced an
empty list and crashed on `response_part[0]` if {INPUT} was missing.
2. Guard the regex no-match case. Some templates contain both
placeholders but not in a recoverable two-example shape, in which
case `re.findall` returns an empty list and `[0]` raises.
3. Initialize `found = None` before the separator-search loop and
raise if the loop never sets it. Previously, if the first
iteration's `re.finditer` was empty the loop broke without binding
`found`, and `found.group(1)` raised AttributeError on the stale
int left over from the outer rfind loop.
Rephrase the final-trim error messages from internal variable names
("input_part") to user-facing wording ("instruction section") and
include a bounded (200-char) excerpt of the offending content so the
error is debuggable without being unbounded.
Add tests/python/test_construct_chat_template_validation.py covering
each failure mode with a fake tokenizer (no HF_TOKEN, no model
download, CPU-only).
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Daniel Han <danielhanchen@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
