Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions src/transformers/cli/serving/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -930,24 +930,25 @@ def get_processor_inputs_from_messages(messages: list[dict], modality: Modality)
for message in messages:
parsed = {"role": message["role"], "content": []}

content = message.get("content")
if modality == Modality.LLM:
if isinstance(message["content"], str):
parsed["content"] = message["content"]
elif isinstance(message["content"], list):
texts = [c["text"] for c in message["content"] if c["type"] == "text"]
if isinstance(content, str):
parsed["content"] = content
elif isinstance(content, list):
texts = [c["text"] for c in content if c["type"] == "text"]
Comment on lines -934 to +938

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

so what happens to the tool-usage into when a tool-LLM is used in cli? Do we not need to re-direct it inside text?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The tool_calls / tool_call_id fields are not forwarded by this func but that's a pre-existing gap, not introduced by this PR.

This function only transforms the content field. It already dropped all other message fields before this change.

I think forwarding tool_calls/tool_call_id through to apply_chat_template so multi-turn tool-use conversations template correctly should be its own PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

parsed["content"] = " ".join(texts)

elif modality == Modality.VLM:
if isinstance(message["content"], str):
parsed["content"].append({"type": "text", "text": message["content"]})
else:
for content in message["content"]:
if content["type"] == "text":
parsed["content"].append(content)
elif content["type"] == "image_url":
if isinstance(content, str):
parsed["content"].append({"type": "text", "text": content})
elif isinstance(content, list):

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

elif instead of else because content can also be None now

for content_block in content:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

content_block name is also use elsewhere in the codebase

if content_block["type"] == "text":
parsed["content"].append(content_block)
elif content_block["type"] == "image_url":
from PIL import Image

url = content["image_url"]["url"]
url = content_block["image_url"]["url"]
if "base64" in url:
image_data = re.sub("^data:image/.+;base64,", "", url)
image = Image.open(BytesIO(base64.b64decode(image_data)))
Expand Down
2 changes: 1 addition & 1 deletion src/transformers/models/smolvlm/processing_smolvlm.py
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ def apply_chat_template(
(isinstance(content, dict) and content["type"] == "video")
for conversation in conversations
for message in conversation
for content in message["content"]
for content in (message.get("content") or [])
)
if chat_template is None and has_video:
# re-assign to the correct default template for BC, if user is not requesting their own template
Expand Down
11 changes: 7 additions & 4 deletions src/transformers/processing_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1804,12 +1804,15 @@ def apply_chat_template(
for conversation in conversations:
images, videos = [], []
for message in conversation:
visuals = [content for content in message["content"] if content["type"] in ["image", "video"]]
content = message.get("content") or []
visuals = [
content_block for content_block in content if content_block["type"] in ["image", "video"]
]
audio_fnames = [
content[key]
for content in message["content"]
content_block[key]
for content_block in content
for key in ["audio", "url", "path"]
if key in content and content["type"] == "audio"
if key in content_block and content_block["type"] == "audio"
]
image_fnames = [
vision_info[key]
Expand Down
6 changes: 4 additions & 2 deletions src/transformers/utils/chat_template_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -543,8 +543,10 @@ def render_jinja_template(
chat = chat.messages
if continue_final_message:
chat = deepcopy(chat)
final_message = chat[-1]["content"]
if isinstance(final_message, (list, tuple)):
final_message = chat[-1].get("content")
if final_message is None:
raise ValueError("continue_final_message is set but the final message has no content to continue!")
elif isinstance(final_message, (list, tuple)):
Comment on lines -546 to +549

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@Rocketknight1 to confirm on these

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, this makes sense to me. continue_final_message was something we initially added for text-only models, so it's not really designed to handle situations where the final message isn't just text to be continued. Erroring out when there's no content is fine.

for content_block in reversed(final_message):
if "text" in content_block:
# Pick the last text block in the message (the first one we hit while iterating in reverse)
Expand Down
25 changes: 25 additions & 0 deletions tests/test_processing_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -1990,6 +1990,31 @@ def test_apply_chat_template_assistant_mask(self):
ids_is_same = processor.tokenizer.encode(assistant_text, add_special_tokens=False), assistant_ids.tolist()
self.assertTrue(text_is_same or ids_is_same)

@require_torch
def test_apply_chat_template_tool_calls_no_content(self):
processor = self.get_processor()

if processor.chat_template is None:
self.skipTest("Processor has no chat template")

if "tool" not in processor.chat_template: # good heuristic to check if template supports tools
self.skipTest("Chat template does not support tools")

messages = [
{
"role": "user",
"content": [{"type": "text", "text": "What is the weather?"}],
},
{
"role": "assistant",
"tool_calls": [{"type": "function", "function": {"name": "get_weather", "arguments": "{}"}}],
},
]

# Regression test for #45290: tokenize=True used to raise KeyError when "content" was missing
result = processor.apply_chat_template(messages, tokenize=True)
self.assertIsInstance(result, list)

def test_get_num_multimodal_tokens_matches_processor_call(self):
"Tests that the helper used internally in vLLM works correctly"

Expand Down
Loading