fixes: surface TTS API errors, plug session leak, clip mic overdrive - #602
Conversation
Fix six verified bugs found in a multi-agent audit: - session: unregister the armed emergency-export exporter on clean stop() so stopped Sessions no longer stay pinned in the process-wide export registry - tts: read streamed error bodies before raise_for_status() in OpenAI and ElevenLabs HTTP synthesis so HTTP errors surface as HTTPStatusError with a provider Error event instead of escaping as httpx.ResponseNotRead - transports: clip float32 mic samples before the int16 cast in LocalTransport so overdriven input saturates instead of wrapping - runtime: make FrozenJournalSnapshot.latest_sequence skip negative out-of-band markers so degraded postmortem views keep the live sequence contract - cli: handle null duration_s in validate report instead of raising TypeError Each fix lands with a regression test verified to fail without it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 36 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: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThe changes harden validation output, degraded journal snapshots, session teardown, microphone PCM conversion, and streamed TTS error handling. Regression tests cover null durations, degraded markers, exporter cleanup, audio saturation, and provider HTTP errors. ChangesReport formatting
Runtime state and lifecycle cleanup
Audio input conversion
Streamed TTS error handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 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 |
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
| return self._records[-1].sequence if self._records else 0 | ||
| # Skip out-of-band markers (e.g. the degraded marker at sequence -1) so | ||
| # the postmortem value matches the live InMemoryRingBuffer counter. | ||
| return max((r.sequence for r in self._records if r.sequence >= 0), default=0) |
There was a problem hiding this comment.
With a valid InMemoryRingBuffer(capacity=1), a degraded transition appends the sequence=-1 marker into a full deque and evicts the only nonnegative record. This max(...) then returns 0 even though the live ring's _seq remains 1 (and can be higher after overflow markers), so post-stop latest_sequence still diverges from the live cursor contract. Preserve/pass the live sequence counter into FrozenJournalSnapshot rather than deriving it solely from retained records, and add a capacity=1 regression test.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@src/easycat/cli/validate.py`:
- Around line 854-857: Update _format_duration to convert numeric inputs safely,
rejecting negative, NaN, and infinite values in favor of "0.00s". Catch
OverflowError when converting excessively large integers, and preserve the
existing boolean/non-numeric fallback and two-decimal formatting for valid
finite non-negative durations.
In `@src/easycat/tts/elevenlabs_tts.py`:
- Around line 266-270: Update the streamed response checks in both
elevenlabs_tts.py lines 266-270 and openai_tts.py lines 150-154 to use the
response’s non-success condition rather than is_error, and await
response.aread() before raise_for_status(). This must cover redirects and all
other non-2xx responses so the exception handler can safely access the response
body.
In `@tests/tts/test_tts_cancellation.py`:
- Around line 47-48: Update the test-double method raise_for_status to include
the explicit -> None return annotation, preserving its existing no-op behavior.
In `@tests/tts/test_tts_elevenlabs.py`:
- Around line 433-435: Update the helper’s __aiter__ method to explicitly return
AsyncIterator[bytes], and add the required typing import. Preserve the existing
iteration over self._chunks and yielded byte chunks.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e73e84b5-a71f-40de-8524-2e07ff824f41
📒 Files selected for processing (13)
src/easycat/cli/validate.pysrc/easycat/runtime/journal_views.pysrc/easycat/session/_session.pysrc/easycat/transports/local.pysrc/easycat/tts/elevenlabs_tts.pysrc/easycat/tts/openai_tts.pytests/cli/test_validate_report_cli.pytests/runtime/test_journal.pytests/session/test_session_lifecycle_teardown.pytests/transports/test_local_transport.pytests/tts/test_tts_cancellation.pytests/tts/test_tts_elevenlabs.pytests/tts/test_tts_openai.py
1 participant
Summary
Fixes six verified bugs found in a multi-agent audit of the codebase. Each finding was adversarially verified (3 independent reviewers per finding) before fixing, and every fix lands with a regression test confirmed to fail without it.
High severity
Session.stop()never unregistered the armed emergency-export exporter, so the exporter closure (holding a strong reference to the Session) stayed pinned in the process-wide export registry after a clean stop — an unbounded leak of stopped Sessions in long-running servers.stop()now unregisters after_mark_closed(); the unregister is idempotent and uninstalls the shared excepthook/atexit hooks when it drains the last entry.HTTPStatusErrorhandler'sexc.response.textraisedhttpx.ResponseNotReadbecause the streamed body was never read — the real API error escaped as a stream error and no providerErrorevent reached the journal. The body is now read while the stream is still open (await response.aread()beforeraise_for_status(); reading in the handler would hitStreamClosedsince the context manager closes the stream first).Low severity
LocalTransportconverted float32 mic buffers with(arr * 32767).astype(np.int16)and no clip, so overdriven samples (>1.0) sign-flipped into opposite-polarity clicks feeding AEC/VAD/STT. Now clips to int16 range like the resampler sites.FrozenJournalSnapshot.latest_sequencereturned the degraded-mode out-of-band marker's sequence (-1) for a degraded in-memory postmortem view, breaking cursor math across thestop()boundary. Now skips negative markers, matching the SQLite path'sMAX(sequence)behavior and the "0 when empty" contract.validate reportraisedTypeErroron a report whoseduration_sis JSON null. Now handled.Test plan
httpx.MockTransportfor the two TTS bugs (the existing MagicMock fakes can't reproduce streaming semantics)uv run pytest tests/session tests/transports tests/tts tests/runtime tests/cli -m 'not integration_external'— 2340 passeduv run ruff check .anduv run ruff format --check .— clean🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests