feat: model manager#49
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds an Engine enum and engine field to VoiceConfig; introduces TTSModelInfo and TTSModelManager for remote voice discovery, caching, download and on-demand loading; centralizes language matching via match_lang used by phonemizers; and forces "espeak" phoneme_type in Piper export. Logging imports consolidated to phoonnx.util.LOG. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Manager as TTSModelManager
participant Cache as LocalCache (json)
participant Remote as External Source (Piper/Mimic3/...)
participant Info as TTSModelInfo
participant Voice as TTSVoice
Note over Manager,Cache: Manager lifecycle & cache persistence
User->>Manager: load()
Manager->>Cache: read cached metadata
Cache-->>Manager: metadata list
User->>Manager: get_piper_voice_list()
Manager->>Remote: fetch descriptors
Remote-->>Manager: descriptors
Manager->>Info: instantiate TTSModelInfo(...)
Info->>Info: __post_init__ (ensure dir, download config)
Manager->>Cache: persist updated metadata
User->>Manager: get_lang_voices(lang)
Manager->>Manager: filter using util.match_lang
Manager-->>User: list[TTSModelInfo]
User->>Info: load(phoneme_type?)
Info->>Info: download model.onnx / tokens.txt as needed
Info->>Voice: create TTSVoice (apply phoneme_type/phonemizer, expose engine)
Voice-->>User: ready-to-use voice
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
phoonnx/phonemizers/base.py (1)
10-10: LGTM! Good refactor to centralize language matching.Delegating to
phoonnx.util.match_langreduces code duplication and centralizes the language validation logic.Static analysis suggests extracting the error message to improve maintainability:
+class UnsupportedLanguageError(ValueError): + """Raised when a language code is not supported.""" + def __init__(self, lang: str): + super().__init__(f"unsupported language code: {lang}") + @staticmethod def match_lang(target_lang: str, valid_langs: List[str]) -> str: """ Validates and returns the closest supported language code. Args: target_lang (str): The language code to validate. Returns: str: The validated language code. Raises: ValueError: If the language code is unsupported. """ lang = match_lang(target_lang, valid_langs) if not lang: - # raise an error for unsupported language - raise ValueError(f"unsupported language code: {target_lang}") + raise UnsupportedLanguageError(target_lang) return langAlso applies to: 99-103
phoonnx/model_manager.py (3)
141-145: Fix unused loop variable.The loop variable
voice_idis not used within the loop body.Apply this diff:
def get_lang_voices(self, lang: str) -> Iterable[TTSModelInfo]: # TODO - sort by lang distance - for voice_id, voice_info in self.voices.items(): + for _voice_id, voice_info in self.voices.items(): if match_lang(voice_info.lang, lang): yield voice_info
226-241: Improve error handling in get_ovos_voice_list.Issue: Catching bare Exception without logging (lines 240-241)
Apply this diff:
for voice in ["miro", "dii"]: repo = f"OpenVoiceOS/pipertts_{lang}_{voice}" try: self.add_voice(TTSModelInfo( lang=lang, voice_id=repo, model_url=f"https://huggingface.co/{repo}/resolve/main/{voice}_{lang}.onnx", config_url=f"https://huggingface.co/{repo}/resolve/main/{voice}_{lang}.onnx.json", )) - except Exception: - continue # not all langs have male + female + except Exception as e: + LOG.debug(f"Voice {repo} not available (not all langs have male + female): {e}") + continue
247-249: Fix ambiguous variable name in supported_langs property.The variable name
lis ambiguous and should be more descriptive.Apply this diff:
@property def supported_langs(self) -> Iterable[str]: - return sorted(set(l.lang for l in self.all_voices)) + return sorted(set(voice.lang for voice in self.all_voices))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
phoonnx/config.py(8 hunks)phoonnx/model_manager.py(1 hunks)phoonnx/phonemizers/base.py(2 hunks)phoonnx/util.py(5 hunks)phoonnx_train/export_onnx.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
phoonnx/phonemizers/base.py (1)
phoonnx/util.py (2)
normalize(686-716)match_lang(21-58)
phoonnx/config.py (1)
phoonnx/model_manager.py (1)
engine(54-55)
phoonnx/model_manager.py (3)
phoonnx/config.py (6)
PhonemeType(49-83)get_phonemizer(402-477)VoiceConfig(87-368)Engine(20-26)Alphabet(29-45)from_dict(219-368)phoonnx/util.py (1)
match_lang(21-58)phoonnx/voice.py (1)
TTSVoice(106-378)
phoonnx/util.py (1)
phoonnx/phonemizers/base.py (1)
match_lang(86-103)
🪛 Ruff (0.14.0)
phoonnx/phonemizers/base.py
102-102: Avoid specifying long messages outside the exception class
(TRY003)
phoonnx/model_manager.py
68-68: Probable use of requests call without timeout
(S113)
76-76: Probable use of requests call without timeout
(S113)
83-83: Probable use of requests call without timeout
(S113)
143-143: Loop control variable voice_id not used within loop body
Rename unused voice_id to _voice_id
(B007)
165-165: Probable use of requests call without timeout
(S113)
172-172: Prefer next(a for a in v["files"] if a.endswith(".onnx")) over single element slice
Replace with next(a for a in v["files"] if a.endswith(".onnx"))
(RUF015)
173-173: Prefer next(a for a in v["files"] if a.endswith(".json")) over single element slice
Replace with next(a for a in v["files"] if a.endswith(".json"))
(RUF015)
176-176: Do not catch blind exception: Exception
(BLE001)
181-181: Probable use of requests call without timeout
(S113)
201-201: Do not catch blind exception: Exception
(BLE001)
240-241: try-except-continue detected, consider logging the exception
(S112)
240-240: Do not catch blind exception: Exception
(BLE001)
249-249: Ambiguous variable name: l
(E741)
phoonnx/util.py
40-40: Ambiguous variable name: l
(E741)
43-43: Do not use bare except
(E722)
45-45: Ambiguous variable name: l
(E741)
47-47: Do not use bare except
(E722)
50-50: Do not use bare except
(E722)
50-51: try-except-continue detected, consider logging the exception
(S112)
🔇 Additional comments (6)
phoonnx/config.py (3)
20-27: LGTM! Clean enum design.The Engine enum clearly identifies supported TTS frameworks. The string inheritance and explicit value assignments make it JSON-serializable and human-readable.
122-124: LGTM! Well-documented engine field.The engine field with a sensible default of Engine.PHOONNX is a good addition. The comment clearly explains its purpose.
229-354: LGTM! Engine assignment logic is consistent.The engine is correctly assigned based on model type detection (phoonnx, piper, mimic3, coqui) and properly passed to the VoiceConfig constructor at line 354.
phoonnx/util.py (2)
13-18: LGTM! Resilient logging configuration.The try/except block provides a graceful fallback when
ovos_utils.logis not available, ensuring the module works in different environments.
598-598: LGTM! Character normalization fix.Replacing "º" with "°" handles the visual similarity of these characters for temperature units.
phoonnx/model_manager.py (1)
27-48: Verify the necessity of the phoneme_type hack at lines 38-39 for current production Piper models.The search found no malformed "PhonemeType.ESPEAK" entries in the repository. However, since this hack targets external Piper-published models (e.g., "es_MX-ald-medium") downloaded at runtime, the absence of evidence in the repo doesn't confirm the hack is obsolete.
Confirm whether this issue still occurs in the latest published Piper voice models you support. If it does not, this is technical debt and should be removed. If it persists, document the specific affected models or add a link to the issue for context.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
phoonnx/model_manager.py (3)
65-71: Add timeouts, raise_for_status, and stream large downloads.Prevent hangs and partial/corrupt files. Also validate HTTP status before parsing JSON/text.
def download_config(self): config_path = self.voice_path / "model.json" if not config_path.is_file(): - r = requests.get(self.config_url) - cfg = r.json() # validate received json + r = requests.get(self.config_url, timeout=30) + r.raise_for_status() + cfg = r.json() # validate received json with open(config_path, "w") as f: json.dump(cfg, f, ensure_ascii=False, indent=4) def download_phoneme_map(self): tokens_path = self.voice_path / "tokens.txt" if self.tokens_url and not tokens_path.is_file(): - tokens = requests.get(self.tokens_url).text + r = requests.get(self.tokens_url, timeout=30) + r.raise_for_status() + tokens = r.text with open(tokens_path, "w") as f: f.write(tokens) def download_model(self): model_path = self.voice_path / "model.onnx" if not model_path.is_file(): - r = requests.get(self.model_url) - with open(model_path, "wb") as f: - f.write(r.content) + with requests.get(self.model_url, stream=True, timeout=120) as r: + r.raise_for_status() + with open(model_path, "wb") as f: + for chunk in r.iter_content(chunk_size=8192): + if chunk: + f.write(chunk)Also applies to: 73-79, 80-85
164-180: Harden Piper voice discovery: timeout, status check, safer file selection, and logging.Avoid hangs and IndexError; log failures instead of print.
def get_piper_voice_list(self): base = "https://huggingface.co/rhasspy/piper-voices/resolve/main/" voice_list = "https://huggingface.co/rhasspy/piper-voices/resolve/main/voices.json" - piper_voices = requests.get(voice_list).json() + resp = requests.get(voice_list, timeout=30) + resp.raise_for_status() + piper_voices = resp.json() for v in piper_voices.values(): try: - voice = TTSModelInfo( - voice_id="piper_" + v["key"], - lang=standardize_lang_tag(v["key"].split("-")[0]), - model_url=base + [a for a in v["files"] if a.endswith(".onnx")][0], - config_url=base + [a for a in v["files"] if a.endswith(".json")][0], - ) + onnx_file = next((a for a in v["files"] if a.endswith(".onnx")), None) + json_file = next((a for a in v["files"] if a.endswith(".json")), None) + if not onnx_file or not json_file: + LOG.warning(f"Missing .onnx/.json for Piper voice {v.get('key')}") + continue + voice = TTSModelInfo( + voice_id="piper_" + v["key"], + lang=standardize_lang_tag(v["key"].split("-")[0]), + model_url=base + onnx_file, + config_url=base + json_file, + ) self.add_voice(voice) - except Exception: - print(f"Failed to get voice info for {v['key']}") + except KeyError as e: + LOG.error(f"Piper voice entry malformed: {e} | data={v}")Add LOG import at top of the file:
+try: + from ovos_utils.log import LOG +except ImportError: + import logging + LOG = logging.getLogger(__name__)
181-205: Harden Mimic3 voice discovery and fix lang assignment.Add timeout/status check and use lang_code instead of non-existent lang attribute.
def get_mimic3_voice_list(self): voice_list = "https://raw.githubusercontent.com/MycroftAI/mimic3/refs/heads/master/mimic3_tts/voices.json" - mimic3_voices = requests.get(voice_list).json() + resp = requests.get(voice_list, timeout=30) + resp.raise_for_status() + mimic3_voices = resp.json() for k, v in mimic3_voices.items(): try: lang = standardize_lang_tag(k.split("/")[0]) speaker_map = {s: idx for idx, s in enumerate(v["speakers"])} config_url = f"https://huggingface.co/mukowaty/mimic3-voices/resolve/main/voices/{k}/config.json" model_url = f"https://huggingface.co/mukowaty/mimic3-voices/resolve/main/voices/{k}/generator.onnx" tokens_url = f"https://huggingface.co/mukowaty/mimic3-voices/resolve/main/voices/{k}/phonemes.txt" phoneme_map_url = f"https://huggingface.co/mukowaty/mimic3-voices/resolve/main/voices/{k}/phoneme_map.txt" voice_info = TTSModelInfo( voice_id="mimic3_" + k, lang=lang, config_url=config_url, tokens_url=tokens_url, model_url=model_url, phoneme_map_url=phoneme_map_url ) - voice_info.config.lang = lang + # Ensure config language is consistent with our standardized lang + voice_info.config.lang_code = lang voice_info.config.speaker_id_map = speaker_map self.add_voice(voice_info) - except Exception: - print(f"Failed to get voice info for {k}") + except KeyError as e: + LOG.error(f"Mimic3 voice entry malformed for {k}: {e}")
🧹 Nitpick comments (6)
phoonnx/model_manager.py (4)
241-243: Avoid bare except; at least log why a voice is skipped.Swallowing all exceptions hides issues and complicates debugging.
- except Exception: - continue # not all langs have male + female + except Exception as e: + LOG.debug(f"OVOS Piper repo missing for {repo}: {e}") + continue # not all langs have male + female
141-148: Align distance threshold with util.match_lang semantics.util.match_lang treats <= 10 as acceptable; here you exclude exactly-10 matches.
- return [v[0] for v in voices if v[1] < 10] + return [v[0] for v in voices if v[1] <= 10]
41-45: Consider downloading and wiring phoneme_map_url (and clarify method name).download_phoneme_map() actually downloads tokens.txt; phoneme_map_url is unused.
- Option A: Rename to download_tokens() for clarity.
- Option B: Also fetch phoneme_map_url (e.g., to phoneme_map.txt/json) and pass its path to TTSVoice.load(phoneme_map=...).
249-251: Nit: avoid ambiguous variable name in supported_langs.- return sorted(set(l.lang for l in self.all_voices)) + return sorted({voice.lang for voice in self.all_voices})phoonnx/phonemizers/base.py (1)
10-10: Avoid name shadowing: alias imported match_lang.Prevents confusion with BasePhonemizer.match_lang staticmethod.
-from phoonnx.util import normalize, match_lang +from phoonnx.util import normalize, match_lang as match_lang_utilphoonnx/util.py (1)
21-59: Harden match_lang: fix docstring, variable naming, and exception handling.Removes bare excepts, clarifies flow, and adds debug logging for fallbacks.
-def match_lang(target_lang: str, valid_langs: Union[str, List[str]]) -> Tuple[str, int]: - """ - Validates and returns the closest supported language code. - - Args: - target_lang (str): The language code to validate. - - Returns: - str: The validated language code. - - Raises: - ValueError: If the language code is unsupported. - """ +def match_lang(target_lang: str, valid_langs: Union[str, List[str]]) -> Tuple[str, int]: + """ + Returns the closest supported language and its distance score. + + Args: + target_lang: The language code to validate (e.g., 'pt-PT'). + valid_langs: A single language code or list of codes to match against. + + Returns: + (best_lang, score): best_lang is the closest match (or 'und'), score is an int distance + where 0 is exact. Scores <= 10 are considered acceptable. + """ if isinstance(valid_langs, str): valid_langs = [valid_langs] if target_lang in valid_langs: return target_lang, 0 best_lang = "und" best_distance = 10000000 - for l in valid_langs: - try: - distance: int = tag_distance(l, target_lang) - except: - try: - l = f"{l.split('-')[0]}-{l.split('-')[1]}" - distance: int = tag_distance(l, target_lang) - except: - try: - distance: int = tag_distance(l.split('-')[0], target_lang) - except: - continue - if distance < best_distance: - best_lang, best_distance = l, distance + for candidate in valid_langs: + distance: Optional[int] = None + # Try exact tag first + try: + distance = tag_distance(candidate, target_lang) + except Exception as e: + LOG.debug(f"tag_distance failed for '{candidate}' vs '{target_lang}': {e}") + # Try primary-secondary form if missing region + if distance is None: + parts = candidate.split("-") + if len(parts) >= 2: + normalized = f"{parts[0]}-{parts[1]}" + try: + distance = tag_distance(normalized, target_lang) + except Exception as e: + LOG.debug(f"tag_distance failed for normalized '{normalized}': {e}") + # Try primary only + if distance is None: + base = candidate.split("-")[0] + try: + distance = tag_distance(base, target_lang) + except Exception as e: + LOG.debug(f"tag_distance failed for base '{base}': {e}") + continue + if distance < best_distance: + best_lang, best_distance = candidate, distance # If the score is low (meaning a good match), return the language if best_distance <= 10: return best_lang, best_distance return "und", 10000
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
phoonnx/model_manager.py(1 hunks)phoonnx/phonemizers/base.py(2 hunks)phoonnx/util.py(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
phoonnx/util.py (1)
phoonnx/phonemizers/base.py (1)
match_lang(86-103)
phoonnx/model_manager.py (3)
phoonnx/config.py (6)
PhonemeType(49-83)get_phonemizer(402-477)VoiceConfig(87-368)Engine(20-26)Alphabet(29-45)from_dict(219-368)phoonnx/util.py (1)
match_lang(21-58)phoonnx/voice.py (1)
TTSVoice(106-378)
phoonnx/phonemizers/base.py (1)
phoonnx/util.py (2)
normalize(686-716)match_lang(21-58)
🪛 GitHub Actions: Run Unit Tests
phoonnx/phonemizers/base.py
[error] 102-102: ValueError: unsupported language code: ar. The language code 'ar' is not recognized by the language matcher in Phonemizer base, causing all Arabic phonemization paths to fail.
🪛 Ruff (0.14.0)
phoonnx/util.py
40-40: Ambiguous variable name: l
(E741)
43-43: Do not use bare except
(E722)
45-45: Ambiguous variable name: l
(E741)
47-47: Do not use bare except
(E722)
50-50: Do not use bare except
(E722)
50-51: try-except-continue detected, consider logging the exception
(S112)
phoonnx/model_manager.py
68-68: Probable use of requests call without timeout
(S113)
76-76: Probable use of requests call without timeout
(S113)
83-83: Probable use of requests call without timeout
(S113)
167-167: Probable use of requests call without timeout
(S113)
174-174: Prefer next(a for a in v["files"] if a.endswith(".onnx")) over single element slice
Replace with next(a for a in v["files"] if a.endswith(".onnx"))
(RUF015)
175-175: Prefer next(a for a in v["files"] if a.endswith(".json")) over single element slice
Replace with next(a for a in v["files"] if a.endswith(".json"))
(RUF015)
178-178: Do not catch blind exception: Exception
(BLE001)
183-183: Probable use of requests call without timeout
(S113)
203-203: Do not catch blind exception: Exception
(BLE001)
242-243: try-except-continue detected, consider logging the exception
(S112)
242-242: Do not catch blind exception: Exception
(BLE001)
251-251: Ambiguous variable name: l
(E741)
phoonnx/phonemizers/base.py
102-102: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (1)
phoonnx/util.py (1)
598-631: Good improvement: unit normalization and locale-aware parsing.Replacing º with ° and handling separators improves robustness. LGTM.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
phoonnx/config.py (1)
325-350: Avoid None tokens; supply sane defaults at construction.pad/blank/bos/eos/word_sep can become None if missing in config; pass defaults explicitly.
Apply this diff:
- include_whitespace = " " in config.get("characters", "") or " " in config.get("phoneme_id_map", {}) + include_whitespace = (" " in (config.get("characters") or "")) or (" " in (config.get("phoneme_id_map") or {})) return VoiceConfig( @@ - pad_token=config.get("pad"), - blank_token=config.get("blank"), - bos_token=config.get("bos"), - eos_token=config.get("eos"), - word_sep_token=config.get("word_sep_token") or config.get("blank_word", " ") + pad_token=config.get("pad", DEFAULT_PAD_TOKEN), + blank_token=config.get("blank", DEFAULT_BLANK_TOKEN), + bos_token=config.get("bos", DEFAULT_BOS_TOKEN), + eos_token=config.get("eos", DEFAULT_EOS_TOKEN), + word_sep_token=config.get("word_sep_token") or config.get("blank_word", DEFAULT_BLANK_WORD_TOKEN) )Additionally, the dataclass default for blank_token appears to use DEFAULT_PAD_TOKEN; adjust to DEFAULT_BLANK_TOKEN (outside this hunk).
♻️ Duplicate comments (3)
phoonnx/config.py (1)
203-205: Restore backward-compatible phoonnx detection (or ensure all configs define phoonnx_version).Relying solely on phoonnx_version will mis-detect existing voices lacking this key. Add a heuristic fallback and exclude other engines explicitly.
Apply this diff:
@staticmethod def is_phoonnx(config: dict[str, Any]) -> bool: - return "phoonnx_version" in config + # Prefer explicit version flag + if "phoonnx_version" in config: + return True + # Exclude known other engines + if (VoiceConfig.is_piper(config) + or VoiceConfig.is_mimic3(config) + or VoiceConfig.is_coqui_vits(config)): + return False + # Backward-compat: typical PhoonNX configs have these keys + return any(k in config for k in ("phoneme_id_map", "num_symbols", "phoneme_type"))To confirm impact, run:
#!/bin/bash # Count configs that reference phoonnx_version rg -n --type=json --type=yaml '"phoonnx_version"\s*:' -C1 || truephoonnx/model_manager.py (2)
185-209: Harden get_mimic3_voice_list: timeout and logging.Same robustness improvements as above.
Apply this diff:
def get_mimic3_voice_list(self): voice_list = "https://raw.githubusercontent.com/MycroftAI/mimic3/refs/heads/master/mimic3_tts/voices.json" - mimic3_voices = requests.get(voice_list).json() + r = requests.get(voice_list, timeout=self.request_timeout) + r.raise_for_status() + mimic3_voices = r.json() for k, v in mimic3_voices.items(): try: @@ voice_info = TTSModelInfo( voice_id="mimic3_" + k, lang=lang, config_url=config_url, tokens_url=tokens_url, model_url=model_url, phoneme_map_url=phoneme_map_url ) voice_info.config.lang = lang voice_info.config.speaker_id_map = speaker_map self.add_voice(voice_info) - except Exception: - print(f"Failed to get voice info for {k}") + except Exception as e: + LOG.error(f"Failed to get mimic3 voice info for {k}: {e}")
168-184: Harden get_piper_voice_list: timeout, status, safer selection, logging.Avoid hangs, handle missing files, and log failures.
Apply this diff:
def get_piper_voice_list(self): base = "https://huggingface.co/rhasspy/piper-voices/resolve/main/" voice_list = "https://huggingface.co/rhasspy/piper-voices/resolve/main/voices.json" - piper_voices = requests.get(voice_list).json() + r = requests.get(voice_list, timeout=self.request_timeout) + r.raise_for_status() + piper_voices = r.json() for v in piper_voices.values(): try: + onnx_file = next((a for a in v["files"] if a.endswith(".onnx")), None) + json_file = next((a for a in v["files"] if a.endswith(".json")), None) + if not onnx_file or not json_file: + LOG.warning(f"Missing files for piper voice {v.get('key')}") + continue voice = TTSModelInfo( voice_id="piper_" + v["key"], lang=standardize_lang_tag(v["key"].split("-")[0]), - model_url=base + [a for a in v["files"] if a.endswith(".onnx")][0], - config_url=base + [a for a in v["files"] if a.endswith(".json")][0], + model_url=base + onnx_file, + config_url=base + json_file, ) self.add_voice(voice) - except Exception: - print(f"Failed to get voice info for {v['key']}") + except Exception as e: + LOG.error(f"Failed to get piper voice info for {v.get('key')}: {e}")
🧹 Nitpick comments (8)
phoonnx/model_manager.py (8)
28-47: Avoid network I/O in post_init (enumeration triggers hundreds of downloads).Instantiate metadata without downloading; defer to load() or an explicit ensure_local(). This reduces latency and failures when listing voices.
34-36: Open JSON with explicit encoding and handle decode errors.Use UTF‑8 and surface errors clearly.
Apply this diff:
- with open(config_path, "r") as f: - config = json.load(f) + with open(config_path, "r", encoding="utf-8") as f: + config = json.load(f)
63-66: Sanitize voice_id in voice_path to prevent path escape.Strip root and parent segments so arbitrary voice_id can’t traverse outside cache base.
Apply this diff:
def voice_path(self) -> Path: - return xdg_cache_home() / "phoonnx" / "voices" / self.voice_id + base = xdg_cache_home() / "phoonnx" / "voices" + parts = [p for p in Path(self.voice_id).parts if p not in ("..", "/")] + return base.joinpath(*parts)
106-112: Expose configurable request timeout on manager.Use a single knob and share it in HTTP calls.
Apply this diff:
class TTSModelManager: def __init__(self, cache_path: Optional[str] = None): self.voices: Dict[str, TTSModelInfo] = {} + self.request_timeout: int = 30 if cache_path: self.cache = JsonStorage(cache_path) else: self.cache = JsonStorageXDG("voices", subfolder="phoonnx")
122-133: Persist enum values as strings in cache for stability.Store PhonemeType.value to avoid non-JSON-serializable objects and simplify reload.
Apply this diff:
def save(self): self.cache.clear() for voice_id, voice_info in self.voices.items(): self.cache[voice_id] = {"voice_id": voice_info.voice_id, "model_url": voice_info.model_url, - "phoneme_type": voice_info.phoneme_type, + "phoneme_type": (voice_info.phoneme_type.value + if isinstance(voice_info.phoneme_type, PhonemeType) + else voice_info.phoneme_type), "lang": voice_info.lang, "tokens_url": voice_info.tokens_url, "phoneme_map_url": voice_info.phoneme_map_url, "config_url": voice_info.config_url} self.cache.store()
134-143: Same for add_voice: store enum .value and persist immediately (optional).Prevents type drift on reload.
Apply this diff:
def add_voice(self, voice_info: TTSModelInfo): self.voices[voice_info.voice_id] = voice_info self.cache[voice_info.voice_id] = {"voice_id": voice_info.voice_id, "model_url": voice_info.model_url, "tokens_url": voice_info.tokens_url, - "phoneme_type": voice_info.phoneme_type, + "phoneme_type": (voice_info.phoneme_type.value + if isinstance(voice_info.phoneme_type, PhonemeType) + else voice_info.phoneme_type), "phoneme_map_url": voice_info.phoneme_map_url, "lang": voice_info.lang, "config_url": voice_info.config_url} + # optionally: self.cache.store()
232-248: Avoid bare except; log and continue.Catching all exceptions without logging hides issues.
Apply this diff:
for lang in piper_ovos: for voice in ["miro", "dii"]: repo = f"OpenVoiceOS/pipertts_{lang}_{voice}" try: self.add_voice(TTSModelInfo( lang=lang, voice_id=repo, model_url=f"https://huggingface.co/{repo}/resolve/main/{voice}_{lang}.onnx", config_url=f"https://huggingface.co/{repo}/resolve/main/{voice}_{lang}.onnx.json", )) - except Exception: - continue # not all langs have male + female + except Exception as e: + LOG.warning(f"Skipping OVOS piper voice {repo}: {e}") + continue # not all langs have male + female
264-266: Rename ambiguous variable.Avoid single-letter l (E741).
Apply this diff:
@property def supported_langs(self) -> Iterable[str]: - return sorted(set(l.lang for l in self.all_voices)) + return sorted(set(v.lang for v in self.all_voices))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
phoonnx/config.py(8 hunks)phoonnx/model_manager.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
phoonnx/config.py (1)
phoonnx/model_manager.py (2)
engine(60-61)alphabet(56-57)
phoonnx/model_manager.py (3)
phoonnx/config.py (6)
PhonemeType(49-83)get_phonemizer(384-459)VoiceConfig(87-350)Engine(20-26)Alphabet(29-45)from_dict(207-350)phoonnx/util.py (1)
match_lang(21-58)phoonnx/voice.py (1)
TTSVoice(106-378)
🪛 Ruff (0.14.0)
phoonnx/model_manager.py
70-70: Probable use of requests call without timeout
(S113)
78-78: Probable use of requests call without timeout
(S113)
85-85: Probable use of requests call without timeout
(S113)
171-171: Probable use of requests call without timeout
(S113)
178-178: Prefer next(a for a in v["files"] if a.endswith(".onnx")) over single element slice
Replace with next(a for a in v["files"] if a.endswith(".onnx"))
(RUF015)
179-179: Prefer next(a for a in v["files"] if a.endswith(".json")) over single element slice
Replace with next(a for a in v["files"] if a.endswith(".json"))
(RUF015)
182-182: Do not catch blind exception: Exception
(BLE001)
187-187: Probable use of requests call without timeout
(S113)
207-207: Do not catch blind exception: Exception
(BLE001)
246-247: try-except-continue detected, consider logging the exception
(S112)
246-246: Do not catch blind exception: Exception
(BLE001)
265-265: Ambiguous variable name: l
(E741)
| if not self.phoneme_type: | ||
| self.phoneme_type = self.config.phoneme_type | ||
| else: | ||
| self.config.phoneme_type = self.phoneme_type | ||
|
|
There was a problem hiding this comment.
Coerce phoneme_type to Enum and keep config in sync.
When loaded from cache, phoneme_type may be a str. Ensure Enum type and update config consistently.
Apply this diff:
if not self.phoneme_type:
self.phoneme_type = self.config.phoneme_type
else:
- self.config.phoneme_type = self.phoneme_type
+ # Accept strings persisted in cache
+ if isinstance(self.phoneme_type, str):
+ try:
+ self.phoneme_type = PhonemeType(self.phoneme_type)
+ except ValueError:
+ LOG.warning(f"Unknown phoneme_type '{self.phoneme_type}' for {self.voice_id}")
+ self.config.phoneme_type = self.phoneme_type
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
phoonnx/model_manager.py (5)
67-73: Add HTTP status check and UTF-8 encoding.The method should propagate HTTP errors and ensure correct encoding.
Apply this diff:
def download_config(self): config_path = self.voice_path / "model.json" if not config_path.is_file(): r = requests.get(self.config_url, timeout=30) + r.raise_for_status() cfg = r.json() # validate received json - with open(config_path, "w") as f: + with open(config_path, "w", encoding="utf-8") as f: json.dump(cfg, f, ensure_ascii=False, indent=4)
1-15: Critical: Add missing LOG import.Line 208 references
LOG.error()butLOGis never imported or defined, which will cause aNameErrorat runtime.Apply this diff to add the logger:
import json import os from dataclasses import dataclass from pathlib import Path from typing import Optional, Dict, Iterable, List import requests from json_database import JsonStorageXDG, JsonStorage from ovos_utils.lang import standardize_lang_tag from ovos_utils.xdg_utils import xdg_cache_home +try: + from ovos_utils.log import LOG +except ImportError: + import logging + LOG = logging.getLogger(__name__) from phoonnx.config import PhonemeType, get_phonemizer, VoiceConfig, Engine, Alphabet
168-183: Add timeout and improve error handling in get_piper_voice_list.Line 171 is missing a timeout, which can cause indefinite hangs. The bare exception at Line 182 should also log errors properly.
Apply this diff:
def get_piper_voice_list(self): base = "https://huggingface.co/rhasspy/piper-voices/resolve/main/" voice_list = "https://huggingface.co/rhasspy/piper-voices/resolve/main/voices.json" - piper_voices = requests.get(voice_list).json() + r = requests.get(voice_list, timeout=30) + r.raise_for_status() + piper_voices = r.json() for v in piper_voices.values(): try: voice = TTSModelInfo( voice_id="piper_" + v["key"], lang=standardize_lang_tag(v["key"].split("-")[0]), model_url=base + [a for a in v["files"] if a.endswith(".onnx")][0], config_url=base + [a for a in v["files"] if a.endswith(".json")][0], ) self.add_voice(voice) - except Exception: - print(f"Failed to get voice info for {v['key']}") + except Exception as e: + LOG.error(f"Failed to get voice info for {v['key']}: {e}")Additionally, consider using
next()instead of single-element list slicing for safer file lookup:- model_url=base + [a for a in v["files"] if a.endswith(".onnx")][0], - config_url=base + [a for a in v["files"] if a.endswith(".json")][0], + onnx_file = next((a for a in v["files"] if a.endswith(".onnx")), None) + json_file = next((a for a in v["files"] if a.endswith(".json")), None) + if not onnx_file or not json_file: + LOG.warning(f"Missing required files for voice {v['key']}") + continue + model_url=base + onnx_file, + config_url=base + json_file,
89-102: Fix phoneme_type override to update config and pass required parameters.Setting
voice.phoneme_typedirectly is ineffective. You must updatevoice.config.phoneme_typeand passalphabetandmodeltoget_phonemizer().Apply this diff:
voice = TTSVoice.load(model_path=model_path, config_path=config_path, phonemes_txt=str(tokens_path) if self.tokens_url else None) # override phoneme_type, if config.json is wrong - voice.phoneme_type = self.phoneme_type - voice.phonemizer = get_phonemizer(self.phoneme_type) + voice.config.phoneme_type = self.phoneme_type + voice.phonemizer = get_phonemizer(self.phoneme_type, + voice.config.alphabet, + voice.config.phonemizer_model) return voice
75-80: Add HTTP status check and UTF-8 encoding.Ensure HTTP errors are propagated and encoding is explicit.
Apply this diff:
def download_phoneme_map(self): tokens_path = self.voice_path / "tokens.txt" if self.tokens_url and not tokens_path.is_file(): - tokens = requests.get(self.tokens_url, timeout=30).text - with open(tokens_path, "w") as f: + r = requests.get(self.tokens_url, timeout=30) + r.raise_for_status() + tokens = r.text + with open(tokens_path, "w", encoding="utf-8") as f: f.write(tokens)
🧹 Nitpick comments (3)
phoonnx/model_manager.py (3)
34-35: Specify UTF-8 encoding when reading JSON.Ensure consistent encoding across different platforms.
Apply this diff:
- with open(config_path, "r") as f: + with open(config_path, "r", encoding="utf-8") as f: config = json.load(f)
82-87: Stream large model files and add status check.Loading entire ONNX models into memory is inefficient; use streaming.
Apply this diff:
def download_model(self): model_path = self.voice_path / "model.onnx" if not model_path.is_file(): - r = requests.get(self.model_url, timeout=120) # Longer timeout for model file - with open(model_path, "wb") as f: - f.write(r.content) + with requests.get(self.model_url, timeout=120, stream=True) as r: + r.raise_for_status() + with open(model_path, "wb") as f: + for chunk in r.iter_content(chunk_size=8192): + if chunk: + f.write(chunk)
232-247: Consider logging exceptions for debugging.Silent exception suppression can hide configuration or availability issues.
Apply this diff:
config_url=f"https://huggingface.co/{repo}/resolve/main/{voice}_{lang}.onnx.json", )) - except Exception: - continue # not all langs have male + female + except Exception as e: + LOG.debug(f"Voice {repo} not available: {e}") + continue # not all langs have male + female
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
phoonnx/model_manager.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
phoonnx/model_manager.py (3)
phoonnx/config.py (6)
PhonemeType(49-83)get_phonemizer(384-459)VoiceConfig(87-350)Engine(20-26)Alphabet(29-45)from_dict(207-350)phoonnx/util.py (1)
match_lang(21-58)phoonnx/voice.py (1)
TTSVoice(106-378)
🪛 Ruff (0.14.0)
phoonnx/model_manager.py
171-171: Probable use of requests call without timeout
(S113)
178-178: Prefer next(a for a in v["files"] if a.endswith(".onnx")) over single element slice
Replace with next(a for a in v["files"] if a.endswith(".onnx"))
(RUF015)
179-179: Prefer next(a for a in v["files"] if a.endswith(".json")) over single element slice
Replace with next(a for a in v["files"] if a.endswith(".json"))
(RUF015)
182-182: Do not catch blind exception: Exception
(BLE001)
187-187: Probable use of requests call without timeout
(S113)
207-207: Do not catch blind exception: Exception
(BLE001)
246-247: try-except-continue detected, consider logging the exception
(S112)
246-246: Do not catch blind exception: Exception
(BLE001)
265-265: Ambiguous variable name: l
(E741)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
phoonnx/config.py (1)
199-199: Verify phoonnx_version field presence in existing models.The simplified
is_phoonnxcheck was flagged in a previous review comment. Ensure all PhoonNX models have thephoonnx_versionfield, or restore fallback logic to check forphoneme_typeandlang_code.phoonnx/util.py (1)
21-58: Address the static analysis issues flagged in previous review.The
match_langfunction still has the issues identified in the previous review:
- Ambiguous variable name
l(line 40)- Bare except clauses (lines 43, 47, 50)
- Missing exception logging
These issues were already documented in past review comments and remain unaddressed.
phoonnx/model_manager.py (5)
1-15: Add module-level timeout constant.As noted in previous reviews, a module-level
REQUEST_TIMEOUTconstant should be added near the imports to centralize HTTP timeout values. Allrequests.*calls should then use this constant.
50-54: Coerce phoneme_type string to Enum when loading from cache.As flagged in previous reviews, when loaded from cache,
phoneme_typemay be a string rather than aPhonemeTypeenum. Add type coercion to ensure consistent enum usage.
67-88: Add HTTP status checks and UTF-8 encoding.Previous reviews correctly identified improvements needed:
- Add
r.raise_for_status()after each request to propagate HTTP errors- Use explicit
encoding="utf-8"when writing text files- Stream the model download using
requests.get(..., stream=True)andr.iter_content()to avoid loading large models into memory
100-102: Fix phoneme_type override to update config and pass correct parameters.As noted in previous reviews:
- Update
voice.config.phoneme_typeinstead ofvoice.phoneme_type- Pass
voice.config.alphabetandvoice.config.phonemizer_modeltoget_phonemizer()
169-185: Add timeout, improve error handling, and use next() for file lookups.Several issues remain from previous reviews:
- Line 172: Missing
timeoutparameter inrequests.get()- Lines 179-180: Use
next(a for a in v["files"] if a.endswith(".onnx"), None)instead of single-element slice to avoid IndexError- Line 183: Bare
except Exceptionshould log the error withLOG.error()instead ofprint()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
phoonnx/config.py(9 hunks)phoonnx/model_manager.py(1 hunks)phoonnx/phoneme_ids.py(1 hunks)phoonnx/util.py(5 hunks)phoonnx/voice.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
phoonnx/config.py (1)
phoonnx/model_manager.py (2)
engine(60-61)alphabet(56-57)
phoonnx/model_manager.py (3)
phoonnx/config.py (6)
PhonemeType(44-78)get_phonemizer(379-454)VoiceConfig(82-345)Engine(15-21)Alphabet(24-40)from_dict(202-345)phoonnx/util.py (1)
match_lang(21-58)phoonnx/voice.py (2)
TTSVoice(100-372)load(120-166)
phoonnx/util.py (1)
phoonnx/phonemizers/base.py (1)
match_lang(86-103)
🪛 Ruff (0.14.0)
phoonnx/model_manager.py
172-172: Probable use of requests call without timeout
(S113)
179-179: Prefer next(a for a in v["files"] if a.endswith(".onnx")) over single element slice
Replace with next(a for a in v["files"] if a.endswith(".onnx"))
(RUF015)
180-180: Prefer next(a for a in v["files"] if a.endswith(".json")) over single element slice
Replace with next(a for a in v["files"] if a.endswith(".json"))
(RUF015)
183-183: Do not catch blind exception: Exception
(BLE001)
208-208: Do not catch blind exception: Exception
(BLE001)
247-248: try-except-continue detected, consider logging the exception
(S112)
247-247: Do not catch blind exception: Exception
(BLE001)
266-266: Ambiguous variable name: l
(E741)
288-288: Comment contains ambiguous ǀ (LATIN LETTER DENTAL CLICK). Did you mean I (LATIN CAPITAL LETTER I)?
(RUF003)
288-288: Comment contains ambiguous ǃ (LATIN LETTER RETROFLEX CLICK). Did you mean ! (EXCLAMATION MARK)?
(RUF003)
288-288: Comment contains ambiguous ɑ (LATIN SMALL LETTER ALPHA). Did you mean a (LATIN SMALL LETTER A)?
(RUF003)
288-288: Comment contains ambiguous ɡ (LATIN SMALL LETTER SCRIPT G). Did you mean g (LATIN SMALL LETTER G)?
(RUF003)
288-288: Comment contains ambiguous ɣ (LATIN SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
288-288: Comment contains ambiguous ɪ (LATIN LETTER SMALL CAPITAL I). Did you mean i (LATIN SMALL LETTER I)?
(RUF003)
288-288: Comment contains ambiguous ɯ (LATIN SMALL LETTER TURNED M). Did you mean w (LATIN SMALL LETTER W)?
(RUF003)
288-288: Comment contains ambiguous ʋ (LATIN SMALL LETTER V WITH HOOK). Did you mean u (LATIN SMALL LETTER U)?
(RUF003)
288-288: Comment contains ambiguous ʏ (LATIN LETTER SMALL CAPITAL Y). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
288-288: Comment contains ambiguous ʔ (LATIN LETTER GLOTTAL STOP). Did you mean ? (QUESTION MARK)?
(RUF003)
288-288: Comment contains ambiguous ˈ (MODIFIER LETTER VERTICAL LINE). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
288-288: Comment contains ambiguous ː (MODIFIER LETTER TRIANGULAR COLON). Did you mean : (COLON)?
(RUF003)
289-289: Comment contains ambiguous ǀ (LATIN LETTER DENTAL CLICK). Did you mean I (LATIN CAPITAL LETTER I)?
(RUF003)
289-289: Comment contains ambiguous ǃ (LATIN LETTER RETROFLEX CLICK). Did you mean ! (EXCLAMATION MARK)?
(RUF003)
289-289: Comment contains ambiguous ɑ (LATIN SMALL LETTER ALPHA). Did you mean a (LATIN SMALL LETTER A)?
(RUF003)
289-289: Comment contains ambiguous ɡ (LATIN SMALL LETTER SCRIPT G). Did you mean g (LATIN SMALL LETTER G)?
(RUF003)
289-289: Comment contains ambiguous ɣ (LATIN SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
289-289: Comment contains ambiguous ɪ (LATIN LETTER SMALL CAPITAL I). Did you mean i (LATIN SMALL LETTER I)?
(RUF003)
289-289: Comment contains ambiguous ɯ (LATIN SMALL LETTER TURNED M). Did you mean w (LATIN SMALL LETTER W)?
(RUF003)
289-289: Comment contains ambiguous ʋ (LATIN SMALL LETTER V WITH HOOK). Did you mean u (LATIN SMALL LETTER U)?
(RUF003)
289-289: Comment contains ambiguous ʏ (LATIN LETTER SMALL CAPITAL Y). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
289-289: Comment contains ambiguous ʔ (LATIN LETTER GLOTTAL STOP). Did you mean ? (QUESTION MARK)?
(RUF003)
289-289: Comment contains ambiguous ˈ (MODIFIER LETTER VERTICAL LINE). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
289-289: Comment contains ambiguous ː (MODIFIER LETTER TRIANGULAR COLON). Did you mean : (COLON)?
(RUF003)
290-290: Comment contains ambiguous ǀ (LATIN LETTER DENTAL CLICK). Did you mean I (LATIN CAPITAL LETTER I)?
(RUF003)
290-290: Comment contains ambiguous ǃ (LATIN LETTER RETROFLEX CLICK). Did you mean ! (EXCLAMATION MARK)?
(RUF003)
290-290: Comment contains ambiguous ɑ (LATIN SMALL LETTER ALPHA). Did you mean a (LATIN SMALL LETTER A)?
(RUF003)
290-290: Comment contains ambiguous ɡ (LATIN SMALL LETTER SCRIPT G). Did you mean g (LATIN SMALL LETTER G)?
(RUF003)
290-290: Comment contains ambiguous ɣ (LATIN SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
290-290: Comment contains ambiguous ɪ (LATIN LETTER SMALL CAPITAL I). Did you mean i (LATIN SMALL LETTER I)?
(RUF003)
290-290: Comment contains ambiguous ɯ (LATIN SMALL LETTER TURNED M). Did you mean w (LATIN SMALL LETTER W)?
(RUF003)
290-290: Comment contains ambiguous ʋ (LATIN SMALL LETTER V WITH HOOK). Did you mean u (LATIN SMALL LETTER U)?
(RUF003)
290-290: Comment contains ambiguous ʏ (LATIN LETTER SMALL CAPITAL Y). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
290-290: Comment contains ambiguous ʔ (LATIN LETTER GLOTTAL STOP). Did you mean ? (QUESTION MARK)?
(RUF003)
290-290: Comment contains ambiguous ˈ (MODIFIER LETTER VERTICAL LINE). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
290-290: Comment contains ambiguous ː (MODIFIER LETTER TRIANGULAR COLON). Did you mean : (COLON)?
(RUF003)
291-291: Comment contains ambiguous ǀ (LATIN LETTER DENTAL CLICK). Did you mean I (LATIN CAPITAL LETTER I)?
(RUF003)
291-291: Comment contains ambiguous ǃ (LATIN LETTER RETROFLEX CLICK). Did you mean ! (EXCLAMATION MARK)?
(RUF003)
291-291: Comment contains ambiguous ɑ (LATIN SMALL LETTER ALPHA). Did you mean a (LATIN SMALL LETTER A)?
(RUF003)
291-291: Comment contains ambiguous ɡ (LATIN SMALL LETTER SCRIPT G). Did you mean g (LATIN SMALL LETTER G)?
(RUF003)
291-291: Comment contains ambiguous ɣ (LATIN SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
291-291: Comment contains ambiguous ɪ (LATIN LETTER SMALL CAPITAL I). Did you mean i (LATIN SMALL LETTER I)?
(RUF003)
291-291: Comment contains ambiguous ɯ (LATIN SMALL LETTER TURNED M). Did you mean w (LATIN SMALL LETTER W)?
(RUF003)
291-291: Comment contains ambiguous ʋ (LATIN SMALL LETTER V WITH HOOK). Did you mean u (LATIN SMALL LETTER U)?
(RUF003)
291-291: Comment contains ambiguous ʏ (LATIN LETTER SMALL CAPITAL Y). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
291-291: Comment contains ambiguous ʔ (LATIN LETTER GLOTTAL STOP). Did you mean ? (QUESTION MARK)?
(RUF003)
291-291: Comment contains ambiguous ˈ (MODIFIER LETTER VERTICAL LINE). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
291-291: Comment contains ambiguous ː (MODIFIER LETTER TRIANGULAR COLON). Did you mean : (COLON)?
(RUF003)
292-292: Comment contains ambiguous ǀ (LATIN LETTER DENTAL CLICK). Did you mean I (LATIN CAPITAL LETTER I)?
(RUF003)
292-292: Comment contains ambiguous ǃ (LATIN LETTER RETROFLEX CLICK). Did you mean ! (EXCLAMATION MARK)?
(RUF003)
292-292: Comment contains ambiguous ɑ (LATIN SMALL LETTER ALPHA). Did you mean a (LATIN SMALL LETTER A)?
(RUF003)
292-292: Comment contains ambiguous ɡ (LATIN SMALL LETTER SCRIPT G). Did you mean g (LATIN SMALL LETTER G)?
(RUF003)
292-292: Comment contains ambiguous ɣ (LATIN SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
292-292: Comment contains ambiguous ɪ (LATIN LETTER SMALL CAPITAL I). Did you mean i (LATIN SMALL LETTER I)?
(RUF003)
292-292: Comment contains ambiguous ɯ (LATIN SMALL LETTER TURNED M). Did you mean w (LATIN SMALL LETTER W)?
(RUF003)
292-292: Comment contains ambiguous ʋ (LATIN SMALL LETTER V WITH HOOK). Did you mean u (LATIN SMALL LETTER U)?
(RUF003)
292-292: Comment contains ambiguous ʏ (LATIN LETTER SMALL CAPITAL Y). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
292-292: Comment contains ambiguous ʔ (LATIN LETTER GLOTTAL STOP). Did you mean ? (QUESTION MARK)?
(RUF003)
292-292: Comment contains ambiguous ˈ (MODIFIER LETTER VERTICAL LINE). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
292-292: Comment contains ambiguous ː (MODIFIER LETTER TRIANGULAR COLON). Did you mean : (COLON)?
(RUF003)
293-293: Comment contains ambiguous ǀ (LATIN LETTER DENTAL CLICK). Did you mean I (LATIN CAPITAL LETTER I)?
(RUF003)
293-293: Comment contains ambiguous ǃ (LATIN LETTER RETROFLEX CLICK). Did you mean ! (EXCLAMATION MARK)?
(RUF003)
293-293: Comment contains ambiguous ɑ (LATIN SMALL LETTER ALPHA). Did you mean a (LATIN SMALL LETTER A)?
(RUF003)
293-293: Comment contains ambiguous ɡ (LATIN SMALL LETTER SCRIPT G). Did you mean g (LATIN SMALL LETTER G)?
(RUF003)
293-293: Comment contains ambiguous ɣ (LATIN SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
293-293: Comment contains ambiguous ɪ (LATIN LETTER SMALL CAPITAL I). Did you mean i (LATIN SMALL LETTER I)?
(RUF003)
293-293: Comment contains ambiguous ɯ (LATIN SMALL LETTER TURNED M). Did you mean w (LATIN SMALL LETTER W)?
(RUF003)
293-293: Comment contains ambiguous ʋ (LATIN SMALL LETTER V WITH HOOK). Did you mean u (LATIN SMALL LETTER U)?
(RUF003)
293-293: Comment contains ambiguous ʏ (LATIN LETTER SMALL CAPITAL Y). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
293-293: Comment contains ambiguous ʔ (LATIN LETTER GLOTTAL STOP). Did you mean ? (QUESTION MARK)?
(RUF003)
293-293: Comment contains ambiguous ˈ (MODIFIER LETTER VERTICAL LINE). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
293-293: Comment contains ambiguous ː (MODIFIER LETTER TRIANGULAR COLON). Did you mean : (COLON)?
(RUF003)
294-294: Comment contains ambiguous ǀ (LATIN LETTER DENTAL CLICK). Did you mean I (LATIN CAPITAL LETTER I)?
(RUF003)
294-294: Comment contains ambiguous ǃ (LATIN LETTER RETROFLEX CLICK). Did you mean ! (EXCLAMATION MARK)?
(RUF003)
294-294: Comment contains ambiguous ɑ (LATIN SMALL LETTER ALPHA). Did you mean a (LATIN SMALL LETTER A)?
(RUF003)
294-294: Comment contains ambiguous ɡ (LATIN SMALL LETTER SCRIPT G). Did you mean g (LATIN SMALL LETTER G)?
(RUF003)
294-294: Comment contains ambiguous ɣ (LATIN SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
294-294: Comment contains ambiguous ɪ (LATIN LETTER SMALL CAPITAL I). Did you mean i (LATIN SMALL LETTER I)?
(RUF003)
294-294: Comment contains ambiguous ɯ (LATIN SMALL LETTER TURNED M). Did you mean w (LATIN SMALL LETTER W)?
(RUF003)
294-294: Comment contains ambiguous ʋ (LATIN SMALL LETTER V WITH HOOK). Did you mean u (LATIN SMALL LETTER U)?
(RUF003)
294-294: Comment contains ambiguous ʏ (LATIN LETTER SMALL CAPITAL Y). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
294-294: Comment contains ambiguous ʔ (LATIN LETTER GLOTTAL STOP). Did you mean ? (QUESTION MARK)?
(RUF003)
294-294: Comment contains ambiguous ˈ (MODIFIER LETTER VERTICAL LINE). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
294-294: Comment contains ambiguous ː (MODIFIER LETTER TRIANGULAR COLON). Did you mean : (COLON)?
(RUF003)
295-295: Comment contains ambiguous ǀ (LATIN LETTER DENTAL CLICK). Did you mean I (LATIN CAPITAL LETTER I)?
(RUF003)
295-295: Comment contains ambiguous ǃ (LATIN LETTER RETROFLEX CLICK). Did you mean ! (EXCLAMATION MARK)?
(RUF003)
295-295: Comment contains ambiguous ɑ (LATIN SMALL LETTER ALPHA). Did you mean a (LATIN SMALL LETTER A)?
(RUF003)
295-295: Comment contains ambiguous ɡ (LATIN SMALL LETTER SCRIPT G). Did you mean g (LATIN SMALL LETTER G)?
(RUF003)
295-295: Comment contains ambiguous ɣ (LATIN SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
295-295: Comment contains ambiguous ɪ (LATIN LETTER SMALL CAPITAL I). Did you mean i (LATIN SMALL LETTER I)?
(RUF003)
295-295: Comment contains ambiguous ɯ (LATIN SMALL LETTER TURNED M). Did you mean w (LATIN SMALL LETTER W)?
(RUF003)
295-295: Comment contains ambiguous ʋ (LATIN SMALL LETTER V WITH HOOK). Did you mean u (LATIN SMALL LETTER U)?
(RUF003)
295-295: Comment contains ambiguous ʏ (LATIN LETTER SMALL CAPITAL Y). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
295-295: Comment contains ambiguous ʔ (LATIN LETTER GLOTTAL STOP). Did you mean ? (QUESTION MARK)?
(RUF003)
295-295: Comment contains ambiguous ˈ (MODIFIER LETTER VERTICAL LINE). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
295-295: Comment contains ambiguous ː (MODIFIER LETTER TRIANGULAR COLON). Did you mean : (COLON)?
(RUF003)
296-296: Comment contains ambiguous ǀ (LATIN LETTER DENTAL CLICK). Did you mean I (LATIN CAPITAL LETTER I)?
(RUF003)
296-296: Comment contains ambiguous ǃ (LATIN LETTER RETROFLEX CLICK). Did you mean ! (EXCLAMATION MARK)?
(RUF003)
296-296: Comment contains ambiguous ɑ (LATIN SMALL LETTER ALPHA). Did you mean a (LATIN SMALL LETTER A)?
(RUF003)
296-296: Comment contains ambiguous ɡ (LATIN SMALL LETTER SCRIPT G). Did you mean g (LATIN SMALL LETTER G)?
(RUF003)
296-296: Comment contains ambiguous ɣ (LATIN SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
296-296: Comment contains ambiguous ɪ (LATIN LETTER SMALL CAPITAL I). Did you mean i (LATIN SMALL LETTER I)?
(RUF003)
296-296: Comment contains ambiguous ɯ (LATIN SMALL LETTER TURNED M). Did you mean w (LATIN SMALL LETTER W)?
(RUF003)
296-296: Comment contains ambiguous ʋ (LATIN SMALL LETTER V WITH HOOK). Did you mean u (LATIN SMALL LETTER U)?
(RUF003)
296-296: Comment contains ambiguous ʏ (LATIN LETTER SMALL CAPITAL Y). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
296-296: Comment contains ambiguous ʔ (LATIN LETTER GLOTTAL STOP). Did you mean ? (QUESTION MARK)?
(RUF003)
296-296: Comment contains ambiguous ˈ (MODIFIER LETTER VERTICAL LINE). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
296-296: Comment contains ambiguous ː (MODIFIER LETTER TRIANGULAR COLON). Did you mean : (COLON)?
(RUF003)
297-297: Comment contains ambiguous ǀ (LATIN LETTER DENTAL CLICK). Did you mean I (LATIN CAPITAL LETTER I)?
(RUF003)
297-297: Comment contains ambiguous ǃ (LATIN LETTER RETROFLEX CLICK). Did you mean ! (EXCLAMATION MARK)?
(RUF003)
297-297: Comment contains ambiguous ɑ (LATIN SMALL LETTER ALPHA). Did you mean a (LATIN SMALL LETTER A)?
(RUF003)
297-297: Comment contains ambiguous ɡ (LATIN SMALL LETTER SCRIPT G). Did you mean g (LATIN SMALL LETTER G)?
(RUF003)
297-297: Comment contains ambiguous ɣ (LATIN SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
297-297: Comment contains ambiguous ɪ (LATIN LETTER SMALL CAPITAL I). Did you mean i (LATIN SMALL LETTER I)?
(RUF003)
297-297: Comment contains ambiguous ɯ (LATIN SMALL LETTER TURNED M). Did you mean w (LATIN SMALL LETTER W)?
(RUF003)
297-297: Comment contains ambiguous ʋ (LATIN SMALL LETTER V WITH HOOK). Did you mean u (LATIN SMALL LETTER U)?
(RUF003)
297-297: Comment contains ambiguous ʏ (LATIN LETTER SMALL CAPITAL Y). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
297-297: Comment contains ambiguous ʔ (LATIN LETTER GLOTTAL STOP). Did you mean ? (QUESTION MARK)?
(RUF003)
297-297: Comment contains ambiguous ˈ (MODIFIER LETTER VERTICAL LINE). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
297-297: Comment contains ambiguous ː (MODIFIER LETTER TRIANGULAR COLON). Did you mean : (COLON)?
(RUF003)
298-298: Comment contains ambiguous ǀ (LATIN LETTER DENTAL CLICK). Did you mean I (LATIN CAPITAL LETTER I)?
(RUF003)
298-298: Comment contains ambiguous ǃ (LATIN LETTER RETROFLEX CLICK). Did you mean ! (EXCLAMATION MARK)?
(RUF003)
298-298: Comment contains ambiguous ɑ (LATIN SMALL LETTER ALPHA). Did you mean a (LATIN SMALL LETTER A)?
(RUF003)
298-298: Comment contains ambiguous ɡ (LATIN SMALL LETTER SCRIPT G). Did you mean g (LATIN SMALL LETTER G)?
(RUF003)
298-298: Comment contains ambiguous ɣ (LATIN SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
298-298: Comment contains ambiguous ɪ (LATIN LETTER SMALL CAPITAL I). Did you mean i (LATIN SMALL LETTER I)?
(RUF003)
298-298: Comment contains ambiguous ɯ (LATIN SMALL LETTER TURNED M). Did you mean w (LATIN SMALL LETTER W)?
(RUF003)
298-298: Comment contains ambiguous ʋ (LATIN SMALL LETTER V WITH HOOK). Did you mean u (LATIN SMALL LETTER U)?
(RUF003)
298-298: Comment contains ambiguous ʏ (LATIN LETTER SMALL CAPITAL Y). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
298-298: Comment contains ambiguous ʔ (LATIN LETTER GLOTTAL STOP). Did you mean ? (QUESTION MARK)?
(RUF003)
298-298: Comment contains ambiguous ˈ (MODIFIER LETTER VERTICAL LINE). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
298-298: Comment contains ambiguous ː (MODIFIER LETTER TRIANGULAR COLON). Did you mean : (COLON)?
(RUF003)
phoonnx/util.py
40-40: Ambiguous variable name: l
(E741)
43-43: Do not use bare except
(E722)
45-45: Ambiguous variable name: l
(E741)
47-47: Do not use bare except
(E722)
50-50: Do not use bare except
(E722)
50-51: try-except-continue detected, consider logging the exception
(S112)
🔇 Additional comments (16)
phoonnx/phoneme_ids.py (1)
6-6: LGTM! Centralized logging import.The migration from dynamic logging import to the centralized
phoonnx.util.LOGimproves consistency and maintainability across the codebase.phoonnx/util.py (1)
13-19: LGTM! Resilient logger initialization.The fallback pattern for LOG initialization is appropriate and follows best practices for handling optional dependencies.
phoonnx/voice.py (1)
17-20: LGTM! Consistent logging migration.The centralized logging import aligns with the refactoring across the codebase.
phoonnx/config.py (4)
5-5: LGTM! Centralized logging import.
15-21: LGTM! Engine enum is well-defined.The Engine enum provides clear framework identification and uses appropriate string enum pattern for serialization.
117-118: LGTM! Engine field addition.The engine field with PHOONNX default is appropriate and well-typed.
212-331: LGTM! Engine assignment logic is correct.The
from_dictmethod correctly detects and assigns the engine type based on model characteristics and passes it to the VoiceConfig constructor.phoonnx/model_manager.py (9)
17-27: LGTM! Well-structured dataclass.The
TTSModelInfodataclass is cleanly defined with appropriate type hints and optional fields.
55-65: LGTM! Property methods are clean.The property methods correctly expose config attributes and compute the cache path.
106-133: LGTM! Manager initialization and cache methods are clean.The
TTSModelManagerinitialization and cache management methods are well-implemented.
135-151: LGTM! Voice management methods are correct.The
add_voiceandget_lang_voicesmethods correctly manage voices and usematch_langfor language matching.
154-168: LGTM! Hardcoded voice list is clean.The
get_proxectonos_voice_listmethod cleanly adds hardcoded voice entries.
211-249: LGTM! OVOS voice list with proper error handling.The
get_ovos_voice_listmethod correctly handles hardcoded voice entries with appropriate error suppression for missing voices.
250-259: LGTM! Phonikud voice entry is clean.
260-267: LGTM! Properties are clean.The
all_voicesandsupported_langsproperties provide appropriate read-only access to aggregated data.
269-306: Demonstration code in main is appropriate.The
__main__section demonstrates the manager's usage effectively. The static analysis warnings about ambiguous Unicode characters in comments are expected—they represent IPA phoneme symbols in the example output.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
Summary by CodeRabbit
New Features
Improvements
Behavior Change