Repository navigation
Render Hugging Face chat templates as transformers does: ConfigureForChatTemplates (0360) - #737
Conversation
… tojson string separators
…C1083; glibc has a features.h)
…ts its own cases (CI and preflight)
|
@codex review @codex security review The diff touches the parser (the Generated by Claude Code |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ff6ce0f8a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…(Codex review) - strftime_now checks the sequence cap as its result grows, so a format of repeated directives fails with ResourceLimitExceeded instead of building a string past maxSequenceSize. - A string given as tojson separators stops unpacking at the third character. - tojson evaluates its indent argument for string values too, as Python does, and only then ignores it. - strftime_now and raise_exception accept a string that host data passes as a view; such a format used to fail as a non-string. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWNL7nyP1j6LYCMdbnWsHk
|
@codex review @codex security review d010800 fixes the three round-1 findings: the Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d010800e66
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…separators (Codex review) - PythonJsonWriter checks the document size before it appends an item or key separator or an indent, and once more at the end, so host strings of any length cannot grow it past maxSequenceSize. - separators accepts a dict and unpacks its keys, as json.dumps does; a dict of more than two keys fails before they are copied. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWNL7nyP1j6LYCMdbnWsHk
Merged onto master c0021a5; CI green and perf approved on this head, with a named fixed per-Load cost (Load/strings +0.60%, Load/int_arith +0.53%, Module/keep +0.48%; follow-up 0384 gates it). Codex: no open P0/P1; the security review never ran.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DTQTriEsFrrBgcYNKDUUF4
The perf track approved #737 with a fixed cost of about 350-430 Ir per Load in plain environments (Load/strings, Load/int_arith, Module/keep) and asked for a follow-up to gate it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWNL7nyP1j6LYCMdbnWsHk
Requested by Ruslan · project thread
Problem
Task 0360, phase 2 (parity batch 2, group F). Phase 1 (#720) added
test/chat_templates/: 72 real Hugging Face chat templates, each rendered with 12 conversations by transformers 5.22.1. 140 of the 722 renders differed from transformers, in four ways:tojson;{% generation %}tag;The API decision (Ruslan, 2026-11-01) is option 1 from the plan:
Behaviour
Public part:
include/jinja2cpp/chat_template.hdeclaresConfigureForChatTemplates(TemplateEnv&, ChatTemplateOptions = {}).ChatTemplateOptions::nowis the clock forstrftime_now. The function:trimBlocks,lstripBlocksandloopControls, turnsautoescapeoff, and capslimits.maxRangeat 100000 (the sandbox'sMAX_RANGE);raise_exception(message), which fails the render withErrorCode::TemplateRuntimeErrorandstr(message);strftime_now(format), which formats as CPython on Linux does in the "C" locale:put_timedirectives,%f, empty%z/%Zand glibc's-flag;maxSequenceSizeas it grows;Internal part:
detail::Dialect(src/dialect.h). It is kept inLoadSettingsthe way the big-int backend is, so a template keeps the dialect it was loaded with. The parser reads it fromt_parseLimits; render code reads it throughRenderContext::GetDialect(). In the Transformers dialect:tojsonis transformers' variant:json.dumps(x, ensure_ascii=False, indent=None, separators=None, sort_keys=False). Keys stay in insertion order, nothing is HTML-escaped, and the result is a plain string, not Markup.Serialize::TransformersJsonMode, picked by name at parse time inFindBuiltinFilter. It is also picked at render time formap('tojson').separatorsunpacks as Python does, from a list, a two-character string or a dict's keys. It stops at the third item.PythonJsonWriternow takesJsonOptions, and Jinja2'stojsonis the same writer with its old options.maxSequenceSize.{% generation %}…{% endgeneration %}renders its body in a scope of its own, like{% with %}. As in transformers' call block,breakandcontinuecannot cross it. The assistant-token mask is not offered; that is task 0377.append,pop,update, …) become the sandbox's unsafe attribute: an undefined whose use fails with "access to attribute 'append' of 'list' object is unsafe.".x.m,x['m'],|attr,map(attribute=)and direct calls.Plain environments are unchanged.
CMake.
JINJA2CPP_WITH_CHAT_TEMPLATES(default ON) buildssrc/chat_template.cppand installs the header.jinja2cpp/features.h, included byconfig.h, defines:JINJA2CPP_HAS_CHAT_TEMPLATES;JINJA2CPP_CHAT_TEMPLATES_TRANSFORMERS, the transformers version, read from the pin intest/chat_templates/requirements.txt.clang-18, C++20, Debug) now builds withchat: 'OFF'.Instruction gate. When the base's bench driver does not know the
transformerssetting, the base counts its ownbench/cases, in bothbenchmark.ymlandscripts/preflight.sh. Otherwise an older base would render the newhf_*cases in a plain environment and fail.Docs.
docs/api-2.0.md: section 3.3 and decision 7.8.test/chat_templates/README.mdandbench/README.mdare updated.SecurityErrorcode.Bench. New cases
hf_qwen3,hf_gpt_oss,hf_deepseek_v31andhf_mistral_smallare next tochat_llama:toolsconversation.settings.jsonhas"transformers": true.python_bench.pybuilds the same environment without depending on transformers, and its output equals the transformers expectations.engines_benchskips these cases.Validation
test/chat_templates: all 722 renders match transformers byte for byte, narrow and wide.divergences.txtis now empty.test/chat_template_preset_test.cppcovers every behaviour above, narrow and wide, including the sequence-cap tests forstrftime_nowand for separators and indent. Each expectation was checked against transformers 5.22.1.ctest, Debug (GCC, -Werror): passed. A Debug build with-DJINJA2CPP_WITH_CHAT_TEMPLATES=OFF: built, and its tests passed.scripts/preflight.sh --perfat 6682416: all checks PASS (clang-format, clang-tidy 22.1.8, null comparisons, use_count, CodeQL pattern, task files, instruction gate).bench/count.py, threshold 3%):Render/chat_llama−0.27%,Render/chat_qwen−0.69%,Render/chat_mistral−0.14%;hf_*rows are new.chat=OFFrow and the instruction gate.ChatTemplate*:Preset*:*ojson*:*Parity*:*Statement*: 7584 passed, no reports.tojsonarguments: 86 templates.separators;%-dflag;%;strftimedirective now never reaches the C library.-O2,test/expressions_test.cpp:756trips-Wsign-conversion. CI does not build with GCC 13.Roles: main session (Opus) explored and implemented; verifier ran as a subagent. Verifier rounds: 1. Red CI pushes: 2.
features.hnot found.#include "features.h"inconfig.hfound glibc's<features.h>on Linux, so local builds hid it. Fixed in 28e1ac1 with"jinja2cpp/features.h".hf_*cases and could not renderhf_gpt_oss. Fixed in 8ff6ce0: such a base counts its own cases.Codex: 5 findings, 5 fixed, 0 declined.
strftime_nowsequence cap;separatorsunpack stops at the third item;indentis evaluated for strings.separators.strftime_nowas astd::string_view, which was rejected; that is fixed too.@codex security reviewwas requested twice, but its status stayed empty, so no security review result exists. Both allowed requests are used.Limits
apply_chat_template: the host buildsmessages,toolsandbos_token, and handles the reasoning aliases.UntrustedLimits.UndefinedError, not a distinct code (task 0377).strftimeresults for the portable directives come from those platforms' C libraries in the "C" locale.Generated by Claude Code
🤖 Generated with Claude Code
https://claude.ai/code/session_01AWNL7nyP1j6LYCMdbnWsHk