fix: set add_generation_prompt to False when apply chat template for multimodal #2859
Conversation
WalkthroughThe update changes the Changes
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Pylint (3.3.7)src/axolotl/utils/collators/mm_chat.py✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/axolotl/utils/collators/mm_chat.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: PyTest from Source Dist (3.11, 2.5.1)
- GitHub Check: PyTest (3.11, 2.7.1)
- GitHub Check: PyTest from Source Dist (3.11, 2.7.1)
- GitHub Check: pre-commit
- GitHub Check: PyTest from Source Dist (3.11, 2.6.0)
- GitHub Check: PyTest (3.11, 2.5.1)
- GitHub Check: PyTest (3.11, 2.6.0)
- GitHub Check: pre-commit
| @@ -50,7 +50,7 @@ def process_rows( | |||
| # This method requires transformers>=4.49.0 | |||
| result = self.processing_strategy.processor.apply_chat_template( | |||
| example["messages"], | |||
| add_generation_prompt=True, | |||
| add_generation_prompt=False, | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
LGTM! This fix aligns with multimodal processing requirements.
Setting add_generation_prompt=False is appropriate for multimodal contexts where you want explicit control over the chat template structure rather than automatically adding generation prompts that might interfere with multimodal processing.
However, please verify that this change doesn't break existing functionality by running the following script to check for any tests or usage patterns that might be affected:
🏁 Script executed:
#!/bin/bash
# Description: Verify the impact of changing add_generation_prompt behavior
# Search for any tests related to this collator
echo "=== Searching for tests related to MultiModalChatDataCollator ==="
fd -e py -x grep -l "MultiModalChatDataCollator" {} \;
# Search for any other usage of add_generation_prompt in the codebase
echo "=== Searching for other usage of add_generation_prompt ==="
rg "add_generation_prompt" -A 3 -B 3
# Search for any documentation or comments about generation prompts in multimodal context
echo "=== Searching for documentation about generation prompts ==="
rg -i "generation.prompt" -A 2 -B 2Length of output: 208150
Add tests for MultiModalChatDataCollator’s new default
I didn’t find any existing tests that exercise MultiModalChatDataCollator in src/axolotl/utils/collators/mm_chat.py—changing add_generation_prompt there to False can alter how multimodal examples are formatted. Please:
• Create or extend unit tests for MultiModalChatDataCollator.process_rows, verifying both with and without a generation prompt
• Add an integration/e2e scenario that runs a simple multimodal example through the collator and confirms the prompt ends (or doesn’t end) with an assistant turn as expected
• Ensure downstream consumers (e.g. in src/axolotl/core/builders/causal.py) still behave correctly when add_generation_prompt=False
🤖 Prompt for AI Agents
In src/axolotl/utils/collators/mm_chat.py at line 53, the default value of
add_generation_prompt was changed to False, which affects how multimodal
examples are formatted. To fix this, add or extend unit tests for
MultiModalChatDataCollator.process_rows to cover cases both with
add_generation_prompt set to True and False, verifying the formatting of the
generation prompt. Additionally, create an integration or end-to-end test that
processes a simple multimodal example through the collator and checks that the
prompt ends correctly with or without an assistant turn. Finally, verify that
downstream consumers, such as those in src/axolotl/core/builders/causal.py,
continue to function correctly when add_generation_prompt is False.
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
Description
For some reason,
add_generation_promptwas set to true which can lead to unexpected effects in multimodal.Motivation and Context
How has this been tested?
Screenshots (if appropriate)
Types of changes
Social Handles (Optional)
Summary by CodeRabbit