Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds comprehensive input validation to TTS (Text-to-Speech) engine initialization methods and performs minor formatting cleanup in documentation. The changes prevent runtime errors by validating configuration parameters early in the initialization process.
Changes:
- Added validation logic to all TTS engine
__init__methods to check for validfine_tunedmodels, required model configuration keys, and supported language/engine combinations - Moved configuration validation and
model_pathassignment earlier in initialization, before expensive operations like engine loading - Removed extraneous blank lines in Docker usage documentation in
app.py
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/classes/tts_engines/yourtts.py | Added validation for fine_tuned model existence and required config keys ('repo', 'samplerate'), plus early model_path assignment |
| lib/classes/tts_engines/xtts.py | Added validation for fine_tuned model existence and required config keys ('repo', 'samplerate') |
| lib/classes/tts_engines/vits.py | Added comprehensive validation for tts_engine, language, fine_tuned, and required config keys; moved validation logic before engine loading |
| lib/classes/tts_engines/tortoise.py | Added comprehensive validation for tts_engine, language, fine_tuned, and required config keys; moved validation logic before engine loading |
| lib/classes/tts_engines/tacotron.py | Added comprehensive validation for tts_engine, language, fine_tuned, and required config keys; moved validation logic before engine loading with fallback logic |
| lib/classes/tts_engines/glowtts.py | Added comprehensive validation for tts_engine, language, fine_tuned, and required config keys; moved validation logic before engine loading |
| lib/classes/tts_engines/fairseq.py | Added validation for fine_tuned model existence and required config keys; moved model path calculation earlier; added spacing cleanup |
| lib/classes/tts_engines/bark.py | Added validation for fine_tuned model existence and required config keys; moved model path calculation earlier |
| app.py | Removed three blank lines from Docker usage documentation for cleaner formatting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| tts_engine = self.session.get('tts_engine') | ||
| language = self.session.get('language') | ||
| fine_tuned = self.session.get('fine_tuned') | ||
| if tts_engine not in default_engine_settings: | ||
| error = f'Invalid tts_engine {tts_engine}.' | ||
| raise ValueError(error) | ||
| engine_langs = default_engine_settings[tts_engine].get('languages', {}) | ||
| if language not in engine_langs: | ||
| error = f'Language {language} not supported by engine {tts_engine}.' | ||
| raise ValueError(error) | ||
| iso_dir = engine_langs[language] | ||
| if fine_tuned not in self.models: | ||
| error = f'Invalid fine_tuned model {fine_tuned}. Available models: {list(self.models.keys())}' | ||
| raise ValueError(error) | ||
| model_cfg = self.models[fine_tuned] | ||
| for required_key in ('repo', 'samplerate', 'sub'): | ||
| if required_key not in model_cfg: | ||
| error = f'fine_tuned model {fine_tuned} is missing required key {required_key}.' | ||
| raise ValueError(error) | ||
| sub_dict = model_cfg['sub'] | ||
| sub = next((key for key, lang_list in sub_dict.items() if iso_dir in lang_list), None) | ||
| if sub is None: | ||
| error = f'{tts_engine} checkpoint for {language} not found!' | ||
| raise KeyError(error) | ||
| self.params['samplerate'] = model_cfg['samplerate'][sub] | ||
| self.model_path = model_cfg['repo'].replace('[lang_iso1]', iso_dir).replace('[xxx]', sub) |
There was a problem hiding this comment.
This validation logic is duplicated across multiple TTS engine files (vits.py, tortoise.py, tacotron.py). Consider extracting this common validation pattern into a shared utility method in TTSUtils or a separate validation helper. This would improve maintainability by centralizing the validation logic and making it easier to update consistently across all engines.
| fine_tuned = self.session.get('fine_tuned') | ||
| if fine_tuned not in self.models: | ||
| error = f'Invalid fine_tuned model {fine_tuned}. Available models: {list(self.models.keys())}' | ||
| raise ValueError(error) | ||
| model_cfg = self.models[fine_tuned] | ||
| for required_key in ('repo', 'samplerate'): | ||
| if required_key not in model_cfg: | ||
| error = f'fine_tuned model {fine_tuned} is missing required key {required_key}.' | ||
| raise ValueError(error) | ||
| self.params['samplerate'] = model_cfg['samplerate'] | ||
| self.model_path = model_cfg['repo'].replace("[lang]", self.session['language']) |
There was a problem hiding this comment.
This validation logic pattern (for fine_tuned models) is duplicated across multiple TTS engine files (bark.py, fairseq.py, yourtts.py, xtts.py). Consider extracting this common validation pattern into a shared utility method in TTSUtils or a separate validation helper to improve maintainability and consistency.
| fine_tuned = self.session.get('fine_tuned') | ||
| if fine_tuned not in self.models: | ||
| error = f'Invalid fine_tuned model {fine_tuned}. Available models: {list(self.models.keys())}' | ||
| raise ValueError(error) | ||
| model_cfg = self.models[fine_tuned] | ||
| for required_key in ('repo', 'samplerate'): | ||
| if required_key not in model_cfg: | ||
| error = f'fine_tuned model {fine_tuned} is missing required key {required_key}.' | ||
| raise ValueError(error) | ||
| self.params['samplerate'] = model_cfg['samplerate'] | ||
| self.model_path = model_cfg['repo'] |
There was a problem hiding this comment.
This validation logic pattern (for fine_tuned models) is duplicated across multiple TTS engine files (bark.py, fairseq.py, yourtts.py, xtts.py). Consider extracting this common validation pattern into a shared utility method in TTSUtils or a separate validation helper to improve maintainability and consistency.
| tts_engine = self.session.get('tts_engine') | ||
| language = self.session.get('language') | ||
| fine_tuned = self.session.get('fine_tuned') | ||
| if tts_engine not in default_engine_settings: | ||
| error = f'Invalid tts_engine {tts_engine}.' | ||
| raise ValueError(error) | ||
| engine_langs = default_engine_settings[tts_engine].get('languages', {}) | ||
| if language not in engine_langs: | ||
| error = f'Language {language} not supported by engine {tts_engine}.' | ||
| raise ValueError(error) | ||
| iso_dir = engine_langs[language] | ||
| if fine_tuned not in self.models: | ||
| error = f'Invalid fine_tuned model {fine_tuned}. Available models: {list(self.models.keys())}' | ||
| raise ValueError(error) | ||
| model_cfg = self.models[fine_tuned] | ||
| for required_key in ('repo', 'samplerate', 'sub'): | ||
| if required_key not in model_cfg: | ||
| error = f'fine_tuned model {fine_tuned} is missing required key {required_key}.' | ||
| raise ValueError(error) | ||
| sub_dict = model_cfg['sub'] | ||
| sub = next((key for key, lang_list in sub_dict.items() if iso_dir in lang_list), None) | ||
| if sub is None: | ||
| error = f'{tts_engine} checkpoint for {language} not found.' | ||
| raise KeyError(error) | ||
| self.params['samplerate'] = model_cfg['samplerate'][sub] | ||
| self.model_path = model_cfg['repo'].replace('[lang_iso1]', iso_dir).replace('[xxx]', sub) |
There was a problem hiding this comment.
This validation logic is duplicated across multiple TTS engine files (vits.py, tacotron.py, glowtts.py). Consider extracting this common validation pattern into a shared utility method in TTSUtils or a separate validation helper. This would improve maintainability by centralizing the validation logic and making it easier to update consistently across all engines.
| tts_engine = self.session.get('tts_engine') | ||
| language = self.session.get('language') | ||
| fine_tuned = self.session.get('fine_tuned') | ||
| if tts_engine not in default_engine_settings: | ||
| error = f'Invalid tts_engine {tts_engine}.' | ||
| raise ValueError(error) | ||
| engine_langs = default_engine_settings[tts_engine].get('languages', {}) | ||
| if language not in engine_langs: | ||
| error = f'Language {language} not supported by engine {tts_engine}.' | ||
| raise ValueError(error) | ||
| iso_dir = engine_langs[language] | ||
| if fine_tuned not in self.models: | ||
| error = f'Invalid fine_tuned model {fine_tuned}. Available models: {list(self.models.keys())}' | ||
| raise ValueError(error) |
There was a problem hiding this comment.
Potential issue: using session.get('tts_engine'), session.get('language'), and session.get('fine_tuned') return None if keys don't exist, which would cause unclear error messages. For example, line 22 would show 'Invalid tts_engine None' and line 30 would show 'Invalid fine_tuned model None'. Consider checking if these values are None first and providing more specific error messages like 'Missing required session key: tts_engine/language/fine_tuned'.
No description provided.