Skip to content

Fix YUAN2 conversation template stripping message characters#3887

Open
JSap0914 wants to merge 1 commit into
lm-sys:mainfrom
JSap0914:fix-yuan2-rstrip-suffix
Open

Fix YUAN2 conversation template stripping message characters#3887
JSap0914 wants to merge 1 commit into
lm-sys:mainfrom
JSap0914:fix-yuan2-rstrip-suffix

Conversation

@JSap0914

Copy link
Copy Markdown

Why are these changes needed?

The YUAN2 separator style in fastchat/conversation.py joins messages with
the literal <n> separator and then calls ret.rstrip("<n>") to drop the
trailing separator before appending seps[0].

str.rstrip treats its argument as a set of characters, not a suffix, so it
removes every trailing <, n, and > character. Any message that ends in one
of those characters gets corrupted. For example:

conv = get_conv_template("yuan2")
conv.append_message(conv.roles[0], "What is the value of n")
conv.append_message(conv.roles[1], None)
conv.get_prompt()
# before: 'What is the value of <sep>'   (trailing "n" lost)
# after:  'What is the value of n<sep>'

The fix removes only the trailing <n> separator with an explicit suffix check,
leaving message content intact.

Related issue number (if applicable)

N/A

Checks

  • I've run format.sh to lint the changes in this PR.
  • I've included any doc changes needed.
  • I've made sure the relevant tests are passing (if applicable).

Verification (Linux, offline):

$ python3 -m pytest tests/test_conversation.py -v
tests/test_conversation.py::TestYuan2Template::test_message_ending_in_angle_bracket_is_preserved PASSED
tests/test_conversation.py::TestYuan2Template::test_message_ending_in_n_is_preserved PASSED
tests/test_conversation.py::TestYuan2Template::test_separator_between_messages_is_kept PASSED
3 passed

The YUAN2 separator style joined messages with the literal '<n>'
separator and then called ret.rstrip('<n>') to drop the trailing
separator before appending seps[0]. str.rstrip treats its argument
as a set of characters, so it stripped every trailing '<', 'n' and
'>' character, corrupting any message that ended in one of them
(e.g. 'What is the value of n' became 'What is the value of ').

Remove only the trailing '<n>' separator with an explicit suffix
check so message content is preserved.

Adds tests/test_conversation.py covering the YUAN2 template.
Copilot AI review requested due to automatic review settings June 16, 2026 08:23

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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.

2 participants