Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts TTS engine initialization order so model-specific paths are computed before engine loading, and refactors a couple engines to store model_path on the instance for reuse during loading.
Changes:
- Move
load_engine()/_load_engine_zs()calls to occur afterself.model_pathis computed (vits, tacotron, glowtts, tortoise). - In
fairseqandbark, compute and storeself.model_pathduring__init__and reuse it inload_engine(). - Simplify
fairseq/barkload logic by removing duplicate model-path derivation code.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/classes/tts_engines/vits.py | Defer engine loading until after model_path is set. |
| lib/classes/tts_engines/tortoise.py | Defer engine loading until after model_path is set. |
| lib/classes/tts_engines/tacotron.py | Defer engine + zero-shot engine loading until after model_path is set. |
| lib/classes/tts_engines/glowtts.py | Defer engine + zero-shot engine loading until after model_path is set. |
| lib/classes/tts_engines/fairseq.py | Persist model_path in __init__ and use it in load_engine(). |
| lib/classes/tts_engines/bark.py | Persist model_path in __init__ and use it in load_engine(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.model_path = model_cfg['repo'].replace("[lang]", self.session['language']) | ||
| self.engine = self.load_engine() | ||
| self.engine_zs = self._load_engine_zs() | ||
| except Exception as e: |
There was a problem hiding this comment.
model_cfg = self.models[self.session['fine_tuned']] can throw KeyError and will be surfaced as a generic ValueError('__init__() error: ...') with only the missing dict key. To keep user-facing errors actionable, catch KeyError here and raise a more descriptive message indicating the invalid fine_tuned option.
| except Exception as e: | |
| except Exception as e: | |
| # Provide a clearer error message when an invalid 'fine_tuned' option is used. | |
| if isinstance(e, KeyError): | |
| fine_tuned = None | |
| try: | |
| # Use get to avoid raising a second KeyError if the key is missing. | |
| fine_tuned = self.session.get('fine_tuned') | |
| except Exception: | |
| # If accessing session fails for any reason, leave fine_tuned as None. | |
| pass | |
| available = None | |
| if hasattr(self, 'models') and isinstance(self.models, dict): | |
| try: | |
| available = ', '.join(self.models.keys()) | |
| except Exception: | |
| available = None | |
| if available: | |
| error = ( | |
| f"Invalid 'fine_tuned' option {fine_tuned!r} for TTS engine " | |
| f"{self.session.get('tts_engine')!r}. " | |
| f"Available options are: {available}" | |
| ) | |
| else: | |
| error = ( | |
| f"Invalid 'fine_tuned' option {fine_tuned!r} for TTS engine " | |
| f"{self.session.get('tts_engine')!r}." | |
| ) | |
| raise ValueError(error) from e |
| model_cfg = self.models[self.session['fine_tuned']] | ||
| self.model_path = model_cfg['repo'] | ||
| self.engine = self.load_engine() |
There was a problem hiding this comment.
model_cfg = self.models[self.session['fine_tuned']] will raise a bare KeyError if fine_tuned is invalid, which then gets wrapped into a generic ValueError('__init__() error: ...') and loses the more actionable context (which model key was invalid). Consider catching KeyError here and raising a clearer message like "Invalid fine_tuned model ''" (optionally chaining the original exception).
| except Exception as e: | ||
| error = 'load_engine(): _load_api() failed' | ||
| raise RuntimeError(error) from e | ||
| engine = self._load_api(self.tts_key, self.model_path) |
There was a problem hiding this comment.
load_engine() no longer wraps _load_api() failures, so exceptions will bubble up without the consistent load_engine(): _load_api() failed context used by other engines in this package. Recommend restoring a try/except around _load_api() and re-raising a RuntimeError (with from e) for consistent, debuggable error reporting.
| engine = self._load_api(self.tts_key, self.model_path) | |
| try: | |
| engine = self._load_api(self.tts_key, self.model_path) | |
| except Exception as e: | |
| error = f"load_engine(): _load_api() failed: {e}" | |
| raise RuntimeError(error) from e |
No description provided.