Commit graph

3 commits

Author SHA1 Message Date
Vineeth Sai
d42256a5c5
Fix construct_chat_template leaking {INPUT}/{OUTPUT} sentinel into the chat template (#6531)
* 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>
2026-06-24 00:40:53 -07:00
Daniel Han
a6dc10dad2
Reduce and tighten comments and docstrings across the test suite (#6429)
* 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>
2026-06-18 01:07:09 -07:00
Ricardo-M-L
af6504f900
fix(chat_templates): check find() return value before slicing on placeholders (#5763)
* 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>
2026-05-25 06:19:01 -07:00