Fix ENAMETOOLONG crash when chat_template is an inline Jinja string - #698
Open
javierdejesusda wants to merge 1 commit into
Open
Fix ENAMETOOLONG crash when chat_template is an inline Jinja string#698javierdejesusda wants to merge 1 commit into
javierdejesusda wants to merge 1 commit into
Conversation
A long literal chat_template made the .jinja resource lookup build a path whose final component exceeds NAME_MAX, so .is_file() raised OSError(ENAMETOOLONG) instead of returning False. Only attempt the template-name lookup when chat_template looks like a bare name, falling through to treat the value as an inline template otherwise.
|
All contributors have signed the CLA ✍️ ✅ |
Author
|
I have read the CLA Document and I hereby sign the CLA |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #629.
When
chat_templateis set to an inline Jinja string rather than the name of a bundled template, the merge crashes before it can finish._set_chat_templateinmergekit/merge.pytreats the value as a template name first, building a path withimportlib.resources.files(chat_templates).joinpath(chat_template + ".jinja")and calling.is_file()on it. For a long inline template that path exceedsNAME_MAX, so.is_file()raisesOSError(ENAMETOOLONG) and the whole merge aborts.The fix only treats
chat_templateas a bundled-template name when it actually looks like one, guarding the.is_file()check withlen(chat_template) < 256 and "\n" not in chat_template and "{" not in chat_template. Inline templates no longer hit the filesystem lookup and fall through to the existing literal-Jinja branch, which already handles them correctly.Added
test_template_long_literal_jinjaintests/test_chat_template.py, which merges with a >300-character inline Jinja template and asserts the merge succeeds and the template round-trips. It reproduces the ENAMETOOLONG failure onmainand passes with this change.Note
Low Risk
Small conditional change in tokenizer chat-template resolution with a targeted test; no auth, data, or merge-math impact.
Overview
Fixes merges that crash when
chat_templateis a long inline Jinja string instead of a bundled template name likechatml._set_chat_templateinmerge.pyused to always probe the filesystem withchat_template + ".jinja". That path can exceedNAME_MAXfor long literals, so.is_file()raisedOSError(ENAMETOOLONG) and aborted the merge.The bundled-template branch now runs only when the value looks like a name: length under 256, no newlines, and no
{. Inline Jinja skips the lookup and uses the existing literal branch. A regression test merges with a 300+ character inline template and checks it round-trips.Reviewed by Cursor Bugbot for commit 0102916. Bugbot is set up for automated code reviews on this repo. Configure here.