providers: delete dead hal0.voice impl; document lemond-only STT/TTS dispatch#647
Conversation
thinmintdev
left a comment
There was a problem hiding this comment.
REVIEWER VERDICT: APPROVE — conditional on green CI (python checks currently pending; the deleted/modified test files are exactly what those checks validate, so certify on green). Adversarial dangling-ref pass is clean.
SPEC (#620): All 5 ACs met.
- Moonshine + Kokoro providers +
voice/shims removed:providers/{moonshine,kokoro}.pydeleted; entirevoice/package (__init__.py,kokoro.py,moonshine.py) deleted; dropped from_PROVIDERS,__all__. - Live exceptions retained:
ComfyUIProvider,FLMProviderkept in registry;LlamaServerProviderintact. _VALID_PROVIDERSreconciled — comfyui gap fixed (added), moonshine/kokoro removed. (This is the issue-mandated fix, not scope creep.)test_legacy_providers_still_registeredrewritten →test_live_providers_registered+test_retired_voice_providers_not_registered; test_kokoro/test_moonshine deleted.- ARCHITECTURE documents lemond-only dispatch + the 3 live exceptions + pi-coder-is-v0.4-gated note.
STANDARDS / adversarial grep: ZERO live importers of providers.moonshine/providers.kokoro/MoonshineProvider/KokoroProvider/hal0.voice anywhere in src/. Remaining moonshine/kokoro string hits are capability keys / lemond recipe names / migration maps / forward-compat enum values — not provider-class usage. Dispatcher has no voice refs. STT/TTS path intact (lemond whispercpp/kokoro recipes).
Non-blocking findings (maintainer option, do NOT gate):
- Behavioral asymmetry:
llama-server/flmstay in_VALID_PROVIDERSand round-trip-as-ignored, butmoonshine/kokoronow hard-ValidationError(schema.py:427provider_valid). #620 explicitly calls the path "vestigial, not a supported runtime" so this is intended — but a user-authored legacy slot TOML withprovider="moonshine"will now fail to load. No shipped/seed TOML does this (verified). If smoother legacy upgrades are wanted, keep-as-ignored (like llama-server/flm) is the safe pattern. Flagging for the merge-gate advisor. - Stale docstrings still list
MoonshineProvider/KokoroProvideras examples:providers/base.py:108,providers/lemonade.py:23.
Merge-ready on green. ARCHITECTURE.md overlaps #649 (see report) — sequential merge needs rebase of the 2nd.
|
Converted to draft — needs a scoping decision. Deleting the Decision needed: (A) keep |
…dispatch (closes #620) Minimal scope per maintainer: remove ONLY the dead local provider implementation — the `hal0.voice` package plus `providers/{kokoro,moonshine}.py` classes that ran Moonshine/Kokoro in-process. Confirmed no live importers. `moonshine` and `kokoro` REMAIN valid capability-provider identifiers everywhere the capability layer uses them (`SlotConfig.provider` validation in `config/schema.py`, `capabilities/config.py` default `provider="kokoro"`, `capabilities/catalog.py`, the backend/model classification in `api/routes`). The actual STT/TTS inference is served by lemond (whispercpp + kokoro recipes) or the corresponding toolbox image — not by an in-process hal0 provider class. Those files are left untouched. - providers/__init__.py: drop the deleted kokoro/moonshine imports + registrations (the runtime provider classes are gone; the identifiers stay valid as config values). - tests/providers/test_lemonade.py: assert the live providers resolve and the retired voice provider classes raise KeyError from get_provider(). - tests/test_import.py: drop the now-deleted hal0.voice module from the import parametrization. - ARCHITECTURE.md: document lemond-only STT/TTS dispatch while noting kokoro/moonshine remain capability providers served by lemond/toolbox. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
832d4cf to
0c1e72c
Compare
thinmintdev
left a comment
There was a problem hiding this comment.
REVIEWER VERDICT: APPROVE — conditional on green CI (python/γ pending; the deleted-test changes are what those checks validate — certify on green). Re-review of the MINIMAL re-scope (squashed commit 0c1e72c, 11 files). Confirmed truly minimal, NO capability-layer regression. All 6 verification points pass:
-
Dead impls deleted, no live importers —
voice/{__init__,kokoro,moonshine}.py+providers/{kokoro,moonshine}.pyall gone. Zero live imports ofhal0.voice/providers.moonshine/providers.kokoro/MoonshineProvider/KokoroProviderinsrc/; the only remaining mentions are docstrings/removal-notes (providers/init.py:15,41; base.py:108; lemonade.py:23). ✓ -
_VALID_PROVIDERSUNCHANGED — still{lemonade, llama-server, flm, moonshine, kokoro}(schema.py:89).config/schema.pyis byte-identical to main (not in the PR diff). Sotest_all_valid_providerspasses legitimately, not by weakening the assertion. ✓ -
SELF_MANAGED_PROVIDERSrestored to{kokoro, moonshine, vibevoice}(state.py:124) — vibevoice is back; the prior over-broad version that dropped it is corrected. ✓ -
test_import.pyno longer referenceshal0.voice— the diff removes the"hal0.voice"entry from the import-smoke list. ✓ -
ARCHITECTURE.md states impls retired, identifiers remain — explicitly: "The dead local MoonshineProvider/KokoroProvider implementation classes (the hal0.voice package …) were deleted in #620 — they had no live importers.
moonshineandkokororemain valid capability-provider identifiers in the config/capability layer (SlotConfig.provider, capabilities/config.py, capabilities/catalog.py …); inference is served by lemond (whispercpp + kokoro recipes)." Correct framing. ✓ -
comfyui-gap is genuinely PRE-EXISTING on main —
_VALID_PROVIDERSon main lackscomfyuiwhileinstaller/etc-hal0/slots/img.toml:18seedsprovider = "comfyui". This latent gap exists identically on main and is untouched here (schema.py byte-identical). Correctly deferred to a separate issue, NOT introduced by this PR. ✓
This addresses my prior review's sole non-blocking concern (the behavioral asymmetry / hard-ValidationError on legacy moonshine/kokoro TOMLs): by leaving _VALID_PROVIDERS intact, those configs still load — no upgrade break. Both prior stale-docstring nits (base.py:108, lemonade.py:23) persist but remain non-blocking.
Scope is exactly: delete dead in-process impls + drop their _PROVIDERS registrations + document lemond-only dispatch. No dispatcher, schema, capabilities, or CLI regression. Merge-ready on green.
Closes #620
Scope (minimal, per maintainer decision)
Delete only the dead local provider implementation — the in-process
classes that ran Moonshine/Kokoro — while keeping
moonshineandkokoroas valid capability-provider identifiers. lemond (and thetoolbox images) serve STT/TTS now, so the in-process hal0 provider
classes had no live importers and are pure dead code.
Deleted (the #620 win — dead code, no live importers)
src/hal0/voice/— the entire re-export shim package (__init__.py,kokoro.py,moonshine.py)src/hal0/providers/kokoro.py—KokoroProviderimplsrc/hal0/providers/moonshine.py—MoonshineProviderimpltests/providers/test_kokoro.py,tests/providers/test_moonshine.py— tests for the deleted implsKept VALID (capability layer untouched — these stay valid provider identifiers)
config/schema.py_VALID_PROVIDERS— still includesmoonshine+kokoro(aSlotConfig(provider="kokoro")validates fine)slots/state.pySELF_MANAGED_PROVIDERS— still{kokoro, moonshine, vibevoice}capabilities/config.pydefaultprovider = "kokoro",capabilities/catalog.py,api/routes/backends.py+models.pyclassification — all unchangedMechanically forced by the deletion
providers/__init__.py— drop the kokoro/moonshine imports +_PROVIDERSregistrations (the runtime classes are gone; the identifiers remain valid config values).get_provider("kokoro")now raisesKeyError.tests/providers/test_lemonade.py— assert live providers resolve and the retired voice classes raiseKeyErrorfromget_provider().Doc (the #620 doc ask)
tests/test_import.py— remove the now-deletedhal0.voicemodule from the import parametrization.ARCHITECTURE.md— document lemond-only STT/TTS dispatch, while explicitly notingkokoro/moonshineremain capability-provider identifiers served by lemond/toolbox (not by an in-process hal0 provider class).Tests / lint
tests/config/test_schema.py tests/test_import.py→ 59 passed (the two failures the broad version introduced —test_all_valid_providersstays green because moonshine/kokoro remain valid;hal0.voiceimport entry removed)tests/slots/ tests/providers/→ 265 passed (no regression from reverting the over-broad enum/SELF_MANAGED changes)ruff check→ all checks passed;ruff format --check→ all formattedDeferred (not in this PR)
comfyuiis not inconfig/schema.py_VALID_PROVIDERSeven thoughinstaller/etc-hal0/slots/img.tomldeclaresprovider = "comfyui". This is a pre-existing latent gap onmain(not introduced or fixed here). Flagging for a separate issue to keep this PR minimal-scope.🤖 Generated with Claude Code