Skip to content

Conversation

abdokaseb
Copy link
Contributor

What does this PR do?

This PR fixes an edge case in apply_chat_template where the continue_final_message parameter could behave unexpectedly if the final message is empty or happens to match a substring in the rendered template.

Previously, the function relied on rindex(final_message.strip()) to locate the final message in the rendered chat. This caused incorrect truncation when the content was empty or matched other parts of the template.

To resolve this, we introduce a special internal marker (CONTINUE_FINAL_MESSAGE_TAG) that is appended to the final message during rendering. After rendering, the template is safely split at this tag, and the marker is removed. This ensures correct and predictable behavior in all cases.

Fixes #40687

Motivation and Context

  • Prevents accidental truncation of rendered chat when final message content is empty or overlaps with template tokens.
  • Provides a robust and explicit way to locate the correct split point.
  • Does not modify user-facing behavior, as the marker is removed before returning the rendered chat.
  • Uses a copy of the conversation to avoid mutating user input.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

@Rocketknight1 @ArthurZucker

@abdokaseb
Copy link
Contributor Author

abdokaseb commented Sep 5, 2025

@Rocketknight1 I made the PR as we discussed on #40687 The main issue (which make the tests fails) is that when the temple end with "trim" instruction. Adding the TAG make it impossible to do the trim instruction.
Not sure should I close the PR or not. it's up to you.

Edit: now I created workaround for it and it worked

@abdokaseb
Copy link
Contributor Author

abdokaseb commented Sep 5, 2025

@Rocketknight1 I make a workaround to solve the trim problem.
I make the TAG end with space, and check if it's removed which mean the template end with trim so I add extra trim after removing the TAG. Otherwise not.

Update: Now it works for evey case

@usepr
Copy link

usepr commented Sep 6, 2025

>> Discussion link

Maybe we can locate the suffix by comparing two rendered strings, one rendered by the real final message, and the other one rendered by a fake final message (the content is FINAL_MESSAGE_TAG). The two rendered strings will look like:

prefix {real content} suffix
prefix FINAL_MESSAGE_TAG suffix

Once we identify the suffix, we can trim it.

@abdokaseb
Copy link
Contributor Author

@usepr It work now with and without the trim, you can work with this branch if you want

Copy link
Member

@Rocketknight1 Rocketknight1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, LGTM now! I think the tests cover most of this, so if they're passing then the new method works. Hopefully this resolves the last of the edge cases!

@Rocketknight1 Rocketknight1 enabled auto-merge (squash) September 8, 2025 17:16
@Rocketknight1 Rocketknight1 merged commit 5a468e5 into huggingface:main Sep 8, 2025
23 checks passed
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

the continue_final_message parameter in apply_chat_template can behave unexpectedly in some cases
4 participants