-
Notifications
You must be signed in to change notification settings - Fork 11k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
main: use jinja chat template system prompt by default #12118
Conversation
From UX perspective, this is very confusing for end-user. I would suggest that we either:
|
Perhaps, but it is already doing this, just wrongly. :)
Makes sense, but maybe in a different PR? |
Yes and it would be better if you we can merge that PR (adding |
Sure, I'll take a look. |
Failing tests seems to just be server outage. |
I'll test this with Phi-4 a bit later |
Ok that works, here is my test command + result (leaving it here for visibility): No system prompt:
Incorrect use
Correctly using system prompt:
|
@ngxson I'll make a followup PR to add template formatting in non-conversation mode, and combined use of -sys and -p together. |
* Use jinja chat template system prompt by default * faster conditional order * remove nested ternary --------- Co-authored-by: Xuan Son Nguyen <son@huggingface.co>
Using the new
--jinja
option would override the builtin system prompt withYou are a helpful assistant
if in conversation mode and/or no prompt was defined.Fixed this behaviour
so thatso that the chat template's default is used instead.-p
option now either defines system prompt (in conversation mode) or user prompt, and does not fill in a defaultAdds some extra safety checks to be able to process an interactive user prompt with no defined system prompt.
@ochafik PTAL
No system prompt:
Incorrect use
-p
Correctly using system prompt: