Fix #2839: initialize punc_res before conditional to prevent UnboundLocalError#2840
Fix #2839: initialize punc_res before conditional to prevent UnboundLocalError#2840Lidang-Jiang wants to merge 1 commit intomodelscope:mainfrom
Conversation
…t UnboundLocalError When punc_model is None or not provided, punc_res was only assigned inside the `if self.punc_model is not None:` block. Downstream code paths (punc_segment speaker diarization and sentence_timestamp) accessed punc_res unconditionally, causing UnboundLocalError. Changes: - Initialize punc_res = None before the conditional block - Add punc_res is None guard in punc_segment path with error log - Add punc_res is None guard in sentence_timestamp path with warning log - Add unit tests covering punc_model=None, empty punc_model, and normal flow Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Lidang-Jiang <lidangjiang@gmail.com>
There was a problem hiding this comment.
Code Review
This pull request fixes an UnboundLocalError in the inference_with_vad method by ensuring punc_res is properly initialized and checked before use when punc_model is None. The changes also include code formatting improvements and a new test suite to verify the fix. Feedback suggests refining the docstring in the new test file to accurately reflect that only the punc_model=None case is tested, as empty strings are not covered and may lead to different initialization issues.
| @@ -0,0 +1,119 @@ | |||
| """Tests for issue #2839: punc_model=None or empty string should not cause UnboundLocalError.""" | |||
There was a problem hiding this comment.
The docstring states that these tests cover punc_model=None or an empty string. However, the tests only cover the None case. An empty string for punc_model would likely cause a different failure mode during model initialization, not the UnboundLocalError this PR aims to fix. To avoid confusion, please update the docstring to only mention the punc_model=None case.
| """Tests for issue #2839: punc_model=None or empty string should not cause UnboundLocalError.""" | |
| """Tests for issue #2839: punc_model=None should not cause UnboundLocalError.""" |
Summary
Fix
UnboundLocalErrorwhenpunc_modelisNoneor not provided inAutoModel.generate()with VAD enabled.Root cause: In
inference_with_vad(),punc_resis only assigned inside theif self.punc_model is not None:block, but downstream code paths (punc_segmentspeaker diarization andsentence_timestamp) access it unconditionally.Fix:
punc_res = Nonebefore the conditional blockpunc_res is Noneguards with appropriate log messages in the two affected code pathsBefore (UnboundLocalError on unpatched code)
After (all tests pass)
Test plan
test_punc_model_none_basic— basic ASR withpunc_model=Nonereturns correct texttest_sentence_timestamp_with_punc_model_none—sentence_timestamp=Truewithpunc_model=Nonereturns emptysentence_infoinstead of crashingtest_punc_model_with_value_still_works— normal flow withpunc_modelprovided still produces punctuated text🤖 Generated with Claude Code