[bugfix] guard _repair_ms_bench against an empty messages list#9608
[bugfix] guard _repair_ms_bench against an empty messages list#9608he-yufeng wants to merge 1 commit into
Conversation
_repair_ms_bench reads messages[0] right after ast.literal_eval, so a row whose messages are empty (e.g. the string "[]") raises IndexError and aborts the whole ms_bench dataset load instead of just skipping that row. The function already returns None to skip rows it can't use (the MOSS case), so do the same for an empty list. Added pure unit tests for the empty, default-system, normal and MOSS cases.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a safety check to the _repair_ms_bench function in swift/dataset/dataset/llm.py to return None when messages is empty, preventing potential index out of bounds errors. Additionally, a comprehensive suite of unit tests has been introduced in tests/general/test_repair_ms_bench.py to verify this behavior and other edge cases. There are no review comments, and the changes look solid.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
1 participant
Summary
_repair_ms_bench(therepair_messageshook for theiic/ms_benchdataset) readsmessages[0]right afterast.literal_eval, so a row whose messages are empty — e.g. the literal string"[]"— raisesIndexError: list index out of rangeand aborts the whole ms_bench dataset load instead of just skipping that one row.The function already returns
Noneto skip rows it can't use (the MOSS case), andMessagesPreprocessordropsNonerows, so returningNonefor an empty list is the same, consistent behaviour.Fix
An early
if not messages: return Noneguard right after the parse.Test
Added
tests/general/test_repair_ms_bench.py(pure unit tests, no network): empty"[]"/[]returnsNone, the default system message is stripped, a normal conversation passes through, and a MOSS row is skipped. The empty case raisesIndexErrorwithout the fix.Verified locally:
pytest tests/general/test_repair_ms_bench.py(4 passed);flake8/isort/yapfclean.