# TTS Engine Implementation - Fails & Lessons Learned

## Step Audio EditX Implementation

### Import Errors
- **Missing import**: Always add `import folder_paths` when using `folder_paths.get_temp_directory()` in node files
- **Bundled code imports**: For complex bundled packages with internal cross-imports, add `sys.path.insert(0, impl_dir)` at top of main files instead of converting all imports

### Audio Utility Functions
- **Temp file creation**: Use `AudioProcessingUtils.save_audio_to_temp_file()` not `save_audio()` (doesn't exist)
- **ComfyUI audio format**: Extract waveform with `audio_tensor['waveform']` and `audio_tensor.get('sample_rate')` before passing to utilities

### Download Configuration
- **HuggingFace file list**: Always verify actual repo structure with `curl` before hardcoding file lists
- **Downloader API**: Use dict format `[{"remote": f, "local": f}]` for unified_downloader, not plain strings
- **File existence**: Check actual HuggingFace repo structure - don't assume file naming patterns (e.g., `model-00001.safetensors` not `model-00001-of-00002.safetensors`)
- **CRITICAL - No cache downloads**: NEVER allow models to auto-download to cache directories. ALL models/weights must be downloaded via our downloader to organized `models/TTS/` folders. Disable auto-download and add to downloader instead.

### Factory Registration
- **Unified model interface**: Always implement `register_<engine>_factory()` when using `unified_model_interface.load_model()`
- **Factory initialization**: Add factory to `initialize_all_factories()` or it won't be registered

### Node Registration
- **Engine branch**: Add engine-specific branch in `_create_engine_node_instance()` for each new engine
- **Variable naming**: Use consistent variable names (`config` not `engine_config`) throughout node code

### Voice References
- **Reference text requirement**: F5-TTS and Step Audio EditX REQUIRE `prompt_text` (transcript), ChatterBox/VibeVoice/Higgs don't
- **Narrator voice mapping**: Map narrator from TTS Text node input by saving `audio_tensor` to temp file with `reference_text`
- **Voice discovery**: Use `get_character_mapping()` not `discover_voices_for_engine()` (latter doesn't exist)

### Pause Tag Format
- **Correct format**: Use `[pause:2]`, `[pause:1.5s]`, `[pause:500ms]` - NOT `<pause_X>`
- **Parser usage**: Use `PauseTagProcessor.parse_pause_tags()` which returns `('text', content)` or `('pause', duration_seconds)` tuples

### Seed Control
- **Global torch state**: Some engines (Step Audio EditX) use global `torch.manual_seed()` for reproducibility, not function parameters
- **CUDA seeds**: Always set both `torch.manual_seed()` and `torch.cuda.manual_seed_all()` for GPU reproducibility

### Quantization Support
- **Device movement**: Bitsandbytes quantized models (int4/int8) CANNOT be moved with `.to()` - wrap in try-except
- **VRAM clearing**: Quantized models stay on GPU; "Clear VRAM" must unload completely (not move to CPU)
- **Error handling**: Catch `ValueError` with "is not supported for" and "8-bit"/"4-bit" in message

### Character Switching Implementation
- **Missing voice fallback**: When `get_character_mapping()` returns `(None, None)` for a character, MUST still add entry to `voice_mapping` with fallback to narrator/default voice
- **Voice mapping consistency**: All characters in parsed segments MUST have entries in `voice_mapping`, even if empty/None
- **Adapter validation**: Adapters that require voice references (Step Audio EditX, F5-TTS) need graceful fallback when voice_ref is None
- **Character parser interaction**: Character parser changes unknown characters to "narrator" - need to ensure original character names are preserved for voice mapping lookup
- **Parser fix**: Add text tag characters to `available_characters` (lowercase) + set language defaults like IndexTTS
- **Working pattern**: `all_available = set(get_available_characters())` + aliases + text tag chars (lowercase) + "narrator" → `set_available_characters(list(all_available))` + `set_character_language_default()` for each char

### Caching Implementation
- **CRITICAL - Planned but skipped**: Caching listed in Phase 1 but often omitted during processor/adapter implementation. Verify with testing (run generation twice on same text - should show cache hit on second)
- **Required in adapters**: Generate stable audio hash, create cache key with all params, check cache BEFORE generation, store after with duration calc
- **Cache key pattern**: `audio_cache.generate_cache_key(engine_type, text=..., audio_component=..., **all_params)`
- **Duration calculation**: Update `_calculate_duration()` with engine sample rate (e.g., 24000 for Step Audio EditX, F5-TTS)

### Model Lifecycle - __del__ Destructor
- **CRITICAL**: Remove `__del__` from engine classes - causes automatic unload after generation ends (when object goes out of scope)
- **Pattern**: F5-TTS and ChatterBox don't have `__del__`, only IndexTTS and StepAudio did (wrong)
- **Correct behavior**: Models stay in VRAM for reuse; only unload on explicit Clear VRAM button, engine switch, or parameter change

### Generation Info Reports - Modular Pattern
- **CRITICAL - Consistency required**: ALL engines must use `ChunkCombiner` + `ChunkTimingHelper` for generation reports
- **Correct pattern** (from ChatterBox 23-Lang, F5-TTS):
  1. `combine_audio_segments()` method accepts `return_info=True` parameter
  2. Uses `ChunkCombiner.combine_chunks()` with `return_info=True` to get `(audio, chunk_info)` tuple
  3. In unified node: call `ChunkTimingHelper.enhance_generation_info(base_info, chunk_info)` for detailed timing
- **Wrong**: Inline concatenation (`torch.cat`) without timing tracking, manual report formatting
- **Example**: IndexTTS/StepAudio initially did inline concat - needed refactor to use ChunkCombiner
- **Required info**: Duration, text length, segments/chunks count, chunk timing breakdown (start/end/text per chunk)

### Segment Parameter Registration
- **Forgot to add**: New engines to `PARAMETER_ENGINES` in `utils/text/segment_parameters.py` for `[seed:X]` tag support

### SRT Processor Import Pattern
- **Wrong**: Using `from nodes.xxx import` - fails with `'nodes' is not a package`
- **Correct**: Use `importlib.util.spec_from_file_location()` for cross-module imports in dynamically loaded processors

### ComfyUI Audio Dict Format
- **Wrong**: Passing `audio_tensor` directly to functions expecting raw tensor
- **Correct**: Extract waveform with `audio_tensor.get('waveform')` - ComfyUI audio is `{"waveform": tensor, "sample_rate": int}`
- **Also**: Check `AudioProcessingUtils.save_audio_to_temp_file(audio, sample_rate)` signature - it returns path, doesn't take path as arg

### ComfyUI Audio Tensor Shape
- **CRITICAL**: ComfyUI expects **3D** `[batch, channels, samples]` NOT 2D `[channels, samples]`
- **Error symptom**: `IndexError: Dimension out of range (expected to be in range of [-1, 0], but got 1)` in `save_audio()`
- **Correct shape conversion**:
  - 1D `[samples]` → 3D `[1, 1, samples]` (two unsqueeze)
  - 2D `[channels, samples]` → 3D `[1, channels, samples]` (one unsqueeze)
- **Wrong**: Squeezing 3D to 2D, or only adding one dimension to 1D
- **Pattern**: `tensor.unsqueeze(0)` adds batch dim to 2D; check VibeVoice processor for reference

### Step Audio EditX Sample Rate
- **Symptom**: Pitch shift in edit/clone modes with external audio (44100 Hz, 48000 Hz)
- **Cause**: Saving audio with wrong sample rate metadata → `torchaudio.load()` misinterprets data
- **Fix**: Resample to 24000 Hz before saving temp files (edit node, TTS text node, TTS SRT node)
- **Prevention**: Always resample to engine's native rate (24000 Hz for Step Audio EditX)

### Unified Model Interface - Adapter & Node Consistency
- **CRITICAL**: ALL engine loading must use `unified_model_interface.load_model()` for smart caching
- **Symptom**: Model loads twice (once for TTS, once for editing/other nodes) - wasting VRAM and time
- **Wrong pattern**: Creating engine instances directly (`engine = StepAudioEditXEngine()`) in adapters or nodes
- **Correct pattern**:
  1. **Adapter** `load_base_model()` uses `unified_model_interface.load_model(config)`
  2. **Audio Editor node** uses `unified_model_interface.load_model(config)`
  3. **Both use same `model_type`** (e.g., "tts") so cache keys match → engine reused
- **Cache key components**: `engine_name`, `model_type`, `model_name`, `device`, `quantization`, etc.
- **Fix Step Audio EditX**: Updated adapter and Audio Editor node to use unified interface with `model_type="tts"`
- **Prevention**: When adding new engines, ALWAYS use unified interface in ALL code paths (adapters, nodes, processors)

### Engine Wrapper Implementation - Always Compare First
- **CRITICAL**: Before writing engine wrapper, open existing reference engine (Step Audio EditX, F5-TTS) side-by-side and compare patterns
- **Common mistakes when NOT comparing**:
  - Missing required imports (`folder_paths`, `resolve_torch_device`, `unified_model_interface`, `extra_paths`)
  - Manual device/dtype resolution instead of using utils functions
  - Missing `_find_model_directory()` with `extra_model_paths` support
  - Using `load_model()` method instead of `_ensure_model_loaded()` pattern
  - Direct model instantiation instead of `unified_model_interface.load_model()`
  - Missing `_model_config` tracking (needed for unload)
  - Not using `tts_model_manager.ensure_device()` before fallback to `.to()`
  - Missing GPU capability detection for dtype auto-selection
- **Prevention**: ALWAYS open Step Audio EditX engine file as reference when creating new engine wrapper, copy the patterns