feat(voice): S1a — unified voice_profiles registry schema (v5_16) - #1890
Conversation
|
Warning Review limit reached
More reviews will be available in 30 minutes and 52 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate 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 see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA new SQL migration ( ChangesVoice Catalog SQL Migration
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)
✨ 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: f260164c70
ℹ️ 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".
|
Triage -- chatgpt-codex-connector[bot] review on PR #1890
P2: 0, P3: 0 Act via |
First Voice-Agents stage (spec §3/§4/§8/§10-Q9). Source-of-truth + routing table for flute-gateway voice selection across ALL engines (omnivoice/vibevoice/ voicebox/ultimate_tts). One row = one resolvable voice; engines keep native profile storage; audio on the shared JuiceFS/MinIO catalog (ref_audio_path). - pmoves_core.voice_profiles: identity(name slug/tags) + routing(engine, engine_specific JSONB) + media(ref_audio_path, sample_rate) + provenance/rights + multitenancy(created_by, is_public, is_active, soft-delete) with CHECK constraints (slug, engine enum, rights_basis enum) + GIN tags index. - pmoves_core.voice_profile_grants: per-user use/clone sharing (Q9). - RLS (Q9): service-role bypass; public/owner/granted read; owner write; grant policies. updated_at trigger. PostgREST grants (mirror v5_13). NOT cast_voice_profiles (device-scoped, untouched). Schema only — flute-gateway loader + endpoints follow as S1b. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
voice_profiles_read's grant-EXISTS read voice_profile_grants, whose owner policy read voice_profiles back → recursive policy dependency for authenticated reads. Fix: denormalize owner_id onto voice_profile_grants (set by a SECURITY DEFINER trigger that bypasses RLS), and make grants_owner policy check owner_id directly — grants policies no longer read voice_profiles. Add grantee-read policy. P1-A (apply path) tracked separately — see PR thread. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…onsciousness) A voice is part of an agent's grounded identity (MOF/prosodic), not a bare clip. New `grounding` JSONB ties a voice to the grounded-persona (v5_12/v5_14) + consciousness-shape (v5_15) substrate agents retrieve at startup — supporting voices grounded in a MIX of paradigm leaders/proponents and mapping a social-media personality back to a consciousness shape (the grounded→YouTube spectrum). Per convergence with SPARK's agent-registry discovery (#1893/#1894) + Z890's DB lane. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
b9b0816 to
cabc08b
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/db/v5_16_voice_catalog.sql`:
- Around line 20-36: Add CHECK constraints to the v5_16_voice_catalog table so
the registry JSON/media shapes stay valid: enforce that tags is a JSON array,
engine_specific and grounding are JSON objects, and keep the existing media
fields compatible with filtering/routing expectations. Update the table
definition near the tags/engine_specific/grounding columns, and mirror the same
validation pattern anywhere else this schema is repeated in the file (including
the referenced later sections) so authenticated writers cannot persist malformed
payloads.
- Line 48: The voice catalog table uses deleted_at as a soft-delete lifecycle
contract, but the current owner permissions still allow hard DELETEs via broad
FOR ALL grants and related policies. Update the grants/policies around the voice
catalog table so authenticated owners can only perform soft-delete behavior and
cannot hard-delete rows; locate the changes in the table definition and the
associated policy/grant blocks for the same object. Keep the ownership path
intact while splitting out DELETE from the broader permissions so
retention/provenance is preserved.
- Line 43: The denormalized owner field is not kept in sync when voice profile
ownership changes, so update the voice catalog migration to either make
voice_profiles.created_by immutable or add a sync mechanism that updates
owner_id whenever created_by changes. Use the voice_profiles and owner_id
columns in the affected table definitions and any related RLS/grant logic so the
authoritative ownership value and the denormalized value cannot diverge.
- Around line 115-129: The `voice_profiles_read` policy references
`pmoves_core.voice_profile_grants` in its EXISTS check, so anon users also need
read access there or they will be blocked despite the policy. Update the SQL in
`v5_16_voice_catalog.sql` near `voice_profiles_read` to either grant anon
`SELECT` on `pmoves_core.voice_profile_grants` (with RLS still enforcing row
filtering) or remove anon from the `voice_profiles` read grant path, keeping the
change consistent with the existing policy definitions.
🪄 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: d6259293-d36d-42b9-9b72-275d264eed05
📒 Files selected for processing (1)
pmoves/db/v5_16_voice_catalog.sql
Author (4090) resolving the RLS/schema-design threads on the security migration (Z890 keeps the DB-apply lane; deploy spine validates live): - CHECK constraints: tags=array, engine_specific/grounding=object, sample_rate>0, audio_duration>=0 — no malformed payloads breaking filtering/routing. - created_by IMMUTABLE (trigger) — keeps the denormalized grants.owner_id in sync (ownership transfer = deliberate service-role flow, out of S1 scope). - Soft-delete contract enforced: split owner FOR ALL → owner_insert + owner_update (no owner hard-DELETE); GRANT DELETE to service_role only — preserves retention/provenance + grant-cascade safety. - anon-read: existing read policy already gates anon to is_public only (no jwt sub → owner/grant checks NULL); live anon-read test runs on the deploy spine. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…() (P1) Convergence pair-review caught it: the policies used the deprecated PostgREST request.jwt.claim.sub/.role GUCs (removed in PostgREST 9.0; not populated by the Supabase this repo runs) → every owner/grant check failed closed, service_bypass was dead code (worked only via service_role BYPASSRLS). Switch all to the repo standard used in the applied migrations (channel_monitor, geometry_bus_rls): auth.uid() for owner/sub, jwt_claim_role() for role. Also: drop duplicated grantee_read policy; align grounding keys to real substrate PKs (persona_ids→personas.persona_id, consciousness_theory_id→consciousness_theories.id), note paradigm/proponents pinned in spec §3 + S1b validate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Triage: coderabbitai[bot] — #1890 (
P2: 0, P3: 0
|
|
Triage — coderabbitai[bot] review on PR #1890 No open P0/P1 findings — all 4 Major findings from coderabbitai[bot] are resolved in this PR's commits:
P2: 0, P3: 0 Triaged by automated triage agent. Human action: none required — verify fixes via |
|
coderabbitai triage — PR #1890 (
P2: 0, P3: 0 |
… undefined) (#1902) Follow-up to merged #1890. jwt_claim_role() is not defined anywhere in repo SQL (fork/initdb/migrations) → CREATE POLICY would abort on apply, leaving the table RLS-incomplete. The two service_bypass policies now target the Postgres service_role role directly (TO service_role USING(true) WITH CHECK(true)) — the dependency-free idiom from service_catalog.sql:73. owner/grant policies already use auth.uid() (Supabase built-in). Apply-blocking fix before Z890's DB-lane apply of v5_16. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…1c (#1896) * docs(voice): fold convergence verdict into spec — §4a discovery unification + S1c From the #1890↔#1893↔#1894 pair-review convergence. Spec changes: - §3: pin grounding JSONB as a CONTRACT (persona_ids→personas.persona_id, consciousness_theory_id→consciousness_theories.id, paradigm_proponent_ids[]); correct consciousness_shape→consciousness_theory_id; /validate enforces shape. - §4: RLS must use repo-standard auth.uid()/jwt_claim_role() (not deprecated request.jwt.claim.* GUCs) — tightens the Q9 RBAC resolution. - §4a (new): "Voice as a discoverable capability — two planes, one join key": voice_profiles stays truth/routing; a discovery shim registers voice in agent_registry (mcp.v1.voice) + a room binding; shared capability vocab; lifecycle-key policy (status vs evolution_stage); naming map; grounding promoted to a shared capability annotation (grounding_source on hirag/cipher) = the spine. - §9: add S1-gate (Z890 migration-applier path) + S1c (cross-lane discovery unification, after SPARK #1893/#1894). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(voice): RLS guidance → auth.uid() + TO service_role (jwt_claim_role undefined) Codex P2: jwt_claim_role() is not defined in repo SQL — directing S1 implementers to it would abort CREATE POLICY. Use the dependency-free idiom: auth.uid() (owner) + TO service_role (service access, service_catalog.sql:73). Avoid both the removed request.jwt.claim.* GUCs and jwt_claim_role(). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ined) (#1897) * fix(db): yt_oauth_cookies RLS uses repo-standard jwt_claim_role() (audit) Supabase-currency audit (operator-requested) follow-up to the #1890 P1 finding. The applied 20260417000000_yt_oauth_cookies.sql policy used the deprecated PostgREST GUC current_setting('request.jwt.claim.role') (removed in 9.0, not populated → dead-code policy; service_role reached the table only via BYPASSRLS). Corrective migration (don't edit applied migrations): drop + recreate yt_oauth_cookies_service_role_all with jwt_claim_role() — the repo standard used in the applied channel_monitor/geometry_bus migrations. Idempotent. Apply via the Z890 DB lane (same as v5_16; pmoves/db is not auto-replayed by apply_migrations_docker.sh — see S1-gate). cast_tts_persistence.sql also uses the deprecated GUC but is an orphan (not auto-applied) — flagged to retire, not fixed here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(db): yt_oauth_cookies policy uses TO service_role (jwt_claim_role undefined) Codex P1: jwt_claim_role() is NOT defined anywhere in repo SQL (fork/initdb/ migrations checked) — CREATE POLICY would abort after the DROP, leaving the table policy-less. Switch to the dependency-free idiom used by applied migrations (service_catalog.sql:73): FOR ALL TO service_role USING(true) WITH CHECK(true). PostgREST runs service-key requests as the service_role Postgres role. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Summary
First Voice Agents implementation stage (S1a) — the unified voice registry schema. Spec:
docs/superpowers/specs/2026-06-26-voice-agents-design.md(§3 contract, §4 registry decision, §8 provenance, §10 Q9 RBAC).pmoves_core.voice_profiles— the source-of-truth + routing table flute-gateway reads to resolve a voice across ALL engines (omnivoice/vibevoice/voicebox/ultimate_tts). One row = one resolvable voice; engines keep native profile storage; audio lives on the shared JuiceFS/MinIO catalog viaref_audio_path.nameslug,display_name,tags), routing (engine,engine_specificJSONB), media (ref_audio_path,sample_rate_hz), provenance/rights, multitenancy (created_by,is_public,is_active, soft-delete).pmoves_core.voice_profile_grants— per-user use/clone sharing (Q9).updated_attrigger. PostgREST grants mirrorv5_13_pmoves_core_rest_grants.sql.Does not touch
cast_voice_profiles(device-scoped, kept as-is). Schema only — flute-gateway loader +GET/POST /v1/voice/profiles+/validatefollow as S1b.Testing
Required Checks
Review Coordination
Follow-up
select_provider_and_params+GET/POST /v1/voice/profiles,/validate.🤖 Generated with Claude Code
Summary by CodeRabbit