From 713abdfd73460c23714d9c630c1de1663a8670a5 Mon Sep 17 00:00:00 2001 From: Artem Akymenko Date: Sun, 26 Jul 2026 09:04:01 +0000 Subject: [PATCH] refactor: voice resolution on ConversionRequest, PipelinePool without job param - Added speakers field to ConversionRequest - Rewrote collect_required_voice_ids(), initialize_voice_cache(), job_voice_fallback(), chapter_voice_spec(), chunk_voice_spec() to accept ConversionRequest instead of job - PipelinePool.get() now takes request= instead of job= - Updated all tests to use ConversionRequest interface - 1493 tests passing --- abogen/application/conversion_request.py | 3 + abogen/domain/pipeline_factory.py | 10 +- abogen/domain/voice_resolution.py | 71 ++++++++---- tests/test_domain_pipeline_factory.py | 6 +- tests/test_regression_webui_domain.py | 12 +-- tests/test_voice_resolution.py | 131 +++++++++++++---------- 6 files changed, 141 insertions(+), 92 deletions(-) diff --git a/abogen/application/conversion_request.py b/abogen/application/conversion_request.py index ea5872d..3e4456e 100644 --- a/abogen/application/conversion_request.py +++ b/abogen/application/conversion_request.py @@ -95,6 +95,9 @@ class ConversionRequest: # --- Metadata --- metadata_tags: Dict[str, Any] = field(default_factory=dict) + # --- Voice profiles (loaded by UI, used by app for voice resolution) --- + speakers: Dict[str, Any] = field(default_factory=dict) + # --- Artifacts --- cover_image_path: Optional[Path] = None cover_image_mime: Optional[str] = None diff --git a/abogen/domain/pipeline_factory.py b/abogen/domain/pipeline_factory.py index 4f79936..0f16080 100644 --- a/abogen/domain/pipeline_factory.py +++ b/abogen/domain/pipeline_factory.py @@ -95,7 +95,8 @@ class PipelinePool: language: str, use_gpu: bool, *, - job: Any = None, + request: Any = None, + events: Any = None, ) -> Any: """Get or create a cached pipeline for the given provider. @@ -103,7 +104,8 @@ class PipelinePool: provider: TTS provider name ("kokoro" or "supertonic"). language: Language code (for kokoro). use_gpu: Whether GPU acceleration is requested. - job: Optional job object for voice cache initialization. + request: ConversionRequest for voice cache initialization. + events: ConversionEvents for logging during cache init. """ provider = str(provider or "kokoro").strip().lower() or "kokoro" if not is_plugin_registered(provider): @@ -116,8 +118,8 @@ class PipelinePool: pipeline = create_pipeline_for_job(provider, language, use_gpu) self._pipelines[provider] = pipeline - if provider == "kokoro" and not self._voice_cache_initialized and job is not None: - initialize_voice_cache(job) + if provider == "kokoro" and not self._voice_cache_initialized and request is not None: + initialize_voice_cache(request, events=events) self._voice_cache_initialized = True return pipeline diff --git a/abogen/domain/voice_resolution.py b/abogen/domain/voice_resolution.py index 83bfd1f..db848a9 100644 --- a/abogen/domain/voice_resolution.py +++ b/abogen/domain/voice_resolution.py @@ -2,6 +2,9 @@ Functions for resolving voice specifications, collecting required voice IDs, and determining the voice to use for chapters and chunks. + +All functions accept ConversionRequest (the app-layer contract) instead of +UI-specific objects. This keeps the domain layer UI-agnostic. """ from __future__ import annotations @@ -29,12 +32,28 @@ def spec_to_voice_ids(spec: Any) -> Set[str]: return set() -def job_voice_fallback(job: Any) -> str: - base = str(getattr(job, "voice", "") or "").strip() +def _get_chapter_overrides(request: Any) -> list: + """Extract chapter overrides from ConversionRequest.""" + cc = getattr(request, "chapter_chunk", None) + if cc is not None: + return getattr(cc, "chapter_overrides", []) or [] + return [] + + +def _get_chunks(request: Any) -> list: + """Extract chunks from ConversionRequest.""" + cc = getattr(request, "chapter_chunk", None) + if cc is not None: + return getattr(cc, "chunks", []) or [] + return [] + + +def job_voice_fallback(request: Any) -> str: + base = str(getattr(request, "voice", "") or "").strip() if base and base != "__custom_mix": return base - speakers = getattr(job, "speakers", None) + speakers = getattr(request, "speakers", None) if isinstance(speakers, dict): narrator = speakers.get("narrator") if isinstance(narrator, dict): @@ -52,7 +71,7 @@ def job_voice_fallback(job: Any) -> str: if candidate and candidate != "__custom_mix": return candidate - for chapter in getattr(job, "chapters", []) or []: + for chapter in _get_chapter_overrides(request): if not isinstance(chapter, dict): continue for key in ("resolved_voice", "voice_formula", "voice"): @@ -63,24 +82,24 @@ def job_voice_fallback(job: Any) -> str: return "" -def collect_required_voice_ids(job: Any) -> Set[str]: +def collect_required_voice_ids(request: Any) -> Set[str]: voices: Set[str] = set() - voices.update(spec_to_voice_ids(job.voice)) - voices.update(spec_to_voice_ids(job_voice_fallback(job))) + voices.update(spec_to_voice_ids(request.voice)) + voices.update(spec_to_voice_ids(job_voice_fallback(request))) - for chapter in getattr(job, "chapters", []) or []: + for chapter in _get_chapter_overrides(request): if not isinstance(chapter, dict): continue for key in ("resolved_voice", "voice_formula", "voice"): voices.update(spec_to_voice_ids(chapter.get(key))) - for chunk in getattr(job, "chunks", []) or []: + for chunk in _get_chunks(request): if not isinstance(chunk, dict): continue for key in ("resolved_voice", "voice_formula", "voice"): voices.update(spec_to_voice_ids(chunk.get(key))) - speakers = getattr(job, "speakers", {}) + speakers = getattr(request, "speakers", {}) if isinstance(speakers, dict): for payload in speakers.values() or []: if not isinstance(payload, dict): @@ -92,30 +111,38 @@ def collect_required_voice_ids(job: Any) -> Set[str]: return voices -def initialize_voice_cache(job: Any) -> None: +def initialize_voice_cache(request: Any, events: Any = None) -> None: + """Initialize voice cache by downloading required voice assets. + + Args: + request: ConversionRequest with voice/chapter/chunk/speaker info. + events: ConversionEvents for logging (optional, for backward compat). + """ + log = (lambda msg, level="info": events.log(msg, level=level)) if events else (lambda msg, level="info": None) + try: - targets = collect_required_voice_ids(job) + targets = collect_required_voice_ids(request) downloaded, errors = ensure_voice_assets( targets, - on_progress=lambda message: job.add_log(message, level="debug"), + on_progress=lambda message: log(message, level="debug"), ) except RuntimeError as exc: - job.add_log(f"Voice cache unavailable: {exc}", level="warning") + log(f"Voice cache unavailable: {exc}", level="warning") return if downloaded: - job.add_log( + log( f"Cached {len(downloaded)} voice asset{'s' if len(downloaded) != 1 else ''} locally.", level="info", ) for voice_id, error in errors.items(): - job.add_log(f"Failed to cache voice '{voice_id}': {error}", level="warning") + log(f"Failed to cache voice '{voice_id}': {error}", level="warning") -def chapter_voice_spec(job: Any, override: Optional[Dict[str, Any]]) -> str: +def chapter_voice_spec(request: Any, override: Optional[Dict[str, Any]]) -> str: if not override: - return job_voice_fallback(job) + return job_voice_fallback(request) resolved = str(override.get("resolved_voice", "")).strip() if resolved: @@ -129,17 +156,17 @@ def chapter_voice_spec(job: Any, override: Optional[Dict[str, Any]]) -> str: if voice: return voice - return job_voice_fallback(job) + return job_voice_fallback(request) -def chunk_voice_spec(job: Any, chunk: Dict[str, Any], fallback: str) -> str: +def chunk_voice_spec(request: Any, chunk: Dict[str, Any], fallback: str) -> str: for key in ("resolved_voice", "voice_formula", "voice"): value = chunk.get(key) if value: return str(value) speaker_id = chunk.get("speaker_id") - speakers = getattr(job, "speakers", None) + speakers = getattr(request, "speakers", None) if isinstance(speakers, dict) and speaker_id in speakers: speaker_entry = speakers.get(speaker_id) or {} if isinstance(speaker_entry, dict): @@ -163,7 +190,7 @@ def chunk_voice_spec(job: Any, chunk: Dict[str, Any], fallback: str) -> str: if fallback: return fallback - return job_voice_fallback(job) + return job_voice_fallback(request) def resolve_fallback_voice_spec( diff --git a/tests/test_domain_pipeline_factory.py b/tests/test_domain_pipeline_factory.py index 255cc1e..99dfa3b 100644 --- a/tests/test_domain_pipeline_factory.py +++ b/tests/test_domain_pipeline_factory.py @@ -124,11 +124,11 @@ class TestPipelinePool: mock_create.return_value = MagicMock() pool = PipelinePool() - job = MagicMock() - pool.get("kokoro", "en", use_gpu=True, job=job) + request = MagicMock() + pool.get("kokoro", "en", use_gpu=True, request=request) assert mock_cache.call_count == 1 - pool.get("kokoro", "en", use_gpu=True, job=job) + pool.get("kokoro", "en", use_gpu=True, request=request) assert mock_cache.call_count == 1 @patch("abogen.domain.pipeline_factory.initialize_voice_cache") diff --git a/tests/test_regression_webui_domain.py b/tests/test_regression_webui_domain.py index 9e49a38..7e50b43 100644 --- a/tests/test_regression_webui_domain.py +++ b/tests/test_regression_webui_domain.py @@ -122,10 +122,10 @@ class TestPipelinePoolRegression: def test_voice_cache_initialized_only_once(self, mock_cache, mock_create): mock_create.return_value = MagicMock() pool = PipelinePool() - job = MagicMock() + request = MagicMock() - pool.get("kokoro", "en", use_gpu=True, job=job) - pool.get("kokoro", "en", use_gpu=True, job=job) + pool.get("kokoro", "en", use_gpu=True, request=request) + pool.get("kokoro", "en", use_gpu=True, request=request) assert mock_cache.call_count == 1 @patch("abogen.domain.pipeline_factory.create_pipeline_for_job") @@ -133,13 +133,13 @@ class TestPipelinePoolRegression: def test_after_dispose_voice_cache_can_reinitialize(self, mock_cache, mock_create): mock_create.return_value = MagicMock() pool = PipelinePool() - job = MagicMock() + request = MagicMock() - pool.get("kokoro", "en", use_gpu=True, job=job) + pool.get("kokoro", "en", use_gpu=True, request=request) assert mock_cache.call_count == 1 pool.dispose_all() assert pool._voice_cache_initialized is False - pool.get("kokoro", "en", use_gpu=True, job=job) + pool.get("kokoro", "en", use_gpu=True, request=request) assert mock_cache.call_count == 2 diff --git a/tests/test_voice_resolution.py b/tests/test_voice_resolution.py index 6c6dc14..ba1de6b 100644 --- a/tests/test_voice_resolution.py +++ b/tests/test_voice_resolution.py @@ -1,6 +1,11 @@ """Tests for voice resolution helpers. Tests import from domain.voice_resolution (new location). +All test objects match the ConversionRequest interface: +- voice: str +- speakers: dict +- chapter_chunk.chapter_overrides: list +- chapter_chunk.chunks: list """ from __future__ import annotations @@ -11,6 +16,25 @@ from unittest.mock import patch import pytest +from abogen.application.conversion_config import ChapterChunkConfig + + +def _make_request( + voice: str = "", + speakers: dict | None = None, + chapter_overrides: list | None = None, + chunks: list | None = None, +) -> SimpleNamespace: + """Create a ConversionRequest-like object for testing.""" + return SimpleNamespace( + voice=voice, + speakers=speakers, + chapter_chunk=ChapterChunkConfig( + chapter_overrides=chapter_overrides or [], + chunks=chunks or [], + ), + ) + # --------------------------------------------------------------------------- # spec_to_voice_ids @@ -72,65 +96,62 @@ class TestSpecToVoiceIds: class TestJobVoiceFallback: - """job_voice_fallback resolves a fallback voice from job attributes.""" + """job_voice_fallback resolves a fallback voice from request attributes.""" def test_direct_voice(self): from abogen.domain.voice_resolution import job_voice_fallback - job = SimpleNamespace(voice="af_heart", speakers=None, chapters=[]) - assert job_voice_fallback(job) == "af_heart" + request = _make_request(voice="af_heart") + assert job_voice_fallback(request) == "af_heart" def test_custom_mix_ignored(self): from abogen.domain.voice_resolution import job_voice_fallback - job = SimpleNamespace(voice="__custom_mix", speakers=None, chapters=[]) - assert job_voice_fallback(job) == "" + request = _make_request(voice="__custom_mix") + assert job_voice_fallback(request) == "" def test_narrator_speaker(self): from abogen.domain.voice_resolution import job_voice_fallback - job = SimpleNamespace( + request = _make_request( voice="__custom_mix", speakers={"narrator": {"resolved_voice": "af_heart"}}, - chapters=[], ) - assert job_voice_fallback(job) == "af_heart" + assert job_voice_fallback(request) == "af_heart" def test_speaker_voice_formula(self): from abogen.domain.voice_resolution import job_voice_fallback - job = SimpleNamespace( + request = _make_request( voice="", speakers={"speaker1": {"voice_formula": "v1*v2"}}, - chapters=[], ) - assert job_voice_fallback(job) == "v1*v2" + assert job_voice_fallback(request) == "v1*v2" def test_chapter_voice(self): from abogen.domain.voice_resolution import job_voice_fallback - job = SimpleNamespace( + request = _make_request( voice="", - speakers=None, - chapters=[{"resolved_voice": "af_bella"}], + chapter_overrides=[{"resolved_voice": "af_bella"}], ) - assert job_voice_fallback(job) == "af_bella" + assert job_voice_fallback(request) == "af_bella" - def test_empty_job(self): + def test_empty_request(self): from abogen.domain.voice_resolution import job_voice_fallback - job = SimpleNamespace(voice="", speakers=None, chapters=[]) - assert job_voice_fallback(job) == "" + request = _make_request(voice="") + assert job_voice_fallback(request) == "" def test_narrator_custom_mix_falls_through(self): from abogen.domain.voice_resolution import job_voice_fallback - job = SimpleNamespace( + request = _make_request( voice="", speakers={"narrator": {"voice": "__custom_mix"}}, - chapters=[{"voice": "af_heart"}], + chapter_overrides=[{"voice": "af_heart"}], ) - assert job_voice_fallback(job) == "af_heart" + assert job_voice_fallback(request) == "af_heart" # --------------------------------------------------------------------------- @@ -144,35 +165,35 @@ class TestChapterVoiceSpec: def test_no_override_uses_fallback(self): from abogen.domain.voice_resolution import chapter_voice_spec - job = SimpleNamespace(voice="af_heart", speakers=None, chapters=[]) - assert chapter_voice_spec(job, None) == "af_heart" + request = _make_request(voice="af_heart") + assert chapter_voice_spec(request, None) == "af_heart" def test_resolved_voice_wins(self): from abogen.domain.voice_resolution import chapter_voice_spec - job = SimpleNamespace(voice="af_heart", speakers=None, chapters=[]) + request = _make_request(voice="af_heart") override = {"resolved_voice": "af_bella", "voice_formula": "x", "voice": "y"} - assert chapter_voice_spec(job, override) == "af_bella" + assert chapter_voice_spec(request, override) == "af_bella" def test_formula_second(self): from abogen.domain.voice_resolution import chapter_voice_spec - job = SimpleNamespace(voice="", speakers=None, chapters=[]) + request = _make_request(voice="") override = {"voice_formula": "v1*v2", "voice": "y"} - assert chapter_voice_spec(job, override) == "v1*v2" + assert chapter_voice_spec(request, override) == "v1*v2" def test_voice_third(self): from abogen.domain.voice_resolution import chapter_voice_spec - job = SimpleNamespace(voice="", speakers=None, chapters=[]) + request = _make_request(voice="") override = {"voice": "af_nicole"} - assert chapter_voice_spec(job, override) == "af_nicole" + assert chapter_voice_spec(request, override) == "af_nicole" def test_empty_override_falls_to_fallback(self): from abogen.domain.voice_resolution import chapter_voice_spec - job = SimpleNamespace(voice="af_heart", speakers=None, chapters=[]) - assert chapter_voice_spec(job, {}) == "af_heart" + request = _make_request(voice="af_heart") + assert chapter_voice_spec(request, {}) == "af_heart" # --------------------------------------------------------------------------- @@ -186,37 +207,37 @@ class TestChunkVoiceSpec: def test_chunk_direct_voice(self): from abogen.domain.voice_resolution import chunk_voice_spec - job = SimpleNamespace(speakers=None) + request = _make_request() chunk = {"resolved_voice": "af_heart"} - assert chunk_voice_spec(job, chunk, "fallback") == "af_heart" + assert chunk_voice_spec(request, chunk, "fallback") == "af_heart" def test_chunk_speaker_lookup(self): from abogen.domain.voice_resolution import chunk_voice_spec - job = SimpleNamespace(speakers={"narrator": {"resolved_voice": "af_bella"}}) + request = _make_request(speakers={"narrator": {"resolved_voice": "af_bella"}}) chunk = {"speaker_id": "narrator"} - assert chunk_voice_spec(job, chunk, "") == "af_bella" + assert chunk_voice_spec(request, chunk, "") == "af_bella" def test_chunk_voice_profile_lookup(self): from abogen.domain.voice_resolution import chunk_voice_spec - job = SimpleNamespace(speakers={"角色A": {"voice": "af_nicole"}}) + request = _make_request(speakers={"角色A": {"voice": "af_nicole"}}) chunk = {"voice_profile": "角色A"} - assert chunk_voice_spec(job, chunk, "") == "af_nicole" + assert chunk_voice_spec(request, chunk, "") == "af_nicole" def test_uses_fallback_string(self): from abogen.domain.voice_resolution import chunk_voice_spec - job = SimpleNamespace(speakers=None) + request = _make_request() chunk = {} - assert chunk_voice_spec(job, chunk, "my_fallback") == "my_fallback" + assert chunk_voice_spec(request, chunk, "my_fallback") == "my_fallback" - def test_fallback_to_job(self): + def test_fallback_to_request(self): from abogen.domain.voice_resolution import chunk_voice_spec - job = SimpleNamespace(voice="af_heart", speakers=None, chapters=[]) + request = _make_request(voice="af_heart") chunk = {} - assert chunk_voice_spec(job, chunk, "") == "af_heart" + assert chunk_voice_spec(request, chunk, "") == "af_heart" # --------------------------------------------------------------------------- @@ -225,50 +246,46 @@ class TestChunkVoiceSpec: class TestCollectRequiredVoiceIds: - """collect_required_voice_ids gathers all voice IDs from a job.""" + """collect_required_voice_ids gathers all voice IDs from a request.""" - def test_includes_job_voice(self): + def test_includes_request_voice(self): from abogen.domain.voice_resolution import collect_required_voice_ids - job = SimpleNamespace(voice="af_heart", chapters=[], chunks=[], speakers={}) + request = _make_request(voice="af_heart") with patch("abogen.domain.voice_resolution.get_voices", return_value={"af_heart"}), \ patch("abogen.domain.voice_resolution.job_voice_fallback", return_value=""): - result = collect_required_voice_ids(job) + result = collect_required_voice_ids(request) assert "af_heart" in result def test_includes_chapter_voices(self): from abogen.domain.voice_resolution import collect_required_voice_ids - job = SimpleNamespace( + request = _make_request( voice="", - chapters=[{"resolved_voice": "af_bella"}], - chunks=[], - speakers={}, + chapter_overrides=[{"resolved_voice": "af_bella"}], ) with patch("abogen.domain.voice_resolution.get_voices", return_value={"af_bella"}), \ patch("abogen.domain.voice_resolution.job_voice_fallback", return_value=""): - result = collect_required_voice_ids(job) + result = collect_required_voice_ids(request) assert "af_bella" in result def test_includes_chunk_voices(self): from abogen.domain.voice_resolution import collect_required_voice_ids - job = SimpleNamespace( + request = _make_request( voice="", - chapters=[], chunks=[{"voice": "af_nicole"}], - speakers={}, ) with patch("abogen.domain.voice_resolution.get_voices", return_value={"af_nicole"}), \ patch("abogen.domain.voice_resolution.job_voice_fallback", return_value=""): - result = collect_required_voice_ids(job) + result = collect_required_voice_ids(request) assert "af_nicole" in result def test_always_includes_kokoro_voices(self): from abogen.domain.voice_resolution import collect_required_voice_ids - job = SimpleNamespace(voice="", chapters=[], chunks=[], speakers={}) + request = _make_request(voice="") with patch("abogen.domain.voice_resolution.get_voices", return_value={"af_heart", "af_bella"}), \ patch("abogen.domain.voice_resolution.job_voice_fallback", return_value=""): - result = collect_required_voice_ids(job) + result = collect_required_voice_ids(request) assert {"af_heart", "af_bella"}.issubset(result)