Skip to content

Test DataCollatorForChatML raises when completion fills window#6319

Merged
albertvillanova merged 2 commits into
mainfrom
fu-6310
Jul 7, 2026
Merged

Test DataCollatorForChatML raises when completion fills window#6319
albertvillanova merged 2 commits into
mainfrom
fu-6310

Conversation

@albertvillanova

@albertvillanova albertvillanova commented Jul 7, 2026

Copy link
Copy Markdown
Member

This PR adds a test covering the edge case where a chat completion is long enough to fill the entire max_length window on its own, so no prompt tokens survive truncation. It verifies that DataCollatorForChatML raises a clear ValueError ("no prompt tokens left after truncation") instead of emitting an all-padding prompt row that would leave nothing to generate from.

Follow-up to:

Changes

  • Add test_chatml_collator_raises_when_completion_fills_window to the GOLD trainer tests, feeding a long completion with a small max_length and asserting the collator rejects it.

Note

Low Risk
Test-only change with no runtime or training logic modifications.

Overview
Adds test_chatml_collator_raises_when_completion_fills_window in the GOLD trainer tests, placed next to the existing truncation tests for DataCollatorForChatML.

The test uses a long user/assistant chat example with max_length=256 so the assistant completion alone exceeds the window and no prompt tokens survive end-keeping truncation. It asserts the collator raises ValueError with message no prompt tokens left after truncation instead of returning a batch with an empty or all-padding prompt that would break generation-style training.

This is test-only coverage for behavior already implemented in DataCollatorForChatML; the PR diff does not change collator or trainer code.

Reviewed by Cursor Bugbot for commit 6939017. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 56fc039. Configure here.

max_length = 256 # smaller than the completion, so no prompt tokens survive
collator = DataCollatorForChatML(tokenizer=llama_tokenizer, max_length=max_length)
with pytest.raises(ValueError, match="no prompt tokens left after truncation"):
collator(examples)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Raises test contradicts truncates test

Medium Severity

The new test reuses the same messages and max_length=256 as test_chatml_collator_truncates_keeping_completion_end, which calls DataCollatorForChatML without error. The collator only raises when the truncated window has zero prompt tokens, which requires the completion alone to span at least max_length tokens—something this shared fixture does not achieve for llama_tokenizer, so pytest.raises will not trigger.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 56fc039. Configure here.

@bot-ci-comment

bot-ci-comment Bot commented Jul 7, 2026

Copy link
Copy Markdown

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.

@albertvillanova albertvillanova merged commit 573599c into main Jul 7, 2026
5 checks passed
@albertvillanova albertvillanova deleted the fu-6310 branch July 7, 2026 14:29
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