feat(voice): S1 — flute-gateway voice_profiles registry (loader + routing + /profiles + /validate) - #1922
Conversation
…files)
Add a lifespan-preloaded, TTL-cached, NATS-invalidated voice profile registry
that maps a requested `voice` slug -> (provider, params) via v5_16
pmoves_core.voice_profiles, plus /v1/voice/profiles + /v1/voice/validate routes.
- voice_registry.py: VoiceProfile dataclass + VoiceRegistry (load/refresh/get/
list/upsert_local/run_ttl_loop/subscribe). Graceful degradation: httpx error /
404 / table-absent (PGRST205/42P01) / non-2xx -> empty cache, _healthy=False,
WARNING, return False. load() never raises out of startup.
- select_provider_and_params: EXPLICIT per-engine translation of engine_specific
into the fixed provider signatures (no synth_kwargs):
vibevoice {voice_preset} -> voice
voicebox {profile_id,voice_type,...} -> voice(+engine)
ultimate_tts {primary_engine,<eng>_voice} -> (engine, voice)
omnivoice {ref_audio,instruct,...} -> (voice=ref_audio, engine=instruct)
- main.py: lifespan preload (after providers/NATS, non-fatal) + TTL task +
voice.registry.update.v1 subscribe; request-mutation hook (_resolve_voice_profile)
ahead of persona resolution at all 3 dispatch sites — mutates request.provider/
engine/voice BEFORE `provider_name = request.provider or DEFAULT_PROVIDER`.
- pmoves_core schema reads/writes set Accept-Profile/Content-Profile headers.
- Grounding contract validation: forbidden consciousness_shape, persona_ids +
consciousness_theory_id resolved against real PKs (substrate-unreachable ->
warning, not error); deferred keys surfaced as warnings.
- 24 DB-free unit tests (httpx mocked); full suite 160 passed / 36 skipped.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a Supabase-backed VoiceRegistry module to flute-gateway for caching "S1" voice profiles, with provider/param translation and capability/grounding validation. Integrates registry-based voice slug resolution into synthesize endpoints and lifespan management, adds ChangesVoice Profile Registry
Sequence Diagram(s)sequenceDiagram
participant Client
participant FluteGateway
participant VoiceRegistry
participant Supabase
Client->>FluteGateway: POST /v1/voice/synthesize (voice slug)
FluteGateway->>VoiceRegistry: get(name)
alt profile found
VoiceRegistry-->>FluteGateway: provider/engine/voice
else not found
FluteGateway->>FluteGateway: fallback persona/intent resolution
end
FluteGateway-->>Client: synthesized audio
Note over FluteGateway,VoiceRegistry: Startup
FluteGateway->>VoiceRegistry: load()
VoiceRegistry->>Supabase: fetch active voice_profiles
Supabase-->>VoiceRegistry: rows
loop TTL loop
VoiceRegistry->>Supabase: refresh()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2df2139ca9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
pmoves/services/flute-gateway/tests/test_voice_profiles.py (1)
1-524: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMissing coverage for NATS invalidation and TTL refresh task.
The suite thoroughly covers
select_provider_and_params,load()/refresh()degradation paths, request mutation, and all the new HTTP endpoints. However, per the PR objectives the registry is described as "kept fresh with a TTL task and NATS subscription" for invalidation — neither the NATS subscribe-and-invalidate callback nor the TTL maintenance loop is exercised anywhere in this file.Given this is the dedicated test-suite layer for the registry, consider adding at least one test that simulates a NATS invalidation message triggering a cache refresh/clear, mirroring the existing
_FakeClientpattern for the underlying HTTP fetch.🤖 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 `@pmoves/services/flute-gateway/tests/test_voice_profiles.py` around lines 1 - 524, Add coverage for the registry’s NATS invalidation and TTL refresh behavior, since `VoiceRegistry` is only tested for HTTP load/refresh today. Introduce tests around the NATS subscription/invalidation callback and the TTL maintenance task so a simulated message causes the cache to clear or refresh as expected. Use the existing `VoiceRegistry`, `_FakeClient`, and `vr` monkeypatch patterns to keep it DB-free and to locate the relevant hook/task code even if names shift.pmoves/services/flute-gateway/voice_registry.py (1)
288-288: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNarrow the startup subscription guard. Catch
nats.errors.Error(or the specific NATS subscribe exceptions you expect) instead ofException; if the broad guard is intentional, add# noqa: BLE001with the startup rationale.🤖 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 `@pmoves/services/flute-gateway/voice_registry.py` at line 288, The startup subscription guard in voice_registry.py is too broad because the exception handler around the NATS subscription path catches Exception. Update the handler in the startup subscription logic to catch nats.errors.Error, or the specific NATS subscribe exception types expected by the code, so only subscription failures are handled. If you intentionally need the broad catch in this startup path, keep the existing structure but add # noqa: BLE001 and a brief startup-specific rationale in the same except block.Source: Linters/SAST tools
🤖 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 `@pmoves/services/flute-gateway/main.py`:
- Around line 1410-1415: The current write path in the voice profile handler
only warms the local `voice_registry`, so other instances stay stale; after the
Supabase write succeeds in the `main.py` flow around `VoiceProfile.from_row` and
`voice_registry.upsert_local`, publish an invalidation/update event to
`REGISTRY_UPDATE_SUBJECT`. Build the event payload using the schema in
`services/common/events.py`, validate it before publishing, and send it via the
existing NATS pub/sub path so all subscribers to `voice.registry.update.v1`
receive the update.
- Around line 283-285: The request handling in the voice update path is
bypassing the registry health gate because it still calls
voice_registry.get(request.voice) even when the registry is unhealthy. Update
the guard in the request mutation flow around the voice_registry lookup so it
no-ops when voice_registry.healthy is false, or explicitly set the registry to
healthy only after a trusted successful refresh/write. Use the existing symbols
voice_registry, request.voice, and the voice_registry.get path to locate the
fix.
- Line 446: The VoiceProfileIn.name field currently only documents the slug
format, so invalid values can still be accepted and persisted by the
/v1/voice/profiles flow. Add actual field-level validation on
VoiceProfileIn.name in main.py using the existing name field definition so it
enforces the 3–64 character length and the allowed slug pattern before any
Supabase write occurs. Keep the constraint close to the model that feeds the
create/profile endpoint so the contract is enforced at input time, not just
described.
In `@pmoves/services/flute-gateway/voice_registry.py`:
- Around line 412-427: In `_resolve_pks()` in `voice_registry.py`, do not return
None for PostgREST 4xx validation/query failures; reserve None only for
transport/network or 5xx substrate errors. Update the httpx call handling so
malformed `persona_ids` or `consciousness_theory_id` queries fail closed by
raising/propagating an error on non-200 4xx responses, while keeping the
existing None fallback for `httpx.HTTPError` and other unrecoverable
connectivity issues.
- Around line 385-387: The Ultimate-TTS profile validation in
select_provider_and_params() is too permissive because it only checks
engine_specific.primary_engine and can still accept a profile that resolves
without any voice. Update the validation branch for engine == "ultimate_tts" to
also require the selected voice key derived from the primary engine (the same
key used later to read engine_specific[f"{primary_engine}_voice"]), and append
an error when that engine-specific voice is missing so invalid profiles are
rejected before persisting.
- Line 40: The VOICE_REGISTRY_TTL_SECONDS parsing in voice_registry.py can raise
during module import and crash the gateway, so move the conversion out of the
top-level DEFAULT_TTL_SECONDS assignment or wrap it in safe parsing with a
fallback. Update the module-level config handling around DEFAULT_TTL_SECONDS to
catch invalid values, log or ignore malformed input, and use the default TTL
instead so imports always succeed.
---
Nitpick comments:
In `@pmoves/services/flute-gateway/tests/test_voice_profiles.py`:
- Around line 1-524: Add coverage for the registry’s NATS invalidation and TTL
refresh behavior, since `VoiceRegistry` is only tested for HTTP load/refresh
today. Introduce tests around the NATS subscription/invalidation callback and
the TTL maintenance task so a simulated message causes the cache to clear or
refresh as expected. Use the existing `VoiceRegistry`, `_FakeClient`, and `vr`
monkeypatch patterns to keep it DB-free and to locate the relevant hook/task
code even if names shift.
In `@pmoves/services/flute-gateway/voice_registry.py`:
- Line 288: The startup subscription guard in voice_registry.py is too broad
because the exception handler around the NATS subscription path catches
Exception. Update the handler in the startup subscription logic to catch
nats.errors.Error, or the specific NATS subscribe exception types expected by
the code, so only subscription failures are handled. If you intentionally need
the broad catch in this startup path, keep the existing structure but add #
noqa: BLE001 and a brief startup-specific rationale in the same except block.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1a39ea20-59f1-4373-b4ec-07327a4d0d63
📒 Files selected for processing (3)
pmoves/services/flute-gateway/main.pypmoves/services/flute-gateway/tests/test_voice_profiles.pypmoves/services/flute-gateway/voice_registry.py
|
Triage — coderabbitai[bot] review (PR #1922, commit 2df2139) P1 findings (act via
P2: 1, P3: 1 |
- voice_registry.load(): add `deleted_at=is.null` so the service-role read honors the soft-delete lifecycle (RLS predicate is bypassed by service key). - _resolve_voice_profile hook: gate on `voice_registry.healthy` (don't read the cache when unhealthy); CLEAR request.voice after a registry match unless a provider-native voice was resolved (never leak the slug as preset/ref_audio). - validate_grounding: validate persona UUIDs locally + distinguish PostgREST 4xx (hard reject) from network/5xx (transient warning) so malformed grounding IDs no longer slip through /validate and profile creation as warnings. - validate_capability(ultimate_tts): also require engine_specific.<primary>_voice (the key select_provider_and_params reads), so voice-less profiles are rejected. - VoiceProfileIn.name: enforce the v5_16 slug pattern with pydantic v2 `pattern=` (was description-only) so bad slugs can't be persisted. - create_voice_profile: publish a fire-and-forget invalidation on voice.registry.update.v1 after a successful write so peer gateway instances refresh instead of waiting for TTL. - _env_int guard: malformed VOICE_REGISTRY_TTL_SECONDS no longer crashes import. - Tests: +9 (soft-delete filter, slug-clear, health-gate no-op, malformed-UUID reject, 400 client-error reject, ultimate_tts voice-key capability, TTL guard, name-pattern 422, invalidation publish). Suite: 169 passed / 36 skipped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rypoint fix Scope note: submodule gitlink promotion removed from this PR — origin/main moved ahead (#1922/#1923/#1925) and the 21 pins were rollbacks/sideways relative to the new base. Submodule promotion will be handled separately after pins are aligned with origin/main. Changes kept: - All non-submodule worktree deltas from Agent Zero SPARK (migrations, pmoves/Makefile is superseded by origin/main #1924, generated kong.yml, tokenism-simulator files, etc.). - Kong entrypoint fix: /bin/bash + /dev/tcp wait loop and migrations bootstrap/up fallback (compatible with origin/main #1924 network/bind changes). - SQL lint allowlist for legacy worktree-delta migrations with to-anon / USING(true) patterns.
Voice S1 (flute-gateway side): a lifespan-preloaded, TTL-cached, NATS-invalidated
voice_profilesregistry that resolves a requested voice slug →(provider, engine, voice)and exposes/v1/voice/profiles+/v1/voice/validate. Built ultracode (understand → design → adversarial gap-check → implement → review).What
voice_registry.py(nomainimport):VoiceProfile+VoiceRegistry(load/refresh/TTL/NATS-subscribe),select_provider_and_params, grounding//validatehelpers.main.py:_resolve_voice_profilehook wired at all 3 dispatch sites; lifespan preload + TTL task + NATS subscribe (+ shutdown);GET/POST /v1/voice/profiles,GET /v1/voice/profiles/{name},GET/POST /v1/voice/validate; config advertisesvoice_profiles.The 4 correctness fixes the adversarial gap-check caught (and this honors)
synth_kwargs— providers have fixed signatures;select_provider_and_paramsdoes explicit per-engine translation (vibevoicevoice_preset→voice; voiceboxprofile_id/voice_type→voice+engine; ultimate_ttsprimary_engine/<engine>_voice→engine+voice; omnivoiceref_audio/instruct→voice+engine).request.provider/engine/voice(not a local) beforeprovider_name = request.provider or DEFAULT_PROVIDER, and short-circuits the persona path so a matched voice slug wins.engine_specificmapping (no passthrough fiction).Accept-Profile: pmoves_core(reads) /Content-Profile(writes) onvoice_profiles/personas/consciousness_theories, else PostgREST 404s.Graceful degradation
VoiceRegistry.load()never raises — httpx error / 404 / table-absent (PGRST205/42P01) / missing creds → empty cache,_healthy=False, WARNING, and the synthesize hook no-ops → existingDEFAULT_PROVIDERpath unchanged. So this is safe to merge before v5_16 is applied (Z890 DB lane); it activates automatically oncevoice_profilesexists.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes