Phase 2 Plan 02-03: IndexTTS on SubprocessBackend (closes #42) - #98
Merged
Conversation
Migrates IndexTTS-2 off the in-process import path and onto the SubprocessBackend primitive shipped in Plan 02-01. Closes issue #42 with a structural fix — the parent's transformers>=5.3 and IndexTTS's transformers<5 now live in separate OS processes and can never collide. * New: backend/engines/indextts/ — sidecar package (__init__.py hosts IndexTTS2Backend, main.py is the sidecar entrypoint, bootstrap.py owns the 3-step venv probe + lazy uv-based bootstrap). * services.tts_backend: IndexTTS2Backend's in-process body removed; registry resolves the class lazily via a _LazyRegistry indirection + PEP 562 __getattr__ re-export. This breaks the import cycle that arose when both subprocess_backend and tts_backend tried to import each other at module load. * docs/engines/indextts.md: install walkthrough + venv resolution order + common errors (linked from is_available()'s unavailable message). * tests: - test_indextts_backward_compat.py (8) — probe priority, no-spawn discipline, HF cache marker preservation (ENGINE-07). - test_indextts_sidecar.py (17) — subclass shape, isolation_mode, parent-side emotion arbitration (vector/audio/text/description), coexist-with-OmniVoice (headline #42 closure), env forwarding. - tests/fixtures/mock_indextts_sidecar.py — stdlib-only sidecar mimicking the production wire protocol; emits 1 s sine wave. - test_issue_fixes.py: two obsolete in-process-conflict tests rewritten to assert the new subprocess contract (no indextts.* import in the parent). Hard constraints honored: backend/services/sonitranslate.py and gpu_sandbox.py are untouched (D1 / D4). Existing v0.2.7 users with OMNIVOICE_INDEXTTS_DIR and a populated HF cache reach a working generation with zero re-download and zero re-install. 44 tests pass across the four exercised files. Full suite: 391 passed, 10 skipped, 13 xfailed, 1 xpassed in 57 s. Smoke: 4 passed. Closes #42. Requirements: ENGINE-02, ENGINE-03, ENGINE-04, ENGINE-07. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR implements subprocess isolation for the IndexTTS-2 TTS engine to resolve incompatible transformers version requirements: OmniVoice requires >=5.3 while IndexTTS requires <5. The solution runs IndexTTS in a dedicated subprocess with its own venv, eliminating in-process import collisions while preserving the engine's emotion-control capabilities through parent-side parameter arbitration. ChangesIndexTTS Subprocess Backend Architecture
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
backend/services/tts_backend.py (1)
1170-1175: 💤 Low valueUnreachable branch and confusing conditional in
__getattr__.The condition on line 1171-1172 is effectively dead code:
_LAZY_REGISTRYkeys are engine IDs ("indextts2"), not class names- Users import
IndexTTS2Backend(the class name), which is handled by lines 1173-1174- The
else Nonebranch on line 1172 is unreachable since_LazyRegistry.__contains__returnsTruefor any key in_LAZY_REGISTRYThe code works correctly, but could be simplified:
🔧 Proposed simplification
def __getattr__(name: str): # pragma: no cover - exercised via tests - if name in _LAZY_REGISTRY: - return _REGISTRY[name if name in _REGISTRY else None] if name == "IndexTTS2Backend": return _REGISTRY["indextts2"] raise AttributeError(f"module {__name__!r} has no attribute {name!r}")If future lazy-loaded class names need support, the pattern can be extended explicitly rather than relying on the registry key lookup.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/tts_backend.py` around lines 1170 - 1175, The __getattr__ function contains an unreachable/ confusing branch: remove the conditional that checks name in _LAZY_REGISTRY and the expression returning _REGISTRY[name if name in _REGISTRY else None]; instead, handle the known class-name alias explicitly (return _REGISTRY["indextts2"] when name == "IndexTTS2Backend") and otherwise raise AttributeError. If you want to support future lazy-loaded class names, add an explicit mapping from class-name strings to registry keys rather than relying on _LAZY_REGISTRY membership.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/engines/indextts/bootstrap.py`:
- Around line 226-256: The try/except blocks around the two subprocess.run calls
(the "uv venv" block and the "uv pip install" block that uses python_path from
_venv_python_path and indextts_clone) only catch subprocess.CalledProcessError
and will surface raw tracebacks on timeouts; update both exception handlers to
also catch subprocess.TimeoutExpired and raise a RuntimeError with a clear,
actionable message (include which command timed out, the timeout value from
_UV_VENV_TIMEOUT_S or _UV_PIP_INSTALL_TIMEOUT_S, the _ENGINES_VENV_DIR or
indextts_clone context as appropriate, and a link to the docs/help) so timeouts
produce user-friendly errors instead of tracebacks.
In `@docs/engines/indextts.md`:
- Line 99: The heading currently uses nested backticks and is malformed; fix it
by removing the outer backticks and keeping a single inline code span for the
import reference so the heading reads like: use a plain heading text with
`import indextts.infer_v2` (e.g., replace "### `IndexTTS bootstrap completed but
`import indextts.infer_v2` still fails`" with "### IndexTTS bootstrap completed
but `import indextts.infer_v2` still fails"). Ensure only one pair of backticks
surrounds the symbol import indextts.infer_v2 and no extra backticks wrap the
entire heading.
- Around line 24-33: The install docs currently omit required uv mirror/index
and retry settings; update the installation steps around the `uv pip install -e
.` guidance to document the environment variables `UV_PYTHON_INSTALL_MIRROR`,
`UV_DEFAULT_INDEX`, `UV_HTTP_TIMEOUT`, and `UV_HTTP_RETRIES`, show example
export commands, and add region-specific notes: recommend Tsinghua as primary
and Aliyun as fallback for China, and explicitly state that Russia has no
blessed PyPI mirror and users must tunnel via VPN to reach official indexes;
ensure the new text sits immediately before the `cd index-tts` / `uv venv .venv`
block and uses the same plain-shell style as surrounding docs.
- Around line 35-58: Add a short HuggingFace token setup section to the IndexTTS
doc that promotes the in-app Settings token field as the primary method and
documents HF_TOKEN only as an override; mention the Settings field path
explicitly (e.g., "Settings → HuggingFace token") and then show
platform-specific environment variable examples for macOS/Linux and Windows
(PowerShell and CMD) to set HF_TOKEN, and note that OMNIVOICE_INDEXTTS_DIR
remains the repo root containing checkpoints/ and pyproject.toml.
In `@tests/backend/services/test_indextts_sidecar.py`:
- Around line 347-353: The test currently only checks that
OmniVoiceBackend.is_available() boolean (ok) didn't change after IndexTTS ran;
strengthen it to also assert the returned message is unchanged so a different
post-IndexTTS failure message is caught. After calling
OmniVoiceBackend.is_available() the second time (ok2, msg2), assert ok2 == ok
and msg2 == msg (using the existing ok, msg, ok2, msg2 variables) and include a
failure message that shows both old and new values for easier debugging.
- Around line 73-76: The teardown is currently swallowing all exceptions from
backend.shutdown() which hides failures and can leave the sidecar running;
update the try/except around backend.shutdown() to capture the exception (e.g.,
except Exception as e) and surface it instead of silently passing — either log
the error and re-raise, or call pytest.fail with a descriptive message including
e, so failures in backend.shutdown() are visible and tests can't silently leave
processes running.
In `@tests/test_issue_fixes.py`:
- Around line 126-134: The test allows the vague token "conflict" which makes
is_available() messages too permissive; update the assertion in
tests/test_issue_fixes.py (the msg_lower variable and the is_available() failure
check) to remove the "conflict" branch and instead assert only explicit
actionable cues (e.g., keep "omnivoice_indextts_dir",
"docs/engines/indextts.md", "git clone", and ensure an install/venv marker such
as "pip install", "venv" or "virtualenv" is present) so the failure message must
point to install/venv/doc guidance rather than the old "conflict" wording.
---
Nitpick comments:
In `@backend/services/tts_backend.py`:
- Around line 1170-1175: The __getattr__ function contains an unreachable/
confusing branch: remove the conditional that checks name in _LAZY_REGISTRY and
the expression returning _REGISTRY[name if name in _REGISTRY else None];
instead, handle the known class-name alias explicitly (return
_REGISTRY["indextts2"] when name == "IndexTTS2Backend") and otherwise raise
AttributeError. If you want to support future lazy-loaded class names, add an
explicit mapping from class-name strings to registry keys rather than relying on
_LAZY_REGISTRY membership.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2dfe2cac-77b9-45fa-9427-a24726cb7dc1
📒 Files selected for processing (10)
.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-03-SUMMARY.mdbackend/engines/indextts/__init__.pybackend/engines/indextts/bootstrap.pybackend/engines/indextts/main.pybackend/services/tts_backend.pydocs/engines/indextts.mdtests/backend/services/test_indextts_backward_compat.pytests/backend/services/test_indextts_sidecar.pytests/fixtures/mock_indextts_sidecar.pytests/test_issue_fixes.py
Comment on lines
+226
to
+256
| try: | ||
| subprocess.run( | ||
| [uv, "venv", str(_ENGINES_VENV_DIR)], | ||
| check=True, | ||
| timeout=_UV_VENV_TIMEOUT_S, | ||
| capture_output=True, | ||
| ) | ||
| except subprocess.CalledProcessError as exc: | ||
| raise RuntimeError( | ||
| f"uv venv failed for IndexTTS bootstrap at {_ENGINES_VENV_DIR}: " | ||
| f"{exc.stderr.decode('utf-8', errors='replace') if exc.stderr else exc}" | ||
| ) from exc | ||
|
|
||
| python_path = _venv_python_path(_ENGINES_VENV_DIR) | ||
| try: | ||
| subprocess.run( | ||
| [ | ||
| uv, "pip", "install", | ||
| "--python", str(python_path), | ||
| "-e", str(indextts_clone), | ||
| ], | ||
| check=True, | ||
| timeout=_UV_PIP_INSTALL_TIMEOUT_S, | ||
| capture_output=True, | ||
| ) | ||
| except subprocess.CalledProcessError as exc: | ||
| raise RuntimeError( | ||
| "uv pip install -e failed during IndexTTS bootstrap " | ||
| f"({indextts_clone}): " | ||
| f"{exc.stderr.decode('utf-8', errors='replace') if exc.stderr else exc}" | ||
| ) from exc |
Contributor
There was a problem hiding this comment.
Handle TimeoutExpired alongside CalledProcessError for clearer error messaging.
Both subprocess.run calls catch CalledProcessError but not TimeoutExpired. If uv venv or uv pip install exceeds the timeout (120s/900s respectively), users see a raw traceback instead of an actionable error message with docs link.
Proposed fix
try:
subprocess.run(
[uv, "venv", str(_ENGINES_VENV_DIR)],
check=True,
timeout=_UV_VENV_TIMEOUT_S,
capture_output=True,
)
- except subprocess.CalledProcessError as exc:
+ except subprocess.TimeoutExpired:
+ raise RuntimeError(
+ f"uv venv timed out after {_UV_VENV_TIMEOUT_S}s while bootstrapping "
+ f"IndexTTS at {_ENGINES_VENV_DIR}. This can happen on slow systems. "
+ "See docs/engines/indextts.md for manual install steps."
+ ) from None
+ except subprocess.CalledProcessError as exc:
raise RuntimeError(
f"uv venv failed for IndexTTS bootstrap at {_ENGINES_VENV_DIR}: "
f"{exc.stderr.decode('utf-8', errors='replace') if exc.stderr else exc}"
) from exc
python_path = _venv_python_path(_ENGINES_VENV_DIR)
try:
subprocess.run(
[
uv, "pip", "install",
"--python", str(python_path),
"-e", str(indextts_clone),
],
check=True,
timeout=_UV_PIP_INSTALL_TIMEOUT_S,
capture_output=True,
)
- except subprocess.CalledProcessError as exc:
+ except subprocess.TimeoutExpired:
+ raise RuntimeError(
+ f"uv pip install timed out after {_UV_PIP_INSTALL_TIMEOUT_S}s while "
+ f"installing IndexTTS from {indextts_clone}. This can happen on slow "
+ "connections or cold caches. See docs/engines/indextts.md."
+ ) from None
+ except subprocess.CalledProcessError as exc:
raise RuntimeError(
"uv pip install -e failed during IndexTTS bootstrap "
f"({indextts_clone}): "
f"{exc.stderr.decode('utf-8', errors='replace') if exc.stderr else exc}"
) from exc🧰 Tools
🪛 Ruff (0.15.13)
[error] 227-227: subprocess call: check for execution of untrusted input
(S603)
[error] 241-241: subprocess call: check for execution of untrusted input
(S603)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/engines/indextts/bootstrap.py` around lines 226 - 256, The try/except
blocks around the two subprocess.run calls (the "uv venv" block and the "uv pip
install" block that uses python_path from _venv_python_path and indextts_clone)
only catch subprocess.CalledProcessError and will surface raw tracebacks on
timeouts; update both exception handlers to also catch subprocess.TimeoutExpired
and raise a RuntimeError with a clear, actionable message (include which command
timed out, the timeout value from _UV_VENV_TIMEOUT_S or
_UV_PIP_INSTALL_TIMEOUT_S, the _ENGINES_VENV_DIR or indextts_clone context as
appropriate, and a link to the docs/help) so timeouts produce user-friendly
errors instead of tracebacks.
Comment on lines
+24
to
+33
| 2. Install the editable package into a fresh venv. Use | ||
| `uv pip install -e .` — **never** `uv sync --all-extras`, which | ||
| would overwrite OmniVoice's lock file with `transformers<5` and | ||
| break the parent process: | ||
|
|
||
| ```bash | ||
| cd index-tts | ||
| uv venv .venv | ||
| uv pip install -e . | ||
| ``` |
Contributor
There was a problem hiding this comment.
Document restricted-network uv mirror/index settings required by docs policy.
The install section currently omits mirror/index/retry guidance for constrained regions. Please add UV_PYTHON_INSTALL_MIRROR, UV_DEFAULT_INDEX, retry/timeout variables, China mirror guidance (Tsinghua primary, Aliyun fallback), and explicit Russia VPN guidance.
Proposed doc patch
2. Install the editable package into a fresh venv. Use
`uv pip install -e .` — **never** `uv sync --all-extras`, which
would overwrite OmniVoice's lock file with `transformers<5` and
break the parent process:
+
+ Restricted-network bootstrap (optional, recommended where needed):
+
+ ```bash
+ export UV_PYTHON_INSTALL_MIRROR=https://github.com/astral-sh/python-build-standalone/releases/download
+ export UV_DEFAULT_INDEX=https://pypi.org/simple
+ export UV_HTTP_TIMEOUT=120
+ export UV_HTTP_RETRIES=5
+ ```
+
+ China: prefer Tsinghua mirror, fallback to Aliyun for package index.
+ Russia: no blessed PyPI mirror; use a VPN tunnel to reach official indexes.As per coding guidelines, docs/**/*.md: “Installation documentation: Document UV_PYTHON_INSTALL_MIRROR and region-specific PyPI mirrors (Tsinghua for China, Aliyun fallback) with explicit guidance that Russia has no blessed PyPI mirror and users should tunnel via VPN”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/engines/indextts.md` around lines 24 - 33, The install docs currently
omit required uv mirror/index and retry settings; update the installation steps
around the `uv pip install -e .` guidance to document the environment variables
`UV_PYTHON_INSTALL_MIRROR`, `UV_DEFAULT_INDEX`, `UV_HTTP_TIMEOUT`, and
`UV_HTTP_RETRIES`, show example export commands, and add region-specific notes:
recommend Tsinghua as primary and Aliyun as fallback for China, and explicitly
state that Russia has no blessed PyPI mirror and users must tunnel via VPN to
reach official indexes; ensure the new text sits immediately before the `cd
index-tts` / `uv venv .venv` block and uses the same plain-shell style as
surrounding docs.
Comment on lines
+35
to
+58
| 3. Download the model weights (~6 GB). Either: | ||
|
|
||
| ```bash | ||
| hf download IndexTeam/IndexTTS-2 --local-dir=checkpoints | ||
| ``` | ||
|
|
||
| or let HuggingFace cache them on first synthesize call (the parent | ||
| forwards `HF_HOME` / `HF_HUB_CACHE` to the sidecar so the cache is | ||
| shared with the rest of OmniVoice's downloads). | ||
|
|
||
| 4. Set the `OMNIVOICE_INDEXTTS_DIR` environment variable to the repo | ||
| root (the directory that contains `checkpoints/` and | ||
| `pyproject.toml`): | ||
|
|
||
| ```bash | ||
| # macOS / Linux | ||
| echo 'export OMNIVOICE_INDEXTTS_DIR=$HOME/code/index-tts' >> ~/.zshrc | ||
| source ~/.zshrc | ||
| ``` | ||
|
|
||
| ```powershell | ||
| # Windows PowerShell | ||
| [Environment]::SetEnvironmentVariable("OMNIVOICE_INDEXTTS_DIR","$env:USERPROFILE\code\index-tts","User") | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add required HuggingFace token setup guidance (Settings-first + env override).
This install flow uses Hugging Face downloads but does not document the required token setup path. Please add a short section that promotes the in-app Settings token field as primary, and documents HF_TOKEN only as an override with macOS/Linux/Windows examples.
Proposed doc patch
+### HuggingFace token setup (required for gated/rate-limited downloads)
+
+Primary (recommended): set your token in OmniVoice UI:
+- **Settings → HuggingFace Token**
+
+Override-only path (advanced): `HF_TOKEN` environment variable.
+
+```bash
+# macOS / Linux
+export HF_TOKEN=hf_xxx
+```
+
+```powershell
+# Windows PowerShell
+[Environment]::SetEnvironmentVariable("HF_TOKEN","hf_xxx","User")
+```
+
+```cmd
+:: Windows CMD
+setx HF_TOKEN hf_xxx
+```As per coding guidelines, docs/**/*.md: “HuggingFace token setup documentation must include both the in-app Settings field path (promoted) and the environment variable override path (documented but not promoted), with platform-specific examples for macOS, Linux, and Windows”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/engines/indextts.md` around lines 35 - 58, Add a short HuggingFace token
setup section to the IndexTTS doc that promotes the in-app Settings token field
as the primary method and documents HF_TOKEN only as an override; mention the
Settings field path explicitly (e.g., "Settings → HuggingFace token") and then
show platform-specific environment variable examples for macOS/Linux and Windows
(PowerShell and CMD) to set HF_TOKEN, and note that OMNIVOICE_INDEXTTS_DIR
remains the repo root containing checkpoints/ and pyproject.toml.
| into your `PATH` (https://docs.astral.sh/uv/) or pre-create the venv | ||
| manually with `uv venv` and `uv pip install -e` as in step 2. | ||
|
|
||
| ### `IndexTTS bootstrap completed but `import indextts.infer_v2` still fails` |
Contributor
There was a problem hiding this comment.
Fix malformed inline code in heading (markdownlint MD038).
The heading nests backticks and is parsed incorrectly. Use a plain heading with a single code span.
Proposed doc patch
-### `IndexTTS bootstrap completed but `import indextts.infer_v2` still fails`
+### IndexTTS bootstrap completed but `import indextts.infer_v2` still fails📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ### `IndexTTS bootstrap completed but `import indextts.infer_v2` still fails` | |
| ### IndexTTS bootstrap completed but `import indextts.infer_v2` still fails |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 99-99: Spaces inside code span elements
(MD038, no-space-in-code)
[warning] 99-99: Spaces inside code span elements
(MD038, no-space-in-code)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/engines/indextts.md` at line 99, The heading currently uses nested
backticks and is malformed; fix it by removing the outer backticks and keeping a
single inline code span for the import reference so the heading reads like: use
a plain heading text with `import indextts.infer_v2` (e.g., replace "###
`IndexTTS bootstrap completed but `import indextts.infer_v2` still fails`" with
"### IndexTTS bootstrap completed but `import indextts.infer_v2` still fails").
Ensure only one pair of backticks surrounds the symbol import indextts.infer_v2
and no extra backticks wrap the entire heading.
Comment on lines
+73
to
+76
| try: | ||
| backend.shutdown() | ||
| except Exception: | ||
| pass |
Contributor
There was a problem hiding this comment.
Avoid swallowing fixture teardown failures.
The bare except Exception: pass can hide failed shutdowns and leave sidecar processes alive, which can make later tests flaky.
Proposed fix
backend = IndexTTS2Backend()
yield backend
try:
backend.shutdown()
- except Exception:
- pass
+ except Exception:
+ proc = getattr(backend, "_proc", None)
+ if proc is not None and proc.poll() is None:
+ proc.kill()
+ proc.wait(timeout=5)🧰 Tools
🪛 Ruff (0.15.13)
[error] 75-76: try-except-pass detected, consider logging the exception
(S110)
[warning] 75-75: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/backend/services/test_indextts_sidecar.py` around lines 73 - 76, The
teardown is currently swallowing all exceptions from backend.shutdown() which
hides failures and can leave the sidecar running; update the try/except around
backend.shutdown() to capture the exception (e.g., except Exception as e) and
surface it instead of silently passing — either log the error and re-raise, or
call pytest.fail with a descriptive message including e, so failures in
backend.shutdown() are visible and tests can't silently leave processes running.
Comment on lines
+347
to
+353
| ok2, msg2 = OmniVoiceBackend.is_available() | ||
| # Whatever state OmniVoiceBackend was in before, it's the same | ||
| # after IndexTTS ran — no new import errors, no new AttributeErrors. | ||
| assert ok2 == ok, ( | ||
| f"OmniVoice availability changed after IndexTTS generate " | ||
| f"({ok}->{ok2}, msg={msg2})" | ||
| ) |
Contributor
There was a problem hiding this comment.
Strengthen the coexistence regression assertion.
This only asserts availability boolean stability. If OmniVoice is unavailable both times, a different post-IndexTTS error message would be missed.
Proposed fix
ok2, msg2 = OmniVoiceBackend.is_available()
# Whatever state OmniVoiceBackend was in before, it's the same
# after IndexTTS ran — no new import errors, no new AttributeErrors.
assert ok2 == ok, (
f"OmniVoice availability changed after IndexTTS generate "
f"({ok}->{ok2}, msg={msg2})"
)
+ if not ok:
+ assert msg2 == pre, (
+ "OmniVoice error message changed after IndexTTS generate; "
+ f"before={pre!r}, after={msg2!r}"
+ )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/backend/services/test_indextts_sidecar.py` around lines 347 - 353, The
test currently only checks that OmniVoiceBackend.is_available() boolean (ok)
didn't change after IndexTTS ran; strengthen it to also assert the returned
message is unchanged so a different post-IndexTTS failure message is caught.
After calling OmniVoiceBackend.is_available() the second time (ok2, msg2),
assert ok2 == ok and msg2 == msg (using the existing ok, msg, ok2, msg2
variables) and include a failure message that shows both old and new values for
easier debugging.
Comment on lines
+126
to
+134
| # Must point at the env-var-driven install path OR the docs. | ||
| msg_lower = msg.lower() | ||
| assert ( | ||
| "omnivoice_indextts_dir" in msg_lower | ||
| or "docs/engines/indextts.md" in msg_lower | ||
| or "uv pip install" in msg_lower | ||
| or "git clone" in msg_lower | ||
| or "conflict" in msg_lower | ||
| ), f"is_available() failure message not actionable: {msg!r}" |
Contributor
There was a problem hiding this comment.
Tighten actionable-message assertion; conflict makes this regression too permissive.
Including "conflict" allows older non-actionable wording to pass and weakens the #42 guidance check. Prefer install/venv/doc cues only.
Proposed test patch
assert (
"omnivoice_indextts_dir" in msg_lower
or "docs/engines/indextts.md" in msg_lower
or "uv pip install" in msg_lower
or "git clone" in msg_lower
- or "conflict" in msg_lower
), f"is_available() failure message not actionable: {msg!r}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_issue_fixes.py` around lines 126 - 134, The test allows the vague
token "conflict" which makes is_available() messages too permissive; update the
assertion in tests/test_issue_fixes.py (the msg_lower variable and the
is_available() failure check) to remove the "conflict" branch and instead assert
only explicit actionable cues (e.g., keep "omnivoice_indextts_dir",
"docs/engines/indextts.md", "git clone", and ensure an install/venv marker such
as "pip install", "venv" or "virtualenv" is present) so the failure message must
point to install/venv/doc guidance rather than the old "conflict" wording.
This was referenced May 20, 2026
This was referenced Jun 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates IndexTTS-2 off the in-process import path and onto the
SubprocessBackendprimitive shipped in Plan 02-01. Closes #42 with a structural fix — IndexTTS runs in its own subprocess + dedicated venv withtransformers<5while OmniVoice keeps itstransformers>=5.3pin. The two libraries now live in different OS processes and can never collide.backend/engines/indextts/— sidecar package__init__.py— hostsIndexTTS2Backend(SubprocessBackend)and parent-side emotion/duration arbitrationmain.py— sidecar entrypoint; JSON-stdio loop; lazy IndexTTS2 model load with progress framesbootstrap.py— 3-step venv probe (${OMNIVOICE_INDEXTTS_DIR}/.venv→engines/indextts/.venv→uv venv+uv pip install -e)services.tts_backend— IndexTTS2Backend's in-process body removed (~150 LOC); registry resolves the class lazily via_LazyRegistry+ PEP 562__getattr__re-export. Breaks the import cycle that arose when bothsubprocess_backendandtts_backendtried to import each other.docs/engines/indextts.md— install walkthrough + venv resolution order; linked fromis_available()'s unavailable message.Requirements covered
test_hf_home_marker_present_after_bootstrapproves the HF cache survives bootstrap byte-for-byte. SubprocessBackend'sos.environ.copy()forwardsHF_HOME / HF_HUB_CACHE / HF_ENDPOINT / HF_TOKENto the sidecar (verified bytest_env_forwarding_to_indextts_sidecar).IndexTTS2Backendis now aSubprocessBackendsubclass.test_indextts_no_inprocess_import_attemptedproves noimport indextts.*ever fires in the parent process.test_coexist_with_omnivoice_in_one_session— OmniVoiceBackend (in-process, transformers>=5.3) and IndexTTS2Backend (subprocess, transformers<5) both servegenerate()in the same Python interpreter. The headline #42 closure test.test_venv_probe_prefers_omnivoice_indextts_dir— existing v0.2.7 users withOMNIVOICE_INDEXTTS_DIR + .venvreach a working generation with zero re-download and zero re-install. The cache-marker test asserts the bootstrap never mutates$HF_HOME/hub/models--IndexTeam--IndexTTS-2/.New artifact paths
backend/engines/indextts/__init__.py—IndexTTS2Backendclassbackend/engines/indextts/main.py— sidecar entrypointbackend/engines/indextts/bootstrap.py—resolve_indextts_venv/is_indextts_installed/INDEXTTS_SIDECAR_SCRIPTtests/backend/services/test_indextts_backward_compat.py(8 tests)tests/backend/services/test_indextts_sidecar.py(17 tests, includes the back-compat regression test)tests/fixtures/mock_indextts_sidecar.py(stdlib-only mock for unit tests)docs/engines/indextts.md.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-03-SUMMARY.mdBack-compat regression test
tests/backend/services/test_indextts_sidecar.py::test_coexist_with_omnivoice_in_one_session— instantiates both OmniVoiceBackend (in-process import ofomnivoice.models.omnivoice) and IndexTTS2Backend (via the mock sidecar fixture), then asserts that calling IndexTTS'sgeneratedoes not break OmniVoice'sis_available(). Pre-Plan 02-03 this would have hit theOffloadedCacheImportError; now the second engine lives in a separate interpreter and the two transformers versions never meet.Test results
Hard constraints honored
backend/services/sonitranslate.pyunchanged (D1)backend/services/gpu_sandbox.pyunchanged (D4)bin/pythonand WindowsScripts/python.exetests/test_issue_fixes.py(which asserted the old in-process error message) were rewritten to validate the new subprocess contract — no functional regression elsewhereDeviation from RESEARCH.md sidecar skeleton
The plan's
<interfaces>block putIndexTTS2Backendinsidebackend/services/tts_backend.py. I moved it intobackend/engines/indextts/__init__.pyto break thesubprocess_backend ↔ tts_backendimport cycle (the alternative — extractingTTSBackendinto a separate module — was a bigger refactor with broader blast radius). The registry uses a_LazyRegistryindirection + PEP 562__getattr__so callers can still writefrom services.tts_backend import IndexTTS2Backend. Details in.planning/phases/02-.../02-03-SUMMARY.md— relevant for the Phase 3 Supertonic-3 author who will follow the same sidecar pattern.Test plan
uv run pytest tests/backend/services/test_indextts_backward_compat.py -x -v→ 8/8 passuv run pytest tests/backend/services/test_indextts_sidecar.py -x -v→ 17/17 passuv run pytest tests/backend/services/test_subprocess_backend.py tests/backend/services/test_tts_backend_registry.py -v→ 19/19 pass (no regressions in the SubprocessBackend primitive or the registry surface)uv run pytest tests/smoke/ -q→ 4 passeduv run pytest tests/ -q --ignore=tests/manual→ 391 passedgit diff backend/services/sonitranslate.py→ empty (D1)git diff backend/services/gpu_sandbox.py→ empty (D4)/enginesreportsavailable=true, isolation_mode="subprocess"→ IndexTTS generate via/api/tts/generatereturns audio → backend shutdown reaps the sidecar within 5 s🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
#42: Library version conflicts with IndexTTS integration