Skip to content

[transformers] set return dict false for transformers v5 compatibility#1325

Open
erictang000 wants to merge 1 commit intoNovaSky-AI:mainfrom
erictang000:return_dict_false
Open

[transformers] set return dict false for transformers v5 compatibility#1325
erictang000 wants to merge 1 commit intoNovaSky-AI:mainfrom
erictang000:return_dict_false

Conversation

@erictang000
Copy link
Collaborator

@erictang000 erictang000 commented Mar 14, 2026

Sets return_dict=False where needed for transformers-v5 compatibility - this can be merged prior to explicitly upgrading to transformers v5 in the pyproject.toml, since vllm still does not technically fully support it in the latest release.

This change should be backwards compatible with transformers v4.*, since the behavior prior to v5 was that the default value for return_dict was None, which was interpreted as False.


Open with Devin

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates calls to tokenizer.apply_chat_template to be compatible with transformers v5 by explicitly setting return_dict=False. While most changes are correct, there is a critical issue in inference_engine_client.py where the code attempts to access ['input_ids'] on the result of apply_chat_template. This will cause a TypeError because with return_dict=False, the function returns a list directly, not a dictionary. This needs to be fixed to prevent runtime errors.

Comment on lines 98 to 103
prompt_token_ids = self.tokenizer.apply_chat_template(
prompts,
add_generation_prompt=True,
return_dict=True,
return_dict=False,
tokenize=True,
)["input_ids"]
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

With return_dict=False, self.tokenizer.apply_chat_template will return a list of token IDs directly, not a dictionary. Accessing ["input_ids"] on the result will cause a TypeError at runtime. You should remove the dictionary key access.

Suggested change
prompt_token_ids = self.tokenizer.apply_chat_template(
prompts,
add_generation_prompt=True,
return_dict=True,
return_dict=False,
tokenize=True,
)["input_ids"]
prompt_token_ids = self.tokenizer.apply_chat_template(
prompts,
add_generation_prompt=True,
return_dict=False,
tokenize=True,
)

Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 3 additional findings in Devin Review.

Open in Devin Review

Comment on lines +101 to 103
return_dict=False,
tokenize=True,
)["input_ids"]
Copy link
Contributor

Choose a reason for hiding this comment

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

🔴 return_dict=False result indexed with ["input_ids"] causes TypeError

The PR changed return_dict=True to return_dict=False at line 101, but line 103 still accesses the result with ["input_ids"]. With return_dict=False, apply_chat_template returns a plain list (or list of lists) of token IDs, not a dictionary. Attempting list["input_ids"] will raise a TypeError: list indices must be integers or slices, not str. This crashes whenever generate() is called with prompts instead of prompt_token_ids.

Suggested change
return_dict=False,
tokenize=True,
)["input_ids"]
return_dict=False,
tokenize=True,
)
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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.

1 participant