fix(review): address 12 CodeRabbit findings from PRs #1066, #1069 - #1070
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRenames and reorders Ultimate-TTS engine display names (13→14), migrates provider/tests to Gradio 4.x /gradio_api/call SSE semantics, adjusts CLI/test defaults and exit thresholds, adds Docker host resolution, tightens Supabase error handling, removes an obsolete Prometheus target, and updates related docs and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Provider as UltimateTTSProvider
participant Gradio as Gradio Server (/gradio_api/call)
participant Storage as Gradio Result (GET)
Client->>Provider: POST /synthesize (build params)
Provider->>Gradio: POST /gradio_api/call/<endpoint> (returns {"event_id": id})
Gradio-->>Provider: 200 {"event_id": id}
Provider->>Gradio: open SSE /gradio_api/call/<endpoint>/<event_id> (stream)
Gradio-->>Provider: "event: process" / partial SSE events
Gradio-->>Provider: "event: done" / final SSE event
Provider->>Storage: GET /gradio_api/.../result.wav
Storage-->>Provider: 200 (WAV bytes)
Provider-->>Client: 200 + audio
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 2
🧹 Nitpick comments (1)
pmoves/services/botz-gateway/main.py (1)
595-599: Address static analysis: narrow exception type and chain the cause.The bare
Exceptioncatch is overly broad and the raisedHTTPExceptionloses the original traceback. Static analysis (Ruff BLE001, B904) correctly flags both issues.♻️ Proposed fix
- except Exception as exc: - raise HTTPException( - status_code=502, - detail=f"Supabase lookup failed: {exc}", - ) + except httpx.RequestError as exc: + raise HTTPException( + status_code=502, + detail=f"Supabase lookup failed: {exc}", + ) from exc
httpx.RequestErroris the base class for connection/timeout errors. If you also want to catch JSON decode errors from a malformed response, add a separate handler forValueErrororhttpx.DecodingError.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pmoves/services/botz-gateway/main.py` around lines 595 - 599, Replace the broad "except Exception as exc:" handler guarding the Supabase lookup with specific exception handlers: catch httpx.RequestError for network/timeout issues and separately catch httpx.DecodingError or ValueError for JSON/decoding failures, and when re-raising HTTPException (status_code=502) include the original exception via "from exc" to preserve the traceback; keep the HTTPException.detail text (e.g., f"Supabase lookup failed: {exc}") but raise it using "raise HTTPException(...) from exc".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pmoves/monitoring/prometheus/prometheus.yml`:
- Line 140: Remove or archive the stale Grafana dashboard and any alerts that
reference the removed scrape target: delete or move
pmoves/monitoring/grafana/dashboards/messaging-gateway.json (and any siblings)
and any alert rules querying job="messaging-gateway" or metrics matching n_*;
search the repo for "job=\"messaging-gateway\"" and "n_" metric patterns to find
and update/remove related dashboards and alert rules so no remaining assets
silently show “No Data.”
In `@pmoves/services/flute-gateway/main.py`:
- Line 130: The ULTIMATE_TTS_URL default in main.py is hardcoded to the wrong
host/port; update the os.getenv call that defines ULTIMATE_TTS_URL so its
default matches the docker-compose/README setting
("http://host.docker.internal:7860") instead of
"http://ultimate-tts-studio:7861"—locate the ULTIMATE_TTS_URL assignment in
pmoves/services/flute-gateway/main.py and replace the default string accordingly
so environment overrides still apply.
---
Nitpick comments:
In `@pmoves/services/botz-gateway/main.py`:
- Around line 595-599: Replace the broad "except Exception as exc:" handler
guarding the Supabase lookup with specific exception handlers: catch
httpx.RequestError for network/timeout issues and separately catch
httpx.DecodingError or ValueError for JSON/decoding failures, and when
re-raising HTTPException (status_code=502) include the original exception via
"from exc" to preserve the traceback; keep the HTTPException.detail text (e.g.,
f"Supabase lookup failed: {exc}") but raise it using "raise HTTPException(...)
from exc".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cffa0e23-7ffe-4f55-b748-93f1d733657f
📒 Files selected for processing (11)
.claude/CLAUDE.md.claude/commands/tts/test-engine.mdpmoves/configs/tac_trees/voice-agents.tac.yamlpmoves/configs/tts-engine-capabilities.yamlpmoves/monitoring/prometheus/prometheus.ymlpmoves/scripts/cast_tts.pypmoves/services/botz-gateway/main.pypmoves/services/cast-tts-gateway/docker-compose.ymlpmoves/services/flute-gateway/main.pypmoves/services/flute-gateway/providers/ultimate_tts.pypmoves/tools/test_all_tts_engines.py
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pmoves/services/flute-gateway/tests/test_ultimate_tts.py (1)
241-247: Minor: URL extraction relies on positional argument.The
mock_streamfunction extracts the URL fromargs[1], which assumes the first positional argument is the method and second is the URL. This matcheshttpx.AsyncClient.stream(method, url, ...)but could be fragile if the mock is ever used differently.Consider using keyword arguments for clarity:
def mock_stream(method, url, **kwargs): if "generate_unified_tts" in url: return _MockSSEStream(synth_sse) return _MockSSEStream(load_sse)This is acceptable for current test purposes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pmoves/services/flute-gateway/tests/test_ultimate_tts.py` around lines 241 - 247, The mock_stream currently reads the URL from args[1], which is fragile; update its signature to explicitly accept method and url (e.g., def mock_stream(method, url, **kwargs):) and use the url parameter to decide which _MockSSEStream to return (return _MockSSEStream(synth_sse) if "generate_unified_tts" in url else _MockSSEStream(load_sse)), keeping mock_client.stream = MagicMock(side_effect=mock_stream) unchanged so tests remain clear and robust.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pmoves/services/flute-gateway/tests/test_ultimate_tts.py`:
- Around line 241-247: The mock_stream currently reads the URL from args[1],
which is fragile; update its signature to explicitly accept method and url
(e.g., def mock_stream(method, url, **kwargs):) and use the url parameter to
decide which _MockSSEStream to return (return _MockSSEStream(synth_sse) if
"generate_unified_tts" in url else _MockSSEStream(load_sse)), keeping
mock_client.stream = MagicMock(side_effect=mock_stream) unchanged so tests
remain clear and robust.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 830939d1-e968-42e1-9696-2627bf207386
📒 Files selected for processing (1)
pmoves/services/flute-gateway/tests/test_ultimate_tts.py
Critical: - Remove dead messaging-gateway Prometheus scrape target - Fix flute-gateway ULTIMATE_TTS_URL default (container=7861, host=7860) Major: - .claude/CLAUDE.md: canonical engine names, 13→14 count - test-engine.md: remove nonexistent --synth flag, use --load-only - voice-agents.tac: /api/ → /gradio_api/call/ for Gradio 4.x - tts-engine-capabilities.yaml: Fish Speech→S1, Qwen3→Voice Design - cast_tts.py: port 7861→7860 in docstring - cast-tts compose: add host.docker.internal extra_hosts mapping - botz-gateway: raise 502/404 instead of fake "unknown" identity - test_all_tts_engines.py: fix display names, respect env URL over pterm, exit 1 when <50% engines pass - ultimate_tts.py: docstring 13→14 engines, add chatterbox_multilingual Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ded engines Align test mocks and assertions with the provider changes from PRs #1066 and #1069: event-based API (POST→event_id, GET→SSE), 14 engines (was 7), 121 params (was 92), updated voice indices and default port. Fixes all 16 CI test failures in tests (3.11). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
POWERFULMOVES
force-pushed
the
fix/coderabbit-review-sweep-1066-1069
branch
from
March 23, 2026 12:23
2b86557 to
40a3d8a
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pmoves/tools/test_all_tts_engines.py`:
- Line 105: The module header docstring still lists old engine display names
while the ENGINES mapping (variable ENGINES and its entries like the "name"
field for engines such as "Fish Speech S1") was updated; update the top-of-file
docstring to match the new display names used in ENGINES and ensure any other
header occurrences (the second header around the other docstring near the second
occurrence) are changed consistently; search for the old names in the file and
replace them with the current display names from ENGINES so the header text and
examples match the actual engine names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7d9f0a5b-4792-4fbe-a16e-061f0cfae410
📒 Files selected for processing (12)
.claude/CLAUDE.md.claude/commands/tts/test-engine.mdpmoves/configs/tac_trees/voice-agents.tac.yamlpmoves/configs/tts-engine-capabilities.yamlpmoves/monitoring/prometheus/prometheus.ymlpmoves/scripts/cast_tts.pypmoves/services/botz-gateway/main.pypmoves/services/cast-tts-gateway/docker-compose.ymlpmoves/services/flute-gateway/main.pypmoves/services/flute-gateway/providers/ultimate_tts.pypmoves/services/flute-gateway/tests/test_ultimate_tts.pypmoves/tools/test_all_tts_engines.py
✅ Files skipped from review due to trivial changes (7)
- pmoves/scripts/cast_tts.py
- .claude/CLAUDE.md
- pmoves/services/flute-gateway/main.py
- pmoves/services/cast-tts-gateway/docker-compose.yml
- pmoves/monitoring/prometheus/prometheus.yml
- pmoves/services/flute-gateway/providers/ultimate_tts.py
- pmoves/configs/tts-engine-capabilities.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- pmoves/services/botz-gateway/main.py
- pmoves/configs/tac_trees/voice-agents.tac.yaml
- .claude/commands/tts/test-engine.md
- pmoves/services/flute-gateway/tests/test_ultimate_tts.py
…ard replace, mock params - ULTIMATE_TTS_URL: fix port regression (7861→7860), switch to host.docker.internal for native Pinokio path (env var override for Docker) - Replace stale messaging-gateway dashboard with Cast-TTS-Gateway dashboard (8 metrics: requests, latency, device discovery, circuit breaker, cache, fallback providers, voice profiles, scheduler) - test_ultimate_tts: use explicit named params in mock_stream signatures - botz-gateway: narrow except Exception to httpx.RequestError + chain via from exc Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
POWERFULMOVES
force-pushed
the
fix/coderabbit-review-sweep-1066-1069
branch
from
March 23, 2026 12:46
40a3d8a to
10f4841
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
pmoves/services/flute-gateway/tests/test_ultimate_tts.py (1)
218-246: Assert the returnedevent_idin the stream poll URL.
post()now returns"mock-evt-123", butmock_stream()never checks that the same ID is used in the follow-up poll. A regression that drops or hardcodes the event id would still pass every happy-path synthesis test that uses this helper.♻️ Tighten the mock with a cheap assertion
- mock_event_response.json.return_value = {"event_id": "mock-evt-123"} + event_id = "mock-evt-123" + mock_event_response.json.return_value = {"event_id": event_id} ... def mock_stream(method, url, **kwargs): + url = str(url) + assert url.endswith(f"/{event_id}"), f"unexpected SSE poll URL: {url}" if "generate_unified_tts" in url: return _MockSSEStream(synth_sse) return _MockSSEStream(load_sse)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pmoves/services/flute-gateway/tests/test_ultimate_tts.py` around lines 218 - 246, The mock for the SSE poll never verifies that the POST-returned event_id is actually used, so update the test helper: capture the event id returned by mock_client.post (currently mock_event_response.json() -> "mock-evt-123") and have mock_stream (or its URL-matching branch for generate_unified_tts) assert that the incoming poll URL contains that same event id (e.g., contains "mock-evt-123" or an "event_id" query param equal to it) before returning _MockSSEStream; reference mock_client.post, mock_event_response.json, mock_stream and _MockSSEStream when making this check.pmoves/monitoring/grafana/dashboards/messaging-gateway.json (1)
43-43: Use$__rate_intervalinstead of fixed[5m]windows in rate queries.Fixed windows don't adapt when users zoom in or out on the dashboard. Grafana's
$__rate_intervalautomatically scales with the visible range and scrape interval, making these queries more flexible and reliable.♻️ Suggested pattern
- { "expr": "sum(rate(cast_tts_requests_total[5m])) by (method)", "legendFormat": "{{method}}" } + { "expr": "sum(rate(cast_tts_requests_total[$__rate_interval])) by (method)", "legendFormat": "{{method}}" }Also applies to: 51–51, 82–83, 91–93, 109–111, 126–126, 135–135, 144–146, 168–168, 177–177, 186–188
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pmoves/monitoring/grafana/dashboards/messaging-gateway.json` at line 43, The Prometheus rate queries in the dashboard use fixed windows like [5m] (e.g., the target expression "histogram_quantile(0.95, sum(rate(cast_tts_latency_seconds_bucket[5m])) by (le))" and other similar expressions) which should be replaced with Grafana's dynamic $__rate_interval; update every rate(...) and increase(...) window that currently uses a fixed duration (all instances of "[5m]" and other fixed brackets referenced in the comment) to use "[$__rate_interval]" so the queries scale with zoom/scrape interval (ensure you update each expression: histogram_quantile(... rate(...[5m]) ...), sum(rate(...[5m])) by (...), and any other places listed in the comment).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pmoves/monitoring/grafana/dashboards/messaging-gateway.json`:
- Line 6: The "uid" field was changed from "messaging-gateway" to
"cast-tts-gateway", which will break existing Grafana URLs/bookmarks and cause
Grafana to treat it as a new dashboard; either revert the "uid" value back to
"messaging-gateway" to preserve continuity or, if this is an intentional
migration, confirm and document the migration plan and update all external
links/automation that reference the old UID ("messaging-gateway") to the new UID
("cast-tts-gateway") and ensure provisioned dashboard cleanup/rename behavior is
handled.
- Around line 65-69: The "Cache Hit Rate" stat uses lifetime totals
(sum(cast_cache_operations_total{operation="hit"}) / ...), which hides recent
changes and can be inconsistent; change the target to use a windowed rate like
sum(rate(cast_cache_operations_total{operation="hit"}[5m])) /
(sum(rate(cast_cache_operations_total{operation="hit"}[5m])) +
sum(rate(cast_cache_operations_total{operation="miss"}[5m]))) * 100, and add
zero-safe handling (e.g., coalesce/divide-by-zero protection) so the panel
"Cache Hit Rate" and its reduceOptions/fieldConfig (e.g., lastNotNull) show
recent hit-rate similar to the other panels that use rate(...[5m]).
In `@pmoves/services/botz-gateway/main.py`:
- Around line 596-604: The 502 handlers currently expose upstream internals by
embedding the raw exception (exc) and response.status_code into the
HTTPException detail; change both raises to return a generic client-facing
message (e.g., "Upstream lookup failed") while logging the full upstream details
server-side using the existing logger (log the exc, response.status_code and
response.text/response.content where available) before raising HTTPException;
update the two raise sites that use HTTPException (the block referencing exc and
the block checking response.status_code) to use the generic detail and ensure
you call logger.error(...) or logger.exception(...) with the full diagnostics.
---
Nitpick comments:
In `@pmoves/monitoring/grafana/dashboards/messaging-gateway.json`:
- Line 43: The Prometheus rate queries in the dashboard use fixed windows like
[5m] (e.g., the target expression "histogram_quantile(0.95,
sum(rate(cast_tts_latency_seconds_bucket[5m])) by (le))" and other similar
expressions) which should be replaced with Grafana's dynamic $__rate_interval;
update every rate(...) and increase(...) window that currently uses a fixed
duration (all instances of "[5m]" and other fixed brackets referenced in the
comment) to use "[$__rate_interval]" so the queries scale with zoom/scrape
interval (ensure you update each expression: histogram_quantile(...
rate(...[5m]) ...), sum(rate(...[5m])) by (...), and any other places listed in
the comment).
In `@pmoves/services/flute-gateway/tests/test_ultimate_tts.py`:
- Around line 218-246: The mock for the SSE poll never verifies that the
POST-returned event_id is actually used, so update the test helper: capture the
event id returned by mock_client.post (currently mock_event_response.json() ->
"mock-evt-123") and have mock_stream (or its URL-matching branch for
generate_unified_tts) assert that the incoming poll URL contains that same event
id (e.g., contains "mock-evt-123" or an "event_id" query param equal to it)
before returning _MockSSEStream; reference mock_client.post,
mock_event_response.json, mock_stream and _MockSSEStream when making this check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7518ec04-e85a-485a-b46f-d07ae807b2a9
📒 Files selected for processing (4)
pmoves/monitoring/grafana/dashboards/messaging-gateway.jsonpmoves/services/botz-gateway/main.pypmoves/services/flute-gateway/main.pypmoves/services/flute-gateway/tests/test_ultimate_tts.py
✅ Files skipped from review due to trivial changes (1)
- pmoves/services/flute-gateway/main.py
- test_all_tts_engines: fix docstring engine names (Kokoro TTS, Fish Speech S1, Qwen Voice Design) to match ENGINES dict - cast-tts dashboard: use rate() + $__rate_interval + > 0 guard for cache hit ratio PromQL (avoids divide-by-zero) - botz-gateway: stop leaking Supabase internals in 502 detail — log to server, return generic "Upstream service unavailable/error" to clients Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pmoves/tools/test_all_tts_engines.py`:
- Around line 698-700: The current skip_pterm logic checks args.url !=
DEFAULT_URL to infer whether --url was supplied, which fails if the user
explicitly passes the default value; update the condition to detect the presence
of the flag itself (e.g. check sys.argv for "--url" or a short form like "-u"
and also for "--url=" style usage) instead of comparing values. Concretely,
replace the args.url != DEFAULT_URL part in the skip_pterm assignment with a
boolean such as ("--url" in sys.argv or any(arg.startswith("--url=") for arg in
sys.argv) or "-u" in sys.argv), keeping the existing env_url_set and
args.no_pterm checks and using the same symbols env_url_set, skip_pterm,
args.url, DEFAULT_URL, and args.no_pterm.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e0930a6e-c9f6-4cb0-8ab2-0fd3fa90c86a
📒 Files selected for processing (3)
pmoves/monitoring/grafana/dashboards/messaging-gateway.jsonpmoves/services/botz-gateway/main.pypmoves/tools/test_all_tts_engines.py
✅ Files skipped from review due to trivial changes (1)
- pmoves/monitoring/grafana/dashboards/messaging-gateway.json
🚧 Files skipped from review as they are similar to previous changes (1)
- pmoves/services/botz-gateway/main.py
Comment on lines
+698
to
+700
| # Skip pterm if user explicitly provided --url or set ULTIMATE_TTS_URL env var | ||
| env_url_set = "ULTIMATE_TTS_URL" in os.environ | ||
| skip_pterm = args.no_pterm or (args.url != DEFAULT_URL) or env_url_set |
Contributor
There was a problem hiding this comment.
Explicit --url detection is value-based, not flag-based.
Line 700 uses args.url != DEFAULT_URL to infer whether --url was provided. If a user explicitly passes the same value as default, pterm preflight can still run and override the intended target URL.
Suggested patch
- parser.add_argument("--url", type=str, default=DEFAULT_URL, help="TTS Studio URL")
+ parser.add_argument("--url", type=str, default=None, help="TTS Studio URL")
@@
- url = args.url.rstrip("/") + "/"
+ cli_url_provided = args.url is not None
+ url = (args.url or DEFAULT_URL).rstrip("/") + "/"
@@
- env_url_set = "ULTIMATE_TTS_URL" in os.environ
- skip_pterm = args.no_pterm or (args.url != DEFAULT_URL) or env_url_set
+ env_url_set = "ULTIMATE_TTS_URL" in os.environ
+ skip_pterm = args.no_pterm or cli_url_provided or env_url_set🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pmoves/tools/test_all_tts_engines.py` around lines 698 - 700, The current
skip_pterm logic checks args.url != DEFAULT_URL to infer whether --url was
supplied, which fails if the user explicitly passes the default value; update
the condition to detect the presence of the flag itself (e.g. check sys.argv for
"--url" or a short form like "-u" and also for "--url=" style usage) instead of
comparing values. Concretely, replace the args.url != DEFAULT_URL part in the
skip_pterm assignment with a boolean such as ("--url" in sys.argv or
any(arg.startswith("--url=") for arg in sys.argv) or "-u" in sys.argv), keeping
the existing env_url_set and args.no_pterm checks and using the same symbols
env_url_set, skip_pterm, args.url, DEFAULT_URL, and args.no_pterm.
POWERFULMOVES
added a commit
that referenced
this pull request
Mar 23, 2026
PHI.t1: CLAIM + RELEASE for z890 session (2026-03-22/23): - 5 PRs merged (#1063, #1064, #1068, #1069, #1070) - python3 hook fix, P7 gates, topology sanitize, CR sweep - PR #1071 open (service runners + prosodic) - 4090-claude pr-trimmed #1070 (3 follow-ups) Roadmap: 3 new claim register entries (P7 gates, CR sweep, TTS runners) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
POWERFULMOVES
added a commit
that referenced
this pull request
Mar 23, 2026
* feat(voice): TTS service runners + announcer persona + prosodic endpoint + ear spec Service runners: add always_on (KittenTTS cli-voice, Kokoro announcer) and on_demand (F5 narrator, Higgs streaming, VibeVoice podcast, IndexTTS2 expressive) config section to tts-engine-capabilities.yaml. Announcer: add broadcast-announcer persona (Kokoro am_adam, speed 0.9) to agent_signatures.yaml and voice-personas.md. Maps to system events, PR completions, deploy notifications over Cast speakers. Prosodic endpoint: wire /v1/voice/synthesize/prosodic in Flute-Gateway. Parses text through prosodic_parser, synthesizes chunks per boundary, stitches with natural pauses/crossfades, returns WAV + BPM timeline in X-Prosodic-* response headers. Prosodic ear spec: create PROSODIC_EAR_SPEC.md documenting the analysis side — pitch extraction, BPM encoding, emotion detection, NATS publishing to tokenism.prosodic.bpm.v1. Phased implementation plan (A-D). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(review): resolve 4 CodeRabbit findings on PR #1071 - agent_signatures.yaml: remove announcer (system voice, not contributor); add NOTE comment pointing to voice-personas.md + service_runners - flute-gateway prosodic endpoint: fix stitch_chunks type mismatches (bytes→numpy, ProsodicChunk→BoundaryType), track successful_chunks to prevent length mismatch, add exception chaining (from exc) - nats-subjects.md: add voice.ear.analysis.v1 + voice.ear.emotion.v1 subjects from PROSODIC_EAR_SPEC.md Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs(agents): log z890-claude session trail + roadmap claims PHI.t1: CLAIM + RELEASE for z890 session (2026-03-22/23): - 5 PRs merged (#1063, #1064, #1068, #1069, #1070) - python3 hook fix, P7 gates, topology sanitize, CR sweep - PR #1071 open (service runners + prosodic) - 4090-claude pr-trimmed #1070 (3 follow-ups) Roadmap: 3 new claim register entries (P7 gates, CR sweep, TTS runners) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(review): resolve 3 remaining CodeRabbit findings on PR #1071 - Convert numpy ndarray to proper WAV bytes before Response (CR #8 critical) - Compact X-Prosodic-Timeline header to avoid proxy size limits (CR #9 major) - Wrap individual chunk synthesis in try/except for graceful degradation (CR #7) - Move numpy import to module level Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
Follow-up sweep addressing 12 unresolved CodeRabbit review comments from merged PRs #1066 and #1069.
Test plan
Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores