lazy load voice info config

[JarbasAl]

Summary by CodeRabbit

• New Features

  • Voice config loader now accepts an explicit engine override when constructing voice configs.

• Refactor

  • Configuration is now loaded lazily for improved initialization performance and memory usage; public behavior remains consistent.

[coderabbitai]

📝 Walkthrough

Walkthrough

TTSModelInfo now replaces its public config dataclass field with a lazy-loaded config property backed by a private _config. VoiceConfig is constructed on first access from stored URLs/asset fields; enum/string normalization occurs in initialization and on first config resolution. VoiceConfig.from_dict accepts an optional engine override.

Changes

Cohort / File(s)	Summary
Model manager (lazy config) phoonnx/model_manager.py	Removed public config: Optional[VoiceConfig] attribute; added private _config and a config property that lazily loads/constructs a VoiceConfig from config_url, vocab/vocab_url, tokenizer_config_url, tokens_url, and local overrides. __post_init__ now normalizes enum/string fields (engine, alphabet, phoneme_type) and prepares internal state. Model-loading helpers preserved but use _config.
Config factory (engine override) phoonnx/config.py	Extended VoiceConfig.from_dict(...) signature to accept optional engine: Optional[Union[str, Engine]]. Engine selection in detection branches now uses the provided engine when present, otherwise falls back to previous branch-specific defaults; string engines are normalized to Engine values before return.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat: model manager #49 — Modifies TTSModelInfo and VoiceConfig.from_dict handling; directly related to lazy config and engine handling changes.
• fix: ovos plugin config loading #57 — Touches VoiceConfig.from_dict signature/behavior; may conflict or overlap with engine/default logic.
• fix: phoonnx config loading #55 — Alters VoiceConfig loading logic and defaults; relevant to model-detection and default engine/phoneme behavior.

Suggested labels

feature

Poem

🐇 I nibble bytes beneath the log,
Quietly shift the eager to fog—
Configs now sleep till first they're sought,
A lazy hop saves loading's trot. ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'lazy load voice info config' accurately describes the main change: converting eager config loading to lazy loading in the TTSModelInfo class through a property-based approach.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into dev

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fixes

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Note

Docstrings generation - SUCCESS
Generated docstrings for this pull request at #109

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

phoonnx/model_manager.py (1)

29-78: ⚠️ Potential issue | 🟡 Minor

The config property is no longer part of the dataclass __repr__.

Since config changed from a dataclass field to a property, it will no longer appear in repr(TTSModelInfo(...)). This is likely the desired behavior (avoids triggering lazy-load on repr), but the example output in the comments at the bottom of the file (lines ~396-405) is now stale and includes config=VoiceConfig(...).

🤖 Fix all issues with AI agents

In `@phoonnx/model_manager.py`:
- Around line 42-58: The conditional for handling tokens_url currently uses a
separate if that can overwrite a previously built self._config from the
vocab_override or vocab_url branches; change that if to an elif so
download_tokens_txt() and the tokens-based VoiceConfig.from_dict(...) only run
when earlier branches did not set self._config, and ensure phoneme_type is
applied to the final config object (self._config) after construction; locate the
logic around self.vocab_override, self.vocab_url, download_vocab(),
download_tokenizer_config(), self.tokens_url, download_tokens_txt(), and
VoiceConfig.from_dict to implement this change.

🧹 Nitpick comments (2)

phoonnx/model_manager.py (2)

80-87: Lazy-load init looks good; os.makedirs still runs eagerly for every voice.

The _config backing field initialization is correct. Note that os.makedirs (line 87) still runs eagerly during construction — so instantiating all ~1400+ TTSModelInfo entries (e.g., via merge_default_voices) creates that many cache directories even if config is never accessed. This pre-dates this PR, but if the goal is to defer I/O, you might consider making this lazy too.

---

29-32: Prefer if self._config is None: over if not self._config:.

Using truthiness to guard lazy initialization is fragile — if VoiceConfig ever defines __bool__ or __len__, this could cause the property to re-compute on every access. The idiomatic pattern for lazy init is an identity check against None.

♻️ Proposed fix

     `@property`
     def config(self) -> VoiceConfig:
         # lazy loaded
-        if not self._config:
+        if self._config is None:

[coderabbitai]

🧹 Nitpick comments (1)

phoonnx/model_manager.py (1)

99-114: Eager makedirs for every voice contradicts the lazy-loading intent.

os.makedirs(self.voice_path, exist_ok=True) on line 106 runs during __post_init__ for every TTSModelInfo instance. With 1400+ registered voices (per merge_default_voices), this issues 1400+ makedirs syscalls at startup, even for voices that are never loaded.

Move directory creation into the config property (or into each download_* method) so it only runs when a voice is actually accessed.

♻️ Proposed fix — defer makedirs to first use

     def __post_init__(self):
         self._config: Optional[VoiceConfig] = None
-        os.makedirs(self.voice_path, exist_ok=True)
 
         # cast strings to enum for consistency
         if not isinstance(self.engine, Engine) and isinstance(self.engine, str):

Then, at the start of the config property (or in a shared helper called by each download method):

     `@property`
     def config(self) -> VoiceConfig:
         if not self._config:
+            os.makedirs(self.voice_path, exist_ok=True)
             if self.config_url: