Fix IndexError in SequenceFeatureExtractor.pad on all-empty batch#47389
Fix IndexError in SequenceFeatureExtractor.pad on all-empty batch#47389Osamaali313 wants to merge 1 commit into
Conversation
When the first element of a batch is an empty list/tuple, `pad` scans for
the first non-empty element to infer the input type:
index = 0
while len(required_input[index]) == 0:
index += 1
if index < len(required_input):
first_element = required_input[index][0]
The `while` loop has no upper bound, so if every element in the batch is
empty it increments `index` past the end and raises
`IndexError: list index out of range` before the following
`if index < len(required_input):` guard can run.
The equivalent code in `PreTrainedTokenizerBase.pad`
(`tokenization_utils_base.py`) does the same "grab the first non-empty
element" with a bounded `for ... break` loop and explicitly leaves
`first_element` as the empty element when the whole batch is empty. Bound
the loop the same way so the existing guard is reached and an all-empty
batch is handled gracefully instead of crashing.
CI recapDashboard: View test results in Grafana |
|
Hey, can you stop making these PRs with your code agent? They're 'theoretical bugs' found by static analysis that never really trigger in practice (this one only fails on invalid inputs, so it's unclear what we should even return in this case). Telling a code agent to find bugs and fix them almost always creates PRs like this which spam maintainers and make open-source maintenance more difficult without improving the code in any real way. |
|
Also we are ML engineers at a leading AI company so we do not need users to prompt Copilot for us. We are able to do that ourselves if we want to 😅 |
|
Thanks, and point taken — this is fair feedback. To be transparent: this came from an automated code-maintenance/bug-fixing pipeline I've been testing for long-running sessions, not a person hand-picking your repo. You've surfaced two real problems with it: (1) it's surfacing low-value, largely theoretical findings that aren't worth a maintainer's time, and (2) it re-pushed to a PR after it was closed, which it should never do. Both are going straight into the pipeline's guardrails. Concretely, I'm removing the HuggingFace organizations from the pipeline's target list so you won't get more of these, and I'll close this PR now. Apologies for the noise, and thanks for taking the time to push back — that's the most useful signal I could've gotten. |
What does this PR do?
SequenceFeatureExtractor.pad(src/transformers/feature_extraction_sequence_utils.py) — the padding entry point for every audio feature extractor (Wav2Vec2, Whisper, CLAP, EnCodec, Gemma3n, … 30 subclasses) — crashes withIndexErrorwhen the whole batch is empty.When the first element of the batch is an empty list/tuple,
padscans for the first non-empty element to infer the input type:The
whileloop has no upper bound, so if every element in the batch is empty it incrementsindexpast the end and raisesIndexError: list index out of range— before the followingif index < len(required_input):guard (which was clearly meant to handle exactly this case) can ever run.Evidence this is unintended
The analogous code in
PreTrainedTokenizerBase.pad(tokenization_utils_base.py) does the same "grab the first non-empty element" with a bounded loop, and its comment explicitly notes that an all-empty batch should just be left alone:Fix
Bound the loop so the existing guard is reached and an all-empty batch is handled gracefully instead of crashing:
Reproduction
IndexError: list index out of range❌BatchFeaturewith 2 (empty) rows ✅A regression test is added in
tests/utils/test_feature_extraction_utils.py.Before submitting
SequenceFeatureExtractorPadTester.test_pad_all_empty_batch_does_not_raise).