fix: add chat_template property to Model for setting chat templates#1455
Open
octo-patch wants to merge 1 commit into
Open
fix: add chat_template property to Model for setting chat templates#1455octo-patch wants to merge 1 commit into
octo-patch wants to merge 1 commit into
Conversation
…lates Add getter/setter property for chat_template on the Model base class that delegates to the underlying interpreter. This allows users to set the chat template on model objects directly (e.g. Mock models in tests), enabling the previously-disabled chat template smoke tests to run correctly. Fixes guidance-ai#1196
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
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 #1196
Problem
The chat template smoke tests in
tests/need_credentials/test_chat_templates.pywere disabled with@pytest.mark.skipbecause the tests setlm.chat_template = ...on Mock model objects, but theModelbase class had nochat_templateproperty. As a result, settinglm.chat_templateonly set a plain attribute on the model instance, rather than updating the interpreter's chat template that is actually used for role-based formatting.Solution
Add a
chat_templateproperty (getter and setter) to theModelbase class inguidance/models/_base/_model.py:self._interpreter.chat_templateif the interpreter supports it (viagetattrwithNoneas default for interpreters that do not have this attribute)self._interpreter.chat_template = value, which is then preserved acrossdeepcopywhen the model is copied during__add__operationsThis allows tests (and users) to configure the chat template on a model object directly:
Re-enable the two previously-skipped smoke tests:
test_chat_format_smokeandtest_chat_format_smoke_with_system.Testing
The tests require
HF_TOKENand download actual HuggingFace models, so they run in theneed_credentialssuite. The fix is mechanically verified:lm.chat_template = valuenow properly setslm._interpreter.chat_template = value, and subsequentdeepcopyof the interpreter (triggered by model copy during__add__) preserves the new template.